1129202 - Implement CanonicalNumericIndexString and use it for typed arrays

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Tom S [:evilpie]]

ES6 imposes all sorts of stuff on array indexes in typed arrays. See for example [[HasProperty]]

If IsDetachedBuffer(buffer) is true, throw a TypeError exception.
If IsInteger(index) is false then return false
If index = −0, return false.

We have IsTypedArrayIndex, but that doesn't accept doubles, like CanonicalNumericIndexString

After this is implemented the MOP needs to be audited and corrected.

Comment 1

[Morgan Phillips [:mrrrgn]]

By my reading this should be a valid test case:

let a = new Int8Array([1]);
assertEq(Object.getOwnPropertyDescriptor(a, "-0"), a[-0]);
assertEq(Object.getOwnPropertyDescriptor(a, "0.0"), a[0.0]);

Comment 2

[Morgan Phillips [:mrrrgn]]

arg, two problems: * Object.getOwnPropertyDescriptor(a, x).value

and these test cases are not valid because of step 4. in  CanonicalNumericIndexString

Comment 3

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.

Comment 4

[Tom S [:evilpie]]

Search for (String)IsTypedArrayIndex should find a lot of relevant places that need to be replaced by CanonicalNumericIndexString.

Comment 6

[André Bargull [:anba]]

Attachment: Bug 1129202 - Part 1: Add assertion to check strings are null-terminated. r=jandem!

The assertion should make it easier to see why no length checks are used and
will make it easier to remember updating this code if we ever remove flat
strings.

Comment 7

[André Bargull [:anba]]

Attachment: Bug 1129202 - Part 1: Allow "Infinity" and "NaN" for canonical numeric index strings. r=jandem!

Depends on D39034

Comment 8

[André Bargull [:anba]]

Attachment: Bug 1129202 - Part 2: Move IsInteger to jsnum.h to allow reusing it. r=jandem!

Part 5 will call this function.

Depends on D39035

Comment 9

[André Bargull [:anba]]

Attachment: Bug 1129202 - Part 3: Change StringIsTypedArrayIndex's return type to allow passing an error. r=jandem!

The next patch adds code which may fail on OOM, so StringIsTypedArrayIndex's
interface needs to be updated to allow passing on errors.

Depends on D39036

Comment 10

[André Bargull [:anba]]

Attachment: Bug 1129202 - Part 4: Allow numbers with fractional parts or exponents for canonical numeric indices. r=jandem!

Depends on D39037

Comment 11

[André Bargull [:anba]]

Attachment: Bug 1129202 - Part 5: Enable now passing test262 tests. r=jandem!

Depends on D39038

Comment 12

[André Bargull [:anba]]

Attachment: Bug 1129202 - Part 6: Add constant for maximum TypedArray length. r=jandem!

The next parts use this constant in static_asserts.

Depends on D39039

Comment 13

[André Bargull [:anba]]

Attachment: Bug 1129202 - Part 7: Add MTypedArrayIndexToInt32 for accessing TypedArrays with any Double index. r=jandem!

The definition of canonical numeric index strings means that any access into a TypedArray
with a Double index never reads from the prototype chain. Instead if the Double is not
representable as an Int32, the access is equivalent to an out-of-bounds access. That
means we can substitute any non-Int32 Double index with an arbitrary index which is OOB.
This helps to avoid deoptimising TypedArray accesses with non-Int32 Double values by
handling them as normal out-of-bounds accesses, which are already optimisable.

Depends on D39040

Comment 14

[André Bargull [:anba]]

Attachment: Bug 1129202 - Part 8: Handle TypedArray non-int32 indices in SetPropIRGenerator. r=jandem!

Handle and report out-of-bounds accesses in SetPropIRGenerator to ensure Ion won't
repeatedly bailout when out-of-bounds accesses happen.

Depends on D39041

Comment 15

[André Bargull [:anba]]

Attachment: Bug 1129202 - Part 9: Handle TypedArray non-int32 indices in HasPropIRGenerator. r=jandem!

That way CacheIR can still attach an IC and Baseline/Ion don't need to take the
slow path when a TypedArray access happens through a Double not representable as
an Int32.

Depends on D39043

Comment 16

[André Bargull [:anba]]

Attachment: Bug 1129202 - Part 10: Handle TypedArray non-int32 indices in GetPropIRGenerator. r=jandem!

That way CacheIR can still attach an IC and Baseline/Ion don't need to take the
slow path when a TypedArray access happens through a Double not representable as
an Int32.

Depends on D39044

Comment 17

[André Bargull [:anba]]

Filed bug 1573803 for Phabricator not displaying the change sets.

Comment 18

[Jan de Mooij [:jandem]]

Hey I'm seeing this stack for the first time now because the patches are in draft state! Let me know if you want me to review :)

Comment 19

[André Bargull [:anba]]

I'll probably request review when the BigInt CacheIR changes have landed.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eafcc6ace2fb
Part 1: Allow "Infinity" and "NaN" for canonical numeric index strings. r=jandem
https://hg.mozilla.org/integration/autoland/rev/2c9bedcdac37
Part 2: Move IsInteger to jsnum.h to allow reusing it. r=jandem
https://hg.mozilla.org/integration/autoland/rev/041413289d85
Part 3: Change StringIsTypedArrayIndex's return type to allow passing an error. r=jandem
https://hg.mozilla.org/integration/autoland/rev/77c80b5f79ea
Part 4: Allow numbers with fractional parts or exponents for canonical numeric indices.  r=jandem
https://hg.mozilla.org/integration/autoland/rev/055d1cad290a
Part 5: Enable now passing test262 tests. r=jandem
https://hg.mozilla.org/integration/autoland/rev/288125d24d23
Part 6: Add constant for maximum TypedArray length. r=jandem
https://hg.mozilla.org/integration/autoland/rev/825e8843d0b1
Part 7: Add MTypedArrayIndexToInt32 for accessing TypedArrays with any Double index. r=jandem
https://hg.mozilla.org/integration/autoland/rev/371d2efe156c
Part 8: Handle TypedArray non-int32 indices in SetPropIRGenerator. r=jandem
https://hg.mozilla.org/integration/autoland/rev/cbeae82d295c
Part 9: Handle TypedArray non-int32 indices in HasPropIRGenerator. r=jandem
https://hg.mozilla.org/integration/autoland/rev/7b07bb35e7b2
Part 10: Handle TypedArray non-int32 indices in GetPropIRGenerator. r=jandem

Comment 21

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/eafcc6ace2fb
https://hg.mozilla.org/mozilla-central/rev/2c9bedcdac37
https://hg.mozilla.org/mozilla-central/rev/041413289d85
https://hg.mozilla.org/mozilla-central/rev/77c80b5f79ea
https://hg.mozilla.org/mozilla-central/rev/055d1cad290a
https://hg.mozilla.org/mozilla-central/rev/288125d24d23
https://hg.mozilla.org/mozilla-central/rev/825e8843d0b1
https://hg.mozilla.org/mozilla-central/rev/371d2efe156c
https://hg.mozilla.org/mozilla-central/rev/cbeae82d295c
https://hg.mozilla.org/mozilla-central/rev/7b07bb35e7b2