Closed
Bug 1131846
Opened 9 years ago
Closed 9 years ago
Crash [@ js::jit::MInstruction::setResumePoint] involving oomAfterAllocations
Categories
(Core :: JavaScript Engine: JIT, defect)
Tracking
()
RESOLVED
FIXED
mozilla39
People
(Reporter: gkw, Assigned: nbp)
References
Details
(Keywords: crash, regression, testcase, Whiteboard: [jsbugmon:update])
Crash Data
Attachments
(2 files)
3.68 KB,
text/plain
|
Details | |
5.75 KB,
patch
|
h4writer
:
review+
lmandel
:
approval-mozilla-aurora+
lmandel
:
approval-mozilla-beta-
|
Details | Diff | Splinter Review |
s = newGlobal() try { evalcx("for (v of this) {}", s) } catch (e) { "" + e } evalcx("\ do {\ for (let y = 0; y < 5; ++y) {\ oomAfterAllocations(70);\ }\ } while (x)\ ", s) crashes js debug shell on m-c changeset ee093ca70666 with --fuzzing-safe --no-threads --ion-eager at js::jit::MInstruction::setResumePoint. 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
Flags: needinfo?
Reporter | ||
Comment 1•9 years ago
|
||
(lldb) bt 5 * thread #1: tid = 0x26c3b7, 0x000000010061d4b1 js-dbg-64-dm-nsprBuild-darwin-ee093ca70666`js::jit::MInstruction::setResumePoint(js::jit::MResumePoint*) [inlined] js::jit::MResumePoint::setInstruction(this=0x0000000000000000, ins=0x0000000104083440) at MIR.h:11735, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x40) * frame #0: 0x000000010061d4b1 js-dbg-64-dm-nsprBuild-darwin-ee093ca70666`js::jit::MInstruction::setResumePoint(js::jit::MResumePoint*) [inlined] js::jit::MResumePoint::setInstruction(this=0x0000000000000000, ins=0x0000000104083440) at MIR.h:11735 frame #1: 0x000000010061d4b1 js-dbg-64-dm-nsprBuild-darwin-ee093ca70666`js::jit::MInstruction::setResumePoint(this=0x0000000104083440, resumePoint=0x0000000000000000) + 17 at MIR.cpp:310 frame #2: 0x0000000100634ba0 js-dbg-64-dm-nsprBuild-darwin-ee093ca70666`js::jit::MBasicBlock::linkOsrValues(this=0x0000000104082e78, start=<unavailable>) + 608 at MIRGraph.cpp:573 frame #3: 0x000000010052ad34 js-dbg-64-dm-nsprBuild-darwin-ee093ca70666`js::jit::IonBuilder::newOsrPreheader(this=0x0000000104081658, predecessor=0x0000000104082690, loopEntry=<unavailable>) + 2724 at IonBuilder.cpp:6578 frame #4: 0x000000010052955a js-dbg-64-dm-nsprBuild-darwin-ee093ca70666`js::jit::IonBuilder::forLoop(this=0x0000000104081658, op=<unavailable>, sn=<unavailable>) + 1594 at IonBuilder.cpp:3112 (lldb)
Flags: needinfo?
Reporter | ||
Comment 2•9 years ago
|
||
autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: https://hg.mozilla.org/mozilla-central/rev/55f95c3b5dbe user: Kannan Vijayan date: Wed Aug 13 11:59:55 2014 -0400 summary: Bug 1004831 - Part 4 - Register native to bytecode mappings when new IonCode is generated. r=h4writer Kannan, is bug 1004831 a likely regressor?
Blocks: 1004831
Flags: needinfo?(kvijayan)
Comment 3•9 years ago
|
||
This is almost definitely a coincidental regressor, not the real cause. The offending line is: cloneRp->setResumePoint(MResumePoint::Copy(graph().alloc(), res)); here, MResumePoint::Copy is returning |nullptr| and eventually causes a nullptr access. 'hg blame' identifies the following patch as adding that line: changeset: 200866:9a48a86bf2ef user: Nicolas B. Pierron <nicolas.b.pierron@mozilla.com> date: Thu Aug 21 20:47:03 2014 +0200 summary: Bug 1042729 part 1 - Make resume point unique to each instruction. r=h4writer
Flags: needinfo?(kvijayan)
Reporter | ||
Comment 4•9 years ago
|
||
Nicolas, is bug 1042729 a likely regressor?
Assignee | ||
Comment 5•9 years ago
|
||
Attachment #8576138 -
Flags: review?(hv1989)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → nicolas.b.pierron
Status: NEW → ASSIGNED
Flags: needinfo?(nicolas.b.pierron)
Updated•9 years ago
|
Attachment #8576138 -
Flags: review?(hv1989) → review+
Assignee | ||
Comment 6•9 years ago
|
||
Bug 1042729 landed in Gecko 34+.
Assignee | ||
Comment 7•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/550637e50cd8
Comment 8•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/550637e50cd8
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Assignee | ||
Comment 9•9 years ago
|
||
Comment on attachment 8576138 [details] [diff] [review] Check the return value of MResumePoint::Copy. Approval Request Comment [Feature/regressing bug #]: bug 1042729 [User impact if declined]: Unlikely OOM during compilation. [Describe test coverage new/current, TreeHerder]: Landed, no test case, as there is no good way to consistently check for it. [Risks and why]: None? [String/UUID change made/needed]: N/A
Attachment #8576138 -
Flags: approval-mozilla-beta?
Attachment #8576138 -
Flags: approval-mozilla-aurora?
Comment 10•9 years ago
|
||
(In reply to Nicolas B. Pierron [:nbp] from comment #9) > [User impact if declined]: > Unlikely OOM during compilation. We've shipped 3 releases with this bug and are pretty late in the 37 Beta cycle. How unlikely is it to hit this OOM condition? Do you think we need to fix this in 37 or can it wait for 38?
Flags: needinfo?(nicolas.b.pierron)
Assignee | ||
Comment 11•9 years ago
|
||
(In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #10) > (In reply to Nicolas B. Pierron [:nbp] from comment #9) > > [User impact if declined]: > > Unlikely OOM during compilation. > > We've shipped 3 releases with this bug and are pretty late in the 37 Beta > cycle. How unlikely is it to hit this OOM condition? Do you think we need to > fix this in 37 or can it wait for 38? I think this is a nice to have. I do not think it is frequent enough to make it mandatory.
Flags: needinfo?(nicolas.b.pierron)
Comment 12•9 years ago
|
||
Comment on attachment 8576138 [details] [diff] [review] Check the return value of MResumePoint::Copy. Given where we are in the cycle, let's skip this fix for 37. I think it's reasonable to take this fix in 38. Beta- Aurora+
Attachment #8576138 -
Flags: approval-mozilla-beta?
Attachment #8576138 -
Flags: approval-mozilla-beta-
Attachment #8576138 -
Flags: approval-mozilla-aurora?
Attachment #8576138 -
Flags: approval-mozilla-aurora+
Updated•9 years ago
|
Assignee | ||
Comment 13•9 years ago
|
||
(In reply to Nicolas B. Pierron [:nbp] from comment #11) > I think this is a nice to have. I do not think it is frequent enough to make > it mandatory. For the record, https://crash-stats.mozilla.com/report/list?range_unit=days&range_value=28&signature=js%3A%3Ajit%3A%3AMInstruction%3A%3AsetResumePoint%28js%3A%3Ajit%3A%3AMResumePoint*%29#tab-reports I see 115 crashes reported to crash-stat over the last 28 days.
You need to log in
before you can comment on or make changes to this bug.
Description
•