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

RESOLVED FIXED in Firefox 54

Status

()

--
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: decoder, Assigned: tcampbell)

Tracking

(Blocks: 1 bug, 5 keywords)

Trunk
mozilla54
x86_64
Linux
assertion, crash, jsbugmon, regression, testcase
Points:
---

Firefox Tracking Flags

(firefox51 unaffected, firefox52 unaffected, firefox-esr52 unaffected, firefox53 unaffected, firefox54 fixed)

Details

(Whiteboard: [jsbugmon:update], crash signature)

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
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

Updated

2 years ago
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]

Comment 1

2 years ago
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)
(Assignee)

Comment 3

2 years ago
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.
(Assignee)

Comment 5

2 years ago
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)

Updated

2 years ago
Assignee: nobody → tcampbell

Updated

2 years ago
status-firefox51: --- → unaffected
status-firefox52: --- → unaffected
status-firefox53: --- → unaffected
(Assignee)

Comment 6

2 years ago
Problem identified. Working on a patch that won't regress performance for existing code, but still lets Bug 1273858 remain.
Comment hidden (mozreview-request)
(Assignee)

Comment 8

2 years ago
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 hidden (mozreview-request)

Comment 10

2 years ago
mozreview-review
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 hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 14

2 years ago
mozreview-review
Comment on attachment 8842291 [details]
Bug 1342483 - Add JSScript::needsBodyEnvironment

https://reviewboard.mozilla.org/r/116152/#review118708
Attachment #8842291 - Flags: review?(jdemooij) → review+

Comment 15

2 years ago
mozreview-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+

Comment 16

2 years ago
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

Comment 17

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4b805bbd9a83
https://hg.mozilla.org/mozilla-central/rev/e5a3bbe621c9
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox54: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
status-firefox-esr52: --- → unaffected
You need to log in before you can comment on or make changes to this bug.