Closed Bug 1590539 Opened 9 months ago Closed 9 months ago

Atomics.load() and friends should check the TypedArray type before coercing the `index` argument

Categories

(Core :: JavaScript Engine, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox72 --- fixed

People

(Reporter: jorendorff, Assigned: lth)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

The first two steps of AtomicReadModifyWrite are:

  1. Let buffer be ? ValidateSharedIntegerTypedArray(typedArray).
  2. Let i be ? ValidateAtomicAccess(typedArray, index).

We fail some test262 tests checking the order of these two steps.

I don't know if this really blocks shipping SAB, but adding the dep so it gets looked at.

Blocks: resab

The tests that fail are:

test262/built-ins/Atomics/add/validate-arraytype-before-index-coercion.js
test262/built-ins/Atomics/and/validate-arraytype-before-index-coercion.js
test262/built-ins/Atomics/compareExchange/validate-arraytype-before-index-coercion.js
test262/built-ins/Atomics/exchange/validate-arraytype-before-index-coercion.js
test262/built-ins/Atomics/load/validate-arraytype-before-index-coercion.js
test262/built-ins/Atomics/or/validate-arraytype-before-index-coercion.js
test262/built-ins/Atomics/store/validate-arraytype-before-index-coercion.js
test262/built-ins/Atomics/sub/validate-arraytype-before-index-coercion.js
test262/built-ins/Atomics/xor/validate-arraytype-before-index-coercion.js

That's weird, because the code in builtins/AtomicsObject.cpp has these tests in the right order, as does the code in MCallOptimize.cpp...

Assignee: nobody → lhansen

I see, the problem is that in order to avoid dispatching on type twice, we first check whether it's a shared TypedArray, then convert the index, and only then filter for the element type of the array. The filtering for the element type needs to happen during the first step.

This totally does not block shipping SAB, but it's not a problem for it to hang off that bug, I think. I'll fix it this week.

Spec compliance requires us to check the element type of the TypedArray at the same time
as we check it's a shared TypedArray, not later. This results in some tests being done
twice, but only for the slow C++ path.

Priority: -- → P2
Type: task → defect
Status: NEW → ASSIGNED
Pushed by lhansen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6be77abcce7f
Filter atomics ops for type early.  r=jorendorff
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
You need to log in before you can comment on or make changes to this bug.