alexcrichton opened PR #14011 from alexcrichton:remove-exn-type to bytecodealliance:main:
This commit is a refactoring of the implementation of exceptions within Wasmtime to remove the concept of a concrete exception type from the hierarchy of types Wasmtime supports for guests. WebAssembly itself has no concept of concrete exception types meaning that Wasmtime's public API, which typically follows the WebAssembly specification, is misaligned in this respect. Specifically:
- The
wasmtime::ExnTypevalue has no correspondance to a construct in WebAssembly itself. Exceptions have a tag and payload values, but they do not have a nameable type.- The
wasmtime::HeapTypeenumcontains aConcreteExnvariant which does not exist in WebAssembly.- The
ExnRef::newconstructor takes a type viaExnRefPre, aTag, and field values. Taking both a type and aTagis a mismatch with wasm runtime semantics where an exception can be constructed with just a tag and field values.These properties have led to a good deal of implementation throughout Wasmtime's surface API itself, in addition to downstream APIs like the C and C++ APIs. Additionally as something that's not actually expressible in WebAssembly it means that these paths are not well tested or fuzzed, meaning that mistakes can leak through. Two known mistakes today are:
- When creating a host-defined
Tagthe internalExnTypeis a stale index that is not properly rooted in aStoreorEngine. This ends up not causing any issues, however, since the field isn't ever read.- The
TypeTrace for WasmHeapTypeimplementation did not canonicalize theCanoncreteExnbranch of the type. This was never expressible in WebAssembly, however, which meant that it ended up being benign at runtime.Overall it seemed subjectively to me like it would be best to better align Wasmtime's public API with the WebAssembly type system in this respect, removing
ExnTypeand various APIs that reference it.Internally this necessitated some refactorings to avoid panics/unreachables when
ConcreteExnappeared, so I've opted to entirely remove theConcreteExnthroughout as well. This permeated into the GC implementation where the layout for exceptions is now a struct with two leading fields as opposed to an object with a bigger header followed by a struct layout. Runtime-wise exceptions are still allocated in the same layout as before, only now the instance/tag fields are part of the struct layout as opposed to being implicitly part of the header computation. Internal methods were renamed to be more precise about what's being operated on.<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
alexcrichton requested fitzgen for a review on PR #14011.
alexcrichton requested wasmtime-compiler-reviewers for a review on PR #14011.
alexcrichton requested wasmtime-fuzz-reviewers for a review on PR #14011.
alexcrichton requested wasmtime-core-reviewers for a review on PR #14011.
alexcrichton updated PR #14011.
cfallin commented on PR #14011:
I'll add some historical context: in design discussions back when this was implemented, we explicitly chose to make "exception of tag T" a distinct type, and not a thin wrapper over a struct, because:
Exception unwinding needs to read out the tag from an exception object at runtime, and should not look up runtime type info to do so. This is why exceptions are a distinct kind of layout: they have a fixed header word, like arrays do (and unlike structs). In contrast, having a struct layout that lines up "just so" to make constant-offset accesses possible at runtime without knowledge of the exception type is an extremely dangerous loaded footgun IMHO: any change to layout code now needs to keep in mind that other parts of the runtime have certain expectations about how a certain struct is laid out. And this code is distinct per GC engine, so every GC engine needs to know about this.
A function signature and an exception object have different representations. And a tag is a thin newtype around a function signature, so it has no distinct identity in the interned type arena. The Wasm spec in essence has "too much implicitness" here: a functy can be used to refer to a signature, or can be used to refer to an exception layout. Reifying exception types as a distinct kind of type lets us make the distinction really clear -- otherwise, we need to always have the implicit context "am I using this signature type index as an exception object or a function"? There are more loaded footguns here.
- We could of course inflate the Tag to more than a thin newtype, but this is just
ExnTypein new clothes, I think. There fundamentally need to be two different interned types.The host API does need something to represent "an exception of this type" for (i) fast allocation, and (ii) fast access to fields. Say that we just have a tag index as above: we might want a "fast exception allocator" but isn't that just an
ExnRefPreas we have today? And isn't "exception-compatible tag" (no results just params) a valuable concept to encode at the type level, so even if we folded the internals together, we'd still wantExnTypeas a newtype around tags that is only constructable for compatible tags?Basically: there were good reasons we did this; risks abound if we collapse the distinction; there have been a few bugs in the current approach but they can be easily fixed; I don't see sufficient data to relegislate the decision. Curious what @fitzgen thinks of course too.
alexcrichton commented on PR #14011:
I understand the knee-jerk reaction here, but I've done my best to not just barge in and change everything. To your points:
Exception unwinding needs to read out the tag from an exception object at runtime, and should not look up runtime type info to do so.
This is explicitly perserved. I consider it a bug as well that acquiring a layout requires runtime engine locks right now. That makes GC tracing that much slower. The
Storeshould have quick and easy access to runtime layouts for tracing, and if it did so then the means by which I've preserved this would be even simpler.The Wasm spec in essence has "too much implicitness" here: a functy can be used to refer to a signature, or can be used to refer to an exception layout.
I don't think this is true, though. A function type alone cannot be used to refer to an exception layout, but rather it needs to be explicitly wrapped up in a tag, and that's the delination for "here's the exception stuff" vs not. Regardless I don't think we should be in the business of providing an opinionated view of wasm's type system, to me Wasmtime should reflect wasm as-is.
A function signature and an exception object have different representations.
This remains so, a
Tagis distinct from aTagTypewhich is distinct from aFuncType. Do you have an example of something in this PR you'd like to change?The host API does need something to represent "an exception of this type" for (i) fast allocation, and (ii) fast access to fields.
This is also explicitly preserved. There's still
ExnRefPre, and we've never had fast access to fields on any GC type from the embedder because it always requires looking up the layout, and that behavior is additionally preserved. Like above I think this is something we should fix, but is orthogonal to the wasm type system mismatch here.Basically: there were good reasons we did this; risks abound if we collapse the distinction; there have been a few bugs in the current approach but they can be easily fixed; I don't see sufficient data to relegislate the decision.
Do you have specific areas of this PR that you would not like to see land in Wasmtime, or would like to change? We routinely refactor core implementation details in response to implementation feedback, and to me the implementation feedback here is "the public API should match WebAssembly, not Wasmtime's own variant", and pushing that change back into the runtime results in this PR.
Put another way, do you disagree with the premise I have that Wasmtime's public API should reflect WebAssembly's type system? For example I don't think
HeapType::ConcreteExnshould exist because it doesn't exist in WebAssembly.
cfallin commented on PR #14011:
Do you have specific areas of this PR that you would not like to see land in Wasmtime, or would like to change? We routinely refactor core implementation details in response to implementation feedback, and to me the implementation feedback here is "the public API should match WebAssembly, not Wasmtime's own variant", and pushing that change back into the runtime results in this PR.
Sure, I think the main thing I don't like about it is that it creates a distributed and ad-hoc invariant between the struct layout code and the runtime: it expects that the tag reference fields (defining instance and defined-tag-index) come first in the layout. I appreciate that you added asserts here but it's still an example of how collapsing the concepts causes more entangled code than we otherwise would have.
I don't oppose refactoring as a concept (of course not!), we just spent a lot of time on this design decision and considered it carefully, and I still see risks here that I'm naming explicitly.
I don't think we should be in the business of providing an opinionated view of wasm's type system, to me Wasmtime should reflect wasm as-is.
For what it's worth I don't see the current design as "opinionated" or something like that: it names an implementation corner that already must exist implicitly (all exceptions with this layout). The Wasm surface language and semantics don't require that, so the spec doesn't have it, but a good implementation certainly does. Even this PR has it, just as a side-table with implicit invariants.
github-actions[bot] added the label wasmtime:c-api on PR #14011.
github-actions[bot] added the label fuzzing on PR #14011.
github-actions[bot] commented on PR #14011:
Subscribe to Label Action
cc @fitzgen
<details>
This issue or pull request has been labeled: "fuzzing", "wasmtime:c-api"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 commented on PR #14011:
Put another way, do you disagree with the premise I have that Wasmtime's public API should reflect WebAssembly's type system? For example I don't think
HeapType::ConcreteExnshould exist because it doesn't exist in WebAssembly.I agree that all else being equal we should match WebAssembly's spec/type system. But all else is not always equal:
InstancePredoesn't exist in the spec, yet is a useful thing to have in the API/implementation- I think that
ExnTypeandExnRefPreare also useful for similar reasonsThat said, I'm not sure that we need to or should have
HeapType::ConcreteExn.Sure, I think the main thing I don't like about it is that it creates a distributed and ad-hoc invariant between the struct layout code and the runtime: it expects that the tag reference fields (defining instance and defined-tag-index) come first in the layout. I appreciate that you added asserts here but it's still an example of how collapsing the concepts causes more entangled code than we otherwise would have.
Big +1 to this.
alexcrichton commented on PR #14011:
Personally I don't feel that this is any more ad-hoc than it was before. Dealing with exnref is already a very large copy/paste of all handling of
structs more-or-less which has itself been the source of historical bugs (updating one and forgetting to update the other). Already today there'sis_exception: boolon aGcStructLayout. Methods are already calledalloc_uninit_struct_or_exn, etc. I also don't understand why we would want to add an entirely separate implementation of GC primitives just for exceptions, only for it to bottom out in what is a glorifiedstruct. Personally I feel it's more valuable to reuse the primitives of the implementation as opposed to making a new primitive each time.Another way to put it is what you're saying is ad-hoc I personally feel is a cleaner separation of concerns. The GC deals with structs and arrays and doesn't have to have special cases for exception layouts. Exceptions are then built on top of this.
For
ConcreteExn, though, I really do think that if Wasmtime keeps it then it's surfacing an opinionated view of the type system. We're inventing something that's not mentioned in the wasm specification and it forces a design of an embedder API which any other embedder is unlikely to ever match (e.g. if the wasm C API ever grew support for exceptions I would be surprised ifExnTypehad any sort of equivalent there). I disagree with the parallel ofInstancePrebecause that's purely about performance and has a clear mapping to what happens in the specification, being mid-way through an algorithm. ForConcreteExnand updatingHeapTypeitself that's totally different because it's entirely changing the AST of the the type hierarchy itself.I don't have the energy to push this uphill, however, so I'm going to close this. I don't agree that what we have in-tree is the best implementation, but it looks like changing it will require more energy than I have.
:cross_mark: alexcrichton closed without merge PR #14011.
cfallin commented on PR #14011:
Thanks for the perspective and sorry that it still doesn't feel right to you! I don't want to prolong the discussion necessarily but just to jot down for any future discussion on this:
- I think it's important to distinguish between
ConcreteExn(midpoint in the lattice we really do name, for implementation reasons) andExnType; the above seems to conflate them in places as part of the same issue.ExnTypeone could also think of as another name for, say,TagType, or "FuncTypewith validated-and-prepared-for-allocating-an-exception flavor"; that last bit should make clear that one needs something like this in the API if one wants to allow the user to amortize this work across multiple instantiations, just asInstancePredoes for instantiation. In other words, it's essential, not incidental, and any future refactors I think will need to keep it unless we regress performance on use-cases like this.- I do very much appreciate the "build on top of a small core calculus" approach, and I guess for a historical note, did try to build exceptions as "just structs" initially. This PR is certainly a proof-point it can still be done without the "runtime type lookup" hazard I hit originally, but there's still the expected-layout hazard and, as a corollary to that, a whole line of optimizations that the conflation rules out (field reordering for smaller objects, like the JVM does; struct-of-arrays transform on certain classes; any kind of variable-width header for some kind of fine-grained marking or metadata; ...). So, at some point in the future if we reconsider this, we should affirmatively decide that we're willing to push the complexity into GC to have "fixed-offset-for-all-types fields" in structs instead that all engines' layout algorithms understand.
Last updated: Aug 30 2026 at 09:07 UTC