Remove handling_trap variable (#4963)

This historically was used to guard against recursive faults but
later refactorings have made this variable somewhat obsolete. The code
that it still protects is not the "meat" of trap handling. Instead the
`jmp_buf_if_trap` is changed to be more like "take" so once a "take"
succeeds it won't be able to recursively call any more "meat".

Overall this shouldn't affect anything, it's just a small internal
cleanup.
This commit is contained in:
Alex Crichton
2022-09-26 17:41:47 -05:00
committed by GitHub
parent 7b311004b5
commit f12ef84cdc
3 changed files with 4 additions and 16 deletions

View File

@@ -216,7 +216,6 @@ mod call_thread_state {
pub struct CallThreadState { pub struct CallThreadState {
pub(super) unwind: UnsafeCell<MaybeUninit<(UnwindReason, Option<Backtrace>)>>, pub(super) unwind: UnsafeCell<MaybeUninit<(UnwindReason, Option<Backtrace>)>>,
pub(super) jmp_buf: Cell<*const u8>, pub(super) jmp_buf: Cell<*const u8>,
pub(super) handling_trap: Cell<bool>,
pub(super) signal_handler: Option<*const SignalHandler<'static>>, pub(super) signal_handler: Option<*const SignalHandler<'static>>,
pub(super) capture_backtrace: bool, pub(super) capture_backtrace: bool,
@@ -246,7 +245,6 @@ mod call_thread_state {
CallThreadState { CallThreadState {
unwind: UnsafeCell::new(MaybeUninit::uninit()), unwind: UnsafeCell::new(MaybeUninit::uninit()),
jmp_buf: Cell::new(ptr::null()), jmp_buf: Cell::new(ptr::null()),
handling_trap: Cell::new(false),
signal_handler, signal_handler,
capture_backtrace, capture_backtrace,
limits, limits,
@@ -406,21 +404,11 @@ impl CallThreadState {
/// * a different pointer - a jmp_buf buffer to longjmp to, meaning that /// * a different pointer - a jmp_buf buffer to longjmp to, meaning that
/// the wasm trap was succesfully handled. /// the wasm trap was succesfully handled.
#[cfg_attr(target_os = "macos", allow(dead_code))] // macOS is more raw and doesn't use this #[cfg_attr(target_os = "macos", allow(dead_code))] // macOS is more raw and doesn't use this
fn jmp_buf_if_trap( fn take_jmp_buf_if_trap(
&self, &self,
pc: *const u8, pc: *const u8,
call_handler: impl Fn(&SignalHandler) -> bool, call_handler: impl Fn(&SignalHandler) -> bool,
) -> *const u8 { ) -> *const u8 {
// If we hit a fault while handling a previous trap, that's quite bad,
// so bail out and let the system handle this recursive segfault.
//
// Otherwise flag ourselves as handling a trap, do the trap handling,
// and reset our trap handling flag.
if self.handling_trap.replace(true) {
return ptr::null();
}
let _reset = ResetCell(&self.handling_trap, false);
// If we haven't even started to handle traps yet, bail out. // If we haven't even started to handle traps yet, bail out.
if self.jmp_buf.get().is_null() { if self.jmp_buf.get().is_null() {
return ptr::null(); return ptr::null();
@@ -442,7 +430,7 @@ impl CallThreadState {
// If all that passed then this is indeed a wasm trap, so return the // If all that passed then this is indeed a wasm trap, so return the
// `jmp_buf` passed to `wasmtime_longjmp` to resume. // `jmp_buf` passed to `wasmtime_longjmp` to resume.
self.jmp_buf.get() self.jmp_buf.replace(ptr::null())
} }
fn set_jit_trap(&self, pc: *const u8, fp: usize) { fn set_jit_trap(&self, pc: *const u8, fp: usize) {

View File

@@ -87,7 +87,7 @@ unsafe extern "C" fn trap_handler(
// handling, and reset our trap handling flag. Then we figure // handling, and reset our trap handling flag. Then we figure
// out what to do based on the result of the trap handling. // out what to do based on the result of the trap handling.
let (pc, fp) = get_pc_and_fp(context, signum); let (pc, fp) = get_pc_and_fp(context, signum);
let jmp_buf = info.jmp_buf_if_trap(pc, |handler| handler(signum, siginfo, context)); let jmp_buf = info.take_jmp_buf_if_trap(pc, |handler| handler(signum, siginfo, context));
// Figure out what to do based on the result of this handling of // Figure out what to do based on the result of this handling of
// the trap. Note that our sentinel value of 1 means that the // the trap. Note that our sentinel value of 1 means that the

View File

@@ -62,7 +62,7 @@ unsafe extern "system" fn exception_handler(exception_info: *mut EXCEPTION_POINT
compile_error!("unsupported platform"); compile_error!("unsupported platform");
} }
} }
let jmp_buf = info.jmp_buf_if_trap(ip, |handler| handler(exception_info)); let jmp_buf = info.take_jmp_buf_if_trap(ip, |handler| handler(exception_info));
if jmp_buf.is_null() { if jmp_buf.is_null() {
ExceptionContinueSearch ExceptionContinueSearch
} else if jmp_buf as usize == 1 { } else if jmp_buf as usize == 1 {