Closed Bug 1322900 Opened 8 years ago Closed 8 years ago

Firefox Nightly 53.0a1 (2016-12-10) crashes in [@ js::jit:: ... ] & [@ ComputeImmediateDominators ]

Categories

(Core :: JavaScript Engine: JIT, defect)

Product:
Core
Component:
JavaScript Engine: JIT
Version:
53 Branch
Platform:
x86_64
Windows 7
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla53
Tracking Flags:
Tracking Status
firefox-esr45 --- unaffected
firefox50 --- unaffected
firefox51 --- unaffected
firefox52 --- unaffected
firefox53 + verified

People

(Reporter: Virtual, Assigned: h4writer)

References

(
URL
)

Details

(Keywords: crash, nightly-community, regression)

Crash Data

Attachments

(1 file)

Patch
(deleted), patch
jandem
: review+
Details | Diff | Splinter Review
STR: 1. Open this URL - http://www.pcgamer.com/watch-as-over-5000-eve-online-pilots-destroy-a-fully-operational-death-star/ and enjoy crash Probably caused by: bug #1310155 - Split IonBuilder into cfg and the rest [Tracking Requested - why for this release]: Regression
Flags: needinfo?(hv1989)
From what I can see another good example of a video that will reproduce this is any twitch video. https://www.twitch.tv/direwolf20/v/106794909 was an example I found, again reproducible.
Do we need bugs for all the new JIT crash signatures? In addition to Bug 1322943 which I filed, I also see these new signatures: http://bit.ly/2hrxmpP http://bit.ly/2gTydhO http://bit.ly/2hGVlA9 http://bit.ly/2gukjSB
(In reply to Marcia Knous [:marcia - use ni] from comment #6) > Do we need bugs for all the new JIT crash signatures? In addition to Bug > 1322943 which I filed, I also see these new signatures: > > http://bit.ly/2hrxmpP > http://bit.ly/2gTydhO > http://bit.ly/2hGVlA9 > http://bit.ly/2gukjSB And this has appeared from 1210 nightly, too. https://crash-stats.mozilla.com/report/index/6529780b-62cf-4cfe-a43c-06b332161211
Tracking 53+ - new crash with varied signatures that is reproducible.
tracking-firefox53: ? → +
Attached patch Patch (deleted) — — Splinter Review
Make sure we iterate all predecessors, even if we remove one. I think this fixes all crashes we were seeing. I'm not sure why the fuzzers haven't complained about this yet. Looking at what is wrong I would assume this would have been a fuzzblocker with "Assertion failure: hasLastIns()". Did anybody of you have such signatures? Could be hard to reduce to a single testcase. Just asking to be sure that is the case. Else I'll need to investigate further why this is hard to detect for fuzzers.
Assignee: nobody → hv1989
Flags: needinfo?(hv1989)
Flags: needinfo?(gary)
Flags: needinfo?(choller)
Attachment #8818579 - Flags: review?(jdemooij)
Comment on attachment 8818579 [details] [diff] [review] Patch Review of attachment 8818579 [details] [diff] [review]: ----------------------------------------------------------------- Good catch! ::: js/src/jit/MIRGraph.cpp @@ +195,5 @@ > for (size_t j = 0; j < block->numPredecessors(); j++) { > if (!block->getPredecessor(j)->isMarked()) > continue; > block->removePredecessor(block->getPredecessor(j)); > + j--; This will underflow when j is 0 and then the j++ following it will overflow to 0 again. For unsigned types like size_t this is well-defined and OK, but it might be simpler to remove the j++ in the loop head and do j++ before the continue.
Attachment #8818579 - Flags: review?(jdemooij) → review+
(In reply to Hannes Verschore [:h4writer] from comment #9) > I'm not sure why the fuzzers haven't complained about this yet. Looking at > what is wrong I would assume this would have been a fuzzblocker with > "Assertion failure: hasLastIns()". Did anybody of you have such signatures? Nope, didn't see it triggered.
Flags: needinfo?(gary)
Crash Signature: [@ js::jit::MBasicBlock::insertBefore ] → [@ js::jit::MBasicBlock::insertBefore ] [@ js::jit::MBasicBlock::numSuccessors ] [@ js::jit::IonBuilder::traverseBytecode ] [@ js::jit::BacktrackingAllocator::buildLivenessInfo] [@ js::jit::BacktrackingAllocator::moveAtEntry ] [@ js::jit::Backtrackin…
Summary: Firefox Nightly 53.0a1 (2016-12-10) crashes in [@ js::jit::MBasicBlock::insertBefore ] → Firefox Nightly 53.0a1 (2016-12-10) crashes in [@ js::jit:: ... ]
Pushed by hv1989@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/7efb1c8b5dfb IonMonkey: Make sure all predecessors are iterated during removal of successor blocks, r=jandem
Crash Signature: js::jit::BacktrackingAllocator::resolveControlFlow ] → js::jit::BacktrackingAllocator::resolveControlFlow ] [@ js::jit::BacktrackingAllocator::mergeAndQueueRegisters]
https://hg.mozilla.org/mozilla-central/rev/7efb1c8b5dfb
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
No crashes since Firefox Nightly 53.0a1 (2016-12-16) build, so I'm marking this bug as verified.
Status: RESOLVED → VERIFIED
status-firefox53: fixed → verified
(In reply to Hannes Verschore [:h4writer] from comment #9) > Created attachment 8818579 [details] [diff] [review] > Patch > > Make sure we iterate all predecessors, even if we remove one. > I think this fixes all crashes we were seeing. > > I'm not sure why the fuzzers haven't complained about this yet. Looking at > what is wrong I would assume this would have been a fuzzblocker with > "Assertion failure: hasLastIns()". Did anybody of you have such signatures? > Could be hard to reduce to a single testcase. Just asking to be sure that is > the case. Else I'll need to investigate further why this is hard to detect > for fuzzers. Sorry for the delayed response. I don't remember seeing this, for sure not as a fuzzblocker.
Flags: needinfo?(choller)
Crash Signature: [@ js::jit::MBasicBlock::insertBefore ] [@ js::jit::MBasicBlock::numSuccessors ] [@ js::jit::IonBuilder::traverseBytecode ] [@ js::jit::BacktrackingAllocator::buildLivenessInfo] [@ js::jit::BacktrackingAllocator::moveAtEntry ] [@ js::jit::Backtrackin… → [@ js::jit::MBasicBlock::insertBefore ] [@ js::jit::MBasicBlock::numSuccessors ] [@ js::jit::MBasicBlock::numSuccessors const ] [@ js::jit::IonBuilder::traverseBytecode ] [@ js::jit::BacktrackingAllocator::buildLivenessInfo ] [@ js::jit::Backtracking…
Summary: Firefox Nightly 53.0a1 (2016-12-10) crashes in [@ js::jit:: ... ] → Firefox Nightly 53.0a1 (2016-12-10) crashes in [@ js::jit:: ... ] & [@ ComputeImmediateDominators ]
Firefox 54.0a2 (2017-04-12) (64-bit) (aurora) just crashed with this listed as related bug. Is this a new thing or this bug also affects 54? https://crash-stats.mozilla.com/report/index/695ca973-da00-457c-bdfe-cddd12170413
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #23) > Firefox 54.0a2 (2017-04-12) (64-bit) (aurora) just crashed with this listed > as related bug. Is this a new thing or this bug also affects 54? > > https://crash-stats.mozilla.com/report/index/695ca973-da00-457c-bdfe- > cddd12170413 Looks as a few of the signatures have crashes in 54/55: js::jit::BacktrackingAllocator::buildLivenessInfo (April) js::jit::BuildDominatorTree (Feb, March and April) 54 only: js::jit::BacktrackingAllocator::resolveControlFlow (crashes in March/April) So it sounds like we need to file new bugs for any new issues.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: