Closed Bug 745236 Opened 8 years ago Closed 8 years ago

fix f.apply(arguments) optimization bugs

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla14

People

(Reporter: luke, Assigned: luke)

Details

Attachments

(2 files)

I realized this morning that bug 740446 mishandles two corner cases with the f.apply(arguments) optimization stemming from the use of ssa instead of bytecode pattern-matching.  Insane test-cases, but trivial fixes.
Attached patch fix 1Splinter Review
Because a magic value can be left on the stack even after applySpeculationFailed, it is possible to call applySpeculationFailed even when needsArgsObj is true.
Attachment #614841 - Flags: review?(bhackett1024)
Attached patch fix 2Splinter Review
If 'arguments' is overwritten, we don't want to clobber it in applySpeculationFailed.
Attachment #614842 - Flags: review?(bhackett1024)
Attachment #614841 - Flags: review?(bhackett1024) → review+
(Note: there is one more fix to review)
Comment on attachment 614842 [details] [diff] [review]
fix 2

Ah, yeah, meant to review both and got distracted.
Attachment #614842 - Flags: review?(bhackett1024) → review+
Attachment #614841 - Flags: approval-mozilla-central?
Attachment #614842 - Flags: approval-mozilla-central?
Comment on attachment 614841 [details] [diff] [review]
fix 1

[triage comment]
low/no risk to mobile.
Attachment #614841 - Flags: approval-mozilla-central? → approval-mozilla-central+
Attachment #614842 - Flags: approval-mozilla-central? → approval-mozilla-central+
https://hg.mozilla.org/mozilla-central/rev/02abd512e339
https://hg.mozilla.org/mozilla-central/rev/19f452f79198
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.