1149797 - Crash [@ js::NativeObject::replaceWithNewEquivalentShape] or Assertion failure: !inDictionary(), at vm/Shape.h

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

// Randomly chosen test: js/src/jit-test/tests/basic/bug646968-7.js
for (let x = 0; x < 9; ++x) {
    (function() {
        eval('var x');
    })();
}

asserts js debug shell on m-c changeset 385840329d91 with --fuzzing-safe --no-threads --ion-eager at Assertion failure: !inDictionary(), at vm/Shape.h and crashes js opt shell at js::NativeObject::replaceWithNewEquivalentShape.

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 385840329d91

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 385840329d91

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/b05e10ed40c4
user:        Jeff Walden
date:        Fri Mar 27 12:29:50 2015 -0400
summary:     Bug 854037 - Make lexical declarations in the initializing component of a for(;;) loop create a fresh binding for each iteration of the loop.  r=shu

Waldo, is bug 854037 a likely regressor?

Comment 1

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

Attachment: stack

(lldb) bt 5
* thread #1: tid = 0x50a02, 0x000000010087394b js-dbg-64-dm-nsprBuild-darwin-385840329d91`JSObject::create(js::ExclusiveContext*, js::gc::AllocKind, js::gc::InitialHeap, JS::Handle<js::Shape*>, JS::Handle<js::ObjectGroup*>) [inlined] js::Shape::slotSpan(this=<unavailable>, this=<unavailable>) const + 67 at Shape.h:905, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x000000010087394b js-dbg-64-dm-nsprBuild-darwin-385840329d91`JSObject::create(js::ExclusiveContext*, js::gc::AllocKind, js::gc::InitialHeap, JS::Handle<js::Shape*>, JS::Handle<js::ObjectGroup*>) [inlined] js::Shape::slotSpan(this=<unavailable>, this=<unavailable>) const + 67 at Shape.h:905
    frame #1: 0x0000000100873908 js-dbg-64-dm-nsprBuild-darwin-385840329d91`JSObject::create(js::ExclusiveContext*, js::gc::AllocKind, js::gc::InitialHeap, JS::Handle<js::Shape*>, JS::Handle<js::ObjectGroup*>) [inlined] js::Shape::slotSpan(this=<unavailable>) const at Shape.h:911
    frame #2: 0x0000000100873908 js-dbg-64-dm-nsprBuild-darwin-385840329d91`JSObject::create(cx=<unavailable>, kind=<unavailable>, heap=<unavailable>, shape=<unavailable>, group=<unavailable>) + 2616 at jsobjinlines.h:277
    frame #3: 0x00000001002b9b6f js-dbg-64-dm-nsprBuild-darwin-385840329d91`js::ClonedBlockObject::clone(cx=0x00000001028a5180, block=<unavailable>) + 223 at ScopeObject.cpp:673
    frame #4: 0x00000001006bfa0f js-dbg-64-dm-nsprBuild-darwin-385840329d91`js::jit::FreshenBlockScope(JSContext*, js::jit::BaselineFrame*) [inlined] js::jit::BaselineFrame::freshenBlock(this=0x00007fff5fbfe878) + 76 at BaselineFrame-inl.h:80
(lldb)

Comment 2

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

Attachment: stack of opt crash

(lldb) bt 5
* thread #1: tid = 0x50ddf, 0x0000000100195df1 js-64-dm-nsprBuild-darwin-385840329d91`js::NativeObject::replaceWithNewEquivalentShape(js::ExclusiveContext*, js::Shape*, js::Shape*, bool) [inlined] js::BarrieredBase<js::Shape*>::pre() at Barrier.h:452, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x0000000100195df1 js-64-dm-nsprBuild-darwin-385840329d91`js::NativeObject::replaceWithNewEquivalentShape(js::ExclusiveContext*, js::Shape*, js::Shape*, bool) [inlined] js::BarrieredBase<js::Shape*>::pre() at Barrier.h:452
    frame #1: 0x0000000100195df1 js-64-dm-nsprBuild-darwin-385840329d91`js::NativeObject::replaceWithNewEquivalentShape(js::ExclusiveContext*, js::Shape*, js::Shape*, bool) [inlined] js::HeapPtr<js::Shape*>::set(this=0x0000000000000000) at Barrier.h:537
    frame #2: 0x0000000100195df1 js-64-dm-nsprBuild-darwin-385840329d91`js::NativeObject::replaceWithNewEquivalentShape(js::ExclusiveContext*, js::Shape*, js::Shape*, bool) [inlined] js::HeapPtr<js::Shape*>::operator=(this=0x0000000000000000) at Barrier.h:523
    frame #3: 0x0000000100195df1 js-64-dm-nsprBuild-darwin-385840329d91`js::NativeObject::replaceWithNewEquivalentShape(js::ExclusiveContext*, js::Shape*, js::Shape*, bool) [inlined] js::Shape::removeFromDictionary(js::NativeObject*) + 21 at Shape.cpp:90
    frame #4: 0x0000000100195ddc js-64-dm-nsprBuild-darwin-385840329d91`js::NativeObject::replaceWithNewEquivalentShape(this=<unavailable>, cx=<unavailable>, oldShape=<unavailable>, newShape=<unavailable>, accessorShape=<unavailable>) + 924 at Shape.cpp:1115
(lldb)

Comment 3

[Jeff Walden [:Waldo]]

The freshenblockscope opcode thinks it can clone the ClonedBlockObject that's at the end of the scope chain for each loop iteration, looking solely at that block object.  But when the block object has indeterminate bindings, because of an eval, or a nested function statement, or any of the things that make the scope object indeterminate -- that's not enough.  The block object might contain variable definitions added dynamically.  So we probably need to add the block object's index to freshenblockscope and have it create the clone using that, then copying in values from the existing block on the scope chain.

Comment 4

[Jeff Walden [:Waldo]]

Attachment: Patch

It's kinda stupid just how much extra work the previous patch went to, to create a clone and copy into it -- wrongly.  Fortunately the process of modifying the current code to be correct pointed out how it'd just be easier to reuse the real, canonical method to create this stuff.  \o/

Comment 5

[Jeff Walden [:Waldo]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9a931cd178a7

Comment 6

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

https://hg.mozilla.org/mozilla-central/rev/9a931cd178a7
https://hg.mozilla.org/mozilla-central/rev/e3b60ee3163b

Comment 7

[Jeff Walden [:Waldo]]

Attachment: Aurora rollup patch

Comment 8

[Jeff Walden [:Waldo]]

Comment on attachment 8587779 [details] [diff] [review]
Aurora rollup patch

Approval Request Comment
[Feature/regressing bug #]: bug 854037
[User impact if declined]: using eval inside a for-loop with a let-declaration in the head will have unexpected behaviors -- I haven't determined what those behaviors are, we should just take this this early in the cycle
[Describe test coverage new/current, TreeHerder]: various testcases landed in the patch
[Risks and why]: very little -- indeed this *simplifies* our current code by reusing an existing, well-used method, making the code even more reliable
[String/UUID change made/needed]: N/A

Comment 9

[Liz Henry (:lizzard) (relman/hg->git project)]

Marking this as unaffected for 38 based on bug 854037.

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8587779 [details] [diff] [review]
Aurora rollup patch

Approving for aurora for crash fix.

Comment 11

[Jeff Walden [:Waldo]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8cef59e11f86