Closed Bug 1202367 Opened 5 years ago Closed 5 years ago

Bug in RegionLock::acquire()

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: lth, Assigned: lth)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

On most platforms this primitive uses a compareExchange in a loop.  When used with GCC __atomic operations (the default most places) or with C11 atomics the atomic_compare_exchange primitive will update the "expected" variable if the CAS fails.  This will break the algorithm; the "expected" variable must be reinitialized on the failing back-edge in the loop.

This is not an important bug since the RegionLock is not currently used, it was introduced for float64 atomics on 32-bit platforms, but that code has not landed.

Since ARM64 and MIPS have copied my broken implementations there will need to be fixes on thos platforms too.
Comment on attachment 8657737 [details] [diff] [review]
Reinitialize expected value inside the lock loop

Review of attachment 8657737 [details] [diff] [review]:
-----------------------------------------------------------------

They think it don't be like it is, but it do.
Attachment #8657737 - Flags: review?(sstangl) → review+
Comment on attachment 8657753 [details] [diff] [review]
MIPS: Reinitialize expected value inside CAS loop

Review of attachment 8657753 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the patch!
Attachment #8657753 - Flags: review?(sstangl) → review+
Attachment #8657753 - Flags: checkin?(lhansen)
Attachment #8657753 - Flags: checkin?(lhansen) → checkin+
You need to log in before you can comment on or make changes to this bug.