From be46865cc152c72537a70425bfd52d405490029a Mon Sep 17 00:00:00 2001 From: Khagan Karimov Date: Thu, 2 Apr 2026 20:22:31 -0400 Subject: [PATCH 1/2] Update StackType to accept supertypes Update mutators --- .../fuzzing/src/generators/gc_ops/mutator.rs | 23 +++- crates/fuzzing/src/generators/gc_ops/ops.rs | 2 +- crates/fuzzing/src/generators/gc_ops/tests.rs | 130 +++++++++++++++++- crates/fuzzing/src/generators/gc_ops/types.rs | 115 ++++++++++++++-- 4 files changed, 253 insertions(+), 17 deletions(-) diff --git a/crates/fuzzing/src/generators/gc_ops/mutator.rs b/crates/fuzzing/src/generators/gc_ops/mutator.rs index 8150440fbe6e..fbef7f934f08 100644 --- a/crates/fuzzing/src/generators/gc_ops/mutator.rs +++ b/crates/fuzzing/src/generators/gc_ops/mutator.rs @@ -14,7 +14,8 @@ use std::collections::BTreeMap; pub struct TypesMutator; impl TypesMutator { - /// Add an empty struct type to a random existing rec group. + /// Add an empty struct in a random existing rec group, or create a rec group + /// and add there when `rec_groups` is empty (if `limits.max_rec_groups` allows). fn add_struct( &mut self, c: &mut Candidates<'_>, @@ -22,16 +23,30 @@ impl TypesMutator { limits: &GcOpsLimits, ) -> mutatis::Result<()> { if c.shrink() - || types.rec_groups.is_empty() || types.type_defs.len() >= usize::try_from(limits.max_types).expect("max_types is too large") { return Ok(()); } + + let max_rec_groups = + usize::try_from(limits.max_rec_groups).expect("max_rec_groups is too large"); + if types.rec_groups.is_empty() && max_rec_groups == 0 { + return Ok(()); + } + c.mutation(|ctx| { - let Some(gid) = ctx.rng().choose(types.rec_groups.keys()).copied() else { - return Ok(()); + let gid = if types.rec_groups.is_empty() { + let new_gid = types.fresh_rec_group_id(ctx.rng()); + types.insert_rec_group(new_gid); + new_gid + } else { + let Some(gid) = ctx.rng().choose(types.rec_groups.keys()).copied() else { + return Ok(()); + }; + gid }; + let tid = types.fresh_type_id(ctx.rng()); let is_final = (ctx.rng().gen_u32() % 4) == 0; let supertype = if (ctx.rng().gen_u32() % 4) == 0 { diff --git a/crates/fuzzing/src/generators/gc_ops/ops.rs b/crates/fuzzing/src/generators/gc_ops/ops.rs index 0c8049e7a661..f760d7db058e 100644 --- a/crates/fuzzing/src/generators/gc_ops/ops.rs +++ b/crates/fuzzing/src/generators/gc_ops/ops.rs @@ -395,7 +395,7 @@ impl GcOps { debug_assert!(operand_types.is_empty()); op.operand_types(&mut operand_types); for ty in operand_types.drain(..) { - StackType::fixup(ty, &mut stack, &mut new_ops, num_types); + StackType::fixup(ty, &mut stack, &mut new_ops, num_types, &self.types); } // Finally, emit the op itself (updates stack abstractly) diff --git a/crates/fuzzing/src/generators/gc_ops/tests.rs b/crates/fuzzing/src/generators/gc_ops/tests.rs index e3e856cbe25c..99e25a213d56 100644 --- a/crates/fuzzing/src/generators/gc_ops/tests.rs +++ b/crates/fuzzing/src/generators/gc_ops/tests.rs @@ -1,7 +1,7 @@ use crate::generators::gc_ops::{ limits::GcOpsLimits, ops::{GcOp, GcOps, OP_NAMES}, - types::{RecGroupId, TypeId, Types}, + types::{RecGroupId, StackType, TypeId, Types}, }; use mutatis; use rand::rngs::StdRng; @@ -86,7 +86,6 @@ fn mutate_gc_ops_with_default_mutator() -> mutatis::Result<()> { session.mutate(&mut ops)?; let wasm = ops.to_wasm_binary(); - println!("wasm: {}", wasmprinter::print_bytes(&wasm).unwrap()); crate::oracles::log_wasm(&wasm); let mut validator = wasmparser::Validator::new_with_features(features); @@ -424,6 +423,8 @@ fn fixup_breaks_one_edge_in_multi_rec_group_type_cycle() { #[test] fn sort_rec_groups_topo_orders_dependencies_first() { + let _ = env_logger::try_init(); + let mut types = Types::new(); let g0 = RecGroupId(0); @@ -463,6 +464,8 @@ fn sort_rec_groups_topo_orders_dependencies_first() { #[test] fn break_rec_group_cycles() { + let _ = env_logger::try_init(); + let mut types = Types::new(); let g0 = RecGroupId(0); @@ -555,3 +558,126 @@ fn break_rec_group_cycles() { assert_eq!(topo.len(), 4); assert_eq!(topo, vec![g3, g2, g1, g0]); } + +#[test] +fn is_subtype_index_accepts_chain() { + let _ = env_logger::try_init(); + + let mut types = Types::new(); + let g0 = RecGroupId(0); + let g1 = RecGroupId(1); + let g2 = RecGroupId(2); + let g3 = RecGroupId(3); + + types.insert_rec_group(g0); + types.insert_rec_group(g1); + types.insert_rec_group(g2); + types.insert_rec_group(g3); + + // Build chain: 1 <- 2 <- 3 + // + // TypeId(1): g0 + // TypeId(2): subtype of 1 + // TypeId(3): g2 + // TypeId(4): g3 + // + // Since type_defs is a BTreeMap, the dense index order used by + // is_subtype_index is: + // + // 0 -> TypeId(1) + // 1 -> TypeId(2) + // 2 -> TypeId(3) + types.insert_empty_struct(TypeId(1), g0, false, None); + types.insert_empty_struct(TypeId(2), g1, false, Some(TypeId(1))); + types.insert_empty_struct(TypeId(3), g2, false, Some(TypeId(2))); + types.insert_empty_struct(TypeId(4), g3, false, Some(TypeId(3))); + + // self + assert!(types.is_subtype_index(0, 0)); // 1 <: 1 + assert!(types.is_subtype_index(1, 1)); // 2 <: 2 + assert!(types.is_subtype_index(2, 2)); // 3 <: 3 + + // requested checks + assert!(types.is_subtype_index(1, 0)); // 2 <: 1 + assert!(types.is_subtype_index(2, 0)); // 3 <: 1 + assert!(types.is_subtype_index(2, 1)); // 3 <: 2 + + // reverse directions must fail + assert!(!types.is_subtype_index(0, 1)); // 1 TypeId(1) + // 1 -> TypeId(2) + // 2 -> TypeId(3) + types.insert_empty_struct(TypeId(1), g, false, None); + types.insert_empty_struct(TypeId(2), g, false, Some(TypeId(1))); + types.insert_empty_struct(TypeId(3), g, false, Some(TypeId(2))); + + let num_types = u32::try_from(types.type_defs.len()).unwrap(); + + // Case 1: stack has subtype 3, op requires supertype 2. + let mut stack = vec![StackType::Struct(Some(2))]; + let mut out = vec![]; + + StackType::fixup( + Some(StackType::Struct(Some(1))), + &mut stack, + &mut out, + num_types, + &types, + ); + + // Accepted as-is: + // - no fixup ops inserted + // - operand consumed from stack + assert!( + out.is_empty(), + "subtype 3 should satisfy required supertype 2" + ); + assert!(stack.is_empty(), "accepted operand should be popped"); + + // Case 2: stack has subtype 3, op requires supertype 1. + let mut stack = vec![StackType::Struct(Some(2))]; + let mut out = vec![]; + + StackType::fixup( + Some(StackType::Struct(Some(0))), + &mut stack, + &mut out, + num_types, + &types, + ); + // Accepted as-is: + assert!( + out.is_empty(), + "subtype 3 should satisfy required supertype 1" + ); + assert!(stack.is_empty(), "accepted operand should be popped"); + + // Case 3: stack has type 1, op requires subtype 2. + let mut stack = vec![StackType::Struct(Some(0))]; + let mut out = vec![]; + + StackType::fixup( + Some(StackType::Struct(Some(1))), + &mut stack, + &mut out, + num_types, + &types, + ); + // Not accepted. Fixup should synthesize the requested concrete type. + assert_eq!(out, vec![GcOp::StructNew { type_index: 1 }]); + assert_eq!(stack, vec![StackType::Struct(Some(0))]); +} diff --git a/crates/fuzzing/src/generators/gc_ops/types.rs b/crates/fuzzing/src/generators/gc_ops/types.rs index d7a822fa956a..d4ba24eecc80 100644 --- a/crates/fuzzing/src/generators/gc_ops/types.rs +++ b/crates/fuzzing/src/generators/gc_ops/types.rs @@ -199,6 +199,43 @@ impl Types { .map(|(gid, _)| *gid) } + /// Returns true iff `sub_index` is the same as or a subtype of `sup_index`. + pub fn is_subtype_index(&self, sub_index: u32, sup_index: u32) -> bool { + if sub_index == sup_index { + return true; + } + + let i = match usize::try_from(sub_index) { + Ok(i) => i, + Err(_) => return false, + }; + let j = match usize::try_from(sup_index) { + Ok(j) => j, + Err(_) => return false, + }; + + let mut cur = match self.type_defs.keys().nth(i).copied() { + Some(t) => t, + None => return false, + }; + let sup = match self.type_defs.keys().nth(j).copied() { + Some(t) => t, + None => return false, + }; + + loop { + if cur == sup { + return true; + } + + let next = match self.type_defs.get(&cur).and_then(|d| d.supertype) { + Some(t) => t, + None => return false, + }; + cur = next; + } + } + /// Topological sort of types by their supertype (supertype before subtype). pub fn sort_types_topo(&self, out: &mut Vec) { let graph = SupertypeGraph { @@ -485,7 +522,7 @@ impl Types { } /// Tracks the required operand type on the abstract value stack. -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] pub enum StackType { /// `externref`. ExternRef, @@ -500,33 +537,65 @@ impl StackType { stack: &mut Vec, out: &mut Vec, num_types: u32, + types: &Types, ) { + log::trace!( + "[StackType::fixup] enter req={req:?} num_types={num_types} stack_len={} stack={stack:?}", + stack.len() + ); let mut result_types = Vec::new(); match req { None => { if stack.is_empty() { + log::trace!("[StackType::fixup] None: empty stack -> emit NullExtern"); Self::emit(GcOp::NullExtern, stack, out, num_types, &mut result_types); } - stack.pop(); + let popped = stack.pop(); + log::trace!("[StackType::fixup] None: pop -> {popped:?} stack={stack:?}"); } Some(Self::ExternRef) => match stack.last() { Some(Self::ExternRef) => { + log::trace!("[StackType::fixup] ExternRef: top ok -> pop"); stack.pop(); } - _ => { + other => { + log::trace!( + "[StackType::fixup] ExternRef: mismatch top={other:?} -> emit NullExtern+pop" + ); Self::emit(GcOp::NullExtern, stack, out, num_types, &mut result_types); - stack.pop(); + let popped = stack.pop(); + log::trace!( + "[StackType::fixup] ExternRef: after emit pop -> {popped:?} stack={stack:?}" + ); } }, Some(Self::Struct(wanted)) => { let ok = match (wanted, stack.last()) { - (Some(wanted), Some(Self::Struct(Some(s)))) => *s == wanted, - (None, Some(Self::Struct(_))) => true, - _ => false, + (Some(wanted), Some(Self::Struct(Some(actual)))) => { + let st = types.is_subtype_index(*actual, wanted); + log::trace!( + "[StackType::fixup] Struct: actual={actual} wanted={wanted} is_subtype_index={st}" + ); + st + } + (None, Some(Self::Struct(_))) => { + log::trace!( + "[StackType::fixup] Struct: abstract wanted, concrete stack -> ok" + ); + true + } + _ => { + log::trace!( + "[StackType::fixup] Struct: no match wanted={wanted:?} last={:?} -> ok=false", + stack.last() + ); + false + } }; if ok { - stack.pop(); + let popped = stack.pop(); + log::trace!("[StackType::fixup] Struct: ok -> pop {popped:?} stack={stack:?}"); } else { match wanted { // When num_types == 0, GcOp::fixup() should have dropped the ops @@ -536,8 +605,14 @@ impl StackType { // StackType::fixup() should insert GcOp::NullStruct() // to satisfy the undropped ops that work with abstract types. None => { + log::trace!( + "[StackType::fixup] Struct synthesize NullStruct stack_before={stack:?}" + ); Self::emit(GcOp::NullStruct, stack, out, num_types, &mut result_types); - stack.pop(); + let popped = stack.pop(); + log::trace!( + "[StackType::fixup] NullStruct: after emit pop -> {popped:?} stack={stack:?}" + ); } Some(t) => { debug_assert_ne!( @@ -545,6 +620,9 @@ impl StackType { "typed struct requirement with num_types == 0; op should have been removed" ); let t = Self::clamp(t, num_types); + log::trace!( + "[StackType::fixup] Struct synthesize StructNew type_index={t} stack_before={stack:?}" + ); Self::emit( GcOp::StructNew { type_index: t }, stack, @@ -552,12 +630,23 @@ impl StackType { num_types, &mut result_types, ); - stack.pop(); + log::trace!( + "[StackType::fixup] StructNew: after emit stack={stack:?} (next: pop operand)" + ); + let popped = stack.pop(); + log::trace!( + "[StackType::fixup] StructNew: pop -> {popped:?} stack={stack:?}" + ); } } } } } + log::trace!( + "[StackType::fixup] leave stack_len={} stack={stack:?} out_len={}", + stack.len(), + out.len() + ); } /// Emit an opcode and update the stack. @@ -568,6 +657,10 @@ impl StackType { num_types: u32, result_types: &mut Vec, ) { + log::trace!( + "[StackType::emit] op={op:?} stack_len_before={} num_types={num_types}", + stack.len() + ); out.push(op); result_types.clear(); op.result_types(result_types); @@ -576,8 +669,10 @@ impl StackType { Self::Struct(Some(t)) => Self::Struct(Some(Self::clamp(*t, num_types))), other => *other, }; + log::trace!("[StackType::emit] push result {clamped_ty:?}"); stack.push(clamped_ty); } + log::trace!("[StackType::emit] leave stack={stack:?}"); } /// Clamp a type index to the number of types. From 5f6027226a53706241fa22c1e43f897ef86bf6fe Mon Sep 17 00:00:00 2001 From: Khagan Karimov Date: Sat, 4 Apr 2026 20:06:41 +0000 Subject: [PATCH 2/2] Address the failure --- crates/fuzzing/src/generators/gc_ops/ops.rs | 72 ++++----- crates/fuzzing/src/generators/gc_ops/tests.rs | 137 +++++++++++++++--- crates/fuzzing/src/generators/gc_ops/types.rs | 111 +++++++++----- 3 files changed, 219 insertions(+), 101 deletions(-) diff --git a/crates/fuzzing/src/generators/gc_ops/ops.rs b/crates/fuzzing/src/generators/gc_ops/ops.rs index f760d7db058e..761348c9e50f 100644 --- a/crates/fuzzing/src/generators/gc_ops/ops.rs +++ b/crates/fuzzing/src/generators/gc_ops/ops.rs @@ -49,7 +49,8 @@ impl GcOps { /// fuel. It also is not guaranteed to avoid traps: it may access /// out-of-bounds of the table. pub fn to_wasm_binary(&mut self) -> Vec { - self.fixup(); + let mut encoding_order_grouped = Vec::with_capacity(self.types.rec_groups.len()); + self.fixup(&mut encoding_order_grouped); let mut module = Module::new(); @@ -106,43 +107,15 @@ impl GcOps { let struct_type_base: u32 = types.len(); - // Sort all types globally so supertypes come before subtypes. - // This is used to order types *within* each rec group. - let mut type_order = Vec::new(); - self.types.sort_types_topo(&mut type_order); - - // Build a position map so we can sort each group's members - // according to the global type order. - let type_position: BTreeMap = type_order - .iter() - .enumerate() - .map(|(i, &id)| (id, i)) - .collect(); - - // Topological sort of rec groups: if a type in group G has a - // supertype in group H, then H appears before G. - let mut group_order = Vec::new(); - self.types.sort_rec_groups_topo(&mut group_order); - - // For each group, collect its members sorted by the global type order. - let mut group_members: BTreeMap> = BTreeMap::new(); - for &gid in &group_order { - if let Some(member_set) = self.types.rec_groups.get(&gid) { - let mut members: Vec = member_set.iter().copied().collect(); - members.sort_by_key(|tid| type_position.get(tid).copied().unwrap_or(usize::MAX)); - group_members.insert(gid, members); - } - } - - // Build the type-id-to-wasm-index map directly. + // Build the type-id-to-wasm-index map from the pre-computed + // encoding order (rec groups in topo order, members sorted by + // supertype-first within each group). let mut type_ids_to_index: BTreeMap = BTreeMap::new(); let mut next_idx = struct_type_base; - for g in &group_order { - if let Some(members) = group_members.get(g) { - for &tid in members { - type_ids_to_index.insert(tid, next_idx); - next_idx += 1; - } + for (_, members) in &encoding_order_grouped { + for &tid in members { + type_ids_to_index.insert(tid, next_idx); + next_idx += 1; } } @@ -166,12 +139,12 @@ impl GcOps { let mut struct_count = 0; - // Emit rec groups in the derived order. - for g in &group_order { - let type_ids = group_members.get(g).map(|v| v.as_slice()).unwrap_or(&[]); - let members: Vec = type_ids.iter().map(encode_ty_id).collect(); + // Emit rec groups in the pre-computed order. + for (_, group_members) in &encoding_order_grouped { + let members: Vec = + group_members.iter().map(encode_ty_id).collect(); types.ty().rec(members); - struct_count += u32::try_from(type_ids.len()).unwrap(); + struct_count += u32::try_from(group_members.len()).unwrap(); } let typed_fn_type_base: u32 = struct_type_base + struct_count; @@ -378,9 +351,13 @@ impl GcOps { /// pre-mutation test cases are even valid! Therefore, we always call this /// method before translating this "AST"-style representation into a raw /// Wasm binary. - pub fn fixup(&mut self) { + pub fn fixup(&mut self, encoding_order_grouped: &mut Vec<(RecGroupId, Vec)>) { self.limits.fixup(); - self.types.fixup(&self.limits); + self.types.fixup(&self.limits, encoding_order_grouped); + let encoding_order: Vec = encoding_order_grouped + .iter() + .flat_map(|(_, members)| members.iter().copied()) + .collect(); let mut new_ops = Vec::with_capacity(self.ops.len()); let mut stack: Vec = Vec::new(); @@ -395,7 +372,14 @@ impl GcOps { debug_assert!(operand_types.is_empty()); op.operand_types(&mut operand_types); for ty in operand_types.drain(..) { - StackType::fixup(ty, &mut stack, &mut new_ops, num_types, &self.types); + StackType::fixup( + ty, + &mut stack, + &mut new_ops, + num_types, + &self.types, + &encoding_order, + ); } // Finally, emit the op itself (updates stack abstractly) diff --git a/crates/fuzzing/src/generators/gc_ops/tests.rs b/crates/fuzzing/src/generators/gc_ops/tests.rs index 99e25a213d56..6c6548cadd5b 100644 --- a/crates/fuzzing/src/generators/gc_ops/tests.rs +++ b/crates/fuzzing/src/generators/gc_ops/tests.rs @@ -9,6 +9,49 @@ use rand::{Rng, SeedableRng}; use wasmparser; use wasmprinter; +/// Flattened encoding order for use in tests. +fn encoding_order(types: &Types) -> Vec { + let type_to_group = types.type_to_group_map(); + let mut grouped = Vec::new(); + types.encoding_order_grouped(&mut grouped, &type_to_group); + grouped + .into_iter() + .flat_map(|(_, members)| members) + .collect() +} + +/// Returns true iff `sub_index` is the same as or a subtype of `sup_index`. +/// +/// The `encoding_order` slice maps dense indices (0, 1, 2, …) to +/// [`TypeId`]s in the same order they appear in the encoded Wasm binary. +fn is_subtype_index( + types: &Types, + sub_index: u32, + sup_index: u32, + encoding_order: &[TypeId], +) -> bool { + if sub_index == sup_index { + return true; + } + + let sub = match encoding_order + .get(usize::try_from(sub_index).expect("sub_index is out of bounds")) + .copied() + { + Some(t) => t, + None => return false, + }; + let sup = match encoding_order + .get(usize::try_from(sup_index).expect("sup_index is out of bounds")) + .copied() + { + Some(t) => t, + None => return false, + }; + + types.is_subtype(sub, sup) +} + /// Creates empty GcOps fn empty_test_ops() -> GcOps { let mut t = GcOps { @@ -120,7 +163,7 @@ fn struct_new_removed_when_no_types() -> mutatis::Result<()> { ops.limits.max_types = 0; ops.ops = vec![GcOp::StructNew { type_index: 42 }]; - ops.fixup(); + ops.fixup(&mut Vec::new()); assert!( ops.ops .iter() @@ -141,7 +184,7 @@ fn local_ops_removed_when_no_params() -> mutatis::Result<()> { GcOp::LocalSet { local_index: 99 }, ]; - ops.fixup(); + ops.fixup(&mut Vec::new()); assert!( ops.ops .iter() @@ -162,7 +205,7 @@ fn global_ops_removed_when_no_globals() -> mutatis::Result<()> { GcOp::GlobalSet { global_index: 99 }, ]; - ops.fixup(); + ops.fixup(&mut Vec::new()); assert!( ops.ops .iter() @@ -257,7 +300,7 @@ fn fixup_check_types_and_indexes() -> mutatis::Result<()> { // Call `fixup` to insert missing types, rewrite the immediates such that // they are within their bounds, insert missing operands, and drop unused // results. - ops.fixup(); + ops.fixup(&mut Vec::new()); // Check that we got the expected `GcOp` sequence after `fixup`: assert_eq!( @@ -357,7 +400,7 @@ fn fixup_preserves_subtyping_within_same_rec_group() { max_types: 10, }; - types.fixup(&limits); + types.fixup(&limits, &mut Vec::new()); assert_eq!(types.rec_group_of(super_ty), Some(g)); assert_eq!(types.rec_group_of(sub_ty), Some(g)); @@ -404,7 +447,7 @@ fn fixup_breaks_one_edge_in_multi_rec_group_type_cycle() { max_types: 10, }; - types.fixup(&limits); + types.fixup(&limits, &mut Vec::new()); let a_super = types.type_defs.get(&a).unwrap().supertype; let c_super = types.type_defs.get(&c).unwrap().supertype; @@ -451,8 +494,9 @@ fn sort_rec_groups_topo_orders_dependencies_first() { types.insert_empty_struct(e, g0, false, None); types.insert_empty_struct(f, g2, false, None); + let type_to_group = types.type_to_group_map(); let mut sorted = Vec::new(); - types.sort_rec_groups_topo(&mut sorted); + types.sort_rec_groups_topo(&mut sorted, &type_to_group); // g3 has no deps, g2 depends on g3, g1 on g2, g0 on g1. assert_eq!( @@ -532,7 +576,8 @@ fn break_rec_group_cycles() { assert_eq!(types.rec_groups.len(), 4); - types.break_rec_group_cycles(); + let type_to_group = types.type_to_group_map(); + types.break_rec_group_cycles(&type_to_group); // All four groups preserved. assert_eq!(types.rec_groups.len(), 4); @@ -553,8 +598,9 @@ fn break_rec_group_cycles() { assert_eq!(types.type_defs.get(&c2).unwrap().supertype, Some(d0)); // Result is a clean chain: g0 -> g1 -> g2 -> g3 + let type_to_group = types.type_to_group_map(); let mut topo = Vec::new(); - types.sort_rec_groups_topo(&mut topo); + types.sort_rec_groups_topo(&mut topo, &type_to_group); assert_eq!(topo.len(), 4); assert_eq!(topo, vec![g3, g2, g1, g0]); } @@ -592,20 +638,71 @@ fn is_subtype_index_accepts_chain() { types.insert_empty_struct(TypeId(3), g2, false, Some(TypeId(2))); types.insert_empty_struct(TypeId(4), g3, false, Some(TypeId(3))); + let order = encoding_order(&types); + // self - assert!(types.is_subtype_index(0, 0)); // 1 <: 1 - assert!(types.is_subtype_index(1, 1)); // 2 <: 2 - assert!(types.is_subtype_index(2, 2)); // 3 <: 3 + assert!(is_subtype_index(&types, 0, 0, &order)); // 1 <: 1 + assert!(is_subtype_index(&types, 1, 1, &order)); // 2 <: 2 + assert!(is_subtype_index(&types, 2, 2, &order)); // 3 <: 3 // requested checks - assert!(types.is_subtype_index(1, 0)); // 2 <: 1 - assert!(types.is_subtype_index(2, 0)); // 3 <: 1 - assert!(types.is_subtype_index(2, 1)); // 3 <: 2 + assert!(is_subtype_index(&types, 1, 0, &order)); // 2 <: 1 + assert!(is_subtype_index(&types, 2, 0, &order)); // 3 <: 1 + assert!(is_subtype_index(&types, 2, 1, &order)); // 3 <: 2 // reverse directions must fail - assert!(!types.is_subtype_index(0, 1)); // 1 TypeId(1) 1 -> TypeId(10) +/// +/// But the correct encoding order (group topo sort) is: +/// 0 -> TypeId(10) 1 -> TypeId(1) +/// +/// A naive key-order approach would conclude "index 0 (TypeId(1)) <: index 1 +/// (TypeId(10))" is false (they're unrelated), while the real encoding order +/// says "index 1 (TypeId(1)) <: index 0 (TypeId(10))" is true. +#[test] +fn is_subtype_index_encoding_order_differs_from_key_order() { + let _ = env_logger::try_init(); + + let mut types = Types::new(); + let g0 = RecGroupId(0); + let g1 = RecGroupId(1); + + types.insert_rec_group(g0); + types.insert_rec_group(g1); + + // TypeId(10) in g0: the supertype (no parent). + // TypeId(1) in g1: subtype of TypeId(10). + // + // BTreeMap key order: [TypeId(1), TypeId(10)] -> dense 0=TypeId(1), 1=TypeId(10) + // Encoding order: [TypeId(10), TypeId(1)] -> dense 0=TypeId(10), 1=TypeId(1) + // (g0 must come before g1 because g1's type has a supertype in g0) + types.insert_empty_struct(TypeId(10), g0, false, None); + types.insert_empty_struct(TypeId(1), g1, false, Some(TypeId(10))); + + let order = encoding_order(&types); + + // Verify that encoding order is indeed reversed from key order. + assert_eq!(order, vec![TypeId(10), TypeId(1)]); + + // index 1 (TypeId(1)) is a subtype of index 0 (TypeId(10)) + assert!(is_subtype_index(&types, 1, 0, &order)); + + // index 0 (TypeId(10)) is NOT a subtype of index 1 (TypeId(1)) + assert!(!is_subtype_index(&types, 0, 1, &order)); + + // Also verify the direct TypeId-based method works. + assert!(types.is_subtype(TypeId(1), TypeId(10))); + assert!(!types.is_subtype(TypeId(10), TypeId(1))); } #[test] @@ -626,6 +723,7 @@ fn stacktype_fixup_accepts_subtype_for_supertype_requirement() { types.insert_empty_struct(TypeId(3), g, false, Some(TypeId(2))); let num_types = u32::try_from(types.type_defs.len()).unwrap(); + let order = encoding_order(&types); // Case 1: stack has subtype 3, op requires supertype 2. let mut stack = vec![StackType::Struct(Some(2))]; @@ -637,6 +735,7 @@ fn stacktype_fixup_accepts_subtype_for_supertype_requirement() { &mut out, num_types, &types, + &order, ); // Accepted as-is: @@ -658,6 +757,7 @@ fn stacktype_fixup_accepts_subtype_for_supertype_requirement() { &mut out, num_types, &types, + &order, ); // Accepted as-is: assert!( @@ -676,6 +776,7 @@ fn stacktype_fixup_accepts_subtype_for_supertype_requirement() { &mut out, num_types, &types, + &order, ); // Not accepted. Fixup should synthesize the requested concrete type. assert_eq!(out, vec![GcOp::StructNew { type_index: 1 }]); diff --git a/crates/fuzzing/src/generators/gc_ops/types.rs b/crates/fuzzing/src/generators/gc_ops/types.rs index d4ba24eecc80..9b77250bbd9b 100644 --- a/crates/fuzzing/src/generators/gc_ops/types.rs +++ b/crates/fuzzing/src/generators/gc_ops/types.rs @@ -4,7 +4,7 @@ use crate::generators::gc_ops::limits::GcOpsLimits; use crate::generators::gc_ops::ops::GcOp; use serde::{Deserialize, Serialize}; use std::collections::btree_map::Entry; -use std::collections::{BTreeMap, BTreeSet}; +use std::collections::{BTreeMap, BTreeSet, HashMap}; use wasmtime_environ::graphs::{Dfs, DfsEvent, Graph}; /// Identifies a `(rec ...)` group. @@ -199,40 +199,53 @@ impl Types { .map(|(gid, _)| *gid) } - /// Returns true iff `sub_index` is the same as or a subtype of `sup_index`. - pub fn is_subtype_index(&self, sub_index: u32, sup_index: u32) -> bool { - if sub_index == sup_index { - return true; - } - - let i = match usize::try_from(sub_index) { - Ok(i) => i, - Err(_) => return false, - }; - let j = match usize::try_from(sup_index) { - Ok(j) => j, - Err(_) => return false, - }; - - let mut cur = match self.type_defs.keys().nth(i).copied() { - Some(t) => t, - None => return false, - }; - let sup = match self.type_defs.keys().nth(j).copied() { - Some(t) => t, - None => return false, - }; - + /// Returns true iff `sub` is the same type as, or a subtype of, `sup`. + /// + /// Walks the supertype chain from `sub` upward looking for `sup`. + pub fn is_subtype(&self, mut sub: TypeId, sup: TypeId) -> bool { loop { - if cur == sup { + if sub == sup { return true; } - let next = match self.type_defs.get(&cur).and_then(|d| d.supertype) { + let next = match self.type_defs.get(&sub).and_then(|d| d.supertype) { Some(t) => t, None => return false, }; - cur = next; + sub = next; + } + } + + /// Return the type encoding order grouped by rec group. + /// + /// Rec groups are emitted in topological order (dependencies first), + /// and within each group members are sorted by the global supertype- + /// first topological order. + pub(crate) fn encoding_order_grouped( + &self, + out: &mut Vec<(RecGroupId, Vec)>, + type_to_group: &BTreeMap, + ) { + let mut type_order = Vec::with_capacity(self.type_defs.len()); + self.sort_types_topo(&mut type_order); + + let type_position: HashMap = type_order + .iter() + .enumerate() + .map(|(i, &id)| (id, i)) + .collect(); + + let mut group_order = Vec::with_capacity(self.rec_groups.len()); + self.sort_rec_groups_topo(&mut group_order, type_to_group); + + out.clear(); + out.reserve(group_order.len()); + for gid in group_order { + if let Some(member_set) = self.rec_groups.get(&gid) { + let mut members: Vec = member_set.iter().copied().collect(); + members.sort_by_key(|tid| type_position[tid]); + out.push((gid, members)); + } } } @@ -263,12 +276,15 @@ impl Types { /// Topological sort of rec groups: if a type in group G has a /// supertype in group H, then H appears before G in the output. - pub fn sort_rec_groups_topo(&self, out: &mut Vec) { - let type_to_group = self.type_to_group_map(); + pub fn sort_rec_groups_topo( + &self, + out: &mut Vec, + type_to_group: &BTreeMap, + ) { let graph = RecGroupGraph { type_defs: &self.type_defs, rec_groups: &self.rec_groups, - type_to_group: &type_to_group, + type_to_group, }; let mut dfs = Dfs::new(graph.nodes()); @@ -326,7 +342,7 @@ impl Types { } /// Build a reverse map from type id to its owning rec group. - fn type_to_group_map(&self) -> BTreeMap { + pub(crate) fn type_to_group_map(&self) -> BTreeMap { self.rec_groups .iter() .flat_map(|(&gid, members)| members.iter().map(move |&tid| (tid, gid))) @@ -335,12 +351,11 @@ impl Types { /// Break cycles in the rec-group dependency graph by dropping cross-group /// supertype edges that are DFS back edges. - pub fn break_rec_group_cycles(&mut self) { - let type_to_group = self.type_to_group_map(); + pub fn break_rec_group_cycles(&mut self, type_to_group: &BTreeMap) { let graph = RecGroupGraph { type_defs: &self.type_defs, rec_groups: &self.rec_groups, - type_to_group: &type_to_group, + type_to_group, }; let mut seen = BTreeSet::new(); @@ -389,7 +404,11 @@ impl Types { } /// Fix up the types to ensure they are within the limits. - pub fn fixup(&mut self, limits: &GcOpsLimits) { + pub fn fixup( + &mut self, + limits: &GcOpsLimits, + encoding_order_grouped: &mut Vec<(RecGroupId, Vec)>, + ) { let max_rec_groups = usize::try_from(limits.max_rec_groups).expect("max_rec_groups is too large"); let max_types = usize::try_from(limits.max_types).expect("max_types is too large"); @@ -463,9 +482,13 @@ impl Types { // 8. Break supertype cycles and rec-group dependency cycles. self.break_supertype_cycles(); - self.break_rec_group_cycles(); + let type_to_group = self.type_to_group_map(); + self.break_rec_group_cycles(&type_to_group); debug_assert!(self.is_well_formed(limits)); + + // 9. Compute encoding order (reuses type_to_group from step 8). + self.encoding_order_grouped(encoding_order_grouped, &type_to_group); } /// Check if the types are well-formed and within configured limits, i.e. @@ -538,6 +561,7 @@ impl StackType { out: &mut Vec, num_types: u32, types: &Types, + encoding_order: &[TypeId], ) { log::trace!( "[StackType::fixup] enter req={req:?} num_types={num_types} stack_len={} stack={stack:?}", @@ -572,9 +596,18 @@ impl StackType { Some(Self::Struct(wanted)) => { let ok = match (wanted, stack.last()) { (Some(wanted), Some(Self::Struct(Some(actual)))) => { - let st = types.is_subtype_index(*actual, wanted); + let sub = encoding_order + .get(usize::try_from(*actual).expect("invalid type index")) + .copied(); + let sup = encoding_order + .get(usize::try_from(wanted).expect("invalid type index")) + .copied(); + let st = match (sub, sup) { + (Some(sub), Some(sup)) => types.is_subtype(sub, sup), + _ => false, + }; log::trace!( - "[StackType::fixup] Struct: actual={actual} wanted={wanted} is_subtype_index={st}" + "[StackType::fixup] Struct: actual={actual} wanted={wanted} is_subtype={st}" ); st }