diff --git a/crates/wasmtime/build.rs b/crates/wasmtime/build.rs index 752cab2a94eb..78ad92f4443b 100644 --- a/crates/wasmtime/build.rs +++ b/crates/wasmtime/build.rs @@ -36,6 +36,14 @@ fn build_c_helpers() { build.define("FEATURE_DEBUG_BUILTINS", None); } + // On MinGW targets work around a bug in the MinGW compiler described at + // https://github.com/bytecodealliance/wasmtime/pull/9688#issuecomment-2573367719 + 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) {