1674777 - Audit JSAPI and add jsapi-tests for large ArrayBuffers

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Jan de Mooij [:jandem]]

This is not the most urgent thing to address for bug 1673557, but we should ensure we have proper tests for this eventually.

Especially the APIs that don't go through the internal accessors are easy to overlook in an audit.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Bug 1674777 part 1 - Change JS::NewArrayBuffer and JS_New*Array size argument from uint32_t to size_t. r?sfink!

A later patch will add a jsapi-test for this.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1674777 part 2 - Enable large ArrayBuffers for jsapi-tests. r?sfink!

Depends on D103273

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1674777 part 3 - Change Get*LengthAndData outparam from uint32_t to size_t. r?sfink!

Depends on D103274

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1674777 part 4 - Change some JSAPI byteLength and byteOffset accessors to return size_t instead of uint32_t. r?sfink!

Of these four, only JS_GetTypedArrayByteLength is used outside jsapi-tests.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d55cbd2be32d
part 1 - Change JS::NewArrayBuffer and JS_New*Array size argument from uint32_t to size_t. r=sfink
https://hg.mozilla.org/integration/autoland/rev/f330737adc50
part 2 - Enable large ArrayBuffers for jsapi-tests. r=sfink
https://hg.mozilla.org/integration/autoland/rev/5539557bbfd3
part 3 - Change Get*LengthAndData outparam from uint32_t to size_t. r=sfink,aosmond
https://hg.mozilla.org/integration/autoland/rev/94684c2cfbc4
part 4 - Change some JSAPI byteLength and byteOffset accessors to return size_t instead of uint32_t. r=sfink

Comment 6

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/d55cbd2be32d
https://hg.mozilla.org/mozilla-central/rev/f330737adc50
https://hg.mozilla.org/mozilla-central/rev/5539557bbfd3
https://hg.mozilla.org/mozilla-central/rev/94684c2cfbc4

Comment 7

[Jan de Mooij [:jandem]]

Attachment: Bug 1674777 part 5 - Change GetObjectAs* APIs to use size_t for outparam instead of uint32_t. r?sfink!,sg!

Note that in the IndexedDB code we pass a Span to Key::EncodeAsString where we check
for large lengths.

Comment 8

[Jan de Mooij [:jandem]]

Attachment: Bug 1674777 part 6 - Change SharedArrayBuffer APIs to use size_t instead of uint32_t. r?lth!

There's only one caller outside jsapi-tests so these are easy to convert.

Depends on D103679

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b142ae1ffe74
part 5 - Change GetObjectAs* APIs to use size_t for outparam instead of uint32_t. r=sfink,sg

Comment 10

[Jan de Mooij [:jandem]]

Attachment: Bug 1674777 part 7 - Change nsIBinaryInputStream.readArrayBuffer to use uint64_t instead of uint32_t. r?kmag!

We're adding support for ArrayBuffers larger than 4 GB to the JS engine (on 64-bit
platforms).

ReadArrayBuffer uses uint32_t values in a number of places. This patch ensures we
throw an exception for code like this: stream.readArrayBuffer(ab.byteLength, ab, ...)).

Comment 11

[Jan de Mooij [:jandem]]

Attachment: Bug 1674777 part 8 - Change nsIArrayBufferInputStream.setData arguments from uint32_t to uint64_t. r?kmag!

Similar to the previous patch. Prevent truncation when assigning to mBufferLength.

Depends on D103759

Comment 12

[Jan de Mooij [:jandem]]

Attachment: Bug 1674777 part 9 - Change GetArrayBufferByteLength return type from uint32_t to size_t. r?sfink!

Note that previous patches already fixed various other callers.

Depends on D103760

Comment 13

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/b142ae1ffe74

Comment 14

[Jan de Mooij [:jandem]]

Attachment: Bug 1674777 part 10 - Fix XPConnect callers for JS_GetTypedArrayLength returning size_t. r?kmag!

For enumerateNames on Xrays, throw an OOM exception upfront. This matches what we do
inside the JS engine when enumerating large typed arrays.

Comment 15

[Jan de Mooij [:jandem]]

Attachment: Bug 1674777 part 11 - Change JS_GetTypedArrayLength return type from uint32_t to size_t. r?sfink!

The callers in AudioWorkletNode.cpp are just comparing the length to the expected length
to guard against detachment.

Depends on D103902

Comment 16

[Jan de Mooij [:jandem]]

Attachment: Bug 1674777 part 12 - Add test for XPConnect array conversions involving large typed arrays. r?kmag!

Comment 17

[Jan de Mooij [:jandem]]

Attachment: Bug 1674777 part 13 - Remove deprecatedGetUint32. r?lth!

Also change wasmMaxSize and wasmMappedSize to use 'get' instead of 'deprecatedGetUint32'.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d4bd35d3c25d
part 6 - Change SharedArrayBuffer APIs to use size_t instead of uint32_t. r=lth

Comment 19

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/d4bd35d3c25d

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/031bc0b1e8e5
part 7 - Change nsIBinaryInputStream.readArrayBuffer to use uint64_t instead of uint32_t. r=kmag
https://hg.mozilla.org/integration/autoland/rev/e23dca0763fc
part 8 - Change nsIArrayBufferInputStream.setData arguments from uint32_t to uint64_t. r=kmag
https://hg.mozilla.org/integration/autoland/rev/42239f7a0ddf
part 9 - Change GetArrayBufferByteLength return type from uint32_t to size_t. r=sfink

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/03c71d1fa7f8
part 10 - Fix XPConnect callers for JS_GetTypedArrayLength returning size_t. r=kmag
https://hg.mozilla.org/integration/autoland/rev/ce348dc69837
part 11 - Change JS_GetTypedArrayLength return type from uint32_t to size_t. r=sfink
https://hg.mozilla.org/integration/autoland/rev/a5b7b61ae148
part 12 - Add test for XPConnect array conversions involving large typed arrays. r=kmag
https://hg.mozilla.org/integration/autoland/rev/f34c46915ae1
part 13 - Remove deprecatedGetUint32. r=lth

Comment 22

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/031bc0b1e8e5
https://hg.mozilla.org/mozilla-central/rev/e23dca0763fc
https://hg.mozilla.org/mozilla-central/rev/42239f7a0ddf

Comment 23

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/03c71d1fa7f8
https://hg.mozilla.org/mozilla-central/rev/ce348dc69837
https://hg.mozilla.org/mozilla-central/rev/a5b7b61ae148
https://hg.mozilla.org/mozilla-central/rev/f34c46915ae1