From 4da15eae7a54ce6f0179fd931347f0b1221ffdfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Vouillon?= Date: Fri, 22 May 2026 13:54:46 +0200 Subject: [PATCH 1/2] Add regression test for missing stack-map slot on re-defined variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Declares a variable that needs stack maps, defines it twice in the same block (`v1` then `v2`), reads both values, then calls a function as a safepoint before returning both. The expected CLIF spills both `v1` and `v2` to separate slots and lists both in the call's `stack_map=[…]`. Without the accompanying fix, `SSABuilder::variables` stores only the latest `Value` per `(Variable, Block)` pair, so the earlier definition is dropped from `values_for_var` and the safepoint pass omits it from the stack map. The test fails with only `v2` spilled and only one entry in `stack_map`. --- cranelift/frontend/src/frontend/safepoints.rs | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/cranelift/frontend/src/frontend/safepoints.rs b/cranelift/frontend/src/frontend/safepoints.rs index 983ae9808c09..6f25426cb542 100644 --- a/cranelift/frontend/src/frontend/safepoints.rs +++ b/cranelift/frontend/src/frontend/safepoints.rs @@ -3015,4 +3015,95 @@ block2: "#, ); } + + /// Regression test for a missed stack-map slot when a stack-map + /// variable is re-defined in the same block while an earlier SSA + /// value bound to it is still live across a safepoint. + /// + /// Previously, `SSABuilder::variables` stored only the *latest* + /// `Value` per `(Variable, Block)`, and `values_for_var` iterated + /// that map, so the earlier `Value` was dropped from the set the + /// safepoint pass propagates stack-map flags onto. The expected + /// output below requires `v1` to be spilled and listed in the + /// `call` instruction's stack map alongside `v2`. + #[test] + fn var_redefined_in_same_block_keeps_earlier_value_in_stack_map() { + let mut sig = Signature::new(CallConv::SystemV); + sig.returns + .push(AbiParam::new(cranelift_codegen::ir::types::I32)); + sig.returns + .push(AbiParam::new(cranelift_codegen::ir::types::I32)); + + let mut fn_ctx = FunctionBuilderContext::new(); + let mut func = Function::with_name_signature(ir::UserFuncName::testcase("sample"), sig); + let mut builder = FunctionBuilder::new(&mut func, &mut fn_ctx); + + let var = builder.declare_var(cranelift_codegen::ir::types::I32); + builder.declare_var_needs_stack_map(var); + + let name = builder + .func + .declare_imported_user_function(ir::UserExternalName { + namespace: 0, + index: 0, + }); + let signature = builder + .func + .import_signature(Signature::new(CallConv::SystemV)); + let func_ref = builder.import_function(ir::ExtFuncData { + name: ir::ExternalName::user(name), + signature, + colocated: true, + patchable: false, + }); + + let block0 = builder.create_block(); + builder.switch_to_block(block0); + + // First definition of `var`. Read it out (giving us an SSA + // value `v1` that we'll keep live across the safepoint below). + let v1 = builder.ins().iconst(ir::types::I32, 11); + builder.def_var(var, v1); + let v1_use = builder.use_var(var); + + // Re-define `var` to a different SSA value within the same + // block. Without the fix, this overwrites the only place + // `v1` was recorded, so `values_for_var(var)` no longer + // returns it and the safepoint pass omits it from the + // upcoming stack map. + let v2 = builder.ins().iconst(ir::types::I32, 22); + builder.def_var(var, v2); + let v2_use = builder.use_var(var); + + // Safepoint at which both `v1_use` (= v1) and `v2_use` (= v2) + // must be live and listed in the stack map. + builder.ins().call(func_ref, &[]); + + builder.ins().return_(&[v1_use, v2_use]); + + builder.seal_all_blocks(); + builder.finalize(); + + assert_eq_output!( + func.display().to_string(), + r#" +function %sample() -> i32, i32 system_v { + ss0 = explicit_slot 4, align = 4 + ss1 = explicit_slot 4, align = 4 + sig0 = () system_v + fn0 = colocated u0:0 sig0 + +block0: + v0 = iconst.i32 11 + stack_store v0, ss0 ; v0 = 11 + v1 = iconst.i32 22 + stack_store v1, ss1 ; v1 = 22 + call fn0(), stack_map=[i32 @ ss0+0, i32 @ ss1+0] + v2 = stack_load.i32 ss0 + v3 = stack_load.i32 ss1 + return v2, v3 +} + "# + ); + } } From 677770c905fb4268ac647f4ee195ffbd57fdc416 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Vouillon?= Date: Fri, 22 May 2026 13:55:13 +0200 Subject: [PATCH 2/2] Track every SSA value bound to a variable, not just the latest per block `SSABuilder::variables` is a `SecondaryMap>>` 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 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>` 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. --- cranelift/frontend/src/ssa.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/cranelift/frontend/src/ssa.rs b/cranelift/frontend/src/ssa.rs index 759e7e353634..1381c05421dd 100644 --- a/cranelift/frontend/src/ssa.rs +++ b/cranelift/frontend/src/ssa.rs @@ -37,6 +37,12 @@ pub struct SSABuilder { /// the variable in the block. variables: SecondaryMap>>, + /// Records every SSA `Value` ever associated with a variable, including + /// values that `variables` has since overwritten within the same block. + /// Used by `values_for_var` to feed `FunctionBuilder::finalize`'s + /// stack-map propagation. + all_var_values: SecondaryMap>, + /// Records the position of the basic blocks and the list of values used but not defined in the /// block. ssa_blocks: SecondaryMap, @@ -110,6 +116,7 @@ impl SSABuilder { /// deallocating memory. pub fn clear(&mut self) { self.variables.clear(); + self.all_var_values.clear(); self.ssa_blocks.clear(); self.variable_pool.clear(); self.inst_pool.clear(); @@ -121,6 +128,7 @@ impl SSABuilder { /// Tests whether an `SSABuilder` is in a cleared state. pub fn is_empty(&self) -> bool { self.variables.is_empty() + && self.all_var_values.is_empty() && self.ssa_blocks.is_empty() && self.calls.is_empty() && self.results.is_empty() @@ -204,7 +212,7 @@ impl SSABuilder { /// Get all of the values associated with the given variable that we have /// inserted in the function thus far. pub fn values_for_var(&self, var: Variable) -> impl Iterator + '_ { - self.variables[var].values().filter_map(|v| v.expand()) + self.all_var_values[var].iter() } /// Declares a new definition of a variable in a given basic block. @@ -212,6 +220,7 @@ impl SSABuilder { /// `ir::DataFlowGraph::append_result`. pub fn def_var(&mut self, var: Variable, val: Value, block: Block) { self.variables[var][block] = PackedOption::from(val); + self.all_var_values[var].insert(val); } /// Declares a use of a variable in a given basic block. Returns the SSA value corresponding @@ -274,6 +283,7 @@ impl SSABuilder { // any of the blocks before `from`. // // So in either case there is no definition in these blocks yet and we can blindly set one. + debug_assert!(self.all_var_values[var].contains(val)); let var_defs = &mut self.variables[var]; while block != from { debug_assert!(var_defs[block].is_none()); @@ -330,6 +340,7 @@ impl SSABuilder { // find a usable definition. So create one. let val = func.dfg.append_block_param(block, ty); var_defs[block] = PackedOption::from(val); + self.all_var_values[var].insert(val); // Now every predecessor needs to pass its definition of this variable to the newly added // block parameter. To do that we have to "recursively" call `use_var`, but there are two