diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 94ed5eb98d..0586530568 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -967,6 +967,20 @@ jobs: if: github.event_name != 'pull_request' run: ./scripts/gc_repsel_matrix.sh --no-build --arms all --json gc-repsel-matrix.json + # GATING, and deliberately so. CLAUDE.md's GC knob kill-policy requires + # every GC knob to have an arm that exercises it; the #7154 instruments + # (PERRY_GC_PROTECT_FROMSPACE, PERRY_GC_ZEAL) would otherwise be dark + # knobs on the subsystem with this repo's worst history of configuration + # rot. This asserts BOTH defaults — inert with the knobs unset, live with + # them set — and refuses to pass unless the zeal arm forced strictly more + # collections than the pressure-only arm, so it cannot go green having + # run zero copying minors (the #6942 / #7024 / #7025 failure mode). + # The detection property itself is a required-gate unit test: + # gc/tests/fromspace_protect.rs::quarantine_catches_a_planted_stale_from_space_deref. + # ~20s: the fixture is sized for ~1200 back-edge polls, not #7154's 240k. + - name: GC rooting-bug instruments (inert-when-off, live-when-on) + run: ./scripts/gc_instrument_smoke.sh target/release/perry + - name: Run GC write-barrier stress tests # Informational: these are ~200s nondeterministic corruption-window # hunts (#5029). Kept out of the gate so a flake never blocks a PR. diff --git a/CLAUDE.md b/CLAUDE.md index 89543a6c55..29fa33cb2a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,7 +8,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co Perry is a native TypeScript compiler written in Rust that compiles TypeScript source code directly to native executables. It uses SWC for TypeScript parsing and LLVM for code generation. -**Current Version:** 0.5.1278 +**Current Version:** 0.5.1277 ## TypeScript Parity Status @@ -132,6 +132,19 @@ Generational mark-sweep GC in `crates/perry-runtime/src/gc.rs` (default since v0 **Escape hatches**: `PERRY_GEN_GC=0`/`off`/`false` reverts to full mark-sweep (bisection only). `PERRY_GEN_GC_EVACUATE=0`/`off`/`false` disables policy evacuation; `=1`/`on`/`true` is accepted as auto-policy allowed, not unconditional evacuation. `PERRY_GC_FORCE_EVACUATE=1` stress-copies every marked non-pinned nursery object only when generated write barriers are active and policy evacuation is allowed. `PERRY_GC_VERIFY_EVACUATION=1` panics if any mutable live slot still points at a forwarded nursery object after an evacuation/rewrite cycle. `PERRY_WRITE_BARRIERS=0`/`off`/`false` disables codegen-emitted write barriers at compile time and runtime exact helper barriers at runtime for benchmark/debug bisection; unset, `=1`/`on`/`true` keep barriers enabled. `PERRY_GC_DIAG=1` prints per-cycle diagnostics, including evacuation-policy decisions for considered cycles and `barriers_inactive` skips. +### Rooting-bug instruments (#7154 family) — what each knob ACTUALLY gates + +A "GC value live but not rooted across a collection point" bug is invisible at collection time: there is nothing for the collector to find. It surfaces one or more cycles later, in a different function, as `TypeError: value is not a function`. These three knobs exist to collapse that latency. **All default-off; every boolean knob's OFF state is asserted in `gc/tests/fromspace_protect.rs`** (`…_DEPTH` is a magnitude, not a mode, so its floor and default are asserted instead). The instruments are **sabotage-tested**, not merely exercised: `quarantine_catches_a_planted_stale_from_space_deref` plants a #7184/#7192-shaped stale from-space pointer and asserts the instrument distinguishes it from the live object that would otherwise be recycled into those bytes — so a green protected run means the detector works, not that nothing was tried. + +| knob | gates EXACTLY | does NOT | +|---|---|---| +| `PERRY_GC_PROTECT_FROMSPACE=1` (or `poison`) | the from-space reset performed by the **copying minor** (`arena::copying_reset_from_spaces_and_flip`). Retired Eden + active-survivor blocks are detached into a bounded quarantine, poison-filled (`0xDEADBEEFBAADF0DE`, `obj_type = 0xDE`) and, at `=1`, `mprotect(PROT_NONE)`d. A stale deref then SIGSEGVs at the faulting instruction; the installed reporter names the address, the retiring minor, and the last-known object's `obj_type`/size, then restores `SIG_DFL` and re-faults so a core/debugger still sees the real site. `poison` skips `mprotect`. | change the non-moving minor's `arena_reset_empty_blocks`, the full mark-sweep's reclaim, old-gen defrag, or the malloc sweep. **A run with zero copying minors protects nothing** — check that `PERRY_GC_DIAG=1` prints a `[gc-fromspace-protect] retired_set=#N` line. | +| `PERRY_GC_PROTECT_FROMSPACE_DEPTH=N` (default 4) | how many retired page-sets stay quarantined. Evicted sets are restored to RW and **recycled back into Eden**, never `dealloc`'d, so footprint is bounded at `N × from-space bytes`. `0` is clamped to 1 — a depth of 0 would read as ON and protect nothing. **Raise this when a suspected bug does not fault**: a value can cross hundreds of collections between its last valid observation and its stale use (one per back-edge poll under zeal). #7154's `new C(…)` reproducer needs `800` — its constructor crosses 600 polls, so the default 4 misses it silently. | — | +| `PERRY_GC_ZEAL=1` | forces an evacuating minor at every **GC safepoint**: `js_gc_loop_safepoint` (loop back-edge) and the outermost microtask-pump safepoint. It bypasses exactly two things — the `GC_SAFEPOINT_PENDING` requirement in `js_gc_loop_safepoint`, and the `gc_budgeted_due_trigger()` "is anything due?" test in `gc_safepoint_moving_minor`. Also makes `gc_force_evacuate_enabled()` true, so survivors actually MOVE. | bypass `gc_safepoint_moving_minor`'s **entry guards**: a safepoint reached mid-allocation (`GC_FLAG_IN_ALLOC`), suppressed (`GC_FLAG_SUPPRESSED`), inside an unsafe FFI zone, under a non-zero `GC_ROOT_LOCK_DEPTH`, or during a budgeted cycle still returns without collecting. Nor does it override an explicit `PERRY_GEN_GC_EVACUATE=0` — that wins, and with it set zeal moves nothing and surfaces nothing. Nor does it emit loop polls — those need the **compile-time** `PERRY_GC_MOVING_LOOP_POLLS=1` (default off since #7161). Zeal on a binary compiled without polls only fires at event-loop boundaries; a compute-only loop never collects. Check `crate::gc::zeal_forced_collections()` is nonzero. There is deliberately **no level 2**: the alloc-point arm forces a conservative stack scan, which makes the copying minor ineligible, so an "every allocation" zeal would run non-moving minors and move nothing. | +| `PERRY_GC_FROMSPACE_SCAN_ABORT=1` | now **implies** `PERRY_GC_FROMSPACE_SCAN=1`. It used to be inert alone (the scan never ran, so nothing aborted, and the run reported success). | — | + +`PERRY_GC_ZEAL=1 PERRY_GC_PROTECT_FROMSPACE=1` together is the pairing that turns a #7154 bug into an immediate precise fault. Compile *and* run with `PERRY_GC_MOVING_LOOP_POLLS=1` for in-loop coverage. + ### GC knob kill-policy (binding) **Every GC env knob either has a required CI arm exercising its OFF state, or it is deleted after one release of soak.** At most one diagnostic-only knob may exist at a time, and it must be labelled untested. diff --git a/Cargo.lock b/Cargo.lock index 56c1ca9484..e4d770c964 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5503,7 +5503,7 @@ checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" [[package]] name = "perry" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "anyhow", "base64", @@ -5563,14 +5563,14 @@ dependencies = [ [[package]] name = "perry-api-manifest" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "serde", ] [[package]] name = "perry-audio-miniaudio" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "cc", "libc", @@ -5578,7 +5578,7 @@ dependencies = [ [[package]] name = "perry-codegen" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "anyhow", "log", @@ -5592,7 +5592,7 @@ dependencies = [ [[package]] name = "perry-codegen-arkts" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "anyhow", "perry-hir", @@ -5600,7 +5600,7 @@ dependencies = [ [[package]] name = "perry-codegen-glance" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "anyhow", "perry-hir", @@ -5608,7 +5608,7 @@ dependencies = [ [[package]] name = "perry-codegen-js" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "anyhow", "perry-dispatch", @@ -5617,7 +5617,7 @@ dependencies = [ [[package]] name = "perry-codegen-swiftui" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "anyhow", "perry-hir", @@ -5625,7 +5625,7 @@ dependencies = [ [[package]] name = "perry-codegen-wasm" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "anyhow", "base64", @@ -5637,7 +5637,7 @@ dependencies = [ [[package]] name = "perry-codegen-wear-tiles" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "anyhow", "perry-hir", @@ -5645,7 +5645,7 @@ dependencies = [ [[package]] name = "perry-container-compose" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "anyhow", "async-trait", @@ -5674,14 +5674,14 @@ dependencies = [ [[package]] name = "perry-container-e2e" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "anyhow", ] [[package]] name = "perry-diagnostics" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "serde", "serde_json", @@ -5689,7 +5689,7 @@ dependencies = [ [[package]] name = "perry-dispatch" -version = "0.5.1278" +version = "0.5.1277" [[package]] name = "perry-doc-fixture-my-bindings" @@ -5700,7 +5700,7 @@ dependencies = [ [[package]] name = "perry-doc-tests" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "anyhow", "clap", @@ -5715,7 +5715,7 @@ dependencies = [ [[package]] name = "perry-ext-ads" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "block2", "objc2", @@ -5725,7 +5725,7 @@ dependencies = [ [[package]] name = "perry-ext-argon2" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "argon2", "perry-ffi", @@ -5733,7 +5733,7 @@ dependencies = [ [[package]] name = "perry-ext-axios" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "perry-ffi", "reqwest", @@ -5742,7 +5742,7 @@ dependencies = [ [[package]] name = "perry-ext-bcrypt" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "bcrypt", "perry-ffi", @@ -5750,7 +5750,7 @@ dependencies = [ [[package]] name = "perry-ext-better-sqlite3" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "perry-ffi", "rusqlite", @@ -5758,7 +5758,7 @@ dependencies = [ [[package]] name = "perry-ext-cheerio" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "perry-ffi", "scraper", @@ -5766,7 +5766,7 @@ dependencies = [ [[package]] name = "perry-ext-commander" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "perry-ffi", "perry-runtime", @@ -5774,7 +5774,7 @@ dependencies = [ [[package]] name = "perry-ext-cron" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "chrono", "cron", @@ -5784,7 +5784,7 @@ dependencies = [ [[package]] name = "perry-ext-dayjs" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "chrono", "perry-ffi", @@ -5792,7 +5792,7 @@ dependencies = [ [[package]] name = "perry-ext-decimal" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "perry-ffi", "rust_decimal", @@ -5800,7 +5800,7 @@ dependencies = [ [[package]] name = "perry-ext-dotenv" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "perry-ffi", "serde_json", @@ -5808,7 +5808,7 @@ dependencies = [ [[package]] name = "perry-ext-ethers" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "perry-ffi", "rand 0.10.1", @@ -5816,7 +5816,7 @@ dependencies = [ [[package]] name = "perry-ext-events" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "perry-ffi", "perry-runtime", @@ -5824,14 +5824,14 @@ dependencies = [ [[package]] name = "perry-ext-exponential-backoff" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-fastify" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "bytes", "http-body-util", @@ -5849,7 +5849,7 @@ dependencies = [ [[package]] name = "perry-ext-fetch" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "bytes", "lazy_static", @@ -5862,7 +5862,7 @@ dependencies = [ [[package]] name = "perry-ext-http" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "bytes", "h2", @@ -5886,7 +5886,7 @@ dependencies = [ [[package]] name = "perry-ext-ioredis" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "lazy_static", "perry-ffi", @@ -5896,7 +5896,7 @@ dependencies = [ [[package]] name = "perry-ext-jsonwebtoken" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "base64", "jsonwebtoken", @@ -5907,7 +5907,7 @@ dependencies = [ [[package]] name = "perry-ext-lru-cache" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "lru", "perry-ffi", @@ -5916,7 +5916,7 @@ dependencies = [ [[package]] name = "perry-ext-moment" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "chrono", "perry-ffi", @@ -5924,7 +5924,7 @@ dependencies = [ [[package]] name = "perry-ext-mongodb" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "bson", "futures-util", @@ -5936,7 +5936,7 @@ dependencies = [ [[package]] name = "perry-ext-mysql2" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "chrono", "perry-ffi", @@ -5946,7 +5946,7 @@ dependencies = [ [[package]] name = "perry-ext-nanoid" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "nanoid", "perry-ffi", @@ -5955,7 +5955,7 @@ dependencies = [ [[package]] name = "perry-ext-net" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "bytes", "perry-ffi", @@ -5968,7 +5968,7 @@ dependencies = [ [[package]] name = "perry-ext-node-forge" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "const-oid 0.9.6", "der 0.7.10", @@ -5987,7 +5987,7 @@ dependencies = [ [[package]] name = "perry-ext-nodemailer" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "lettre", "perry-ffi", @@ -5997,7 +5997,7 @@ dependencies = [ [[package]] name = "perry-ext-pdf" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "perry-ffi", "printpdf", @@ -6005,7 +6005,7 @@ dependencies = [ [[package]] name = "perry-ext-pg" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "perry-ffi", "sqlx", @@ -6014,7 +6014,7 @@ dependencies = [ [[package]] name = "perry-ext-ratelimit" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "governor", "perry-ffi", @@ -6022,7 +6022,7 @@ dependencies = [ [[package]] name = "perry-ext-sharp" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "fast_image_resize", "image", @@ -6032,14 +6032,14 @@ dependencies = [ [[package]] name = "perry-ext-slugify" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-streams" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "lazy_static", "perry-ffi", @@ -6048,7 +6048,7 @@ dependencies = [ [[package]] name = "perry-ext-undici" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "perry-ffi", "perry-runtime", @@ -6057,7 +6057,7 @@ dependencies = [ [[package]] name = "perry-ext-uuid" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "perry-ffi", "uuid", @@ -6065,7 +6065,7 @@ dependencies = [ [[package]] name = "perry-ext-validator" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "perry-ffi", "regex", @@ -6075,7 +6075,7 @@ dependencies = [ [[package]] name = "perry-ext-ws" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "futures-util", "lazy_static", @@ -6088,7 +6088,7 @@ dependencies = [ [[package]] name = "perry-ext-zlib" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "brotli", "flate2", @@ -6098,7 +6098,7 @@ dependencies = [ [[package]] name = "perry-ffi" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "dashmap", "once_cell", @@ -6107,7 +6107,7 @@ dependencies = [ [[package]] name = "perry-hir" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "anyhow", "perry-api-manifest", @@ -6125,7 +6125,7 @@ dependencies = [ [[package]] name = "perry-parser" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "anyhow", "perry-diagnostics", @@ -6137,7 +6137,7 @@ dependencies = [ [[package]] name = "perry-runtime" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "anyhow", "base64", @@ -6178,14 +6178,14 @@ dependencies = [ [[package]] name = "perry-runtime-static" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "perry-runtime", ] [[package]] name = "perry-stdlib" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "aes 0.8.4", "aes 0.9.1", @@ -6280,14 +6280,14 @@ dependencies = [ [[package]] name = "perry-stdlib-static" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "perry-stdlib", ] [[package]] name = "perry-transform" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "anyhow", "perry-hir", @@ -6296,14 +6296,14 @@ dependencies = [ [[package]] name = "perry-ui" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "perry-ui-model", ] [[package]] name = "perry-ui-android" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "base64", "itoa", @@ -6320,7 +6320,7 @@ dependencies = [ [[package]] name = "perry-ui-geisterhand" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "rand 0.10.1", "serde", @@ -6330,7 +6330,7 @@ dependencies = [ [[package]] name = "perry-ui-gtk4" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "base64", "cairo-rs 0.22.0", @@ -6353,7 +6353,7 @@ dependencies = [ [[package]] name = "perry-ui-ios" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "base64", "block2", @@ -6369,7 +6369,7 @@ dependencies = [ [[package]] name = "perry-ui-macos" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "base64", "block2", @@ -6384,7 +6384,7 @@ dependencies = [ [[package]] name = "perry-ui-model" -version = "0.5.1278" +version = "0.5.1277" [[package]] name = "perry-ui-test" @@ -6395,11 +6395,11 @@ dependencies = [ [[package]] name = "perry-ui-testkit" -version = "0.5.1278" +version = "0.5.1277" [[package]] name = "perry-ui-tvos" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "base64", "block2", @@ -6415,7 +6415,7 @@ dependencies = [ [[package]] name = "perry-ui-visionos" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "base64", "block2", @@ -6431,7 +6431,7 @@ dependencies = [ [[package]] name = "perry-ui-watchos" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "block2", "libc", @@ -6444,7 +6444,7 @@ dependencies = [ [[package]] name = "perry-ui-windows" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "base64", "libc", @@ -6461,14 +6461,14 @@ dependencies = [ [[package]] name = "perry-ui-windows-winui" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "perry-ui-windows", ] [[package]] name = "perry-updater" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "anyhow", "base64", @@ -6484,7 +6484,7 @@ dependencies = [ [[package]] name = "perry-wasm-host" -version = "0.5.1278" +version = "0.5.1277" dependencies = [ "wasmi", ] diff --git a/Cargo.toml b/Cargo.toml index 0fdbe5e04c..08317d4427 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -292,7 +292,7 @@ codegen-units = 16 codegen-units = 16 [workspace.package] -version = "0.5.1278" +version = "0.5.1277" edition = "2021" license = "MIT" repository = "https://github.com/PerryTS/perry" diff --git a/changelog.d/7196-gc-rooting-bug-instruments.md b/changelog.d/7196-gc-rooting-bug-instruments.md new file mode 100644 index 0000000000..9f0b318a70 --- /dev/null +++ b/changelog.d/7196-gc-rooting-bug-instruments.md @@ -0,0 +1,15 @@ +feat(gc): instruments that make a #7154-class rooting bug fault precisely instead of a cycle later. + +A GC value that is live but not rooted across a collection point leaves nothing behind at collection time — there is literally nothing for the collector to find. The nursery recycles the address on the very next allocation, the stale pointer reads a valid unrelated object, and the program dies one or more cycles later, in a different function, as `TypeError: value is not a function`. Ten investigation rounds on #7154 were spent on that detection latency rather than on the bug. Three default-off instruments collapse it. + +**1. From-space quarantine / protection — `PERRY_GC_PROTECT_FROMSPACE`.** After an evacuating minor, do not recycle from-space. `arena/quarantine.rs` detaches the retired Eden + active-survivor blocks (leaving `data = null` tombstones so block-index semantics are unchanged), fills them with a poison word whose first byte reads as an invalid `obj_type` (`0xDE`), and `mprotect(PROT_NONE)`s their page-aligned interior. A stale dereference now SIGSEGVs **at the faulting instruction**, with the holder still live on the stack. The installed SIGSEGV/SIGBUS reporter prints the faulting address, which minor retired it, the last-known object that occupied that offset (`obj_type` + size, from a census taken before poisoning) and a native backtrace, then restores `SIG_DFL` and returns so the instruction re-faults — a core file, debugger or crash reporter still sees the real site. `=poison` selects poison without `mprotect`, which is also what the sub-page block edges `mprotect` cannot cover always get; protected and poisoned byte counts are reported separately so a run can never claim page protection it did not get. `PERRY_GC_PROTECT_FROMSPACE_DEPTH` (default 4, minimum 1) bounds memory: the quarantine is a ring, and expired sets are restored to read/write and recycled **back into Eden** rather than freed, so nothing that was ever `mprotect`ed is handed to the system allocator and steady-state footprint is `depth × from-space bytes`. + +**2. GC zeal — `PERRY_GC_ZEAL=1`.** Force an evacuating minor at every GC safepoint (loop back-edge polls and the outermost microtask-pump boundary) instead of only when nursery pressure is due, so an unrooted value moves on its FIRST exposure rather than whenever an unrelated allocation burst happens to line up. Implies `gc_force_evacuate_enabled()` — a zealous minor that left survivors in place would move nothing and could not surface the bug — while still losing to an explicit `PERRY_GEN_GC_EVACUATE=0`, so the two knobs cannot silently disagree. + +**3. Verify-roots gap closures.** `PERRY_GC_FROMSPACE_SCAN_ABORT=1` now **implies** `PERRY_GC_FROMSPACE_SCAN=1`; on its own it used to be completely inert (`run_fromspace_scan` returned at the enablement gate, so there was nothing to abort and the run reported success — an investigator reaching for the abort switch mid-hunt got a green run and no scan). The abort path now also prints a collector backtrace, and every offender sample reports the **target's** `obj_type` alongside the owner's, which is the field that distinguishes a dead closure (`4`) from a dead object (`2`) when triaging `value is not a function`. + +**Documented against the knob kill-policy.** CLAUDE.md and `docs/src/internals/memory-model.md` gain a table stating what each knob gates *exactly* and, as importantly, what it does not — prior rounds were misled by knobs whose real effect differed from their name. Two caveats are called out explicitly because both can produce a vacuously green run: `PERRY_GC_PROTECT_FROMSPACE` gates **only** the copying minor's from-space reset (a run with zero copying minors protects nothing — check for the `[gc-fromspace-protect] retired_set=#N` line under `PERRY_GC_DIAG=1`), and `PERRY_GC_ZEAL` cannot emit loop back-edge polls that codegen never produced (those need the compile-time `PERRY_GC_MOVING_LOOP_POLLS=1`, default off since #7161; `crate::gc::zeal_forced_collections()` is the live-subject counter). There is deliberately no `PERRY_GC_ZEAL=2` "every allocation" level: the allocation-point arm forces a conservative stack scan, which makes the copying minor ineligible, so that level would run non-moving minors and move nothing — the exact shape of the `PERRY_GC_FORCE_EVACUATE` inertness defect (#6942 / #6946). + +**Exercised, not dark.** The kill-policy's requirement is an arm that exercises the knob, and these get two. `gc/tests/fromspace_protect.rs` (12 tests, in the required `cargo-test` gate) asserts **both** states of every boolean knob — the OFF arm proves the collector is byte-for-byte unchanged when the instrument is off, the ON arm asserts its subject was live (an object actually moved) before believing the result — and `quarantine_catches_a_planted_stale_from_space_deref` **sabotage-tests** the detector: it plants the #7184 / #7192 shape (a mutator keeping a pre-collection address across an evacuating minor), then asserts the instrument reports poison where the un-instrumented control reads a valid recycled object. That control is what makes the verdict meaningful — it demonstrates the bug is genuinely invisible without the instrument. On top of that, `scripts/gc_instrument_smoke.sh` runs in `gc-stress` as a gating step covering the integrated path a unit test cannot reach (codegen emitting back-edge polls → zeal firing on them → the copying minor → quarantine retirement in a real compiled program); it fails unless the zeal arm forces strictly more collections than the pressure-only arm, so it cannot go green having run zero copying minors. + +Platform note: page protection is Unix-only. `mprotect` / `sigaction` / `sysconf` are not exposed by the `libc` crate on `x86_64-pc-windows-msvc`, a target `perry-runtime` is genuinely built for (`test.yml`'s `windows-build`, and `release-packages.yml` via `perry-ui-windows`). The syscall helpers are `cfg(unix)`-gated per the existing `pty::native` precedent; off Unix `=1` degrades to `poison`, visibly rather than silently, because `bytes_protected` stays 0 while `bytes_poisoned` counts the whole retired range. diff --git a/crates/perry-runtime/src/arena/mod.rs b/crates/perry-runtime/src/arena/mod.rs index 6c361783da..80cb3a1c6e 100644 --- a/crates/perry-runtime/src/arena/mod.rs +++ b/crates/perry-runtime/src/arena/mod.rs @@ -12,6 +12,9 @@ mod allocators; mod block; mod inline; mod page_meta; +/// #7154 tooling: from-space quarantine + poison + `mprotect` so a stale +/// pointer faults at the instruction that used it. Default-off. +mod quarantine; mod reset; mod stats; mod walk; @@ -81,6 +84,15 @@ pub(crate) use reset::{ }; pub use reset::{arena_reset_all_blocks_to_zero, arena_reset_empty_blocks}; +// quarantine.rs (#7154 from-space protection; default-off) +pub(crate) use quarantine::{copying_quarantine_from_spaces_and_flip, protect_fromspace_enabled}; +#[cfg(test)] +pub(crate) use quarantine::{ + parse_protection_mode, parse_quarantine_depth, quarantine_depth, FromSpaceProtection, + ProtectionModeGuard, QUARANTINE_POISON_OBJ_TYPE, QUARANTINE_POISON_WORD, +}; +pub use quarantine::{quarantine_stats, QuarantineStats}; + // stats.rs pub(crate) use stats::{active_survivor_space, inactive_survivor_space}; pub use stats::{ diff --git a/crates/perry-runtime/src/arena/quarantine.rs b/crates/perry-runtime/src/arena/quarantine.rs new file mode 100644 index 0000000000..9d316de83b --- /dev/null +++ b/crates/perry-runtime/src/arena/quarantine.rs @@ -0,0 +1,1050 @@ +//! From-space quarantine, poison and page protection (#7154 tooling). +//! +//! # What this exists to fix +//! +//! After an evacuating minor, `copying_reset_from_spaces_and_flip` sets every +//! Eden / active-survivor block's `offset` back to 0 and the bump allocator +//! starts handing the same bytes out again on the very next allocation. A +//! mutator register that still holds a *from-space* address — the #7154 family +//! of codegen rooting bugs — therefore does not fault. It reads a perfectly +//! valid, freshly-constructed, completely unrelated object. The program keeps +//! running and dies one or more cycles later, in a different function, as +//! `TypeError: value is not a function`. +//! +//! Detection latency is the whole problem: the fault is separated from its +//! cause by an arbitrary amount of execution. +//! +//! This module removes the recycling. Under `PERRY_GC_PROTECT_FROMSPACE` the +//! evacuated from-space blocks are not reset — they are **detached from the +//! arena** into a bounded quarantine ring, poisoned, and (in the default mode) +//! `mprotect(PROT_NONE)`d. A stale dereference then faults **at the faulting +//! instruction**, with the holder still on the stack, and the installed +//! SIGSEGV/SIGBUS reporter names the address, the minor that retired it, and +//! the last-known object that lived there. +//! +//! # What the knob actually gates (read this before trusting a run) +//! +//! `PERRY_GC_PROTECT_FROMSPACE` gates **only** the from-space reset performed +//! by the **copying (evacuating) minor** — `copying_reset_from_spaces_and_flip` +//! and nothing else. It does *not*: +//! +//! - touch the non-moving minor's in-place sweep (`arena_reset_empty_blocks`), +//! - touch the full mark-sweep's block reclaim, +//! - touch old-gen defrag or the malloc registry sweep, +//! - make a collection happen that would not otherwise have happened. +//! +//! So a run with this knob on and **zero copying minors** protects nothing and +//! proves nothing. `quarantine_stats()` reports `sets_retired` precisely so a +//! green result can be checked against its subject having been live (CLAUDE.md, +//! "four ways a gate can be unable to fail", #4). Pair it with +//! `PERRY_GC_ZEAL=1` to guarantee evacuating minors actually run. +//! +//! # Modes +//! +//! | value | effect | +//! |---|---| +//! | unset / `0` / `off` / `false` | inert — the normal reset path runs, byte-for-byte | +//! | `1` / `on` / `true` | poison **and** `mprotect(PROT_NONE)` the page-aligned interior | +//! | `poison` | poison only, no `mprotect` (for hosts/tests where faulting is not wanted) | +//! +//! `mprotect` needs page granularity and arena blocks are `alloc`'d at 16-byte +//! alignment, so the *page-aligned interior* of each block is protected and the +//! (at most two) sub-page edge fragments are poison-filled instead. Both are +//! counted separately — a protected-byte total that silently collapsed to zero +//! would read exactly like a clean run. +//! +//! # Platform support +//! +//! Page protection and the fault reporter are **Unix-only**: they are built on +//! `mprotect` / `sigaction` / `sysconf`, none of which the `libc` crate exposes +//! on `x86_64-pc-windows-msvc` — a target `perry-runtime` really is compiled +//! for (`test.yml`'s `windows-build` job, and `release-packages.yml` via +//! `perry-ui-windows` → `perry-runtime`). On non-Unix targets `ProtectPages` +//! degrades to poison-only rather than failing to build. That degradation is +//! *observable, not silent*: `mprotect_range` reports failure, so `protected` +//! stays `None` and `bytes_protected` stays 0 while `bytes_poisoned` counts the +//! whole retired range — which is exactly what the separate counters exist to +//! reveal. +//! +//! # Memory bound +//! +//! `PERRY_GC_PROTECT_FROMSPACE_DEPTH` (default 4) caps how many retired +//! page-sets stay quarantined. Evicting a set restores `PROT_READ|PROT_WRITE` +//! and hands the blocks **back to Eden** rather than freeing them, so the +//! quarantine is a ring buffer: steady-state footprint is bounded by +//! `depth × from-space bytes` and no `mprotect`ed page is ever handed to +//! `dealloc`. +//! +//! **Depth is the knob to raise when a suspected bug does not fault.** A stale +//! pointer is only caught while the page-set it names is still quarantined, and +//! under `PERRY_GC_ZEAL=1` a value can cross *hundreds* of collections between +//! its last valid observation and its stale use — one per loop back-edge poll. +//! Measured on #7154's `new C(…)` reproducer: the constructor body runs 600 +//! polls, so the caller's stale register is 600 retirements old by the time +//! `js_ctor_return_override` publishes it, and the default depth of 4 misses it +//! silently. `PERRY_GC_PROTECT_FROMSPACE_DEPTH=800` faults on the first use. +//! Rule of thumb: depth ≥ the number of safepoints the suspect value survives. +//! +//! Two consequences worth knowing before reading numbers off a protected run, +//! both direct and both only under the knob: +//! +//! - Quarantined bytes are subtracted from `ARENA_TOTAL_BYTES` when the block +//! leaves the arena, so `arena_total_bytes()` — and therefore the arena-bytes +//! GC trigger — under-reports real RSS by up to `depth × from-space bytes`. +//! Fewer automatic triggers, not more. Pair with `PERRY_GC_ZEAL=1` if the +//! point of the run is collection frequency. +//! - RSS is genuinely higher than an unprotected run for the same reason. This +//! is a debug instrument; do not benchmark under it. + +use super::*; +use std::sync::atomic::{AtomicBool, AtomicU64, Ordering as AtomicOrdering}; +use std::sync::Mutex; + +/// Poison fill word. Chosen so that reading the first 8 bytes as a `GcHeader` +/// yields `obj_type = 0xDE` — outside every `GC_TYPE_*` constant — and a +/// nonsense `size`, so a walker or type dispatch trips immediately instead of +/// wandering. As a NaN-boxed value it is not a pointer tag either. +pub(crate) const QUARANTINE_POISON_WORD: u64 = + 0xDEAD_BEEF_BAAD_F000 | QUARANTINE_POISON_OBJ_TYPE as u64; + +/// `obj_type` byte a poisoned allocation presents — the low byte of +/// [`QUARANTINE_POISON_WORD`], because `GcHeader` is `#[repr(C)]` with +/// `obj_type` first. Deliberately not a member of the `GC_TYPE_*` family. +pub(crate) const QUARANTINE_POISON_OBJ_TYPE: u8 = 0xDE; + +const DEFAULT_QUARANTINE_DEPTH: usize = 4; + +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub(crate) enum FromSpaceProtection { + /// Knob unset or explicitly off — the ordinary reset path runs. + Off, + /// Poison the retired bytes, do not change page protection. + PoisonOnly, + /// Poison, then `mprotect(PROT_NONE)` the page-aligned interior. + ProtectPages, +} + +/// Pure knob parse, so the mapping is testable without mutating process +/// environment (the live reader caches in a `OnceLock`, as every other GC knob +/// does, and a cached knob cannot be re-read by a later test). +pub(crate) fn parse_protection_mode(raw: Option<&str>) -> FromSpaceProtection { + match raw { + Some("1") | Some("on") | Some("true") => FromSpaceProtection::ProtectPages, + Some("poison") => FromSpaceProtection::PoisonOnly, + _ => FromSpaceProtection::Off, + } +} + +#[cfg(test)] +thread_local! { + /// Test-only override. Thread-local, so one test enabling the instrument + /// cannot change the collector's behaviour for any other test. + static MODE_OVERRIDE: Cell> = const { Cell::new(None) }; +} + +pub(crate) fn fromspace_protection_mode() -> FromSpaceProtection { + #[cfg(test)] + if let Some(mode) = MODE_OVERRIDE.with(Cell::get) { + return mode; + } + use std::sync::OnceLock; + static CACHED: OnceLock = OnceLock::new(); + *CACHED.get_or_init(|| { + parse_protection_mode(std::env::var("PERRY_GC_PROTECT_FROMSPACE").ok().as_deref()) + }) +} + +/// RAII test override for the protection mode. +#[cfg(test)] +pub(crate) struct ProtectionModeGuard(Option); + +#[cfg(test)] +impl ProtectionModeGuard { + pub(crate) fn set(mode: FromSpaceProtection) -> Self { + let previous = MODE_OVERRIDE.with(|cell| cell.replace(Some(mode))); + Self(previous) + } +} + +#[cfg(test)] +impl Drop for ProtectionModeGuard { + fn drop(&mut self) { + MODE_OVERRIDE.with(|cell| cell.set(self.0)); + } +} + +#[inline] +pub(crate) fn protect_fromspace_enabled() -> bool { + fromspace_protection_mode() != FromSpaceProtection::Off +} + +/// Pure knob parse for the quarantine depth. `0` and unparsable values are +/// rejected: a depth of zero would evict every set on the cycle it was created, +/// making the instrument inert while still reading as ON. +pub(crate) fn parse_quarantine_depth(raw: Option<&str>) -> usize { + raw.and_then(|raw| raw.parse::().ok()) + .map(|depth| depth.max(1)) + .unwrap_or(DEFAULT_QUARANTINE_DEPTH) +} + +/// How many retired page-sets stay quarantined before the oldest is recycled. +pub(crate) fn quarantine_depth() -> usize { + use std::sync::OnceLock; + static CACHED: OnceLock = OnceLock::new(); + *CACHED.get_or_init(|| { + parse_quarantine_depth( + std::env::var("PERRY_GC_PROTECT_FROMSPACE_DEPTH") + .ok() + .as_deref(), + ) + }) +} + +#[cfg(unix)] +fn page_size() -> usize { + use std::sync::OnceLock; + static CACHED: OnceLock = OnceLock::new(); + *CACHED.get_or_init(|| { + let raw = unsafe { libc::sysconf(libc::_SC_PAGESIZE) }; + if raw > 0 { + raw as usize + } else { + 4096 + } + }) +} + +/// Non-Unix: no `sysconf`. Only used to size the would-be protected interior, +/// which `mprotect_range` then declines to protect, so the value is inert. +#[cfg(not(unix))] +fn page_size() -> usize { + 4096 +} + +/// One object that lived in a quarantined block, kept so a fault reporter can +/// say *what* the stale pointer used to point at. 12 bytes packed; a 1 MB block +/// of 48-byte objects costs ~256 KB of census. +#[derive(Clone, Copy)] +struct CensusEntry { + /// Byte offset of the **user pointer** (header + 8) within the block. + user_offset: u32, + size: u32, + obj_type: u8, +} + +struct QuarantinedBlock { + data: *mut u8, + size: usize, + /// Bytes that were in use when the block was retired. + used: usize, + /// `mprotect`ed sub-range, if any: `(base, len)`. + protected: Option<(usize, usize)>, + census: Vec, +} + +// SAFETY: the raw pointer is an owned arena block. The registry that holds +// these is only read (a) by the owning thread and (b) by the fault reporter, +// which reads addresses and never dereferences the payload. +unsafe impl Send for QuarantinedBlock {} + +struct QuarantinedSet { + /// Which retirement this was; monotonically increasing per process. + seq: u64, + blocks: Vec, +} + +static QUARANTINE_SEQ: AtomicU64 = AtomicU64::new(0); +static SETS_RETIRED: AtomicU64 = AtomicU64::new(0); +static BLOCKS_QUARANTINED: AtomicU64 = AtomicU64::new(0); +static BYTES_PROTECTED: AtomicU64 = AtomicU64::new(0); +static BYTES_POISONED: AtomicU64 = AtomicU64::new(0); +static BLOCKS_RECYCLED: AtomicU64 = AtomicU64::new(0); + +/// Process-global so the fault reporter — which can run on any thread and must +/// not touch a `thread_local!` — can resolve a faulting address. Read from the +/// signal handler with `try_lock` only. +static REGISTRY: Mutex> = Mutex::new(Vec::new()); + +/// Live snapshot for tests and for `PERRY_GC_DIAG` reporting. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub struct QuarantineStats { + /// Number of copying minors whose from-space this instrument retired. A + /// clean verdict with `sets_retired == 0` means the instrument never ran. + pub sets_retired: u64, + pub blocks_quarantined: u64, + pub bytes_protected: u64, + pub bytes_poisoned: u64, + /// Blocks whose quarantine expired and which went back into Eden. + pub blocks_recycled: u64, + /// Page-sets currently held. + pub sets_held: usize, +} + +pub fn quarantine_stats() -> QuarantineStats { + QuarantineStats { + sets_retired: SETS_RETIRED.load(AtomicOrdering::Relaxed), + blocks_quarantined: BLOCKS_QUARANTINED.load(AtomicOrdering::Relaxed), + bytes_protected: BYTES_PROTECTED.load(AtomicOrdering::Relaxed), + bytes_poisoned: BYTES_POISONED.load(AtomicOrdering::Relaxed), + blocks_recycled: BLOCKS_RECYCLED.load(AtomicOrdering::Relaxed), + sets_held: REGISTRY.lock().map(|sets| sets.len()).unwrap_or(0), + } +} + +/// Walk `[data, data + used)` header-by-header, recording what lived there. +/// +/// Mirrors the arena walker's layout assumptions (8-byte `GcHeader`, `size` +/// covers header + payload, 8-byte stride). Stops at the first inconsistency +/// rather than guessing — a partial census still names most addresses, and a +/// wrong one would be a lying instrument. +/// +/// # Safety +/// `data` must point at `used` readable bytes of a retired arena block. +unsafe fn build_census(data: *mut u8, used: usize) -> Vec { + let mut census = Vec::new(); + let mut pos = 0usize; + while pos + crate::gc::GC_HEADER_SIZE <= used { + let header = data.add(pos) as *const crate::gc::GcHeader; + let size = (*header).size as usize; + if size < crate::gc::GC_HEADER_SIZE || pos + size > used { + break; + } + census.push(CensusEntry { + user_offset: (pos + crate::gc::GC_HEADER_SIZE) as u32, + size: size as u32, + obj_type: (*header).obj_type, + }); + pos += (size + 7) & !7; + } + census +} + +/// Which censused object, if any, covers `offset` within its block. +/// +/// Matches an object's WHOLE extent, header included: +/// `[user_offset - GC_HEADER_SIZE, user_offset - GC_HEADER_SIZE + size)`. +/// Both bounds are load-bearing and each was wrong at some point: +/// +/// - `size` covers header + payload while `user_offset` already skips the +/// header, so bounding at `user_offset + size` overshoots by `GC_HEADER_SIZE` +/// and attributes an address in the NEXT object's header to this one — the +/// report then confidently names the wrong last-known object. +/// - Bounding at the payload end instead (`user_offset + size - GC_HEADER_SIZE`) +/// leaves *header* addresses attributed to nobody, and a raw header address is +/// exactly what this family of bugs hands you: #7192's shape publishes +/// `%inst`, the pre-header allocation pointer, so the faulting address lands +/// on an object's header rather than inside its payload. Measured on the +/// reverted-#7192 reproducer, that turned a correct attribution into +/// "(no census entry covers this offset)". +/// +/// Addresses in the inter-object padding left by the `(size + 7) & !7` stride +/// belong to no object and correctly return `None`. +fn census_lookup(census: &[CensusEntry], offset: usize) -> Option<(u32, u8, u32)> { + census + .iter() + .rev() + .find(|entry| { + let start = (entry.user_offset as usize).saturating_sub(crate::gc::GC_HEADER_SIZE); + offset >= start && offset < start + entry.size as usize + }) + .map(|entry| (entry.user_offset, entry.obj_type, entry.size)) +} + +/// Fill `[data, data + used)` with the poison word. +/// +/// # Safety +/// `data` must point at `used` writable bytes of a retired arena block. +unsafe fn poison(data: *mut u8, used: usize) { + let words = used / 8; + let ptr = data as *mut u64; + for i in 0..words { + ptr.add(i).write(QUARANTINE_POISON_WORD); + } + let tail_start = words * 8; + for i in tail_start..used { + data.add(i) + .write((QUARANTINE_POISON_WORD >> (8 * (i % 8))) as u8); + } +} + +/// The page-aligned interior of `[base, base + size)`, or `None` when the block +/// is too small / too badly aligned to contain a whole page. +fn page_interior(base: usize, size: usize) -> Option<(usize, usize)> { + let page = page_size(); + let start = base.checked_add(page - 1)? & !(page - 1); + let end = (base.checked_add(size)?) & !(page - 1); + if end > start { + Some((start, end - start)) + } else { + None + } +} + +#[cfg(unix)] +fn mprotect_range(base: usize, len: usize, prot: libc::c_int) -> bool { + // SAFETY: `base`/`len` are page-aligned and describe memory this process + // owns (an arena block detached from the arena and held by the quarantine). + unsafe { libc::mprotect(base as *mut libc::c_void, len, prot) == 0 } +} + +/// Non-Unix: there is no `mprotect`. Reporting failure is what makes the +/// degradation to poison-only visible in `bytes_protected` rather than silent. +#[cfg(not(unix))] +fn mprotect_range(_base: usize, _len: usize, _prot: i32) -> bool { + false +} + +fn unprotect(block: &QuarantinedBlock) { + if let Some((base, len)) = block.protected { + #[cfg(unix)] + mprotect_range(base, len, libc::PROT_READ | libc::PROT_WRITE); + // Unreachable on non-Unix: `protected` is only ever `Some` when + // `mprotect_range` succeeded, and there it always fails. + #[cfg(not(unix))] + let _ = (base, len); + } +} + +/// Retire one detached from-space block into the pending set. +/// +/// # Safety +/// `data` must be an arena block of `size` bytes that has been removed from its +/// arena (tombstoned) and unregistered from the page metadata, with `used` +/// bytes of retired object data at its base. +unsafe fn retire_block( + data: *mut u8, + size: usize, + used: usize, + mode: FromSpaceProtection, +) -> QuarantinedBlock { + let census = build_census(data, used); + poison(data, used.min(size)); + let mut protected = None; + if mode == FromSpaceProtection::ProtectPages { + #[cfg(unix)] + let prot_none = libc::PROT_NONE; + #[cfg(not(unix))] + let prot_none = 0i32; + if let Some((base, len)) = page_interior(data as usize, size) { + if mprotect_range(base, len, prot_none) { + protected = Some((base, len)); + BYTES_PROTECTED.fetch_add(len as u64, AtomicOrdering::Relaxed); + } + } + } + // Poisoned-but-unprotected bytes: the whole retired range when in + // poison-only mode, otherwise the sub-page edge fragments mprotect could + // not cover. Counted separately so a run can never claim page protection it + // did not get. + let poisoned_uncovered = match protected { + Some((base, len)) => { + let head = base.saturating_sub(data as usize); + let tail = size.saturating_sub(head + len); + (head + tail).min(used) + } + None => used, + }; + BYTES_POISONED.fetch_add(poisoned_uncovered as u64, AtomicOrdering::Relaxed); + BLOCKS_QUARANTINED.fetch_add(1, AtomicOrdering::Relaxed); + QuarantinedBlock { + data, + size, + used, + protected, + census, + } +} + +/// Push a freshly-retired page-set and evict past the depth bound. Returns the +/// blocks whose quarantine expired, restored to read/write and ready to go back +/// into Eden. +fn push_set_and_evict(blocks: Vec) -> Vec { + let seq = QUARANTINE_SEQ.fetch_add(1, AtomicOrdering::Relaxed); + let depth = quarantine_depth(); + let mut evicted = Vec::new(); + if let Ok(mut sets) = REGISTRY.lock() { + // Counted only once the set is actually registered. `sets_retired` is + // the live-subject evidence for every protected run (CLAUDE.md, "a gate + // must assert its subject was live"), so it must never claim a + // retirement that `sets_held` cannot account for. + SETS_RETIRED.fetch_add(1, AtomicOrdering::Relaxed); + sets.push(QuarantinedSet { seq, blocks }); + while sets.len() > depth { + let oldest = sets.remove(0); + for block in oldest.blocks { + unprotect(&block); + evicted.push(block); + } + } + } else { + // Poisoned registry. These blocks are already detached from the arena, + // unregistered, and subtracted from `ARENA_TOTAL_BYTES`, and + // `QuarantinedBlock` has no `Drop` — dropping them here would leak the + // whole from-space. Recycle immediately instead. + for block in blocks { + unprotect(&block); + evicted.push(block); + } + } + BLOCKS_RECYCLED.fetch_add(evicted.len() as u64, AtomicOrdering::Relaxed); + evicted +} + +/// Quarantining variant of [`super::reset::copying_reset_from_spaces_and_flip`]. +/// +/// Detaches every from-space block that holds retired bytes, leaving the +/// arena's `blocks` vector the same LENGTH (a `data = null` tombstone in each +/// vacated slot) so block-index semantics — `active_survivor_block_index_range`, +/// `block_has_live`, the walkers — are unchanged. Recycled blocks from an +/// expired quarantine set are reinstalled into Eden. +pub(crate) fn copying_quarantine_from_spaces_and_flip() -> ArenaResetStats { + let mode = fromspace_protection_mode(); + debug_assert_ne!(mode, FromSpaceProtection::Off); + install_fault_reporter(); + sync_inline_arena_state(); + + let mut retired = Vec::new(); + let mut reset_blocks = 0usize; + let mut reusable_bytes = 0usize; + + // --- Eden ------------------------------------------------------------- + let eden_detached = ARENA.with(|arena| unsafe { + let arena = &mut *arena.get(); + detach_used_blocks(arena) + }); + for (data, size, used) in eden_detached { + reset_blocks += 1; + // SAFETY: `detach_used_blocks` tombstoned the slot and unregistered the + // block's page metadata, so nothing else can reach these bytes. + retired.push(unsafe { retire_block(data, size, used, mode) }); + } + + // --- active survivor from-space --------------------------------------- + let active = ACTIVE_SURVIVOR.with(|active| active.get()); + let survivor_detached = + with_survivor_arena_mut(active, |arena| unsafe { detach_used_blocks(arena) }); + for (data, size, used) in survivor_detached { + reset_blocks += 1; + // SAFETY: as above. + retired.push(unsafe { retire_block(data, size, used, mode) }); + } + + let recycled = push_set_and_evict(retired); + + if std::env::var_os("PERRY_GC_DIAG").is_some() { + let stats = quarantine_stats(); + eprintln!( + "[gc-fromspace-protect] mode={:?} retired_set=#{} blocks={} sets_held={}/{} bytes_protected={} bytes_poisoned={} blocks_recycled={}", + mode, + stats.sets_retired.saturating_sub(1), + reset_blocks, + stats.sets_held, + quarantine_depth(), + stats.bytes_protected, + stats.bytes_poisoned, + stats.blocks_recycled + ); + } + + // Hand expired blocks back to Eden rather than `dealloc`ing them: nothing + // that was ever `mprotect`ed is returned to the system allocator. + ARENA.with(|arena| unsafe { + let arena = &mut *arena.get(); + for block in recycled { + reusable_bytes = reusable_bytes.saturating_add(block.used); + arena.install_reserved_block(ArenaBlock { + data: block.data, + size: block.size, + offset: 0, + dead_cycles: 0, + }); + } + ensure_usable_current_block(arena); + crate::gc::ARENA_FREE_LIST.with(|fl| fl.borrow_mut().clear()); + crate::gc::ARENA_FREE_LIST_NONEMPTY.with(|c| c.set(false)); + INLINE_STATE.with(|s| { + let inline = &mut *s.get(); + if !inline.data.is_null() { + let block = &arena.blocks[arena.current]; + inline.data = block.data; + inline.offset = block.offset; + inline.size = block.size; + } + }); + }); + + with_survivor_arena_mut(active, |arena| { + arena.current = arena + .blocks + .iter() + .position(|block| !block.data.is_null()) + .unwrap_or(0); + }); + ACTIVE_SURVIVOR.with(|active_cell| active_cell.set(1 - active)); + + ArenaResetStats { + reset_blocks, + reusable_bytes, + deallocated_blocks: 0, + deallocated_bytes: 0, + } +} + +/// Remove every block with `offset != 0` from `arena`, leaving a tombstone in +/// its slot. Returns `(data, size, used)` for each. Blocks that were already +/// empty hold nothing retired and are left alone. +/// +/// # Safety +/// Caller must hold exclusive access to `arena`. +unsafe fn detach_used_blocks(arena: &mut Arena) -> Vec<(*mut u8, usize, usize)> { + let mut detached = Vec::new(); + for block in arena.blocks.iter_mut() { + if block.data.is_null() || block.offset == 0 { + continue; + } + let base = block.data as usize; + let size = block.size; + let used = block.offset; + unregister_block_generation(base, size); + ARENA_TOTAL_BYTES.with(|total| total.set(total.get().saturating_sub(size))); + detached.push((block.data, size, used)); + block.data = std::ptr::null_mut(); + block.size = 0; + block.offset = 0; + block.dead_cycles = 0; + } + arena.current = 0; + detached +} + +/// Point Eden's `current` at a live slot after detaching, installing a fresh +/// block if the whole region was retired. +/// +/// This is for `INLINE_STATE`, not for `Arena::alloc`. Allocation itself is +/// already tombstone-safe on every path: a tombstone has `size == 0`, so +/// `ArenaBlock::alloc` fails its `bumped > self.size` test and returns `None` +/// without ever dereferencing `data`; `try_alloc_after_gc` then forward-scans +/// the other blocks and `install_reserved_block` reuses the tombstone slot. +/// (`copying_quarantine_from_spaces_and_flip` leaves the *survivor* arena's +/// `current` on a tombstone in exactly that situation and relies on this — see +/// `survivor_current_on_tombstone_still_allocates_correctly`.) +/// +/// Eden is the one that cannot tolerate it, because the caller copies +/// `blocks[current]` straight into `INLINE_STATE` for codegen's inline bump +/// allocator, and a tombstone would publish `data = null, size = 0`. +/// +/// # Safety +/// Caller must hold exclusive access to `arena`. +unsafe fn ensure_usable_current_block(arena: &mut Arena) { + if arena + .blocks + .get(arena.current) + .is_some_and(|block| !block.data.is_null()) + { + return; + } + if let Some(idx) = arena + .blocks + .iter() + .position(|block| !block.data.is_null() && block.offset == 0) + { + arena.current = idx; + return; + } + arena.install_fresh_block(BLOCK_SIZE); +} + +// --------------------------------------------------------------------------- +// Fault reporter +// --------------------------------------------------------------------------- + +static REPORTER_INSTALLED: AtomicBool = AtomicBool::new(false); + +/// Non-Unix: no `sigaction`, so there is no reporter to install. `ProtectPages` +/// has already degraded to poison-only (see `mprotect_range`), so there is also +/// nothing that could fault. +#[cfg(not(unix))] +fn install_fault_reporter() {} + +/// Install the SIGSEGV/SIGBUS reporter, once, the first time a page-set is +/// retired. Only installed when the knob is on, so a normal build never touches +/// signal disposition. +#[cfg(unix)] +fn install_fault_reporter() { + if fromspace_protection_mode() != FromSpaceProtection::ProtectPages { + return; + } + if REPORTER_INSTALLED.swap(true, AtomicOrdering::SeqCst) { + return; + } + // SAFETY: standard `sigaction` install with a `SA_SIGINFO` handler. + unsafe { + let mut action: libc::sigaction = std::mem::zeroed(); + action.sa_sigaction = fromspace_fault_handler as *const () as usize; + action.sa_flags = libc::SA_SIGINFO | libc::SA_ONSTACK; + libc::sigemptyset(&mut action.sa_mask); + libc::sigaction(libc::SIGSEGV, &action, std::ptr::null_mut()); + libc::sigaction(libc::SIGBUS, &action, std::ptr::null_mut()); + } +} + +/// Minimal `write(2)`-based formatter. Deliberately avoids `format!`/`eprintln!` +/// so the common path through the handler does not allocate. +#[cfg(unix)] +struct FaultWriter { + buf: [u8; 1024], + len: usize, +} + +#[cfg(unix)] +impl FaultWriter { + fn new() -> Self { + Self { + buf: [0; 1024], + len: 0, + } + } + fn str(&mut self, s: &str) { + for &byte in s.as_bytes() { + if self.len < self.buf.len() { + self.buf[self.len] = byte; + self.len += 1; + } + } + } + fn hex(&mut self, mut value: usize) { + self.str("0x"); + let mut digits = [0u8; 16]; + let mut n = 0; + if value == 0 { + digits[0] = b'0'; + n = 1; + } + while value != 0 { + let nibble = (value & 0xf) as u8; + digits[n] = if nibble < 10 { + b'0' + nibble + } else { + b'a' + nibble - 10 + }; + n += 1; + value >>= 4; + } + for i in (0..n).rev() { + if self.len < self.buf.len() { + self.buf[self.len] = digits[i]; + self.len += 1; + } + } + } + fn dec(&mut self, value: u64) { + let mut digits = [0u8; 20]; + let mut n = 0; + let mut value = value; + if value == 0 { + digits[0] = b'0'; + n = 1; + } + while value != 0 { + digits[n] = b'0' + (value % 10) as u8; + n += 1; + value /= 10; + } + for i in (0..n).rev() { + if self.len < self.buf.len() { + self.buf[self.len] = digits[i]; + self.len += 1; + } + } + } + fn flush(&self) { + // SAFETY: writing `self.len` initialized bytes to stderr. + unsafe { + libc::write(2, self.buf.as_ptr() as *const libc::c_void, self.len); + } + } +} + +#[cfg(unix)] +extern "C" fn fromspace_fault_handler( + signum: libc::c_int, + info: *mut libc::siginfo_t, + _ctx: *mut libc::c_void, +) { + let addr = if info.is_null() { + 0usize + } else { + // SAFETY: `info` is the kernel-provided siginfo for a SIGSEGV/SIGBUS. + unsafe { (*info).si_addr() as usize } + }; + + let mut out = FaultWriter::new(); + // `try_lock`: never block inside a signal handler. A contended registry + // costs the census line, not the report. + let described = REGISTRY.try_lock().ok().and_then(|sets| { + sets.iter().find_map(|set| { + set.blocks.iter().find_map(|block| { + let base = block.data as usize; + if addr < base || addr >= base + block.size { + return None; + } + let offset = addr - base; + let entry = census_lookup(&block.census, offset); + Some((set.seq, base, block.used, entry)) + }) + }) + }); + + match described { + Some((seq, base, used, entry)) => { + out.str("\n[gc-fromspace-protect] FAULT: signal "); + out.dec(signum as u64); + out.str(" at "); + out.hex(addr); + out.str("\n This address is RETIRED FROM-SPACE. The evacuating minor moved or\n freed the object here and the holder kept the pre-collection address.\n block="); + out.hex(base); + out.str(" +"); + out.dec((addr - base) as u64); + out.str(" retired_bytes="); + out.dec(used as u64); + out.str(" retired_by_minor=#"); + out.dec(seq); + match entry { + Some((user_offset, obj_type, size)) => { + out.str("\n last-known object: user_ptr="); + out.hex(base + user_offset as usize); + out.str(" obj_type="); + out.dec(obj_type as u64); + out.str(" size="); + out.dec(size as u64); + } + None => { + out.str("\n last-known object: (no census entry covers this offset)"); + } + } + out.str("\n The faulting instruction IS the stale use. Backtrace:\n"); + out.flush(); + emit_native_backtrace(); + } + None => { + out.str("\n[gc-fromspace-protect] signal "); + out.dec(signum as u64); + out.str(" at "); + out.hex(addr); + out.str(" is NOT in quarantined from-space (unrelated fault)\n"); + out.flush(); + } + } + + // Restore the default disposition and return, so the instruction re-faults + // and the process dies exactly where it should — core file, debugger, + // crash reporter all see the real site. + // SAFETY: standard handler teardown. + unsafe { + let mut action: libc::sigaction = std::mem::zeroed(); + action.sa_sigaction = libc::SIG_DFL; + libc::sigemptyset(&mut action.sa_mask); + libc::sigaction(signum, &action, std::ptr::null_mut()); + } +} + +#[cfg(all(unix, any(target_os = "macos", target_os = "linux")))] +fn emit_native_backtrace() { + const MAX_FRAMES: usize = 64; + let mut frames = [std::ptr::null_mut::(); MAX_FRAMES]; + // SAFETY: `backtrace`/`backtrace_symbols_fd` are the async-signal-safe pair + // (`_fd` writes directly and does not call `malloc`). + unsafe { + let n = libc::backtrace(frames.as_mut_ptr(), MAX_FRAMES as libc::c_int); + if n > 0 { + libc::backtrace_symbols_fd(frames.as_ptr(), n, 2); + } + } +} + +#[cfg(all(unix, not(any(target_os = "macos", target_os = "linux"))))] +fn emit_native_backtrace() {} + +#[cfg(test)] +mod census_tests { + use super::*; + use crate::gc::GC_HEADER_SIZE; + + /// Two adjacent 48-byte objects at block offsets 0 and 48. + fn two_objects() -> Vec { + vec![ + CensusEntry { + user_offset: GC_HEADER_SIZE as u32, + size: 48, + obj_type: 1, + }, + CensusEntry { + user_offset: (48 + GC_HEADER_SIZE) as u32, + size: 48, + obj_type: 4, + }, + ] + } + + /// The fault report's "last-known object" line is the field an investigator + /// reads to tell a dead closure (`obj_type = 4`) from a dead object (`2`). + /// An instrument that names the WRONG object there is worse than one that + /// says nothing, so both bounds are pinned. + #[test] + fn census_attributes_headers_and_payloads_to_the_right_object() { + let census = two_objects(); + + // A payload address belongs to its own object. + assert_eq!( + census_lookup(&census, GC_HEADER_SIZE + 8).map(|e| e.1), + Some(1), + "a payload address must name the object that owns it" + ); + + // The LAST byte of object 0's payload is still object 0. + assert_eq!( + census_lookup(&census, 47).map(|e| e.1), + Some(1), + "the final payload byte must not spill into the next object" + ); + + // Offset 48 is object 1's HEADER. Before the fix this was attributed to + // object 0 (the `user_offset + size` overshoot); bounding at the payload + // end instead made it unattributed. It is object 1. + assert_eq!( + census_lookup(&census, 48).map(|e| (e.0, e.1)), + Some(((48 + GC_HEADER_SIZE) as u32, 4)), + "an object's header address must name THAT object, not its neighbour" + ); + + // Past the end of the last object: nobody. + assert_eq!( + census_lookup(&census, 96), + None, + "an offset beyond every censused object must not be attributed" + ); + + // Padding between objects belongs to nobody rather than to the object + // before it. + let padded = vec![CensusEntry { + user_offset: GC_HEADER_SIZE as u32, + size: 44, // 44 -> stride rounds to 48, leaving 4 pad bytes + obj_type: 2, + }]; + assert_eq!( + census_lookup(&padded, 43).map(|e| e.1), + Some(2), + "last real byte is still the object" + ); + assert_eq!( + census_lookup(&padded, 45), + None, + "stride padding must report no covering entry, not the previous object" + ); + } +} + +#[cfg(test)] +mod tombstone_tests { + use super::*; + + fn tombstone() -> ArenaBlock { + ArenaBlock { + data: std::ptr::null_mut(), + size: 0, + offset: 0, + dead_cycles: 0, + } + } + + /// `copying_quarantine_from_spaces_and_flip` can leave the survivor arena's + /// `current` pointing at a tombstone: `detach_used_blocks` tombstones every + /// block that held retired bytes, and when that is *all* of them the + /// `position(..).unwrap_or(0)` fixup lands on slot 0, which is itself a + /// tombstone (`data = null`, `size = 0`). + /// + /// Review flagged that as a Critical fault risk. It is not one, and this + /// pins why so a future change to the bump allocator cannot quietly make it + /// true: a tombstone has `size == 0`, so `ArenaBlock::alloc` fails its + /// `bumped > self.size` test and returns `None` **without dereferencing + /// `data`**; `try_alloc_after_gc` then forward-scans the remaining blocks, + /// and `install_reserved_block` reuses the tombstone slot when none has + /// room. Note the ordinary non-quarantine path reaches the same state — + /// `reset_region_to_zero` sets `current = 0` unconditionally — so this is a + /// property the allocator has always had to have. + #[test] + fn alloc_is_correct_when_current_points_at_a_tombstone() { + // A standalone arena over memory this test owns: no page-meta + // registration, no GC coupling, nothing another test can observe. + const SIZE: usize = 4096; + let layout = std::alloc::Layout::from_size_align(SIZE, 16).unwrap(); + // SAFETY: non-zero size, valid alignment. + let backing = unsafe { std::alloc::alloc(layout) }; + assert!(!backing.is_null()); + + let mut arena = Arena { + blocks: vec![ArenaBlock { + data: backing, + size: SIZE, + offset: 0, + dead_cycles: 0, + }], + current: 0, + generation: HeapGeneration::Nursery, + space: HeapSpace::Survivor0, + }; + let live_base = backing as usize; + let live_size = SIZE; + + // Shape it exactly as a full-survivor retirement leaves it. + arena.blocks.push(tombstone()); + arena.current = arena.blocks.len() - 1; + + // (1) A tombstone `current` must not fault, and must not be allocated + // from. The forward scan has to find the real block instead. + let ptr = arena.alloc(64, 8); + assert!( + !ptr.is_null(), + "alloc through a tombstone current returned null" + ); + let addr = ptr as usize; + assert!( + addr >= live_base && addr + 64 <= live_base + live_size, + "alloc must come from the live block, not the tombstone \ + (ptr={addr:#x}, live=[{live_base:#x}, {:#x}))", + live_base + live_size + ); + assert_ne!( + arena.current, + arena.blocks.len() - 1, + "the forward scan must repoint `current` off the tombstone" + ); + + // (2) The memory is genuinely usable — a tombstone-derived pointer + // would fault or alias here. + unsafe { + std::ptr::write_bytes(ptr, 0xA5, 64); + assert!((0..64).all(|i| *ptr.add(i) == 0xA5)); + } + + // (3) A tombstone `current` with NO other block able to satisfy the + // request must still fail cleanly (return `None` from every block) + // rather than dereferencing the null `data`. `try_alloc_after_gc` is + // the part `Arena::alloc` runs before reaching for a fresh block; a + // tombstone must simply not answer. + arena.current = arena.blocks.len() - 1; + for block in arena.blocks.iter_mut() { + if !block.data.is_null() { + block.offset = block.size; + } + } + assert!( + arena.try_alloc_after_gc(64, 8).is_none(), + "a full arena whose `current` is a tombstone must report no room, \ + not hand back a pointer derived from `data = null`" + ); + + // SAFETY: `backing` came from this layout and nothing else owns it. + unsafe { std::alloc::dealloc(backing, layout) }; + } +} diff --git a/crates/perry-runtime/src/arena/reset.rs b/crates/perry-runtime/src/arena/reset.rs index 1a208530fa..d981ce6d2f 100644 --- a/crates/perry-runtime/src/arena/reset.rs +++ b/crates/perry-runtime/src/arena/reset.rs @@ -101,7 +101,17 @@ pub(crate) fn active_survivor_block_index_range() -> std::ops::Range { /// Reset Eden and the active survivor from-space, then flip the survivor /// roles so the to-space populated by the copying collector becomes active. +/// +/// **This is the single site `PERRY_GC_PROTECT_FROMSPACE` gates** (#7154 +/// tooling). With the knob on, the from-space blocks are quarantined instead of +/// recycled, so a stale pointer faults immediately; with it off — the default — +/// the branch below is not taken and this function behaves exactly as it always +/// has. No other reclaim path (the non-moving minor's `arena_reset_empty_blocks`, +/// the full mark-sweep, old-gen defrag) is affected by that knob. pub(crate) fn copying_reset_from_spaces_and_flip() -> ArenaResetStats { + if protect_fromspace_enabled() { + return copying_quarantine_from_spaces_and_flip(); + } sync_inline_arena_state(); let mut reset_blocks = 0usize; let mut reusable_bytes = 0usize; diff --git a/crates/perry-runtime/src/gc/fromspace_scan.rs b/crates/perry-runtime/src/gc/fromspace_scan.rs index 2604141fc8..9c02b97245 100644 --- a/crates/perry-runtime/src/gc/fromspace_scan.rs +++ b/crates/perry-runtime/src/gc/fromspace_scan.rs @@ -62,6 +62,11 @@ pub(crate) struct FromSpaceRef { /// True when the target carries `GC_FLAG_FORWARDED` — i.e. it MOVED and /// this reference was simply not rewritten. pub(super) target_forwarded: bool, + /// The TARGET's `obj_type`, read off its (still intact, pre-flip) header. + /// The single most useful field when triaging a `value is not a function`: + /// `4` is a closure, `2` an object. Reported as `0xFF` when the target is + /// too low in memory to carry a header. + pub(super) target_obj_type: u8, /// True when the word was NaN-boxed, false when it was a bare address. pub(super) nanboxed: bool, /// Remembered-set coverage of THIS SLOT's page, the decisive split: @@ -105,14 +110,38 @@ pub(crate) struct FromSpaceScanReport { const MAX_SAMPLES: usize = 32; +fn truthy(raw: Option<&str>) -> bool { + matches!(raw, Some("1") | Some("on") | Some("true")) +} + +/// Resolve both scan knobs together. +/// +/// **ABORT IMPLIES SCAN** (#7154 tooling). `PERRY_GC_FROMSPACE_SCAN_ABORT=1` on +/// its own used to be completely inert: `run_fromspace_scan` returned at the +/// `fromspace_scan_enabled()` gate, the scan never ran, there was nothing to +/// abort, and the run reported success. A knob that reads as "abort on the +/// first offender" and silently does nothing is exactly the class of defect the +/// GC knob kill-policy exists for — and exactly what an investigator reaching +/// for the abort switch mid-hunt would be misled by. +/// +/// Pure so it is testable without mutating the process environment (the live +/// readers cache in a `OnceLock`). +pub(super) fn resolve_scan_knobs(scan: Option<&str>, abort: Option<&str>) -> (bool, bool) { + let abort = truthy(abort); + (truthy(scan) || abort, abort) +} + pub(super) fn fromspace_scan_enabled() -> bool { use std::sync::OnceLock; static CACHED: OnceLock = OnceLock::new(); *CACHED.get_or_init(|| { - matches!( - std::env::var("PERRY_GC_FROMSPACE_SCAN").as_deref(), - Ok("1") | Ok("on") | Ok("true") + resolve_scan_knobs( + std::env::var("PERRY_GC_FROMSPACE_SCAN").ok().as_deref(), + std::env::var("PERRY_GC_FROMSPACE_SCAN_ABORT") + .ok() + .as_deref(), ) + .0 }) } @@ -120,10 +149,13 @@ fn fromspace_scan_abort() -> bool { use std::sync::OnceLock; static CACHED: OnceLock = OnceLock::new(); *CACHED.get_or_init(|| { - matches!( - std::env::var("PERRY_GC_FROMSPACE_SCAN_ABORT").as_deref(), - Ok("1") | Ok("on") | Ok("true") + resolve_scan_knobs( + std::env::var("PERRY_GC_FROMSPACE_SCAN").ok().as_deref(), + std::env::var("PERRY_GC_FROMSPACE_SCAN_ABORT") + .ok() + .as_deref(), ) + .1 }) } @@ -187,11 +219,11 @@ unsafe fn scan_object(header: *mut GcHeader, report: &mut FromSpaceScanReport) { } // The target is in from-space. Did it move (missing rewrite) or was it // never evacuated (dangling)? - let target_forwarded = if target >= GC_HEADER_SIZE { + let (target_forwarded, target_obj_type) = if target >= GC_HEADER_SIZE { let th = (target - GC_HEADER_SIZE) as *const GcHeader; - (*th).gc_flags & GC_FLAG_FORWARDED != 0 + ((*th).gc_flags & GC_FLAG_FORWARDED != 0, (*th).obj_type) } else { - false + (false, 0xFF) }; if target_forwarded { report.missing_rewrites += 1; @@ -226,6 +258,7 @@ unsafe fn scan_object(header: *mut GcHeader, report: &mut FromSpaceScanReport) { target, target_space, target_forwarded, + target_obj_type, nanboxed: matches!(word, super::root_words::RootWord::Nanboxed { .. }), slot_dirty_now: super::barrier::dirty_now_for_addr(words.add(i) as usize), slot_ever_dirty: super::barrier::ever_dirty_for_addr(words.add(i) as usize), @@ -282,13 +315,14 @@ pub(crate) fn scan_heap_for_fromspace_refs() -> FromSpaceScanReport { fn describe(r: &FromSpaceRef) -> String { format!( - " owner={:#x} type={} space={:?} +{} {} -> {:#x} ({:?}) {} [slot dirty_now={} ever_dirty={} owner_flags={:#x} marked={}]", + " owner={:#x} type={} space={:?} +{} {} -> {:#x} (type={} {:?}) {} [slot dirty_now={} ever_dirty={} owner_flags={:#x} marked={}]", r.owner_header, r.owner_obj_type, r.owner_space, r.slot_offset, if r.nanboxed { "nanbox" } else { "bare" }, r.target, + r.target_obj_type, r.target_space, if r.target_forwarded { "MISSING-REWRITE (target moved)" @@ -304,6 +338,16 @@ fn describe(r: &FromSpaceRef) -> String { fn report_and_abort(report: &FromSpaceScanReport) -> ! { emit_report(report, "abort"); + // The scan runs inside the collector, so this backtrace names the + // COLLECTION, not the mutator store that created the stale slot — which is + // exactly the limitation `PERRY_GC_PROTECT_FROMSPACE` exists to remove + // (there the fault happens at the stale *use*, with the holder live). Still + // printed: it pins which collector phase and trigger observed the offender, + // and it is free on a path that is about to abort. + eprintln!( + "[gc-fromspace-scan abort] collector backtrace:\n{}", + std::backtrace::Backtrace::force_capture() + ); panic!( "gc from-space scan: {} missing rewrite(s), {} dangling reference(s) survived the rewrite pass", report.missing_rewrites, report.dangling diff --git a/crates/perry-runtime/src/gc/mod.rs b/crates/perry-runtime/src/gc/mod.rs index 73be4a03e4..ba74e47e67 100644 --- a/crates/perry-runtime/src/gc/mod.rs +++ b/crates/perry-runtime/src/gc/mod.rs @@ -82,7 +82,12 @@ mod verify; /// the rewrite pass own root enumeration. Debug-only /// (`PERRY_GC_FROMSPACE_SCAN=1`). mod fromspace_scan; +/// #7154 tooling: force an evacuating minor at every safepoint so an unrooted +/// value dies/moves on its FIRST exposure. Debug-only (`PERRY_GC_ZEAL=1`). +mod zeal; pub use verify::*; +pub use zeal::zeal_forced_collections; +pub(crate) use zeal::{gc_zeal_enabled, note_zeal_forced_collection}; #[cfg(feature = "diagnostics")] mod heap_snapshot; #[cfg(feature = "diagnostics")] @@ -244,11 +249,17 @@ pub fn gen_gc_evacuate_enabled() -> bool { } fn gc_force_evacuate_enabled() -> bool { + // `PERRY_GC_ZEAL=1` implies forced evacuation (#7154 tooling): a zealous + // minor that leaves survivors in place would move nothing, and "an unrooted + // value moves on its first exposure" is the entire contract of zeal mode. + // Still subject to `gen_gc_evacuate_enabled()` — an explicit + // `PERRY_GEN_GC_EVACUATE=0` wins, so the two knobs cannot silently disagree. gen_gc_evacuate_enabled() - && matches!( - std::env::var("PERRY_GC_FORCE_EVACUATE").as_deref(), - Ok("1") | Ok("on") | Ok("true") - ) + && (gc_zeal_enabled() + || matches!( + std::env::var("PERRY_GC_FORCE_EVACUATE").as_deref(), + Ok("1") | Ok("on") | Ok("true") + )) } fn gc_verify_evacuation_enabled() -> bool { diff --git a/crates/perry-runtime/src/gc/policy.rs b/crates/perry-runtime/src/gc/policy.rs index 7c69d3ee6b..f603374221 100644 --- a/crates/perry-runtime/src/gc/policy.rs +++ b/crates/perry-runtime/src/gc/policy.rs @@ -1722,8 +1722,16 @@ pub(crate) fn gc_safepoint_moving_minor() { return; } _ => { - // No nursery-pressure trigger is due — nothing to collect here. - return; + // No nursery-pressure trigger is due — nothing to collect here... + // unless zeal is on (#7154 tooling), in which case the point of the + // mode is to collect anyway so an unrooted value moves on its first + // exposure. `gc_force_evacuate_enabled()` is true under zeal, so + // this minor MOVES survivors rather than sweeping in place. + if !super::gc_zeal_enabled() { + return; + } + super::note_zeal_forced_collection(); + GcTriggerKind::ArenaBytes } }; let pre_in_use = crate::arena::arena_in_use_bytes(); @@ -1756,7 +1764,15 @@ pub(crate) fn gc_safepoint_moving_minor() { /// one thread-local read). #[no_mangle] pub extern "C" fn js_gc_loop_safepoint() { - if !gc_moving_loop_polls_enabled() || !GC_SAFEPOINT_PENDING.with(Cell::get) { + if !gc_moving_loop_polls_enabled() { + return; + } + // Zeal (#7154 tooling) collects at EVERY poll, not only when the alloc-point + // arm already deferred one. Zeal cannot conjure a poll codegen never emitted, + // so the `gc_moving_loop_polls_enabled()` gate above still applies — see + // `gc/zeal.rs` for why that means "compile AND run with + // `PERRY_GC_MOVING_LOOP_POLLS=1`". + if !GC_SAFEPOINT_PENDING.with(Cell::get) && !super::gc_zeal_enabled() { return; } gc_safepoint_moving_minor(); diff --git a/crates/perry-runtime/src/gc/tests/fromspace_protect.rs b/crates/perry-runtime/src/gc/tests/fromspace_protect.rs new file mode 100644 index 0000000000..3536f3b5cb --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/fromspace_protect.rs @@ -0,0 +1,440 @@ +//! Teeth for the #7154 detection-latency instruments: from-space quarantine +//! (`PERRY_GC_PROTECT_FROMSPACE`) and GC zeal (`PERRY_GC_ZEAL`). +//! +//! Every test asserts BOTH directions of its knob. The GC knob kill-policy in +//! CLAUDE.md requires an exercised OFF state for every knob, and the reason is +//! recorded right there: `PERRY_GC_FORCE_EVACUATE` was inert for every +//! `gc()`-driven test for months and nobody noticed, because only the ON arm was +//! ever asserted and the ON arm did nothing. +//! +//! These are *debug instruments*, so the correctness bar is higher than usual, +//! not lower: an instrument that reports clean when the heap is dirty is worse +//! than no instrument, and one that changes the collector when it is switched +//! off is a landmine in every future bisect. + +use super::super::*; +use super::support::*; +use crate::arena::FromSpaceProtection; + +// --------------------------------------------------------------------------- +// Knob parsing — pure, so both states are asserted without touching the +// process environment (the live readers cache in a `OnceLock`, so a test that +// set an env var would be at the mercy of which test ran first). +// --------------------------------------------------------------------------- + +#[test] +fn protection_knob_parses_off_poison_and_protect() { + use crate::arena::parse_protection_mode; + // OFF is the default and every unrecognised spelling: a typo must not + // silently enable an instrument that detaches arena blocks. + for raw in [ + None, + Some("0"), + Some("off"), + Some("false"), + Some("yes"), + Some(""), + ] { + assert_eq!( + parse_protection_mode(raw), + FromSpaceProtection::Off, + "{raw:?} must leave from-space protection OFF" + ); + } + for raw in ["1", "on", "true"] { + assert_eq!( + parse_protection_mode(Some(raw)), + FromSpaceProtection::ProtectPages, + "{raw} must select mprotect + poison" + ); + } + assert_eq!( + parse_protection_mode(Some("poison")), + FromSpaceProtection::PoisonOnly, + "`poison` must select the no-mprotect fallback" + ); +} + +#[test] +fn quarantine_depth_rejects_zero_and_garbage() { + use crate::arena::parse_quarantine_depth; + // A depth of 0 would evict each set on the cycle it was created — the + // instrument would read as ON and protect nothing. + assert_eq!(parse_quarantine_depth(Some("0")), 1); + assert_eq!(parse_quarantine_depth(Some("1")), 1); + assert_eq!(parse_quarantine_depth(Some("16")), 16); + assert_eq!(parse_quarantine_depth(Some("banana")), 4); + assert_eq!(parse_quarantine_depth(None), 4); +} + +#[test] +fn zeal_knob_parses_both_states() { + use super::super::zeal::parse_zeal; + for raw in [None, Some("0"), Some("off"), Some("false"), Some("2")] { + assert!(!parse_zeal(raw), "{raw:?} must leave zeal OFF"); + } + for raw in ["1", "on", "true"] { + assert!(parse_zeal(Some(raw)), "{raw} must enable zeal"); + } +} + +/// The gap this closes: `PERRY_GC_FROMSPACE_SCAN_ABORT=1` used to be completely +/// inert on its own — `run_fromspace_scan` returned at the +/// `fromspace_scan_enabled()` gate, so there was never anything to abort and the +/// run reported success. An investigator reaching for the abort switch mid-hunt +/// got a green run and no scan. +#[test] +fn fromspace_scan_abort_implies_the_scan_runs() { + use super::super::fromspace_scan::resolve_scan_knobs; + assert_eq!(resolve_scan_knobs(None, None), (false, false)); + assert_eq!(resolve_scan_knobs(Some("1"), None), (true, false)); + assert_eq!( + resolve_scan_knobs(None, Some("1")), + (true, true), + "ABORT alone must turn the scan ON, not silently do nothing" + ); + assert_eq!(resolve_scan_knobs(Some("1"), Some("1")), (true, true)); + assert_eq!(resolve_scan_knobs(Some("0"), Some("0")), (false, false)); +} + +// --------------------------------------------------------------------------- +// From-space quarantine, driven through a real copying minor. +// --------------------------------------------------------------------------- + +/// The OFF arm. With the knob unset the copying minor must take the ordinary +/// reset path, retire nothing, and leave the quarantine untouched — a normal +/// build pays exactly nothing for this instrument existing. +#[test] +fn protection_off_retires_no_from_space() { + let _guard = CopyingNurseryTestGuard::new(1); + let before = crate::arena::quarantine_stats(); + + let live = young_leaf(); + js_shadow_slot_set(0, string_bits(live)); + let _ = gc_collect_minor(); + + let after = crate::arena::quarantine_stats(); + assert_eq!( + after.sets_retired, before.sets_retired, + "with PERRY_GC_PROTECT_FROMSPACE off, a copying minor must recycle \ + from-space exactly as it always has" + ); + assert_eq!(after.blocks_quarantined, before.blocks_quarantined); +} + +/// The ON arm, in `poison` mode so the assertions can read the retired bytes +/// instead of faulting on them. +/// +/// Three things are asserted together, because any one alone can pass +/// vacuously: (1) the subject was live — an object actually MOVED, so this was +/// a real evacuating minor; (2) the from-space bytes it moved out of are now +/// poison rather than recyclable; (3) the instrument says so in its counters. +#[test] +fn protection_poisons_the_from_space_an_object_moved_out_of() { + let _guard = CopyingNurseryTestGuard::new(1); + let _mode = crate::arena::ProtectionModeGuard::set(FromSpaceProtection::PoisonOnly); + let before = crate::arena::quarantine_stats(); + + let from_space_addr = young_leaf(); + js_shadow_slot_set(0, string_bits(from_space_addr)); + let _ = gc_collect_minor(); + let to_space_addr = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + + // (1) subject-was-live: the minor evacuated, so `from_space_addr` really is + // a retired address and not simply the object's current home. + assert_ne!( + to_space_addr, from_space_addr, + "test premise: the copying minor must have MOVED the rooted object" + ); + + // (2) the retired bytes are poison, and present a header no dispatch can + // mistake for a live object. + let poison_word = unsafe { *(from_space_addr as *const u64) }; + assert_eq!( + poison_word, + crate::arena::QUARANTINE_POISON_WORD, + "the retired from-space payload must read as poison, not as a \ + freshly-recycled object" + ); + let header_obj_type = unsafe { *((from_space_addr - GC_HEADER_SIZE) as *const u8) }; + assert_eq!( + header_obj_type, + crate::arena::QUARANTINE_POISON_OBJ_TYPE, + "the retired header must present the invalid-object sentinel" + ); + + // (3) the counters agree, so a future run can tell a protected cycle from + // one where the instrument never engaged. + let after = crate::arena::quarantine_stats(); + assert_eq!( + after.sets_retired, + before.sets_retired + 1, + "exactly one from-space page-set must have been retired" + ); + assert!( + after.blocks_quarantined > before.blocks_quarantined, + "at least one block must have been quarantined" + ); + assert!( + after.bytes_poisoned > before.bytes_poisoned, + "poison-only mode must count poisoned bytes" + ); + assert_eq!( + after.bytes_protected, before.bytes_protected, + "poison-only mode must NOT claim mprotected bytes" + ); + + // The survivor is untouched by any of this. + assert_eq!( + crate::arena::classify_heap_space(to_space_addr), + crate::arena::active_survivor_space(), + "the evacuated copy must still be a normal, readable survivor" + ); +} + +/// The memory bound. Long runs must not OOM: the quarantine is a ring, and +/// evicted blocks go back into Eden rather than being freed (nothing that was +/// ever `mprotect`ed is handed to `dealloc`). +#[test] +fn quarantine_is_bounded_and_recycles_expired_blocks() { + let _guard = CopyingNurseryTestGuard::new(1); + let _mode = crate::arena::ProtectionModeGuard::set(FromSpaceProtection::PoisonOnly); + let depth = crate::arena::quarantine_depth(); + let before = crate::arena::quarantine_stats(); + + for _ in 0..(depth + 3) { + let live = young_leaf(); + js_shadow_slot_set(0, string_bits(live)); + let _ = gc_collect_minor(); + } + + let after = crate::arena::quarantine_stats(); + assert!( + after.sets_retired >= before.sets_retired + depth as u64, + "test premise: enough minors must have run to overflow the ring \ + (before={}, after={}, depth={depth})", + before.sets_retired, + after.sets_retired + ); + assert!( + after.sets_held <= depth, + "the quarantine must never hold more than PERRY_GC_PROTECT_FROMSPACE_DEPTH \ + page-sets (held={}, depth={depth})", + after.sets_held + ); + assert!( + after.blocks_recycled > before.blocks_recycled, + "expired sets must be recycled back into Eden, not leaked \ + (before={}, after={})", + before.blocks_recycled, + after.blocks_recycled + ); +} + +/// **The sabotage test.** Everything above asserts the instrument *ran*; this +/// asserts it *detects the thing it was built for*, by planting the bug rather +/// than waiting for one. +/// +/// The plant is the #7184 / #7192 shape reduced to its essence: a mutator keeps +/// a pre-collection address across an evacuating minor (there, a caller's +/// register or an out-of-frame shadow slot; here, a local `usize`), and then +/// dereferences it. Both arms run the identical plant, and the OFF arm is what +/// makes the ON arm mean anything: +/// +/// - OFF — the from-space bytes are recycled, so the stale address reads as a +/// **valid, live, unrelated object**. The deref silently succeeds. That is +/// precisely why a #7154-class bug takes ten rounds to find, and it is the +/// red control: the bug is genuinely invisible without the instrument. +/// - ON — the same address reads as poison carrying an `obj_type` no dispatch +/// can accept. The stale use is caught at the use. +/// +/// A regression that made the quarantine miss the retired range would flip the +/// ON arm to look like the OFF arm, and this test would fail. A regression that +/// made it protect bytes that were never retired would break the OFF arm. +#[test] +fn quarantine_catches_a_planted_stale_from_space_deref() { + // --- red control: WITHOUT the instrument, the stale deref succeeds ------ + let recycled_is_live_object = { + let _guard = CopyingNurseryTestGuard::new(1); + let stale = young_leaf(); + js_shadow_slot_set(0, string_bits(stale)); + let _ = gc_collect_minor(); + let moved_to = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + assert_ne!( + moved_to, stale, + "test premise: the minor must have MOVED the object, so `stale` is \ + genuinely a retired from-space address" + ); + + // The mutator keeps allocating, as it would after the bad collection. + // The bump allocator hands the retired bytes straight back out. + for _ in 0..8 { + let _ = young_leaf(); + } + + // Deref the stale address. Without the instrument this reads whatever + // now lives there — well-formed memory, not poison. + let word = unsafe { *(stale as *const u64) }; + assert_ne!( + word, + crate::arena::QUARANTINE_POISON_WORD, + "the OFF arm must NOT poison — otherwise the ON arm proves nothing" + ); + word + }; + + // --- the instrument: the same plant is caught --------------------------- + { + let _guard = CopyingNurseryTestGuard::new(1); + let _mode = crate::arena::ProtectionModeGuard::set(FromSpaceProtection::PoisonOnly); + let before = crate::arena::quarantine_stats(); + + let stale = young_leaf(); + js_shadow_slot_set(0, string_bits(stale)); + let _ = gc_collect_minor(); + let moved_to = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + assert_ne!(moved_to, stale, "test premise: the object must have MOVED"); + + // Same mutator pressure as the red control — the retired bytes must + // stay quarantined rather than being handed back out. + for _ in 0..8 { + let _ = young_leaf(); + } + + // Live-subject check before believing the verdict (CLAUDE.md, "a gate + // must assert its subject was live"). + let after = crate::arena::quarantine_stats(); + assert_eq!( + after.sets_retired, + before.sets_retired + 1, + "the instrument must actually have retired this minor's from-space" + ); + + let word = unsafe { *(stale as *const u64) }; + assert_eq!( + word, + crate::arena::QUARANTINE_POISON_WORD, + "the planted stale deref must land on poison, not on recycled bytes" + ); + let obj_type = unsafe { *((stale - GC_HEADER_SIZE) as *const u8) }; + assert_eq!( + obj_type, + crate::arena::QUARANTINE_POISON_OBJ_TYPE, + "the retired header must present an obj_type no dispatch accepts" + ); + assert_ne!( + word, recycled_is_live_object, + "the two arms must genuinely differ — if they agree, one of them is \ + not exercising what it claims" + ); + } +} + +// --------------------------------------------------------------------------- +// Zeal +// --------------------------------------------------------------------------- + +/// Zeal's whole contract: collect at a safepoint where nothing is due. Both +/// arms, because the OFF arm is what proves the safepoint was genuinely idle — +/// without it, a passing ON arm could just be ordinary heap pressure. +#[test] +fn zeal_collects_at_a_safepoint_with_no_pressure_due() { + let _guard = CopyingNurseryTestGuard::new(1); + let _triggers = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + reset_scan_fallback_counters(); + + // OFF: an idle safepoint must collect nothing. + { + let _zeal = super::super::zeal::ZealGuard::set(false); + gc_safepoint_moving_minor(); + } + assert_eq!( + safepoint_drain_count(SafepointDrainKind::NurseryMinor), + 0, + "test premise: with no trigger due and zeal off, the safepoint must be idle" + ); + + // ON: the same idle safepoint must now run a minor. + let forced_before = zeal_forced_collections(); + { + let _zeal = super::super::zeal::ZealGuard::set(true); + gc_safepoint_moving_minor(); + } + assert_eq!( + safepoint_drain_count(SafepointDrainKind::NurseryMinor), + 1, + "PERRY_GC_ZEAL=1 must force a minor at every safepoint" + ); + assert!( + zeal_forced_collections() > forced_before, + "the forced collection must be COUNTED — a zeal run reporting 0 forced \ + collections exercised nothing, and a clean verdict from it is vacuous" + ); +} + +/// A zealous minor that leaves survivors in place would move nothing, so it +/// could not surface a stale-pointer bug at all. Zeal therefore implies forced +/// evacuation — but must still lose to an explicit `PERRY_GEN_GC_EVACUATE=0`, +/// so the two knobs can never silently disagree about whether objects move. +#[test] +fn zeal_implies_forced_evacuation() { + // Split by ambient policy so BOTH branches assert something. The previous + // `force_enabled() || !evacuate_enabled()` form was satisfied by its right + // operand alone: under an ambient `PERRY_GEN_GC_EVACUATE=0` it passed + // without exercising zeal at all, and reported nothing to say so — the + // exact vacuous-green shape the kill-policy exists to catch. + if !gen_gc_evacuate_enabled() { + // Precedence arm: an explicit `PERRY_GEN_GC_EVACUATE=0` must beat zeal, + // so the two knobs can never silently disagree about whether objects + // move. + let _zeal_on = super::super::zeal::ZealGuard::set(true); + assert!( + !gc_force_evacuate_enabled(), + "an explicit PERRY_GEN_GC_EVACUATE=0 must win over zeal" + ); + return; + } + // Implication arm: evacuation is permitted, so zeal must turn it on. + let _zeal_off = super::super::zeal::ZealGuard::set(false); + let off = gc_force_evacuate_enabled(); + let _zeal_on = super::super::zeal::ZealGuard::set(true); + assert!( + gc_force_evacuate_enabled(), + "evacuation is permitted, so zeal must force it (force_off={off})" + ); +} + +/// Zeal and protection are designed to compose — that pairing is what turns a +/// #7154 bug into an immediate fault instead of a cycle-late `TypeError`. This +/// asserts they actually run together rather than one disabling the other. +#[test] +fn zeal_and_protection_compose() { + let _guard = CopyingNurseryTestGuard::new(1); + let _triggers = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + let _mode = crate::arena::ProtectionModeGuard::set(FromSpaceProtection::PoisonOnly); + let _zeal = super::super::zeal::ZealGuard::set(true); + reset_scan_fallback_counters(); + let before = crate::arena::quarantine_stats(); + + let from_space_addr = young_leaf(); + js_shadow_slot_set(0, string_bits(from_space_addr)); + gc_safepoint_moving_minor(); + + assert_eq!( + safepoint_drain_count(SafepointDrainKind::NurseryMinor), + 1, + "zeal must have forced the minor" + ); + let after = crate::arena::quarantine_stats(); + assert_eq!( + after.sets_retired, + before.sets_retired + 1, + "the zeal-forced minor's from-space must have been quarantined" + ); + assert_eq!( + unsafe { *(from_space_addr as *const u64) }, + crate::arena::QUARANTINE_POISON_WORD, + "zeal + protection: the address the value moved out of must be poison \ + immediately, not on some later cycle" + ); +} diff --git a/crates/perry-runtime/src/gc/tests/mod.rs b/crates/perry-runtime/src/gc/tests/mod.rs index e0b038a1bc..6da4238116 100644 --- a/crates/perry-runtime/src/gc/tests/mod.rs +++ b/crates/perry-runtime/src/gc/tests/mod.rs @@ -11,6 +11,7 @@ mod dead_owner_side_tables; mod debt_pacer; mod error_side_tables; mod evacuation; +mod fromspace_protect; mod fromspace_scan; mod helper_stores; mod host_safepoints; diff --git a/crates/perry-runtime/src/gc/zeal.rs b/crates/perry-runtime/src/gc/zeal.rs new file mode 100644 index 0000000000..d3736b1175 --- /dev/null +++ b/crates/perry-runtime/src/gc/zeal.rs @@ -0,0 +1,115 @@ +//! GC zeal mode (#7154 tooling) — `PERRY_GC_ZEAL`. +//! +//! # Why +//! +//! A #7154-class bug is a value that is live but not rooted across a collection +//! point. Whether it is *caught* depends entirely on whether a collection +//! happens to land inside that window. In a normal run the window is a few +//! instructions wide and collections are tens of megabytes apart, so the bug is +//! observed only when an unrelated allocation burst lines up with it — which is +//! why the #7154 hunt needed a `zod` workload and ten rounds. +//! +//! Zeal removes the coincidence. Modelled on V8's `--stress-scavenge` and +//! SpiderMonkey's `gcZeal`, it forces an **evacuating** minor at every GC +//! safepoint, so an unrooted value moves on its FIRST exposure, deterministically. +//! +//! # What the knob actually gates +//! +//! `PERRY_GC_ZEAL=1`: +//! +//! 1. Every loop back-edge poll (`js_gc_loop_safepoint`) runs a minor, instead +//! of only draining an already-deferred one (`GC_SAFEPOINT_PENDING`). +//! 2. Every outermost microtask-pump safepoint runs a minor, instead of only +//! when `gc_budgeted_due_trigger()` reports nursery/old pressure. +//! 3. `gc_force_evacuate_enabled()` becomes true, so the minor **moves** every +//! marked non-pinned nursery object rather than leaving survivors in place. +//! Without this a zealous minor could run and move nothing, which would be a +//! gate that cannot fail. +//! +//! It does **not** change which collections are *sound* — every forced +//! collection runs at a point the collector already treats as a precise-root +//! safepoint. It only changes how often. +//! +//! ## Point 1 requires a compile-time opt-in too +//! +//! Loop back-edge polls are only *emitted* when the compiler ran with +//! `PERRY_GC_MOVING_LOOP_POLLS=1` (default off since #7161). Zeal cannot +//! conjure a poll that codegen never emitted. A binary compiled without polls +//! still gets (2) and (3) — event-loop-boundary zeal — but a compute-only loop +//! that never yields will not collect at all. **For the #7154 hunt, compile AND +//! run with `PERRY_GC_MOVING_LOOP_POLLS=1`.** `zeal_forced_collections()` +//! reports how many collections zeal actually forced, so "clean under zeal" can +//! be checked against zeal having done anything. +//! +//! # Why there is no allocation-point level +//! +//! An obvious `PERRY_GC_ZEAL=2` would collect at every allocation. It was +//! deliberately not implemented: the allocation-point arm in `gc_check_trigger` +//! takes `ManualGcScanGuard::force_full_scan`, and a forced conservative stack +//! scan makes the copying minor ineligible +//! (`CopiedMinorFallbackReason::ConservativeStack`). A level 2 would therefore +//! run many *non-moving* minors and move nothing — a knob whose name promises +//! relocation stress and whose effect is sweep pressure. That is precisely the +//! failure `PERRY_GC_FORCE_EVACUATE` already cost this project once (#6942 / +//! #6946), so the level does not exist rather than existing untrustworthy. + +use std::sync::atomic::{AtomicU64, Ordering}; + +/// Collections zeal has forced that would not otherwise have run. The live- +/// subject counter for every zeal-based verdict. +static ZEAL_FORCED: AtomicU64 = AtomicU64::new(0); + +/// Pure knob parse, so the mapping is testable without mutating the process +/// environment (the live reader caches in a `OnceLock`). +pub(crate) fn parse_zeal(raw: Option<&str>) -> bool { + matches!(raw, Some("1") | Some("on") | Some("true")) +} + +#[cfg(test)] +thread_local! { + /// Test-only override. Thread-local, so one test turning zeal on cannot + /// change collector behaviour for any other test. + static ZEAL_OVERRIDE: std::cell::Cell> = const { std::cell::Cell::new(None) }; +} + +/// `PERRY_GC_ZEAL=1`/`on`/`true` — force an evacuating minor at every safepoint. +pub(crate) fn gc_zeal_enabled() -> bool { + #[cfg(test)] + if let Some(zeal) = ZEAL_OVERRIDE.with(std::cell::Cell::get) { + return zeal; + } + use std::sync::OnceLock; + static CACHED: OnceLock = OnceLock::new(); + *CACHED.get_or_init(|| parse_zeal(std::env::var("PERRY_GC_ZEAL").ok().as_deref())) +} + +/// RAII test override for zeal. +#[cfg(test)] +pub(crate) struct ZealGuard(Option); + +#[cfg(test)] +impl ZealGuard { + pub(crate) fn set(enabled: bool) -> Self { + Self(ZEAL_OVERRIDE.with(|cell| cell.replace(Some(enabled)))) + } +} + +#[cfg(test)] +impl Drop for ZealGuard { + fn drop(&mut self) { + ZEAL_OVERRIDE.with(|cell| cell.set(self.0)); + } +} + +#[inline] +pub(crate) fn note_zeal_forced_collection() { + ZEAL_FORCED.fetch_add(1, Ordering::Relaxed); +} + +/// How many collections zeal has forced. A zeal run that reports `0` here +/// exercised nothing (most often: the binary was compiled without +/// `PERRY_GC_MOVING_LOOP_POLLS=1` and the workload never reached the event +/// loop). +pub fn zeal_forced_collections() -> u64 { + ZEAL_FORCED.load(Ordering::Relaxed) +} diff --git a/docs/src/internals/memory-model.md b/docs/src/internals/memory-model.md index c2479ecd04..66f2f02098 100644 --- a/docs/src/internals/memory-model.md +++ b/docs/src/internals/memory-model.md @@ -119,6 +119,48 @@ Idle nursery blocks observed empty for 2 GC cycles are `dealloc`'d back to the O | `PERRY_WRITE_BARRIERS=0` / `off` / `false` | Disable codegen-emitted write barriers at compile time and runtime exact helper barriers at runtime for benchmark/debug bisection. Unset, `=1`, `=on`, and `=true` keep barriers enabled. | | `PERRY_GC_DIAG=1` | Print per-cycle diagnostics, including one evacuation-policy line for cycles where evacuation was considered and for `barriers_inactive` skips. | +## Rooting-bug instruments + +A value that is live but not rooted across a collection point leaves nothing +behind at collection time — there is literally nothing for the collector to +find. The nursery then recycles the address immediately, so the stale pointer +reads a valid unrelated object and the program dies a cycle or more later, in a +different function, as `TypeError: value is not a function`. These knobs exist +to collapse that detection latency. **All are default-off and inert when off.** + +| Env var | Effect | +|---|---| +| `PERRY_GC_PROTECT_FROMSPACE=1` | After an **evacuating (copying) minor**, do not recycle from-space. Retired Eden and active-survivor blocks are detached into a bounded quarantine, filled with a poison pattern whose first byte reads as an invalid `obj_type` (`0xDE`), and `mprotect(PROT_NONE)`'d over their page-aligned interior. A stale dereference then SIGSEGVs **at the faulting instruction**, with the holder still on the stack. The installed reporter prints the faulting address, which minor retired it, and the last-known object that lived there (`obj_type`, size) plus a native backtrace, then restores `SIG_DFL` and returns so the instruction re-faults — a core file or debugger still sees the real crash site. | +| `PERRY_GC_PROTECT_FROMSPACE=poison` | As above without `mprotect`: poison only. Use where a fault is unwanted, or for the sub-page block edges `mprotect` cannot cover (those are always poison-filled and counted separately). | +| `PERRY_GC_PROTECT_FROMSPACE_DEPTH=N` | How many retired page-sets stay quarantined (default `4`, minimum `1`). Expired sets are restored to read/write and **recycled back into Eden**, never freed, so the quarantine is a ring: steady-state footprint is bounded by `N × from-space bytes` and no `mprotect`'d page is ever handed to the system allocator. | +| `PERRY_GC_ZEAL=1` | Force an evacuating minor at **every GC safepoint** — loop back-edge polls and the outermost microtask-pump boundary — instead of only when nursery pressure is due. Implies `PERRY_GC_FORCE_EVACUATE`, so survivors actually move — but an explicit `PERRY_GEN_GC_EVACUATE=0` still wins, and with it set zeal moves nothing and therefore surfaces nothing. Zeal also does **not** bypass `gc_safepoint_moving_minor`'s entry guards (in-allocation, suppressed, unsafe FFI zone, non-zero root-lock depth, budgeted cycle): a safepoint reached in any of those states still declines to collect. Modelled on V8 `--stress-scavenge` / SpiderMonkey `gcZeal`. Composes with the two above; that pairing is what turns a rooting bug into an immediate precise fault. | +| `PERRY_GC_FROMSPACE_SCAN_ABORT=1` | Abort on the **first** offending slot the whole-heap from-space scan finds, printing slot, holder, target (including the target's `obj_type`) and a collector backtrace. Now implies `PERRY_GC_FROMSPACE_SCAN=1`; previously it was silently inert on its own. | + +Two caveats these instruments are explicit about, because both have burned +prior investigations: + +- `PERRY_GC_PROTECT_FROMSPACE` gates **only** the copying minor's from-space + reset. A run with the knob on and zero copying minors protects nothing. Check + for a `[gc-fromspace-protect] retired_set=#N` line under `PERRY_GC_DIAG=1`. +- **Depth is the knob to raise when a suspected bug does not fault.** A stale + pointer is only caught while the page-set it names is still quarantined, and + under zeal a value can cross hundreds of collections between its last valid + observation and its stale use — one per loop back-edge poll. On #7154's + `new C(…)` reproducer the constructor body runs 600 polls, so the caller's + stale register is 600 retirements old by the time the return-override + publishes it: the default depth of 4 misses it silently, and + `PERRY_GC_PROTECT_FROMSPACE_DEPTH=800` faults on the first use. Rule of thumb: + depth ≥ the number of safepoints the suspect value survives. +- `PERRY_GC_ZEAL` cannot emit loop back-edge polls that codegen never produced. + Those require the **compile-time** `PERRY_GC_MOVING_LOOP_POLLS=1` (default off + since #7161). Without it, zeal only fires at event-loop boundaries and a + compute-only loop never collects at all. Compile *and* run with the poll opt-in. +- **Page protection is Unix-only.** `mprotect` / `sigaction` / `sysconf` are not + exposed by the `libc` crate on `x86_64-pc-windows-msvc`, a target + `perry-runtime` is genuinely built for. On non-Unix hosts `=1` degrades to + `poison`, which is visible rather than silent: `bytes_protected` stays `0` + while `bytes_poisoned` counts the whole retired range. + ## Why this design The combination — NaN-boxing for cheap value representation, per-thread arenas to avoid cross-thread sync, precise shadow stack + conservative stack scan for safe root discovery under an opaque optimizer (LLVM), generational aging for nursery-friendly workloads — is what lets Perry both go through LLVM and run a managed language without a fight. diff --git a/scripts/addr_class_allowlist.txt b/scripts/addr_class_allowlist.txt index 6e39bc2da8..6619b6dd5b 100644 --- a/scripts/addr_class_allowlist.txt +++ b/scripts/addr_class_allowlist.txt @@ -19,6 +19,7 @@ crates/perry-runtime/src/object/native_module.rs | "O_SYMLINK" => Some(0x200000) # # Grandfathered GcHeader-cast files: crates/perry-runtime/src/arena/allocators.rs | * | arena allocator/walker internals: header addresses come from block iteration or fresh allocation, never from NaN-box payloads +crates/perry-runtime/src/arena/quarantine.rs | let header = data.add(pos) as *const crate::gc::GcHeader; | #7154 from-space quarantine census: `data + pos` comes from linear block iteration over a detached arena block (the same discipline as arena/walk.rs), never from a NaN-box payload, so no handle band can reach it; the walk stops at the first header whose size does not cover the remaining bytes crates/perry-runtime/src/arena/tests.rs | * | arena allocator/walker internals: header addresses come from block iteration or fresh allocation, never from NaN-box payloads crates/perry-runtime/src/arena/walk.rs | * | arena allocator/walker internals: header addresses come from block iteration or fresh allocation, never from NaN-box payloads crates/perry-runtime/src/array/alloc.rs | * | pre-existing GcHeader probe predating addr_class; address validated by call-site guards (magnitude/registry/is_valid_obj_ptr) -- migrate to addr_class::try_read_gc_header in a follow-up diff --git a/scripts/gc_instrument_smoke.sh b/scripts/gc_instrument_smoke.sh new file mode 100755 index 0000000000..e31063cb8a --- /dev/null +++ b/scripts/gc_instrument_smoke.sh @@ -0,0 +1,140 @@ +#!/usr/bin/env bash +# End-to-end exercised arm for the #7154 rooting-bug instruments +# (`PERRY_GC_PROTECT_FROMSPACE`, `PERRY_GC_ZEAL`). +# +# WHY THIS EXISTS +# +# CLAUDE.md's GC knob kill-policy is binding: a knob with no arm exercising it +# is a configuration nobody has verified, and this repo has repeatedly paid for +# that (`PERRY_GC_FORCE_EVACUATE` inert for every `gc()`-driven test, #6942 / +# #6946; the matrix's `--pressure` knob disabling the path it measured, #7024). +# +# The *detection* property — "a planted stale from-space deref is caught" — is +# asserted as a unit test in the required `cargo-test` gate +# (`gc/tests/fromspace_protect.rs::quarantine_catches_a_planted_stale_from_space_deref`). +# What a unit test cannot cover is the INTEGRATED path: codegen actually +# emitting back-edge polls, zeal actually firing on them, the copying minor +# actually running, and the quarantine actually retiring its from-space in a +# real compiled program. That is this script. +# +# NON-VACUITY IS THE POINT. Per CLAUDE.md's "four ways a gate can be unable to +# fail" #4, a gate must assert its subject was live. A protected run with zero +# copying minors protects nothing and would pass silently. So this script does +# not merely check the program's output: it requires the zeal arm to produce +# strictly MORE quarantine retirements than the no-zeal arm, which can only +# happen if zeal genuinely forced collections that pressure would not have. +# +# Usage: scripts/gc_instrument_smoke.sh [path-to-perry] +# Expects target/release/perry and PERRY_RUNTIME_DIR-resolvable staticlibs. + +set -euo pipefail + +PERRY_BIN="${1:-target/release/perry}" +if [[ ! -x "$PERRY_BIN" ]]; then + echo "FAIL: no perry binary at $PERRY_BIN" >&2 + exit 1 +fi +PERRY_BIN="$(cd "$(dirname "$PERRY_BIN")" && pwd)/$(basename "$PERRY_BIN")" +export PERRY_RUNTIME_DIR="${PERRY_RUNTIME_DIR:-$(dirname "$PERRY_BIN")}" +export PERRY_NO_AUTO_OPTIMIZE=1 + +WORK="$(mktemp -d)" +trap 'rm -rf "$WORK"' EXIT + +# A deliberately SMALL correct program that still exercises the whole path: +# a constructor that allocates inside a loop (so codegen emits back-edge polls +# and the instance survives a collection inside the callee — the #7192 shape), +# called in an outer loop, with the caller reading a field back afterwards so a +# stale read cannot go unnoticed. Sized for ~1200 polls, not #7154's 240k, so +# the zeal arm costs seconds rather than minutes. +cat > "$WORK/fixture.ts" <<'TS' +class Holder { + payload: any; + constructor(n: number) { + const bits: any[] = []; + for (let i = 0; i < 40; i++) { + bits.push({ i: i, s: "x" }); + } + this.payload = { n: n, len: bits.length }; + } +} + +function run(): number { + let bad = 0; + for (let r = 0; r < 30; r++) { + const h = new Holder(r); + const p = h.payload; + if (p === null || p === undefined) { + bad++; + } else if ((p.n as number) !== r || (p.len as number) !== 40) { + bad++; + } + } + return bad; +} + +console.log("bad", run()); +TS + +echo "== compiling fixture with PERRY_GC_MOVING_LOOP_POLLS=1 (zeal needs the polls) ==" +PERRY_GC_MOVING_LOOP_POLLS=1 "$PERRY_BIN" compile "$WORK/fixture.ts" -o "$WORK/fixture" >/dev/null + +# $1 = label, rest = env assignments. Echoes the retirement count. +run_arm() { + local label="$1"; shift + local out rc retired + set +e + out="$(env "$@" PERRY_GC_MOVING_LOOP_POLLS=1 PERRY_GC_DIAG=1 "$WORK/fixture" 2>&1)" + rc=$? + set -e + retired="$(grep -c 'gc-fromspace-protect. mode=' <<<"$out" || true)" + if [[ $rc -ne 0 ]]; then + echo "FAIL [$label]: exited $rc" >&2 + echo "$out" | tail -30 >&2 + exit 1 + fi + if ! grep -q '^bad 0$' <<<"$out"; then + echo "FAIL [$label]: expected 'bad 0', got:" >&2 + grep '^bad' <<<"$out" >&2 || echo "(no 'bad' line)" >&2 + exit 1 + fi + echo " [$label] correct output, exit 0, quarantine retirements=$retired" + echo "$retired" +} + +echo "== arm 1: instruments OFF (baseline correctness) ==" +off_retired="$(run_arm off | tail -1)" +if [[ "$off_retired" -ne 0 ]]; then + echo "FAIL: the instrument retired $off_retired page-sets with the knob OFF." >&2 + echo " Default-off must mean inert." >&2 + exit 1 +fi + +echo "== arm 2: PROTECT_FROMSPACE=1 without zeal (pressure-only) ==" +nozeal_retired="$(run_arm protect PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=64 | tail -1)" + +echo "== arm 3: PROTECT_FROMSPACE=1 + ZEAL=1 (the investigation pairing) ==" +zeal_retired="$(run_arm protect+zeal PERRY_GC_ZEAL=1 PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=64 | tail -1)" + +# ---- non-vacuity gate ------------------------------------------------------- +# The subject must have been LIVE. Without this, every arm above could pass +# having run zero copying minors — the exact failure mode #6942/#7024/#7025 +# were filed for. +if [[ "$zeal_retired" -eq 0 ]]; then + echo "FAIL: zeal + protection retired ZERO from-space page-sets." >&2 + echo " The instruments did not run, so a clean result proves nothing." >&2 + echo " Most likely: codegen emitted no back-edge polls, or the copying" >&2 + echo " minor was ineligible (conservative stack scan / pinned young)." >&2 + exit 1 +fi +if [[ "$zeal_retired" -le "$nozeal_retired" ]]; then + echo "FAIL: zeal did not force any additional collection" >&2 + echo " (no-zeal=$nozeal_retired, zeal=$zeal_retired)." >&2 + echo " PERRY_GC_ZEAL is inert on this build — it must collect at" >&2 + echo " safepoints where no trigger is due." >&2 + exit 1 +fi + +echo +echo "PASS: instruments inert when off (0 retirements), live when on" +echo " (no-zeal=$nozeal_retired, zeal=$zeal_retired retirements), program correct in all arms."