1355554 - Atomics don't support wrapped TypedArrays

Status: RESOLVED FIXED

(Core :: JavaScript: Standard Library, enhancement, P3)

Description

[André Bargull [:anba]]

Test case:
---
var g = newGlobal();
var ta = g.eval("new Int32Array(new SharedArrayBuffer(4))");
Atomics.add(ta, 0, 1);
---

Expected: No TypeError
Actual: Throws TypeError

Comment 1

[André Bargull [:anba]]

Taking this bug, so I can use it for an Atomics overhaul to improve spec-compatibility.

Comment 2

[André Bargull [:anba]]

Attachment: Bug 1355554 - Part 1: Handle cross-compartment wrappers in AtomicsObject. r=jorendorff!

Comment 3

[André Bargull [:anba]]

Attachment: Bug 1355554 - Part 2: Call Value::setNumber(uint32_t) instead of the double overload. r=jorendorff!

This is a bit faster, because it only requires a range check and not a
"is this double an integer" check.

Depends on D24074

Comment 4

[André Bargull [:anba]]

Attachment: Bug 1355554 - Part 2: Add step comments and update some variable names to follow spec. r=jorendorff!

Variable names were not updated to follow the spec when the spec uses single
letter names, for example if the current name is "timeout", but the spec uses
"t", the name was left unchanged for better code readability.

Depends on D24075

Comment 5

[André Bargull [:anba]]

Attachment: Bug 1355554 - Part 4: Remove unused methods. r=jorendorff!

These methods were previously used for atomic operations on Uint8ClampedArray, but
this feature was removed some time ago.

Depends on D24076

Comment 6

[André Bargull [:anba]]

Attachment: Bug 1355554 - Part 3: Rename AtomicsBinop to AtomicReadModifyWrite and remove macros. r=jorendorff!

And update Atomics.exchange to use AtomicReadModifyWrite per spec.

Depends on D24077

Comment 7

[André Bargull [:anba]]

Attachment: Bug 1355554 - Part 6: Fold ExchangeOrStore into js::atomics_store. r=jorendorff!

Depends on D24078

Comment 8

[André Bargull [:anba]]

Attachment: Bug 1355554 - Part 4: Rename GetSharedTypedArray and GetTypedArrayIndex per spec. r=jorendorff!

• GetSharedTypedArray is ValidateSharedIntegerTypedArray in the spec
• and GetTypedArrayIndex is ValidateAtomicAccess.

Depends on D24079

Comment 9

[André Bargull [:anba]]

Attachment: Bug 1355554 - Part 8: Validate TypedArray kind before coercing the rest of the arguments. r=jorendorff!

Depends on D24080

Comment 10

[André Bargull [:anba]]

Attachment: Bug 1355554 - Part 9: Add test262 tests. r=jorendorff!

Depends on D24081

Comment 11

[André Bargull [:anba]]

Attachment: Bug 1355554 - Part 5: Replace byte-offset computation with call to TypedArrayObject::byteOffset(). r=jorendorff!

Depends on D24082

Comment 12

[André Bargull [:anba]]

Attachment: Bug 1355554 - Part 6: Remove Atomics functions from header file. r=jorendorff!

Depends on D24083

Comment 13

[André Bargull [:anba]]

Attachment: Bug 1355554 - Part 7: Remove outdated comments about atomics implementation details. r=jorendorff!

Depends on D24084

Comment 14

[André Bargull [:anba]]

Attachment: Bug 1355554 - Part 13: Move Int8 case before Uint8 to match other switch-statements. r=jorendorff!

Depends on D24085

Comment 15

[André Bargull [:anba]]

Attachment: Bug 1355554 - Part 14: Add inline helpers to avoid code repetition. r=jorendorff!

The switch-statements in CompareExchange, atomics_load, atomics_store, and
AtomicReadModifyWrite repeat the same type casts for each scalar type. Add
helper functions to reduce this code duplication.

Depends on D24086

Comment 16

[Jason Orendorff [:jorendorff]]

Anba, do you want to land these?

Comment 17

[André Bargull [:anba]]

The patches need to be rewritten, because they no longer apply after the BigInt integration for Atomics.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f81d65748d1f
Part 1: Handle cross-compartment wrappers in AtomicsObject. r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/986b2ba8582f
Part 2: Add step comments and update some variable names to follow spec. r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/bd174ee04205
Part 3: Rename AtomicsBinop to AtomicReadModifyWrite and remove macros. r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/4f297a91c4cb
Part 4: Rename GetSharedTypedArray and GetTypedArrayIndex per spec. r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/5ef0fd33df8f
Part 5: Replace byte-offset computation with call to TypedArrayObject::byteOffset(). r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/646b52cf410f
Part 6: Remove Atomics functions from header file. r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/b4607c8f4661
Part 7: Remove outdated comments about atomics implementation details. r=jorendorff

Comment 19

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/f81d65748d1f
https://hg.mozilla.org/mozilla-central/rev/986b2ba8582f
https://hg.mozilla.org/mozilla-central/rev/bd174ee04205
https://hg.mozilla.org/mozilla-central/rev/4f297a91c4cb
https://hg.mozilla.org/mozilla-central/rev/5ef0fd33df8f
https://hg.mozilla.org/mozilla-central/rev/646b52cf410f
https://hg.mozilla.org/mozilla-central/rev/b4607c8f4661