Skip to content

Fix safepoint stack slot reuse#13469

Open
angelnereira wants to merge 1 commit into
bytecodealliance:mainfrom
angelnereira:fix-gc-safepoint-slot-reuse
Open

Fix safepoint stack slot reuse#13469
angelnereira wants to merge 1 commit into
bytecodealliance:mainfrom
angelnereira:fix-gc-safepoint-slot-reuse

Conversation

@angelnereira
Copy link
Copy Markdown

Summary

Fixes #13461.

The safepoint spiller walks instructions backwards and can free a stack slot for a value defined by a safepoint instruction before assigning stack-map slots for values live across that same safepoint. If the freed slot is reused, the stack map can point at a slot that contains the instruction result rather than the value that must remain live across the call.

This changes the rewrite order so safepoint stack-map entries are assigned before result slots for that instruction are freed for reuse. It also adds the GC regression test from the issue to cover the null-reference trap that exposed this.

Testing

  • cargo fmt --all -- --check
  • cargo test -p cranelift-frontend safepoints
  • cargo build -p wasmtime-cli
  • target/debug/wasmtime wast -Wgc -Wfunction-references -Wwide-arithmetic -Wsimd -Wthreads -Wreference-types /tmp/safepoint-reload-aliased-ref-null.wast
  • WASMTIME_TEST_GC_KEYWORDS=safepoint-reload-aliased-ref-null cargo test -p wasmtime-cli --test wast safepoint-reload-aliased-ref-null

@angelnereira angelnereira requested review from a team as code owners May 24, 2026 04:51
@angelnereira angelnereira requested review from fitzgen and removed request for a team May 24, 2026 04:51
gfx added a commit to wado-lang/wasmtime that referenced this pull request May 24, 2026
Backport of bytecodealliance#13469 (fixes bytecodealliance#13461).

The safepoint spiller walks instructions backwards and can free a stack
slot for a value defined by a safepoint instruction before assigning
stack-map slots for values live across that same safepoint. If the freed
slot is reused, the stack map can point at a slot that contains the
instruction result rather than the value that must remain live across the
call. This surfaced as a `null reference` trap on a provably non-null GC
ref carried across a call safepoint.

Reorder the rewrite so safepoint stack-map entries are assigned before
result slots for that instruction are freed for reuse.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@gfx
Copy link
Copy Markdown

gfx commented May 24, 2026

Confirmed — this fixes #13461 on our end.

We originally hit the null reference trap in Wado, whose compiler emits Wasm GC code with (ref null $t) values that stay live across call safepoints.

What I did:

Thanks for the quick turnaround.

gfx added a commit to wado-lang/wado that referenced this pull request May 24, 2026
Bump the vendor/wasmtime fork (wado-lang/wasmtime, gfx/wasmtime-45) to
pick up the backport of bytecodealliance/wasmtime#13469, which fixes the
Cranelift safepoint-spiller regression (#13461) that miscompiled GC refs
live across call safepoints into null reads.

This unblocks the wasmtime 44->45 upgrade: the 7 previously failing e2e
tests (serde_json_* and value_copy_nested_array_helper_chain) now pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
gfx added a commit to wado-lang/wado that referenced this pull request May 24, 2026
Bump the vendor/wasmtime fork (wado-lang/wasmtime, gfx/wasmtime-45) to
pick up the backport of bytecodealliance/wasmtime#13469, which fixes the
Cranelift safepoint-spiller regression (#13461) that miscompiled GC refs
live across call safepoints into null reads.

This unblocks the wasmtime 44->45 upgrade: the 7 previously failing e2e
tests (serde_json_* and value_copy_nested_array_helper_chain) now pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
gfx added a commit to wado-lang/wado that referenced this pull request May 24, 2026
Bump the vendor/wasmtime fork (wado-lang/wasmtime, gfx/wasmtime-45) to
pick up the backport of bytecodealliance/wasmtime#13469, which fixes the
Cranelift safepoint-spiller regression (#13461) that miscompiled GC refs
live across call safepoints into null reads.

This unblocks the wasmtime 44->45 upgrade: the 7 previously failing e2e
tests (serde_json_* and value_copy_nested_array_helper_chain) now pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the cranelift Issues related to the Cranelift code generator label May 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cranelift Issues related to the Cranelift code generator

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cranelift: GC reference reads back as null after a call safepoint (regression from #13228)

2 participants