khagankhan opened PR #12538 from khagankhan:rec-struct-mutators to bytecodealliance:main:
Mutators added for
(rec ...)groups and struct types
- Define a mutation that adds an empty struct type to an existing
(rec ...)group- Define a mutation that removes a struct type from an existing
(rec ...)group- Define a mutation that moves a struct type within an existing
(rec ...)group- Define a mutation that moves a struct type from an existing
(rec ...)group to another- Define a mutation that duplicates a
(rec ...)group- Define a mutation that removes a whole
(rec ...)group- Define a mutation that merges two
(rec ...)groups- Define a mutation that splits a
(rec ...)group in two, if possibleI have implemented the above mutators. I will keep updating/refining them as we add more features to recursive groups and struct definitions. Ready for the review! Thanks!
cc @fitzgen @eeide
khagankhan requested fitzgen for a review on PR #12538.
khagankhan requested wasmtime-fuzz-reviewers for a review on PR #12538.
khagankhan updated PR #12538.
github-actions[bot] added the label fuzzing on PR #12538.
github-actions[bot] commented on PR #12538:
Subscribe to Label Action
cc @fitzgen
<details>
This issue or pull request has been labeled: "fuzzing"Thus the following users have been cc'd because of the following labels:
- fitzgen: fuzzing
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
fitzgen submitted PR review:
Thanks, just a few little things before merging.
fitzgen created PR review comment:
Can we pull each of the mutations out into their own helper functions? This method is way too big after adding all these new mutations inline.
Something like this:
impl GcOpsMutator { fn add_new_struct_type_to_rec_group( &self, c: &mut Candidates<'_>, ops: &mut GcOps, ) -> mutatis::Result<()> { if c.shrink() || ops.types.rec_groups.is_empty() || ops.types.type_defs.len() >= usize::try_from(ops.limits.max_types).unwrap() { return Ok(()); } c.mutation(|ctx| { let group_id = ctx.rng().choose(&ops.types.rec_groups) .expect("already checked for empty rec groups above"); let new_tid = ops.types.next_type_id(); ops.types.insert_empty_struct(new_tid, group_id); log::debug!("Added empty struct type {new_tid:?} to rec group {group_id:?}"); Ok(()) }) } // etc... for the other mutations } impl Mutate<GcOps> for GcOpsMutator { fn mutate(&mut self, c: &mut Candidates<'_>, ops: &mut GcOps) -> mutatis::Result<()> { self.add_gc_op(c, ops)?; self.remove_gc_op(c, ops)?; self.add_new_struct_type_to_rec_group(c, ops)?; // etc... Ok(()) } }
fitzgen created PR review comment:
We've already checked for the case where
rec_groups.is_empty()above, so we can just unwrap/expect here, instead of checking that it is non-empty again.
fitzgen created PR review comment:
Same thing about double-checking for emptiness. Here and elsewhere.
fitzgen created PR review comment:
This can be simplified to
debug_assert_ne!(num_types, 0, "...");
fitzgen created PR review comment:
I think
saturating_addisn't good enough here. The fuzzer could mutate the serialized representation ofTypesto make a type or rec group have its max id, at which point these methods and theirsaturating_addwill saturate and return that same id.You could do something like the following to choose either
max(keys) + 1or else the first gap in the keys:let mut last_id = None; let mut keys = self.type_defs.keys().rev(); let next_id = 'outer: { while let Some(key) = keys.next() { match (key.0.checked_add(1), last_key) { (Some(x), None) => break 'outer TypeId(x), (Some(x), Some(y)) if x != y => break 'outer TypeId(x), (Some(_), Some(_)) | (None, _) => { last_id = Some(key.0); } } } match last_id { None => TypeId(0), Some(x) => TypeId(x - 1), } }; debug_assert!(!self.type_defs.contains_key(next_id)); next_idBut this is kinda complicated for what it is doing and does devolve into an $O(n)$ loop, but probably pretty rarely.
The other thing we could do here is just take an
rng: &mut mutatis::Rngparameter and do something likefor _ in 0..1000 { let id = TypeId(rng.gen_u32()); if !self.type_defs.contains_key(&id) { return id } } panic!("failed to generate a new TypeId in 1000 iterations; bad RNG?")which should be good enough.
khagankhan updated PR #12538.
khagankhan commented on PR #12538:
Thanks @fitzgen! Ready for the review!
fitzgen submitted PR review:
Thanks!
fitzgen added PR #12538 Implement rec groups and struct mutators to the merge queue.
fitzgen merged PR #12538.
fitzgen removed PR #12538 Implement rec groups and struct mutators from the merge queue.
Last updated: Feb 24 2026 at 04:36 UTC