Cranelift: initial try_call / try_call_indirect (exception) support. #10510

cfallin · 2025-04-02T00:39:53Z

This PR adds try_call and try_call_indirect instructions, and lowerings on four of five ISAs (x86-64, aarch64, riscv64, pulley; s390x has its own non-shared ABI code that will need separate work).

It extends CLIF to support these instructions as new kinds of branches, and extends block-calls to accept retN and exnN block-call args that carry the normal return values or exception payloads (respectively) into the appropriate successor blocks.

It wires up the "normal return path" so that it continues to work. It updates the ABI so that unwinding is possible without an initial register state at throw: specifically, as per our RFC, all registers are clobbered. It also includes metadata in the MachBuffer that describes exception-catch destinations. However, no unwinder exists to interpret these catch-destinations yet, so they are untested.

cfallin · 2025-04-02T00:40:21Z

This builds on #10502; only the last commit is new.

cfallin · 2025-04-02T00:42:27Z

Procedural note: I'm happy to go through review on this and try to land it with the "normal return" path working (ah, I should add runtests too!), or we can wait until I've implemented a test unwinder in the clif-util function runner. I'm happy to go either way.

I'm also happy to add a Cranelift feature to gate this off by default until we've got it fully tested, if desired.

cfallin · 2025-04-02T00:44:12Z

Also, I haven't built full Wasmtime since rebasing so some newer CLIF-producing code needs to be updated; I'll put this in draft until then (and the runtests).

fitzgen · 2025-04-02T01:11:58Z

Procedural note: I'm happy to go through review on this and try to land it with the "normal return" path working (ah, I should add runtests too!), or we can wait until I've implemented a test unwinder in the clif-util function runner. I'm happy to go either way.

Testing the normal path only for the moment seems fine by me.

Will take a look at this PR tomorrow.

fitzgen

Looks great! A bunch of nitpicks and a couple more serious comments below.

cranelift/codegen/meta/src/shared/instructions.rs

cranelift/codegen/src/ir/dfg.rs

cranelift/codegen/src/ir/entities.rs

cranelift/codegen/src/isa/x64/abi.rs

fitzgen · 2025-04-02T20:50:46Z

cranelift/codegen/src/machinst/buffer.rs

+    /// Any exception handler targets at a given location.
+    pub exception_handlers: SmallVec<[(CodeOffset, PackedOption<ir::ExceptionTag>, CodeOffset); 8]>,


Looks like the exception payload registers for each handler are not included in here. But I think they should be, or else the unwinder implementation will have to just know what we happen to use since, while system-v documents this, there is no such existing docs/constraints for our tail convention. We could even theoretically let regalloc pick the registers, if we wanted.

That's technically true, but there are a number of reasons that pushed me toward "fixed payload registers" and "not listed in metadata":

Letting these be dynamic and allocating them with regalloc is pretty complex, because non-fixed-reg-but-still-reg-constrained defs on calls are error-prone -- we have to carefully poke holes in the clobbers to try to force regalloc to use those holes, then we have to assert that it did the right thing (because otherwise everything is clobbered). We really want a "def after clobbers" constraint but that is a weird thing in its own way.

Naming different registers per exception handler record per callsite is going to be quite bloated -- right now we have three u32s here, vs. 5.

Naming a variable-length list of registers (we don't specify that there are exactly e.g. two payload regs; that's a property of the platform and calling convention) is also complex: do we have an owned smallvec/vec per record here? separate list and ranges in it?

Letting the register choice be dynamic means that the unwinder needs a way to take an index and set an arbitrary register, rather than a fixed one. That rules out small handwritten longjmp-like trampolines/stubs (set rsp/rbp/rax and jump).

So basically it seemed like too much generality, leading to far too much complexity, vs. saying that each platform gets to specify fixed payload regs. I think I'm comfortable with specifying that the payload registers are a fixed choice and this choice becomes part of our public contract (we should document this somewhere; I'll add it to the doc-comments on try_call / try_call_indirect and/or the CallConv arms...).

That seems fair. Figuring out a good place to document this for users is going to be annoying.

cranelift/filetests/filetests/isa/x64/exceptions.clif

cranelift/filetests/filetests/verifier/exceptions.clif

cfallin · 2025-04-04T23:24:46Z

@fitzgen I think I've addressed everything except for the unresolved comments above (see comments, happy to take suggestions). I think #10502 also needs a review (Alex deferred to you per [here]#10502 (review)); I'll make the update that Ulrich suggests in a separate commit). Thanks!

cfallin · 2025-04-04T23:52:32Z

Rebased on top of latest #10502 (and had to squash the stack of review-feedback commits to avoid too many conflicts-at-every-step but they're still at this branch)

fitzgen

Thanks!

cranelift/codegen/src/isa/call_conv.rs

cranelift/codegen/src/verifier/mod.rs

This PR adds `try_call` and `try_call_indirect` instructions, and lowerings on four of five ISAs (x86-64, aarch64, riscv64, pulley; s390x has its own non-shared ABI code that will need separate work). It extends CLIF to support these instructions as new kinds of branches, and extends block-calls to accept `retN` and `exnN` block-call args that carry the normal return values or exception payloads (respectively) into the appropriate successor blocks. It wires up the "normal return path" so that it continues to work. It updates the ABI so that unwinding is possible without an initial register state at throw: specifically, as per our RFC, all registers are clobbered. It also includes metadata in the `MachBuffer` that describes exception-catch destinations. However, no unwinder exists to interpret these catch-destinations yet, so they are untested.

cfallin · 2025-04-07T23:39:07Z

Thanks! All addressed; while looking over it I noticed I still had try_call_indirect lowerings as a TODO and added the ~20 lines of final plumbing to wire it up. Separated that as last commit and wanted to flag before merging -- lemme know if good.

Following-up on bytecodealliance#10510 this adds the corresponding support to the s390x target. Due to the separate ABI implementation, this currently needs to duplicate some of the underlying logic. I hope to be able to refactor ABI handling in the future to be able to share more of that code across all targets.

Following-up on #10510 this adds the corresponding support to the s390x target. Due to the separate ABI implementation, this currently needs to duplicate some of the underlying logic. I hope to be able to refactor ABI handling in the future to be able to share more of that code across all targets.

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 bytecodealliance#10510 for Cranelift-level exception support, bytecodealliance#10919 for an unwinder, and bytecodealliance#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 bytecodealliance#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/

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 bytecodealliance#10510 for Cranelift-level exception support, bytecodealliance#10919 for an unwinder, and bytecodealliance#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/

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 bytecodealliance#10510 for Cranelift-level exception support, bytecodealliance#10919 for an unwinder, and bytecodealliance#11230 for exception objects built on top of GC, in addition a bunch of smaller fix and enabling PRs around those. [Wasm exception-handling proposal]: https://github.com/WebAssembly/exception-handling/

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 bytecodealliance#10510 for Cranelift-level exception support, bytecodealliance#10919 for an unwinder, and bytecodealliance#11230 for exception objects built on top of GC, in addition a bunch of smaller fix and enabling PRs around those. [Wasm exception-handling proposal]: https://github.com/WebAssembly/exception-handling/ prtest:full

* WebAssembly exception-handling support. 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. [Wasm exception-handling proposal]: https://github.com/WebAssembly/exception-handling/ prtest:full * Permit UnwindToWasm to have unused fields in Pulley builds (for now). * Resolve miri-caught reborrowing issue. * Ignore exceptions tests in miri for now (Pulley not supported). * Use wasmtime_test on exceptions tests. * Get tests passing on pulley platforms * Add a check to `supports_host` for the generated test and assert failure also when that is false. * Remove `pulley_unsupported` test as it falls out of `#[wasmtime_test]` * Remove `exceptions_store` helper as it falls out of `#[wasmtime_test]` * Remove miri annotations as they fall out of `#[wasmtime_test]` * Remove dead import * Skip some unsupported tests entirely in `#[wasmtime_test]` If the selected compiler doesn't support the host at all then there's no need to run it. Actually running it could misinterpret `CraneliftNative` as "run with pulley" otherwise, so avoid such false negatives. * Cranelift: dynamic contexts: account for outgoing-args area. --------- Co-authored-by: Alex Crichton <[email protected]>

* WebAssembly exception-handling support. 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 bytecodealliance#10510 for Cranelift-level exception support, bytecodealliance#10919 for an unwinder, and bytecodealliance#11230 for exception objects built on top of GC, in addition a bunch of smaller fix and enabling PRs around those. [Wasm exception-handling proposal]: https://github.com/WebAssembly/exception-handling/ prtest:full * Permit UnwindToWasm to have unused fields in Pulley builds (for now). * Resolve miri-caught reborrowing issue. * Ignore exceptions tests in miri for now (Pulley not supported). * Use wasmtime_test on exceptions tests. * Get tests passing on pulley platforms * Add a check to `supports_host` for the generated test and assert failure also when that is false. * Remove `pulley_unsupported` test as it falls out of `#[wasmtime_test]` * Remove `exceptions_store` helper as it falls out of `#[wasmtime_test]` * Remove miri annotations as they fall out of `#[wasmtime_test]` * Remove dead import * Skip some unsupported tests entirely in `#[wasmtime_test]` If the selected compiler doesn't support the host at all then there's no need to run it. Actually running it could misinterpret `CraneliftNative` as "run with pulley" otherwise, so avoid such false negatives. * Cranelift: dynamic contexts: account for outgoing-args area. --------- Co-authored-by: Alex Crichton <[email protected]>

cfallin requested review from a team as code owners April 2, 2025 00:39

cfallin requested review from fitzgen and removed request for a team April 2, 2025 00:39

cfallin marked this pull request as draft April 2, 2025 00:44

github-actions bot added cranelift Issues related to the Cranelift code generator cranelift:area:aarch64 Issues related to AArch64 backend. cranelift:meta Everything related to the meta-language. labels Apr 2, 2025

cfallin force-pushed the lets-be-truly-exceptional branch from 4624c01 to 5ac1f50 Compare April 2, 2025 20:37

fitzgen reviewed Apr 2, 2025

View reviewed changes

cfallin marked this pull request as ready for review April 4, 2025 23:22

cfallin force-pushed the lets-be-truly-exceptional branch from 8f95de2 to 648e17e Compare April 4, 2025 23:51

cfallin force-pushed the lets-be-truly-exceptional branch from 648e17e to 2b9ae27 Compare April 4, 2025 23:54

cfallin requested a review from a team as a code owner April 4, 2025 23:54

cfallin requested review from fitzgen and removed request for a team April 4, 2025 23:54

fitzgen approved these changes Apr 7, 2025

View reviewed changes

cranelift/codegen/src/isa/call_conv.rs Show resolved Hide resolved

cranelift/codegen/src/verifier/mod.rs Show resolved Hide resolved

cfallin force-pushed the lets-be-truly-exceptional branch from 2b9ae27 to c6d6fad Compare April 7, 2025 22:55

cfallin force-pushed the lets-be-truly-exceptional branch from f758ca8 to 5b336d7 Compare April 7, 2025 23:36

Add try_call_indirect lowering as well.

a405022

cfallin force-pushed the lets-be-truly-exceptional branch from 5b336d7 to a405022 Compare April 7, 2025 23:41

fitzgen approved these changes Apr 7, 2025

View reviewed changes

uweigand mentioned this pull request Apr 9, 2025

s390x: Add try_call / try_call_indirect support #10557

Merged

cfallin mentioned this pull request Apr 9, 2025

Cranelift: roll forward to RA2 once bytecodealliance/regalloc2#218 is fixed #10562

Closed

bjorn3 mentioned this pull request Apr 17, 2025

Support unwinding on panics rust-lang/rustc_codegen_cranelift#1567

Open

cfallin mentioned this pull request Apr 23, 2025

Support cleanup during unwinding #1677

Closed

cfallin mentioned this pull request Jul 25, 2025

WebAssembly exception-handling support. #11326

Merged

		/// Any exception handler targets at a given location.
		pub exception_handlers: SmallVec<[(CodeOffset, PackedOption<ir::ExceptionTag>, CodeOffset); 8]>,

Cranelift: initial try_call / try_call_indirect (exception) support. #10510

Cranelift: initial try_call / try_call_indirect (exception) support. #10510

Uh oh!

Conversation

cfallin commented Apr 2, 2025

Uh oh!

cfallin commented Apr 2, 2025

Uh oh!

cfallin commented Apr 2, 2025

Uh oh!

cfallin commented Apr 2, 2025

Uh oh!

fitzgen commented Apr 2, 2025

Uh oh!

fitzgen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fitzgen Apr 2, 2025

Choose a reason for hiding this comment

Uh oh!

cfallin Apr 3, 2025

Choose a reason for hiding this comment

Uh oh!

fitzgen Apr 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cfallin commented Apr 4, 2025

Uh oh!

cfallin commented Apr 4, 2025

Uh oh!

fitzgen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cfallin commented Apr 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants