From 53330ddee0a3965e368d100e14222e3e9ae5a8b3 Mon Sep 17 00:00:00 2001 From: Khagan Karimov Date: Mon, 10 Nov 2025 17:40:52 -0700 Subject: [PATCH 1/3] Stack fixup for new types --- crates/fuzzing/src/generators/gc_ops/ops.rs | 198 ++++++++++-------- crates/fuzzing/src/generators/gc_ops/tests.rs | 2 +- crates/fuzzing/src/generators/gc_ops/types.rs | 78 +++++++ 3 files changed, 189 insertions(+), 89 deletions(-) diff --git a/crates/fuzzing/src/generators/gc_ops/ops.rs b/crates/fuzzing/src/generators/gc_ops/ops.rs index 4933e2c36c34..b743d9653fd4 100644 --- a/crates/fuzzing/src/generators/gc_ops/ops.rs +++ b/crates/fuzzing/src/generators/gc_ops/ops.rs @@ -1,12 +1,12 @@ //! Operations for the `gc` operations. +use crate::generators::gc_ops::types::StackType; use crate::generators::gc_ops::{ limits::GcOpsLimits, types::{CompositeType, RecGroupId, StructType, TypeId, Types}, }; use mutatis::{Context, Generate, mutators as m}; use serde::{Deserialize, Serialize}; -use smallvec::SmallVec; use std::collections::BTreeMap; use wasm_encoder::{ CodeSection, ConstExpr, EntityType, ExportKind, ExportSection, Function, FunctionSection, @@ -245,7 +245,8 @@ impl GcOps { self.types.fixup(&self.limits); let mut new_ops = Vec::with_capacity(self.ops.len()); - let mut stack = 0; + let mut stack: Vec = Vec::new(); + let num_types = self.types.type_defs.len() as u32; for mut op in self.ops.iter().copied() { if self.limits.max_types == 0 @@ -268,24 +269,27 @@ impl GcOps { op.fixup(&self.limits); - let mut temp = SmallVec::<[_; 4]>::new(); - - while stack < op.operands_len() { - temp.push(GcOp::Null()); - stack += 1; + match &mut op { + GcOp::StructNew(t) | GcOp::TakeStructCall(t) | GcOp::TakeTypedStructCall(t) => { + if num_types > 0 { + *t = *t % num_types; + } + } + _ => {} } - temp.push(op); - stack = stack - op.operands_len() + op.results_len(); + op.operand_types().iter().for_each(|ty| { + StackType::fixup_stack_type(*ty, &mut stack, &mut new_ops, num_types); + }); - new_ops.extend(temp); + // Finally, emit the op itself (updates stack abstractly) + StackType::emit(op, &mut stack, &mut new_ops, num_types); } - // Insert drops to balance the final stack state - for _ in 0..stack { + // Balance any leftovers with drops (works for any type) + for _ in 0..stack.len() { new_ops.push(GcOp::Drop()); } - self.ops = new_ops; } @@ -301,79 +305,68 @@ impl GcOps { macro_rules! define_gc_ops { ( $( - $op:ident $( ( $($limit_var:ident : $limit:expr => $ty:ty),* ) )? : $params:expr => $results:expr , + $op:ident + $( ( $($limit_var:ident : $limit:expr => $ty:ty),* ) )? + : ($operand_req:expr, $params:expr) => ($result_type:expr, $results:expr) , )* ) => { - /// The operations for the `gc` operations. #[derive(Copy, Clone, Debug, Serialize, Deserialize)] - pub(crate) enum GcOp { + /// The operations that can be performed by the `gc` function. + pub enum GcOp { $( + #[allow(missing_docs, reason = "macro-generated code")] $op ( $( $($ty),* )? ), )* } - /// Names of the operations for testing purposes. #[cfg(test)] - pub const OP_NAMES: &'static[&'static str] = &[ - $( - stringify!($op), - )* + pub(crate) const OP_NAMES: &'static [&'static str] = &[ + $( stringify!($op), )* ]; impl GcOp { #[cfg(test)] - pub fn name(&self) -> &'static str { - match self { - $( - Self::$op (..) => stringify!($op), - )* - } + pub(crate) fn name(&self) -> &'static str { + match self { $( Self::$op(..) => stringify!($op), )* } } - pub fn operands_len(&self) -> usize { - match self { - $( - Self::$op (..) => $params, - )* - } + pub(crate) fn operands_len(&self) -> usize { + match self { $( Self::$op(..) => $params, )* } } - pub fn results_len(&self) -> usize { + #[allow(unreachable_patterns, reason = "macro-generated code")] + pub(crate) fn operand_types(&self) -> Vec { match self { + // special-cases + Self::TakeTypedStructCall(t) => vec![StackType::Struct(Some(*t))], $( - Self::$op (..) => $results, - )* + Self::$op(..) => match $operand_req { + None => vec![], + Some(req) => if $params == 0 { vec![] } else { vec![req; $params] }, + } + ),* } } - } - $( - #[allow(non_snake_case, reason = "macro-generated code")] - fn $op( - _ctx: &mut mutatis::Context, - _limits: &GcOpsLimits, - stack: usize, - ) -> mutatis::Result<(GcOp, usize)> { - #[allow(unused_comparisons, reason = "macro-generated code")] - { - debug_assert!(stack >= $params); - } + pub(crate) fn results_len(&self) -> usize { + match self { $( Self::$op(..) => $results, )* } + } - let op = GcOp::$op( - $($({ - let limit_fn = $limit as fn(&GcOpsLimits) -> $ty; - let limit = (limit_fn)(_limits); - debug_assert!(limit > 0); - m::range(0..=limit - 1).generate(_ctx)? - }),*)? - ); - let new_stack = stack - $params + $results; - Ok((op, new_stack)) + #[allow(unreachable_patterns, reason = "macro-generated code")] + pub(crate) fn result_types(&self) -> Vec { + match self { + // special-cases + Self::StructNew(t) => vec![StackType::Struct(Some(*t))], + $( + Self::$op(..) => match $result_type { + None => vec![], + Some(ty) => if $results == 0 { vec![] } else { vec![ty; $results] }, + } + ),* + } } - )* - impl GcOp { - fn fixup(&mut self, limits: &GcOpsLimits) { + pub(crate) fn fixup(&mut self, limits: &GcOpsLimits) { match self { $( Self::$op( $( $( $limit_var ),* )? ) => { @@ -396,13 +389,13 @@ macro_rules! define_gc_ops { let mut valid_choices: Vec< fn(&mut Context, &GcOpsLimits, usize) -> mutatis::Result<(GcOp, usize)> > = vec![]; + $( #[allow(unused_comparisons, reason = "macro-generated code")] if stack >= $params $($( && { let limit_fn = $limit as fn(&GcOpsLimits) -> $ty; - let limit = (limit_fn)(&ops.limits); - limit > 0 + (limit_fn)(&ops.limits) > 0 } )*)? { valid_choices.push($op); @@ -412,36 +405,69 @@ macro_rules! define_gc_ops { let f = *ctx.rng() .choose(&valid_choices) .expect("should always have a valid op choice"); - (f)(ctx, &ops.limits, stack) } } + + $( + #[allow(non_snake_case, reason = "macro-generated code")] + fn $op( + _ctx: &mut mutatis::Context, + _limits: &GcOpsLimits, + stack: usize, + ) -> mutatis::Result<(GcOp, usize)> { + #[allow(unused_comparisons, reason = "macro-generated code")] + { debug_assert!(stack >= $params); } + + let op = GcOp::$op( + $($({ + let limit_fn = $limit as fn(&GcOpsLimits) -> $ty; + let limit = (limit_fn)(_limits); + debug_assert!(limit > 0); + m::range(0..=limit - 1).generate(_ctx)? + }),*)? + ); + Ok((op, stack - $params + $results)) + } + )* }; } define_gc_ops! { - Gc : 0 => 3, + Gc : (None, 0) => (Some(StackType::ExternRef), 3), - MakeRefs : 0 => 3, - TakeRefs : 3 => 0, + MakeRefs : (None, 0) => (Some(StackType::ExternRef), 3), + TakeRefs : (Some(StackType::ExternRef), 3) => (None, 0), // Add one to make sure that out of bounds table accesses are possible, but still rare. - TableGet(elem_index: |ops| ops.table_size + 1 => u32) : 0 => 1, - TableSet(elem_index: |ops| ops.table_size + 1 => u32) : 1 => 0, - - GlobalGet(global_index: |ops| ops.num_globals => u32) : 0 => 1, - GlobalSet(global_index: |ops| ops.num_globals => u32) : 1 => 0, - - LocalGet(local_index: |ops| ops.num_params => u32) : 0 => 1, - LocalSet(local_index: |ops| ops.num_params => u32) : 1 => 0, - - StructNew(type_index: |ops| ops.max_types => u32) : 0 => 0, - TakeStructCall(type_index: |ops| ops.max_types => u32) : 1 => 0, - TakeTypedStructCall(type_index: |ops| ops.max_types => u32) : 1 => 0, - - Drop : 1 => 0, - - Null : 0 => 1, + TableGet(elem_index: |ops| ops.table_size + 1 => u32) + : (None, 0) => (Some(StackType::ExternRef), 1), + TableSet(elem_index: |ops| ops.table_size + 1 => u32) + : (Some(StackType::ExternRef), 1) => (None, 0), + + GlobalGet(global_index: |ops| ops.num_globals => u32) + : (None, 0) => (Some(StackType::ExternRef), 1), + GlobalSet(global_index: |ops| ops.num_globals => u32) + : (Some(StackType::ExternRef), 1) => (None, 0), + + LocalGet(local_index: |ops| ops.num_params => u32) + : (None, 0) => (Some(StackType::ExternRef), 1), + LocalSet(local_index: |ops| ops.num_params => u32) + : (Some(StackType::ExternRef), 1) => (None, 0), + + // Handled specially in result_types() + StructNew(type_index: |ops| ops.max_types => u32) + : (None, 0) => (Some(StackType::Anything), 1), + TakeStructCall(type_index: |ops| ops.max_types => u32) + : (Some(StackType::Struct(None)), 1) => (None, 0), + // Handled specially in operand_types() + // StackType::Anythng is just a placeholder + TakeTypedStructCall(type_index: |ops| ops.max_types => u32) + : (Some(StackType::Anything), 1) => (None, 0), + + Drop : (Some(StackType::Anything), 1) => (None, 0), + + Null : (None, 0) => (Some(StackType::ExternRef), 1), } impl GcOp { @@ -497,16 +523,12 @@ impl GcOp { } Self::StructNew(x) => { func.instruction(&Instruction::StructNew(x + struct_type_base)); - func.instruction(&Instruction::Call(take_structref_idx)); } - Self::TakeStructCall(x) => { - func.instruction(&Instruction::StructNew(x + struct_type_base)); + Self::TakeStructCall(_x) => { func.instruction(&Instruction::Call(take_structref_idx)); } Self::TakeTypedStructCall(x) => { - let s = struct_type_base + x; let f = typed_first_func_index + x; - func.instruction(&Instruction::StructNew(s)); func.instruction(&Instruction::Call(f)); } } diff --git a/crates/fuzzing/src/generators/gc_ops/tests.rs b/crates/fuzzing/src/generators/gc_ops/tests.rs index b5907c0f84c6..62214874ed98 100644 --- a/crates/fuzzing/src/generators/gc_ops/tests.rs +++ b/crates/fuzzing/src/generators/gc_ops/tests.rs @@ -250,7 +250,7 @@ fn test_wat_string() -> mutatis::Result<()> { global.set 0 global.get 0 struct.new 5 - call 3 + drop drop drop drop diff --git a/crates/fuzzing/src/generators/gc_ops/types.rs b/crates/fuzzing/src/generators/gc_ops/types.rs index 5cd44ae61863..a000855142ec 100644 --- a/crates/fuzzing/src/generators/gc_ops/types.rs +++ b/crates/fuzzing/src/generators/gc_ops/types.rs @@ -1,6 +1,7 @@ //! Types for the `gc` operations. use crate::generators::gc_ops::limits::GcOpsLimits; +use crate::generators::gc_ops::ops::GcOp; use serde::{Deserialize, Serialize}; use std::collections::{BTreeMap, BTreeSet}; @@ -88,3 +89,80 @@ impl Types { ); } } + +/// This is used to track the requirements for the operands of an operation. +#[derive(Copy, Clone, Debug)] +pub enum StackType { + /// Any value is used for reauested operand not a type left on stack (only for Drop and specially handled ops) + Anything, + /// `externref` + ExternRef, + /// None = any non-null `(ref $*)`; Some(t) = exact `(ref $t)` + Struct(Option), +} + +impl StackType { + /// Fixes the stack type to match the given requirement. + pub fn fixup_stack_type( + req: StackType, + stack: &mut Vec, + out: &mut Vec, + num_types: u32, + ) { + match req { + Self::Anything => { + // Anything can accept any type - just pop if available + // If stack is empty, synthesize null (anyref compatible) + if stack.pop().is_none() { + // Create a null externref (will be converted if needed) + Self::emit(GcOp::Null(), stack, out, num_types); + stack.pop(); // consume just-synthesized externref + } + } + Self::ExternRef => match stack.last() { + Some(Self::ExternRef) => { + stack.pop(); + } + _ => { + Self::emit(GcOp::Null(), stack, out, num_types); + stack.pop(); // consume just-synthesized externref + } + }, + Self::Struct(wanted) => { + let ok = match (wanted, stack.last()) { + (Some(wanted), Some(Self::Struct(Some(s)))) => *s == wanted, + (None, Some(Self::Struct(_))) => true, + _ => false, + }; + + if ok { + stack.pop(); + } else { + // Ensure there *is* a struct to consume. + let t = match wanted { + Some(t) => Self::clamp(t, num_types), + None => Self::clamp(0, num_types), + }; + Self::emit(GcOp::StructNew(t), stack, out, num_types); + stack.pop(); // consume the synthesized struct + } + } + } + } + + pub(crate) fn emit(op: GcOp, stack: &mut Vec, out: &mut Vec, num_types: u32) { + out.push(op); + op.result_types().iter().for_each(|ty| { + // Clamp struct type indices when pushing to stack + let clamped_ty = match ty { + Self::Struct(Some(t)) => Self::Struct(Some(Self::clamp(*t, num_types))), + other => *other, + }; + stack.push(clamped_ty); + }); + } + + fn clamp(t: u32, n: u32) -> u32 { + if n == 0 { 0 } else { t % n } + } +} From 8423dcc9dfac6f1e0c722e26b613ec0e4a5fe3ce Mon Sep 17 00:00:00 2001 From: Khagan Karimov Date: Mon, 10 Nov 2025 18:15:56 -0700 Subject: [PATCH 2/3] Add new test for fixup() --- crates/fuzzing/src/generators/gc_ops/tests.rs | 80 +++++++++++++++++++ crates/fuzzing/src/generators/gc_ops/types.rs | 2 +- 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/crates/fuzzing/src/generators/gc_ops/tests.rs b/crates/fuzzing/src/generators/gc_ops/tests.rs index 62214874ed98..4c4d062a5160 100644 --- a/crates/fuzzing/src/generators/gc_ops/tests.rs +++ b/crates/fuzzing/src/generators/gc_ops/tests.rs @@ -297,3 +297,83 @@ fn emits_empty_rec_groups_and_validates() -> mutatis::Result<()> { Ok(()) } + +#[test] +fn fixup_check_types_and_indexes() -> mutatis::Result<()> { + let _ = env_logger::try_init(); + + // Create GcOps with 5 types so that 7 % 5 = 2 + let mut ops = test_ops(5, 5, 5); + ops.limits.max_types = 5; + + // We create max types 5 and out ouf bounds for the type index + ops.ops = vec![ + GcOp::TakeTypedStructCall(27), + GcOp::GlobalSet(0), + GcOp::StructNew(24), + GcOp::LocalSet(0), + ]; + + // Call fixup to resolve dependencies + // this should fix the types by inserting missing types + // also put the indexes in the correct bounds + ops.fixup(); + + // Verify that fixup() + // The expected sequence should be: + // 1. StructNew(_) - inserted by fixup to satisfy TakeTypedStructCall(_) + // 2. TakeTypedStructCall(_) - now has Struct(_) on stack + // 3. Null() - inserted by fixup to satisfy GlobalSet(0) + // 4. GlobalSet(0) - now has ExternRef on stack + // 5. StructNew(_) - produces Struct(_) + // 6. Null() - inserted by fixup to satisfy LocalSet(0) + // 7. LocalSet(0) - now has ExternRef on stack + // 8. Drop() - inserted by fixup to consume ExternRef before Drop() + + // This is the expected sequence in wat format: + // loop ;; label = @1 + // struct.new 7 + // call 6 + // ref.null extern + // global.set 0 + // struct.new 9 + // ref.null extern + // local.set 0 + // drop + // br 0 (;@1;) + // end + + // Find the index of TakeTypedStructCall(_) after fixup + let take_call_idx = ops + .ops + .iter() + .position(|op| matches!(op, GcOp::TakeTypedStructCall(_))) + .expect("TakeTypedStructCall(_) should be present after fixup"); + + // Verify that StructNew(_) appears before TakeTypedStructCall(_) + let struct_new_2_before = ops + .ops + .iter() + .take(take_call_idx) + .any(|op| matches!(op, GcOp::StructNew(_))); + + assert!( + struct_new_2_before, + "fixup should insert StructNew(_) before TakeTypedStructCall(_) to satisfy the dependency" + ); + + // Verify the sequence validates correctly + let wasm = ops.to_wasm_binary(); + let wat = wasmprinter::print_bytes(&wasm).unwrap(); + log::debug!("wat:\n{}", wat); + let feats = wasmparser::WasmFeatures::default(); + feats.reference_types(); + feats.gc(); + let mut validator = wasmparser::Validator::new_with_features(feats); + assert!( + validator.validate_all(&wasm).is_ok(), + "GC validation should pass after fixup" + ); + + Ok(()) +} diff --git a/crates/fuzzing/src/generators/gc_ops/types.rs b/crates/fuzzing/src/generators/gc_ops/types.rs index a000855142ec..289da7c190a1 100644 --- a/crates/fuzzing/src/generators/gc_ops/types.rs +++ b/crates/fuzzing/src/generators/gc_ops/types.rs @@ -114,7 +114,7 @@ impl StackType { // Anything can accept any type - just pop if available // If stack is empty, synthesize null (anyref compatible) if stack.pop().is_none() { - // Create a null externref (will be converted if needed) + // Create a null externref Self::emit(GcOp::Null(), stack, out, num_types); stack.pop(); // consume just-synthesized externref } From 800198666ad7b42285fba044833b63bc25e7bc10 Mon Sep 17 00:00:00 2001 From: Khagan Karimov Date: Mon, 10 Nov 2025 18:58:27 -0700 Subject: [PATCH 3/3] Address clippy failure --- crates/fuzzing/src/generators/gc_ops/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fuzzing/src/generators/gc_ops/tests.rs b/crates/fuzzing/src/generators/gc_ops/tests.rs index 4c4d062a5160..f1911de86593 100644 --- a/crates/fuzzing/src/generators/gc_ops/tests.rs +++ b/crates/fuzzing/src/generators/gc_ops/tests.rs @@ -365,7 +365,7 @@ fn fixup_check_types_and_indexes() -> mutatis::Result<()> { // Verify the sequence validates correctly let wasm = ops.to_wasm_binary(); let wat = wasmprinter::print_bytes(&wasm).unwrap(); - log::debug!("wat:\n{}", wat); + log::debug!("{wat}"); let feats = wasmparser::WasmFeatures::default(); feats.reference_types(); feats.gc();