Skip to content

Commit 789f037

Browse files
authored
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
1 parent 3a14fa3 commit 789f037

File tree

7 files changed

+135
-160
lines changed

7 files changed

+135
-160
lines changed

cranelift/filetests/src/function_runner.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -675,17 +675,15 @@ extern "C-unwind" fn __cranelift_throw(
675675
})
676676
};
677677
unsafe {
678-
match wasmtime_unwinder::compute_throw_action(
678+
match wasmtime_unwinder::Handler::find(
679679
&unwind_host,
680680
frame_handler,
681681
exit_pc,
682682
exit_fp,
683683
entry_fp,
684684
) {
685-
wasmtime_unwinder::ThrowAction::Handler { pc, sp, fp } => {
686-
wasmtime_unwinder::resume_to_exception_handler(pc, sp, fp, payload1, payload2);
687-
}
688-
wasmtime_unwinder::ThrowAction::None => {
685+
Some(handler) => handler.resume(payload1, payload2),
686+
None => {
689687
panic!("Expected a handler to exit for throw of tag {tag} at pc {exit_pc:x}");
690688
}
691689
}

crates/unwinder/src/arch/mod.rs

Lines changed: 51 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -40,57 +40,57 @@ cfg_if::cfg_if! {
4040
/// the frame is later popped.
4141
pub use imp::get_stack_pointer;
4242

43-
/// Resume execution at the given PC, SP, and FP, with the given
44-
/// payload values, according to the tail-call ABI's exception
45-
/// scheme. Note that this scheme does not restore any other
46-
/// registers, so the given state is all that we need.
47-
///
48-
/// # Safety
49-
///
50-
/// This method requires:
51-
///
52-
/// - the `sp` and `fp` to correspond to an active stack frame
53-
/// (above the current function), in code using Cranelift's
54-
/// `tail` calling convention.
55-
///
56-
/// - The `pc` to correspond to a `try_call` handler
57-
/// destination, as emitted in Cranelift metadata, or
58-
/// otherwise a target that is expecting the tail-call ABI's
59-
/// exception ABI.
60-
///
61-
/// - The Rust frames between the unwind destination and this
62-
/// frame to be unwind-safe: that is, they cannot have `Drop`
63-
/// handlers for which safety requires that they run.
64-
pub unsafe fn resume_to_exception_handler(
65-
pc: usize,
66-
sp: usize,
67-
fp: usize,
68-
payload1: usize,
69-
payload2: usize,
70-
) -> ! {
71-
// Without this ASAN seems to nondeterministically trigger an
72-
// internal assertion when running tests with threads. Not entirely
73-
// clear what's going on here but it seems related to the fact that
74-
// there's Rust code on the stack which is never cleaned up due to
75-
// the jump out of `imp::resume_to_exception_handler`.
76-
//
77-
// This function is documented as something that should be called to
78-
// clean up the entire thread's shadow memory and stack which isn't
79-
// exactly what we want but this at least seems to resolve ASAN
80-
// issues for now. Probably a heavy hammer but better than false
81-
// positives I suppose...
82-
#[cfg(asan)]
83-
{
84-
unsafe extern "C" {
85-
fn __asan_handle_no_return();
43+
impl crate::Handler {
44+
/// Resume execution at the given PC, SP, and FP, with the given
45+
/// payload values, according to the tail-call ABI's exception
46+
/// scheme. Note that this scheme does not restore any other
47+
/// registers, so the given state is all that we need.
48+
///
49+
/// # Safety
50+
///
51+
/// This method requires:
52+
///
53+
/// - the `sp` and `fp` to correspond to an active stack frame
54+
/// (above the current function), in code using Cranelift's
55+
/// `tail` calling convention.
56+
///
57+
/// - The `pc` to correspond to a `try_call` handler
58+
/// destination, as emitted in Cranelift metadata, or
59+
/// otherwise a target that is expecting the tail-call ABI's
60+
/// exception ABI.
61+
///
62+
/// - The Rust frames between the unwind destination and this
63+
/// frame to be unwind-safe: that is, they cannot have `Drop`
64+
/// handlers for which safety requires that they run.
65+
pub unsafe fn resume(
66+
&self,
67+
payload1: usize,
68+
payload2: usize,
69+
) -> ! {
70+
// Without this ASAN seems to nondeterministically trigger an
71+
// internal assertion when running tests with threads. Not entirely
72+
// clear what's going on here but it seems related to the fact that
73+
// there's Rust code on the stack which is never cleaned up due to
74+
// the jump out of `imp::resume_to_exception_handler`.
75+
//
76+
// This function is documented as something that should be called to
77+
// clean up the entire thread's shadow memory and stack which isn't
78+
// exactly what we want but this at least seems to resolve ASAN
79+
// issues for now. Probably a heavy hammer but better than false
80+
// positives I suppose...
81+
#[cfg(asan)]
82+
{
83+
unsafe extern "C" {
84+
fn __asan_handle_no_return();
85+
}
86+
unsafe {
87+
__asan_handle_no_return();
88+
}
8689
}
8790
unsafe {
88-
__asan_handle_no_return();
91+
imp::resume_to_exception_handler(self.pc, self.sp, self.fp, payload1, payload2)
8992
}
9093
}
91-
unsafe {
92-
imp::resume_to_exception_handler(pc, sp, fp, payload1, payload2)
93-
}
9494
}
9595

9696
/// Get the return address in the function at the next-older
@@ -101,21 +101,20 @@ cfg_if::cfg_if! {
101101
/// - Requires that `fp` is a valid frame-pointer value for an
102102
/// active stack frame (above the current function), in code
103103
/// using Cranelift's `tail` calling convention.
104-
pub use imp::get_next_older_pc_from_fp;
105-
104+
use imp::get_next_older_pc_from_fp;
106105

107106
/// The offset of the saved old-FP value in a frame, from the
108107
/// location pointed to by a given FP.
109-
pub const NEXT_OLDER_FP_FROM_FP_OFFSET: usize = imp::NEXT_OLDER_FP_FROM_FP_OFFSET;
108+
const NEXT_OLDER_FP_FROM_FP_OFFSET: usize = imp::NEXT_OLDER_FP_FROM_FP_OFFSET;
110109

111110
/// The offset of the next older SP value, from the value of a
112111
/// given FP.
113-
pub const NEXT_OLDER_SP_FROM_FP_OFFSET: usize = imp::NEXT_OLDER_SP_FROM_FP_OFFSET;
112+
const NEXT_OLDER_SP_FROM_FP_OFFSET: usize = imp::NEXT_OLDER_SP_FROM_FP_OFFSET;
114113

115114
/// Assert that the given `fp` is aligned as expected by the
116115
/// host platform's implementation of the Cranelift tail-call
117116
/// ABI.
118-
pub use imp::assert_fp_is_aligned;
117+
use imp::assert_fp_is_aligned;
119118

120119
/// If we have the above host-specific implementations, we can
121120
/// implement `Unwind`.

crates/unwinder/src/throw.rs

Lines changed: 54 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -19,64 +19,61 @@
1919
use crate::{Frame, Unwind};
2020
use core::ops::ControlFlow;
2121

22-
/// Throw action to perform.
23-
#[derive(Clone, Debug)]
24-
pub enum ThrowAction {
25-
/// Jump to the given handler with the given SP and FP values.
26-
Handler {
27-
/// Program counter of handler return point.
28-
pc: usize,
29-
/// Stack pointer to restore before jumping to handler.
30-
sp: usize,
31-
/// Frame pointer to restore before jumping to handler.
32-
fp: usize,
33-
},
34-
/// No handler found.
35-
None,
22+
/// Description of a frame on the stack that is ready to catch an exception.
23+
#[derive(Debug)]
24+
pub struct Handler {
25+
/// Program counter of handler return point.
26+
pub pc: usize,
27+
/// Stack pointer to restore before jumping to handler.
28+
pub sp: usize,
29+
/// Frame pointer to restore before jumping to handler.
30+
pub fp: usize,
3631
}
3732

38-
/// Implementation of stack-walking to find a handler.
39-
///
40-
/// This function searches for a handler in the given range of stack
41-
/// frames, starting from the throw stub and up to a specified entry
42-
/// frame.
43-
///
44-
/// # Safety
45-
///
46-
/// The safety of this function is the same as [`crate::visit_frames`] where the
47-
/// values passed in configuring the frame pointer walk must be correct and
48-
/// Wasm-defined for this to not have UB.
49-
pub unsafe fn compute_throw_action<F: Fn(&Frame) -> Option<(usize, usize)>>(
50-
unwind: &dyn Unwind,
51-
frame_handler: F,
52-
exit_pc: usize,
53-
exit_trampoline_frame: usize,
54-
entry_frame: usize,
55-
) -> ThrowAction {
56-
// SAFETY: the safety of `visit_frames` relies on the correctness of the
57-
// parameters passed in which is forwarded as a contract to this function
58-
// tiself.
59-
let result = unsafe {
60-
crate::stackwalk::visit_frames(
61-
unwind,
62-
exit_pc,
63-
exit_trampoline_frame,
64-
entry_frame,
65-
|frame| {
66-
log::trace!("visit_frame: frame {frame:?}");
67-
if let Some((handler_pc, handler_sp)) = frame_handler(&frame) {
68-
return ControlFlow::Break(ThrowAction::Handler {
69-
pc: handler_pc,
70-
sp: handler_sp,
71-
fp: frame.fp(),
72-
});
73-
}
74-
ControlFlow::Continue(())
75-
},
76-
)
77-
};
78-
match result {
79-
ControlFlow::Break(action) => action,
80-
ControlFlow::Continue(()) => ThrowAction::None,
33+
impl Handler {
34+
/// Implementation of stack-walking to find a handler.
35+
///
36+
/// This function searches for a handler in the given range of stack
37+
/// frames, starting from the throw stub and up to a specified entry
38+
/// frame.
39+
///
40+
/// # Safety
41+
///
42+
/// The safety of this function is the same as [`crate::visit_frames`] where the
43+
/// values passed in configuring the frame pointer walk must be correct and
44+
/// Wasm-defined for this to not have UB.
45+
pub unsafe fn find<F: Fn(&Frame) -> Option<(usize, usize)>>(
46+
unwind: &dyn Unwind,
47+
frame_handler: F,
48+
exit_pc: usize,
49+
exit_trampoline_frame: usize,
50+
entry_frame: usize,
51+
) -> Option<Handler> {
52+
// SAFETY: the safety of `visit_frames` relies on the correctness of the
53+
// parameters passed in which is forwarded as a contract to this function
54+
// tiself.
55+
let result = unsafe {
56+
crate::stackwalk::visit_frames(
57+
unwind,
58+
exit_pc,
59+
exit_trampoline_frame,
60+
entry_frame,
61+
|frame| {
62+
log::trace!("visit_frame: frame {frame:?}");
63+
if let Some((handler_pc, handler_sp)) = frame_handler(&frame) {
64+
return ControlFlow::Break(Handler {
65+
pc: handler_pc,
66+
sp: handler_sp,
67+
fp: frame.fp(),
68+
});
69+
}
70+
ControlFlow::Continue(())
71+
},
72+
)
73+
};
74+
match result {
75+
ControlFlow::Break(handler) => Some(handler),
76+
ControlFlow::Continue(()) => None,
77+
}
8178
}
8279
}

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ use core::ptr::NonNull;
99
use pulley_interpreter::interp::{DoneReason, RegType, TrapKind, Val, Vm, XRegVal};
1010
use pulley_interpreter::{Reg, XReg};
1111
use wasmtime_environ::{BuiltinFunctionIndex, HostCall, Trap};
12+
#[cfg(feature = "gc")]
13+
use wasmtime_unwinder::Handler;
1214
use wasmtime_unwinder::Unwind;
1315

1416
/// Interpreter state stored within a `Store<T>`.
@@ -526,22 +528,20 @@ impl InterpreterRef<'_> {
526528
#[cfg(feature = "gc")]
527529
pub(crate) unsafe fn resume_to_exception_handler(
528530
mut self,
529-
pc: usize,
530-
sp: usize,
531-
fp: usize,
531+
handler: &Handler,
532532
payload1: usize,
533533
payload2: usize,
534534
) {
535535
unsafe {
536536
let vm = self.vm();
537537
vm[XReg::x0].set_u64(payload1 as u64);
538538
vm[XReg::x1].set_u64(payload2 as u64);
539-
vm[XReg::sp].set_ptr(core::ptr::with_exposed_provenance_mut::<u8>(sp));
540-
vm.set_fp(core::ptr::with_exposed_provenance_mut(fp));
539+
vm[XReg::sp].set_ptr(core::ptr::with_exposed_provenance_mut::<u8>(handler.sp));
540+
vm.set_fp(core::ptr::with_exposed_provenance_mut(handler.fp));
541541
}
542542
let state = self.vm_state();
543543
debug_assert!(state.raise.is_none());
544-
self.vm_state().raise = Some(Raise::ResumeToExceptionHandler(pc));
544+
self.vm_state().raise = Some(Raise::ResumeToExceptionHandler(handler.pc));
545545
}
546546
}
547547

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,7 @@ impl InterpreterRef<'_> {
6161
#[cfg(feature = "gc")]
6262
pub(crate) unsafe fn resume_to_exception_handler(
6363
self,
64-
_pc: usize,
65-
_sp: usize,
66-
_fp: usize,
64+
_handler: &wasmtime_unwinder::Handler,
6765
_payload1: usize,
6866
_payload2: usize,
6967
) {

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

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,18 @@
11
//! Exception-throw logic for Wasm exceptions.
22
3-
use core::ptr::NonNull;
4-
5-
use wasmtime_environ::TagIndex;
6-
use wasmtime_unwinder::{Frame, ThrowAction};
7-
83
use super::{VMContext, VMStore};
94
use crate::{store::AutoAssertNoGc, vm::Instance};
5+
use core::ptr::NonNull;
6+
use wasmtime_environ::TagIndex;
7+
use wasmtime_unwinder::{Frame, Handler};
108

119
/// Compute the target of the pending exception on the store.
1210
///
1311
/// # Safety
1412
///
1513
/// The stored last-exit state in `store` either must be valid, or
1614
/// must have a zeroed exit FP if no Wasm is on the stack.
17-
pub unsafe fn compute_throw_action(store: &mut dyn VMStore) -> ThrowAction {
15+
pub unsafe fn compute_handler(store: &mut dyn VMStore) -> Option<Handler> {
1816
let mut nogc = AutoAssertNoGc::new(store.store_opaque_mut());
1917

2018
// Get the tag identity relative to the store.
@@ -44,7 +42,7 @@ pub unsafe fn compute_throw_action(store: &mut dyn VMStore) -> ThrowAction {
4442
// `Func::call` -- then the only possible action we can take is
4543
// `None` (i.e., no handler, unwind to entry from host).
4644
if exit_fp == 0 {
47-
return ThrowAction::None;
45+
return None;
4846
}
4947

5048
// 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 {
119117
None
120118
};
121119
let unwinder = nogc.unwinder();
122-
let action = unsafe {
123-
wasmtime_unwinder::compute_throw_action(
124-
unwinder,
125-
handler_lookup,
126-
exit_pc,
127-
exit_fp,
128-
entry_fp,
129-
)
130-
};
131-
120+
let action = unsafe { Handler::find(unwinder, handler_lookup, exit_pc, exit_fp, entry_fp) };
132121
log::trace!("throw action: {action:?}");
133122
action
134123
}

0 commit comments

Comments
 (0)