The following testcase crashes on mozilla-central revision c7935d540027 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --disable-debug --enable-optimize, run with --fuzzing-safe --ion-offthread-compile=off --ion-eager): for (var i = 0; i < 10; ++i) {} for (var i = 0; i < 3; i++) { throw eval(raisesException); function ff() {} } Backtrace: received signal SIGSEGV, Segmentation fault. 0x00000000009076eb in JSObject::is<js::EnvironmentObject> (this=0x0) at js/src/vm/EnvironmentObject.h:1015 #0 0x00000000009076eb in JSObject::is<js::EnvironmentObject> (this=0x0) at js/src/vm/EnvironmentObject.h:1015 #1 js::EnvironmentIter::incrementScopeIter (this=this@entry=0x7fffffffca50) at js/src/vm/EnvironmentObject.cpp:1255 #2 0x00000000004c3714 in js::EnvironmentIter::operator++ (this=0x7fffffffca50) at js/src/vm/EnvironmentObject.h:695 #3 js::UnwindAllEnvironmentsInFrame (cx=cx@entry=0x7ffff6926800, ei=...) at js/src/vm/Interpreter.cpp:1073 #4 0x00000000006fd516 in js::jit::DebugEpilogue (cx=cx@entry=0x7ffff6926800, frame=frame@entry=0x7fffffffd1d8, pc=pc@entry=0x7ffff6920d8e "\232", ok=<optimized out>) at js/src/jit/VMFunctions.cpp:780 #5 0x0000000000649137 in js::jit::OnLeaveBaselineFrame (frameOk=<optimized out>, rfe=0x7fffffffd158, pc=<optimized out>, frame=..., cx=0x7ffff6926800) at js/src/jit/JitFrames.cpp:492 #6 js::jit::HandleExceptionBaseline (pc=0x7ffff6920d8e "\232", rfe=<optimized out>, frame=..., cx=0x7ffff6926800) at js/src/jit/JitFrames.cpp:755 #7 js::jit::HandleException (rfe=<optimized out>) at js/src/jit/JitFrames.cpp:905 #8 0x00000480e9ec515b in ?? () #9 0x00007fffffffd208 in ?? () #10 0x00007fffffffd158 in ?? () #11 0x0000000000000000 in ?? () rax 0x0 0 rbx 0x7fffffffca50 140737488341584 rcx 0x1b76a60 28797536 rdx 0x7ffff4684080 140737293860992 rsi 0x7ffff4684080 140737293860992 rdi 0x7fffffffca50 140737488341584 rbp 0xdf4f80 14634880 rsp 0x7fffffffc9f8 140737488341496 r8 0x7ffff45d9388 140737293161352 r9 0x7ffff45d9340 140737293161280 r10 0x7ffff45d9310 140737293161232 r11 0x0 0 r12 0x7ffff6926800 140737330178048 r13 0x7fffffffffff 140737488355327 r14 0x7fffffffca68 140737488341608 r15 0x1 1 rip 0x9076eb <js::EnvironmentIter::incrementScopeIter()+43> => 0x9076eb <js::EnvironmentIter::incrementScopeIter()+43>: mov (%rax),%rax 0x9076ee <js::EnvironmentIter::incrementScopeIter()+46>: mov (%rax),%rax

JSBugMon: Bisection requested, result: autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: https://hg.mozilla.org/mozilla-central/rev/da80011188ed user: Ted Campbell date: Fri Feb 10 13:49:21 2017 -0500 summary: Bug 1273858 - Ion-compile JSOP_PUSHLEXICALENV/JSOP_POPLEXICALENV r=jandem This iteration took 220.738 seconds to run.

Ted, is bug 1273858 a likely regressor?

Yes. That is very likely me. The whole series should probably come out then and I'll have to revisit it. What are my next steps?

I don't think this is a [fuzzblocker] yet to warrant immediate backout. You can just fix the bug and submit the patch here, i.e. proceed as per normal.

I won't be able to get to it tonight. It does reproduce for me, but I'll need to run the fix idea past :jandem.

Problem identified. Working on a patch that won't regress performance for existing code, but still lets Bug 1273858 remain.

Problem: Under certain scenarios, Ion would optimize out the |envChain| slot while it was still holding a lexical environment. This would lead to failures when a bailout to baseline occurred. Solution: In the CompileInfo class, we can determine if a slot must be kept live for reasons not described in the MIR graph. It currently handles CallObject and NamedLambdaEnvironment, so we extend to support other lexical environments. The analysis is naive and will keep the |envChain| slot alive in rare cases that it would not have before, but there are no correctness concerns, just an extra store instruction added to jitcode.

Comment on attachment 8842291 [details] Bug 1342483 - Add JSScript::needsBodyEnvironment https://reviewboard.mozilla.org/r/116152/#review118616 Looks fine and makes a lot of sense, my only concern is that needsBodyEnvironmentObjectImpl can be called off-thread so I think I'd prefer doing this in the CompileInfo constructor (similar to what we do for thisSlotForDerivedClassConstructor) to avoid races (or false positives from TSan).

Comment on attachment 8842291 [details] Bug 1342483 - Add JSScript::needsBodyEnvironment https://reviewboard.mozilla.org/r/116152/#review118708

Comment on attachment 8843325 [details] Bug 1342483 - Preserve envChain in Ion if script uses lexical environments https://reviewboard.mozilla.org/r/117096/#review118710 Great!

Pushed by tcampbell@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4b805bbd9a83 Add JSScript::needsBodyEnvironment r=jandem https://hg.mozilla.org/integration/autoland/rev/e5a3bbe621c9 Preserve envChain in Ion if script uses lexical environments r=jandem

https://hg.mozilla.org/mozilla-central/rev/4b805bbd9a83 https://hg.mozilla.org/mozilla-central/rev/e5a3bbe621c9