PJS: Array.prototype.scanPar per-element thunk reads and writes result array; vulnerable to bailout-and-restart

RESOLVED FIXED in mozilla33

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: lth, Assigned: lth)

Tracking

unspecified
mozilla33
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 years ago
Array.prototype.scanPar() (Array.js, function ArrayScanPar) is a three-phase operation where the last phase, "phase2", both reads from and writes to the working array "buffer".  The phase is not racy, but since it updates the array in place it will compute the wrong result if the phase bails out and is restarted.

The problem is seen on jit-test/tests/parallel/Array-scanPar-sorted.js (at least on my Quad Linux64 system), which must be run with the --slow switch to the jit_test.py harness.

The problem is probably also the cause of intermittent apparent-regression bug 1023262, which started failing on some platforms on Array-scanpar-sum after PJS GC landed (PJS GC may change the timing of bailout behavior, in particular it tends to change the timing of bailouts for Zone GC, since those tend to be diminished).
(Assignee)

Comment 1

4 years ago
Created attachment 8438348 [details] [diff] [review]
Patch

Arguably a stopgap solution: it allocates a second buffer and computes the final result into it.
Attachment #8438348 - Flags: review?(nmatsakis)
(Assignee)

Updated

4 years ago
Attachment #8438348 - Flags: review?(nmatsakis)
(Assignee)

Comment 3

4 years ago
Created attachment 8438560 [details] [diff] [review]
Patch, v2

https://tbpl.mozilla.org/?tree=Try&rev=893c66b33a03
Attachment #8438348 - Attachment is obsolete: true
Attachment #8438560 - Flags: review?(nmatsakis)
Attachment #8438560 - Flags: review?(nmatsakis) → review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/661dfa9fb949

Friendly reminder that your commit message should be summarizing what the patch is actually doing, not just restating the problem it's fixing. Thanks!
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Committing_Rules_and_Responsibilities#Checkin_comment
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/661dfa9fb949
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.