wasmtime / PR #12923 Exceptions: add exception-specific (... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #12923 Exceptions: add exception-specific (...

Wasmtime GitHub notifications bot (Apr 01 2026 at 03:24):

cfallin opened PR #12923 from cfallin:exception-fuzzer to bytecodealliance:main:

This fuzzer uses a description of a set of "scenarios", arbitrarily generated, to produce a specific kind of module that tests throw/catch behavior. The module contains a chain of functions that invoke each other; one will throw, and the rest may have catch clauses that do or do not catch.

Wasmtime GitHub notifications bot (Apr 01 2026 at 03:24):

cfallin requested fitzgen for a review on PR #12923.

Wasmtime GitHub notifications bot (Apr 01 2026 at 03:24):

cfallin requested wasmtime-fuzz-reviewers for a review on PR #12923.

Wasmtime GitHub notifications bot (Apr 01 2026 at 03:24):

cfallin requested wasmtime-default-reviewers for a review on PR #12923.

Wasmtime GitHub notifications bot (Apr 01 2026 at 07:05):

github-actions[bot] added the label fuzzing on PR #12923.

Wasmtime GitHub notifications bot (Apr 01 2026 at 07:05):

github-actions[bot] added the label wasmtime:docs on PR #12923.

Wasmtime GitHub notifications bot (Apr 01 2026 at 07:06):

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

Subscribe to Label Action

cc @fitzgen

<details>
This issue or pull request has been labeled: "fuzzing", "wasmtime:docs"

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 (Apr 01 2026 at 17:35):

fitzgen submitted PR review:

Generally LGTM, but I want to take another look at it after the requested changes below. Thanks!

Wasmtime GitHub notifications bot (Apr 01 2026 at 17:35):

fitzgen created PR review comment:

Maybe define a constant at the top of the file, rather than open-coding 4 here

Wasmtime GitHub notifications bot (Apr 01 2026 at 17:35):

fitzgen created PR review comment:

u32::try_from(...).unwrap()

Wasmtime GitHub notifications bot (Apr 01 2026 at 17:35):

fitzgen created PR review comment:

These tests should use mutatis::check instead of a home-rolled/open-coded equivalent.

Also, you might want to be more precise with the wasm features enabled in the validator, just to be a little more tidy.

Wasmtime GitHub notifications bot (Apr 01 2026 at 17:35):

fitzgen created PR review comment:

It seems like the decoys are always before the real catch, do we want to also exercise decoys after the real catch? Okay if follow up.

Wasmtime GitHub notifications bot (Apr 01 2026 at 17:35):

fitzgen created PR review comment:

FWIW, this might be a little more future proof to reordering type definitions as
        let fn_type_void_to_i32 = types.len();
Similar for the others below.

https://docs.rs/wasm-encoder/latest/wasm_encoder/struct.TypeSection.html#method.len

Wasmtime GitHub notifications bot (Apr 01 2026 at 17:35):

fitzgen created PR review comment:

I think this one could just be derive(Mutate) on SimpleValType. This will also hook up the DefaultMutate implementation (unless you tell it not to).

https://docs.rs/mutatis/latest/mutatis/_guide/derive_macro/index.html

Wasmtime GitHub notifications bot (Apr 01 2026 at 17:35):

fitzgen created PR review comment:

Better to move the if conditions outside of the c.mutation(...) so that we don't even register a mutation candidate if we know we won't do that mutation.

Additionally, adding a param should be guarded on if !c.shrink(), so that we never do a mutation that makes the test case larger when we are trying to minimize a test case.

Also, these mutations are basically the same as the default mutations for Vec, but with a tiny bit of extra logic specific to our limits. It might be better to either

just use the default Vec mutations, and allow fixup to clean up the limits and such, or

use the default Vec mutations and then chain on a map to clamp the value afterwards:
rust use mutatis::mutators as m; let mutator = m::default::<Vec<_>>().map(|_ctx, params| { params.truncate(MAX_TAG_PARAMS); Ok(()) }); mutator.mutate(ctx, &mut value.params)

Wasmtime GitHub notifications bot (Apr 01 2026 at 17:35):

fitzgen created PR review comment:

This should just be multiple multiple .mutate(ctx) calls (only one of which will actually be performed):
value.throw_tag.mutate(ctx)?;
value.throw_depth.mutate(ctx)?;
value.catch_depth.mutate(ctx)?;
value.catch_kind.mutate(ctx)?;
value.decoy_matches.mutate(ctx)?;
Ok(())
And in fact this is exactly what would be generated by putting derive(Mutate) on Scenario, so we should just do that.

Wasmtime GitHub notifications bot (Apr 01 2026 at 17:35):

fitzgen created PR review comment:

Same here

Wasmtime GitHub notifications bot (Apr 01 2026 at 17:35):

fitzgen created PR review comment:

We shouldn't need Arbitrary here. It only exists in the GC ops fuzzer for temp compat.

Wasmtime GitHub notifications bot (Apr 01 2026 at 17:35):

fitzgen created PR review comment:

Maybe we should also test pulley?

Wasmtime GitHub notifications bot (Apr 01 2026 at 17:35):

fitzgen created PR review comment:

Is the generated Wasm not known to be valid? I think it is, no?

Wasmtime GitHub notifications bot (Apr 01 2026 at 17:35):

fitzgen created PR review comment:

Ditto regarding matching on a random int to choose which field to mutate vs registering multiple mutation candidates by calling .mutate(...) on each field.

And I think we could derive(Mutate) on ExceptionOps too.

Probably can just derive on all the "AST" types, honestly, and then just let fixup clean up the limits and such.

Wasmtime GitHub notifications bot (Apr 01 2026 at 22:00):

cfallin updated PR #12923.

Wasmtime GitHub notifications bot (Apr 01 2026 at 22:00):

cfallin created PR review comment:

Done!

Wasmtime GitHub notifications bot (Apr 01 2026 at 22:00):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Apr 01 2026 at 22:00):

cfallin created PR review comment:

Done!

Wasmtime GitHub notifications bot (Apr 01 2026 at 22:00):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Apr 01 2026 at 22:01):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Apr 01 2026 at 22:01):

cfallin created PR review comment:

Done!

Wasmtime GitHub notifications bot (Apr 01 2026 at 22:01):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Apr 01 2026 at 22:01):

cfallin created PR review comment:

Done!

Wasmtime GitHub notifications bot (Apr 01 2026 at 22:01):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Apr 01 2026 at 22:01):

cfallin created PR review comment:

I switched to derive(Mutate) where possible but it seems that the generated code from the derive macro can't switch between enum variants (see your TODO comment here) so there's still a manual impl for this and CatchKind.

Wasmtime GitHub notifications bot (Apr 01 2026 at 22:01):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Apr 01 2026 at 22:01):

cfallin created PR review comment:

(see above)

Wasmtime GitHub notifications bot (Apr 01 2026 at 22:01):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Apr 01 2026 at 22:01):

cfallin created PR review comment:

Gone now as we use derived mutators.

Wasmtime GitHub notifications bot (Apr 01 2026 at 22:01):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Apr 01 2026 at 22:01):

cfallin created PR review comment:

Gone now as we derive mutators.

Wasmtime GitHub notifications bot (Apr 01 2026 at 22:01):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Apr 01 2026 at 22:01):

cfallin created PR review comment:

Yep, gone now too.

Wasmtime GitHub notifications bot (Apr 01 2026 at 22:01):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Apr 01 2026 at 22:01):

cfallin created PR review comment:

Done!

I kept the produces-valid-wasm test but removed the runs-oracle-successfully test because the latter is, well, just fuzzing and seemed a bit redundant.