Update the host ABI of Wasmtime to return failure #9675

alexcrichton · 2024-11-25T17:59:33Z

This commit updates the "array call" ABI of Wasmtime used to transition from wasm to the host to explicitly return a bool indicating whether the call succeeded or not. Previously functions would implicitly unwind via longjmp and thus no explicit checks were necessary. The burden of unwinding is now placed on Cranelift-compiled code itself instead of the caller.

There are a few pieces of rationale for this change:

Primarily I was implementing initial integration of Pulley where the host setjmp and longjmp cannot be used to maintain the Pulley interpreter state. My initial plan for handling this was to handle traps a bit differently in Pulley where having direct access to whether a function trapped or not in the interpreter bytecode is easier to deal with.
Additionally it's generally not safe to call longjmp from Rust as it will not run on-stack destructors. This is ok today in the sense that we shouldn't have these in Wasmtime, but directly returning to compiled code improves our safety story here a bit where we just won't have to deal with the problem of skipping destructors.
In the future I'd like to move away from setjmp and longjmp entirely in the host to a Cranelift-native solution. This change would be required for such a migration anyway so it's my hope to make such a Cranelift-based implementation easier in the future. This might be necessary, for example, when implementing the exception-handling proposal for Wasmtime.

Functionally this commit does not remove all usages of call-longjmp-from-Rust. Notably all libcalls and builtins still use this helper in the trampolines generated in Rust. I plan on going through the libcalls and updating their ABIs and signatures to reflect this in follow-up commits. As a result a few preexisting functions that should go away are marked #[deprecated] for internal use in this commit. I'll be cleaning that up as follow-ups. For now this commit handles the "hard part" of host functions by ensuring that the new bool return value is plumbed in all the locations correctly.

alexcrichton · 2024-11-25T18:02:23Z

I'll note that this has a bit of #9673 mixed in but doesn't depend on it.

Also to expand on:

I plan on going through the libcalls and updating their ABIs and signatures to reflect this in follow-up commits

The categories of libcalls are:

Some libcalls, but not all, need to be able to raise traps. For example we support memory.grow raising a trap because of the embedder API but we don't support table-lazy-init raising a trap.
All libcalls, however, need to catch Rust panics and do something with them.

My plan is to update the ABI of all libcalls that return traps to return some encoding of the current return value plus whether it trapped. That'll suffice for catching panics and handling traps. For all other libcalls which can't actually trap my plan is to abort the process. I plan on doing this by removing the catch_unwind entirely. That is no longer necessary for safety reasons in recent versions of Rust. That means that only libcalls which can raise a trap will have panics shepherded across them, and panics in other libcalls will abort the process.

pchickey

Broad strokes looks good, though I'm not quite enough of a cranelift expert to catch anything subtle here.

pchickey · 2024-11-25T18:44:03Z

crates/wasmtime/src/runtime/vm/libcalls.rs

@@ -109,6 +109,7 @@ pub mod raw {
                ) $( -> libcall!(@ty $result))? {
                    $(#[cfg($attr)])?
                    {
+                        #[allow(deprecated)] // FIXME: need to update this


Still a FIXME here and below in same file

Ah yeah this is intentional where it's deferred to a future PR to handle the usages for libcalls

pchickey · 2024-11-25T18:44:41Z

crates/wasmtime/src/runtime/vm/traphandlers.rs

@@ -72,11 +83,16 @@ pub unsafe fn raise_user_trap(error: Error, needs_backtrace: bool) -> ! {
 /// the nearest `setjmp`. The panic will then be resumed from where it is
 /// caught.
 ///
+/// FIXME: this function should get removed in favor of
+/// `catch_unwind_and_record_trap` or a variant thereof.
+///


cfallin

Cranelift compilation bits look good to me.

github-actions · 2024-11-26T03:02:22Z

Subscribe to Label Action

cc @fitzgen

This issue or pull request has been labeled: "pulley", "wasmtime:api"

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

fitzgen: pulley

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

Learn more.

This commit updates the "array call" ABI of Wasmtime used to transition from wasm to the host to explicitly return a `bool` indicating whether the call succeeded or not. Previously functions would implicitly unwind via `longjmp` and thus no explicit checks were necessary. The burden of unwinding is now placed on Cranelift-compiled code itself instead of the caller. There are a few pieces of rationale for this change: * Primarily I was implementing initial integration of Pulley where the host `setjmp` and `longjmp` cannot be used to maintain the Pulley interpreter state. My initial plan for handling this was to handle traps a bit differently in Pulley where having direct access to whether a function trapped or not in the interpreter bytecode is easier to deal with. * Additionally it's generally not safe to call `longjmp` from Rust as it will not run on-stack destructors. This is ok today in the sense that we shouldn't have these in Wasmtime, but directly returning to compiled code improves our safety story here a bit where we just won't have to deal with the problem of skipping destructors. * In the future I'd like to move away from `setjmp` and `longjmp` entirely in the host to a Cranelift-native solution. This change would be required for such a migration anyway so it's my hope to make such a Cranelift-based implementation easier in the future. This might be necessary, for example, when implementing the `exception-handling` proposal for Wasmtime. Functionally this commit does not remove all usages of call-`longjmp`-from-Rust. Notably all libcalls and builtins still use this helper in the trampolines generated in Rust. I plan on going through the libcalls and updating their ABIs and signatures to reflect this in follow-up commits. As a result a few preexisting functions that should go away are marked `#[deprecated]` for internal use in this commit. I'll be cleaning that up as follow-ups. For now this commit handles the "hard part" of host functions by ensuring that the new `bool` return value is plumbed in all the locations correctly. prtest:full

This commit is the equivalent of bytecodealliance#9675 for libcalls used in core wasm. All libcalls now communicate whether or not they trapped through their return value instead of implicitly calling `longjmp` to exit from the libcall. This is to make integration with Pulley easier to avoid the need to `longjmp` over Pulley execution. Libcall definitions have changed where appropriate and the `catch_unwind_and_record_trap` function introduced in bytecodealliance#9675 was refactored to better support multiple types of values being returned from libcalls (instead of just `Result<()>`). Note that changes have been made to both the Cranelift translation layer and the Winch translation layer for this as the ABI of various libcalls are all changing.

This commit is the analog of bytecodealliance#9675 but for component libcalls. This follows suit from the previous commit which updates all component libcalls to explicitly returning whether a trap happened and if so performing said trap. This then removes the need for the old trap handling functions in the runtime which means they can all be deleted now as they're no longer necessary.

This commit is the equivalent of bytecodealliance#9675 for libcalls used in core wasm. All libcalls now communicate whether or not they trapped through their return value instead of implicitly calling `longjmp` to exit from the libcall. This is to make integration with Pulley easier to avoid the need to `longjmp` over Pulley execution. Libcall definitions have changed where appropriate and the `catch_unwind_and_record_trap` function introduced in bytecodealliance#9675 was refactored to better support multiple types of values being returned from libcalls (instead of just `Result<()>`). Note that changes have been made to both the Cranelift translation layer and the Winch translation layer for this as the ABI of various libcalls are all changing.

This commit is the analog of bytecodealliance#9675 but for component libcalls. This follows suit from the previous commit which updates all component libcalls to explicitly returning whether a trap happened and if so performing said trap. This then removes the need for the old trap handling functions in the runtime which means they can all be deleted now as they're no longer necessary.

This commit is the equivalent of bytecodealliance#9675 for libcalls used in core wasm. All libcalls now communicate whether or not they trapped through their return value instead of implicitly calling `longjmp` to exit from the libcall. This is to make integration with Pulley easier to avoid the need to `longjmp` over Pulley execution. Libcall definitions have changed where appropriate and the `catch_unwind_and_record_trap` function introduced in bytecodealliance#9675 was refactored to better support multiple types of values being returned from libcalls (instead of just `Result<()>`). Note that changes have been made to both the Cranelift translation layer and the Winch translation layer for this as the ABI of various libcalls are all changing.

This commit is the analog of bytecodealliance#9675 but for component libcalls. This follows suit from the previous commit which updates all component libcalls to explicitly returning whether a trap happened and if so performing said trap. This then removes the need for the old trap handling functions in the runtime which means they can all be deleted now as they're no longer necessary.

This commit is the equivalent of bytecodealliance#9675 for libcalls used in core wasm. All libcalls now communicate whether or not they trapped through their return value instead of implicitly calling `longjmp` to exit from the libcall. This is to make integration with Pulley easier to avoid the need to `longjmp` over Pulley execution. Libcall definitions have changed where appropriate and the `catch_unwind_and_record_trap` function introduced in bytecodealliance#9675 was refactored to better support multiple types of values being returned from libcalls (instead of just `Result<()>`). Note that changes have been made to both the Cranelift translation layer and the Winch translation layer for this as the ABI of various libcalls are all changing.

This commit is the analog of bytecodealliance#9675 but for component libcalls. This follows suit from the previous commit which updates all component libcalls to explicitly returning whether a trap happened and if so performing said trap. This then removes the need for the old trap handling functions in the runtime which means they can all be deleted now as they're no longer necessary.

This commit is a continuation of the plan of implementing host calls in Pulley through bytecodealliance#9665, bytecodealliance#9675, and bytecodealliance#9693. Here the `Compiler::call_indirect_host` method is updated to take a new type, `HostCall`, which indicates what type of host call is being performed. This is then serialized to a 32-bit integer which will be present in the pulley instruction being generated. This 32-bit integer will then be used to perform a dispatch (the dispatch is left for a future PR with more Pulley integration). This new `HostCall` structure is defined with `BuiltinFunctionIndex` internally. Additionally a new `ComponentBuiltinFunctionIndex` is added to enumerate the same set of indexes for components as well. Along the way the split between component transcoders/builtins were removed and they're now all lumped together in one macro for builtins. (no need to have two separate macros). This new `HostCall` is used to implement the `call_indirect_host` instruction for Pulley to fill out an unimplemented piece of code.