1891195 - Assertion failure: asValue() % sizeof(std::conditional_t<std::is_void_v<std::remove_pointer_t<U>>, char, std::remove_pointer_t<U>>) == 0, at vm/SharedMem.h:84

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: debug stack

for (let i = 0; i < 999; i++) {
  new BigInt64Array(new BigInt64Array(createUserArrayBuffer(function(){})));
}

(gdb) bt
#0  SharedMem<void*>::cast<long long*> (this=0xffffbdb0) at /home/ubu32gx500/trees/mozilla-central/js/src/vm/SharedMem.h:80
#1  js::ElementSpecific<long long, js::UnsharedOps>::setFromTypedArray (target=..., targetLength=0, source=..., sourceLength=0, offset=0) at /home/ubu32gx500/trees/mozilla-central/js/src/vm/TypedArrayObject-inl.h:322
#2  0x586302ee in (anonymous namespace)::TypedArrayObjectTemplate<long long>::fromTypedArray (cx=0xf761a100, other=..., isWrapped=<optimized out>, proto=...) at /home/ubu32gx500/trees/mozilla-central/js/src/vm/TypedArrayObject.cpp:1340
#3  0x585fedbd in (anonymous namespace)::TypedArrayObjectTemplate<long long>::fromArray (cx=0xf761a100, other=..., proto=...) at /home/ubu32gx500/trees/mozilla-central/js/src/vm/TypedArrayObject.cpp:1254
#4  0x585e4fa6 in (anonymous namespace)::TypedArrayObjectTemplate<long long>::create (args=..., cx=<optimized out>) at /home/ubu32gx500/trees/mozilla-central/js/src/vm/TypedArrayObject.cpp:528
#5  (anonymous namespace)::TypedArrayObjectTemplate<long long>::class_constructor (cx=0xf761a100, argc=1, vp=0xf662e058) at /home/ubu32gx500/trees/mozilla-central/js/src/vm/TypedArrayObject.cpp:488
/snip

Run with --fuzzing-safe --no-threads --no-baseline --no-ion, compile with 'CXX="clang++ -msse2 -mfpmath=sse"' PKG_CONFIG_PATH=/usr/lib/x86_64-linux-gnu/pkgconfig 'CC="clang -msse2 -mfpmath=sse"' AR=ar sh ../configure --host=x86_64-pc-linux-gnu --target=i686-pc-linux --enable-debug --enable-debug-symbols --with-ccache --enable-gczeal --enable-rust-simd --disable-tests, tested on m-c rev 036ac9a41b52.

This goes as far back as m-c rev 443c7bf9d76b (August 2023), will try and go back further.

Setting s-s as a start, and since this seems to involve createUserArrayBuffer, setting needinfo? from :anba.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

I'll confirm again in a few hours but the regression range is trending towards the vicinity of bug 1841113 via https://hg.mozilla.org/mozilla-central/rev/05d609026ed78c4a130db46ab03e65d49a167902 .

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1841113

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Due to skipped revisions, the first bad revision could be any of:
changeset:   https://hg.mozilla.org/mozilla-central/rev/5fc7d804af3e
user:        André Bargull
date:        Fri Jul 07 13:07:56 2023 +0000
summary:     Bug 1841113 - Part 2: Implement ArrayBuffer transfer proposal. r=spidermonkey-reviewers,jandem

changeset:   https://hg.mozilla.org/mozilla-central/rev/05d609026ed7
user:        André Bargull
date:        Fri Jul 07 13:07:56 2023 +0000
summary:     Bug 1841113 - Part 3: Add additional ArrayBuffer transfer proposal tests. r=spidermonkey-reviewers,jandem

changeset:   https://hg.mozilla.org/mozilla-central/rev/41d2512a836c
user:        André Bargull
date:        Fri Jul 07 13:07:56 2023 +0000
summary:     Bug 1841113 - Part 4: Steal or realloc malloced buffers. r=spidermonkey-reviewers,jandem

Andre, is bug 1841113 a likely regressor?

Comment 4

[André Bargull [:anba]]

(In reply to Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) from comment #3)

Andre, is bug 1841113 a likely regressor?

Bug 1841113 added the testing function createUserArrayBuffer, but the assertion can also be triggered through other means, for example the createExternalArrayBuffer testing function:

for (let i = 1; i < 100; ++i) {
  new Float64Array(new Float64Array(createExternalArrayBuffer(0)));
}

Comment 5

[André Bargull [:anba]]

This is just a too zealous assertion:

• ElementSpecific::setFromTypedArray calls SharedMem::cast which asserts here the memory is aligned.
• This is correct per-se, but doesn't handle the zero memory case.
• std::malloc(0) has implementation defined behaviour and isn't required to be aligned to max_align_t.
• And when using jemalloc, malloc(0) will use its tiniest allocation class, so the memory is aligned to sizeof(void*), which means four byte alignment.
• BigInt64Array / BigUint64Array / Float64Array require eight byte alignment, so four byte aligned memory triggers the assertion.

The fix is to use an early return when copying zero elements, so SharedMem::cast won't be called and its assertion won't be triggered.

This issue isn't security-sensitive and can be opened up.

Comment 6

[André Bargull [:anba]]

Attachment: Bug 1891195: Don't assert when copying zero bytes from unaligned memory. r=sfink!

SharedMem::cast asserts when the memory is unaligned to the target type, so
we have to exit early from ElementSpecific::setFromTypedArray when copying
zero elements, because malloc(0) can return unaligned memory. malloc(n)
with n > 0 is required to return memory at least as strictly aligned as for
max_align_t, so when sourceLength > 0, SharedMem::cast won't trigger an
assertion. A static assertion was added to ensure this property always holds.

Additionally added runtime assertion to check (Shared)ArrayBuffers are always
created with properly aligned memory (i.e. aligned to ARRAY_BUFFER_ALIGNMENT).
Furthermore ARRAY_BUFFER_ALIGNMENT is now static asserted to match to the
size of the largest possible typed array scalar.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/46ce6a4ee76e
Don't assert when copying zero bytes from unaligned memory. r=sfink

Comment 8

[pstanciu]

Backed out for causing SM bustages on Assertions.h

• Backout link
• Push with failures
• Failure Log

Comment 9

[André Bargull [:anba]]

Sigh, old compilers..

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f422d60880fc
Don't assert when copying zero bytes from unaligned memory. r=sfink

Comment 11

[Cristian Tuns]

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