Stack types #12015

khagankhan · 2025-11-11T01:24:04Z

Stack fixup

We currently have every instruction balanced itself (for example, if an op leaves a struct onto the stack it immediately calls a function that consumes that struct) like we did for our generative fuzzer. However, for mutation based fuzzers this may have some bias.

This PR removes that and fixes the stack in the end. It keeps abstract stack types and check the required types then fixes the actual stack.

+cc @fitzgen @eeide

khagankhan · 2025-11-11T02:12:06Z

crates/fuzzing/src/generators/gc_ops/ops.rs

+            pub(crate) fn operand_types(&self) -> Vec<StackType> {
                match self {
+                    // special-cases
+                    Self::TakeTypedStructCall(t) => vec![StackType::Struct(Some(*t))],


I’d like to remove these “special cases” that require an index. The macro expansion doesn’t accept it and I’d rather not complicate the macro further. If you know a clean way to handle this, please share it with me. I’ll address it in the next PR anyway.

khagankhan · 2025-11-11T02:12:22Z

crates/fuzzing/src/generators/gc_ops/ops.rs

+            pub(crate) fn result_types(&self) -> Vec<StackType> {
+                match self {
+                    // special-cases
+                    Self::StructNew(t)        => vec![StackType::Struct(Some(*t))],


Same "special case" here

khagankhan · 2025-11-11T02:12:56Z

@fitzgen Ready for review!

github-actions · 2025-11-11T03:54:12Z

Subscribe to Label Action

cc @fitzgen

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 .github/subscribe-to-label.json configuration file.

Learn more.

alexcrichton · 2025-11-14T02:51:51Z

Thanks! Nick's out this week and I so far haven't had a chance to look at this. I may end up deferring this to Nick when he gets back as I'll otherwise have to boot back up on a lot of context here, but do you have other subsequent PRs ready to go which are built on this and so it'd be good to get this in sooner rather than later?

khagankhan · 2025-11-14T02:57:34Z

Hey Alex! I am mostly working on a repo on GitLab where I am ahead. The Wasmtime PRs tend to lag behind my current work because I address comments, failed tests etc. Since Nick and I meet weekly (except this week) and go over everything, I think it makes sense to defer this to him. Thank you for the comment!

fitzgen

I'm back now, thanks for your patience!

fitzgen · 2025-11-19T19:21:29Z

crates/fuzzing/src/generators/gc_ops/ops.rs

+            match &mut op {
+                GcOp::StructNew(t) | GcOp::TakeStructCall(t) | GcOp::TakeTypedStructCall(t) => {
+                    if num_types > 0 {
+                        *t = *t % num_types;
+                    }


I think this fixing-up of ops should go in GcOp::fixup. We can add num_types as a parameter there. And if num_types == 0, we should probably just remove this op, no? How do we struct.new or call a function that takes a concrete struct reference if we don't define any types?

fitzgen · 2025-11-19T19:24:01Z

crates/fuzzing/src/generators/gc_ops/ops.rs

-        /// 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.


Stylistically, it is quite rare to have the doc comment below the #[...] attributes. Mind reverting this code motion?

fitzgen · 2025-11-19T19:25:47Z

crates/fuzzing/src/generators/gc_ops/ops.rs


-            pub fn results_len(&self) -> usize {
+            #[allow(unreachable_patterns, reason = "macro-generated code")]
+            pub(crate) fn operand_types(&self) -> Vec<StackType> {


To avoid a ton of temporary allocations, and reuse a single allocation instead, let's have this method take a mutable vector as an out parameter:

Suggested change

pub(crate) fn operand_types(&self) -> Vec<StackType> {

pub(crate) fn operand_types(&self, types: &mut Vec<StackType>) {

fitzgen · 2025-11-19T19:26:54Z

crates/fuzzing/src/generators/gc_ops/ops.rs

-                let new_stack = stack - $params + $results;
-                Ok((op, new_stack))
+            #[allow(unreachable_patterns, reason = "macro-generated code")]
+            pub(crate) fn result_types(&self) -> Vec<StackType> {


And let's also do an out parameter here as well.

fitzgen · 2025-11-19T19:28:59Z

crates/fuzzing/src/generators/gc_ops/ops.rs

+            fn $op(
+                _ctx: &mut mutatis::Context,
+                _limits: &GcOpsLimits,
+                stack: usize,
+            ) -> mutatis::Result<(GcOp, usize)> {


Should this still be taking a stack: usize and returning a new usize? Shouldn't it be taking a stack: &mut Vec<StackType> now instead?

I am back!!!

Does this also mean that when we generate a new op, we should also be checking type compatibility, not just maintaining stack depth?

@fitzgen

Does this also mean that when we generate a new op, we should also be checking type compatibility, not just maintaining stack depth?

Yes, we either have to do that (if we continue the existing approach) or else we switch to a new approach and instead just generate a random sequence of arbitrary ops and then rely on the fixup pass to make it valid. The latter might be easier long term, so that there is only one code path we have to maintain.

But also, backing up, we shouldn't really need to generate op sequences from scratch. The whole point of using a mutation-based paradigm is that we can generate arbitrary inputs via a series of mutations over time. So, instead of generating whole op sequences, we should be able to get away with something like

impl Generate<GcOps> for GcOpsMutator { fn generate(&mut self, ctx: &mut Context) -> mutatis::Result<GcOps> { let mut ops = GcOps::default(); for _ in 0..N { self.mutate(ctx, &mut ops)?; } Ok(ops) } }

(And we should probably build the generic version of this into mutatis directly eventually)

Awesome! That was the answer I needed. After addressing this I will push

Thanks a lot

fitzgen · 2025-11-19T19:33:57Z

crates/fuzzing/src/generators/gc_ops/types.rs

+    /// Any value is used for reauested operand not a type left on stack (only for Drop and specially handled ops)
+    Anything,


Let's align with the Wasm spec and call this AnyRef.

We will eventually want to differentiate between (ref struct) and (ref any) and (ref eq) as well.

Aside: we will need to track nullability for all our different ref types eventually as well.

fitzgen · 2025-11-19T19:39:08Z

crates/fuzzing/src/generators/gc_ops/types.rs

+                // 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
+                    Self::emit(GcOp::Null(), stack, out, num_types);
+                    stack.pop(); // consume just-synthesized externref
+                }


(ref extern) is a different type hierarchy from (ref any), that is there is no single top type of everything, so there is no function that can take anything and I want to make sure we don't build that assumption in here.

Backing up a bit: I am a bit confused about this function's purpose and why it is necessary. It seems like we shouldn't ever be changing the stack types, we should be only be doing the opposite: fixing up our ops given the types that are actually on the stack. The former doesn't make sense (we can't change what types are on the stack at this point without changing what instructions we emitted earlier).

khagankhan added 2 commits November 10, 2025 17:40

Stack fixup for new types

53330dd

Add new test for fixup()

8423dcc

khagankhan requested a review from a team as a code owner November 11, 2025 01:24

khagankhan requested review from alexcrichton and removed request for a team November 11, 2025 01:24

Address clippy failure

8001986

khagankhan commented Nov 11, 2025

View reviewed changes

github-actions bot added the fuzzing Issues related to our fuzzing infrastructure label Nov 11, 2025

alexcrichton requested review from fitzgen and removed request for alexcrichton November 14, 2025 03:00

fitzgen reviewed Nov 19, 2025

View reviewed changes

	pub(crate) fn operand_types(&self) -> Vec<StackType> {
	pub(crate) fn operand_types(&self, types: &mut Vec<StackType>) {

		/// Any value is used for reauested operand not a type left on stack (only for Drop and specially handled ops)
		Anything,

Stack types #12015

Are you sure you want to change the base?

Stack types #12015

Conversation

khagankhan commented Nov 11, 2025

Stack fixup

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

khagankhan commented Nov 11, 2025

Uh oh!

github-actions bot commented Nov 11, 2025

Subscribe to Label Action

Uh oh!

alexcrichton commented Nov 14, 2025

Uh oh!

khagankhan commented Nov 14, 2025

Uh oh!

fitzgen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

khagankhan Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

khagankhan Dec 1, 2025 •

edited

Loading