1169639 - Crash [@ js::GlobalObject::getIntrinsicValue] or Assertion failure: !getSlot(INTRINSICS).isUndefined(), at vm/GlobalObject.h

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

evalcx("({}=1)", evalcx("lazy"))

asserts js debug shell on m-c changeset e537a1ba501b with --fuzzing-safe --no-threads --no-ion at Assertion failure: !getSlot(INTRINSICS).isUndefined(), at vm/GlobalObject.h and crashes js opt shell at js::GlobalObject::getIntrinsicValue.

Debug configure options:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

python -u ~/fuzzing/js/compileShell.py -b "--enable-debug --enable-more-deterministic --enable-nspr-build" -r e537a1ba501b

Opt configure options:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --disable-debug --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

python -u ~/fuzzing/js/compileShell.py -b "--disable-debug --enable-more-deterministic --enable-nspr-build" -r e537a1ba501b

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/241420daaa13
user:        Jeff Walden
date:        Fri Apr 17 21:57:50 2015 -0700
summary:     Bug 1155900 - Make destructuring right-hand-side expressions that correspond to left-hand-side object patterns pass the RequireObjectCoercible gauntlet before any properties are destructured out of them.  r=shu

Waldo, is bug 1155900 a likely regressor?

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: debug stack

(lldb) bt 5
* thread #1: tid = 0xd58b, 0x00000001002438e5 js-dbg-64-dm-nsprBuild-darwin-e537a1ba501b`js::GlobalObject::intrinsicsHolder(this=<unavailable>) + 261 at GlobalObject.h:586, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x00000001002438e5 js-dbg-64-dm-nsprBuild-darwin-e537a1ba501b`js::GlobalObject::intrinsicsHolder(this=<unavailable>) + 261 at GlobalObject.h:586
    frame #1: 0x0000000100243955 js-dbg-64-dm-nsprBuild-darwin-e537a1ba501b`js::GlobalObject::maybeGetIntrinsicValue(this=<unavailable>, id=<unavailable>, vp=0x00007fff5fbfd7c0) + 21 at GlobalObject.h:591
    frame #2: 0x0000000100242d7c js-dbg-64-dm-nsprBuild-darwin-e537a1ba501b`js::GlobalObject::getIntrinsicValue(cx=0x00000001028a5180, global=<unavailable>, name=<unavailable>, value=<unavailable>) + 44 at GlobalObject.h:606
    frame #3: 0x0000000100207dfd js-dbg-64-dm-nsprBuild-darwin-e537a1ba501b`Interpret(JSContext*, js::RunState&) [inlined] js::GetIntrinsicOperation(cx=0x00000001028a5180, pc=<unavailable>) + 5 at Interpreter-inl.h:272
    frame #4: 0x0000000100207df8 js-dbg-64-dm-nsprBuild-darwin-e537a1ba501b`Interpret(cx=0x00000001028a5180, state=0x00007fff5fbfdae8) + 49368 at Interpreter.cpp:3097
(lldb)

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: opt stack

(lldb) bt 5
* thread #1: tid = 0xd697, 0x000000010018a3e3 js-64-dm-nsprBuild-darwin-e537a1ba501b`js::GlobalObject::getIntrinsicValue(JSContext*, JS::Handle<js::GlobalObject*>, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>) [inlined] js::NativeObject::lastProperty(this=0x0000000000000000) const at NativeObject.h:390, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x8)
  * frame #0: 0x000000010018a3e3 js-64-dm-nsprBuild-darwin-e537a1ba501b`js::GlobalObject::getIntrinsicValue(JSContext*, JS::Handle<js::GlobalObject*>, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>) [inlined] js::NativeObject::lastProperty(this=0x0000000000000000) const at NativeObject.h:390
    frame #1: 0x000000010018a3e3 js-64-dm-nsprBuild-darwin-e537a1ba501b`js::GlobalObject::getIntrinsicValue(JSContext*, JS::Handle<js::GlobalObject*>, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>) [inlined] js::NativeObject::lookupPure(this=0x0000000000000000) at NativeObject.cpp:232
    frame #2: 0x000000010018a3e3 js-64-dm-nsprBuild-darwin-e537a1ba501b`js::GlobalObject::getIntrinsicValue(JSContext*, JS::Handle<js::GlobalObject*>, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>) + 35 at GlobalObject.h:593
    frame #3: 0x000000010018a3c0 js-64-dm-nsprBuild-darwin-e537a1ba501b`js::GlobalObject::getIntrinsicValue(JSContext*, JS::Handle<js::GlobalObject*>, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>) [inlined] js::GlobalObject::maybeGetIntrinsicValue(this=<unavailable>, name=0x000000010331b340) at GlobalObject.h:600
    frame #4: 0x000000010018a3c0 js-64-dm-nsprBuild-darwin-e537a1ba501b`js::GlobalObject::getIntrinsicValue(cx=0x00000001021ab0f0, global=<unavailable>, name=<unavailable>, value=<unavailable>) + 32 at GlobalObject.h:606
(lldb)

Comment 3

[Jeff Walden [:Waldo]]

The gist is that the intrinsics holder is created only when the Object class is initialized, but this particular syntax doesn't itself initialize the Object class.  We'lll have to make the intrinsicsHolder() function fallible, which means adding handles and fallibility to a bunch of stuff.  Much yak to shave -- started Friday, more to finish today probably.

Comment 4

[Christian Holler (:decoder)]

JSBugMon: The testcase found in this bug no longer reproduces (tried revision 203e1025a826).

Comment 5

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

I can still reproduce this using rev 203e1025a826 on a Mac, though.

Comment 6

[Jeff Walden [:Waldo]]

Attachment: Centralize intrinsicsHolder accesses separately from Object class initialization, as it's now possible to use an intrinsic without entering such initialization

This sort of spiraled a bit, because intrinsics-holder creation no longer necessarily occurs after object class initialization.  But I think this manages to fix this bug and update the other users accordingly.

Comment 7

[Shu-yu Guo [:shu]]

Comment on attachment 8630264 [details] [diff] [review]
Centralize intrinsicsHolder accesses separately from Object class initialization, as it's now possible to use an intrinsic without entering such initialization

Review of attachment 8630264 [details] [diff] [review]:
-----------------------------------------------------------------

This does seem easier to reason about: no more worrying about initialization phases.

It does change the perf characteristic, though the branch is very predictable. I imagine there are no perf regressions.

::: js/src/jsapi.cpp
@@ +2486,3 @@
>      if (!getterNameAtom)
>          return false;
> +    RootedPropertyName getterNameName(cx, getterNameAtom->asPropertyName());

Maybe just |getterName|?

@@ +2507,3 @@
>          if (!setterNameAtom)
>              return false;
> +        RootedPropertyName setterNameName(cx, setterNameAtom->asPropertyName());

Ditto.

::: js/src/vm/ForOfIterator.cpp
@@ +169,5 @@
>  ForOfIterator::materializeArrayIterator()
>  {
>      MOZ_ASSERT(index != NOT_ARRAY);
>  
> +    Handle<GlobalObject*> global = cx_->global();

Nit: just pass in cx_->global() inline below if there are no other uses of global in the function.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6e49d0bf0819

Comment 9

[Jeff Walden [:Waldo]]

(In reply to Shu-yu Guo [:shu] from comment #7)
> ::: js/src/jsapi.cpp
> @@ +2486,3 @@
> >      if (!getterNameAtom)
> >          return false;
> > +    RootedPropertyName getterNameName(cx, getterNameAtom->asPropertyName());
> 
> Maybe just |getterName|?

Sure, if |getterName| weren't the const char* passed into this method, used to create |getterNameAtom|.  Pfui.

> @@ +2507,3 @@
> >          if (!setterNameAtom)
> >              return false;
> > +        RootedPropertyName setterNameName(cx, setterNameAtom->asPropertyName());
> 
> Ditto.

Ditto.

Comment 10

[Phil Ringnalda (:philor)]

I don't really remember where hazard builds hide the actual information you need about your failures, but it's somewhere in https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=6e49d0bf0819&filter-searchStr=hazard

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/9d52e081c34e

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Function '_ZN2js12GlobalObject17setIntrinsicValueEP9JSContextN2JS6HandleIPS0_EEPNS_12PropertyNameENS4_INS3_5ValueEEE|uint8 js::GlobalObject::setIntrinsicValue(JSContext*, class JS::Handle<js::GlobalObject*>, js::PropertyName*, class JS::Handle<JS::Value>)' has unrooted 'name' of type 'js::PropertyName*' live across GC call '_ZN2js12GlobalObject19getIntrinsicsHolderEP9JSContextN2JS6HandleIPS0_EE|js::NativeObject* js::GlobalObject::getIntrinsicsHolder(JSContext*, class JS::Handle<js::GlobalObject*>)' at js/src/vm/GlobalObject.h:648
    js/src/vm/GlobalObject.h:647: Call(1,2, __temp_3 := cx*.runtime())
    js/src/vm/GlobalObject.h:647: Call(2,3, __temp_4 := global.operator 112())
    js/src/vm/GlobalObject.h:647: Call(3,4, __temp_2 := __temp_3*.isSelfHostingGlobal(__temp_4**.field:0.field:0))
    js/src/vm/GlobalObject.h:647: Call(4,5, __temp_1 := __builtin_expect(!__temp_2*,0))
    js/src/vm/GlobalObject.h:647: Assume(5,6, (__temp_1* != 0), true)
    js/src/vm/GlobalObject.h:647: Call(6,7, MOZ_ReportAssertionFailure("cx->runtime()->isSelfHostingGlobal(global)","/builds/slave/l64-br-haz_m-in_dep-0000000000/build/source/js/src/vm/GlobalObject.h",647))
    js/src/vm/GlobalObject.h:647: Assign(7,8, 0 := 647)
    js/src/vm/GlobalObject.h:647: Call(8,9, abort())
    js/src/vm/GlobalObject.h:648: Assign(9,10, __temp_6 := global*)
    js/src/vm/GlobalObject.h:648: Call(10,11, __temp_5 := getIntrinsicsHolder(cx*,__temp_6*)) [[GC call]]
    js/src/vm/GlobalObject.h:648: Call(11,12, __temp_7*.GuardObjectNotifier(0))
    js/src/vm/GlobalObject.h:648: Call(12,13, holder.Rooted(cx*,__temp_5*.field:0,__temp_7))
    js/src/vm/GlobalObject.h:648: Call(13,14, __temp_7.~GuardObjectNotifier())
    js/src/vm/GlobalObject.h:649: Call(14,15, __temp_8 := holder.operator 49())
    js/src/vm/GlobalObject.h:649: Assume(15,18, null(__temp_8**), false)
    js/src/vm/GlobalObject.h:651: Call(18,19, __temp_9*.Handle(0,holder,0))
    js/src/vm/GlobalObject.h:651: Assign(19,20, __temp_10 := value*)
    js/src/vm/GlobalObject.h:651: Call(20,21, return := SetProperty(cx*,__temp_9*,name*,__temp_10*))
GC Function: _ZN2js12GlobalObject19getIntrinsicsHolderEP9JSContextN2JS6HandleIPS0_EE|js::NativeObject* js::GlobalObject::getIntrinsicsHolder(JSContext*, class JS::Handle<js::GlobalObject*>)
    uint8 js::DefineProperty(js::ExclusiveContext*, class JS::Handle<JSObject*>, js::PropertyName*, class JS::Handle<JS::Value>, (uint8)(JSContext*,class JS::Handle<JSObject*>,class JS::Handle<jsid>,class JS::MutableHandle<JS::Value>)*, (uint8)(JSContext*,class JS::Handle<JSObject*>,class JS::Handle<jsid>,class JS::MutableHandle<JS::Value>,JS::ObjectOpResult*)*, uint32)
    uint8 js::DefineProperty(js::ExclusiveContext*, class JS::Handle<JSObject*>, class JS::Handle<jsid>, class JS::Handle<JS::Value>, (uint8)(JSContext*,class JS::Handle<JSObject*>,class JS::Handle<jsid>,class JS::MutableHandle<JS::Value>)*, (uint8)(JSContext*,class JS::Handle<JSObject*>,class JS::Handle<jsid>,class JS::MutableHandle<JS::Value>,JS::ObjectOpResult*)*, uint32)
    uint8 js::DefineProperty(js::ExclusiveContext*, class JS::Handle<JSObject*>, class JS::Handle<jsid>, class JS::Handle<JS::Value>, (uint8)(JSContext*,class JS::Handle<JSObject*>,class JS::Handle<jsid>,class JS::MutableHandle<JS::Value>)*, (uint8)(JSContext*,class JS::Handle<JSObject*>,class JS::Handle<jsid>,class JS::MutableHandle<JS::Value>,JS::ObjectOpResult*)*, uint32, JS::ObjectOpResult*)
    IndirectCall: op

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/45b7b670c32e

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/45b7b670c32e