Restore POSIX signal handling on MacOS behind a feature flag #3063
Conversation
As described in Issue #3052, the switch to Mach Exception handling removed `unix::StoreExt` from the public API of crate on MacOS. That is a breaking change and makes it difficult for some application to upgrade to the current stable Wasmtime. As a workaround this PR introduces a feature flag called `posix-signals-on-macos` that restores the old behaviour on MacOS. The flag is disabled by default.
| @@ -90,6 +90,7 @@ wasi-crypto = ["wasmtime-wasi-crypto"] | |||
| wasi-nn = ["wasmtime-wasi-nn"] | |||
| uffd = ["wasmtime/uffd"] | |||
| all-arch = ["wasmtime/all-arch"] | |||
| posix-signals-on-macos = ["wasmtime/posix-signals-on-macos"] | |||
There was a problem hiding this comment.
Not sure about the name. Bikeshedding is welcome. Possible alternatives: macos-signals, macos-posix-signals.
There was a problem hiding this comment.
I think it's probably fine to skip this feature on the wasmtime-cli crate since it's mostly the wasmtime embedding crate that wants this.
There was a problem hiding this comment.
It is useful for conditionally enabling tests/all/custom_signal_handler.rs on MacOS and running the tests with cargo test --features=posix-signals-on-macos custom_signal_handler
But indeed it's not needed for production. Should I remove it?
| @@ -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.
This code is taken from https://github.com/bytecodealliance/wasmtime/pull/2723/files#diff-560b66f5eac16eef20c78bcaf2c66582dbc6cf76fa1cc72faf6bca7b736cd660L124 after renaming Unwind to wasmtime_longjmp. The comment is also adjusted accordingly.
| @@ -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.
This code is taken from https://github.com/bytecodealliance/wasmtime/pull/2723/files#diff-560b66f5eac16eef20c78bcaf2c66582dbc6cf76fa1cc72faf6bca7b736cd660L205 without any changes.
| @@ -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.
This code is taken from https://github.com/bytecodealliance/wasmtime/pull/2723/files#diff-560b66f5eac16eef20c78bcaf2c66582dbc6cf76fa1cc72faf6bca7b736cd660L223 without any changes.
|
@alexcrichton: please take a look |
github-actions
bot
added
the
wasmtime:api
Subscribe to Label Actioncc @peterhuene
This issue or pull request has been labeled: "wasmtime:api"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
| @@ -90,6 +90,7 @@ wasi-crypto = ["wasmtime-wasi-crypto"] | |||
| wasi-nn = ["wasmtime-wasi-nn"] | |||
| uffd = ["wasmtime/uffd"] | |||
| all-arch = ["wasmtime/all-arch"] | |||
| posix-signals-on-macos = ["wasmtime/posix-signals-on-macos"] | |||
There was a problem hiding this comment.
I think it's probably fine to skip this feature on the wasmtime-cli crate since it's mostly the wasmtime embedding crate that wants this.
Thanks! I agree. We will also keep an eye on it on our side and let you know if it breaks. |
|
That all sounds reasonable to me, thanks again for the PR! |
This patch is taken from bytecodealliance#3079 which in turn a cherry-pick of bytecodealliance#3063 adjusted for v0.26: bytecodealliance#3063 - Restore POSIX signal handling on MacOS behind a feature flag As described in Issue bytecodealliance#3052, the switch to Mach Exception handling removed unix::StoreExt from the public API of crate on MacOS. That is a breaking change and makes it difficult for some application to upgrade to the current stable Wasmtime. As a workaround this PR introduces a feature flag called posix-signals-on-macos that restores the old behaviour on MacOS. The flag is disabled by default. - Fix test guard - Fix formatting in the test
This patch is taken from bytecodealliance#3063 adjusted for v0.27: - Restore POSIX signal handling on MacOS behind a feature flag As described in Issue bytecodealliance#3052, the switch to Mach Exception handling removed unix::StoreExt from the public API of crate on MacOS. That is a breaking change and makes it difficult for some application to upgrade to the current stable Wasmtime. As a workaround this PR introduces a feature flag called posix-signals-on-macos that restores the old behaviour on MacOS. The flag is disabled by default. - Fix test guard - Fix formatting in the test
As described in Issue #3052, the switch to Mach Exception handling
removed
unix::StoreExtfrom the public API of the crate on MacOS.That is a breaking change and makes it difficult for some
application to upgrade to the current stable Wasmtime.
As a workaround this PR introduces a feature flag called
posix-signals-on-macosthat when enabled restores the old behaviouron MacOS. The flag is disabled by default.
The only non-trivial changes in this PR are in traphandlers/unix.rs that
restore MacOS-related code removed from traphandlers.rs in PR #2723.