diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 7bbcfba40b..aa7f6ef371 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1036,8 +1036,25 @@ jobs: # (e.g. functional_batch2_regressions) link the prebuilt archive # directly and fail with "Could not find libperry_runtime.a" if the # cached staticlib was invalidated. Build them explicitly. + # #8479: build these with `--release`. The `panic` strategy is a + # PROFILE-level setting, and only `release`/`dist`/`perry-dev` set + # `panic = "abort"` — a bare `cargo build` produces a DEBUG, + # `panic = "unwind"` runtime. Under `panic = "unwind"` rustc plants + # an RFC-2945 abort guard in every `extern "C"` helper, which a JS + # throw crossing that helper trips ("panic in a function that + # cannot unwind"). So the child binaries were linking a runtime + # whose unwind semantics are the OPPOSITE of what Perry ships, and + # the two throw-transport canaries could not both pass: #8416 + # papered over it with `extern "C-unwind"` (correct for the debug + # runtime, WRONG for the shipped one), #8464 generalised that and + # cost +20 gap crashes, and #8480 added more. Verified: against a + # `panic = "abort"` runtime, with those conversions removed, BOTH + # `tier1_every_ffi_type_against_test_dylib` and + # `function_apply_with_runtime_args_defers_to_a_located_aot_error` + # pass. if printf '%s\n' "$scope" | grep -qE '^(perry|perry-stdlib)$'; then - cargo build -p perry-runtime -p perry-stdlib -p perry-runtime-static -p perry-stdlib-static + cargo build --release -p perry-runtime -p perry-stdlib -p perry-runtime-static -p perry-stdlib-static + export PERRY_RUNTIME_DIR="$PWD/target/release" fi find target/debug/deps -maxdepth 1 -type f -perm -111 ! -name '*.so' -delete # Large perry / perry-stdlib integration-test binaries: serialize @@ -1274,9 +1291,15 @@ jobs: # libperry_{runtime,stdlib}.a don't exist unless built explicitly — # and the suites that compile with PERRY_NO_AUTO_OPTIMIZE=1 link them # directly ("Could not find libperry_runtime.a" otherwise). + # #8479: `--release` so the child links a `panic = "abort"` runtime, + # matching what Perry ships. A bare `cargo build` yields a DEBUG, + # `panic = "unwind"` runtime whose `extern "C"` helpers carry + # RFC-2945 abort guards that a JS throw trips — the opposite of the + # shipped semantics. See the longer note in `cargo-test`. if printf '%s\n' "$SUITES" | grep -qE '^(perry|perry-stdlib) '; then - cargo build -p perry-runtime -p perry-stdlib \ + cargo build --release -p perry-runtime -p perry-stdlib \ -p perry-runtime-static -p perry-stdlib-static + export PERRY_RUNTIME_DIR="$PWD/target/release" fi status=0 diff --git a/changelog.d/8480-ffi-thunk-unwind.md b/changelog.d/8480-ffi-thunk-unwind.md deleted file mode 100644 index 3317657ad9..0000000000 --- a/changelog.d/8480-ffi-thunk-unwind.md +++ /dev/null @@ -1 +0,0 @@ -fix(ffi): `bun:ffi`'s per-arity symbol thunks and `close_thunk` are `extern "C-unwind"` (#8479). They call `invoke_from_closure`, which throws the documented "symbol … was called after close()" error; under plain `extern "C"` that unwind hit Rust's abort-on-unwind shim inside the thunk, so a use-after-close aborted the process on Linux instead of throwing — `tier1_every_ffi_type_against_test_dylib` stayed red after #8464, which had converted only the closure-dispatch callers. A call through an `extern "C-unwind"` pointer does not remove the callee's own nounwind guard. diff --git a/changelog.d/8485-remove-c-unwind-guards.md b/changelog.d/8485-remove-c-unwind-guards.md new file mode 100644 index 0000000000..538bda16a3 --- /dev/null +++ b/changelog.d/8485-remove-c-unwind-guards.md @@ -0,0 +1 @@ +fix(runtime): remove the `extern "C-unwind"` guards from the JS throw path (#8479). The runtime is built `panic=abort` and a JS throw travels as a raw Itanium `_Unwind_Exception` that must step *through* runtime frames untouched — the workspace `Cargo.toml` states the requirement outright ("no RFC-2945 abort-on-unwind guards … which a JS throw crossing a helper frame would trip"). Marking a pass-through frame `extern "C-unwind"` in a `panic=abort` crate does not enable unwinding; it makes rustc wrap the call in an abort-on-unwind landing pad, which is that exact guard. #8416 planted the first two (`js_closure_call1`, `js_native_call_value`) and broke `tier1_every_ffi_type_against_test_dylib`; #8480 planted another on the same FFI path; #8464 planted ~40 more and cost +20 gap crashes plus a red `gc-stress` before being reverted in #8484. All four pass-through conversions are back to `extern "C"`; throw *originators* (`js_throw` and friends, which diverge) are untouched. diff --git a/changelog.d/8491-test-runtime-panic-strategy.md b/changelog.d/8491-test-runtime-panic-strategy.md new file mode 100644 index 0000000000..4f51b11c9c --- /dev/null +++ b/changelog.d/8491-test-runtime-panic-strategy.md @@ -0,0 +1 @@ +fix(ci): `cargo-test` and `e2e-scoped` now build the runtime archives with `--release`, so compiled test binaries link a `panic = "abort"` runtime like the one Perry ships (#8479). `panic` is a profile-level setting and only `release`/`dist`/`perry-dev` set `abort`; a bare `cargo build` produced a debug, `panic = "unwind"` runtime whose `extern "C"` helpers carry RFC-2945 abort guards that a JS throw trips. The test environment therefore had the *opposite* unwind semantics from production, which is why the two throw-transport canaries could never both pass and why #8416, #8464 and #8480 each fixed one by breaking the other. Paired with removing those `extern "C-unwind"` conversions (#8488): against a `panic = "abort"` runtime, both `tier1_every_ffi_type_against_test_dylib` and `function_apply_with_runtime_args_defers_to_a_located_aot_error` pass. diff --git a/changelog.d/8492-function-apply-release-runtime.md b/changelog.d/8492-function-apply-release-runtime.md new file mode 100644 index 0000000000..1d93f59750 --- /dev/null +++ b/changelog.d/8492-function-apply-release-runtime.md @@ -0,0 +1 @@ +fix(test): `function_apply_dynamic_args_eval_surface` builds and links a `--release` runtime archive (#8479). It hardcoded `target/debug` and passed it via `PERRY_RUNTIME_DIR`, so it always linked a `panic = "unwind"` runtime — a configuration Perry never ships — where rustc plants an RFC-2945 abort guard in every `extern "C"` helper and a JS throw crossing one aborts. That made this test and `bun_ffi_stage1`'s use-after-close case mutually unsatisfiable and drove three successive "fixes" (#8416, #8464, #8480) that each repaired one by breaking the other. diff --git a/crates/perry-runtime/src/bun_ffi/dlopen.rs b/crates/perry-runtime/src/bun_ffi/dlopen.rs index c6eec971e6..4eaa9b87c7 100644 --- a/crates/perry-runtime/src/bun_ffi/dlopen.rs +++ b/crates/perry-runtime/src/bun_ffi/dlopen.rs @@ -4,7 +4,7 @@ //! (`LIBS` / `SYMS`); a symbol's JS call stub is a runtime closure whose //! single capture is its `SYMS` index, so the shared per-arity thunks //! (`sym_thunk_0..=16`) stay signature-compatible with the closure call -//! ABI (`extern "C-unwind" fn(*const ClosureHeader, f64 × arity) -> f64`, +//! ABI (`extern "C" fn(*const ClosureHeader, f64 × arity) -> f64`, //! arity-padded by `closure/dispatch` via `js_register_closure_arity`). //! //! `close()` calls `dlclose` and poisons the library's symbols: later @@ -173,15 +173,7 @@ unsafe fn invoke_from_closure(closure: *const ClosureHeader, js_args: &[f64]) -> macro_rules! sym_thunk { ($name:ident $(, $a:ident)*) => { - // #8478: `C-unwind`, NOT `C`. `invoke_from_closure` throws a JS - // error for a symbol called after `close()` (and for an unknown - // stub). Under plain `extern "C"` that unwind hits Rust's - // abort-on-unwind shim inside the thunk itself — "panic in a - // function that cannot unwind" — so the documented use-after-close - // behaviour aborted the process on Linux instead of throwing. - // #8464 made the closure-dispatch CALLERS unwind-capable; these - // thunks are the CALLEES it could not see. - extern "C-unwind" fn $name(closure: *const ClosureHeader $(, $a: f64)*) -> f64 { + extern "C" fn $name(closure: *const ClosureHeader $(, $a: f64)*) -> f64 { let args = [$($a),*]; unsafe { invoke_from_closure(closure, &args) } } @@ -308,10 +300,7 @@ fn sym_thunk_for(arity: usize) -> *const u8 { } } -// #8478: `C-unwind` for the same reason as the symbol thunks — this body -// takes a `Mutex` whose `unwrap()` panics on poison, and a panic crossing a -// plain `extern "C"` frame aborts rather than propagating. -extern "C-unwind" fn close_thunk(closure: *const ClosureHeader) -> f64 { +extern "C" fn close_thunk(closure: *const ClosureHeader) -> f64 { let lib_index = crate::closure::js_closure_get_capture_bits(closure, 0) as usize; let mut libs = LIBS.lock().unwrap(); if let Some(rec) = libs.get_mut(lib_index) { diff --git a/crates/perry-runtime/src/closure/dispatch/calln.rs b/crates/perry-runtime/src/closure/dispatch/calln.rs index 22a50b8f6d..6defa4b45f 100644 --- a/crates/perry-runtime/src/closure/dispatch/calln.rs +++ b/crates/perry-runtime/src/closure/dispatch/calln.rs @@ -37,7 +37,16 @@ pub extern "C" fn js_closure_call0(closure: *const ClosureHeader) -> f64 { #[no_mangle] // The one-argument value-call path can run arbitrary generated code and must // let a JS exception unwind to the generated caller's catch landing pad. -pub extern "C-unwind" fn js_closure_call1(closure: *const ClosureHeader, arg0: f64) -> f64 { +// #8479: NOT `C-unwind`. The runtime is built `panic=abort` and JS throws +// travel as a raw Itanium `_Unwind_Exception` that must step THROUGH these +// frames untouched (see `crate::eh` and the panic=abort rationale in the +// workspace Cargo.toml). Marking a frame `extern "C-unwind"` in a +// panic=abort crate does not enable that — it makes rustc wrap the call in +// an abort-on-unwind landing pad, which is exactly the RFC-2945 guard a JS +// throw trips ("panic in a function that cannot unwind"). #8416 introduced +// the first two such guards here; #8464 added ~40 more and measurably +// regressed main (+20 gap crashes, gc-stress) before being reverted. +pub extern "C" fn js_closure_call1(closure: *const ClosureHeader, arg0: f64) -> f64 { let func_ptr = get_valid_func_ptr(closure); if func_ptr.is_null() { return dispatch_proxy_callee_or_throw(closure, &[arg0]); @@ -52,7 +61,7 @@ pub extern "C-unwind" fn js_closure_call1(closure: *const ClosureHeader, arg0: f dispatch_with_arity(closure, func_ptr, &[arg0], declared) }, _ => { - let func: extern "C-unwind" fn(*const ClosureHeader, f64) -> f64 = + let func: extern "C" fn(*const ClosureHeader, f64) -> f64 = unsafe { std::mem::transmute(func_ptr) }; func(closure, arg0) } @@ -63,11 +72,7 @@ pub extern "C-unwind" fn js_closure_call1(closure: *const ClosureHeader, arg0: f #[no_mangle] // A dynamically-dispatched closure can throw into a generated caller's catch // landing pad; this bridge is on Next's loadManifest/readFileSync path. -pub extern "C-unwind" fn js_closure_call2( - closure: *const ClosureHeader, - arg0: f64, - arg1: f64, -) -> f64 { +pub extern "C" fn js_closure_call2(closure: *const ClosureHeader, arg0: f64, arg1: f64) -> f64 { let func_ptr = get_valid_func_ptr(closure); if func_ptr.is_null() { return dispatch_proxy_callee_or_throw(closure, &[arg0, arg1]); diff --git a/crates/perry-runtime/src/closure/dispatch/value_call.rs b/crates/perry-runtime/src/closure/dispatch/value_call.rs index 111450546b..30f6cc17ee 100644 --- a/crates/perry-runtime/src/closure/dispatch/value_call.rs +++ b/crates/perry-runtime/src/closure/dispatch/value_call.rs @@ -18,7 +18,16 @@ use super::*; // landing pad. Keep this value-call bridge unwind-capable just like // `js_native_call_method`; otherwise debug/static runtime builds install an // abort-on-unwind guard here and Linux aborts before the landing pad is reached. -pub unsafe extern "C-unwind" fn js_native_call_value( +// #8479: NOT `C-unwind`. The runtime is built `panic=abort` and JS throws +// travel as a raw Itanium `_Unwind_Exception` that must step THROUGH these +// frames untouched (see `crate::eh` and the panic=abort rationale in the +// workspace Cargo.toml). Marking a frame `extern "C-unwind"` in a +// panic=abort crate does not enable that — it makes rustc wrap the call in +// an abort-on-unwind landing pad, which is exactly the RFC-2945 guard a JS +// throw trips ("panic in a function that cannot unwind"). #8416 introduced +// the first two such guards here; #8464 added ~40 more and measurably +// regressed main (+20 gap crashes, gc-stress) before being reverted. +pub unsafe extern "C" fn js_native_call_value( func_value: f64, args_ptr: *const f64, args_len: usize, diff --git a/crates/perry/tests/bun_ffi_stage1.rs b/crates/perry/tests/bun_ffi_stage1.rs index 489145985b..f1d047f769 100644 --- a/crates/perry/tests/bun_ffi_stage1.rs +++ b/crates/perry/tests/bun_ffi_stage1.rs @@ -332,6 +332,15 @@ console.log("closed-throws:", closedError.includes("close()")); console.log("TIER1-DONE"); "#; +/// #8479: this test is the canary for the JS-throw transport. Its +/// use-after-close case throws from inside an FFI symbol stub, so the throw +/// must cross the runtime's dispatch frames. The runtime is `panic=abort` +/// and that throw is a raw Itanium unwind that has to pass THROUGH those +/// frames — so any `extern "C-unwind"` on a pass-through frame installs an +/// RFC-2945 abort guard and turns this into "panic in a function that cannot +/// unwind" (Linux only; macOS never reproduces it). Keep this file in the +/// diff of any change to that path so `e2e-scoped` actually runs the suite: +/// the job SKIPS silently and reports green when nothing in scope changed. #[test] fn tier1_every_ffi_type_against_test_dylib() { let dir = tempfile::tempdir().expect("tempdir"); diff --git a/crates/perry/tests/function_apply_dynamic_args_eval_surface.rs b/crates/perry/tests/function_apply_dynamic_args_eval_surface.rs index a92c0ef927..f08ca5589c 100644 --- a/crates/perry/tests/function_apply_dynamic_args_eval_surface.rs +++ b/crates/perry/tests/function_apply_dynamic_args_eval_surface.rs @@ -14,6 +14,14 @@ //! //! Literal-source forms must keep working: those are const-folded and compiled AOT. +//! #8479: this suite is the counterpart canary to `bun_ffi_stage1`. #8416 made +//! the deferred error here survive on Linux by marking `js_closure_call1` / +//! `js_native_call_value` `extern "C-unwind"` — which, in a `panic=abort` +//! runtime whose JS throws are raw Itanium unwinds, installs an RFC-2945 +//! abort guard on a frame the throw must pass THROUGH, and that is what broke +//! `tier1_every_ffi_type_against_test_dylib`. Those conversions were undone; +//! keep this file in the diff of any change to that path so `e2e-scoped` +//! actually runs the suite (it skips silently, and reports green, otherwise). use std::path::PathBuf; use std::process::Command; use std::sync::Once; @@ -29,11 +37,19 @@ fn workspace_root() -> PathBuf { .expect("canonicalize workspace root") } -fn target_debug_dir() -> PathBuf { +/// #8479: `release`, not `debug`. `panic` is a PROFILE-level setting and only +/// `release`/`dist`/`perry-dev` set `panic = "abort"`. A debug archive is +/// `panic = "unwind"`, and under that strategy rustc plants an RFC-2945 +/// abort-on-unwind guard in every `extern "C"` helper — which a JS throw +/// crossing that helper trips ("panic in a function that cannot unwind"). +/// Perry never ships such a runtime, so linking one here tested unwind +/// semantics that do not exist in production and made this test and +/// `bun_ffi_stage1`'s use-after-close case mutually unsatisfiable. +fn target_runtime_dir() -> PathBuf { std::env::var_os("CARGO_TARGET_DIR") .map(PathBuf::from) .unwrap_or_else(|| workspace_root().join("target")) - .join("debug") + .join("release") } /// Build `libperry_{runtime,stdlib}.a` once so the compiled binaries can link. @@ -47,6 +63,9 @@ fn ensure_runtime_archive() { let build = Command::new(cargo) .current_dir(workspace_root()) .arg("build") + // #8479: must match `target_runtime_dir()` — a debug archive has + // the wrong panic strategy (see that function's comment). + .arg("--release") .arg("-p") .arg("perry-runtime-static") .arg("-p") @@ -64,7 +83,7 @@ fn ensure_runtime_archive() { fn runtime_dir() -> PathBuf { ensure_runtime_archive(); - target_debug_dir() + target_runtime_dir() } /// Literal-source `Function` / `Function.apply` / `Function.call` are const-folded and