Stream: git-wasmtime

Topic: wasmtime / PR #11326 WIP: initial WebAssembly exception-h...


view this post on Zulip Wasmtime GitHub notifications bot (Jul 25 2025 at 23:43):

cfallin opened PR #11326 from cfallin:wasm-exceptions to bytecodealliance:main:

This PR introduces support for the [Wasm exception-handling proposal], which introduces a conventional try/catch mechanism to WebAssembly. The PR supports modules that use try_table to register handlers for a lexical scope; and provides throw and throw_ref that allocate (in the first case) and throw exception objects.

This PR builds on top of the work in #10510 for Cranelift-level exception support, #10919 for an unwinder, and #11230 for exception objects built on top of GC, in addition a bunch of smaller fix and enabling PRs around those. It is currently stacked on top of #11321.

This PR does not yet provide host-boundary-crossing exceptions; exceptions that are not caught in a given Wasm activation become traps at the host boundary. That support will come in a subsequent PR.

Because exceptions do not yet cross the host boundary, this also does not yet enable the assert_exception wast directive, and so cannot yet support the spec-tests. That will also come in a subsequent PR.

[Wasm exception-handling proposal]: https://github.com/WebAssembly/exception-handling/

view this post on Zulip Wasmtime GitHub notifications bot (Jul 25 2025 at 23:45):

cfallin commented on PR #11326:

Logistical note: I'm posting this as a draft now to get early feedback and because I know folks are waiting to see how it is shaping up. I'm on vacation for two weeks starting now (back Mon Aug 11) and will plan to polish then. I'm hoping to actually get host-boundary integration built as well, if I can, in this PR, to enable spec-tests, but if that turns out to be too much then it will come right after. Following that, fuzzing is the only piece that remains, I think.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 26 2025 at 00:49):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 26 2025 at 01:15):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 26 2025 at 01:16):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 26 2025 at 02:01):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 26 2025 at 02:01):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 26 2025 at 02:01):

cfallin edited PR #11326:

This PR introduces support for the [Wasm exception-handling proposal], which introduces a conventional try/catch mechanism to WebAssembly. The PR supports modules that use try_table to register handlers for a lexical scope; and provides throw and throw_ref that allocate (in the first case) and throw exception objects.

This PR builds on top of the work in #10510 for Cranelift-level exception support, #10919 for an unwinder, and #11230 for exception objects built on top of GC, in addition a bunch of smaller fix and enabling PRs around those.

This PR does not yet provide host-boundary-crossing exceptions; exceptions that are not caught in a given Wasm activation become traps at the host boundary. That support will come in a subsequent PR.

Because exceptions do not yet cross the host boundary, this also does not yet enable the assert_exception wast directive, and so cannot yet support the spec-tests. That will also come in a subsequent PR.

[Wasm exception-handling proposal]: https://github.com/WebAssembly/exception-handling/

view this post on Zulip Wasmtime GitHub notifications bot (Jul 26 2025 at 02:05):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 26 2025 at 21:10):

alexcrichton submitted PR review:

I've left some high-level thoughts here and there, but definitely feel free to defer anything to issues as you feel appropriate.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 26 2025 at 21:10):

alexcrichton created PR review comment:

Could this be made safe and return an action to perform rather than performing the action? I get jittery skipping Rust frames nowadays and I feel it works best when this propagates upwards as far as it can the action to take before actually taking the action. One example is that with Pulley this can't return ! because it'll need to propagate to the interpreter loop to update interpreter state to perform the pseudo-longjmp

view this post on Zulip Wasmtime GitHub notifications bot (Jul 26 2025 at 21:10):

alexcrichton created PR review comment:

Could this be skipped if the exception tables section is empty?

view this post on Zulip Wasmtime GitHub notifications bot (Jul 26 2025 at 21:10):

alexcrichton created PR review comment:

In the long-term I think we'll want to handle this differently because store here aliases nogc without the compiler knowing. This is probably fine for now but if this sticks around can you file an issue to improve this in the future?

One possible idea would be to move the InstanceId into the VMContext and to read that out and then lookup through nogc the InstanceId. Another option would be to add a helper method to Instance which takes the store and the VMContext and returns Pin<&mut Instance> connected to the lifetime of the store, avoiding creating a second store pointer and requiring the store has nothing else borrowed at the same time.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 26 2025 at 21:10):

alexcrichton created PR review comment:

Could this be done with a #[cfg(debug_assertions)] perhaps? That'll help avoid paging this in if necessary and by construction this should always be true as a result of compilation such that it shouldn't be required to check here too.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 26 2025 at 21:10):

alexcrichton created PR review comment:

This might be best to switch to GC_TYPES instead of GC since that's the true reliance here, there's no need to enable structs/arrays to enable exceptions too. (just that under the hood exceptions is implemented with GC types)

view this post on Zulip Wasmtime GitHub notifications bot (Jul 26 2025 at 21:10):

alexcrichton created PR review comment:

Mind adding a tests/disas test for this showing the exception tables?

view this post on Zulip Wasmtime GitHub notifications bot (Jul 26 2025 at 21:10):

alexcrichton created PR review comment:

To confirm, parse here is a pretty cheap operation?

view this post on Zulip Wasmtime GitHub notifications bot (Jul 26 2025 at 21:10):

alexcrichton created PR review comment:

Could you expand the documentation here that this should be used judiciously and specifically mention that the closure f is executed under a read lock? That'll help communicate that this can block for awhile waiting for a write lock to finish and additionally this is blocking all writers at the same time.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 26 2025 at 21:10):

alexcrichton created PR review comment:

Mind double-checking there are tests for these new offset methods (and the ones below) in vmcontext.rs?

view this post on Zulip Wasmtime GitHub notifications bot (Jul 26 2025 at 21:10):

alexcrichton created PR review comment:

I'm not sure whether it's viable, but personally what I'd ideally like to see is the request-to-throw propagated even through the libcall here through the Result. That'd likely require some finesse and refactoring but I suspect such refactoring is going to be required no matter what for Pulley support eventually.

In lieu of that though I'd ideally like to see the "do the throw" action no higher than here, so my comment below about threading the action-to-do would be handled around here. (although I could also be missing other places where it's acted upon)

view this post on Zulip Wasmtime GitHub notifications bot (Jul 29 2025 at 16:52):

fitzgen submitted PR review:

Shaping up nicely!

We had talked elsewhere about removing the exception composite type variant and having tags refer to just their function type (but we would now optionally have a GC struct layout for a function type's parameters for use with exceptions). This would better align us with the Wasm spec and make it so that there are less new additions to the types registry code and also fewer interactions at runtime with its locking and tables and all that. Are you still planning on pursuing this?

view this post on Zulip Wasmtime GitHub notifications bot (Jul 29 2025 at 16:52):

fitzgen created PR review comment:

Might as well smallvec![] these temporaries.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 29 2025 at 16:52):

fitzgen created PR review comment:

and there are no frames on the stack between the Wasm and here that must run to maintain some safety invariant, and cannot be unwound over.

This is ~impossible to determine in arbitrary code, which is why pushing the command-return all the way out to the libcall is nice, since we know there is ~nothing on the stack before the Wasm frames at that point.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 29 2025 at 16:52):

fitzgen created PR review comment:

One possible idea would be to move the InstanceId into the VMContext and to read that out and then lookup through nogc the InstanceId. Another option would be to add a helper method to Instance which takes the store and the VMContext and returns Pin<&mut Instance> connected to the lifetime of the store, avoiding creating a second store pointer and requiring the store has nothing else borrowed at the same time.

Another variant on this idea: we could also have a method on the store that takes a vmctx and gives an instance, something like

impl StoreOpaque {
    // Safety: vmctx must be a valid pointer
    pub unsafe fn instance_from_vmctx(&mut self, vmctx: *mut VMContext) -> Option<Pin<&mut Instance>> {
        // Validate that the vmctx is from this store and all that...
    }
}

view this post on Zulip Wasmtime GitHub notifications bot (Jul 29 2025 at 16:52):

fitzgen created PR review comment:

Where is this used? I don't actually see it used anywhere and C-f isn't finding anything.

Also, it is probably better, in terms of horizontal scalability, to clone the layout out of the registry than to hold the lock for the duration of whatever code needs to work with it. If that is something we need to do often, we can look into making that cheaper, eg by storing layouts in an Arc or something.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 29 2025 at 16:52):

fitzgen created PR review comment:

Rather than dynamically assert that these are unreachable at runtime, we can statically assert that they are unreachable at compile time via

impl From<VMStructRef> for VMGcRef {
    fn from(s: VMStructRef) -> VMGcRef {
        match s {}
    }
}

etc...

This is nice because any mistakes are caught at compile time.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 29 2025 at 16:52):

fitzgen created PR review comment:

I don't think this is correct? I think we mark our own (callee) vmctx as "the" vmctx for Cranelift and the caller's vmctx is the second argument that follows afterwards. For example:

https://github.com/bytecodealliance/wasmtime/blob/1155d6dfbfe859cad85cfe0d5700d4acefd04745/crates/cranelift/src/lib.rs#L145-L160

view this post on Zulip Wasmtime GitHub notifications bot (Jul 29 2025 at 17:06):

cfallin commented on PR #11326:

We had talked elsewhere about removing the exception composite type variant and having tags refer to just their function type (but we would now optionally have a GC struct layout for a function type's parameters for use with exceptions). This would better align us with the Wasm spec and make it so that there are less new additions to the types registry code and also fewer interactions at runtime with its locking and tables and all that. Are you still planning on pursuing this?

Ah, sorry, I hadn't made a note in the PR message here, but: I tried and abandoned that path. (Or more precisely, having exception layouts hang off of the function type; TagType is already a thin newtype wrapper around FuncType.) At a high level, pulling on that string seemed to unwind way too much structure and hit too many places that really still wanted a concrete type for an exception object. If you're curious, my WIP branch is here (still has many type-errors, mid-refactor). In essence I think that path leads to more complexity, not less, unfortunately.

also fewer interactions at runtime with its locking and tables and all that

The current implementation performs no locking or accesses to the type registry at runtime; it uses the dynamic context mechanism in Cranelift to get straight to the instance, then look up tags (VMTagDefinitions), and compares tag IDs (instance-id/defined-tag-index). In particular, the tag information is at a constant offset in the exception object (similar to array lengths) so we don't need a layout to write the generic path.

(Still on PTO but will respond occasionally to keep review moving)

view this post on Zulip Wasmtime GitHub notifications bot (Jul 29 2025 at 17:11):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 29 2025 at 17:11):

cfallin created PR review comment:

Sorry, this is left over from an earlier implementation that fetched layouts dynamically before I had put the tag info at a constant offset in exception objects. (Or rather, it was always at a constant offset per allocator, but I hadn't done the plumbing that array-lengths have to avoid getting it from the layout.)

I'll remove this; but to clarify, we have no need to access layouts or the type registry at all at runtime with the exceptions implementation. The unwinder is completely generic over exception layouts and only compares tag identities.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 29 2025 at 17:45):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 29 2025 at 17:45):

cfallin created PR review comment:

Yes -- it reads two u32s for lengths of arrays, then builds an ExceptionTable with slices over arrays.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 29 2025 at 17:53):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 29 2025 at 17:53):

cfallin created PR review comment:

This is in the context of a Call (codegen for a callsite), so the "caller" is the current callsite; and this change is pulling out an existing expression that was bound as let caller_vmctx = ... (see the diff in unchecked_call_impl for example). Happy to rename it to something else, but callee_vmctx would be inaccurate/misleading in this context, IMHO. our_vmctx maybe?

view this post on Zulip Wasmtime GitHub notifications bot (Jul 29 2025 at 17:54):

cfallin edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 29 2025 at 18:13):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 29 2025 at 18:13):

cfallin created PR review comment:

Ah, good call; the first two (methods on VMGcRef) can't follow that pattern because VMGcRef is not actually an uninhabitable enum in the GC-disabled build, so I followed that in the From impls as well, but those can follow the empty-match pattern...

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 00:52):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 01:03):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 01:03):

cfallin created PR review comment:

(Removed in rebase.)

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 01:16):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 03:56):

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

Subscribe to Label Action

cc @fitzgen

<details>
This issue or pull request has been labeled: "cranelift", "cranelift:area:aarch64", "fuzzing"

Thus the following users have been cc'd because of the following labels:

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 20:34):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 20:34):

cfallin created PR review comment:

We actually need the GC, though, since we allocate (exception) objects, no?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 20:34):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 20:34):

cfallin created PR review comment:

Sure, happy to do so. I was following precedent in the rest of this parsing code that returns a clean Err on e.g. missing sections; but I agree we don't have to do so, since our overall safety condition is that the loaded code artifact is the exact output that our compile step produced. I'll add a comment noting this.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 20:35):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 20:35):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 20:35):

cfallin created PR review comment:

Added!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 20:35):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 20:35):

cfallin created PR review comment:

Done!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 20:36):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 20:36):

cfallin created PR review comment:

(resolving but feel free to unresolve if you'd like a different name here)

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 20:36):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 20:36):

cfallin created PR review comment:

Done!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 20:45):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 20:45):

alexcrichton created PR review comment:

We need Wasmtime's implementation of a GC yeah, but we don't need the GC-the-wasm-proposal which is what WasmFeatures::GC is here.

Wasmparser's validation is here which effectively equates GC_TYPES to "every heap type other than functions/exceptions". That's based on a now-incorrect assumption that exnref wouldn't require a GC in wasmtime.

On another hand though you can probably delete this entirely. If the exceptions proposal is enabled but GC is disabled then we can let the validator figure it out. If GC_TYPES is disabled then the program can't use exnref anywhere. The only real constraint is that if EXCEPTIONS is enabled then feature = "gc" should be enabled since our runtime support requires the GC.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 21:43):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 21:47):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 21:47):

cfallin created PR review comment:

Addressed in 3c48653a25 -- I went with a variant of StoreOpaque::instance_from_vmctx, except with the additional safety requirement that the vmctx be from that store. This is valid/satisfiable from the stack walk because all Wasm frames in an activation (up to entry from host) will belong to instances in a single store; and is nice not just for perf reasons but because I was having trouble reasoning about the aliasing implications of creating an instance pointer first, and reading out its store pointer, then comparing it to see if we return Some or not. (It would be perfectly valid for another thread to own some other Store; that thread could mutate the instance pointers freely per Rust's aliasing rules; so I think we can't have the instance pointer then do the check, we have to know by invariant that it belongs to us first; but fortunately we do.)

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 21:49):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 21:50):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 21:50):

cfallin created PR review comment:

Ah, right, I had been assuming we separately checked that feature = "gc" was enabled when the GC feature is enabled at the wasmparser level, then leaning on that as the load-bearing thing to ensure we have feature = "gc". Addressed in a076222f88 -- switched to an explicit check under not(feature = "gc") to ensure EXCEPTIONS is not enabled.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 22:00):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 12 2025 at 22:00):

cfallin created PR review comment:

It could, but it adds some complication to parsing, and an empty section is 8 bytes of content (plus the section header of 64 bytes) so IMHO not too big a deal?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 13 2025 at 07:08):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 13 2025 at 07:08):

cfallin created PR review comment:

I've been working on this refactor and agree that it's much cleaner, with one caveat: I'm having trouble finding a way to generically provide catch_unwind_and_record_trap with the store (which it needs to compute the exception unwind action). It takes a FnOnce which ordinarily itself carries the unique ownership of the store. The usual idiom would be to take a &mut Store (or moral equivalent) as another arg, and re-pass that into the FnOnce; but the various use-sites use either InstanceAndStore (libcalls; array-call shim), or ComponentInstance (host funcs for the component case), or Caller (typed funcrefs). I suppose we could require the closure to return the HostResult and a mut store borrow of some sort. I'm a little lost with the various wrapper types we have and their invariants, though, and could use some ideas!

My WIP branch for this refactor is here; see last commit.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 13 2025 at 07:21):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 13 2025 at 07:21):

cfallin created PR review comment:

Or said another way -- what I want is I think something like AsContextMut, but at the monomorphized level (for StoreOpaque). I'm not sure whether that's reasonable to build out, or not. I could use a pairing session to work through this, if someone's up for it (@fitzgen or @alexcrichton?).

view this post on Zulip Wasmtime GitHub notifications bot (Aug 13 2025 at 08:06):

cfallin edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 13 2025 at 14:46):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 13 2025 at 14:46):

alexcrichton created PR review comment:

I'm not sure I quite follow but sounds like you got it working? Certainly happy to talk in more depth or do a closer review later too

view this post on Zulip Wasmtime GitHub notifications bot (Aug 13 2025 at 15:09):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 13 2025 at 15:09):

cfallin created PR review comment:

kind of -- this does mean that we resolve the exception very late, after it is recorded in the CallThreadState, because the store is fundamentally owned by the closure so we can't have access to it once we get the HostResult (which may be an exception throw). Instead we resolve it when the trampoline re-enters Rust code to call unwind(). That implies that we potentially need a new GC root now if a CallThreadState exists, though perhaps we can rely on the fact that the "record trap/exception" and "unwind" happen very close to each other and get away with storing a raw VMExnRef instead. Happy to hear thoughts from @fitzgen on that one...

view this post on Zulip Wasmtime GitHub notifications bot (Aug 13 2025 at 15:27):

fitzgen submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 13 2025 at 15:27):

fitzgen created PR review comment:

Generally it is okay to store a raw GC ref as long as it is within an AutoAssertNoGc scope, so that we dynamically catch any errors. But creating one of those requires access to a store, which seems like it is the difficult bit here? I'd have to look more into the exact code to say anything more, maybe we can discuss it in the Cranelift meeting today or something

view this post on Zulip Wasmtime GitHub notifications bot (Aug 14 2025 at 23:28):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 14 2025 at 23:46):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 00:14):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 00:17):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 00:19):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 00:20):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 00:20):

cfallin created PR review comment:

Addressed in 1f9f65ad4f6bd24d1fe15120845689f1e11bbd46!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 00:20):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 00:20):

cfallin created PR review comment:

Addressed in 1f9f65ad4f6bd24d1fe15120845689f1e11bbd46!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 00:20):

cfallin created PR review comment:

Addressed in 1f9f65ad4f6bd24d1fe15120845689f1e11bbd46!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 00:20):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 00:36):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 00:36):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 00:36):

cfallin created PR review comment:

Done!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 00:39):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 00:51):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 00:54):

cfallin commented on PR #11326:

@alexcrichton @fitzgen I've now done the refactor we discussed and also added support for Wasm-to-host, host-to-Wasm, and host-to-host (via Func::new -> Func::call) exception throws. The remaining bit to address is Pulley support (I suspect this should be relatively straightforward now?) and adding assert_exception to our WAST runner then enabling spec-tests. I believe the core should be ready for another pass.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 16:29):

alexcrichton created PR review comment:

I think that this isn't quite true any more?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 16:29):

alexcrichton created PR review comment:

I believe this is unsound because there's a rust destructor on the stack (AutoAssertNoGc) which is skipped as part of the longjmp and/or unwind resumption.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 16:29):

alexcrichton created PR review comment:

Could the expect here be propagated to the caller? Or otherwise could this have a # Panics section of the documentation and be renamed to unwrap_pending_exception?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 16:29):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 16:29):

alexcrichton created PR review comment:

Would it be possible to have TrapReason::Exception directly instead of, even at the lowest layers, encoding through the indirection of anyhow::Error?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 16:29):

alexcrichton created PR review comment:

Should this debug_assert! the previous exception was none?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 16:29):

alexcrichton created PR review comment:

Could unsafe from these two functions be dropped?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 16:29):

alexcrichton created PR review comment:

If it's not too difficult to extract, it might be nice to land the refactoring of how exceptions are allocated/managed separately from this PR which has the other runtime/compiler bits.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 16:29):

alexcrichton created PR review comment:

Since self.unwind must be set by this function, could this be modeled as:

let state = match reason {
    // ...
};
self.unwind.set(state);

to have compiler checks that every arm sets it?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 16:29):

alexcrichton created PR review comment:

When updating this, mind updating the comment of the pseudo-struct at the top of this file as well?

Orthogonally, though, I'm a bit concerned about this because it looks like it's basically redundant information in the VMContext where we have one field for the fp of the exit trampoline and one field for the fp of the frame before the exit trampoline. That seems like it'd be very easy to confuse the two. Could the previous field be removed to only have the fp of the exit trampoline which can be used to infer the fp of the previous frame of the trampoline?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 16:29):

alexcrichton created PR review comment:

Could this be updated to disable wasm_gc and wasm_function_references, or not explictily enable them, since they shouldn't be required?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 16:29):

alexcrichton created PR review comment:

Instead of implementing these on a public type, could these be implemented on a private internal type? That way we don't have to worry about these details leaking through into the public API

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 16:29):

alexcrichton created PR review comment:

Mind adding # Safety documentation to the function, as well as // SAFETY: ... docs to the unsafe block?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 16:29):

alexcrichton created PR review comment:

I think this is copy/pasted from vm/instance.rs, could this be encapsulated over there to avoid having the duplication so far from the original source?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 17:00):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 17:00):

cfallin created PR review comment:

The thing is that these are necessary for the public API -- the way that a Func throws an exception to its caller is to return an Err with a boxed Rooted<ExnRef>, so we need to implement Error (and its required trait Display in turn).

Perhaps we could wrap this further, e.g. wrap Rooted<ExnRef> in an Exception type. The RFC describes this as option 4 for the host API -- I had figured that the exnref itself serves that purpose, but we could give it a nicer type for sure. Thoughts?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 17:06):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 17:06):

cfallin created PR review comment:

Woah, I'm surprised my tests didn't catch that -- it will leave the no-GC state set, even, keeping the store in a broken state. (I suppose not so surprised, with only short test cases.) Thanks for catching this!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 17:39):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 17:39):

alexcrichton created PR review comment:

Structurally something about this is going to need to be adjusted, but I'm not sure how best that would be done. My understanding of this is that anything rooted in catch_unwind_and_record_trap effectively needs to have this logic applied. The code here works for Func::new and Linker::func_new, but there's a lot of other locations that this isn't applied to just yet:

Overall our story for entering/exiting wasm is pretty messy unfortunately. For example one could argue that CallHook::{CallingHost,ReturningFromHost} should be invoked for libcalls but they aren't. Otherwise handling that is already duplicated across components and core wasm.

Personally I think the best option is going to be to hook into catch_unwind_and_record_trap directly since that's the "narrow waist" for everything anyway. IIRC though you were saying how that's not feasible today since it doesn't have a store nor can easily get a store. I think I see how to do this, so I'll work on a separate PR to thread a store into catch_unwind_and_record_trap for this PR to hook into.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 17:49):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 17:51):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 17:51):

cfallin created PR review comment:

Great point re: the redundancy -- I was hesitant at first to remove the original Wasm-frame exit FP field, but now I've done that in 3f542bb08e. As a bonus, trampolines are even shorter (no load-from-FP-to-get-Wasm-FP).

Re: comment -- the pseudo-struct comment doesn't describe these fields, only one field for VMStoreContext. I did grep for last_wasm_exit_fp and try to ensure I updated anything relevant...

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 17:56):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 17:56):

cfallin created PR review comment:

I tried to bottom out all of the ways that host functions could be provided and it did seem to all lead to HostFunc -- perhaps I missed somewhere. This is unfortunately the best we can do though unless we do have a store in catch_unwind_and_record_trap, as far as I can tell. I agree the whole situation is kind of horrific right now...

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 18:19):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 18:19):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 18:19):

cfallin created PR review comment:

Updated!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 18:19):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 18:19):

cfallin created PR review comment:

Updated!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 18:21):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 18:21):

cfallin created PR review comment:

Done!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 18:21):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 18:21):

alexcrichton created PR review comment:

IMO this required duplication if we don't have a store when handling traps is motivation to push harder on getting a store available while handling traps. I've done this refactoring https://github.com/bytecodealliance/wasmtime/pull/11441 which I believe means that this code here can be scoped to HostResult implementations which can now easily access the store.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 18:55):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 22:26):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 22:29):

cfallin commented on PR #11326:

I've refactored on top of #11441 and this is indeed much nicer; I've removed the From impls that would otherwise pass through boxed exceptions from hostcalls without converting to a pending exception, so I'm more confident we won't miss any additional hostcall paths now.

The current challenge is dealing with rooting: the factor has placed the point at which we capture the Rooted<ExnRef> (refactor to wrap that up in a wasmtime::Exception TBD) outside the root handle scope for the hostcall, so any newly allocated exception from the host becomes unrooted. Pulling this string further may require additional refactors to put the LIFO-scope management in common across all hostcalls; just starting to look into this. More thoughts (or a pairing session if that'd be easier) welcome, @alexcrichton...

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 22:29):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 22:29):

cfallin created PR review comment:

Resolved in refactor (modulo rooting issues below)!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 22:30):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 22:30):

cfallin created PR review comment:

Done!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 22:30):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 22:30):

cfallin created PR review comment:

Fixed!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 22:30):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 22:30):

cfallin created PR review comment:

Yep, this makes more sense -- removed now in refactor.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 22:30):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 22:30):

cfallin created PR review comment:

Done!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 22:30):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 22:30):

cfallin created PR review comment:

Done!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 22:49):

alexcrichton commented on PR #11326:

Oh good point! I think it'd be reasonable to move this management into the trap-handling bits (or around the enter_host_from_wasm bits). That future-proofs this for component-model-gc support and additionally enables eventually cleaning up libcalls that currently create their own scopes as they no longer need to

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 22:51):

cfallin commented on PR #11326:

Actually, I think there are deeper issues with rooting, exceptions, and Result / ? ergonomics that we didn't really address in the RFC, and I'm finding I'm bumping into these design questions now. The current state of this PR has working-but-for-unrooted-ref-panics in cargo test --test all -- exceptions, demonstrating a few examples of the issue.

The basic question is: if we return an exception as a GC object boxed in a Rooted<ExnRef> (eventually inside a nicely-packaged wasmtime::Exception type) in the Err arm of a Result, how should we expect this to interact nicely with handle scopes?

The most basic example is a host function that allocates an ExnRef and returns Err(exnref.into()), and has no other scopes of its own. We can fix up this PR by moving the root scope teardown outside of the point that we root a pending exception on the store, so that can work fine.

But in the general case, with user host code creating nested RootScopes, we cannot really expect automatic ? passing-up-the-Err to work well. Consider:

fn my_hostcall(...) -> Result<()> {
  let scope = RootScope::new(&mut caller);
  helper(&mut scope)?;
  Ok(())
}

fn helper(ctx: impl AsContextMut<'_>, ...) -> Result<()> {
  return Err(ExnRef::new(...).into());
}

This will typecheck fine (ExnRef::new() returns a Rooted<ExnRef>, we currently take that as an Error impl directly, we can wrap it up in wasmtime::Exception once I do that); but the exception object will be unrooted once the ? propagates it past the user's own RootScope.

More generally, Err-carried types should generally expect to own their error information. Our lack of lifetimes to denote valid scope is an explicit ergonomic choice, which is great in general, but hides the fact that we will get a dynamic scope-violation (unrooting) error here.

Should we use a ManuallyRooted instead? I think that's much worse -- it's way too easy to leak, since its Drop impl doesn't unroot (because it can't, because it doesn't have a mut borrow of the Store -- again a totally valid choice that is nice in 99% of cases).

It's somewhat tempting to say "don't do that" to all of this -- it's no different than returning any GC ref type at all. If we take this option, we just fix the exact extent of the root scope when calling out to the host, we root any returned exception when it comes back (concretely: put the HostResult conversion inside the root scope somehow, probably by pulling the scope out to catch_and_record_traps). My broader concern here is that I'm not sure that it has the nice error-propagation properties that we expected. Basically, it's a nonlocal/non-composable footgun: anyone creating a scope anywhere inside complex host logic could cause an exception created on deep to blow up dynamically with a panic.

One alternative is to have an explicit store.throw(exn) as part of the public API, and then provide a public-facing tombstone type that the user can propagate upward through Results. This does make things a little weird in other ways, because it's implicit state (we could have an API around "cancel pending exception" too if someone wants to "catch" it in host code), but it feels the least footgunny in a lot of others. Basically it's extending what we decided to do internally (root the exception on the store, safely) to user code.

The last option does differ from what we agreed to in the RFC (but the RFC is underspecified with respect to rooting behavior in general, on the other hand) -- so I'd want some consensus here, at least, before building that. Any thoughts?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 22:52):

cfallin edited a comment on PR #11326:

Actually, I think there are deeper issues with rooting, exceptions, and Result / ? ergonomics that we didn't really address in the RFC, and I'm finding I'm bumping into these design questions now. The current state of this PR has working-but-for-unrooted-ref-panics examples of all directions of exception transfer in cargo test --test all -- exceptions, demonstrating a few examples of the issue.

The basic question is: if we return an exception as a GC object boxed in a Rooted<ExnRef> (eventually inside a nicely-packaged wasmtime::Exception type) in the Err arm of a Result, how should we expect this to interact with handle scopes?

The most basic example is a host function that allocates an ExnRef and returns Err(exnref.into()), and has no other scopes of its own. We can fix up this PR by moving the root scope teardown outside of the point that we root a pending exception on the store, so that can work fine.

But in the general case, with user host code creating nested RootScopes, we cannot really expect automatic ? passing-up-the-Err to work well. Consider:

fn my_hostcall(...) -> Result<()> {
  let scope = RootScope::new(&mut caller);
  helper(&mut scope)?;
  Ok(())
}

fn helper(ctx: impl AsContextMut<'_>, ...) -> Result<()> {
  return Err(ExnRef::new(...).into());
}

This will typecheck fine (ExnRef::new() returns a Rooted<ExnRef>, we currently take that as an Error impl directly, we can wrap it up in wasmtime::Exception once I do that); but the exception object will be unrooted once the ? propagates it past the user's own RootScope.

More generally, Err-carried types should generally expect to own their error information. Our lack of lifetimes to denote valid scope is an explicit ergonomic choice, which is great in general, but hides the fact that we will get a dynamic scope-violation (unrooting) error here.

Should we use a ManuallyRooted instead? I think that's much worse -- it's way too easy to leak, since its Drop impl doesn't unroot (because it can't, because it doesn't have a mut borrow of the Store -- again a totally valid choice that is nice in 99% of cases).

It's somewhat tempting to say "don't do that" to all of this -- it's no different than returning any GC ref type at all. If we take this option, we just fix the exact extent of the root scope when calling out to the host, we root any returned exception when it comes back (concretely: put the HostResult conversion inside the root scope somehow, probably by pulling the scope out to catch_and_record_traps). My broader concern here is that I'm not sure that it has the nice error-propagation properties that we expected. Basically, it's a nonlocal/non-composable footgun: anyone creating a scope anywhere inside complex host logic could cause an exception created on deep to blow up dynamically with a panic.

One alternative is to have an explicit store.throw(exn) as part of the public API, and then provide a public-facing tombstone type that the user can propagate upward through Results. This does make things a little weird in other ways, because it's implicit state (we could have an API around "cancel pending exception" too if someone wants to "catch" it in host code), but it feels the least footgunny in a lot of others. Basically it's extending what we decided to do internally (root the exception on the store, safely) to user code.

The last option does differ from what we agreed to in the RFC (but the RFC is underspecified with respect to rooting behavior in general, on the other hand) -- so I'd want some consensus here, at least, before building that. Any thoughts?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2025 at 23:48):

alexcrichton commented on PR #11326:

Good points!

Basically, it's a nonlocal/non-composable footgun

To some degree this is inherently true of Rooted and ManuallyRooted. As you point out there's big downsides to using both. That being said I do agree this is particularly exacerbated due to the nature of "just propagate the error upwards please" which makes it more likely an exception might fly all the way up past whatever scopes are in use.

One alternative is to have an explicit store.throw(exn) as part of the public API

I like this approach personally. What I might recommend is something like StoreContextMut::throw(&mut self, &Rooted<ExnRef>) -> wasmtime::ThrownException so that way the user doesn't have to create the tombstone themselves and can immediately return that error upwards the stack.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 16 2025 at 01:42):

cfallin commented on PR #11326:

wasmtime::ThrownException so that way the user doesn't have to create the tombstone themselves and can immediately return that error upwards the stack.

Indeed, that was my thought as well. I suppose one could actually also have a signature throw(...) -> Result<Uninhabited, ThrownException> so that one could do store.throw(exn)?; -- or both, thrown_exception(...) -> ThrownException and throw(...) -- but at this point I'm bikeshedding a bit. I'll prototype this to see how it feels.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 18 2025 at 15:22):

alexcrichton commented on PR #11326:

I'd recommend choosing between:

where the latter uses a generic T instead of Uninhabited so that way it can be coerced to any possible type without casts/etc. I'd be on the fence myself about these two signatures, I agree that using ? would be nice. One possible compromise could be a Result-based throw for convenience and a method on ExnRef for creating SomeExceptionType, so that way we have both and whatever's most appropriate can be used?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 06:56):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 07:00):

cfallin commented on PR #11326:

Alright, I've built this out -- the API on Store and friends now has a throw as described, and in addition to the lower-level {take,set,has}_pending_exception(), also has a catch helper so one can do something like

store.catch(|store| {
    let exn = ...;
    store.throw(exn)?;
    Ok(())
  },
  |store, exn| {
    Err("caught an exception".into())
  })?;

or similar.


This just needs

then I'll flip off the "draft" flag.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 07:09):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 17:15):

cfallin created PR review comment:

Resolved in latest refactor.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 17:15):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 17:15):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 17:15):

cfallin created PR review comment:

Done!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 17:24):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 17:25):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 17:25):

cfallin created PR review comment:

Done in 45415e4886!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 18:08):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 18:17):

cfallin commented on PR #11326:

New final boss appears: Pulley support will, I think, require refactoring how setjmp/longjmp is emulated. Right now Pulley special-cases the raise() libcall and does an interpreter-register-level longjmp across all Wasm frames.

I believe what we'll ideally want to do instead, to avoid too much duplication in an alternative raise() impl, is to actually invoke the real implementation but then indirect the longjmp through a match on the store's executor: either a real longjmp to the real jmp_buf in the CallThreadState, or a mutation of the interpreter state and a return. Then similarly wasmtime_unwinder::resume_to_exception_handler can have another "host" backend that takes the store, fetches the interpreter state, and updates it.

The alternative is to try to get access to the store inside the Pulley CallIndirectHost impl and determine its UnwindState, but that seems to pull in too much across abstraction boundaries -- for example, it requires Pulley to take the exception payload off the store's root (i.e., have access to the full Store, not just some slice of it).

I'm tempted to try to land this first and defer Pulley support to a separate PR, with some help (probably from @alexcrichton) to determine the best path here -- it's somewhat close (the stackwalk works fine!) but definitely not trivial.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 18:18):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 18:22):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 18:22):

cfallin edited a comment on PR #11326:

Alright, I've built this out -- the API on Store and friends now has a throw as described, and in addition to the lower-level {take,set,has}_pending_exception(), also has a catch helper so one can do something like

store.catch(|store| {
    let exn = ...;
    store.throw(exn)?;
    Ok(())
  },
  |store, exn| {
    Err("caught an exception".into())
  })?;

or similar.


This just needs

then I'll flip off the "draft" flag.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 18:22):

cfallin edited a comment on PR #11326:

Alright, I've built this out -- the API on Store and friends now has a throw as described, and in addition to the lower-level {take,set,has}_pending_exception(), also has a catch helper so one can do something like

store.catch(|store| {
    let exn = ...;
    store.throw(exn)?;
    Ok(())
  },
  |store, exn| {
    Err("caught an exception".into())
  })?;

or similar.


This just needs

then I'll flip off the "draft" flag.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 20:16):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 20:26):

fitzgen commented on PR #11326:

I'm tempted to try to land this first and defer Pulley support to a separate PR, with some help (probably from @alexcrichton) to determine the best path here -- it's somewhat close (the stackwalk works fine!) but definitely not trivial.

SGTM, we have a history of landing support for new Wasm proposals a backend at a time (e.g. tail calls, ref types) and as long as there isn't a fundamental reason an implementation approach can't be extended to the unsupported backends that would require redoing the supported backends, then I don't see any reason for disallowing incremental PRs.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 20:35):

alexcrichton commented on PR #11326:

Agreed yeah definitely feel free to defer Pulley, I'm happy to dive in afterwards and/or have a call to discuss how best to get it working

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 21:30):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 21:49):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 21:58):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 22:01):

cfallin commented on PR #11326:

The full test suite is passing now!

There's quite a lot of rebase churn with the ongoing refactors on main -- to avoid too much rebase pain I'm going to re-squash down to one commit then rebase that as a unit (rather than fixing conflicts at each of N steps). Then I'll see how easy it is to pull out the refactor to exception object layouts (to have one tag slot and share more with structs) -- no guarantees on that. Either way, will mark ready once it's static.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 22:01):

cfallin edited a comment on PR #11326:

Alright, I've built this out -- the API on Store and friends now has a throw as described, and in addition to the lower-level {take,set,has}_pending_exception(), also has a catch helper so one can do something like

store.catch(|store| {
    let exn = ...;
    store.throw(exn)?;
    Ok(())
  },
  |store, exn| {
    Err("caught an exception".into())
  })?;

or similar.


This just needs

then I'll flip off the "draft" flag.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 22:01):

cfallin edited a comment on PR #11326:

Alright, I've built this out -- the API on Store and friends now has a throw as described, and in addition to the lower-level {take,set,has}_pending_exception(), also has a catch helper so one can do something like

store.catch(|store| {
    let exn = ...;
    store.throw(exn)?;
    Ok(())
  },
  |store, exn| {
    Err("caught an exception".into())
  })?;

or similar.


This just needs

then I'll flip off the "draft" flag.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 22:07):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 22:26):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 22:28):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 22:28):

cfallin created PR review comment:

Done (#11467)!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 22:30):

cfallin updated PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 22:31):

cfallin has marked PR #11326 as ready for review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 22:31):

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

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 22:31):

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

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 22:31):

cfallin requested wasmtime-core-reviewers for a review on PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 22:31):

cfallin requested wasmtime-compiler-reviewers for a review on PR #11326.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2025 at 22:31):

cfallin requested alexcrichton for a review on PR #11326.


Last updated: Jan 10 2026 at 02:36 UTC