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)
Tracking
()
VERIFIED
FIXED
mozilla53
Tracking | Status | |
---|---|---|
firefox-esr45 | --- | unaffected |
firefox50 | --- | unaffected |
firefox51 | --- | unaffected |
firefox52 | --- | unaffected |
firefox53 | + | verified |
People
(Reporter: Virtual, Assigned: h4writer)
References
()
Details
(Keywords: crash, nightly-community, regression)
Crash Data
Attachments
(1 file)
1.58 KB,
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)
Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)
Reporter
|
||
Comment 1•8 years ago
|
||
Crashlog report:
https://crash-stats.mozilla.com/report/index/bp-b9d2f7db-b028-4a3a-9b2a-04df62161211
Crash Signature: [@ js::jit::MBasicBlock::insertBefore ]
Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)
Reporter
|
||
Updated•8 years ago
|
Blocks: 1310155
Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)
Reporter
|
||
Comment 2•8 years ago
|
||
"Speedy" Regression window (mozilla-central)
Good:
https://ftp.mozilla.org/pub/firefox/nightly/2016/12/2016-12-09-03-02-12-mozilla-central/
Bad:
https://ftp.mozilla.org/pub/firefox/nightly/2016/12/2016-12-10-03-02-06-mozilla-central/
Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d8f63b2935af0915a6a24b3ea8e27d9a09f66416&tochange=8404d26166a35406f46ff237ed132735c98882b2
Has Regression Range: --- → yes
Has STR: --- → yes
Also crashing on x86
https://crash-stats.mozilla.com/report/index/16cd5daa-6f31-4bd1-bd0a-3da612161212
Updated•8 years ago
|
status-firefox50:
--- → unaffected
status-firefox51:
--- → unaffected
Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)
Reporter
|
||
Updated•8 years ago
|
status-firefox-esr45:
--- → unaffected
Comment 4•8 years ago
|
||
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.
Comment 5•8 years ago
|
||
The Penny Arcade homepage also crashes us here.
https://crash-stats.mozilla.com/report/index/7f4bd0b6-80c6-48b3-9f5c-155d52161212
Comment 6•8 years ago
|
||
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
Comment 7•8 years ago
|
||
(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
Assignee | ||
Comment 9•8 years ago
|
||
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 12•8 years ago
|
||
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)
Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)
Reporter
|
||
Updated•8 years ago
|
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…
Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)
Reporter
|
||
Updated•8 years ago
|
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:: ... ]
Comment 14•8 years ago
|
||
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
Updated•8 years ago
|
Crash Signature: js::jit::BacktrackingAllocator::resolveControlFlow ] → js::jit::BacktrackingAllocator::resolveControlFlow ]
[@ js::jit::BacktrackingAllocator::mergeAndQueueRegisters]
Comment 15•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)
Reporter
|
||
Comment 17•8 years ago
|
||
No crashes since Firefox Nightly 53.0a1 (2016-12-16) build,
so I'm marking this bug as verified.
Status: RESOLVED → VERIFIED
Comment 18•8 years ago
|
||
(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)
Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)
Reporter
|
||
Updated•8 years ago
|
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 ]
Comment 23•8 years ago
|
||
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
Comment 24•8 years ago
|
||
(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.
Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)
Reporter
|
||
Updated•7 years ago
|
Keywords: nightly-community
Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)
Reporter
|
||
Updated•7 years ago
|
QA Contact: Virtual
Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)
Reporter
|
||
Updated•5 years ago
|
Keywords: crashreportid
You need to log in
before you can comment on or make changes to this bug.
Description
•