Closed
Bug 1398105
Opened 7 years ago
Closed 7 years ago
Crash in js::jit::LRecoverInfo::OperandIter::settle
Categories
(Core :: JavaScript Engine: JIT, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox55 | --- | unaffected |
firefox56 | --- | unaffected |
firefox57 | + | fixed |
People
(Reporter: calixte, Assigned: nbp)
References
Details
(Keywords: crash)
Crash Data
Attachments
(1 file)
18.98 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is report bp-c055907c-6515-457f-ac80-0d1190170908. ============================================================= There are 20 crashes in nightly 57 with buildid 20170907220212. The pushlog for this nightly can be found at [1]. :nbp, could you investigate please ? [1] https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d8e238b811d3dc74515065ae8cab6c74baf0295f&tochange=b4c1ad9565ee9d00d96501c4a83083daf25c1413
Flags: needinfo?(nicolas.b.pierron)
Assignee | ||
Comment 1•7 years ago
|
||
Calixte, thanks, perfect need-info. This might not be Bug 966743, but this might be highlighted by it. I will investigate later today.
Reporter | ||
Comment 2•7 years ago
|
||
I can reproduce the crash [1] with url [2]. [1] https://crash-stats.mozilla.com/report/index/dfa0b06c-0e27-4120-8dae-264c20170908 [2] http://www.bestbuy.ca/en-ca/product/ge-30-5-0-cu-ft-freestanding-smooth-top-electric-range-jcbs630skss-stainless-steel/10605166.aspx
Crash Signature: [@ js::jit::LRecoverInfo::OperandIter::settle] → [@ js::jit::LRecoverInfo::OperandIter::settle]
[@ js::jit::LRecoverInfo::appendResumePoint]
[@ js::jit::LRecoverInfo::New]
Comment 3•7 years ago
|
||
These were coming from me, thanks for creating the bug. :twisniewski ran mozregression and came up with: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=5eb5af7c30a999bd03d3df13067640b9967875d1&tochange=8b1881ead0b6c37563d14e814f397ec3c6c5e4fb
Comment 4•7 years ago
|
||
This is the #10 Windows topcrash in Nightly 20170908100218.
Assignee | ||
Comment 5•7 years ago
|
||
This issue comes from Bug 966743. I am currently looking at how to properly fix this issue.
Assignee: nobody → nicolas.b.pierron
Blocks: 966743
Status: NEW → ASSIGNED
Flags: needinfo?(nicolas.b.pierron)
Reporter | ||
Comment 6•7 years ago
|
||
The signature 'js::jit::MBasicBlock::flagOperandsOfPrunedBranches' appeared with buildid 20170907220212 and one of the crashy urls is [1]. [1] http://www.bestbuy.ca/en-ca/product/lexar-media-lexar-media-32gb-95mb-s-sdhc-memory-card-lsd32gcb1nl633/10417724.aspx?
Crash Signature: [@ js::jit::LRecoverInfo::OperandIter::settle]
[@ js::jit::LRecoverInfo::appendResumePoint]
[@ js::jit::LRecoverInfo::New] → [@ js::jit::LRecoverInfo::OperandIter::settle]
[@ js::jit::LRecoverInfo::appendResumePoint]
[@ js::jit::LRecoverInfo::New]
[@ js::jit::MBasicBlock::flagOperandsOfPrunedBranches]
Assignee | ||
Comment 7•7 years ago
|
||
This patch adds a test case which reproduces the same assertion seen on the website mentioned in comment 2. The problem is that when we inline Array.prototype.push from a fun.apply call with the argument object of an inlined function, we did not increased the number of slots of the basic block, leading to various memory issues. This patch fix this issue, by moving the mutation of the number of slots in the CallInfo::pushFormals function, and solves this issue by carrying the apply_ flag, which records whether we should extend or not the number of slots in the basic block.
Attachment #8906671 -
Flags: review?(jdemooij)
Comment 8•7 years ago
|
||
Comment on attachment 8906671 [details] [diff] [review] CallInfo::pushFormals is now responsible for allocating space fun.apply arguments. Review of attachment 8906671 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit-test/tests/ion/array-push-multiple-with-funapply.js @@ +22,5 @@ > + > +if (!("oomAtAllocation" in this)) > + quit(); > +if (canIoncompile() != true) > + quit(); Still not a fan of restricting tests to certain flags/modes. This test is also pretty complicated - the bug here is not an OOM issue IIUC so I'm not sure why we need oomAtAllocation etc.
Attachment #8906671 -
Flags: review?(jdemooij) → review+
Assignee | ||
Comment 10•7 years ago
|
||
(In reply to Nicolas B. Pierron [:nbp] from comment #7) > This patch adds a test case which reproduces the same assertion seen on the > website mentioned in comment 2. I verified, with the patch, I am unable to cause the crash seen previously on the URL from comment 2.
Comment 12•7 years ago
|
||
Adding a Linux signature which started using 20170907220212.
Crash Signature: [@ js::jit::LRecoverInfo::OperandIter::settle]
[@ js::jit::LRecoverInfo::appendResumePoint]
[@ js::jit::LRecoverInfo::New]
[@ js::jit::MBasicBlock::flagOperandsOfPrunedBranches] → [@ js::jit::LRecoverInfo::OperandIter::settle]
[@ js::jit::LRecoverInfo::appendResumePoint]
[@ js::jit::LRecoverInfo::New]
[@ js::jit::MBasicBlock::flagOperandsOfPrunedBranches]
[@ js::jit::LIRGeneratorShared::assignSafepoint]
Comment 13•7 years ago
|
||
Pushed by npierron@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d7bb391e9644 CallInfo::pushFormals is now responsible for allocating space fun.apply arguments. r=jandem
Updated•7 years ago
|
tracking-firefox57:
--- → +
Comment 15•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d7bb391e9644
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Updated•7 years ago
|
Priority: -- → P1
Comment hidden (spam) |
You need to log in
before you can comment on or make changes to this bug.
Description
•