2005448 - Wasm-GC code crashes with --gc-param=semispaceNurseryEnabled=1

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect, P3)

Description

[Julian Seward [:jseward]]

.. whereas --gc-param=semispaceNurseryEnabled=0 seems OK.

STR:

x86_64-linux, Fedora 42.

This revision:

  ◆  zvtqozzk [e-mail] 2025-12-11 07:12:40 main git_head() 54da8f6b
  │  Bug 2005307 - Send pre-flight headers when communicating with MARS
     over OHTTP for sponsored topsites. r=home-newtab-reviewers,nbarrett

A version of JetStream3 that has the j2cl-box2d-wasm test is needed. I am
using:

  https://github.com/danleh/JetStream.git

  commit ab0c2eb94af3857e62ffd3bc0931b4c6ebff81ec
    (HEAD -> j2cl-box2d, origin/j2cl-box2d)

  commit 0fc1bfeaf1dc921bd08d089d2e92ebfeb92c1b50
    (origin/main, origin/HEAD, main)

Configure, build:

  mkdir -p BX64/DOG

  cd BX64/DOG

  CC="ccache clang" CXX="ccache clang++" ../../src/configure \
    --enable-debug --enable-optimize="-g -Og"  --disable-tests \
    --disable-shared-js --disable-sysroot

  make --quiet -j8

Run, no failure:

  (cd /nfs/info/JETSTREAM3/JetStreamJ2CLBox2D && \
     /nfs/compx/MC_ALT/js/BX64/DOG/dist/bin/js --spectre-mitigations=off \
     --write-protect-code=off --gc-param=semispaceNurseryEnabled=0 \
     cli.js j2cl-box2d-wasm)

Run, fails:

• as above but with --gc-param=semispaceNurseryEnabled=1

• There are 3 possible outcomes:

  (1) Assertion failure: zone->runtimeFromAnyThread() == rt,
  at /nfs/compx/MC_ALT/js/src/gc/Marking.cpp:166

  (2) It completes successfully

  (3) Segmentation fault (core dumped)

  These seem to happen randomly, each roughly 1/3 of the time.

Notes:

(1) Depends on optimisation level. I tried also with --disable-optimize
and was not easily able to reproduce the failure.

(2) Running on valgrind (requires --enable-valgrind --disable-jemalloc in
configure) gets many complaints of uninitialised value uses resulting
from poisoning in the JS heap, with --gc-param=semispaceNurseryEnabled=1.
With it set to =0 there are no such complaints.

I wonder if there is some kind of race condition, in which the mutator and
collector are running in parallel, but uncoordinated.

This is possibly (or possibly not) related to sporadic crashing seen during
work on bug 1992888.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: Bug 2005448 - Make Wasm GC array postbarriers less conservative so they work with semispace nursery r=jseward!,sfink!

The problem here is that wasm GC barriers for arrays work by casting elements
to GCPtr, and that is conservative about what it considers tenured. Without
semispace this is suboptimal but works. It doesn't work with semispace enabled.

The conservative part comes from CellPtrEdge::maybeInRememberedSet which checks
the address against the nursery. This will return true for allocations owned
by nursery things that are not allocated in nursery memory, in this case wasm
GC array trailers.

This has the effect that we will add store buffer entries for edges we don't
need to. Without semispace enabled this means we will tenure more things than
we need to in minor GC, including things only reachable through dead nursery
things. With semispace enabled we may keep storebuffer edges in dead nursery
things which results in UAF on the next nursery collection.

The approach of using individual cell pointer edges for the array element post
barrier is itself suboptimal which may be why this problem has not shown up
elsewhere. For JSObject arrays we use the slots edge store buffer where an
entry can apply to a range of slots, rather than add an entrie for each
element. The JIT may just use the whole cell store buffer.

The patch fixes this by selecting the barrier wrapper class to use based on
whether the owner is tenured or not. GCPtr is appopriate for tenured things
(and has the postbarrier) and PreBarriered is used for nursery thing (and
doesn't have the prebarrier). This approach is kind of hacky, but is used in
other places in the engine (e.g. weakmaps). I previously filed bug 2003769
about this.

This also adds some storebuffer APIs to add edges you know are from tenured
objects to skip the maybeInRememberedSet check. For wasm GC this means we can
check this in JIT code and maybe skip the VM call for the precise barrier
(which turned out not tto be so precise).

There were some performance changes for the shell when I tested on macOS but
Linux wasn't much affected. I'm in the process of running performance tests for
the browser.

Comment 2

[Julian Seward [:jseward]]

With the patch in comment 1, I can no longer reproduce this crashing. Thanks.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/76eabb0febd4
https://hg.mozilla.org/integration/autoland/rev/340a1d966eb4
Make Wasm GC array postbarriers less conservative so they work with semispace nursery r=sfink,rhunt

Comment 4

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/340a1d966eb4