From 54eec538952bf4ed83395e5366d85932934b824c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 18 Sep 2026 13:26:13 +0200 Subject: [PATCH 1/4] perf(runtime): route the shadow-stack TLS through the hot-cache fast path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every non-throwing `try` entry paid a real out-of-line `_tlv_get_addr` call, unconditionally, that nothing else in `try_push_with_kind` did. Profiling `try { t += v; } catch { t = 0; }` (differenced inside each binary: `(try80-try16)/64` vs `(loop80-loop16)/64` control, per CLAUDE.md's verification method, N=20000, median of 7) confirmed the 178-instruction/entry baseline and pinned the cost to one leaf via `sample`+`PERRY_KEEP_SYMBOLS=1` at 20M iterations: `try_push_with_kind`+176 -> `_tlv_get_addr`, ~6% of instructions retired per entry. Root cause: `CatchSavepoint::capture()`'s shadow field is captured unconditionally on every entry (`shadow_stack_savepoint`), and once any function anywhere in the process has pushed a shadow frame — true almost immediately for any real program, since a `catch (e)` binding itself needs one — the `SHADOW_FRAMES` latch from #7469's wave-1 fix is already set, so the latch stops making this read rare in practice. Unlike `EXCEPTION_STATE`, `CALL_METHOD_DEPTH`, and the named `runtime_handle_stack`/`temp_roots` fields, `SHADOW` was still a raw `thread_local!`, so this one read never got the `tls_hot` fast path at all. Fix: move `SHADOW` (crates/perry-runtime/src/gc/roots/shadow_stack.rs) from `thread_local!` to `crate::perry_thread_local!` — a pure storage-mechanism swap (same type, same `.with()`/`.try_with()` call sites, same const-init/drop-free semantics: `ShadowStackState` still has no `Drop`, so no destructor is registered) that changes nothing about liveness, throw-time restore, or the fixed-address contract `js_shadow_frame_enter` depends on for its whole-activation pointer cache. Confirmed via `--trace llvm`/`sample` that the leaf disappears entirely post-change. Measured (mybase = own build at this commit's parent, 9df5075fbe, codegen-units=16; both PERRY_NO_AUTO_OPTIMIZE=1): before: try 179.0-179.4 instr/entry, control -0.001..-0.2 after: try 164.3-165.7 instr/entry, control -0.14..+0.44 -> ~14-15 instructions/entry (~8%), controls stay noise-floor both arms. `try_push_with_kind`'s remaining `CatchSavepoint::capture()` work and `js_try_end`'s own (already tls_hot-fast) EXCEPTION_STATE resolution are UNTOUCHED — js_try_end disassembles to ~28 near-minimal instructions with nothing left to cut without threading a pointer through codegen from push to end, which was evaluated and deferred as materially riskier (touches try_stmt.rs's early-exit/closure/ generator/async call sites) for a smaller remaining win. scripts/thread_local_cold_allowlist.json: only the `shadow_stack.rs: 2 -> 1` line this change causes. `_hot_declarations` is left untouched — verify() never reads it (only `files` is enforced) — because a plain `--update` on unmodified main already produces 466, not the committed 460: six pre-existing, unrelated `perry_thread_local!` additions had drifted the count before this change touched anything. Fixing that drift is not this PR's job. Correctness: the shadow stack is the GC's precise root set, not pure exception bookkeeping, so this was checked past "it compiles": - Added test-files/test_gap_try_entry_shadow_hot_tls.ts (byte-identical to node v26.5.1 --experimental-strip-types --no-warnings, checked against both this change and the pristine parent arm): same-level catch, a throw 4 frames deep, a throw crossing Array.prototype.map's runtime trampoline, finally on both the normal and throwing paths, nested try with an inner rethrow, a catch that itself throws, 25 levels of try/finally nesting with finally-order and try_depth restored checked afterward, and a non-throwing control loop. Every caught value is read back after GC-pressure allocation. - Ran that fixture, plus the pre-existing test_gap_gc_catch_param_rooting.ts, test_gap_try_savepoint_subsystems.ts, test_gap_try_finally_no_catch_rethrow.ts and test_gap_try_setjmp_volatile.ts, under PERRY_GC_SCHEDULE_SEED (5 seeds) + PERRY_GC_SCHEDULE_RATE=1 + PERRY_GC_SCHEDULE_ALLOC_KB=0 + PERRY_GC_PROTECT_FROMSPACE=1 + PERRY_GC_DIAG=1 (per CLAUDE.md's rooting-bug instruments) — a collect-at-every-safepoint, evacuating, quarantine-and-mprotect schedule. All 5 seeds matched node byte-for-byte; the diagnostic output confirms the instrument was live, not vacuous (forced_collections=2566, copying_minors=2566, moved_objects=37000, fromspace retired_set up to #4 with bytes_protected growing) — no stale-shadow-stack SIGSEGV, no output drift. - `RUST_TEST_THREADS=1 cargo test --release -p perry-runtime --lib`: 4010 passed. The 2 failures (gc::tests::copy_slot_decode::sabotaged_remembering_arm_is_refused_by_the_coverage_cross_check, gc::tests::heap_generation::a_free_or_move_outside_every_scope_is_caught_in_debug_builds) are pre-existing: reverted this change with `git checkout --` and reran just those two on the pristine parent commit — identical failures, same messages, confirming they assert debug_assert!-gated behavior compiled out under --release, unrelated to this change. - `cargo fmt --all -- --check`, `scripts/check_file_size.sh`, `scripts/check_test_registration.py`, `RUSTFLAGS="-D warnings" cargo check -p perry-runtime --all-targets`: all clean. --- .../src/gc/roots/shadow_stack.rs | 33 ++- scripts/thread_local_cold_allowlist.json | 2 +- .../test_gap_try_entry_shadow_hot_tls.ts | 218 ++++++++++++++++++ 3 files changed, 247 insertions(+), 6 deletions(-) create mode 100644 test-files/test_gap_try_entry_shadow_hot_tls.ts diff --git a/crates/perry-runtime/src/gc/roots/shadow_stack.rs b/crates/perry-runtime/src/gc/roots/shadow_stack.rs index a8a8f28d61..2cc83225dd 100644 --- a/crates/perry-runtime/src/gc/roots/shadow_stack.rs +++ b/crates/perry-runtime/src/gc/roots/shadow_stack.rs @@ -207,11 +207,34 @@ impl ShadowStackState { } } -thread_local! { - /// `const`-initialized and drop-free, so the access is a plain TLS address - /// computation with no lazy-init or destructor-registration check. The - /// buffer is reserved lazily on the first push instead of eagerly at thread - /// start, and released by [`ShadowBufferGuard`]. +crate::perry_thread_local! { + /// `const`-initialized and drop-free, so the access has no lazy-init or + /// destructor-registration check. The buffer is reserved lazily on the + /// first push instead of eagerly at thread start, and released by + /// [`ShadowBufferGuard`]. + /// + /// Was a raw `thread_local!` until #try-entry-perf: every `try`/`catch` + /// pays this exact resolution once per entry, unconditionally, through + /// `shadow_stack_savepoint()` — and `SHADOW_FRAMES` (the latch that lets + /// most other savepoint fields skip their own thread-local read, see + /// `crate::exception::savepoints`) is set by the FIRST shadow-frame push + /// anywhere in the process, which for real programs is essentially + /// "immediately" (any function with a pointer-typed local pushes one, and + /// a caught exception's own binding needs a slot). So the latch does not + /// make this read rare in practice, and profiling a `try`/`catch` loop + /// (`node --experimental-strip-types`-verified against `test-files/`) + /// showed it as a genuine leaf `_tlv_get_addr` call, ~6% of total + /// instructions retired per non-throwing `try` entry — the one savepoint + /// field NOT already routed through `tls_hot`'s cache (`EXCEPTION_STATE`, + /// `CALL_METHOD_DEPTH` and the named `runtime_handle_stack`/`temp_roots` + /// fields all were). Moving to `perry_thread_local!` is a pure + /// storage-mechanism swap — same type, same `.with()`/`.try_with()` + /// call sites below, same const-init/drop-free semantics (needs_drop is + /// still false, so no destructor is registered) — so it changes nothing + /// about liveness, restore-on-throw, or the address-stability contract + /// [`js_shadow_frame_enter`] depends on (the value still lives at a + /// fixed, never-reallocated thread-local address for the thread's whole + /// lifetime). pub(crate) static SHADOW: UnsafeCell = const { UnsafeCell::new(ShadowStackState { ptr: std::ptr::null_mut(), diff --git a/scripts/thread_local_cold_allowlist.json b/scripts/thread_local_cold_allowlist.json index ad4f7ff13c..c4cc721aba 100644 --- a/scripts/thread_local_cold_allowlist.json +++ b/scripts/thread_local_cold_allowlist.json @@ -38,7 +38,7 @@ "crates/perry-runtime/src/gc/promote_in_place.rs": 11, "crates/perry-runtime/src/gc/roots/runtime_handles.rs": 2, "crates/perry-runtime/src/gc/roots/scan_mode.rs": 1, - "crates/perry-runtime/src/gc/roots/shadow_stack.rs": 2, + "crates/perry-runtime/src/gc/roots/shadow_stack.rs": 1, "crates/perry-runtime/src/gc/roots/temp_roots.rs": 1, "crates/perry-runtime/src/gc/scan_fallback.rs": 2, "crates/perry-runtime/src/gc/shape_install.rs": 1, diff --git a/test-files/test_gap_try_entry_shadow_hot_tls.ts b/test-files/test_gap_try_entry_shadow_hot_tls.ts new file mode 100644 index 0000000000..a2a59b222d --- /dev/null +++ b/test-files/test_gap_try_entry_shadow_hot_tls.ts @@ -0,0 +1,218 @@ +// Adversarial coverage for the `try`-entry perf change that moved `SHADOW` +// (`crates/perry-runtime/src/gc/roots/shadow_stack.rs`) from a raw +// `thread_local!` to `crate::perry_thread_local!` so `try_push_with_kind`'s +// `CatchSavepoint::capture()` stops paying a real `_tlv_get_addr` call on +// every non-throwing `try` entry. The change is a pure storage-mechanism +// swap (same type, same `.with()`/`.try_with()` call sites, same +// const-init/drop-free semantics), but the shadow stack IS the GC's precise +// root set for named locals and expression temporaries, and its savepoint is +// what a throw restores when unwinding skips frame-pop epilogues (#1830). +// This file exercises every shape that depends on that machinery staying +// exactly correct: a throw caught at the entry level, a throw several +// frames deep, a throw crossing a runtime helper callback, `finally` on both +// the normal and throwing paths, a nested `try` with an inner rethrow, a +// `catch` block that itself throws, deep `try` nesting, and — last — a +// non-throwing control so the fast (unchanged-behavior) path is checked too. +// Every caught value is read back AFTER GC-pressure allocation so a +// mis-rooted exception (swept, not just moved) would show up as wrong output +// rather than a crash. + +function churn(n: number): number { + const a: unknown[] = []; + for (let i = 0; i < n; i++) { + a.push({ i, s: "churn" + i, nested: { i } }); + } + return a.length; +} + +// 1. Throw caught at the same level. +function sameLevel(): string { + try { + throw new Error("same-level"); + } catch (e) { + churn(300); + return (e as Error).message; + } +} +console.log("sameLevel", sameLevel()); + +// 2. Throw from a nested function several frames down. +function deep4(): number { + throw new Error("deep4"); +} +function deep3(): number { + return deep4(); +} +function deep2(): number { + return deep3(); +} +function deep1(): number { + return deep2(); +} +function nestedFrames(): string { + try { + deep1(); + return "unreached"; + } catch (e) { + churn(300); + return (e as Error).message; + } +} +console.log("nestedFrames", nestedFrames()); + +// 3. Throw crossing a runtime helper (inside an Array.prototype.map +// callback — the throw unwinds through the runtime's own map trampoline). +function throughMap(): string { + const src = [1, 2, 3, 4, 5]; + try { + src.map((x) => { + if (x === 3) throw new Error("through-map " + x); + return x * 2; + }); + return "unreached"; + } catch (e) { + churn(300); + return (e as Error).message; + } +} +console.log("throughMap", throughMap()); + +// 4a. finally runs on the NORMAL (non-throwing) path. +let finallyNormalRuns = 0; +function finallyNormal(): number { + try { + return 42; + } finally { + finallyNormalRuns++; + } +} +console.log("finallyNormal", finallyNormal(), "ran", finallyNormalRuns); + +// 4b. finally runs on the THROWING path, then the exception still propagates. +let finallyThrowRuns = 0; +function finallyThrow(): number { + try { + throw new Error("finally-throw"); + } finally { + finallyThrowRuns++; + } +} +try { + finallyThrow(); + console.log("finallyThrow (WRONG): did not throw"); +} catch (e) { + churn(300); + console.log("finallyThrow caught", (e as Error).message, "ran", finallyThrowRuns); +} + +// 5. Nested try with an inner rethrow — the outer catch must see the SAME +// (rethrown) exception, with its shadow-rooted fields intact. +function nestedRethrow(): string { + try { + try { + const err: any = new Error("inner"); + err.tag = "rethrow-tag"; + throw err; + } catch (inner) { + churn(200); + throw inner; + } + } catch (outer) { + churn(200); + const e = outer as any; + return e.message + "/" + e.tag; + } +} +console.log("nestedRethrow", nestedRethrow()); + +// 6. A try whose catch itself throws — the ORIGINAL exception is replaced by +// the catch's own throw, and outer code must see the new one. +function catchThrows(): string { + try { + try { + throw new Error("first"); + } catch (e) { + churn(200); + throw new Error("from-catch:" + (e as Error).message); + } + } catch (e2) { + churn(200); + return (e2 as Error).message; + } +} +console.log("catchThrows", catchThrows()); + +// 7. Deep try nesting — an exception thrown at the bottom must unwind +// through every level, running each finally exactly once, in order, and +// land in the outermost catch with the shadow stack balanced afterward. +const finallyOrder: number[] = []; +function deepNestTry(levels: number): string { + if (levels === 0) { + throw new Error("deep-nest-bottom"); + } + try { + return deepNestTry(levels - 1); + } finally { + finallyOrder.push(levels); + } +} +try { + deepNestTry(25); + console.log("deepNestTry (WRONG): did not throw"); +} catch (e) { + churn(300); + console.log( + "deepNestTry caught", + (e as Error).message, + "finally count", + finallyOrder.length, + "finally order ok", + finallyOrder.every((v, i) => v === i + 1) + ); +} + +// A second, unrelated try AFTER the deep unwind: proves try_depth and the +// shadow stack were left exactly where they should be, not off by the +// number of levels just unwound. +function afterDeepUnwind(): string { + try { + throw new Error("after-deep-unwind"); + } catch (e) { + return (e as Error).message; + } +} +console.log("afterDeepUnwind", afterDeepUnwind()); + +// 8. Non-throwing control: the hot (unchanged-behavior) path. A tight loop +// of try/catch blocks that never throw, mixed with ones that do, so the +// fast entry/exit accounting can't drift relative to the slow throw path. +function controlLoop(): number { + let total = 0; + for (let i = 0; i < 200; i++) { + try { + total += i; + if (i % 37 === 0) { + try { + if (i % 74 === 0) throw new Error("control-inner " + i); + total += 1; + } catch (e) { + churn(20); + total -= 1; + } + } + } catch (e) { + total = -1; // never reached + } + } + return total; +} +console.log("controlLoop", controlLoop()); + +// Final sanity: try_depth is back at zero — a fresh top-level try still +// catches correctly after everything above. +try { + throw new Error("final-sanity"); +} catch (e) { + console.log("finalSanity", (e as Error).message); +} +console.log("done"); From 2080583bdb58a0a8f0f44c8fc6cc87685c73beef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 18 Sep 2026 14:12:47 +0200 Subject: [PATCH 2/4] docs: changelog fragment for #10619 --- changelog.d/10619-shadow-stack-hot-tls.md | 27 +++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 changelog.d/10619-shadow-stack-hot-tls.md diff --git a/changelog.d/10619-shadow-stack-hot-tls.md b/changelog.d/10619-shadow-stack-hot-tls.md new file mode 100644 index 0000000000..45cb2a72e9 --- /dev/null +++ b/changelog.d/10619-shadow-stack-hot-tls.md @@ -0,0 +1,27 @@ +Give the shadow-stack root state (`SHADOW`) the `tls_hot` fast path. It was the +last savepoint field still declared as a raw `thread_local!`, so every +`CatchSavepoint::capture()` — once per `try` entry — paid `_tlv_get_addr` for +it on Darwin, while `EXCEPTION_STATE`, `CALL_METHOD_DEPTH` and the named +`runtime_handle_stack`/`temp_roots` fields were already routed through +`tls_hot`. Wave 1's "has any thread used this subsystem" latch does not make +the read rare here: it is set by the first shadow-frame push anywhere in the +process, and a `catch (e)` binding needs a shadow slot itself. + +Profiling pinned it to one call site, `try_push_with_kind+176 -> +_tlv_get_addr`, about 6% of instructions retired per entry. The change is a +storage-mechanism swap — same type, call sites, const-init and drop-free +semantics — so the address-stability contract `js_shadow_frame_enter` depends +on is unchanged. A non-throwing `try` entry goes 179.3 -> 164.8 instructions +(-8.1%), differenced within each binary with the loop control at the noise +floor in both arms. The probe understates it: `SHADOW` is read on many paths, +not only this one. + +Because the shadow stack is the GC's precise root set rather than bookkeeping, +the new fixture attacks rooting: a throw four frames down, a throw crossing +`Array.prototype.map`'s runtime trampoline, `finally` on both paths, nested +`try` with an inner rethrow, a `catch` that itself throws, and 25-level nesting +— every caught value read back after GC-pressure allocation. It plus four +existing exception/rooting fixtures ran under five GC-schedule seeds at +RATE=1 with from-space protection, all byte-identical to node, with +forced_collections=2566 / copying_minors=2566 / moved_objects=37000 confirming +the instrument was live. From e39f637e051faece1e318f5cb673d0f1e0ee53aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 19 Sep 2026 10:23:55 +0200 Subject: [PATCH 3/4] fix(runtime): cfg-split the shadow-stack hot-TLS path to Darwin aarch64 #10619 routed SHADOW through perry_thread_local! unconditionally for an ~8% try-entry win, measured on Darwin. CI's cargo-test job (Linux, debug profile) SIGSEGV'd on it (run 35374727647, job 105594641738); the crash was never pinned to a specific test or line (stdout block-buffering ate the FAILED line), and it did not reproduce on macOS with the Darwin path, on macOS with the Darwin path forced off, or on a Linux x86_64 VM matching the CI runner's triple through 1968+ of ~4013 tests. tls_hot.rs's own docs say the hot-cache shortcut only pays for itself on Darwin aarch64; everywhere else it is strictly more work than the raw thread_local! it replaced (an extra HOT resolution plus a slot indirection), win or no SIGSEGV. So SHADOW's declaration is now cfg-split: perry_thread_local! only under all(target_vendor = "apple", target_arch = "aarch64", target_pointer_width = "64"), a plain thread_local! (the pre-#10619 form) everywhere else. This keeps the measured win where it was measured, removes the pessimization elsewhere, and removes CI's only exposure to the unconfirmed SIGSEGV mechanism. The open investigation is tracked in #10709. Re-verified: debug and release perry-runtime lib tests, GC-stress with live copying minors and from-space quarantine, the gap fixture byte-identical to node 26.5.1, and a fresh differential probe showing the try-entry win survives on Darwin (162.2 -> 152.0 instructions/ entry, ~6.3%, bare-loop control ~1 instruction in both arms). --- changelog.d/10619-shadow-stack-hot-tls.md | 62 ++++++++++++ .../src/gc/roots/shadow_stack.rs | 99 ++++++++++++++----- scripts/thread_local_cold_allowlist.json | 2 +- 3 files changed, 135 insertions(+), 28 deletions(-) diff --git a/changelog.d/10619-shadow-stack-hot-tls.md b/changelog.d/10619-shadow-stack-hot-tls.md index 45cb2a72e9..38b345fd27 100644 --- a/changelog.d/10619-shadow-stack-hot-tls.md +++ b/changelog.d/10619-shadow-stack-hot-tls.md @@ -25,3 +25,65 @@ existing exception/rooting fixtures ran under five GC-schedule seeds at RATE=1 with from-space protection, all byte-identical to node, with forced_collections=2566 / copying_minors=2566 / moved_objects=37000 confirming the instrument was live. + +**Follow-up: CI's `cargo-test` job (Linux, debug profile) SIGSEGV'd on this +change** (run 35374727647, job 105594641738) — no `FAILED` line survived +(libtest's stdout is block-buffered under CI), so the crashing test was never +named. Confirmed this PR's own regression: `cargo-test` was clean on #10644, +#10647, #10650, #10651 against the same base. + +Reproduction, in the exact failing configuration +(`RUST_TEST_THREADS=1 cargo test -p perry-runtime --lib`, debug, no +`--release`), across every environment tried, came back clean: +- macOS arm64, the shipped Darwin `perry_thread_local!` path: 4013 passed. +- macOS arm64 with the Darwin `pthread`-TSD path forced off (`hot()` forced + onto the generic `hot_via_tls()` route every non-Darwin-aarch64 target + already uses): 4013 passed, 0 failed — this weakens, though doesn't + disprove, a re-entrancy theory in the generic path itself. +- Linux x86_64 (qemu-emulated Ubuntu VM on the CI-pinned + `nightly-2026-08-20` toolchain, matching `ubuntu-latest`'s triple): ran + clean through 1968+ of ~4013 tests in alphabetical order, well past the + async_hooks region where CI's log cuts off, before a later, unrelated + regex-replace stress test stalled under emulation (not a crash — verified + it runs in well under a second natively; almost certainly a qemu-specific + slowdown, not tied to this change). + +The SIGSEGV's mechanism was never pinned down — see #10709 for the full +writeup, including the two open hypotheses (re-entrancy in `tls_hot.rs`'s +`fill()`, which writes its last field specifically to guard against a +half-filled cache being *used* re-entrantly but does not stop `fill()` being +*called* again re-entrantly; and plain debug-profile stack depth on CI's +default thread, a signature this repo has hit before). Rather than let it +evaporate, it's tracked there and the change is narrowed instead: + +`SHADOW`'s declaration is now cfg-split — +`crate::perry_thread_local!` only under +`all(target_vendor = "apple", target_arch = "aarch64", target_pointer_width = "64")`, +a plain `thread_local!` (the pre-#10619 form) everywhere else. This isn't +only a hedge against the unconfirmed SIGSEGV: `tls_hot.rs`'s own module docs +already say the published-cache shortcut is Darwin-aarch64-specific, and +everywhere else "resolving a thread-local is already a fixed offset and the +extra cache indirection has no demonstrated benefit" — so routing `SHADOW` +through it unconditionally was strictly more work (one extra `HOT` +resolution plus a slot-array indirection) on every other target for a win +that was only ever measured on Darwin. `scripts/thread_local_cold_allowlist.json`'s +`shadow_stack.rs` count goes back to 2 (its pre-#10619 value) to match. + +Re-verified on this Darwin host post-split, differential probe (own builds, +base = this PR's parent 9df5075fbe, arm = this fix, both +`PERRY_NO_AUTO_OPTIMIZE=1`, median of 7, N=20000/40000): try-entry marginal +cost 162.2 -> 152.0 instructions/entry (-10.2, ~6.3%), bare-loop control +~1.0 instructions in both arms (noise floor relative to the ~150-instruction +signal) — the win the original commit measured (-8.1%) survives, because the +fix does not touch the Darwin code path at all. + +Also re-ran: debug and release `cargo test -p perry-runtime --lib` (release: +4010 passed, the same 2 pre-existing `debug_assert!`-gated failures noted +above); the gap fixture plus GC-stress (seeds 1 and 42, +`PERRY_GC_SCHEDULE_RATE=1 PERRY_GC_PROTECT_FROMSPACE=1 +PERRY_GC_VERIFY_EVACUATION=1 PERRY_GC_FROMSPACE_SCAN_ABORT=1`) both +byte-identical to node with non-zero copying minors +(`copying_minors=76 moved_objects=35258`) and a live +`[gc-fromspace-protect] retired_set=#75` line; `cargo fmt --all -- --check`; +`scripts/run_lint_gates.sh` (the only failure is the pre-existing, known-red +public-baseline step); `scripts/check_thread_locals.py`/`--self-test`. diff --git a/crates/perry-runtime/src/gc/roots/shadow_stack.rs b/crates/perry-runtime/src/gc/roots/shadow_stack.rs index 2cc83225dd..6c1b3c4555 100644 --- a/crates/perry-runtime/src/gc/roots/shadow_stack.rs +++ b/crates/perry-runtime/src/gc/roots/shadow_stack.rs @@ -207,34 +207,79 @@ impl ShadowStackState { } } +// [`SHADOW`]'s storage mechanism is cfg-split by platform. Both arms are +// `const`-initialized and drop-free, so neither pays a lazy-init or +// destructor-registration check; the buffer is reserved lazily on the first +// push instead of eagerly at thread start, and released by +// [`ShadowBufferGuard`], on either arm. +// +// # Why `perry_thread_local!` only on Apple aarch64 +// +// Until #10619, `SHADOW` was unconditionally a raw `thread_local!`: every +// `try`/`catch` paid this exact resolution once per entry, through +// `shadow_stack_savepoint()` — and `SHADOW_FRAMES` (the latch that lets most +// other savepoint fields skip their own thread-local read, see +// `crate::exception::savepoints`) is set by the FIRST shadow-frame push +// anywhere in the process, which for real programs is essentially +// "immediately" (any function with a pointer-typed local pushes one, and a +// caught exception's own binding needs a slot). So the latch does not make +// this read rare in practice, and profiling a `try`/`catch` loop +// (`node --experimental-strip-types`-verified against `test-files/`) showed +// it as a genuine leaf `_tlv_get_addr` call, ~6% of total instructions +// retired per non-throwing `try` entry — the one savepoint field NOT already +// routed through `tls_hot`'s cache (`EXCEPTION_STATE`, `CALL_METHOD_DEPTH` +// and the named `runtime_handle_stack`/`temp_roots` fields all were). +// +// But that fast path is itself Darwin-aarch64-specific: `tls_hot.rs`'s own +// module docs say the published-cache shortcut only exists there, and +// everywhere else "resolving a thread-local is already a fixed offset and +// the extra cache indirection has no demonstrated benefit." #10619 shipped +// the swap unconditionally anyway, and CI's `cargo-test` job — Linux, debug +// profile — hit a SIGSEGV that a plain `thread_local!` never produced +// (`cargo-test` run 35374727647, job 105594641738). The cause was never +// isolated to a specific line: the same debug suite ran clean on macOS +// through the Darwin `perry_thread_local!` path, and also ran clean on +// macOS with the Darwin `pthread`-TSD path forced off (so `hot()` took the +// generic `hot_via_tls()` route `perry_thread_local!` uses on every +// non-Darwin-aarch64 target) — only actual Linux/x86_64 reproduced it, and +// under qemu-emulated Linux the debug suite is roughly an order of +// magnitude slower than native, which made pinning the exact crashing test +// impractical here. See #10709 for the open investigation. +// +// Since the −8% `try`-entry win was only ever measured on Darwin +// (`perry_thread_local!`'s whole premise doesn't apply anywhere the direct +// TLS access is already a fixed offset — see `tls_hot.rs`), routing through +// it off that platform was strictly more work for no measured benefit even +// before the SIGSEGV: one more thread-local resolution (`HOT`) plus a +// slot-array indirection, replacing the single direct TLS access a raw +// `thread_local!` already was. So the fast path now ships only where it was +// measured, and every other target keeps the original raw form — which +// also means this file's `thread_local!` count goes back to what it was +// before #10619 (see `scripts/thread_local_cold_allowlist.json`). +#[cfg(all( + target_vendor = "apple", + target_arch = "aarch64", + target_pointer_width = "64" +))] crate::perry_thread_local! { - /// `const`-initialized and drop-free, so the access has no lazy-init or - /// destructor-registration check. The buffer is reserved lazily on the - /// first push instead of eagerly at thread start, and released by - /// [`ShadowBufferGuard`]. - /// - /// Was a raw `thread_local!` until #try-entry-perf: every `try`/`catch` - /// pays this exact resolution once per entry, unconditionally, through - /// `shadow_stack_savepoint()` — and `SHADOW_FRAMES` (the latch that lets - /// most other savepoint fields skip their own thread-local read, see - /// `crate::exception::savepoints`) is set by the FIRST shadow-frame push - /// anywhere in the process, which for real programs is essentially - /// "immediately" (any function with a pointer-typed local pushes one, and - /// a caught exception's own binding needs a slot). So the latch does not - /// make this read rare in practice, and profiling a `try`/`catch` loop - /// (`node --experimental-strip-types`-verified against `test-files/`) - /// showed it as a genuine leaf `_tlv_get_addr` call, ~6% of total - /// instructions retired per non-throwing `try` entry — the one savepoint - /// field NOT already routed through `tls_hot`'s cache (`EXCEPTION_STATE`, - /// `CALL_METHOD_DEPTH` and the named `runtime_handle_stack`/`temp_roots` - /// fields all were). Moving to `perry_thread_local!` is a pure - /// storage-mechanism swap — same type, same `.with()`/`.try_with()` - /// call sites below, same const-init/drop-free semantics (needs_drop is - /// still false, so no destructor is registered) — so it changes nothing - /// about liveness, restore-on-throw, or the address-stability contract - /// [`js_shadow_frame_enter`] depends on (the value still lives at a - /// fixed, never-reallocated thread-local address for the thread's whole - /// lifetime). + /// See the cfg-split rationale above this declaration. + pub(crate) static SHADOW: UnsafeCell = const { + UnsafeCell::new(ShadowStackState { + ptr: std::ptr::null_mut(), + len: 0, + cap: 0, + frame_top: usize::MAX, + }) + }; +} + +#[cfg(not(all( + target_vendor = "apple", + target_arch = "aarch64", + target_pointer_width = "64" +)))] +thread_local! { + /// See the cfg-split rationale above this declaration's sibling arm. pub(crate) static SHADOW: UnsafeCell = const { UnsafeCell::new(ShadowStackState { ptr: std::ptr::null_mut(), diff --git a/scripts/thread_local_cold_allowlist.json b/scripts/thread_local_cold_allowlist.json index c4cc721aba..ad4f7ff13c 100644 --- a/scripts/thread_local_cold_allowlist.json +++ b/scripts/thread_local_cold_allowlist.json @@ -38,7 +38,7 @@ "crates/perry-runtime/src/gc/promote_in_place.rs": 11, "crates/perry-runtime/src/gc/roots/runtime_handles.rs": 2, "crates/perry-runtime/src/gc/roots/scan_mode.rs": 1, - "crates/perry-runtime/src/gc/roots/shadow_stack.rs": 1, + "crates/perry-runtime/src/gc/roots/shadow_stack.rs": 2, "crates/perry-runtime/src/gc/roots/temp_roots.rs": 1, "crates/perry-runtime/src/gc/scan_fallback.rs": 2, "crates/perry-runtime/src/gc/shape_install.rs": 1, From 59297109944e508c0a5456eb297036752a3ea2e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 19 Sep 2026 11:07:15 +0200 Subject: [PATCH 4/4] docs(runtime): confirm the shadow-stack SIGSEGV was caused by #10619 Pushed the cfg-gated fix and read CI's own cargo-test job on the real failing runner: green (run 35433215970) where the unconditional perry_thread_local! swap was red (run 35374727647), same commit otherwise. That settles causation directly, superseding the local repro attempts (macOS both arms, qemu Linux) which all came back clean and were inconclusive on their own -- including a qemu-VM A/B whose two SIGKILLs were momentarily misread as a reproduced crash before being traced to an operator pkill -f self-match, not a fault. Also resolves why cargo-test stayed green on #10644/#10647/#10650/ #10651 against the same base: none of them touch shadow_stack.rs, and this PR was never merged to main, so their runs never contained the change at all. The internal mechanism inside tls_hot.rs's resolution path is still not understood; #10709 tracks that open half. This commit only updates the code comment and changelog to say plainly what is now confirmed versus what remains unknown. --- changelog.d/10619-shadow-stack-hot-tls.md | 43 ++++++++++++------- .../src/gc/roots/shadow_stack.rs | 21 +++++---- 2 files changed, 39 insertions(+), 25 deletions(-) diff --git a/changelog.d/10619-shadow-stack-hot-tls.md b/changelog.d/10619-shadow-stack-hot-tls.md index 38b345fd27..bc25f3b14e 100644 --- a/changelog.d/10619-shadow-stack-hot-tls.md +++ b/changelog.d/10619-shadow-stack-hot-tls.md @@ -29,32 +29,43 @@ the instrument was live. **Follow-up: CI's `cargo-test` job (Linux, debug profile) SIGSEGV'd on this change** (run 35374727647, job 105594641738) — no `FAILED` line survived (libtest's stdout is block-buffered under CI), so the crashing test was never -named. Confirmed this PR's own regression: `cargo-test` was clean on #10644, -#10647, #10650, #10651 against the same base. +named. `cargo-test` was clean on #10644, #10647, #10650, #10651 against the +same base — not a contradiction, once checked: none of those four PRs touch +`shadow_stack.rs`, and this PR was never merged to `main`, so their +`cargo-test` runs (branch merged with `main`) never contained this change to +begin with. Only this branch's own run ever exercised it, and that run was +red. Reproduction, in the exact failing configuration (`RUST_TEST_THREADS=1 cargo test -p perry-runtime --lib`, debug, no -`--release`), across every environment tried, came back clean: +`--release`), across every environment tried locally, came back clean and was +ultimately inconclusive: - macOS arm64, the shipped Darwin `perry_thread_local!` path: 4013 passed. - macOS arm64 with the Darwin `pthread`-TSD path forced off (`hot()` forced onto the generic `hot_via_tls()` route every non-Darwin-aarch64 target - already uses): 4013 passed, 0 failed — this weakens, though doesn't - disprove, a re-entrancy theory in the generic path itself. + already uses): 4013 passed, 0 failed. - Linux x86_64 (qemu-emulated Ubuntu VM on the CI-pinned `nightly-2026-08-20` toolchain, matching `ubuntu-latest`'s triple): ran clean through 1968+ of ~4013 tests in alphabetical order, well past the - async_hooks region where CI's log cuts off, before a later, unrelated - regex-replace stress test stalled under emulation (not a crash — verified - it runs in well under a second natively; almost certainly a qemu-specific - slowdown, not tied to this change). + async_hooks region where CI's log cuts off. qemu turned out to be unusable + as an instrument beyond that: one specific unrelated regex-replace stress + test is pathologically slow under emulation (fast natively), and an + attempted A/B there produced two SIGKILLs that were first misread as a + reproduced crash — they were an operator `pkill -f` self-matching its own + remote shell (a documented pitfall), not a fault. -The SIGSEGV's mechanism was never pinned down — see #10709 for the full -writeup, including the two open hypotheses (re-entrancy in `tls_hot.rs`'s -`fill()`, which writes its last field specifically to guard against a -half-filled cache being *used* re-entrantly but does not stop `fill()` being -*called* again re-entrantly; and plain debug-profile stack depth on CI's -default thread, a signature this repo has hit before). Rather than let it -evaporate, it's tracked there and the change is narrowed instead: +So local reproduction never settled it either way. Causation was confirmed +the direct way instead: pushing this cfg-gated fix and reading CI's own +`cargo-test` job on the exact failing runner — green (run 35433215970, job +105871415920), where the unconditional swap was red. The PR *is* what caused +the SIGSEGV; gating it off Linux/non-Darwin is what fixed it. The internal +mechanism inside `tls_hot.rs`'s resolution path is still not understood — +this is a fix by removing the exposure, not by finding the fault. See #10709 +for the open half of the investigation (`fill()`'s `temp_roots`-last +ordering guards against a half-filled cache being *used* re-entrantly, not +against `fill()` being *called* again re-entrantly, which remains a live +suspect). The change is narrowed rather than left as a mystery with no +mitigation: `SHADOW`'s declaration is now cfg-split — `crate::perry_thread_local!` only under diff --git a/crates/perry-runtime/src/gc/roots/shadow_stack.rs b/crates/perry-runtime/src/gc/roots/shadow_stack.rs index 6c1b3c4555..4d247489d6 100644 --- a/crates/perry-runtime/src/gc/roots/shadow_stack.rs +++ b/crates/perry-runtime/src/gc/roots/shadow_stack.rs @@ -236,15 +236,18 @@ impl ShadowStackState { // the extra cache indirection has no demonstrated benefit." #10619 shipped // the swap unconditionally anyway, and CI's `cargo-test` job — Linux, debug // profile — hit a SIGSEGV that a plain `thread_local!` never produced -// (`cargo-test` run 35374727647, job 105594641738). The cause was never -// isolated to a specific line: the same debug suite ran clean on macOS -// through the Darwin `perry_thread_local!` path, and also ran clean on -// macOS with the Darwin `pthread`-TSD path forced off (so `hot()` took the -// generic `hot_via_tls()` route `perry_thread_local!` uses on every -// non-Darwin-aarch64 target) — only actual Linux/x86_64 reproduced it, and -// under qemu-emulated Linux the debug suite is roughly an order of -// magnitude slower than native, which made pinning the exact crashing test -// impractical here. See #10709 for the open investigation. +// (`cargo-test` run 35374727647, job 105594641738). Local reproduction (macOS +// on both the Darwin path and with it forced off, and a qemu-emulated Linux +// x86_64 VM) never faulted, so causation was confirmed the direct way +// instead: `cargo-test` on CI itself, gating `SHADOW` back to this cfg split, +// came back green (run 35433215970, job 105871415920) at the same commit +// that was red with the swap unconditional — so the PR *is* what caused it. +// The internal mechanism is still not understood: this fixes it by removing +// the change from every platform where it had no benefit anyway, not by +// finding the fault inside `tls_hot.rs`'s resolution path. See #10709 for +// the open half of the investigation (`fill()`'s `temp_roots`-last ordering +// guards against a half-filled cache being *used* re-entrantly, not against +// `fill()` being *called* re-entrantly, which remains a live suspect). // // Since the −8% `try`-entry win was only ever measured on Darwin // (`perry_thread_local!`'s whole premise doesn't apply anywhere the direct