wasmtime / PR #12015 Stack types · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #12015 Stack types

Wasmtime GitHub notifications bot (Nov 11 2025 at 01:24):

khagankhan opened PR #12015 from khagankhan:stack-types to bytecodealliance:main:

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

Wasmtime GitHub notifications bot (Nov 11 2025 at 01:24):

khagankhan requested alexcrichton for a review on PR #12015.

Wasmtime GitHub notifications bot (Nov 11 2025 at 01:24):

khagankhan requested wasmtime-fuzz-reviewers for a review on PR #12015.

Wasmtime GitHub notifications bot (Nov 11 2025 at 01:58):

khagankhan updated PR #12015.

Wasmtime GitHub notifications bot (Nov 11 2025 at 02:12):

khagankhan submitted PR review.

Wasmtime GitHub notifications bot (Nov 11 2025 at 02:12):

khagankhan created PR review comment:

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.

Wasmtime GitHub notifications bot (Nov 11 2025 at 02:12):

khagankhan submitted PR review.

Wasmtime GitHub notifications bot (Nov 11 2025 at 02:12):

khagankhan created PR review comment:

Same "special case" here

Wasmtime GitHub notifications bot (Nov 11 2025 at 02:12):

khagankhan commented on PR #12015:

@fitzgen Ready for review!

Wasmtime GitHub notifications bot (Nov 11 2025 at 03:54):

github-actions[bot] commented on PR #12015:

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>

Wasmtime GitHub notifications bot (Nov 14 2025 at 02:51):

alexcrichton commented on PR #12015:

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?

Wasmtime GitHub notifications bot (Nov 14 2025 at 02:57):

khagankhan commented on PR #12015:

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!

Wasmtime GitHub notifications bot (Nov 14 2025 at 03:00):

alexcrichton requested fitzgen for a review on PR #12015.

Wasmtime GitHub notifications bot (Nov 19 2025 at 19:41):

fitzgen created PR review comment:

(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).

Wasmtime GitHub notifications bot (Nov 19 2025 at 19:41):

fitzgen created PR review comment:

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

Wasmtime GitHub notifications bot (Nov 19 2025 at 19:41):

fitzgen created PR review comment:

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

Wasmtime GitHub notifications bot (Nov 19 2025 at 19:41):

fitzgen created PR review comment:

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.

Wasmtime GitHub notifications bot (Nov 19 2025 at 19:41):

fitzgen submitted PR review:

I'm back now, thanks for your patience!

Wasmtime GitHub notifications bot (Nov 19 2025 at 19:41):

fitzgen created PR review comment:

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?

Wasmtime GitHub notifications bot (Nov 19 2025 at 19:41):

fitzgen created PR review comment:

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?

Wasmtime GitHub notifications bot (Nov 19 2025 at 19:41):

fitzgen created PR review comment:

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:
            pub(crate) fn operand_types(&self, types: &mut Vec<StackType>) {

Wasmtime GitHub notifications bot (Dec 01 2025 at 20:38):

khagankhan submitted PR review.

Wasmtime GitHub notifications bot (Dec 01 2025 at 20:38):

khagankhan created PR review comment:

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?

Wasmtime GitHub notifications bot (Dec 01 2025 at 20:39):

khagankhan edited PR review comment.

Wasmtime GitHub notifications bot (Dec 01 2025 at 22:36):

fitzgen created PR review comment:

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)

Wasmtime GitHub notifications bot (Dec 01 2025 at 22:36):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Dec 01 2025 at 22:39):

khagankhan submitted PR review.

Wasmtime GitHub notifications bot (Dec 01 2025 at 22:39):

khagankhan created PR review comment:

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

Thanks a lot

Last updated: Dec 06 2025 at 07:03 UTC