diff --git a/CLAUDE.md b/CLAUDE.md index 9e56f54c9b..7e9c8918ff 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.1340 +**Current Version:** 0.5.1341 ## TypeScript Parity Status diff --git a/Cargo.lock b/Cargo.lock index 68d351c2df..b732dcbef5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5547,7 +5547,7 @@ checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" [[package]] name = "perry" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "anyhow", "base64", @@ -5607,14 +5607,14 @@ dependencies = [ [[package]] name = "perry-api-manifest" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "serde", ] [[package]] name = "perry-audio-miniaudio" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "cc", "libc", @@ -5622,7 +5622,7 @@ dependencies = [ [[package]] name = "perry-codegen" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "anyhow", "inkwell", @@ -5639,7 +5639,7 @@ dependencies = [ [[package]] name = "perry-codegen-arkts" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "anyhow", "perry-hir", @@ -5647,7 +5647,7 @@ dependencies = [ [[package]] name = "perry-codegen-glance" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "anyhow", "perry-hir", @@ -5655,7 +5655,7 @@ dependencies = [ [[package]] name = "perry-codegen-js" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "anyhow", "perry-dispatch", @@ -5664,7 +5664,7 @@ dependencies = [ [[package]] name = "perry-codegen-swiftui" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "anyhow", "perry-hir", @@ -5672,7 +5672,7 @@ dependencies = [ [[package]] name = "perry-codegen-wasm" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "anyhow", "base64", @@ -5684,7 +5684,7 @@ dependencies = [ [[package]] name = "perry-codegen-wear-tiles" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "anyhow", "perry-hir", @@ -5692,7 +5692,7 @@ dependencies = [ [[package]] name = "perry-container-compose" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "anyhow", "async-trait", @@ -5721,14 +5721,14 @@ dependencies = [ [[package]] name = "perry-container-e2e" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "anyhow", ] [[package]] name = "perry-diagnostics" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "serde", "serde_json", @@ -5736,7 +5736,7 @@ dependencies = [ [[package]] name = "perry-dispatch" -version = "0.5.1340" +version = "0.5.1341" [[package]] name = "perry-doc-fixture-my-bindings" @@ -5747,7 +5747,7 @@ dependencies = [ [[package]] name = "perry-doc-tests" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "anyhow", "clap", @@ -5762,7 +5762,7 @@ dependencies = [ [[package]] name = "perry-ext-ads" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "block2", "objc2", @@ -5772,7 +5772,7 @@ dependencies = [ [[package]] name = "perry-ext-argon2" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "argon2", "perry-ffi", @@ -5780,7 +5780,7 @@ dependencies = [ [[package]] name = "perry-ext-axios" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "perry-ffi", "reqwest", @@ -5789,7 +5789,7 @@ dependencies = [ [[package]] name = "perry-ext-bcrypt" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "bcrypt", "perry-ffi", @@ -5797,7 +5797,7 @@ dependencies = [ [[package]] name = "perry-ext-better-sqlite3" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "perry-ffi", "rusqlite", @@ -5805,7 +5805,7 @@ dependencies = [ [[package]] name = "perry-ext-cheerio" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "perry-ffi", "scraper", @@ -5813,7 +5813,7 @@ dependencies = [ [[package]] name = "perry-ext-commander" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "perry-ffi", "perry-runtime", @@ -5821,7 +5821,7 @@ dependencies = [ [[package]] name = "perry-ext-cron" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "chrono", "cron", @@ -5831,7 +5831,7 @@ dependencies = [ [[package]] name = "perry-ext-dayjs" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "chrono", "perry-ffi", @@ -5839,7 +5839,7 @@ dependencies = [ [[package]] name = "perry-ext-decimal" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "perry-ffi", "rust_decimal", @@ -5847,7 +5847,7 @@ dependencies = [ [[package]] name = "perry-ext-dotenv" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "perry-ffi", "serde_json", @@ -5855,7 +5855,7 @@ dependencies = [ [[package]] name = "perry-ext-ethers" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "perry-ffi", "rand 0.10.1", @@ -5863,7 +5863,7 @@ dependencies = [ [[package]] name = "perry-ext-events" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "perry-ffi", "perry-runtime", @@ -5871,14 +5871,14 @@ dependencies = [ [[package]] name = "perry-ext-exponential-backoff" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-fastify" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "bytes", "http-body-util", @@ -5896,7 +5896,7 @@ dependencies = [ [[package]] name = "perry-ext-fetch" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "bytes", "lazy_static", @@ -5909,7 +5909,7 @@ dependencies = [ [[package]] name = "perry-ext-http" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "bytes", "h2", @@ -5933,7 +5933,7 @@ dependencies = [ [[package]] name = "perry-ext-ioredis" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "lazy_static", "perry-ffi", @@ -5943,7 +5943,7 @@ dependencies = [ [[package]] name = "perry-ext-jsonwebtoken" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "base64", "jsonwebtoken", @@ -5954,7 +5954,7 @@ dependencies = [ [[package]] name = "perry-ext-lru-cache" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "lru", "perry-ffi", @@ -5963,7 +5963,7 @@ dependencies = [ [[package]] name = "perry-ext-moment" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "chrono", "perry-ffi", @@ -5971,7 +5971,7 @@ dependencies = [ [[package]] name = "perry-ext-mongodb" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "bson", "futures-util", @@ -5983,7 +5983,7 @@ dependencies = [ [[package]] name = "perry-ext-mysql2" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "chrono", "perry-ffi", @@ -5993,7 +5993,7 @@ dependencies = [ [[package]] name = "perry-ext-nanoid" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "nanoid", "perry-ffi", @@ -6002,7 +6002,7 @@ dependencies = [ [[package]] name = "perry-ext-net" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "bytes", "perry-ffi", @@ -6015,7 +6015,7 @@ dependencies = [ [[package]] name = "perry-ext-node-forge" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "const-oid 0.9.6", "der 0.7.10", @@ -6034,7 +6034,7 @@ dependencies = [ [[package]] name = "perry-ext-nodemailer" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "lettre", "perry-ffi", @@ -6044,7 +6044,7 @@ dependencies = [ [[package]] name = "perry-ext-pdf" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "perry-ffi", "printpdf", @@ -6052,7 +6052,7 @@ dependencies = [ [[package]] name = "perry-ext-pg" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "perry-ffi", "sqlx", @@ -6061,7 +6061,7 @@ dependencies = [ [[package]] name = "perry-ext-ratelimit" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "governor", "perry-ffi", @@ -6069,7 +6069,7 @@ dependencies = [ [[package]] name = "perry-ext-sharp" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "fast_image_resize", "image", @@ -6079,14 +6079,14 @@ dependencies = [ [[package]] name = "perry-ext-slugify" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-streams" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "lazy_static", "perry-ffi", @@ -6095,7 +6095,7 @@ dependencies = [ [[package]] name = "perry-ext-undici" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "perry-ffi", "perry-runtime", @@ -6104,7 +6104,7 @@ dependencies = [ [[package]] name = "perry-ext-uuid" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "perry-ffi", "uuid", @@ -6112,7 +6112,7 @@ dependencies = [ [[package]] name = "perry-ext-validator" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "perry-ffi", "regex", @@ -6122,7 +6122,7 @@ dependencies = [ [[package]] name = "perry-ext-ws" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "futures-util", "lazy_static", @@ -6135,7 +6135,7 @@ dependencies = [ [[package]] name = "perry-ext-zlib" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "brotli", "flate2", @@ -6145,7 +6145,7 @@ dependencies = [ [[package]] name = "perry-ffi" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "dashmap", "once_cell", @@ -6154,7 +6154,7 @@ dependencies = [ [[package]] name = "perry-hir" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "anyhow", "perry-api-manifest", @@ -6172,7 +6172,7 @@ dependencies = [ [[package]] name = "perry-parser" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "anyhow", "perry-diagnostics", @@ -6184,7 +6184,7 @@ dependencies = [ [[package]] name = "perry-runtime" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "anyhow", "base64", @@ -6226,14 +6226,14 @@ dependencies = [ [[package]] name = "perry-runtime-static" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "perry-runtime", ] [[package]] name = "perry-stdlib" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "aes 0.8.4", "aes 0.9.1", @@ -6328,14 +6328,14 @@ dependencies = [ [[package]] name = "perry-stdlib-static" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "perry-stdlib", ] [[package]] name = "perry-transform" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "anyhow", "perry-hir", @@ -6344,14 +6344,14 @@ dependencies = [ [[package]] name = "perry-ui" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "perry-ui-model", ] [[package]] name = "perry-ui-android" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "base64", "itoa", @@ -6368,7 +6368,7 @@ dependencies = [ [[package]] name = "perry-ui-geisterhand" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "rand 0.10.1", "serde", @@ -6378,7 +6378,7 @@ dependencies = [ [[package]] name = "perry-ui-gtk4" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "base64", "cairo-rs 0.22.0", @@ -6401,7 +6401,7 @@ dependencies = [ [[package]] name = "perry-ui-ios" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "base64", "block2", @@ -6417,7 +6417,7 @@ dependencies = [ [[package]] name = "perry-ui-macos" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "base64", "block2", @@ -6432,7 +6432,7 @@ dependencies = [ [[package]] name = "perry-ui-model" -version = "0.5.1340" +version = "0.5.1341" [[package]] name = "perry-ui-test" @@ -6443,11 +6443,11 @@ dependencies = [ [[package]] name = "perry-ui-testkit" -version = "0.5.1340" +version = "0.5.1341" [[package]] name = "perry-ui-tvos" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "base64", "block2", @@ -6463,7 +6463,7 @@ dependencies = [ [[package]] name = "perry-ui-visionos" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "base64", "block2", @@ -6479,7 +6479,7 @@ dependencies = [ [[package]] name = "perry-ui-watchos" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "block2", "libc", @@ -6492,7 +6492,7 @@ dependencies = [ [[package]] name = "perry-ui-windows" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "base64", "libc", @@ -6509,14 +6509,14 @@ dependencies = [ [[package]] name = "perry-ui-windows-winui" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "perry-ui-windows", ] [[package]] name = "perry-updater" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "anyhow", "base64", @@ -6532,7 +6532,7 @@ dependencies = [ [[package]] name = "perry-wasm-host" -version = "0.5.1340" +version = "0.5.1341" dependencies = [ "wasmi", ] diff --git a/Cargo.toml b/Cargo.toml index 800fb5c329..d8bb7d5111 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -315,7 +315,7 @@ codegen-units = 16 codegen-units = 16 [workspace.package] -version = "0.5.1340" +version = "0.5.1341" edition = "2021" license = "MIT" repository = "https://github.com/PerryTS/perry" diff --git a/changelog.d/7602-array-push-barrier-parent-gate.md b/changelog.d/7602-array-push-barrier-parent-gate.md new file mode 100644 index 0000000000..e09541831f --- /dev/null +++ b/changelog.d/7602-array-push-barrier-parent-gate.md @@ -0,0 +1,145 @@ +### Codegen: the array-push write barrier is gated on a live parent-generation test (#7511) + +Write barriers were **29.6% of `push_cls`'s symbolicated leaf profile** at +v0.5.1339 — re-measured after #7536, #7594 and #7596, because the ticket's +headline was taken at v0.5.1325 and three tickets this campaign were worked from +stale numbers. It had not collapsed; it had grown. + +| symbol | leaf samples | share | +|---|--:|--:| +| `js_write_barrier_slot` | 346 | 15.1% | +| `write_barrier_decoded_parent` | 161 | 7.0% | +| `barrier_child_prologue` | 139 | 6.1% | +| `incremental_mark_barrier_value` | 30 | 1.3% | +| **total** | **676 / 2286** | **29.6%** | + +**All of it is one call site**, and the call graph names it: `chunk + 568`, the +`keep.push(new Node(...))`. `push_cls_ts__Node_constructor` appears as a leaf +with no barrier beneath it at all — the constructor's two `number` field stores +already pay nothing, because a declared-`number` field selects the raw-f64 +representation and its store is guarded by an inline plain-finite check with a +downgrading cold fallback (`property_set.rs`, the `ptr_shape_set.raw_store` +arm). That is why #7536 measured `push_cls` unchanged. + +**What the surviving barrier actually does, by `PERRY_GC_TRACE` counters:** + +| bench | calls | `non_pointer_child_skips` | `parent_not_old_skips` | `old_to_young_slow_hits` | `new_inserts` | +|---|--:|--:|--:|--:|--:| +| `push_cls` | 19,945,222 | 0 | 19,743,573 (99.0%) | 0 | **0** | +| `churn_alloc` | 19,945,222 | 0 | 19,743,573 | 0 | **0** | +| `churn` | 19,945,222 | 0 | 19,743,573 | 0 | **0** | +| `tree` | 62,612,898 | 20,867,845 | 41,271,511 | 0 | **0** | +| `cycles` | 15,674,953 | 7,832,430 | 7,368,973 | 0 | **0** | +| `retain` | 6,304,687 | 0 | **0** | 3,204,922 | 6,268 | + +The remembered set is **never inserted into — not once — in 20 million calls** on +the three headline benches. And #7536's value-side test can reach none of it: +`non_pointer_child_skips == 0`, because the pushed value genuinely *is* a heap +pointer. `expr_produces_non_pointer_bits_by_construction` is not merely +unhelpful here, it is *correct* to say "pointer". **The waste is entirely +parent-side**, which is a question no by-construction proof can answer: `keep` +crosses hundreds of collections between its allocation and its last push, so +"the parent is young" is exactly the kind of claim #7501 showed gets revoked at +runtime. + +So it is decided by a **live test at the store**, which is what #7501 concluded +collector-facing metadata requires: + +```text +parent_may_need_remembering(parent) := + (header(parent).gc_flags & GC_FLAG_TENURED) != 0 + || PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT != 0 +``` + +Both clauses are load-bearing, for unrelated reasons. + +**The TENURED clause.** The remembered set only ever needs an entry when +`barrier_parent_needs_remembering` classifies the parent `Old`. Soundness is the +superset property again — `Old ⟹ TENURED`, so `!TENURED ⟹ !Old`, and this gate +can only skip a subset of what the runtime already skips. Every path that places +an object into an old-gen block sets the bit in the same breath +(`gc/copying.rs:612–637` selects `arena_alloc_gc_old` and `GC_FLAG_TENURED` from +one `promote` expression; `gc/oldgen.rs:1740` and `:1838`; `buffer/header.rs:486`; +`typedarray/mod.rs:722`; `json_tape.rs` via `arena_alloc_gc_old_born_tenured`), +and nothing ever clears it on a live object. The stronger justification is that +a parent which is neither physically old nor logically tenured is **fully traced +by every minor GC** (`gc/trace.rs:747`), so its edges are rediscovered. + +That invariant is true but **unenforced**: `arena_alloc_gc_old` writes +`GC_FLAG_ARENA | gc_birth_extra_flags()` and leaves the bit to each of its eight +callers, so a ninth would compile, pass every test, and strand a child in +generated code only. Per this repo's gate doctrine an invariant a fast path +depends on must be able to fail, so it is pinned over the production birth paths +by `every_old_gen_birth_path_sets_tenured`. + +A `debug_assert!` in `barrier_parent_needs_remembering` — the better enforcement, +since it rechecks at every old-parent store in every debug run — was written and +then **reverted**: dozens of existing tests build old-gen fixtures straight from +`arena_alloc_gc_old` without the bit (`alloc_old_test_object`, +`alloc_old_test_array`, `alloc_old_test_promise`, most of `gc/tests/oldgen.rs`), +some deliberately, so it fired on fixtures rather than defects. Why it is absent +is recorded where someone would next try to add it; making it shippable means +fixing those fixtures first. + +**The incremental clause.** Skipping the call also skips +`barrier_child_prologue`'s `incremental_mark_barrier_value` — the +insertion/SATB shading, which is not a generational question and must never be +dropped while a cycle is live. A zero count *proves* this thread's +`INCREMENTAL_MARK_BARRIER_VALID_PTRS` is null, because +`incremental_mark_barrier_enable` installs the thread-local **before** +incrementing the count — an ordering `gc/barrier.rs` already documents as +load-bearing. This is the same gate, on the same already-exported global, that +`expr/shadow_inline.rs` and `expr/shadow_slot.rs` emit for the root shading +barrier; no new runtime symbol was needed. + +The gate reads the array's header byte at `arr_handle - 7`, which the `nofwd` +block **already loads** for the forwarding test in the dominating block — so the +whole test is one `and`, one `icmp`, one global load and an `or`. It is emitted +only inside `apush.inbounds`, reached only after that header test, so the +dereference rests on a validation the existing code already performed. The slot +store stays unconditional and outside the branch; only the barrier moves. Under +`PERRY_WRITE_BARRIERS=0` nothing is emitted at all, so that knob's A/B does not +acquire a predicate wrapped around an empty arm. + +**Measured.** Barrier calls on the same counters: `push_cls` / `churn_alloc` / +`churn` 19,945,222 → **119,674** (−99.4%), `push_num` 19,584,064 → 117,506. +`retain`'s real work is bit-identical across arms — `old_to_young_slow_hits` +3,204,922 and `new_inserts` 6,268 in both — so every genuine remembered-set +insert survives, and GC cycle counts are unchanged on every bench (105 / 42 / 16 +/ 7 / 4 / 18). On the pinned quiet mini (load ~1.5, best-of-5 `user+sys`, both +arms against the same runtime archive, baseline measured twice): `push_cls` +0.650 → 0.490 s (**1.33x**), `churn_alloc` 0.660 → 0.510 (1.29x), `push_num` +0.310 → 0.250 (1.24x), `churn` 0.950 → 0.800 (1.19x); `tree`, `retain`, +`cycles`, `deeplist` and `churn_read` flat, because their barrier traffic is +class-field stores rather than array pushes — the honest scope limit, and the +remaining #7511 lever. All ten bench binaries produce byte-identical stdout, +including `cls_mistyped.ts`, and both arms re-run clean under +`PERRY_GC_VERIFY_EVACUATION=1 PERRY_GC_VERIFY_MARK=1` and under +`PERRY_GC_ZEAL=1 PERRY_GC_PROTECT_FROMSPACE=1` with the instrument proven live +(110 `[gc-fromspace-protect] mode=ProtectPages retired_set=#N` sets on +`push_cls`). + +**Tests.** `perry-runtime`'s new `gc::tests::inline_generation_gate_contract` +pins the codegen comparand against `GC_FLAG_TENURED`; asserts the invariant +directly over the three birth paths where the bit is a caller's obligation +rather than a consequence of surviving (`arena_alloc_gc_old_born_tenured`, the +large-object arm, `buffer_alloc`); and carries a **stranding witness** — an +old+tenured parent, a young child, and the exact gated store sequence, where a +sabotaged always-false gate leaves the old→young edge unrecorded and +`verify_old_to_young_edges_covered` rejects it, while the shipped gate keeps it +and the remembered-set scan marks the child. A third arm stores through a +*nursery* parent and asserts nothing is stranded, so the sabotage arm is shown +to be about the parent's generation and not about skipping a barrier per se, and +a fourth pins that an active incremental cycle forces the call for that same +nursery parent. Codegen-side structure — barrier inside `apush.barrier`, slot +store outside it, both clauses present — is pinned in `array_push.rs`'s +`parent_gate_tests`, which run under `cargo test -p perry-codegen --lib` rather +than in the tag-only integration suite. + +That last test initially could not fail. Replacing the gate's `or` with a +constant-true leaves both `and i8 …, 32` and the incremental global in the IR — +the clauses are still computed, just no longer consulted — so substring matching +stayed green while the gate had stopped gating, which is CLAUDE.md hazard 4 +applied to a test rather than a job. It now follows the `cond_br`'s condition +back to its definition and requires an `or i1` of an i8 header test and an i32 +count test. diff --git a/crates/perry-codegen/src/expr/array_push.rs b/crates/perry-codegen/src/expr/array_push.rs index 923435ca12..82b2c07ba1 100644 --- a/crates/perry-codegen/src/expr/array_push.rs +++ b/crates/perry-codegen/src/expr/array_push.rs @@ -19,9 +19,9 @@ use super::{ emit_array_numeric_write_note_on_block, emit_jsvalue_slot_store_with_flags_on_block, emit_jsvalue_slot_store_with_value_bits_on_block, emit_root_nanbox_store_on_block, emit_typed_feedback_register_site, emit_write_barrier, - expr_has_numeric_pointer_free_array_layout, lower_expr, lower_expr_native, - nanbox_pointer_inline, raw_f64_layout_fact, unbox_to_i64, FnCtx, TypedFeedbackContract, - TypedFeedbackKind, + emit_write_barrier_slot_generation_tested, expr_has_numeric_pointer_free_array_layout, + lower_expr, lower_expr_native, nanbox_pointer_inline, raw_f64_layout_fact, unbox_to_i64, FnCtx, + TypedFeedbackContract, TypedFeedbackKind, }; fn emit_array_handle_length(ctx: &mut FnCtx<'_>, array_handle: &str) -> String { @@ -416,7 +416,19 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // Inline store: arr+8+length*8 = value, length++. ctx.current_block = inbounds_idx; - { + // #7511: the barrier is emitted separately, behind an inline + // live test of the PARENT's generation, so the store emitter + // below is told not to emit it. Everything else about the store + // — the slot write, the string addref, the layout note, and + // their ordering — is unchanged. + // + // `js_write_barrier_slot` still lands in exactly the position it + // did before (after the layout note, before the numeric-write + // note and the length bump), because a collection reached + // between the store and the barrier would run with the + // old→young edge unrecorded. The block is split here rather + // than the call being sunk to the end of the block. + let (length, element_addr, barrier_value_bits) = { let blk = ctx.block(); let length = blk.safe_load_i32_from_ptr(&arr_handle); let length_i64 = blk.zext(I32, &length, I64); @@ -436,7 +448,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { layout_note_needed, &arr_handle, &element_addr, - write_barrier_needed, + false, ) } else { emit_jsvalue_slot_store_with_flags_on_block( @@ -449,17 +461,48 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { layout_note_needed, &arr_handle, &element_addr, - write_barrier_needed, + false, ) }; + // The store emitter only hands back the bits when it needed + // them itself; the barrier needs them whenever it is + // emitted, so materialize them here otherwise. + let barrier_value_bits = if write_barrier_needed { + Some( + value_bits + .clone() + .unwrap_or_else(|| blk.bitcast_double_to_i64(&v)), + ) + } else { + None + }; // #7469: provably dead under `declared_all_pointer` — the // `nofwd` admission test proved both raw-f64 bits already // clear, and clearing them is this call's only effect. if !value_is_numeric && !declared_all_pointer { - let value_bits = - value_bits.unwrap_or_else(|| blk.bitcast_double_to_i64(&v)); + let value_bits = barrier_value_bits + .clone() + .or(value_bits) + .unwrap_or_else(|| blk.bitcast_double_to_i64(&v)); emit_array_numeric_write_note_on_block(blk, &arr_handle, &value_bits); } + (length, element_addr, barrier_value_bits) + }; + if let Some(child_bits) = barrier_value_bits { + // `arr_handle` reached this block through the `nofwd` header + // test, so it is a live, non-forwarded GC array user + // pointer — the precondition for reading its header byte. + emit_write_barrier_slot_generation_tested( + ctx, + &arr_handle, + &arr_handle, + &element_addr, + &child_bits, + "apush", + ); + } + { + let blk = ctx.block(); let new_length = blk.add(I32, &length, "1"); let arr_ptr = blk.inttoptr(I64, &arr_handle); // GC_STORE_AUDIT(POINTER_FREE): array length header update has no child pointer. @@ -662,3 +705,192 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { _ => unreachable!("expr/mod.rs dispatched a variant not handled by this submodule"), } } + +#[cfg(test)] +mod parent_gate_tests { + use perry_hir::types::Type; + use perry_hir::{Expr, Function, Module as HirModule, Stmt}; + + /// `const a = []; a.push({v: 1});` — a pointer-valued push into a local + /// array, which is the shape whose barrier #7511 gates. + fn pushing_ir() -> String { + let mut hir = HirModule::new("apush_parent_gate_test"); + hir.functions.push(Function { + id: 0, + name: "pushes".to_string(), + type_params: Vec::new(), + params: Vec::new(), + return_type: Type::Any, + body: vec![ + Stmt::Let { + id: 0, + name: "a".to_string(), + ty: Type::Any, + mutable: true, + init: Some(Expr::Array(Vec::new())), + }, + Stmt::Expr(Expr::ArrayPush { + array_id: 0, + value: Box::new(Expr::Object(vec![("v".to_string(), Expr::Number(1.0))])), + }), + Stmt::Return(Some(Expr::LocalGet(0))), + ], + is_async: false, + is_generator: false, + is_strict: true, + is_exported: false, + captures: Vec::new(), + decorators: Vec::new(), + was_plain_async: false, + was_unrolled: false, + }); + let opts = crate::CompileOptions { + emit_ir_only: true, + ..Default::default() + }; + let bytes = crate::compile_module(&hir, opts).expect("test module compiles"); + String::from_utf8(bytes).expect("LLVM IR is UTF-8") + } + + fn assert_default_barrier_env_not_disabled() { + assert!( + !matches!( + std::env::var("PERRY_WRITE_BARRIERS").as_deref(), + Ok("0") | Ok("off") | Ok("false") + ), + "this test describes DEFAULT barrier emission; PERRY_WRITE_BARRIERS must be unset or on" + ); + } + + /// Block labels carry a uniquing suffix (`apush.barrier.21:`), so collect + /// the gated block's body by walking labels rather than by substring — + /// `apush.barrier.done.22:` would otherwise match a `apush.barrier.` prefix + /// test and silently hand back the WRONG block, which is exactly the block + /// the store is supposed to be in. + fn gated_barrier_block(ir: &str) -> String { + let mut body = Vec::new(); + let mut inside = false; + for line in ir.lines() { + if let Some(label) = line.strip_suffix(':') { + if !label.starts_with(char::is_whitespace) { + inside = label.starts_with("apush.barrier.") + && !label.starts_with("apush.barrier.done"); + continue; + } + } + if inside { + body.push(line); + } + } + assert!( + !body.is_empty(), + "no `apush.barrier.` block in the emitted IR — the push did not take the \ + gated inline tier, so this test would be vacuous:\n{ir}" + ); + body.join("\n") + } + + /// The barrier call must sit in its own block, reached only through the + /// parent-generation `cond_br`, and both clauses of the gate must be + /// present. + #[test] + fn array_push_barrier_is_gated_on_the_parent_header() { + assert_default_barrier_env_not_disabled(); + let ir = pushing_ir(); + assert!( + ir.contains("js_write_barrier_slot"), + "the pointer-valued push must still emit a barrier at all:\n{ir}" + ); + let gated = gated_barrier_block(&ir); + assert!( + gated.contains("js_write_barrier_slot"), + "the gated block must be the one holding the barrier call:\n{gated}" + ); + // Count CALL sites only — the module's `declare` line names the symbol + // too, and counting it would make this compare 2 against 1 forever. + assert_eq!( + ir.matches("call void @js_write_barrier_slot").count(), + gated.matches("call void @js_write_barrier_slot").count(), + "every array-push barrier must be inside the gate — an ungated one would be the \ + cost this ticket exists to remove:\n{ir}" + ); + assert_gate_condition_is_both_clauses(&ir); + } + + /// Follow the `cond_br`'s condition back to its definition and require it to + /// be the `or` of a `GC_FLAG_TENURED` header test and the incremental-count + /// test. + /// + /// Checking only that the IR *contains* `and i8 …, 32` and the global's name + /// is not enough, and this is not hypothetical: replacing the `or` with a + /// constant-true left both of those substrings in place (the clauses are + /// still computed, just no longer consulted) and the test stayed green while + /// the gate had stopped gating. A branch that is always taken is precisely + /// the failure this ticket's perf claim rests on not happening. + fn assert_gate_condition_is_both_clauses(ir: &str) { + let br = ir + .lines() + .find(|l| l.contains("br i1") && l.contains("label %apush.barrier.")) + .unwrap_or_else(|| panic!("no gated branch in the emitted IR:\n{ir}")); + let cond = br + .split_whitespace() + .nth(2) + .and_then(|c| c.strip_suffix(',')) + .unwrap_or_else(|| panic!("cannot read the branch condition from {br:?}")); + let def = ir + .lines() + .find(|l| l.trim_start().starts_with(&format!("{cond} = "))) + .unwrap_or_else(|| panic!("no definition of {cond} in:\n{ir}")); + assert!( + def.contains("or i1"), + "the gate's branch condition must be the OR of both clauses, not {def:?} — a \ + condition that is not an `or` of the two tests is a gate that never skips" + ); + let mut operands = def + .split("or i1 ") + .nth(1) + .expect("or operands") + .split(", ") + .map(str::trim); + let tenured = operands.next().expect("tenured operand"); + let incremental = operands.next().expect("incremental operand"); + let def_of = |name: &str| { + ir.lines() + .find(|l| l.trim_start().starts_with(&format!("{name} = "))) + .unwrap_or_else(|| panic!("no definition of {name} in:\n{ir}")) + .to_string() + }; + assert!( + def_of(tenured).contains("icmp ne i8"), + "the first clause must be the parent's header-byte test, got {:?}", + def_of(tenured) + ); + assert!( + def_of(incremental).contains("icmp ne i32"), + "the second clause must be the incremental-count test, got {:?}", + def_of(incremental) + ); + assert!( + ir.contains("and i8") && ir.contains(", 32"), + "the header test must mask GC_FLAG_TENURED (0x20):\n{ir}" + ); + assert!( + ir.contains("@PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT"), + "dropping the incremental clause would skip the insertion barrier's shading:\n{ir}" + ); + } + + /// The SLOT STORE is unconditional: it must NOT be inside the gated block. + /// Only the bookkeeping moves. + #[test] + fn array_push_slot_store_stays_outside_the_gate() { + assert_default_barrier_env_not_disabled(); + let ir = pushing_ir(); + let gated = gated_barrier_block(&ir); + assert!( + !gated.contains("store double"), + "the element store must stay OUTSIDE the gate — a store that only happens when the \ + parent is tenured would drop the value entirely:\n{gated}" + ); + } +} diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index c494d3f3cf..e86a342177 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -122,8 +122,9 @@ pub(crate) use write_barrier::{ emit_jsvalue_slot_store_pointer_tested, emit_jsvalue_slot_store_scalar_aware_on_block, emit_jsvalue_slot_store_with_flags_on_block, emit_jsvalue_slot_store_with_value_bits_on_block, emit_root_heap_word_store_on_block, emit_root_nanbox_store_on_block, emit_write_barrier, - emit_write_barrier_slot_on_block, lower_array_super_init, lower_event_emitter_subclass_init, - lower_node_stream_super_init, lower_stream_super_init, + emit_write_barrier_slot_generation_tested, emit_write_barrier_slot_on_block, + lower_array_super_init, lower_event_emitter_subclass_init, lower_node_stream_super_init, + lower_stream_super_init, }; // Issue #1098 phase 3: the `FnCtx` definition stays in this trunk, but its diff --git a/crates/perry-codegen/src/expr/write_barrier.rs b/crates/perry-codegen/src/expr/write_barrier.rs index 6be29f54c8..22696bf24f 100644 --- a/crates/perry-codegen/src/expr/write_barrier.rs +++ b/crates/perry-codegen/src/expr/write_barrier.rs @@ -9,7 +9,7 @@ use super::{lower_expr, FnCtx}; use crate::block::LlBlock; use crate::nanbox::double_literal; use crate::native_value::LoweredValue; -use crate::types::{DOUBLE, I1, I32, I64}; +use crate::types::{DOUBLE, I1, I32, I64, I8}; /// Gen-GC Phase C2 helper: emit a write barrier after heap-store sites /// by default. Only explicit `PERRY_WRITE_BARRIERS=0`/`off`/`false` @@ -53,6 +53,136 @@ pub(crate) fn emit_write_barrier_slot_on_block( ); } +/// #7511 — `GC_FLAG_TENURED`, the one header bit that decides whether a +/// parent's slot store can possibly need remembering. +/// +/// Pinned against the runtime constant by +/// `perry-runtime`'s `gc::tests::inline_generation_gate_contract`; codegen +/// cannot `use` the runtime crate, so the value is duplicated and the test is +/// what keeps the two from drifting. +const GC_FLAG_TENURED_I8: &str = "32"; // 0x20 + +/// #7511 — emit the `i1` predicate "this store may need remembered-set work", +/// as a **superset** of the condition the runtime barrier itself acts on. +/// +/// ## What the barrier actually does on these workloads +/// +/// On `push_cls` / `churn_alloc` / `churn` the array push is the only surviving +/// barrier call site, and `PERRY_GC_TRACE` counts 19,945,222 calls of which +/// 19,743,573 (99.0%) end in `parent_not_old_skips` — with +/// `old_to_young_slow_hits == 0` and `new_inserts == 0`. The remembered set is +/// **never inserted into, not once**, yet the call is made 20 million times and +/// costs ~30% of leaf profile. #7536's value-side test cannot reach any of it: +/// `non_pointer_child_skips == 0`, because the pushed value genuinely *is* a +/// heap pointer. The waste is entirely parent-side. +/// +/// ## The two clauses +/// +/// The call is skipped only when BOTH are false. +/// +/// 1. **`gc_flags & GC_FLAG_TENURED`** — the remembered set exists so a minor +/// GC can skip retracing parents it treats as black leaves. Those are +/// exactly the objects that are physically old-gen +/// (`barrier_parent_needs_remembering`'s `classify_heap_generation == Old`) +/// or logically tenured (`GC_FLAG_TENURED`, whose doc records that a tenured +/// object may stay physically in the nursery while "the trace pretends +/// they're old-gen" — `gc/trace.rs:747`). A parent that is neither is +/// **fully traced by every minor GC**, so the edge is rediscovered and needs +/// no record. +/// +/// Soundness rests on `Old ⟹ TENURED`, i.e. `!TENURED ⟹ !Old`, so this +/// predicate can only skip a subset of what the runtime already skips — the +/// same superset discipline `emit_may_carry_heap_pointer_check` uses. Every +/// path that places an object into an old-gen block sets the bit in the same +/// breath, and nothing ever clears it: +/// * `gc/copying.rs:612–637` — one `promote` expression selects +/// `arena_alloc_gc_old` AND `GC_FLAG_TENURED`. +/// * `gc/oldgen.rs:1740`, `gc/oldgen.rs:1838` — evacuation / old-page +/// defrag both OR the bit onto the destination header. +/// * `buffer/header.rs:486`, `typedarray/mod.rs:722` — direct old-gen +/// allocations set it immediately after allocating. +/// * `json_tape.rs` allocates through `arena_alloc_gc_old_born_tenured`. +/// The three sites that write the bit all OR it in; none masks it out. +/// +/// 2. **`PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT != 0`** — skipping the +/// call also skips `barrier_child_prologue`'s +/// `incremental_mark_barrier_value`, the insertion/SATB shading, which is +/// NOT a generational question and must never be dropped while a cycle is +/// live. A zero count *proves* this thread's +/// `INCREMENTAL_MARK_BARRIER_VALID_PTRS` is null, because +/// `incremental_mark_barrier_enable` installs the thread-local BEFORE +/// incrementing the count (`gc/barrier.rs:676–693`, where that ordering is +/// documented as load-bearing for exactly this reason). This is the same +/// gate, on the same global, that `expr/shadow_inline.rs` and +/// `expr/shadow_slot.rs` already emit for the root shading barrier. +/// +/// ## Why a live test and not a static claim +/// +/// #7501: even a static layout *declaration* is revoked at runtime. Generation +/// is strictly worse — an object's generation changes underneath any static +/// proof the moment a collection promotes it, and `keep` in `chunk()` crosses +/// hundreds of collections between its allocation and its last push. There is +/// no by-construction proof that a parent is young, which is precisely why this +/// reads the live header at the store instead. +/// +/// `parent_handle` must be a **validated, non-forwarded GC user pointer** — +/// the caller is responsible for having tested that, because this dereferences +/// `parent_handle - 7`. +pub(crate) fn emit_parent_may_need_remembering_check( + blk: &mut LlBlock, + parent_handle: &str, +) -> String { + let gc_flags_addr = blk.sub(I64, parent_handle, "7"); + let gc_flags_ptr = blk.inttoptr(I64, &gc_flags_addr); + let gc_flags = blk.load(I8, &gc_flags_ptr); + let tenured_bits = blk.and(I8, &gc_flags, GC_FLAG_TENURED_I8); + let is_tenured = blk.icmp_ne(I8, &tenured_bits, "0"); + let active = blk.load_atomic_seq_cst(I32, "@PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT", 4); + let incremental_active = blk.icmp_ne(I32, &active, "0"); + blk.or(I1, &is_tenured, &incremental_active) +} + +/// #7511 — `js_write_barrier_slot` behind +/// [`emit_parent_may_need_remembering_check`]. +/// +/// Terminates the current block. On return `ctx.current_block` is the +/// continuation, which every path reaches, so the caller goes on emitting +/// exactly as before. +/// +/// Emits nothing at all when barrier emission is compile-time disabled, so +/// `PERRY_WRITE_BARRIERS=0` does not leave a predicate and two blocks wrapped +/// around an empty arm — that knob exists to A/B the barrier's cost, and dead +/// IR in one arm makes the comparison lie (the same reasoning +/// `emit_jsvalue_slot_store_pointer_tested` records). +pub(crate) fn emit_write_barrier_slot_generation_tested( + ctx: &mut FnCtx<'_>, + parent_handle: &str, + parent_bits: &str, + slot_addr: &str, + child_bits: &str, + stem: &str, +) { + if !crate::codegen::write_barriers_enabled() { + return; + } + let barrier_idx = ctx.new_block(&format!("{}.barrier", stem)); + let done_idx = ctx.new_block(&format!("{}.barrier.done", stem)); + let barrier_label = ctx.block_label(barrier_idx); + let done_label = ctx.block_label(done_idx); + { + let blk = ctx.block(); + let needed = emit_parent_may_need_remembering_check(blk, parent_handle); + blk.cond_br(&needed, &barrier_label, &done_label); + } + ctx.current_block = barrier_idx; + { + let blk = ctx.block(); + emit_write_barrier_slot_on_block(blk, parent_bits, slot_addr, child_bits); + blk.br(&done_label); + } + ctx.current_block = done_idx; +} + pub(crate) fn emit_root_nanbox_store_on_block(blk: &mut LlBlock, value: &str, root_slot: &str) { blk.store(DOUBLE, value, root_slot); let value_bits = blk.bitcast_double_to_i64(value); diff --git a/crates/perry-runtime/src/gc/barrier.rs b/crates/perry-runtime/src/gc/barrier.rs index d67de9fc2d..e92ef4deec 100644 --- a/crates/perry-runtime/src/gc/barrier.rs +++ b/crates/perry-runtime/src/gc/barrier.rs @@ -1234,6 +1234,19 @@ pub(super) fn barrier_parent_needs_remembering(parent_addr: usize, external_slot crate::arena::classify_heap_generation(parent_addr), crate::arena::HeapGeneration::Old ) { + // #7511: generated code skips this whole call when the parent's header + // has no `GC_FLAG_TENURED` (`emit_parent_may_need_remembering_check`), + // which is sound only while `Old ⟹ TENURED` — and nothing in the + // allocator enforces that, so it is pinned by + // `gc::tests::inline_generation_gate_contract` over the production + // birth paths instead. + // + // A `debug_assert!` here was tried and REVERTED: it is the right + // enforcement point in principle, but dozens of tests build old-gen + // fixtures straight from `arena_alloc_gc_old` without the bit + // (`alloc_old_test_object`, `alloc_old_test_promise`, the + // `gc/tests/oldgen.rs` family), some deliberately. It fired on those, + // not on a defect. Reinstating it means fixing those fixtures first. return true; } external_slot && malloc_gc_parent_addr(parent_addr) diff --git a/crates/perry-runtime/src/gc/tests/inline_generation_gate_contract.rs b/crates/perry-runtime/src/gc/tests/inline_generation_gate_contract.rs new file mode 100644 index 0000000000..28d7e1cc4a --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/inline_generation_gate_contract.rs @@ -0,0 +1,258 @@ +//! #7511 — the contract the codegen-side inline PARENT-generation gate rests on. +//! +//! `perry-codegen`'s `expr::write_barrier::emit_parent_may_need_remembering_check` +//! puts the array push's `js_write_barrier_slot` call behind ONE inline test of +//! the parent's live header byte plus one global: +//! +//! ```text +//! parent_may_need_remembering(parent) := +//! (header(parent).gc_flags & GC_FLAG_TENURED) != 0 +//! || PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT != 0 +//! ``` +//! +//! Two independent obligations, and this file pins both. +//! +//! **1. `Old ⟹ TENURED`.** The remembered set only ever needs an entry when +//! `barrier_parent_needs_remembering` classifies the parent `Old`, so skipping +//! the call on a TENURED-clear header is sound exactly while no live old-gen +//! object can lack the bit. Nothing in the allocator enforces that: +//! `arena_alloc_gc_old` writes `GC_FLAG_ARENA | gc_birth_extra_flags()` and +//! leaves `GC_FLAG_TENURED` to each of its eight callers. A ninth caller that +//! forgets it compiles, passes every existing test, and strands a live child in +//! generated code only. `every_old_gen_birth_path_sets_tenured` is what turns +//! that into a red build, and `barrier_parent_needs_remembering` carries the +//! matching `debug_assert!` so every old-parent store in every debug/test run +//! re-checks it. +//! +//! **2. The incremental clause is not optional.** Skipping the call also skips +//! `barrier_child_prologue`'s `incremental_mark_barrier_value` — the +//! insertion/SATB shading, which is not a generational question at all. A zero +//! count *proves* this thread's `INCREMENTAL_MARK_BARRIER_VALID_PTRS` is null, +//! because `incremental_mark_barrier_enable` installs the thread-local BEFORE +//! incrementing the count; a non-zero count must force the call even for a +//! nursery parent. `the_incremental_clause_forces_the_call_for_a_young_parent` +//! pins that, and fails if the clause is dropped. + +use super::super::*; +use super::support::*; +use std::sync::atomic::Ordering; + +/// The exact flag comparand emitted by `emit_parent_may_need_remembering_check` +/// (codegen spells it `"32"`). Kept as a literal so a drift on either side has +/// to be mirrored by hand rather than silently inherited. +const CODEGEN_GC_FLAG_TENURED: u8 = 0x20; + +/// The codegen predicate, reproduced exactly. +fn codegen_parent_may_need_remembering(parent_flags: u8, incremental_active: u32) -> bool { + parent_flags & CODEGEN_GC_FLAG_TENURED != 0 || incremental_active != 0 +} + +fn header_flags(user_ptr: usize) -> u8 { + unsafe { (*header_from_user_ptr(user_ptr as *const u8)).gc_flags } +} + +#[test] +fn codegen_tenured_comparand_matches_the_runtime_flag() { + assert_eq!( + CODEGEN_GC_FLAG_TENURED, GC_FLAG_TENURED, + "codegen emits `and i8 %gc_flags, {CODEGEN_GC_FLAG_TENURED}` — if the runtime flag moves, \ + the emitted gate silently tests the wrong bit" + ); +} + +/// **The invariant, made able to fail.** Every production path that can place a +/// live object at an address `classify_heap_generation` calls `Old` must leave +/// `GC_FLAG_TENURED` set on it. +/// +/// Deliberately exercises the birth paths that do NOT go through tenuring — +/// those are the ones where the bit is a caller's remembered obligation rather +/// than a consequence of having survived: +/// +/// * the size-independent born-old wrapper (`arena_alloc_gc_old_born_tenured`), +/// * the large-object arm of the ordinary nursery allocator, which diverts +/// anything over `LARGE_OBJECT_THRESHOLD_BYTES` straight into old-gen, +/// * `buffer_alloc`, which is old-gen because its bytes are handed to FFI. +/// +/// The survivor/evacuation paths (`gc/copying.rs`, `gc/oldgen.rs`) set the bit +/// in the same expression that selects the old-gen allocation, so they cannot +/// drift apart; these three can. +#[test] +fn every_old_gen_birth_path_sets_tenured() { + let _guard = GcTestIsolationGuard::new(); + + let born_tenured = + crate::arena::arena_alloc_gc_old_born_tenured(64, 8, GC_TYPE_OBJECT) as usize; + let large = crate::arena::arena_alloc_gc( + crate::gc::LARGE_OBJECT_THRESHOLD_BYTES + 64, + 8, + GC_TYPE_OBJECT, + ) as usize; + let buffer = crate::buffer::buffer_alloc(128) as usize; + + for (label, addr) in [ + ("arena_alloc_gc_old_born_tenured", born_tenured), + ("large-object birth", large), + ("buffer_alloc", buffer), + ] { + assert_eq!( + crate::arena::classify_heap_generation(addr), + crate::arena::HeapGeneration::Old, + "{label} is supposed to be an OLD-generation birth — if it is not, this test has \ + stopped covering the invariant it exists for" + ); + assert_ne!( + header_flags(addr) & GC_FLAG_TENURED, + 0, + "{label} produced a LIVE old-gen object with GC_FLAG_TENURED clear. The #7511 inline \ + gate skips the write barrier on exactly that header, so a store into this object \ + would leave a real old->young edge unrecorded" + ); + } +} + +/// Perform an array-element store the way the guarded codegen sequence does: +/// the slot write is unconditional, and the barrier runs only when `gate` +/// accepts the PARENT. Returns whether the barrier was called. +unsafe fn gated_slot_store( + parent: *mut crate::object::ObjectHeader, + fields: *mut u64, + child_bits: u64, + gate: impl Fn(u8, u32) -> bool, +) -> bool { + *fields = child_bits; + let flags = (*header_from_user_ptr(parent as *const u8)).gc_flags; + let active = crate::gc::PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT.load(Ordering::SeqCst); + if gate(flags, active) { + js_write_barrier_slot(ptr_bits(parent as usize), fields as u64, child_bits); + return true; + } + false +} + +/// **The stranding witness.** An OLD+TENURED parent, a YOUNG child, and the +/// exact store sequence codegen now emits. +/// +/// 1. shipped gate, tenured parent — the barrier runs, the old→young edge +/// verifier is satisfied, and a remembered-set scan marks the child; +/// 2. SABOTAGED gate (always answers "no work" — the shape an inverted branch +/// or a wrong comparand produces) — the same store leaves the edge +/// unrecorded and the verifier REJECTS it: that child is stranded, and the +/// next minor frees it under a live reference; +/// 3. shipped gate, NURSERY parent — the barrier is skipped and the verifier is +/// still satisfied, which is the case the elision exists for and proves (2) +/// is about the parent's generation, not about skipping a barrier per se. +#[test] +fn sabotaged_parent_gate_strands_a_young_child_the_shipped_gate_keeps() { + let _guard = GcTestIsolationGuard::new(); + + // (1) shipped gate, OLD+TENURED parent. Old parent allocated FIRST: `young` + // is a bare `usize` that production mode neither roots nor pins, so no + // allocation may follow it. + reset_remembered_set(); + clear_marks(); + let (old_obj, fields) = unsafe { alloc_old_test_object(1) }; + let old_header = unsafe { header_from_user_ptr(old_obj as *const u8) }; + // `alloc_old_test_object` calls `arena_alloc_gc_old` directly and does NOT + // set the bit — production old-gen objects always carry it (pinned by + // `every_old_gen_birth_path_sets_tenured`), so set it here to model one. + unsafe { (*old_header).gc_flags |= GC_FLAG_TENURED | GC_FLAG_MARKED }; + let young = crate::arena::arena_alloc_gc(40, 8, GC_TYPE_OBJECT) as usize; + assert!( + unsafe { + gated_slot_store( + old_obj, + fields, + ptr_bits(young), + codegen_parent_may_need_remembering, + ) + }, + "a TENURED parent must take the barrier branch" + ); + let stats = verify_old_to_young_edges_covered(); + assert_eq!(stats.checked_old_to_young_edges, 1); + assert_eq!(stats.missing_edges, 0); + let valid_ptrs = build_valid_pointer_set(); + let scan = mark_remembered_set_roots(&valid_ptrs); + assert_eq!(scan.newly_marked, 1, "the child must survive the minor"); + unsafe { (*old_header).gc_flags &= !GC_FLAG_MARKED }; + clear_marks(); + remembered_set_clear(); + + // (2) SABOTAGE: a gate that never accepts. + fn never_needs_remembering(_flags: u8, _active: u32) -> bool { + false + } + reset_remembered_set(); + clear_marks(); + let (old_obj2, fields2) = unsafe { alloc_old_test_object(1) }; + let old_header2 = unsafe { header_from_user_ptr(old_obj2 as *const u8) }; + unsafe { (*old_header2).gc_flags |= GC_FLAG_TENURED | GC_FLAG_MARKED }; + let young2 = crate::arena::arena_alloc_gc(40, 8, GC_TYPE_OBJECT) as usize; + assert!( + !unsafe { gated_slot_store(old_obj2, fields2, ptr_bits(young2), never_needs_remembering) }, + "the sabotaged gate must skip the barrier — otherwise this arm proves nothing" + ); + let rejected = std::panic::catch_unwind(verify_old_to_young_edges_covered); + assert!( + rejected.is_err(), + "a skipped barrier on an OLD parent must leave the old->young edge unrecorded — this is \ + the stranded child the shipped gate must never produce" + ); + unsafe { (*old_header2).gc_flags &= !GC_FLAG_MARKED }; + clear_marks(); + remembered_set_clear(); + + // (3) shipped gate, NURSERY parent — skipped, and nothing is stranded. + reset_remembered_set(); + clear_marks(); + let (young_parent, yfields) = unsafe { alloc_nursery_test_object(1) }; + let yheader = unsafe { header_from_user_ptr(young_parent as *const u8) }; + assert_eq!( + unsafe { (*yheader).gc_flags } & GC_FLAG_TENURED, + 0, + "a fresh nursery object must not be TENURED — otherwise this arm exercises nothing" + ); + let young3 = crate::arena::arena_alloc_gc(40, 8, GC_TYPE_OBJECT) as usize; + assert!( + !unsafe { + gated_slot_store( + young_parent, + yfields, + ptr_bits(young3), + codegen_parent_may_need_remembering, + ) + }, + "a nursery parent publishes no old->young edge, so the gate must skip the call" + ); + let young_stats = verify_old_to_young_edges_covered(); + assert_eq!( + young_stats.missing_edges, 0, + "a young->young store strands nothing when its barrier is skipped" + ); + clear_marks(); + remembered_set_clear(); +} + +/// **The incremental clause is load-bearing.** With a cycle active, a nursery +/// parent must still force the call, because the skipped work includes +/// `incremental_mark_barrier_value`'s insertion shading and that has nothing to +/// do with generations. Deleting the clause from the emitted predicate turns +/// this red. +#[test] +fn the_incremental_clause_forces_the_call_for_a_young_parent() { + let nursery_parent_flags = GC_FLAG_ARENA; + assert_eq!( + nursery_parent_flags & GC_FLAG_TENURED, + 0, + "the fixture must be a non-tenured parent for this test to mean anything" + ); + assert!( + !codegen_parent_may_need_remembering(nursery_parent_flags, 0), + "with no cycle active a nursery parent is exactly the case the gate skips" + ); + assert!( + codegen_parent_may_need_remembering(nursery_parent_flags, 1), + "with an incremental cycle active the SAME parent must take the call — otherwise the \ + store skips its insertion barrier and a live object is swept" + ); +} diff --git a/crates/perry-runtime/src/gc/tests/mod.rs b/crates/perry-runtime/src/gc/tests/mod.rs index 604a8cfb3f..eb1b0a2a21 100644 --- a/crates/perry-runtime/src/gc/tests/mod.rs +++ b/crates/perry-runtime/src/gc/tests/mod.rs @@ -19,6 +19,7 @@ mod global_bootstrap; mod helper_stores; mod host_safepoints; mod incremental_sweep_reclaim; +mod inline_generation_gate_contract; mod inline_pointer_bearing_contract; mod layout_trace; mod lazy_tape_side_alloc;