Wasmtime GitHub notifications bot (Dec 12 2025 at 18:17): Wasmtime GitHub notifications bot (Dec 12 2025 at 18:17):fitzgen opened PR #12163 from fitzgen:wasmtime-error to bytecodealliance:main:
This new
Errorhas an API that is 99% identical toanyhow::Error's API, but additionally handles memory exhaustion.This commit only introduces the
wasmtime_internal_errorcrate into our workspace, along with its regular tests and OOM tests. This commit does not, however, migrate Wasmtime's internals or public-facing API over to the new error type yet. That is left for follow up work.In order to continue fitting
Result<(), Error>in one word, there is quite a bit of unsafe code inError's implementation, mostly surrounding the manual creation of our own moral equivalent ofBox<dyn Error>with explicit vtables and type erasure so that we get a thin pointer to a trait object rather thanBox<dyn Error>'s fat pointer. To alleviate the associated risks, I've been testing this code under MIRI throughout its whole development, as well as thoroughly testing the API so that MIRI can dynamically exercise all the code paths. Furthermore, I've enabled testing this crate under MIRI in CI.Part of https://github.com/bytecodealliance/wasmtime/issues/12069
<!--
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
-->
Wasmtime GitHub notifications bot (Dec 12 2025 at 18:17):fitzgen requested alexcrichton for a review on PR #12163.
Wasmtime GitHub notifications bot (Dec 12 2025 at 18:17):fitzgen requested wasmtime-fuzz-reviewers for a review on PR #12163.
Wasmtime GitHub notifications bot (Dec 12 2025 at 18:17):fitzgen requested wasmtime-core-reviewers for a review on PR #12163.
Wasmtime GitHub notifications bot (Dec 12 2025 at 18:17):fitzgen requested wasmtime-default-reviewers for a review on PR #12163.
Wasmtime GitHub notifications bot (Dec 12 2025 at 18:31):fitzgen updated PR #12163.
Wasmtime GitHub notifications bot (Dec 12 2025 at 18:33):fitzgen updated PR #12163.
Wasmtime GitHub notifications bot (Dec 12 2025 at 18:41):fitzgen updated PR #12163.
Wasmtime GitHub notifications bot (Dec 12 2025 at 18:47):fitzgen updated PR #12163.
Wasmtime GitHub notifications bot (Dec 12 2025 at 19:00):fitzgen commented on PR #12163:
I am not sure why the version bump followed by
cargo vetis failing. Digging in...https://github.com/bytecodealliance/wasmtime/actions/runs/20176633165/job/57926334414?pr=12163
Wasmtime GitHub notifications bot (Dec 12 2025 at 19:53):fitzgen commented on PR #12163:
I am not sure why the version bump followed by
cargo vetis failing. Digging in...Third bullet point over in https://github.com/bytecodealliance/wasmtime/pull/12164
Wasmtime GitHub notifications bot (Dec 12 2025 at 19:56):fitzgen updated PR #12163.
Wasmtime GitHub notifications bot (Dec 12 2025 at 19:59):fitzgen updated PR #12163.
Wasmtime GitHub notifications bot (Dec 12 2025 at 21:29):fitzgen updated PR #12163.
Wasmtime GitHub notifications bot (Dec 12 2025 at 21:30):fitzgen commented on PR #12163:
Third bullet point over in #12164
Rebased on top of that PR now that it merged
Wasmtime GitHub notifications bot (Dec 12 2025 at 21:42):fitzgen updated PR #12163.
Wasmtime GitHub notifications bot (Dec 12 2025 at 21:51):fitzgen commented on PR #12163:
Grrrr this bump-and-vet CI failure is really annoying me. Works when I run the exact same commands locally:
<details>
nick@hamilton :: (wasmtime-error) :: ~/wasmtime $ rustc scripts/publish.rs && ./publish bump-patch bumping `component-macro-test-helpers`... bumping `component-async-tests`... bumping `wizer-fuzz`... bumping `regex-test`... bumping `regex-bench`... bumping `uap-bench`... bumping `wasmtime-environ-fuzz`... bumping `wasmtime-bench-api`... bumping `wasi-preview1-component-adapter`... bumping `verify-component-adapter`... bumping `byte-array-literals`... bumping `wasi-nn-example-winml`... bumping `wasi-nn-example-named`... bumping `wasi-nn-example`... bumping `classification-component-onnx`... bumping `wasi-nn-example-pytorch`... bumping `wasmtime-test-macros`... bumping `wiggle-test`... bumping `test-programs`... bumping `test-programs-artifacts`... bumping `wasmtime-test-util`... bumping `wasmtime-fuzzing`... bumping `wasm-spec-interpreter`... bumping `wasmtime-c-api`... bumping `cranelift-tools`... bumping `cranelift-assembler-x64-fuzz`... bumping `cranelift-fuzzgen`... bumping `cranelift-filetests`... bumping `isle-fuzz`... bumping `veri_ir`... bumping `veri_engine`... bumping `islec`... bumping `pulley-interpreter-fuzz`... bumping `cranelift-bitset`... 0.128.0 => 0.128.1 bumping `wasmtime-internal-math`... bumping `pulley-macros`... bumping `pulley-interpreter`... bumping `cranelift-srcgen`... 0.128.0 => 0.128.1 bumping `cranelift-assembler-x64-meta`... 0.128.0 => 0.128.1 bumping `cranelift-assembler-x64`... 0.128.0 => 0.128.1 bumping `cranelift-isle`... 0.128.0 => 0.128.1 bumping `cranelift-entity`... 0.128.0 => 0.128.1 bumping `cranelift-bforest`... 0.128.0 => 0.128.1 bumping `cranelift-codegen-shared`... 0.128.0 => 0.128.1 bumping `cranelift-codegen-meta`... 0.128.0 => 0.128.1 bumping `cranelift-control`... 0.128.0 => 0.128.1 bumping `cranelift-codegen`... 0.128.0 => 0.128.1 bumping `cranelift-reader`... 0.128.0 => 0.128.1 bumping `cranelift-serde`... 0.128.0 => 0.128.1 bumping `cranelift-module`... 0.128.0 => 0.128.1 bumping `cranelift-frontend`... 0.128.0 => 0.128.1 bumping `cranelift-native`... 0.128.0 => 0.128.1 bumping `cranelift-object`... 0.128.0 => 0.128.1 bumping `cranelift-interpreter`... 0.128.0 => 0.128.1 bumping `wasmtime-internal-jit-icache-coherence`... bumping `wasmtime-internal-unwinder`... bumping `cranelift-jit`... 0.128.0 => 0.128.1 bumping `cranelift`... 0.128.0 => 0.128.1 bumping `wiggle-generate`... bumping `wiggle-macro`... bumping `wasmtime-internal-error`... bumping `wasmtime-internal-versioned-export-macros`... bumping `wasmtime-internal-slab`... bumping `wasmtime-internal-component-util`... bumping `wasmtime-internal-wit-bindgen`... bumping `wasmtime-internal-component-macro`... bumping `wasmtime-internal-jit-debug`... bumping `wasmtime-internal-fiber`... bumping `wasmtime-environ`... bumping `wasmtime-internal-wmemcheck`... bumping `wasmtime-internal-cranelift`... bumping `wasmtime-internal-cache`... bumping `winch-codegen`... bumping `wasmtime-internal-winch`... bumping `wasmtime`... bumping `wiggle`... bumping `wasi-common`... bumping `wasmtime-wasi-io`... bumping `wasmtime-wasi`... bumping `wasmtime-wasi-http`... bumping `wasmtime-wasi-nn`... bumping `wasmtime-wasi-config`... bumping `wasmtime-wasi-keyvalue`... bumping `wasmtime-wasi-threads`... bumping `wasmtime-wasi-tls`... bumping `wasmtime-wasi-tls-nativetls`... bumping `wasmtime-wast`... bumping `wasmtime-internal-c-api-macros`... bumping `wasmtime-c-api-impl`... bumping `wasmtime-wizer`... bumping `wasmtime-cli-flags`... bumping `wasmtime-internal-explorer`... bumping `wasmtime-cli`... 41.0.0 => 41.0.1 Running: `"cargo" "fetch" "--offline"` nick@hamilton :: (wasmtime-error *) :: ~/wasmtime $ echo $? 0</details>
Wasmtime GitHub notifications bot (Dec 12 2025 at 21:54):fitzgen edited a comment on PR #12163:
Grrrr this bump-and-vet CI failure is really annoying me.
Works when I run the exact same commands locally:<details>
nick@hamilton :: (wasmtime-error) :: ~/wasmtime $ rustc scripts/publish.rs && ./publish bump-patch bumping `component-macro-test-helpers`... bumping `component-async-tests`... bumping `wizer-fuzz`... bumping `regex-test`... bumping `regex-bench`... bumping `uap-bench`... bumping `wasmtime-environ-fuzz`... bumping `wasmtime-bench-api`... bumping `wasi-preview1-component-adapter`... bumping `verify-component-adapter`... bumping `byte-array-literals`... bumping `wasi-nn-example-winml`... bumping `wasi-nn-example-named`... bumping `wasi-nn-example`... bumping `classification-component-onnx`... bumping `wasi-nn-example-pytorch`... bumping `wasmtime-test-macros`... bumping `wiggle-test`... bumping `test-programs`... bumping `test-programs-artifacts`... bumping `wasmtime-test-util`... bumping `wasmtime-fuzzing`... bumping `wasm-spec-interpreter`... bumping `wasmtime-c-api`... bumping `cranelift-tools`... bumping `cranelift-assembler-x64-fuzz`... bumping `cranelift-fuzzgen`... bumping `cranelift-filetests`... bumping `isle-fuzz`... bumping `veri_ir`... bumping `veri_engine`... bumping `islec`... bumping `pulley-interpreter-fuzz`... bumping `cranelift-bitset`... 0.128.0 => 0.128.1 bumping `wasmtime-internal-math`... bumping `pulley-macros`... bumping `pulley-interpreter`... bumping `cranelift-srcgen`... 0.128.0 => 0.128.1 bumping `cranelift-assembler-x64-meta`... 0.128.0 => 0.128.1 bumping `cranelift-assembler-x64`... 0.128.0 => 0.128.1 bumping `cranelift-isle`... 0.128.0 => 0.128.1 bumping `cranelift-entity`... 0.128.0 => 0.128.1 bumping `cranelift-bforest`... 0.128.0 => 0.128.1 bumping `cranelift-codegen-shared`... 0.128.0 => 0.128.1 bumping `cranelift-codegen-meta`... 0.128.0 => 0.128.1 bumping `cranelift-control`... 0.128.0 => 0.128.1 bumping `cranelift-codegen`... 0.128.0 => 0.128.1 bumping `cranelift-reader`... 0.128.0 => 0.128.1 bumping `cranelift-serde`... 0.128.0 => 0.128.1 bumping `cranelift-module`... 0.128.0 => 0.128.1 bumping `cranelift-frontend`... 0.128.0 => 0.128.1 bumping `cranelift-native`... 0.128.0 => 0.128.1 bumping `cranelift-object`... 0.128.0 => 0.128.1 bumping `cranelift-interpreter`... 0.128.0 => 0.128.1 bumping `wasmtime-internal-jit-icache-coherence`... bumping `wasmtime-internal-unwinder`... bumping `cranelift-jit`... 0.128.0 => 0.128.1 bumping `cranelift`... 0.128.0 => 0.128.1 bumping `wiggle-generate`... bumping `wiggle-macro`... bumping `wasmtime-internal-error`... bumping `wasmtime-internal-versioned-export-macros`... bumping `wasmtime-internal-slab`... bumping `wasmtime-internal-component-util`... bumping `wasmtime-internal-wit-bindgen`... bumping `wasmtime-internal-component-macro`... bumping `wasmtime-internal-jit-debug`... bumping `wasmtime-internal-fiber`... bumping `wasmtime-environ`... bumping `wasmtime-internal-wmemcheck`... bumping `wasmtime-internal-cranelift`... bumping `wasmtime-internal-cache`... bumping `winch-codegen`... bumping `wasmtime-internal-winch`... bumping `wasmtime`... bumping `wiggle`... bumping `wasi-common`... bumping `wasmtime-wasi-io`... bumping `wasmtime-wasi`... bumping `wasmtime-wasi-http`... bumping `wasmtime-wasi-nn`... bumping `wasmtime-wasi-config`... bumping `wasmtime-wasi-keyvalue`... bumping `wasmtime-wasi-threads`... bumping `wasmtime-wasi-tls`... bumping `wasmtime-wasi-tls-nativetls`... bumping `wasmtime-wast`... bumping `wasmtime-internal-c-api-macros`... bumping `wasmtime-c-api-impl`... bumping `wasmtime-wizer`... bumping `wasmtime-cli-flags`... bumping `wasmtime-internal-explorer`... bumping `wasmtime-cli`... 41.0.0 => 41.0.1 Running: `"cargo" "fetch" "--offline"` nick@hamilton :: (wasmtime-error *) :: ~/wasmtime $ echo $? 0</details>
Edit: d'oh I missed that CI is doing a
cargo vetafter the version bump.
Wasmtime GitHub notifications bot (Dec 12 2025 at 21:55):fitzgen edited a comment on PR #12163:
Grrrr this bump-and-vet CI failure is really annoying me.
Works when I run the exact same commands locally:<details>
nick@hamilton :: (wasmtime-error) :: ~/wasmtime $ rustc scripts/publish.rs && ./publish bump-patch bumping `component-macro-test-helpers`... bumping `component-async-tests`... bumping `wizer-fuzz`... bumping `regex-test`... bumping `regex-bench`... bumping `uap-bench`... bumping `wasmtime-environ-fuzz`... bumping `wasmtime-bench-api`... bumping `wasi-preview1-component-adapter`... bumping `verify-component-adapter`... bumping `byte-array-literals`... bumping `wasi-nn-example-winml`... bumping `wasi-nn-example-named`... bumping `wasi-nn-example`... bumping `classification-component-onnx`... bumping `wasi-nn-example-pytorch`... bumping `wasmtime-test-macros`... bumping `wiggle-test`... bumping `test-programs`... bumping `test-programs-artifacts`... bumping `wasmtime-test-util`... bumping `wasmtime-fuzzing`... bumping `wasm-spec-interpreter`... bumping `wasmtime-c-api`... bumping `cranelift-tools`... bumping `cranelift-assembler-x64-fuzz`... bumping `cranelift-fuzzgen`... bumping `cranelift-filetests`... bumping `isle-fuzz`... bumping `veri_ir`... bumping `veri_engine`... bumping `islec`... bumping `pulley-interpreter-fuzz`... bumping `cranelift-bitset`... 0.128.0 => 0.128.1 bumping `wasmtime-internal-math`... bumping `pulley-macros`... bumping `pulley-interpreter`... bumping `cranelift-srcgen`... 0.128.0 => 0.128.1 bumping `cranelift-assembler-x64-meta`... 0.128.0 => 0.128.1 bumping `cranelift-assembler-x64`... 0.128.0 => 0.128.1 bumping `cranelift-isle`... 0.128.0 => 0.128.1 bumping `cranelift-entity`... 0.128.0 => 0.128.1 bumping `cranelift-bforest`... 0.128.0 => 0.128.1 bumping `cranelift-codegen-shared`... 0.128.0 => 0.128.1 bumping `cranelift-codegen-meta`... 0.128.0 => 0.128.1 bumping `cranelift-control`... 0.128.0 => 0.128.1 bumping `cranelift-codegen`... 0.128.0 => 0.128.1 bumping `cranelift-reader`... 0.128.0 => 0.128.1 bumping `cranelift-serde`... 0.128.0 => 0.128.1 bumping `cranelift-module`... 0.128.0 => 0.128.1 bumping `cranelift-frontend`... 0.128.0 => 0.128.1 bumping `cranelift-native`... 0.128.0 => 0.128.1 bumping `cranelift-object`... 0.128.0 => 0.128.1 bumping `cranelift-interpreter`... 0.128.0 => 0.128.1 bumping `wasmtime-internal-jit-icache-coherence`... bumping `wasmtime-internal-unwinder`... bumping `cranelift-jit`... 0.128.0 => 0.128.1 bumping `cranelift`... 0.128.0 => 0.128.1 bumping `wiggle-generate`... bumping `wiggle-macro`... bumping `wasmtime-internal-error`... bumping `wasmtime-internal-versioned-export-macros`... bumping `wasmtime-internal-slab`... bumping `wasmtime-internal-component-util`... bumping `wasmtime-internal-wit-bindgen`... bumping `wasmtime-internal-component-macro`... bumping `wasmtime-internal-jit-debug`... bumping `wasmtime-internal-fiber`... bumping `wasmtime-environ`... bumping `wasmtime-internal-wmemcheck`... bumping `wasmtime-internal-cranelift`... bumping `wasmtime-internal-cache`... bumping `winch-codegen`... bumping `wasmtime-internal-winch`... bumping `wasmtime`... bumping `wiggle`... bumping `wasi-common`... bumping `wasmtime-wasi-io`... bumping `wasmtime-wasi`... bumping `wasmtime-wasi-http`... bumping `wasmtime-wasi-nn`... bumping `wasmtime-wasi-config`... bumping `wasmtime-wasi-keyvalue`... bumping `wasmtime-wasi-threads`... bumping `wasmtime-wasi-tls`... bumping `wasmtime-wasi-tls-nativetls`... bumping `wasmtime-wast`... bumping `wasmtime-internal-c-api-macros`... bumping `wasmtime-c-api-impl`... bumping `wasmtime-wizer`... bumping `wasmtime-cli-flags`... bumping `wasmtime-internal-explorer`... bumping `wasmtime-cli`... 41.0.0 => 41.0.1 Running: `"cargo" "fetch" "--offline"` nick@hamilton :: (wasmtime-error *) :: ~/wasmtime $ echo $? 0</details>
Edit: d'oh I missed a command from CI. Can repro.
Wasmtime GitHub notifications bot (Dec 12 2025 at 22:10):fitzgen updated PR #12163.
Wasmtime GitHub notifications bot (Dec 12 2025 at 22:18):fitzgen commented on PR #12163:
Looks like https://github.com/bytecodealliance/wasmtime/pull/12163/commits/fbac398ef538a6069b4f9d3da45b7ef3f2df4842 did indeed fix the
cargo vetissues.
Wasmtime GitHub notifications bot (Dec 12 2025 at 23:45):github-actions[bot] commented on PR #12163:
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 (Dec 15 2025 at 18:56):fitzgen commented on PR #12163:
@cfallin do you feel comfortable reviewing this since Alex is out-of-office right now?
Wasmtime GitHub notifications bot (Dec 15 2025 at 19:05):cfallin commented on PR #12163:
Sure, happy to do so!
Wasmtime GitHub notifications bot (Dec 15 2025 at 19:05):cfallin requested cfallin for a review on PR #12163.
Wasmtime GitHub notifications bot (Dec 16 2025 at 01:03):cfallin submitted PR review:
Partial review but I wanted to checkpoint these comments at least -- will keep reading from
lib.rsonward in the new crate tomorrow!This is very nice code overall; just little nits mostly.
Wasmtime GitHub notifications bot (Dec 16 2025 at 01:03):cfallin created PR review comment:
I assume that we are doing this direct import from the error crate for now since it's not integrated into Wasmtime, but is it the long-term goal to reexport the public error types from
wasmtimeitself? If so, should we leave a TODO/FIXME here noting that?
Wasmtime GitHub notifications bot (Dec 16 2025 at 01:03):cfallin created PR review comment:
Could we have a static-assert somewhere that (instantiating for some error-type
E) asserts thatvtableandbacktracehave the same offsets in both types? It's almost tautological today as they're right next to each other, list the same field prefix, and arerepr(C), but it can't hurt to double-check if someone adds fields in the future...
Wasmtime GitHub notifications bot (Dec 16 2025 at 01:03):cfallin created PR review comment:
Could we add a comment here describing why we need to manually implement vtables rather than (say) use a
dyn Traitfat-pointer? I know there are some interesting layout-size tradeoffs you navigated here to arrive at this.
Wasmtime GitHub notifications bot (Dec 16 2025 at 01:03):cfallin created PR review comment:
Is there a reason this has to be nested within
new()? I understand the encapsulation is nice (it's only instantiated here, nowhere else) but the indents are a little deep and it makes the code harder to read for me at least -- in-place struct impls are nicest when they're small or ad-hoc IMHO.(Not a huge deal, just my aesthetic preference)
Wasmtime GitHub notifications bot (Dec 16 2025 at 01:03):cfallin created PR review comment:
s/seemlessly/seamlessly/
Wasmtime GitHub notifications bot (Dec 16 2025 at 01:03):cfallin created PR review comment:
Is the idea that backtracing is not yet OOM-safe? (Will it be eventually or will we document the settings needed for OOM-safety, including disabling backtraces?) Can we leave a comment here describing why disable?
Wasmtime GitHub notifications bot (Dec 16 2025 at 01:03):cfallin created PR review comment:
This is a very nice and clever trick, on the one hand. On the other, I worry that it feels a little brittle: for example if there is some weird platform in the future (perhaps one that we support only Pulley on) that has restrictive alignment or padding requirements, or if we change the fields of the two structs below such that they get the same alignment, or whatever, or if
danglingno longer returns the first valid pointer (such that it will be unaligned for the higher alignment), this is no longer true.In particular, I see the docs for
NonNull::danglingonly specify that it returns a valid, aligned pointer, but we aren't asserting here that that pointer is not aligned forDynError. Concretely, if the alignments are 8 and 16 respectively, nothing in the docs seems to disallowNonNull::<OutOfMemory>::dangling()from returning 0x10 rather than 0x08, I think? Or, randomly, some bits that turn out to be a valid box allocation address for aDynError?The docs say:
Note that the address of the returned pointer may potentially be that of a valid pointer, which means this must not be used as a “not yet initialized” sentinel value.
So: perhaps we could do traditional LSB pointer tagging/masking? Or is that not viable for other reasons?
Wasmtime GitHub notifications bot (Dec 16 2025 at 08:59):bjorn3 submitted PR review.
Wasmtime GitHub notifications bot (Dec 16 2025 at 08:59):bjorn3 created PR review comment:
backtrace-rs allocates memory.
Wasmtime GitHub notifications bot (Dec 16 2025 at 18:41):cfallin created PR review comment:
Ah, I see this doc-comment now -- I think I had asked for an explanation of the need for a raw vtable to be added elsewhere as a comment (in review part 1) so a reference to this would be sufficient. (Thanks for the writeup!)
Wasmtime GitHub notifications bot (Dec 16 2025 at 18:41):cfallin submitted PR review:
And here's a review on the rest of the diff. Thanks for building all of this -- in general it looks quite good! My only real concern is the dangling-pointer issue in prior review. With that addressed (or adequately documented if I'm wrong about the bad case?) I'm happy to r+.
I'll note as well that my review is at the level of "plausible and looks correct"; Alex may still have opinions about what is best but I suspect that can be left to followup as well.
Wasmtime GitHub notifications bot (Dec 16 2025 at 18:41):cfallin created PR review comment:
Would it make sense to implement
Dropon this struct with a panic, to ensure that we always deconstruct it viainto_*instead?
Wasmtime GitHub notifications bot (Dec 16 2025 at 18:43):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Dec 16 2025 at 18:43):cfallin created PR review comment:
Yep, so "can we leave a comment here describing why?"
Wasmtime GitHub notifications bot (Dec 16 2025 at 20:32):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Dec 16 2025 at 20:32):fitzgen created PR review comment:
Yes, I have follow up work to publicly re-export
wasmtime_erroraswasmtime::error(and also continue to havewasmtime::Errorandwasmtime::Resultat the top level for convenience).But also, the fuzzing crate is internal and will never be published, so it doesn't really matter how it names the types, directly or via re-export. It already, for example, pulls in
wasmtime-environand uses that directly in a few places, rather than constraining itself to only using the canonical publicwasmtimeAPI.I can just fix it right now in my current WIP branch to actually get
wasmtimeusing this crate instead ofanyhow, rather than add a comment that I immediately remove, if that is okay with you.
Wasmtime GitHub notifications bot (Dec 16 2025 at 20:38):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Dec 16 2025 at 20:38):cfallin created PR review comment:
Ah, yes, that would be fine too -- not a big deal either way, just wanted to ensure we don't keep an intermediate state of affairs around forever.
Wasmtime GitHub notifications bot (Dec 16 2025 at 20:43):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Dec 16 2025 at 20:43):fitzgen created PR review comment:
There is a regular
#[test]for this at the bottom of this file. It used to be a static assert, but I changed it because it was easier to debug as a regular#[test], since static asserts can't contain any custom formatting in the diagnostic messages. But the assertions do indeed exist.
Wasmtime GitHub notifications bot (Dec 16 2025 at 20:52):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Dec 16 2025 at 20:52):cfallin created PR review comment:
Ah, neat, I missed that then -- sorry and all good!
Wasmtime GitHub notifications bot (Dec 16 2025 at 20:57):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Dec 16 2025 at 20:57):fitzgen created PR review comment:
Well the conversion does happen without anyone seeming to have done anything :-p
Wasmtime GitHub notifications bot (Dec 16 2025 at 20:58):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Dec 16 2025 at 20:58):fitzgen created PR review comment:
No reason besides making it clear what the encapsulation is, but I can change it.
Wasmtime GitHub notifications bot (Dec 16 2025 at 21:21):fitzgen updated PR #12163.
Wasmtime GitHub notifications bot (Dec 16 2025 at 21:25):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Dec 16 2025 at 21:25):fitzgen created PR review comment:
This gets a little funky in various places, for example in
BoxedDynError, where we are in aDropimplementation and so we only have&mut selfand are forced to make a raw copy of anOwnedPtrin order to turn it into aBoxand run the innerDropand deallocate it, but then there is still anOwnedPointeron the&mut self. So we would need to essentially turn that into anOption<OwnedPointer>, effectively acting as a drop flag and adding a dynamic check to all uses, which is unnecessary overhead. So I'd prefer not to.
Wasmtime GitHub notifications bot (Dec 16 2025 at 21:27):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Dec 16 2025 at 21:27):fitzgen created PR review comment:
Addressed in https://github.com/bytecodealliance/wasmtime/pull/12163/changes/20b06e36d3fcb7f1930afc53e3b9c5dad9492fc4
Wasmtime GitHub notifications bot (Dec 16 2025 at 21:27):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Dec 16 2025 at 21:27):fitzgen created PR review comment:
Addressed in https://github.com/bytecodealliance/wasmtime/pull/12163/changes/6a9cfa6378be8ccf291a1e61a83848eb4cb6f719 and https://github.com/bytecodealliance/wasmtime/pull/12163/changes/2f327112f853cf108c1b9f5dc4cc77a15ed8c055
Wasmtime GitHub notifications bot (Dec 16 2025 at 21:29):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Dec 16 2025 at 21:29):fitzgen created PR review comment:
Addressed in https://github.com/bytecodealliance/wasmtime/pull/12163/changes/2f327112f853cf108c1b9f5dc4cc77a15ed8c055
Wasmtime GitHub notifications bot (Dec 16 2025 at 21:29):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Dec 16 2025 at 21:29):fitzgen created PR review comment:
Addressed in https://github.com/bytecodealliance/wasmtime/pull/12163/changes/2f327112f853cf108c1b9f5dc4cc77a15ed8c055
Wasmtime GitHub notifications bot (Dec 16 2025 at 21:29):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Dec 16 2025 at 21:29):fitzgen created PR review comment:
Addressed in https://github.com/bytecodealliance/wasmtime/pull/12163/changes/0add4dc3e7248bed97059552496d0269f88faa31
Wasmtime GitHub notifications bot (Dec 16 2025 at 21:31):fitzgen commented on PR #12163:
@cfallin I believe I've addressed all of your feedback, and this should be ready for another look.
Wasmtime GitHub notifications bot (Dec 16 2025 at 21:31):fitzgen requested cfallin for a review on PR #12163.
Wasmtime GitHub notifications bot (Dec 16 2025 at 21:47):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Dec 16 2025 at 21:47):cfallin created PR review comment:
OK, yeah, no need to do that. Was mostly wondering if there was a reasonable path to add a little more dynamically-asserted safety here but it's not critical and this is all encapsulated locally anyway.
Wasmtime GitHub notifications bot (Dec 16 2025 at 21:50):cfallin submitted PR review:
OK, this looks correct to me and seems reasonable as far as my opinions of error interfaces go!
Wasmtime GitHub notifications bot (Dec 16 2025 at 22:32):fitzgen commented on PR #12163:
Regarding this CI failure: https://github.com/bytecodealliance/wasmtime/actions/runs/20283910974/job/58252868624
I don't think this PR can pass that CI step and merge until @wasmtime-publish is an owner of the
wasmtime-internal-errorcrate. I created an empty crate and published it to crates.io at v0.0.0 aswasmtime-internal-error, and I tried to addwasmtime-publishto it, but I don't have that account's credentials so I can't accept the ownership invite on its behalf.
Wasmtime GitHub notifications bot (Dec 16 2025 at 22:33):fitzgen commented on PR #12163:
Will try again, just in case that CI step will somehow accept the invite.
Wasmtime GitHub notifications bot (Dec 16 2025 at 22:40):cfallin commented on PR #12163:
I don't think this PR can pass that CI step and merge until @wasmtime-publish is an owner of the wasmtime-internal-error crate. I created an empty crate and published it to crates.io at v0.0.0 as wasmtime-internal-error, and I tried to add wasmtime-publish to it, but I don't have that account's credentials so I can't accept the ownership invite on its behalf.
Seems like an oversight in our procedures -- @alexcrichton when you're back, would you be willing to write up a runbook entry for "I added a new crate to Wasmtime" with the way to make the current publish flow happy? (It seems I've gotten caught in similar issues a few times too, whether it be the "published crates" list or deferred problems on next release or ...)
Wasmtime GitHub notifications bot (Dec 16 2025 at 22:54):fitzgen commented on PR #12163:
I don't think this PR can pass that CI step and merge until @wasmtime-publish is an owner of the
wasmtime-internal-errorcrate. I created an empty crate and published it to crates.io at v0.0.0 aswasmtime-internal-error, and I tried to addwasmtime-publishto it, but I don't have that account's credentials so I can't accept the ownership invite on its behalf.This seems to have made the
cargo vetCI job start failing again :weary:
Wasmtime GitHub notifications bot (Dec 17 2025 at 16:18):fitzgen updated PR #12163.
Wasmtime GitHub notifications bot (Dec 17 2025 at 16:18):fitzgen has enabled auto merge for PR #12163.
Wasmtime GitHub notifications bot (Dec 17 2025 at 19:16):fitzgen merged PR #12163.
Wasmtime GitHub notifications bot (Dec 19 2025 at 21:35):alexcrichton submitted PR review:
I mentioned this in a Zulip DM as well, but personally I think this crate should be folded into
wasmtime-environ. That's going to be what everything else depends on to use this error (I think, right?) so I'm not sure it needs to be split out like this. New crates involves vets, publishing, checks in CI, documentation, messaging, conventions, etc, etc. I've no doubt it was easier to develop initially as a separate crate but personally I think the balance after it's "done" is the other direction of being in the preexisting crates
Wasmtime GitHub notifications bot (Dec 19 2025 at 21:35):alexcrichton created PR review comment:
My impression is that most of the code in this crate is copied from
anyhow, and if so the upstream licensing of MIT/Apache-2.0 I believe means we can't "just" relicense this here.
Wasmtime GitHub notifications bot (Dec 19 2025 at 21:35):alexcrichton created PR review comment:
Could we actually delete this macro and use
format_errinstead? I'm otherwise a bit worried about copyinganyhowtoo too closely here and causing confusion. Basically the nameanyhowfeels like it should be reserved foranyhow-the-crate, while the other macros here are all generic enough it seems fine to use them
Wasmtime GitHub notifications bot (Dec 19 2025 at 21:35):alexcrichton created PR review comment:
I would naively expect that forcing everything, through
inline(never), to go through a closure probably on the stack would pessimize the caller quite a bit even in the fast path (e.g. bigger stack frames, more spills, etc). Did you find this to help internally during development though?
Wasmtime GitHub notifications bot (Dec 19 2025 at 21:35):alexcrichton created PR review comment:
This should be
unsafeI think since it's safe to copy aroundinnerandOwnedPtrcan be constructed from anyNonNull
Wasmtime GitHub notifications bot (Dec 19 2025 at 21:35):alexcrichton created PR review comment:
Could this use
Relaxedhere instead since there explicitly aren't any orderings with these operations with respect to anything else other than the bool itself?
Wasmtime GitHub notifications bot (Dec 19 2025 at 21:35):alexcrichton created PR review comment:
This might also benefit from
#[repr(transparent)]to ensure that ABI considerations don't think that this is a struct
Wasmtime GitHub notifications bot (Dec 19 2025 at 21:35):alexcrichton created PR review comment:
Or, actually, I'd alternatively bikeshed that the signature of this method should be:
pub(crate) fn bikeshed_me<T>() -> Result<Box<MaybeUninit<T>>, OutOfMemory>;and let callers handle initialization
Wasmtime GitHub notifications bot (Dec 19 2025 at 21:35):alexcrichton created PR review comment:
One thing I might recommend here is that once
ptrexists to convert it toBox<MabyeUninit<T>>which enablesBox::writeto safely convert that toBox<T>which can help reduce theunsafehere
Wasmtime GitHub notifications bot (Dec 19 2025 at 21:35):alexcrichton created PR review comment:
How come this trait has an
Etype parameter which forcescore::convert::Infalliblehere?
Wasmtime GitHub notifications bot (Dec 19 2025 at 21:35):alexcrichton created PR review comment:
This documentation, and much below, talsk about a
Tbut there's noTin this trait definition. Is that referring toSelf?
Wasmtime GitHub notifications bot (Dec 19 2025 at 21:35):alexcrichton created PR review comment:
Instead of using
unsafecould this doBox::new(OutOfMemory::new())? That won't actually allocate anything for zero-sized-types
Wasmtime GitHub notifications bot (Dec 19 2025 at 21:35):alexcrichton created PR review comment:
Perhaps
NonNull::dangling()here?
Wasmtime GitHub notifications bot (Dec 19 2025 at 21:35):alexcrichton created PR review comment:
One downside of this is that it's not possible to have a payload in
OutOfMemory, such as how many bytes were allocated. Is that something we might want in the future?
Wasmtime GitHub notifications bot (Dec 19 2025 at 21:50):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Dec 19 2025 at 21:50):alexcrichton created PR review comment:
Ended up resolving this as "no, this is not a copy, but is inspired by"
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:21):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:21):fitzgen created PR review comment:
I guess? I'm always hesitant to use weaker orderings just out of an abundance of caution.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:25):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:25):fitzgen created PR review comment:
I didn't, I just wanted to make sure that we weren't inserting things like OOM-handling code on every
?in the code base. I'd hoped that LLVM would recognize that this is marked cold and not eagerly expand the stack frame for the whole caller function, just on the slow path, but I haven't measured anything yet.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:28):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:28):fitzgen created PR review comment:
This would increase the size of the diff (and associated amount of porting work) a huge amount for the next PR that migrates Wasmtime to using this error type. And this will be useful to embedders migrating e.g. host functions over to use
wasmtime::Errorfor the same reason.I don't think it is out of place since we are explicitly advertising API compatibility with
anyhow.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:31):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:31):fitzgen created PR review comment:
I was matching
Box::new's API/signature. Doing the uninit version could be fine, but I'm not really too worried about it, seems pretty minor.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:32):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:32):fitzgen created PR review comment:
Here I was matching
anyhow's API, since this trait is public. I don't know why they made that decision. I guess to allow implementing it for results with foreign errors, but ensuring that the error has the bounds necessary to be convertible into ananyhow::Error. And it just happens that it isn't necessary forOption<T>.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:33):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:33):fitzgen created PR review comment:
Correct, I matched
anyhow's API, but this is an original implementation.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:34):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:34):fitzgen created PR review comment:
Sort of, it is the actual underlying error type that is either
Selfor for whichSelfis a newtype wrapper around. I can clarify.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:34):fitzgen edited PR review comment.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:49):fitzgen created PR review comment:
My thinking is that keeping
Result<(), wasmtime::Error>in a single pointer is more important.But what we can do to claw some context back is to do something like add space on the store or a global or something to hold that information, and have our fallible collections code write that context. Or we can simply log messages or something.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:49):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:52):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:52):fitzgen created PR review comment:
It originally was that, but Chris pushed back saying that we have no guarantee that the dangling pointer will always be the smallest pointer that is aligned for the type, and so it is possible that the dangling pointer could "accidentally" be aligned for
*mut DynErrorwhich could make discriminating between our variants ambiguous. So I switched it from the dangling pointer to an explicit0x1.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:52):fitzgen edited PR review comment.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:54):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:54):alexcrichton created PR review comment:
What I'm worried about is that if
fis a non-zero-size-type it forces all captured variables to go onto the stack, meaning that even in the fast path the caller function here has a larger stack frame because it might dynamically take the path to store all the captures offonto the stack, even if it never actually executes that at runtime.In my experience the
#[cold]bit tends to go well on some error constructors or specific paths, but I don't think we need to worry about it everywhere within the error type implementation here. Once you're on the error path that's already in the slow path for most cases so making the slow path that much more faster I would expect to be in a bit of a diminishing returns situation.Personally I'd say that we shouldn't try out things like this until it's found to be necessary as otherwise it seems like this could accidentally and unnecessarily bloat binaries and/or cause unexpected slowdowns.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:55):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:55):alexcrichton created PR review comment:
While I don't doubt it's a lot of work, does this really move the needle that much? This'd be an extra
s/anyhow/format_err/at the end basically is what I'd expect. For embedders we could leave this but mark it#[deprecated]for example.Otherwise though this feels a bit too close to
anyhowitself as while this is mostly API compatible withanyhowthere's no guarantee that we'll trackanyhowover time. Ifanyhowadds features it could make it much more confusing to callers when our version doesn't have them.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:56):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:56):alexcrichton created PR review comment:
My goal is to reduce the amount of
unsafehere. There are twounsafeblocks in this function that aren't necessary withBox<MaybeUninit<T>>due toBox::write
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:57):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Dec 22 2025 at 17:57):alexcrichton created PR review comment:
Personally I'd say we should trim things down for our own needs given that we're not making a super-general-purpose error-handling library, but rather just a library for our own needs.
Wasmtime GitHub notifications bot (Dec 22 2025 at 18:01):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Dec 22 2025 at 18:01):alexcrichton created PR review comment:
Well keeping a single-pointer-repr technically doesn't clash with containing more info here. For example a tagged-pointer representation could be used to carry payload bits for the OOM representation (e.g.
1 | (size << 1)== oom, and bit 0 == 0 means an allocated error).I noticed there were quite a few places that relied on
OutOfMemorybeing a zero-sized-type so I realize that this is not a small ask, but carrying the payload of how many bytes requested can be kind of a big deal. Seeing "out of memory" can mean something radically different if it's allocating 2M bytes vs 200 bytes. I'd like to ideally move in the direction of carrying the payload information for that extra debugging context, even if it's not immediately possible just yet
Wasmtime GitHub notifications bot (Dec 22 2025 at 18:04):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Dec 22 2025 at 18:04):alexcrichton created PR review comment:
I'm not sure I understand that pushback, if this is
::dangling()how is the alignment of the other error related? In both casesis_oomstill tests againstOOM_REPRwhich means the return value of::dangling()doesn't really matter is what I'd expect
Wasmtime GitHub notifications bot (Dec 22 2025 at 18:11):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Dec 22 2025 at 18:11):cfallin created PR review comment:
You're right that alignment doesn't matter but the actual value does. As quoted
Note that the address of the returned pointer may potentially be that of a valid pointer, which means this must not be used as a “not yet initialized” sentinel value.
the docs for
NonNull::danglingbasically are saying that the value could be anything. That means that nothing guarantees thatOOM_REPRis also some other value, like an allocated boxed error value.In constraint form, if we take the dangling-ptr approach again, and if we have a boxed-error pointer: we are given that
dangling == OOM_REPR, and we are given thatp == <some box address>, and we know thatOOM_REPRis a valid aligned pointer value, and nothing else. From those facts, we cannot prove thatp != OOM_REPR, even thoughpis not supposed to be an OOM.
Wasmtime GitHub notifications bot (Dec 22 2025 at 18:11):cfallin edited PR review comment.
Wasmtime GitHub notifications bot (Dec 22 2025 at 18:32):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Dec 22 2025 at 18:32):fitzgen created PR review comment:
FWIW, I just looked at the code for the
.context(...)extension method with and without this macro, and the code is much better with.
Wasmtime GitHub notifications bot (Dec 22 2025 at 18:38):alexcrichton created PR review comment:
Ah ok I see what you mean. I did some digging and the clause of the standard library docs comes from https://github.com/rust-lang/rust/pull/52508. One reason for adding the clause is that the first addresable item in memory is technically a valid address, so equality against the return value of
NonNull::dangling()is not suitable for "is this uninitialized". That means that my idea of usingNonNull::dangling()is doing exactly what you're not supposed to do, so seems good to not use it.
Wasmtime GitHub notifications bot (Dec 22 2025 at 18:38):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Dec 22 2025 at 18:40):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Dec 22 2025 at 18:40):alexcrichton created PR review comment:
Could you share the snippet you were looking at? I'd be curious to poke around it as well
Wasmtime GitHub notifications bot (Dec 22 2025 at 18:51):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Dec 22 2025 at 18:51):fitzgen created PR review comment:
For sure, if we can figure out a way to have the context and the single-pointer repr, then I'm all for it.
The potential hiccups I foresee with the approach you describe is that it works when we have an
&OutOfMemory(since the context is packed into the pointer bits, but it doesn't work for when we have just anOutOfMemorythat isn't behind a reference (because now we have nowhere to pack that context into). This is probably workable, but could make the API funky in various ways.
Wasmtime GitHub notifications bot (Dec 22 2025 at 18:53):fitzgen created PR review comment:
I think being able to call
std::fs::read_to_string(...).context(...)is pretty important for usability and it does require theEtype parameter.I would also push back a little bit on the idea that we aren't making a general purpose error-handling library, because this needs to be usable by anyone writing e.g. Wasmtime host functions which could really be doing just about anything, so it feels pretty general purpose to me.
Wasmtime GitHub notifications bot (Dec 22 2025 at 18:53):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Dec 22 2025 at 18:53):fitzgen edited PR review comment.
Wasmtime GitHub notifications bot (Dec 22 2025 at 19:14):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Dec 22 2025 at 19:14):alexcrichton created PR review comment:
Oh if it's required it's fine to have of course, and in that case mind adding documentation to that effect?
And yeah it's true this is general, but my original hunch for the
Ehere is it's some inference-related thing in niche contexts or something like that. If that were the case I don't think we're general enough to, for example, care about Rust 1.50.0 inference behavior on one platform with a specific set of constraitns in play. Foranyhow, however, something like that is plausibly in-scope.If, however,
Eis just needed for some nontrivial number of usages then that's fine, I'd just prefer to see documentation since naively I don't know why this is here.
Wasmtime GitHub notifications bot (Dec 22 2025 at 19:17):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Dec 22 2025 at 19:17):alexcrichton created PR review comment:
Oh I have no doubt it'll be tricky, but personally I think it'd be worth it. I've filed https://github.com/bytecodealliance/wasmtime/issues/12199 for this
Last updated: Jan 10 2026 at 20:04 UTC