// 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?

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

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

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.

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/

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

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

Marking this as unaffected for 38 based on bug 854037.

Comment on attachment 8587779 [details] [diff] [review] Aurora rollup patch Approving for aurora for crash fix.