From abfee9248dda3dd781fe234176d245378929cc2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 3 Aug 2026 15:19:43 +0200 Subject: [PATCH] fix(build)!: the SHIPPED runtime was built panic=unwind (#7302) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [profile.dist] — what release-packages.yml builds the shipped libperry_{runtime,stdlib}.a with — inherits release but re-declared panic = "unwind", and an explicit re-declaration wins over inherits. #7305 changed only [profile.release], so every local build, every CI job and the whole parity suite were correct while the artifact users install would have aborted on the first JS throw crossing an extern "C" helper with an interior Rust call (RFC 2945 guards): a throwing getter, a JSON.parse error, a throwing map callback. Confirmed from the rustc invocations (cargo build --profile dist -v): before, force-unwind-tables=yes with no -C panic=abort; after, both. Caught before any release was cut. Guarded by crates/perry/src/panic_profile_contract.rs in cargo-test: every profile that builds a shipped runtime must DECLARE panic=abort (inherits is deliberately not accepted as evidence, since a harmless- looking override is the failure mode), plus a test pinning force-unwind-tables in .cargo/config.toml. Both falsified before landing. --- Cargo.toml | 14 ++- changelog.d/7309-dist-profile-panic-abort.md | 32 ++++++ crates/perry/src/main.rs | 1 + crates/perry/src/panic_profile_contract.rs | 110 +++++++++++++++++++ 4 files changed, 156 insertions(+), 1 deletion(-) create mode 100644 changelog.d/7309-dist-profile-panic-abort.md create mode 100644 crates/perry/src/panic_profile_contract.rs diff --git a/Cargo.toml b/Cargo.toml index 981b2e4338..d0ce6d9a07 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -237,7 +237,19 @@ opt-level = 1 inherits = "release" lto = "thin" codegen-units = 1 -panic = "unwind" +# MUST match [profile.release] (#7302). `dist` is what release-packages.yml +# builds the SHIPPED libperry_{runtime,stdlib}.a with, and the exception +# transport requires the unwinder to step runtime Rust frames with +# longjmp-equivalent semantics. Under panic="unwind" rustc plants RFC-2945 +# abort-on-unwind guards in every `extern "C"` helper that contains an +# interior Rust call, so a JS throw crossing such a helper — a throwing +# getter, a JSON.parse error, a throwing map callback — aborts the process +# instead of being caught. This line said "unwind" and so overrode the +# inherited value: it made the shipped runtime, and only the shipped +# runtime, fail that way. Paired with -C force-unwind-tables=yes from +# .cargo/config.toml; the runtime self-checks for the tables on the first +# `try` and aborts loudly if they are missing. +panic = "abort" strip = true opt-level = 3 diff --git a/changelog.d/7309-dist-profile-panic-abort.md b/changelog.d/7309-dist-profile-panic-abort.md new file mode 100644 index 0000000000..49e580c2f2 --- /dev/null +++ b/changelog.d/7309-dist-profile-panic-abort.md @@ -0,0 +1,32 @@ +### Fix: the SHIPPED runtime was built with the wrong panic strategy (#7302) + +The exception transport (#7305) requires the unwinder to step through runtime +Rust frames with `longjmp`-equivalent semantics, which is why +`[profile.release]` moved to `panic = "abort"`: under `panic = "unwind"` +rustc plants RFC-2945 abort-on-unwind guards in every `extern "C"` function +containing an interior Rust call, so a JS throw crossing such a helper — a +throwing getter, a `JSON.parse` error, a throwing `map` callback — aborts the +process instead of being caught. + +`[profile.dist]` — what `release-packages.yml` builds the shipped +`libperry_{runtime,stdlib}.a` with — *inherits* `release` but then +re-declared `panic = "unwind"`, and an explicit re-declaration wins over +`inherits`. #7305 changed only `[profile.release]`, so **every local build, +every CI job and the entire parity suite were correct while the artifact +users install would have aborted on the first cross-helper throw.** Nothing +failed to compile and nothing went red; the two configurations differ only in +the profile the release workflow happens to use. + +Confirmed directly from the rustc invocations (`cargo build --profile dist +-v`): before, `force-unwind-tables=yes` with no `-C panic=abort`; after, both +present. Caught before any release was cut from the merged EH work. + +Guarded so it cannot recur silently: `crates/perry/src/panic_profile_contract.rs` +runs in `cargo-test` (per PR) and asserts that every profile which builds a +shipped runtime archive declares `panic = "abort"` — deliberately treating +`inherits` as *not* evidence, since an innocuous-looking override is exactly +the failure mode. A second test pins `-C force-unwind-tables=yes` in +`.cargo/config.toml`, because abort alone omits the tables and the transport +cannot step runtime frames without them. Both were falsified before landing: +re-introducing the `unwind` value fails the first test with the diagnostic +above. diff --git a/crates/perry/src/main.rs b/crates/perry/src/main.rs index 5c4fd7f450..5d19940525 100644 --- a/crates/perry/src/main.rs +++ b/crates/perry/src/main.rs @@ -5,6 +5,7 @@ mod commands; mod compat_reports; #[cfg(test)] +mod panic_profile_contract; mod shadow_layout_contract; mod telemetry; #[cfg(test)] diff --git a/crates/perry/src/panic_profile_contract.rs b/crates/perry/src/panic_profile_contract.rs new file mode 100644 index 0000000000..0f0c49acca --- /dev/null +++ b/crates/perry/src/panic_profile_contract.rs @@ -0,0 +1,110 @@ +//! The panic-strategy contract for every profile that builds a runtime +//! archive Perry links into compiled programs (#7302). +//! +//! The exception transport requires the unwinder to step *through* runtime +//! Rust frames with `longjmp`-equivalent semantics. Under +//! `panic = "unwind"` rustc plants RFC-2945 abort-on-unwind guards in every +//! `extern "C"` function that contains an interior Rust call, so a JS throw +//! crossing such a helper — a throwing getter, a `JSON.parse` error, a +//! throwing `map` callback — aborts the process instead of being caught. +//! +//! This is not hypothetical and it is not caught by any other gate: +//! `[profile.dist]` (what `release-packages.yml` builds the SHIPPED +//! `libperry_{runtime,stdlib}.a` with) *inherits* `release` but then +//! re-declared `panic = "unwind"`, which wins. #7302 changed only +//! `[profile.release]`, so every local build, every CI job and the whole +//! parity suite were correct while the artifact users install would have +//! aborted on the first cross-helper throw. Nothing failed to compile; +//! nothing went red. +//! +//! So the invariant is asserted here, in `cargo-test` (per PR), by reading +//! the workspace manifest: a profile that ships a runtime must say +//! `panic = "abort"`, and `inherits` is not accepted as evidence because +//! the failure mode is precisely an override that looks harmless. + +#[cfg(test)] +mod tests { + /// Profiles used to build runtime archives that get linked into user + /// programs. `perry-dev` inherits release and never re-declares panic, + /// but it is listed so that a future re-declaration is caught too. + const SHIPPING_PROFILES: &[&str] = &["release", "dist", "perry-dev"]; + + fn workspace_manifest() -> String { + let path = concat!(env!("CARGO_MANIFEST_DIR"), "/../../Cargo.toml"); + std::fs::read_to_string(path) + .unwrap_or_else(|e| panic!("workspace manifest unreadable at {path}: {e}")) + } + + /// The `panic = ...` value declared *directly* in `[profile.]`, + /// or `None` when the profile does not re-declare it (and therefore + /// takes its parent's). + fn declared_panic(manifest: &str, profile: &str) -> Option { + let header = format!("[profile.{profile}]"); + let start = manifest + .find(&header) + .unwrap_or_else(|| panic!("[profile.{profile}] not found in the workspace manifest")) + + header.len(); + let body = &manifest[start..]; + // Stop at the next table header so a `[profile.X.package.Y]` + // override block is not misread as part of this profile. + let end = body.find("\n[").unwrap_or(body.len()); + for line in body[..end].lines() { + let line = line.trim(); + if let Some(rest) = line.strip_prefix("panic") { + let rest = rest.trim_start(); + if let Some(v) = rest.strip_prefix('=') { + return Some(v.trim().trim_matches('"').to_string()); + } + } + } + None + } + + #[test] + fn shipping_profiles_build_the_runtime_panic_abort() { + let manifest = workspace_manifest(); + // The subject must be LIVE: if the parser stops finding the key it + // is measuring nothing, and every profile would pass vacuously. + assert_eq!( + declared_panic(&manifest, "release").as_deref(), + Some("abort"), + "[profile.release] must declare panic = \"abort\" (see module docs)" + ); + + for profile in SHIPPING_PROFILES { + match declared_panic(&manifest, profile) { + // Re-declared: it must be abort. An `inherits` line does + // NOT protect against this — that is exactly how the + // shipped `dist` runtime ended up on the unwind strategy. + Some(v) => assert_eq!( + v, "abort", + "[profile.{profile}] declares panic = \"{v}\"; a runtime built that way \ + aborts the process on any JS throw that crosses an extern \"C\" helper \ + with an interior Rust call (RFC 2945). See the module docs." + ), + // Not re-declared: inherits release, which the assertion + // above pinned to abort. + None => {} + } + } + } + + /// The abort strategy is only half the contract: `panic = "abort"` + /// omits unwind tables by default, and without them the unwinder cannot + /// step runtime frames at all — every cross-helper throw is stranded + /// rather than caught. The flag lives in `.cargo/config.toml` because a + /// profile cannot carry rustflags. + #[test] + fn unwind_tables_are_forced_for_the_workspace() { + let path = concat!(env!("CARGO_MANIFEST_DIR"), "/../../.cargo/config.toml"); + let cfg = std::fs::read_to_string(path) + .unwrap_or_else(|e| panic!("cargo config unreadable at {path}: {e}")); + let normalized = cfg.replace(['"', ' ', '\n'], ""); + assert!( + normalized.contains("force-unwind-tables=yes"), + ".cargo/config.toml must force unwind tables: panic=abort omits them, and the \ + exception transport cannot step runtime frames without them (the runtime \ + self-checks on the first `try` and aborts loudly). See the module docs." + ); + } +}