Package up exception handler state (#11577)

* Package up exception handler state

This commit is a minor refactoring of the `wasmtime-unwinder` crate to
use a `Handler` structure as a "package" for the pc/fp/sp triple that's
required to resume to an exception handler. Freestanding functions are now
associated methods/functions of `Handler` such as finding a frame on the
stack and resuming to a handler.

This doesn't actually change any behavior, just moving some things
around to prepare for a future refactoring.

* Fix some CI issues

* Fix conditional build

Author: alexcrichton

cranelift/filetests/src/function_runner.rs

@@ -675,17 +675,15 @@ extern "C-unwind" fn __cranelift_throw(
             })
     };
     unsafe {
-        match wasmtime_unwinder::compute_throw_action(
+        match wasmtime_unwinder::Handler::find(
             &unwind_host,
             frame_handler,
             exit_pc,
             exit_fp,
             entry_fp,
         ) {
-            wasmtime_unwinder::ThrowAction::Handler { pc, sp, fp } => {
-                wasmtime_unwinder::resume_to_exception_handler(pc, sp, fp, payload1, payload2);
-            }
-            wasmtime_unwinder::ThrowAction::None => {
+            Some(handler) => handler.resume(payload1, payload2),
+            None => {
                 panic!("Expected a handler to exit for throw of tag {tag} at pc {exit_pc:x}");
             }
         }

crates/unwinder/src/arch/mod.rs

@@ -40,57 +40,57 @@ cfg_if::cfg_if! {
         /// the frame is later popped.
         pub use imp::get_stack_pointer;
 
-        /// Resume execution at the given PC, SP, and FP, with the given
-        /// payload values, according to the tail-call ABI's exception
-        /// scheme. Note that this scheme does not restore any other
-        /// registers, so the given state is all that we need.
-        ///
-        /// # Safety
-        ///
-        /// This method requires:
-        ///
-        /// - the `sp` and `fp` to correspond to an active stack frame
-        ///   (above the current function), in code using Cranelift's
-        ///   `tail` calling convention.
-        ///
-        /// - The `pc` to correspond to a `try_call` handler
-        ///   destination, as emitted in Cranelift metadata, or
-        ///   otherwise a target that is expecting the tail-call ABI's
-        ///   exception ABI.
-        ///
-        /// - The Rust frames between the unwind destination and this
-        ///   frame to be unwind-safe: that is, they cannot have `Drop`
-        ///   handlers for which safety requires that they run.
-        pub unsafe fn resume_to_exception_handler(
-            pc: usize,
-            sp: usize,
-            fp: usize,
-            payload1: usize,
-            payload2: usize,
-        ) -> ! {
-            // Without this ASAN seems to nondeterministically trigger an
-            // internal assertion when running tests with threads. Not entirely
-            // clear what's going on here but it seems related to the fact that
-            // there's Rust code on the stack which is never cleaned up due to
-            // the jump out of `imp::resume_to_exception_handler`.
-            //
-            // This function is documented as something that should be called to
-            // clean up the entire thread's shadow memory and stack which isn't
-            // exactly what we want but this at least seems to resolve ASAN
-            // issues for now. Probably a heavy hammer but better than false
-            // positives I suppose...
-            #[cfg(asan)]
-            {
-                unsafe extern "C" {
-                    fn __asan_handle_no_return();
+        impl crate::Handler {
+            /// Resume execution at the given PC, SP, and FP, with the given
+            /// payload values, according to the tail-call ABI's exception
+            /// scheme. Note that this scheme does not restore any other
+            /// registers, so the given state is all that we need.
+            ///
+            /// # Safety
+            ///
+            /// This method requires:
+            ///
+            /// - the `sp` and `fp` to correspond to an active stack frame
+            ///   (above the current function), in code using Cranelift's
+            ///   `tail` calling convention.
+            ///
+            /// - The `pc` to correspond to a `try_call` handler
+            ///   destination, as emitted in Cranelift metadata, or
+            ///   otherwise a target that is expecting the tail-call ABI's
+            ///   exception ABI.
+            ///
+            /// - The Rust frames between the unwind destination and this
+            ///   frame to be unwind-safe: that is, they cannot have `Drop`
+            ///   handlers for which safety requires that they run.
+            pub unsafe fn resume(
+                &self,
+                payload1: usize,
+                payload2: usize,
+            ) -> ! {
+                // Without this ASAN seems to nondeterministically trigger an
+                // internal assertion when running tests with threads. Not entirely
+                // clear what's going on here but it seems related to the fact that
+                // there's Rust code on the stack which is never cleaned up due to
+                // the jump out of `imp::resume_to_exception_handler`.
+                //
+                // This function is documented as something that should be called to
+                // clean up the entire thread's shadow memory and stack which isn't
+                // exactly what we want but this at least seems to resolve ASAN
+                // issues for now. Probably a heavy hammer but better than false
+                // positives I suppose...
+                #[cfg(asan)]
+                {
+                    unsafe extern "C" {
+                        fn __asan_handle_no_return();
+                    }
+                    unsafe {
+                        __asan_handle_no_return();
+                    }
                 }
                 unsafe {
-                    __asan_handle_no_return();
+                    imp::resume_to_exception_handler(self.pc, self.sp, self.fp, payload1, payload2)
                 }
             }
-            unsafe {
-                imp::resume_to_exception_handler(pc, sp, fp, payload1, payload2)
-            }
         }
 
         /// Get the return address in the function at the next-older
@@ -101,21 +101,20 @@ cfg_if::cfg_if! {
         /// - Requires that `fp` is a valid frame-pointer value for an
         ///   active stack frame (above the current function), in code
         ///   using Cranelift's `tail` calling convention.
-        pub use imp::get_next_older_pc_from_fp;
-
+        use imp::get_next_older_pc_from_fp;
 
         /// The offset of the saved old-FP value in a frame, from the
         /// location pointed to by a given FP.
-        pub const NEXT_OLDER_FP_FROM_FP_OFFSET: usize = imp::NEXT_OLDER_FP_FROM_FP_OFFSET;
+        const NEXT_OLDER_FP_FROM_FP_OFFSET: usize = imp::NEXT_OLDER_FP_FROM_FP_OFFSET;
 
         /// The offset of the next older SP value, from the value of a
         /// given FP.
-        pub const NEXT_OLDER_SP_FROM_FP_OFFSET: usize = imp::NEXT_OLDER_SP_FROM_FP_OFFSET;
+        const NEXT_OLDER_SP_FROM_FP_OFFSET: usize = imp::NEXT_OLDER_SP_FROM_FP_OFFSET;
 
         /// Assert that the given `fp` is aligned as expected by the
         /// host platform's implementation of the Cranelift tail-call
         /// ABI.
-        pub use imp::assert_fp_is_aligned;
+        use imp::assert_fp_is_aligned;
 
         /// If we have the above host-specific implementations, we can
         /// implement `Unwind`.

crates/unwinder/src/throw.rs

@@ -19,64 +19,61 @@
 use crate::{Frame, Unwind};
 use core::ops::ControlFlow;
 
-/// Throw action to perform.
-#[derive(Clone, Debug)]
-pub enum ThrowAction {
-    /// Jump to the given handler with the given SP and FP values.
-    Handler {
-        /// Program counter of handler return point.
-        pc: usize,
-        /// Stack pointer to restore before jumping to handler.
-        sp: usize,
-        /// Frame pointer to restore before jumping to handler.
-        fp: usize,
-    },
-    /// No handler found.
-    None,
+/// Description of a frame on the stack that is ready to catch an exception.
+#[derive(Debug)]
+pub struct Handler {
+    /// Program counter of handler return point.
+    pub pc: usize,
+    /// Stack pointer to restore before jumping to handler.
+    pub sp: usize,
+    /// Frame pointer to restore before jumping to handler.
+    pub fp: usize,
 }
 
-/// Implementation of stack-walking to find a handler.
-///
-/// This function searches for a handler in the given range of stack
-/// frames, starting from the throw stub and up to a specified entry
-/// frame.
-///
-/// # Safety
-///
-/// The safety of this function is the same as [`crate::visit_frames`] where the
-/// values passed in configuring the frame pointer walk must be correct and
-/// Wasm-defined for this to not have UB.
-pub unsafe fn compute_throw_action<F: Fn(&Frame) -> Option<(usize, usize)>>(
-    unwind: &dyn Unwind,
-    frame_handler: F,
-    exit_pc: usize,
-    exit_trampoline_frame: usize,
-    entry_frame: usize,
-) -> ThrowAction {
-    // SAFETY: the safety of `visit_frames` relies on the correctness of the
-    // parameters passed in which is forwarded as a contract to this function
-    // tiself.
-    let result = unsafe {
-        crate::stackwalk::visit_frames(
-            unwind,
-            exit_pc,
-            exit_trampoline_frame,
-            entry_frame,
-            |frame| {
-                log::trace!("visit_frame: frame {frame:?}");
-                if let Some((handler_pc, handler_sp)) = frame_handler(&frame) {
-                    return ControlFlow::Break(ThrowAction::Handler {
-                        pc: handler_pc,
-                        sp: handler_sp,
-                        fp: frame.fp(),
-                    });
-                }
-                ControlFlow::Continue(())
-            },
-        )
-    };
-    match result {
-        ControlFlow::Break(action) => action,
-        ControlFlow::Continue(()) => ThrowAction::None,
+impl Handler {
+    /// Implementation of stack-walking to find a handler.
+    ///
+    /// This function searches for a handler in the given range of stack
+    /// frames, starting from the throw stub and up to a specified entry
+    /// frame.
+    ///
+    /// # Safety
+    ///
+    /// The safety of this function is the same as [`crate::visit_frames`] where the
+    /// values passed in configuring the frame pointer walk must be correct and
+    /// Wasm-defined for this to not have UB.
+    pub unsafe fn find<F: Fn(&Frame) -> Option<(usize, usize)>>(
+        unwind: &dyn Unwind,
+        frame_handler: F,
+        exit_pc: usize,
+        exit_trampoline_frame: usize,
+        entry_frame: usize,
+    ) -> Option<Handler> {
+        // SAFETY: the safety of `visit_frames` relies on the correctness of the
+        // parameters passed in which is forwarded as a contract to this function
+        // tiself.
+        let result = unsafe {
+            crate::stackwalk::visit_frames(
+                unwind,
+                exit_pc,
+                exit_trampoline_frame,
+                entry_frame,
+                |frame| {
+                    log::trace!("visit_frame: frame {frame:?}");
+                    if let Some((handler_pc, handler_sp)) = frame_handler(&frame) {
+                        return ControlFlow::Break(Handler {
+                            pc: handler_pc,
+                            sp: handler_sp,
+                            fp: frame.fp(),
+                        });
+                    }
+                    ControlFlow::Continue(())
+                },
+            )
+        };
+        match result {
+            ControlFlow::Break(handler) => Some(handler),
+            ControlFlow::Continue(()) => None,
+        }
     }
 }

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

@@ -9,6 +9,8 @@ use core::ptr::NonNull;
 use pulley_interpreter::interp::{DoneReason, RegType, TrapKind, Val, Vm, XRegVal};
 use pulley_interpreter::{Reg, XReg};
 use wasmtime_environ::{BuiltinFunctionIndex, HostCall, Trap};
+#[cfg(feature = "gc")]
+use wasmtime_unwinder::Handler;
 use wasmtime_unwinder::Unwind;
 
 /// Interpreter state stored within a `Store<T>`.
@@ -526,22 +528,20 @@ impl InterpreterRef<'_> {
     #[cfg(feature = "gc")]
     pub(crate) unsafe fn resume_to_exception_handler(
         mut self,
-        pc: usize,
-        sp: usize,
-        fp: usize,
+        handler: &Handler,
         payload1: usize,
         payload2: usize,
     ) {
         unsafe {
             let vm = self.vm();
             vm[XReg::x0].set_u64(payload1 as u64);
             vm[XReg::x1].set_u64(payload2 as u64);
-            vm[XReg::sp].set_ptr(core::ptr::with_exposed_provenance_mut::<u8>(sp));
-            vm.set_fp(core::ptr::with_exposed_provenance_mut(fp));
+            vm[XReg::sp].set_ptr(core::ptr::with_exposed_provenance_mut::<u8>(handler.sp));
+            vm.set_fp(core::ptr::with_exposed_provenance_mut(handler.fp));
         }
         let state = self.vm_state();
         debug_assert!(state.raise.is_none());
-        self.vm_state().raise = Some(Raise::ResumeToExceptionHandler(pc));
+        self.vm_state().raise = Some(Raise::ResumeToExceptionHandler(handler.pc));
     }
 }
 

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

@@ -61,9 +61,7 @@ impl InterpreterRef<'_> {
     #[cfg(feature = "gc")]
     pub(crate) unsafe fn resume_to_exception_handler(
         self,
-        _pc: usize,
-        _sp: usize,
-        _fp: usize,
+        _handler: &wasmtime_unwinder::Handler,
         _payload1: usize,
         _payload2: usize,
     ) {

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

@@ -1,20 +1,18 @@
 //! Exception-throw logic for Wasm exceptions.
 
-use core::ptr::NonNull;
-
-use wasmtime_environ::TagIndex;
-use wasmtime_unwinder::{Frame, ThrowAction};
-
 use super::{VMContext, VMStore};
 use crate::{store::AutoAssertNoGc, vm::Instance};
+use core::ptr::NonNull;
+use wasmtime_environ::TagIndex;
+use wasmtime_unwinder::{Frame, Handler};
 
 /// Compute the target of the pending exception on the store.
 ///
 /// # Safety
 ///
 /// The stored last-exit state in `store` either must be valid, or
 /// must have a zeroed exit FP if no Wasm is on the stack.
-pub unsafe fn compute_throw_action(store: &mut dyn VMStore) -> ThrowAction {
+pub unsafe fn compute_handler(store: &mut dyn VMStore) -> Option<Handler> {
     let mut nogc = AutoAssertNoGc::new(store.store_opaque_mut());
 
     // Get the tag identity relative to the store.
@@ -44,7 +42,7 @@ pub unsafe fn compute_throw_action(store: &mut dyn VMStore) -> ThrowAction {
     // `Func::call` -- then the only possible action we can take is
     // `None` (i.e., no handler, unwind to entry from host).
     if exit_fp == 0 {
-        return ThrowAction::None;
+        return None;
     }
 
     // Walk the stack, looking up the module with each PC, and using
@@ -119,16 +117,7 @@ pub unsafe fn compute_throw_action(store: &mut dyn VMStore) -> ThrowAction {
         None
     };
     let unwinder = nogc.unwinder();
-    let action = unsafe {
-        wasmtime_unwinder::compute_throw_action(
-            unwinder,
-            handler_lookup,
-            exit_pc,
-            exit_fp,
-            entry_fp,
-        )
-    };
-
+    let action = unsafe { Handler::find(unwinder, handler_lookup, exit_pc, exit_fp, entry_fp) };
     log::trace!("throw action: {action:?}");
     action
 }