Closed
Bug 1112632
Opened 9 years ago
Closed 9 years ago
Scalar Replacement & Eliminate*DeadResumePointOperands can cause invalid bailouts.
Categories
(Core :: JavaScript Engine: JIT, defect)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla37
Tracking | Status | |
---|---|---|
firefox35 | --- | unaffected |
firefox36 | + | fixed |
firefox37 | --- | fixed |
People
(Reporter: nbp, Assigned: nbp)
References
Details
Attachments
(1 file)
1.51 KB,
patch
|
jandem
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Scalar Replacement can flag instructions as being recovered on bailout, and replace uses of the object / array by a constant / slot-value instead. Combined with EliminateTriviallyDeadResumePointOperands, this can shorten the live-span of recovered instructions. In case of bailout, this means that where we expected an object we now have a MagicValue flowing on the stack, which might be responsible for crashes / interrupted executions of JavaScript. This bug appeared, during the test of Bug 1073033 & Bug 991720 & Bug 1110939, in the test suite while running compacting GC with ion/recover-array.js (arrayWithGCInit) test case. This likely needs to be backported back to the addition of Scalar Replacement, such that we do not risk crashing because of Magic values flowing into baseline.
Assignee | ||
Comment 1•9 years ago
|
||
[Tracking Requested - why for this release]: Tracking Fx36, as this issue might be cause crashes / interruption of the JS code.
Blocks: 1069307
status-firefox35:
--- → unaffected
status-firefox36:
--- → affected
status-firefox37:
--- → affected
tracking-firefox36:
--- → ?
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8537901 -
Flags: review?(jdemooij)
Comment 4•9 years ago
|
||
Comment on attachment 8537901 [details] [diff] [review] Flag object/array allocations as implictly used as we are removing uses. Review of attachment 8537901 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/ScalarReplacement.cpp @@ +251,5 @@ > : alloc_(alloc), > obj_(obj), > startBlock_(obj->block()) > { > + // Annoate the instruction such that we do not replace it by a Nite: Annotate (typo) @@ +254,5 @@ > { > + // Annoate the instruction such that we do not replace it by a > + // Magic(JS_OPTIMIZED_OUT) in case of removed uses. > + if (!obj_->isImplicitlyUsed()) > + obj_->setImplicitlyUsed(); Nit: instead of the if you can also do obj_->setImplicitlyUsedUnchecked(). @@ +702,5 @@ > arr_(arr), > startBlock_(arr->block()), > state_(nullptr) > { > + // Annoate the instruction such that we do not replace it by a Same here.
Attachment #8537901 -
Flags: review?(jdemooij) → review+
Assignee | ||
Comment 5•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f4eba33e0d61
Comment 6•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f4eba33e0d61
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Comment 7•9 years ago
|
||
Nicolas, can we have an uplift request for aurora? thanks
Flags: needinfo?(nicolas.b.pierron)
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 8537901 [details] [diff] [review] Flag object/array allocations as implictly used as we are removing uses. Approval Request Comment [Feature/regressing bug #]: Bug 1069307 [User impact if declined]: This issue might be cause crashes / interruption of the JS code. [Describe test coverage new/current, TBPL]: 2 weeks of mozilla-inbound & central. [Risks and why]: Low risk, as this patch add flags which are used to prevent optimizations. [String/UUID change made/needed]: N/A
Flags: needinfo?(nicolas.b.pierron)
Attachment #8537901 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
Attachment #8537901 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 9•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/5200b09176a2
Updated•9 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•