Stream: git-wasmtime

Topic: wasmtime / PR #13449 cranelift: include every SSA value b...

Wasmtime GitHub notifications bot (May 22 2026 at 12:38):

vouillon opened PR #13449 from vouillon:gc-fix to bytecodealliance:main:

SSABuilder::variables is a SecondaryMap<Variable, SecondaryMap<Block, PackedOption<Value>>> that stores one Value per (Variable, Block) pair, so each def_var overwrites whatever the prior definition in that block was. values_for_var then iterates this map and returns only the latest Value per block. That is the wrong set for the safepoint pass: FunctionBuilder::finalize uses values_for_var to propagate stack-map flags from a variable onto its associated values, and any earlier SSA value bound to the variable in the same block is silently dropped even if it is still live across a later safepoint.

Concretely, Wasm code that re-defines a stack-map variable while an earlier SSA value bound to it is still live on the operand stack across an intervening safepoint hits the pattern:

    local.get N           ;; pushes the SSA value of the first def
    ...
    call $foo             ;; safepoint
    local.tee N           ;; redefines the variable
    ...
    array.new_fixed       ;; another safepoint that may GC

Without this fix the safepoint pass omits the first def's value from the stack map, GC's trace doesn't mark it, sweep removes it from OASR, the slot is freed, and the subsequent init-barrier in array.new_fixed inc-refs a freed slot, corrupting the OASR list.

Add all_var_values: SecondaryMap<Variable, EntitySet<Value>> to SSABuilder, and maintain the invariant that every Value written to variables[var][block] is also in all_var_values[var]. The only places that store a Value into variables[var][block] for the first time are def_var and find_var's block-param-creation tail, so those are the two sites that need to insert. use_var_nonlocal's predecessor-copy loop only re-stores a val that find_var just
returned, so the value is already in all_var_values; a debug_assert! documents and checks this. Finally switch
values_for_var to iterate the new set.

Wasmtime GitHub notifications bot (May 22 2026 at 12:38):

vouillon requested wasmtime-compiler-reviewers for a review on PR #13449.

Wasmtime GitHub notifications bot (May 22 2026 at 12:38):

vouillon requested cfallin for a review on PR #13449.

Wasmtime GitHub notifications bot (May 22 2026 at 14:34):

github-actions[bot] added the label cranelift on PR #13449.

Wasmtime GitHub notifications bot (May 22 2026 at 22:17):

:thumbs_up: cfallin submitted PR review:

Good find -- thank you!

Wasmtime GitHub notifications bot (May 22 2026 at 22:17):

cfallin added PR #13449 cranelift: include every SSA value bound to a variable in stack maps to the merge queue.

Wasmtime GitHub notifications bot (May 22 2026 at 22:42):

:check: cfallin merged PR #13449.

Wasmtime GitHub notifications bot (May 22 2026 at 22:42):

cfallin removed PR #13449 cranelift: include every SSA value bound to a variable in stack maps from the merge queue.

Wasmtime GitHub notifications bot (May 23 2026 at 09:37):

:memo: bjorn3 submitted PR review.

Wasmtime GitHub notifications bot (May 23 2026 at 09:37):

:speech_balloon: bjorn3 created PR review comment:

I don't think this should use EntitySet. EntitySet uses O(max value id) memory rather than O(number of values on the set) memory, which will waste a lot of memory for these sparse sets.

Also would it be possible to skip maintaining this info when stack maps are not requested by the user of Cranelift?

Last updated: Jun 01 2026 at 09:49 UTC