Bug 168605

Summary: ASSERTION FAILED: "!scope.exception()" with Object.isSealed/isFrozen and uninitialized module bindings
Product: WebKit Reporter: André Bargull <andre.bargull>
Component: JavaScriptCoreAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, fpizlo, keith_miller, mark.lam, msaboff, saam, ysuzuki
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch saam: review+

Description André Bargull 2017-02-20 09:44:35 PST
Revision: 212634

Test case, t.js:
---
import* as self from "./t.js";

Object.isSealed(self);

Object.isFrozen(self);

export let a;
export function b(){}
---

Triggers this assertion:
---
ASSERTION FAILED: !scope.exception()
../../Source/JavaScriptCore/runtime/JSModuleNamespaceObject.cpp(130) : static bool JSC::JSModuleNamespaceObject::getOwnPropertySlot(JSC::JSObject*, JSC::ExecState*, JSC::PropertyName, JSC::PropertySlot&)
---

Stacktrace:
---
#0  0x00007ffff6dc6f98 in WTFCrash () at ../../Source/WTF/wtf/Assertions.cpp:323
#1  0x00007ffff6bc9856 in JSC::JSModuleNamespaceObject::getOwnPropertySlot (cell=0x7fffaef88110, exec=0x7fffffffca10, propertyName=..., slot=...)
    at ../../Source/JavaScriptCore/runtime/JSModuleNamespaceObject.cpp:130
#2  0x00007ffff6be471d in JSC::JSObject::getOwnPropertyDescriptor (this=0x7fffaef88110, exec=0x7fffffffca10, propertyName=..., descriptor=...) at ../../Source/JavaScriptCore/runtime/JSObject.cpp:3187
#3  0x00007ffff6c626b3 in JSC::objectConstructorIsSealed (exec=0x7fffffffca10) at ../../Source/JavaScriptCore/runtime/ObjectConstructor.cpp:642
...
---
Comment 1 Yusuke Suzuki 2017-02-21 00:16:24 PST
(In reply to comment #0)
> Revision: 212634
> 
> Test case, t.js:
> ---
> import* as self from "./t.js";
> 
> Object.isSealed(self);
> 
> Object.isFrozen(self);
> 
> export let a;
> export function b(){}
> ---
> 
> Triggers this assertion:
> ---
> ASSERTION FAILED: !scope.exception()
> ../../Source/JavaScriptCore/runtime/JSModuleNamespaceObject.cpp(130) :
> static bool JSC::JSModuleNamespaceObject::getOwnPropertySlot(JSC::JSObject*,
> JSC::ExecState*, JSC::PropertyName, JSC::PropertySlot&)
> ---
> 
> Stacktrace:
> ---
> #0  0x00007ffff6dc6f98 in WTFCrash () at
> ../../Source/WTF/wtf/Assertions.cpp:323
> #1  0x00007ffff6bc9856 in JSC::JSModuleNamespaceObject::getOwnPropertySlot
> (cell=0x7fffaef88110, exec=0x7fffffffca10, propertyName=..., slot=...)
>     at ../../Source/JavaScriptCore/runtime/JSModuleNamespaceObject.cpp:130
> #2  0x00007ffff6be471d in JSC::JSObject::getOwnPropertyDescriptor
> (this=0x7fffaef88110, exec=0x7fffffffca10, propertyName=..., descriptor=...)
> at ../../Source/JavaScriptCore/runtime/JSObject.cpp:3187
> #3  0x00007ffff6c626b3 in JSC::objectConstructorIsSealed
> (exec=0x7fffffffca10) at
> ../../Source/JavaScriptCore/runtime/ObjectConstructor.cpp:642
> ...
> ---

OK, this is because objectConstrutorIsFrozen does not check the error state when iterating property names. I'll upload the patch.
Comment 2 Yusuke Suzuki 2017-02-21 00:25:21 PST
Created attachment 302240 [details]
Patch
Comment 3 Saam Barati 2017-02-21 00:27:27 PST
Comment on attachment 302240 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=302240&action=review

r=me

> Source/JavaScriptCore/runtime/ObjectConstructor.cpp:643
> +        RETURN_IF_EXCEPTION(scope, encodedJSValue());

Style: you can use  "{ }" instead of "encodedJSValue()"

> Source/JavaScriptCore/runtime/ObjectConstructor.cpp:684
> +        RETURN_IF_EXCEPTION(scope, encodedJSValue());

ditto
Comment 4 Yusuke Suzuki 2017-02-21 01:06:24 PST
Comment on attachment 302240 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=302240&action=review

>> Source/JavaScriptCore/runtime/ObjectConstructor.cpp:643
>> +        RETURN_IF_EXCEPTION(scope, encodedJSValue());
> 
> Style: you can use  "{ }" instead of "encodedJSValue()"

Fixed.

>> Source/JavaScriptCore/runtime/ObjectConstructor.cpp:684
>> +        RETURN_IF_EXCEPTION(scope, encodedJSValue());
> 
> ditto

Fixed.
Comment 5 Yusuke Suzuki 2017-02-21 01:09:30 PST
Committed r212710: <http://trac.webkit.org/changeset/212710>
Comment 6 Mark Lam 2017-02-21 07:14:17 PST
Comment on attachment 302240 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=302240&action=review

>>> Source/JavaScriptCore/runtime/ObjectConstructor.cpp:643
>>> +        RETURN_IF_EXCEPTION(scope, encodedJSValue());
>> 
>> Style: you can use  "{ }" instead of "encodedJSValue()"
> 
> Fixed.

Actually, returning encodedJSValue() is the right thing to do because {} returns double 0 on 32-bit instead of the empty value.  That said, this is an error condition, and the client really shouldn't be using the returned value.