Closed
Bug 938144
Opened 11 years ago
Closed 10 years ago
Crash [@ js::jit::MBasicBlock::setLoopDepth] or random assertions with OOM
Categories
(Core :: JavaScript Engine: JIT, defect)
Tracking
()
RESOLVED
FIXED
mozilla29
People
(Reporter: decoder, Assigned: decoder)
Details
(Keywords: crash, testcase)
Crash Data
Attachments
(2 files)
405 bytes,
text/plain
|
Details | |
1.08 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
The following testcase crashes on mozilla-central revision 7b014f0f3b03 (threadsafe build, run with --ion-eager --fuzzing-safe --thread-count=2 --ion-parallel-compile=on --ion-regalloc=backtracking): evaluate("\ if (typeof oomAfterAllocations == 'function') {\ oomAfterAllocations(1);\ } \ ");
Assignee | ||
Comment 1•11 years ago
|
||
Assignee | ||
Comment 2•11 years ago
|
||
This OOM crash seems to depend on thread scheduling, you might need to run it multiple times (use a loop of 100 to be sure). Most of the time, I get the mentioned crash but sometimes, also random assertions pop up. It would be very helpful to kill this one as it causes lots of signatures.
Assignee | ||
Comment 3•10 years ago
|
||
Ok, the setLoopDepth crash seems to be simple:
> // Create a new block inheriting from the predecessor.
> MBasicBlock *split = MBasicBlock::NewSplitEdge(graph, block->info(), *block);
> split->setLoopDepth(block->loopDepth());
NewSplitEdge returns NULL here due to OOM, and we don't check it. Adding a check and returning false fixes the issue. However, nobody calls JS_ReportOutOfMemory here and we just exit with code 0 (no OOM reported).
Jandem, can you advise me how to fix this? Should I just add the check to return false without other modifications?
Flags: needinfo?(jdemooij)
Comment 4•10 years ago
|
||
(In reply to Christian Holler (:decoder) from comment #3) > Jandem, can you advise me how to fix this? Should I just add the check to > return false without other modifications? Yes, that's the right fix. This code may run off-thread so we can't report OOM here. It looks like we ignore OOM during compilation, but we make sure we don't crash or leak in this case, so that's fine I think.
Flags: needinfo?(jdemooij)
Assignee | ||
Comment 5•10 years ago
|
||
Comment 6•10 years ago
|
||
Comment on attachment 8344628 [details] [diff] [review] js-setLoopDepth-oom.patch Review of attachment 8344628 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!
Attachment #8344628 -
Flags: review?(jdemooij) → review+
Assignee | ||
Comment 7•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/c227b066f842
https://hg.mozilla.org/mozilla-central/rev/c227b066f842
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Comment 9•10 years ago
|
||
Can't reproduce this locally, on a Ubuntu 13.04 x86 machine. I did look for the signature from this report in Socorro though and I didn't find any crashes for the last 4 weeks.
Keywords: verifyme
You need to log in
before you can comment on or make changes to this bug.
Description
•