-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Restore POSIX signal handling on MacOS behind a feature flag #3063
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,7 +53,8 @@ pub unsafe fn platform_init() { | |
} | ||
|
||
// On ARM, handle Unaligned Accesses. | ||
if cfg!(target_arch = "arm") || cfg!(target_os = "freebsd") { | ||
// On Darwin, guard page accesses are raised as SIGBUS. | ||
if cfg!(target_arch = "arm") || cfg!(target_os = "macos") || cfg!(target_os = "freebsd") { | ||
register(&mut PREV_SIGBUS, libc::SIGBUS); | ||
} | ||
} | ||
|
@@ -99,6 +100,42 @@ unsafe extern "C" fn trap_handler( | |
return true; | ||
} | ||
info.capture_backtrace(pc); | ||
// On macOS this is a bit special, unfortunately. If we were to | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code is taken from https://github.com/bytecodealliance/wasmtime/pull/2723/files#diff-560b66f5eac16eef20c78bcaf2c66582dbc6cf76fa1cc72faf6bca7b736cd660L124 after renaming |
||
// `siglongjmp` out of the signal handler that notably does | ||
// *not* reset the sigaltstack state of our signal handler. This | ||
// seems to trick the kernel into thinking that the sigaltstack | ||
// is still in use upon delivery of the next signal, meaning | ||
// that the sigaltstack is not ever used again if we immediately | ||
// call `wasmtime_longjmp` here. | ||
// | ||
// Note that if we use `longjmp` instead of `siglongjmp` then | ||
// the problem is fixed. The problem with that, however, is that | ||
// `setjmp` is much slower than `sigsetjmp` due to the | ||
// preservation of the proceses signal mask. The reason | ||
// `longjmp` appears to work is that it seems to call a function | ||
// (according to published macOS sources) called | ||
// `_sigunaltstack` which updates the kernel to say the | ||
// sigaltstack is no longer in use. We ideally want to call that | ||
// here but I don't think there's a stable way for us to call | ||
// that. | ||
// | ||
// Given all that, on macOS only, we do the next best thing. We | ||
// return from the signal handler after updating the register | ||
// context. This will cause control to return to our shim | ||
// function defined here which will perform the | ||
// `wasmtime_longjmp` (`siglongjmp`) for us. The reason this | ||
// works is that by returning from the signal handler we'll | ||
// trigger all the normal machinery for "the signal handler is | ||
// done running" which will clear the sigaltstack flag and allow | ||
// reusing it for the next signal. Then upon resuming in our custom | ||
// code we blow away the stack anyway with a longjmp. | ||
if cfg!(target_os = "macos") { | ||
unsafe extern "C" fn wasmtime_longjmp_shim(jmp_buf: *const u8) { | ||
wasmtime_longjmp(jmp_buf) | ||
} | ||
set_pc(context, wasmtime_longjmp_shim as usize, jmp_buf as usize); | ||
return true; | ||
} | ||
wasmtime_longjmp(jmp_buf) | ||
}); | ||
|
||
|
@@ -155,6 +192,15 @@ unsafe fn get_pc(cx: *mut libc::c_void, _signum: libc::c_int) -> *const u8 { | |
}; | ||
let cx = &*(cx as *const libc::ucontext_t); | ||
(cx.uc_mcontext.psw.addr - trap_offset) as *const u8 | ||
} else if #[cfg(all(target_os = "macos", target_arch = "x86_64"))] { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code is taken from https://github.com/bytecodealliance/wasmtime/pull/2723/files#diff-560b66f5eac16eef20c78bcaf2c66582dbc6cf76fa1cc72faf6bca7b736cd660L205 without any changes. |
||
let cx = &*(cx as *const libc::ucontext_t); | ||
(*cx.uc_mcontext).__ss.__rip as *const u8 | ||
} else if #[cfg(all(target_os = "macos", target_arch = "x86"))] { | ||
let cx = &*(cx as *const libc::ucontext_t); | ||
(*cx.uc_mcontext).__ss.__eip as *const u8 | ||
} else if #[cfg(all(target_os = "macos", target_arch = "aarch64"))] { | ||
let cx = &*(cx as *const libc::ucontext_t); | ||
(*cx.uc_mcontext).__ss.__pc as *const u8 | ||
} else if #[cfg(all(target_os = "freebsd", target_arch = "x86_64"))] { | ||
let cx = &*(cx as *const libc::ucontext_t); | ||
cx.uc_mcontext.mc_rip as *const u8 | ||
|
@@ -164,6 +210,41 @@ unsafe fn get_pc(cx: *mut libc::c_void, _signum: libc::c_int) -> *const u8 { | |
} | ||
} | ||
|
||
// This is only used on macOS targets for calling an unwinding shim | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code is taken from https://github.com/bytecodealliance/wasmtime/pull/2723/files#diff-560b66f5eac16eef20c78bcaf2c66582dbc6cf76fa1cc72faf6bca7b736cd660L223 without any changes. |
||
// function to ensure that we return from the signal handler. | ||
// | ||
// See more comments above where this is called for what it's doing. | ||
unsafe fn set_pc(cx: *mut libc::c_void, pc: usize, arg1: usize) { | ||
cfg_if::cfg_if! { | ||
if #[cfg(not(target_os = "macos"))] { | ||
drop((cx, pc, arg1)); | ||
unreachable!(); // not used on these platforms | ||
} else if #[cfg(target_arch = "x86_64")] { | ||
let cx = &mut *(cx as *mut libc::ucontext_t); | ||
(*cx.uc_mcontext).__ss.__rip = pc as u64; | ||
(*cx.uc_mcontext).__ss.__rdi = arg1 as u64; | ||
// We're simulating a "pseudo-call" so we need to ensure | ||
// stack alignment is properly respected, notably that on a | ||
// `call` instruction the stack is 8/16-byte aligned, then | ||
// the function adjusts itself to be 16-byte aligned. | ||
// | ||
// Most of the time the stack pointer is 16-byte aligned at | ||
// the time of the trap but for more robust-ness with JIT | ||
// code where it may ud2 in a prologue check before the | ||
// stack is aligned we double-check here. | ||
if (*cx.uc_mcontext).__ss.__rsp % 16 == 0 { | ||
(*cx.uc_mcontext).__ss.__rsp -= 8; | ||
} | ||
} else if #[cfg(target_arch = "aarch64")] { | ||
let cx = &mut *(cx as *mut libc::ucontext_t); | ||
(*cx.uc_mcontext).__ss.__pc = pc as u64; | ||
(*cx.uc_mcontext).__ss.__x[0] = arg1 as u64; | ||
} else { | ||
compile_error!("unsupported macos target architecture"); | ||
} | ||
} | ||
} | ||
|
||
/// A function for registering a custom alternate signal stack (sigaltstack). | ||
/// | ||
/// Rust's libstd installs an alternate stack with size `SIGSTKSZ`, which is not | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about the name. Bikeshedding is welcome. Possible alternatives:
macos-signals
,macos-posix-signals
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's probably fine to skip this feature on the
wasmtime-cli
crate since it's mostly thewasmtime
embedding crate that wants this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is useful for conditionally enabling
tests/all/custom_signal_handler.rs
on MacOS and running the tests withcargo test --features=posix-signals-on-macos custom_signal_handler
But indeed it's not needed for production. Should I remove it?