Closed Bug 1342483 Opened 4 years ago Closed 4 years ago

Crash [@ JSObject::is<js::EnvironmentObject>] or Assertion failure: !isExtensible() && v.isPrivateGCThing(), at vm/EnvironmentObject.h:501

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox51 --- unaffected
firefox52 --- unaffected
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- fixed

People

(Reporter: decoder, Assigned: tcampbell)

Details

(5 keywords, Whiteboard: [jsbugmon:update])

Crash Data

Attachments

(2 files)

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
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
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?
Flags: needinfo?(tcampbell)
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?
Flags: needinfo?(tcampbell)
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.
Assignee: nobody → tcampbell
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).
Attachment #8842291 - Flags: review?(jdemooij)
Comment on attachment 8842291 [details]
Bug 1342483 - Add JSScript::needsBodyEnvironment

https://reviewboard.mozilla.org/r/116152/#review118708
Attachment #8842291 - Flags: review?(jdemooij) → review+
Comment on attachment 8843325 [details]
Bug 1342483 - Preserve envChain in Ion if script uses lexical environments

https://reviewboard.mozilla.org/r/117096/#review118710

Great!
Attachment #8843325 - Flags: review?(jdemooij) → review+
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
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.