Bug 168605 – ASSERTION FAILED: "!scope.exception()" with Object.isSealed/isFrozen and uninitialized module bindings

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[André Bargull]

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
...
---

[Yusuke Suzuki]

(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.

[Yusuke Suzuki]

Created attachment 302240 [details]
Patch

[Saam Barati]

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

[Yusuke Suzuki]

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.

[Yusuke Suzuki]

Committed r212710: <http://trac.webkit.org/changeset/212710>

[Mark Lam]

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.