(function() { var a01 var b02 var c03 var d04 var e05 var f06 var g07 var h08 let i09 var j10 var k11 var l12 var m13 function n14() { eval() } })() asserts js debug shell on m-c changeset 0e631971b841 with --ion-eager --no-threads at Assertion failure: startOfUndefined <= nfixed, at jit/IonMacroAssembler.cpp. 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-optimize --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: https://hg.mozilla.org/mozilla-central/rev/0fd815999686 user: Shu-yu Guo date: Wed Oct 29 19:41:42 2014 -0700 summary: Bug 1089761 - Fix initializing lexicals to throw on touch on CallObject. (r=jandem,Waldo) Shu-yu, is bug 1089761 a possible regressor?

(lldb) bt 5 * thread #1: tid = 0x2cc601, 0x0000000100307b2e js-dbg-opt-64-dm-nsprBuild-darwin-0e631971b841`js::jit::MacroAssembler::initGCSlots(js::jit::Register, js::jit::Register, js::NativeObject*, bool) [inlined] JSObject::lastProperty(this=<unavailable>, clasp=<unavailable>) const + 28 at jsobj.h:129, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0) * frame #0: 0x0000000100307b2e js-dbg-opt-64-dm-nsprBuild-darwin-0e631971b841`js::jit::MacroAssembler::initGCSlots(js::jit::Register, js::jit::Register, js::NativeObject*, bool) [inlined] JSObject::lastProperty(this=<unavailable>, clasp=<unavailable>) const + 28 at jsobj.h:129 frame #1: 0x0000000100307b12 js-dbg-opt-64-dm-nsprBuild-darwin-0e631971b841`js::jit::MacroAssembler::initGCSlots(this=<unavailable>, templateObj=<unavailable>, initFixedSlots=<unavailable>, obj=<unavailable>, slots=<unavailable>) + 994 at IonMacroAssembler.cpp:996 frame #2: 0x0000000100306d63 js-dbg-opt-64-dm-nsprBuild-darwin-0e631971b841`js::jit::MacroAssembler::initGCThing(this=0x000000010482b060, templateObj=0x0000000101f31100, initFixedSlots=true, obj=<unavailable>, slots=<unavailable>) + 403 at IonMacroAssembler.cpp:1074 frame #3: 0x000000010025b4a8 js-dbg-opt-64-dm-nsprBuild-darwin-0e631971b841`js::jit::CodeGenerator::visitNewCallObject(this=0x000000010482b000, lir=<unavailable>) + 344 at CodeGenerator.cpp:4507 frame #4: 0x0000000100259807 js-dbg-opt-64-dm-nsprBuild-darwin-0e631971b841`js::jit::CodeGenerator::generateBody(this=0x000000010482b000) + 935 at CodeGenerator.cpp:4108 (lldb)

Just realised that GC is on the stack, so setting s-s and assuming sec-high as a start.

Basically, initGCSlots wasn't dealing with CallObjects on functions with aliased lexicals correctly. Luckily, the way uninitialized lexicals appear in the slot vector is also segmented, like undefineds. Terrence, you did this optimization so flagging you for review.

(In reply to Gary Kwong [:gkw] [:nth10sd] GMT+8 after Oct 9 from comment #2) > Just realised that GC is on the stack, so setting s-s and assuming sec-high > as a start. GC isn't on the stack, that's just the helper method name. I don't think this is s-s; ISTM both copySlotsFromTemplate and fillSlotsWithUndefined wouldn't do anything for values of startOfUndefined >= nfixed.

Comment on attachment 8515666 [details] [diff] [review] Deal with uninitialized slots in MacroAssembler::initGCSlots. Review of attachment 8515666 [details] [diff] [review]: ----------------------------------------------------------------- Thanks! ::: js/src/jit/IonMacroAssembler.cpp @@ +996,5 @@ > MOZ_ASSERT(nslots > 0); > + uint32_t first = nslots; > + for (; first != 0; --first) { > + if (!IsUninitializedLexical(templateObj->getSlot(first - 1))) { > + *startOfUninitialized = first; What is guaranteeing that startOfUninitialized will actually be initialized here? Do CallObject have a reserved slot? @@ +1033,5 @@ > + // The template object may be a CallObject, in which case we need to > + // account for uninitialized lexical slots as well as undefined > + // slots. Unitialized lexical slots always appear at the very end of > + // slots, after undefined. > + uint32_t startOfUndefined, startOfUninitialized; Please initialize both of these to nslots to be on the safe side and to ensure changing compiler dataflow analysis doesn't intermittently cause warnings. @@ +1064,4 @@ > fillSlotsWithUndefined(Address(obj, 0), slots, 0, ndynamic); > + > + // Fill uninitialized slots if necessary. > + if (startOfUninitialized >= nfixed) { I think this branch and the prior one are equivalent to |if (start >= end) return;| at the top of FillSlotsWithConstantValue. Please move this branch there to clean up the duplicated and less straightforward branches here.

(In reply to Terrence Cole [:terrence] from comment #5) > Comment on attachment 8515666 [details] [diff] [review] > Deal with uninitialized slots in MacroAssembler::initGCSlots. > > Review of attachment 8515666 [details] [diff] [review]: > ----------------------------------------------------------------- > > Thanks! > > ::: js/src/jit/IonMacroAssembler.cpp > @@ +996,5 @@ > > MOZ_ASSERT(nslots > 0); > > + uint32_t first = nslots; > > + for (; first != 0; --first) { > > + if (!IsUninitializedLexical(templateObj->getSlot(first - 1))) { > > + *startOfUninitialized = first; > > What is guaranteeing that startOfUninitialized will actually be initialized > here? Do CallObject have a reserved slot? > > @@ +1033,5 @@ > > + // The template object may be a CallObject, in which case we need to > > + // account for uninitialized lexical slots as well as undefined > > + // slots. Unitialized lexical slots always appear at the very end of > > + // slots, after undefined. > > + uint32_t startOfUndefined, startOfUninitialized; > > Please initialize both of these to nslots to be on the safe side and to > ensure changing compiler dataflow analysis doesn't intermittently cause > warnings. > > @@ +1064,4 @@ > > fillSlotsWithUndefined(Address(obj, 0), slots, 0, ndynamic); > > + > > + // Fill uninitialized slots if necessary. > > + if (startOfUninitialized >= nfixed) { > > I think this branch and the prior one are equivalent to |if (start >= end) > return;| at the top of FillSlotsWithConstantValue. Please move this branch > there to clean up the duplicated and less straightforward branches here. The first branch of |startOfUninitialized < nfixed| is, but not this one. If we have |startOfUninitialized < nfixed| (which is certainly possible, if some of the fixed slots are lexicals), |start| is going to be |startOfUninitialized - nfixed|, which will overflow to something huge, and |start >= end| is going to be true inside FillSlotsWithConstantValue.

(In reply to Shu-yu Guo [:shu] from comment #6) > (In reply to Terrence Cole [:terrence] from comment #5) > > Comment on attachment 8515666 [details] [diff] [review] > > Deal with uninitialized slots in MacroAssembler::initGCSlots. > > > > Review of attachment 8515666 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > Thanks! > > > > ::: js/src/jit/IonMacroAssembler.cpp > > @@ +996,5 @@ > > > MOZ_ASSERT(nslots > 0); > > > + uint32_t first = nslots; > > > + for (; first != 0; --first) { > > > + if (!IsUninitializedLexical(templateObj->getSlot(first - 1))) { > > > + *startOfUninitialized = first; > > > > What is guaranteeing that startOfUninitialized will actually be initialized > > here? Do CallObject have a reserved slot? > > > > @@ +1033,5 @@ > > > + // The template object may be a CallObject, in which case we need to > > > + // account for uninitialized lexical slots as well as undefined > > > + // slots. Unitialized lexical slots always appear at the very end of > > > + // slots, after undefined. > > > + uint32_t startOfUndefined, startOfUninitialized; > > > > Please initialize both of these to nslots to be on the safe side and to > > ensure changing compiler dataflow analysis doesn't intermittently cause > > warnings. > > > > @@ +1064,4 @@ > > > fillSlotsWithUndefined(Address(obj, 0), slots, 0, ndynamic); > > > + > > > + // Fill uninitialized slots if necessary. > > > + if (startOfUninitialized >= nfixed) { > > > > I think this branch and the prior one are equivalent to |if (start >= end) > > return;| at the top of FillSlotsWithConstantValue. Please move this branch > > there to clean up the duplicated and less straightforward branches here. > > The first branch of |startOfUninitialized < nfixed| is, but not this one. If > we have |startOfUninitialized < nfixed| (which is certainly possible, if > some of the fixed slots are lexicals), |start| is going to be > |startOfUninitialized - nfixed|, which will overflow to something huge, and > |start >= end| is going to be true inside FillSlotsWithConstantValue. Oh, I'm tired. I don't know why I typed out that explanation to say you're wrong, when in fact you're right.

(In reply to Shu-yu Guo [:shu] from comment #4) > GC isn't on the stack, that's just the helper method name. I don't think > this is s-s; ISTM both copySlotsFromTemplate and fillSlotsWithUndefined > wouldn't do anything for values of startOfUndefined >= nfixed. Ok, opening up then.

tracking to ensure this gets backported as per https://bugzilla.mozilla.org/show_bug.cgi?id=1089761#c13

Carrying r=terrence, applied comments.

Approval information same as bug 1089761.

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/3c88f1fc617c under suspicion of ggc bustage: https://treeherder.mozilla.org/ui/logviewer.html#?job_id=3601006&repo=mozilla-inbound

(In reply to Wes Kocher (:KWierso) from comment #13) > Backed out in > https://hg.mozilla.org/integration/mozilla-inbound/rev/3c88f1fc617c under > suspicion of ggc bustage: > > https://treeherder.mozilla.org/ui/logviewer. > html#?job_id=3601006&repo=mozilla-inbound Pretty sure this is not my fault? https://treeherder.mozilla.org/ui/index.html#/jobs?repo=mozilla-inbound&revision=370927f1465a has that failure, and it's after your backout.

Nope, it wasn't.