From c988c09bd84334de228895a0f36a57488b3648e7 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 6 Jan 2025 07:58:21 -0800 Subject: [PATCH] Test out `-D__USE_MINGW_SETJMP_NON_SEH` With respect to #9688 prtest:full --- crates/wasmtime/build.rs | 6 +++ crates/wasmtime/src/runtime/vm/helpers.c | 60 ++---------------------- 2 files changed, 11 insertions(+), 55 deletions(-) diff --git a/crates/wasmtime/build.rs b/crates/wasmtime/build.rs index 752cab2a94eb..4d502cb5edd9 100644 --- a/crates/wasmtime/build.rs +++ b/crates/wasmtime/build.rs @@ -36,6 +36,12 @@ fn build_c_helpers() { build.define("FEATURE_DEBUG_BUILTINS", None); } + if std::env::var("CARGO_CFG_WINDOWS").is_ok() + && std::env::var("CARGO_CFG_TARGET_ENV").ok().as_deref() == Some("gnu") + { + build.define("__USE_MINGW_SETJMP_NON_SEH", None); + } + println!("cargo:rerun-if-changed=src/runtime/vm/helpers.c"); build.file("src/runtime/vm/helpers.c"); build.compile("wasmtime-helpers"); diff --git a/crates/wasmtime/src/runtime/vm/helpers.c b/crates/wasmtime/src/runtime/vm/helpers.c index ace0cf6f73dd..2ecaff40b35c 100644 --- a/crates/wasmtime/src/runtime/vm/helpers.c +++ b/crates/wasmtime/src/runtime/vm/helpers.c @@ -61,65 +61,15 @@ typedef sigjmp_buf platform_jmp_buf; #define CONCAT(a, b) CONCAT2(a, b) #define VERSIONED_SYMBOL(a) CONCAT(a, VERSIONED_SUFFIX) -// Define one function here, `wasmtime_setjmp_inverted`, which returns the -// negation of whether the call succeeded. Define then the actual import below -// of `wasmtime_setjmp_*` which returns the negation of this negation which -// means it returns whether the function invocation succeeded or not. -// -// Why in the world would we do this? For now: MinGW. In -// bytecodealliance/wasmtime#9675 that PR was originally failing CI only on -// MinGW and seems to be fixed by this. In that PR the signature of `body` here -// changed from a `void` return to a `bool` returned. That means that the body -// of this function changed from the historical: -// -// body(payload, callee); -// return 1; -// -// to what we actually want: -// -// return body(payload, callee); -// -// For some reason though this causes issues when unwinding via `longjmp` on -// Windows. Tests would exit with the error message: -// -// code 0xc0000028: An invalid or unaligned stack was encountered during an -// unwind operation. (os error 543) -// -// Debugging revealed that if this: -// -// return body(payload, callee); -// -// were written as: -// -// bool ret = body(payload, callee); -// return ret; -// -// then the bug would be "fixed". This "fix" didn't work in release mode -// however, leading to the current fix. For whatever reason it seems that -// unwinding is broken if there's not code between the `body(...)` indirect -// call and the function return. The `!` here below, inverting the return value, -// is the source of that "code". -// -// Ideally this `*_inverted` shim would go away and get past CI. It's unclear -// whether we're dealing with a miscompile in GCC, bad unwinding information -// generated by Cranelift for JIT code, or what. For now "this seems to work" -// but we'll also be in the process of forwarding this to some other Windows -// folks to see better what's going on. -static bool wasmtime_setjmp_inverted(void **buf_storage, - bool (*body)(void *, void *), - void *payload, void *callee) { +bool VERSIONED_SYMBOL(wasmtime_setjmp)(void **buf_storage, + bool (*body)(void *, void *), + void *payload, void *callee) { platform_jmp_buf buf; if (platform_setjmp(buf) != 0) { - return true; + return false; } *buf_storage = &buf; - return !body(payload, callee); -} - -bool VERSIONED_SYMBOL(wasmtime_setjmp)(void **buf_storage, - bool (*body)(void *, void *), - void *payload, void *callee) { - return !wasmtime_setjmp_inverted(buf_storage, body, payload, callee); + return body(payload, callee); } void VERSIONED_SYMBOL(wasmtime_longjmp)(void *JmpBuf) {