From 7eb2aeb4659903db336a075f74d447acdbcd6b8b Mon Sep 17 00:00:00 2001 From: jdalton Date: Sat, 1 Aug 2026 10:28:29 -0400 Subject: [PATCH] fix(codegen): make every GC root store dominate the collection points after it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #7184 fixed one instance of "a live GC value is invisible to the moving minor because its root store silently no-ops": the store's slot index fell outside the pushed shadow frame, so js_shadow_slot_bind bounds-checked it away. This is its sibling — the store is emitted in-frame, but LATE. The invariant is that a GC-managed value's root store must DOMINATE every subsequent site that can trigger a collection; four lowerings broke it by keeping the value in a bare SSA register across a call that allocates, which under PERRY_GC_MOVING_LOOP_POLLS=1 reaches a back-edge poll and an evacuating minor. new C(...) is the load-bearing one. The instance was a raw register while the constructor body ran. It is rooted inside the callee (the `this` parameter has a shadow slot), so the minor does not free it — it MOVES it and rewrites the callee's root, leaving the caller's register naming from-space. js_gc_init_typed_shape_layout then installed the layout descriptor on the abandoned copy and js_ctor_return_override published that dead address into the caller's shadow slot: a *rooted* slot holding a dangling pointer, which is exactly why #7154's from-space scan only ever saw offenders one or more cycles after the target died, with correct layout coverage. The instance is now temp-rooted across the constructor and re-read afterwards, on both the standalone-_constructor symbol path and the inline-ctor path. A class with no constructor, no fields and no heritage runs no user code in that window and keeps its previous IR exactly. Three more sites of the same shape: * Expr::ObjectSpread — `{ ...a, k: f() }` allocated the object and then wrote every field through a register held across each part's lowering, with no rooting at all. Expr::Object has used RootedHandle for this since #6951; the spread form never got it. * Expr::ClassExprFresh — same for the fresh class object built by a class-expression factory, across its static-field initializers, captured arguments, symbol statics and `static { … }` blocks. * property / element stores — `o.k = f()` evaluates the reference first and the value second (spec order), leaving the receiver in a register across `f()`. The slot it was loaded from is a root and gets rewritten; the register does not, so the store landed in from-space and the field never appeared on the object the program kept. This is #7114 with a receiver instead of a string literal. temp_root::ReceiverGuard roots it only when the value expression can collect, so an inert RHS keeps its previous IR. temp_root_scope_begin now takes the caller's extra reason to open a scope, because `new C()` with no arguments still needs a marker to cut against. Verified by test-files/test_gap_gc_new_instance_rooting.ts: wrong 9 times in 400 iterations at pure 73a9084ea under PERRY_GC_MOVING_LOOP_POLLS=1 (3/3 runs, deterministic), clean 5/5 by default, clean 10/10 with this fix. A codegen regression test pins the def-use chain: the value js_ctor_return_override publishes must be re-derived from the instance's temp root. Also adds scripts/gc_root_dominance_check.py, the static checker that found these — it builds each function's CFG from the emitted LLVM, computes real Cooper/Harvey/Kennedy dominance, and reports every root store that does not dominate a preceding collection point. Both shipped bugs of this class are invisible to runtime GC probes, because at the moment of the collection there is nothing for the collector to find. Over the 196-module sfw-registry corpus it reported 234 violations before this change and 1 after. sfw-registry --help under PERRY_GC_MOVING_LOOP_POLLS=1 is still red — at least one more site of this class remains — so this does not close #7154 and stopgap #7161 stays. Its default arm is unchanged (clean 5/5). Refs #7154, #7184, #7161, #7114, #6951. --- CLAUDE.md | 2 +- Cargo.lock | 152 ++-- Cargo.toml | 2 +- changelog.d/7192-root-store-dominance.md | 17 + crates/perry-codegen/src/expr/index_set.rs | 12 + .../src/expr/logical_collections.rs | 19 +- crates/perry-codegen/src/expr/property_set.rs | 12 + .../src/expr/static_field_meta.rs | 32 +- crates/perry-codegen/src/expr/temp_root.rs | 82 ++- crates/perry-codegen/src/lower_call/new.rs | 90 ++- .../tests/temp_root_operand_temporaries.rs | 126 ++++ scripts/gc_root_dominance_check.py | 692 ++++++++++++++++++ .../test_gap_gc_new_instance_rooting.ts | 49 ++ 13 files changed, 1202 insertions(+), 85 deletions(-) create mode 100644 changelog.d/7192-root-store-dominance.md create mode 100755 scripts/gc_root_dominance_check.py create mode 100644 test-files/test_gap_gc_new_instance_rooting.ts diff --git a/CLAUDE.md b/CLAUDE.md index 9882fb3783..2869076f49 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.1276 +**Current Version:** 0.5.1277 ## TypeScript Parity Status diff --git a/Cargo.lock b/Cargo.lock index 9ec1ad5619..e4d770c964 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5503,7 +5503,7 @@ checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" [[package]] name = "perry" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "anyhow", "base64", @@ -5563,14 +5563,14 @@ dependencies = [ [[package]] name = "perry-api-manifest" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "serde", ] [[package]] name = "perry-audio-miniaudio" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "cc", "libc", @@ -5578,7 +5578,7 @@ dependencies = [ [[package]] name = "perry-codegen" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "anyhow", "log", @@ -5592,7 +5592,7 @@ dependencies = [ [[package]] name = "perry-codegen-arkts" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "anyhow", "perry-hir", @@ -5600,7 +5600,7 @@ dependencies = [ [[package]] name = "perry-codegen-glance" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "anyhow", "perry-hir", @@ -5608,7 +5608,7 @@ dependencies = [ [[package]] name = "perry-codegen-js" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "anyhow", "perry-dispatch", @@ -5617,7 +5617,7 @@ dependencies = [ [[package]] name = "perry-codegen-swiftui" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "anyhow", "perry-hir", @@ -5625,7 +5625,7 @@ dependencies = [ [[package]] name = "perry-codegen-wasm" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "anyhow", "base64", @@ -5637,7 +5637,7 @@ dependencies = [ [[package]] name = "perry-codegen-wear-tiles" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "anyhow", "perry-hir", @@ -5645,7 +5645,7 @@ dependencies = [ [[package]] name = "perry-container-compose" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "anyhow", "async-trait", @@ -5674,14 +5674,14 @@ dependencies = [ [[package]] name = "perry-container-e2e" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "anyhow", ] [[package]] name = "perry-diagnostics" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "serde", "serde_json", @@ -5689,7 +5689,7 @@ dependencies = [ [[package]] name = "perry-dispatch" -version = "0.5.1276" +version = "0.5.1277" [[package]] name = "perry-doc-fixture-my-bindings" @@ -5700,7 +5700,7 @@ dependencies = [ [[package]] name = "perry-doc-tests" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "anyhow", "clap", @@ -5715,7 +5715,7 @@ dependencies = [ [[package]] name = "perry-ext-ads" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "block2", "objc2", @@ -5725,7 +5725,7 @@ dependencies = [ [[package]] name = "perry-ext-argon2" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "argon2", "perry-ffi", @@ -5733,7 +5733,7 @@ dependencies = [ [[package]] name = "perry-ext-axios" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "perry-ffi", "reqwest", @@ -5742,7 +5742,7 @@ dependencies = [ [[package]] name = "perry-ext-bcrypt" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "bcrypt", "perry-ffi", @@ -5750,7 +5750,7 @@ dependencies = [ [[package]] name = "perry-ext-better-sqlite3" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "perry-ffi", "rusqlite", @@ -5758,7 +5758,7 @@ dependencies = [ [[package]] name = "perry-ext-cheerio" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "perry-ffi", "scraper", @@ -5766,7 +5766,7 @@ dependencies = [ [[package]] name = "perry-ext-commander" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "perry-ffi", "perry-runtime", @@ -5774,7 +5774,7 @@ dependencies = [ [[package]] name = "perry-ext-cron" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "chrono", "cron", @@ -5784,7 +5784,7 @@ dependencies = [ [[package]] name = "perry-ext-dayjs" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "chrono", "perry-ffi", @@ -5792,7 +5792,7 @@ dependencies = [ [[package]] name = "perry-ext-decimal" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "perry-ffi", "rust_decimal", @@ -5800,7 +5800,7 @@ dependencies = [ [[package]] name = "perry-ext-dotenv" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "perry-ffi", "serde_json", @@ -5808,7 +5808,7 @@ dependencies = [ [[package]] name = "perry-ext-ethers" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "perry-ffi", "rand 0.10.1", @@ -5816,7 +5816,7 @@ dependencies = [ [[package]] name = "perry-ext-events" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "perry-ffi", "perry-runtime", @@ -5824,14 +5824,14 @@ dependencies = [ [[package]] name = "perry-ext-exponential-backoff" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-fastify" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "bytes", "http-body-util", @@ -5849,7 +5849,7 @@ dependencies = [ [[package]] name = "perry-ext-fetch" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "bytes", "lazy_static", @@ -5862,7 +5862,7 @@ dependencies = [ [[package]] name = "perry-ext-http" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "bytes", "h2", @@ -5886,7 +5886,7 @@ dependencies = [ [[package]] name = "perry-ext-ioredis" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "lazy_static", "perry-ffi", @@ -5896,7 +5896,7 @@ dependencies = [ [[package]] name = "perry-ext-jsonwebtoken" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "base64", "jsonwebtoken", @@ -5907,7 +5907,7 @@ dependencies = [ [[package]] name = "perry-ext-lru-cache" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "lru", "perry-ffi", @@ -5916,7 +5916,7 @@ dependencies = [ [[package]] name = "perry-ext-moment" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "chrono", "perry-ffi", @@ -5924,7 +5924,7 @@ dependencies = [ [[package]] name = "perry-ext-mongodb" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "bson", "futures-util", @@ -5936,7 +5936,7 @@ dependencies = [ [[package]] name = "perry-ext-mysql2" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "chrono", "perry-ffi", @@ -5946,7 +5946,7 @@ dependencies = [ [[package]] name = "perry-ext-nanoid" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "nanoid", "perry-ffi", @@ -5955,7 +5955,7 @@ dependencies = [ [[package]] name = "perry-ext-net" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "bytes", "perry-ffi", @@ -5968,7 +5968,7 @@ dependencies = [ [[package]] name = "perry-ext-node-forge" -version = "0.5.1276" +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.1276" +version = "0.5.1277" dependencies = [ "lettre", "perry-ffi", @@ -5997,7 +5997,7 @@ dependencies = [ [[package]] name = "perry-ext-pdf" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "perry-ffi", "printpdf", @@ -6005,7 +6005,7 @@ dependencies = [ [[package]] name = "perry-ext-pg" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "perry-ffi", "sqlx", @@ -6014,7 +6014,7 @@ dependencies = [ [[package]] name = "perry-ext-ratelimit" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "governor", "perry-ffi", @@ -6022,7 +6022,7 @@ dependencies = [ [[package]] name = "perry-ext-sharp" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "fast_image_resize", "image", @@ -6032,14 +6032,14 @@ dependencies = [ [[package]] name = "perry-ext-slugify" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-streams" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "lazy_static", "perry-ffi", @@ -6048,7 +6048,7 @@ dependencies = [ [[package]] name = "perry-ext-undici" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "perry-ffi", "perry-runtime", @@ -6057,7 +6057,7 @@ dependencies = [ [[package]] name = "perry-ext-uuid" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "perry-ffi", "uuid", @@ -6065,7 +6065,7 @@ dependencies = [ [[package]] name = "perry-ext-validator" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "perry-ffi", "regex", @@ -6075,7 +6075,7 @@ dependencies = [ [[package]] name = "perry-ext-ws" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "futures-util", "lazy_static", @@ -6088,7 +6088,7 @@ dependencies = [ [[package]] name = "perry-ext-zlib" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "brotli", "flate2", @@ -6098,7 +6098,7 @@ dependencies = [ [[package]] name = "perry-ffi" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "dashmap", "once_cell", @@ -6107,7 +6107,7 @@ dependencies = [ [[package]] name = "perry-hir" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "anyhow", "perry-api-manifest", @@ -6125,7 +6125,7 @@ dependencies = [ [[package]] name = "perry-parser" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "anyhow", "perry-diagnostics", @@ -6137,7 +6137,7 @@ dependencies = [ [[package]] name = "perry-runtime" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "anyhow", "base64", @@ -6178,14 +6178,14 @@ dependencies = [ [[package]] name = "perry-runtime-static" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "perry-runtime", ] [[package]] name = "perry-stdlib" -version = "0.5.1276" +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.1276" +version = "0.5.1277" dependencies = [ "perry-stdlib", ] [[package]] name = "perry-transform" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "anyhow", "perry-hir", @@ -6296,14 +6296,14 @@ dependencies = [ [[package]] name = "perry-ui" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "perry-ui-model", ] [[package]] name = "perry-ui-android" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "base64", "itoa", @@ -6320,7 +6320,7 @@ dependencies = [ [[package]] name = "perry-ui-geisterhand" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "rand 0.10.1", "serde", @@ -6330,7 +6330,7 @@ dependencies = [ [[package]] name = "perry-ui-gtk4" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "base64", "cairo-rs 0.22.0", @@ -6353,7 +6353,7 @@ dependencies = [ [[package]] name = "perry-ui-ios" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "base64", "block2", @@ -6369,7 +6369,7 @@ dependencies = [ [[package]] name = "perry-ui-macos" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "base64", "block2", @@ -6384,7 +6384,7 @@ dependencies = [ [[package]] name = "perry-ui-model" -version = "0.5.1276" +version = "0.5.1277" [[package]] name = "perry-ui-test" @@ -6395,11 +6395,11 @@ dependencies = [ [[package]] name = "perry-ui-testkit" -version = "0.5.1276" +version = "0.5.1277" [[package]] name = "perry-ui-tvos" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "base64", "block2", @@ -6415,7 +6415,7 @@ dependencies = [ [[package]] name = "perry-ui-visionos" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "base64", "block2", @@ -6431,7 +6431,7 @@ dependencies = [ [[package]] name = "perry-ui-watchos" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "block2", "libc", @@ -6444,7 +6444,7 @@ dependencies = [ [[package]] name = "perry-ui-windows" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "base64", "libc", @@ -6461,14 +6461,14 @@ dependencies = [ [[package]] name = "perry-ui-windows-winui" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "perry-ui-windows", ] [[package]] name = "perry-updater" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "anyhow", "base64", @@ -6484,7 +6484,7 @@ dependencies = [ [[package]] name = "perry-wasm-host" -version = "0.5.1276" +version = "0.5.1277" dependencies = [ "wasmi", ] diff --git a/Cargo.toml b/Cargo.toml index 8de3599f96..08317d4427 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -292,7 +292,7 @@ codegen-units = 16 codegen-units = 16 [workspace.package] -version = "0.5.1276" +version = "0.5.1277" edition = "2021" license = "MIT" repository = "https://github.com/PerryTS/perry" diff --git a/changelog.d/7192-root-store-dominance.md b/changelog.d/7192-root-store-dominance.md new file mode 100644 index 0000000000..65bf31b367 --- /dev/null +++ b/changelog.d/7192-root-store-dominance.md @@ -0,0 +1,17 @@ +### Fixed + +- **codegen: a freshly-allocated value could be held across a collection point before its root store, unrooting it under the moving minor (#7154 partial)**. #7184 fixed one instance of "a live GC value is invisible to the moving minor because its root store silently no-ops"; this is its sibling — the root store is emitted, but *late*. The invariant is that a GC-managed value's root store must **dominate** every subsequent site that can trigger a collection; four lowerings broke it by keeping the value in a bare LLVM SSA register across a call that allocates, which under `PERRY_GC_MOVING_LOOP_POLLS=1` reaches a back-edge poll and an evacuating minor: + - **`new C(…)` (`lower_call/new.rs`)** — the load-bearing one. The instance was a raw register while the constructor body ran. It is rooted inside the callee (the `this` parameter has a shadow slot), so the minor does not free it — it *moves* it and rewrites the callee's root, leaving the caller's register naming from-space. `js_gc_init_typed_shape_layout` then installed the layout descriptor on the abandoned copy and `js_ctor_return_override` published that dead address into the caller's shadow slot: a *rooted* slot holding a dangling pointer, which is exactly why #7154's from-space scan only ever saw offenders one or more cycles after the target died, with correct layout coverage. The instance is now temp-rooted across the constructor and re-read afterwards, on both the standalone-`_constructor` symbol path and the inline-ctor path. A class with no constructor, no fields and no heritage runs no user code in that window and keeps its previous IR exactly. + - **`Expr::ObjectSpread` (`expr/logical_collections.rs`)** — `{ ...a, k: f() }` allocated the object and then wrote every field through a register held across each part's lowering, with no rooting at all. `Expr::Object` has used `RootedHandle` for this since #6951; the spread form never got it. + - **`Expr::ClassExprFresh` (`expr/static_field_meta.rs`)** — same shape for the fresh class object built by a class-expression factory, across its static-field initializers, captured arguments, symbol statics and `static { … }` blocks. + - **property / element stores (`expr/property_set.rs`, `expr/index_set.rs`)** — `o.k = f()` and `o[k] = f()` evaluate the reference first and the value second (spec order), leaving the receiver in a register across `f()`. The *slot* it was loaded from is a root and gets rewritten; the register does not, so the store landed in from-space and the field never appeared on the object the program kept. This is #7114 with a receiver instead of a string literal. A new `temp_root::ReceiverGuard` roots the receiver only when the value expression can collect, so stores with an inert RHS keep their previous IR. + + `temp_root::temp_root_scope_begin` now takes the caller's extra reason to open a scope, because `new C()` with no arguments still needs a marker to cut the instance root against. + + Verified end to end by `test-files/test_gap_gc_new_instance_rooting.ts` — `new C(n)` whose constructor allocates in a loop, then reading a field of the instance. It is wrong 9 times in 400 iterations at pure `73a9084ea` under `PERRY_GC_MOVING_LOOP_POLLS=1` (3/3 runs, deterministic), clean 5/5 by default, and clean 10/10 with this fix. + + `sfw-registry --help` under `PERRY_GC_MOVING_LOOP_POLLS=1` is **still red**: at least one more site of this class remains, so this does not close #7154 and stopgap #7161 stays. Its default arm is unchanged (clean 5/5). + +### Added + +- **`scripts/gc_root_dominance_check.py`** — the static checker that found the above, kept as a debug tool. It parses perry-emitted LLVM IR, builds each function's CFG, computes real Cooper/Harvey/Kennedy dominance, and reports every shadow-slot root store that does **not** dominate a preceding collection point, with the intervening call named. Both shipped bugs of this class are invisible to runtime GC probes — at the moment of the collection there is nothing for the collector to find — so a static pass over the emitted IR is the only instrument that sees them before they crash. Deliberately one-sided: `NONCOLLECTING` is the only place a call is declared safe and each entry cites the runtime source line that proves it, so a missing entry costs a false positive and never a missed bug. Over the 196-module `sfw-registry` corpus it reported 234 violations before this change and 1 after. Exits non-zero on any violation, so it can gate. diff --git a/crates/perry-codegen/src/expr/index_set.rs b/crates/perry-codegen/src/expr/index_set.rs index 4d6c441b80..aa89eb277e 100644 --- a/crates/perry-codegen/src/expr/index_set.rs +++ b/crates/perry-codegen/src/expr/index_set.rs @@ -1426,12 +1426,18 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { } if let Expr::String(literal) = index.as_ref() { let obj_box = lower_expr(ctx, object)?; + // #7154: the value expression can collect, and an evacuating + // minor inside it relocates the receiver out from under + // `obj_box`. Root it across the evaluation and re-read below. + let recv_guard = + super::temp_root::guard_store_receiver(ctx, object, &obj_box, value); let (val_double, _val_bits) = lower_value_for_dynamic_index_set( ctx, value, "index_set.literal_string_value_bits", "literal_string_index_set_helper_edge", )?; + let obj_box = super::temp_root::reread_store_receiver(ctx, &recv_guard, &obj_box); let key_idx = ctx.strings.intern(literal); let key_handle_global = format!("@{}", ctx.strings.entry(key_idx).handle_global); let obj_bits = ctx.block().bitcast_double_to_i64(&obj_box); @@ -1470,10 +1476,14 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { (DOUBLE, &val_double), ], ); + super::temp_root::release_store_receiver(ctx, recv_guard); return Ok(val_double); } if is_string_expr(ctx, index) { let obj_box = lower_expr(ctx, object)?; + // #7154: see the literal-key arm above. + let recv_guard = + super::temp_root::guard_store_receiver(ctx, object, &obj_box, value); let key_box = lower_expr(ctx, index)?; let (val_double, _val_bits) = lower_value_for_dynamic_index_set( ctx, @@ -1481,6 +1491,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { "index_set.string_value_bits", "string_index_set_helper_edge", )?; + let obj_box = super::temp_root::reread_store_receiver(ctx, &recv_guard, &obj_box); let obj_bits = ctx.block().bitcast_double_to_i64(&obj_box); super::property_set::emit_nullish_write_guard( ctx, @@ -1516,6 +1527,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { (DOUBLE, &val_double), ], ); + super::temp_root::release_store_receiver(ctx, recv_guard); return Ok(val_double); } // Fallback with runtime STRING_TAG check, matching IndexGet. diff --git a/crates/perry-codegen/src/expr/logical_collections.rs b/crates/perry-codegen/src/expr/logical_collections.rs index 9b415ffc68..28a9f67a4b 100644 --- a/crates/perry-codegen/src/expr/logical_collections.rs +++ b/crates/perry-codegen/src/expr/logical_collections.rs @@ -913,6 +913,18 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { "js_object_alloc", &[(I32, &class_id), (I32, &count_str)], ); + // #7154: the half-built object is a raw SSA register while every + // part is lowered, and a part's initializer allocates (zod's + // `classic/schemas.ts` builds a 269-key spread whose values are + // `$ZodAny()` etc. — full JS calls). An evacuating minor inside one + // of those relocates the object, and every later + // `js_object_set_field_by_name` then writes into abandoned + // from-space memory, so the fields silently vanish from the copy + // the caller receives. This is the same rooting contract + // `Expr::Object` has used since #6951; `ObjectSpread` never got it. + let protect_handle = + super::temp_root::any_may_trigger_gc(ctx, parts.iter().map(|(_, v)| v)); + let rooted = super::temp_root::rooted_handle_begin(ctx, &obj_handle, protect_handle); for (key_opt, value_expr) in parts { if let Some(key) = key_opt { // Static key:value pair. @@ -920,6 +932,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { let key_idx = ctx.strings.intern(key); let key_handle_global = format!("@{}", ctx.strings.entry(key_idx).handle_global); + let obj_handle = super::temp_root::rooted_handle_get(ctx, &rooted); let blk = ctx.block(); let key_box = blk.load(DOUBLE, &key_handle_global); let key_bits = blk.bitcast_double_to_i64(&key_box); @@ -932,13 +945,17 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // `...expr` spread — copy all own fields from the // source object into `obj_handle`. let src_box = lower_expr(ctx, value_expr)?; + let obj_handle = super::temp_root::rooted_handle_get(ctx, &rooted); ctx.block().call_void( "js_object_copy_own_fields", &[(I64, &obj_handle), (DOUBLE, &src_box)], ); } } - Ok(nanbox_pointer_inline(ctx.block(), &obj_handle)) + let obj_handle = super::temp_root::rooted_handle_get(ctx, &rooted); + let boxed = nanbox_pointer_inline(ctx.block(), &obj_handle); + super::temp_root::rooted_handle_release(ctx, rooted); + Ok(boxed) } // -------- Object.assign(target, ...sources) -------- diff --git a/crates/perry-codegen/src/expr/property_set.rs b/crates/perry-codegen/src/expr/property_set.rs index 9d12aa7193..cbe97f1d6d 100644 --- a/crates/perry-codegen/src/expr/property_set.rs +++ b/crates/perry-codegen/src/expr/property_set.rs @@ -50,7 +50,10 @@ fn lower_runtime_property_set_by_name( value: &Expr, ) -> Result { let recv_box = lower_expr(ctx, object)?; + // #7154: root the receiver across the value's evaluation, which allocates. + let recv_guard = super::temp_root::guard_store_receiver(ctx, object, &recv_box, value); let val_double = lower_expr(ctx, value)?; + let recv_box = super::temp_root::reread_store_receiver(ctx, &recv_guard, &recv_box); let key_idx = ctx.strings.intern(property); let dispatch_global = ctx.strings.static_dispatch_global(key_idx); let blk = ctx.block(); @@ -60,6 +63,7 @@ fn lower_runtime_property_set_by_name( "js_object_set_field_by_property_id", &[(I64, &obj_bits), (I64, &property_id), (DOUBLE, &val_double)], ); + super::temp_root::release_store_receiver(ctx, recv_guard); Ok(val_double) } @@ -995,12 +999,18 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { } } let obj_box = lower_expr(ctx, object)?; + // #7154: the value expression can collect, and an evacuating minor + // inside it relocates the receiver out from under `obj_box` -- + // `obj.k = f()` then writes `k` into abandoned from-space memory + // and the field never appears on the object the program keeps. + let recv_guard = super::temp_root::guard_store_receiver(ctx, object, &obj_box, value); let (val_double, _val_bits) = lower_value_for_dynamic_property_set( ctx, value, "property_set.dynamic_value_bits", "dynamic_property_set_helper_edge", )?; + let obj_box = super::temp_root::reread_store_receiver(ctx, &recv_guard, &obj_box); // Intern the field name in the StringPool (same one the // matching getter uses, so they share the global string). let key_idx = ctx.strings.intern(property); @@ -1023,6 +1033,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { "js_object_set_field_by_name", &[(I64, &obj_bits), (I64, &key_raw), (DOUBLE, &val_double)], ); + super::temp_root::release_store_receiver(ctx, recv_guard); return Ok(val_double); } let site_id = emit_typed_feedback_register_site( @@ -1040,6 +1051,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { (DOUBLE, &val_double), ], ); + super::temp_root::release_store_receiver(ctx, recv_guard); Ok(val_double) } diff --git a/crates/perry-codegen/src/expr/static_field_meta.rs b/crates/perry-codegen/src/expr/static_field_meta.rs index 361925e323..9304764f58 100644 --- a/crates/perry-codegen/src/expr/static_field_meta.rs +++ b/crates/perry-codegen/src/expr/static_field_meta.rs @@ -424,10 +424,27 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { "js_class_object_pin_parent", &[(I64, &obj), (I32, &tcid_str)], ); + // #7154: the fresh class object is a raw SSA register while every + // static initializer and captured argument is lowered, and those + // allocate. An evacuating minor relocates it, after which each + // remaining `js_object_set_field_by_name` writes into from-space — + // the statics land on the abandoned copy. Same rooting contract + // `Expr::Object` has used since #6951. + // + // `captured_args` forces protection on its own, independently of + // whether the capture *expressions* collect: the snapshot below + // allocates a `js_array_alloc` accumulator and grows it with + // `js_array_push_f64` per element, and those are collection points + // even when every element is an inert `LocalGet`. + let protect_handle = !captured_args.is_empty() + || !symbol_statics.is_empty() + || super::temp_root::any_may_trigger_gc(ctx, named_statics.iter().map(|(_, v)| v)); + let rooted = super::temp_root::rooted_handle_begin(ctx, &obj, protect_handle); for (name, init) in named_statics { let key_idx = ctx.strings.intern(name); let key_handle_global = format!("@{}", ctx.strings.entry(key_idx).handle_global); let v = lower_expr(ctx, init)?; + let obj = super::temp_root::rooted_handle_get(ctx, &rooted); let blk = ctx.block(); let key_box = blk.load(DOUBLE, &key_handle_global); let key_bits = blk.bitcast_double_to_i64(&key_box); @@ -472,6 +489,9 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { let caps_box = nanbox_pointer_inline(ctx.block(), &caps_arr); let key_idx = ctx.strings.intern("__perry_ctor_caps"); let key_handle_global = format!("@{}", ctx.strings.entry(key_idx).handle_global); + // #7154: re-read the class object — the capture lowerings above + // are arbitrary expressions and may have moved it. + let obj = super::temp_root::rooted_handle_get(ctx, &rooted); let blk = ctx.block(); let key_box = blk.load(DOUBLE, &key_handle_global); let key_bits = blk.bitcast_double_to_i64(&key_box); @@ -481,10 +501,13 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { &[(I64, &obj), (I64, &key_raw), (DOUBLE, &caps_box)], ); } - let obj_box = nanbox_pointer_inline(ctx.block(), &obj); for (key, init) in symbol_statics { let k = lower_expr(ctx, key)?; let v = lower_expr(ctx, init)?; + // #7154: both lowerings above can collect; re-derive the + // receiver from the root rather than reusing the register. + let obj = super::temp_root::rooted_handle_get(ctx, &rooted); + let obj_box = nanbox_pointer_inline(ctx.block(), &obj); ctx.block().call( DOUBLE, "js_object_set_symbol_property", @@ -523,10 +546,17 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { }) .unwrap_or_default(); for fn_name in block_fns { + // #7154: a static block runs arbitrary user code, so re-derive + // the receiver from the root before each one. + let obj = super::temp_root::rooted_handle_get(ctx, &rooted); + let obj_box = nanbox_pointer_inline(ctx.block(), &obj); ctx.block() .call_void("js_static_this_arm_value", &[(DOUBLE, &obj_box)]); ctx.block().call(DOUBLE, &fn_name, &[]); } + let obj = super::temp_root::rooted_handle_get(ctx, &rooted); + let obj_box = nanbox_pointer_inline(ctx.block(), &obj); + super::temp_root::rooted_handle_release(ctx, rooted); Ok(obj_box) } // Issue #711 part 2: `.prototype = ` pattern. diff --git a/crates/perry-codegen/src/expr/temp_root.rs b/crates/perry-codegen/src/expr/temp_root.rs index d41fabf63f..a907d91953 100644 --- a/crates/perry-codegen/src/expr/temp_root.rs +++ b/crates/perry-codegen/src/expr/temp_root.rs @@ -551,6 +551,71 @@ pub(crate) fn temp_root_release(ctx: &mut FnCtx<'_>, guard: Option) { } } +/// The receiver of a property/element STORE, kept valid across the evaluation +/// of the value being stored (#7154). +/// +/// `o.k = f()` and `o[k] = f()` evaluate the reference first and the value +/// second — spec order, and codegen follows it. That leaves the receiver in an +/// SSA register while `f()` runs, and `f()` allocates. A back-edge poll inside +/// it drives an evacuating minor which relocates the receiver; the *slot* the +/// register was loaded from is a root and gets rewritten, but the register does +/// not, so the store lands in abandoned from-space memory and the field never +/// appears on the object the program keeps. +/// +/// This is the store-side instance of the [module invariant](self): property +/// (2) — a rewritten location — is worthless without property (3), reading that +/// location again below the collection point. It is #7114 with a receiver +/// instead of a string literal. +/// +/// A temp root (not a re-load) is the required strategy: re-lowering `object` +/// would observe an assignment made by `f()` itself, which is a miscompile +/// rather than a rooting fix — see [`operand_is_reloadable`]. +pub(crate) struct ReceiverGuard { + slot: Option, +} + +/// Root `recv` (the lowered `object`) if evaluating `value` can collect. +/// Emits nothing otherwise, so stores with an inert RHS keep their old IR. +pub(crate) fn guard_store_receiver( + ctx: &mut FnCtx<'_>, + object: &Expr, + recv: &str, + value: &Expr, +) -> ReceiverGuard { + let collects = expr_may_trigger_gc(ctx, value); + let slot = match operand_protection(ctx, object, collects) { + OperandProtection::Root => Some(temp_root_push_double(ctx, recv)), + // `Reload`/`Reuse` both mean the register survives: a string literal + // cannot be a store receiver, and a proven non-pointer is not movable. + OperandProtection::Reload | OperandProtection::Reuse => None, + }; + ReceiverGuard { slot } +} + +/// Re-read the receiver below the value's evaluation. Returns `recv` unchanged +/// when nothing was rooted. +pub(crate) fn reread_store_receiver( + ctx: &mut FnCtx<'_>, + guard: &ReceiverGuard, + recv: &str, +) -> String { + match &guard.slot { + Some(idx) => { + let idx = idx.clone(); + temp_root_get_double(ctx, &idx) + } + None => recv.to_string(), + } +} + +/// Drop the guard. Call it *after* the store, not before: the store helper +/// allocates (key interning, field-array growth, shape transition). +pub(crate) fn release_store_receiver(ctx: &mut FnCtx<'_>, guard: ReceiverGuard) { + if let Some(idx) = guard.slot { + temp_root_truncate(ctx, &idx); + } +} + /// A freshly allocated container handle (object, array, …) that generated code /// keeps writing into while it lowers the initializer expressions. /// @@ -727,10 +792,19 @@ pub(crate) fn operand_protection( /// at each, which is exactly the bookkeeping that gets missed. /// /// A null word decodes to nothing, so the marker itself roots no object. -/// Emits nothing when no operand could ever need rooting. -pub(crate) fn temp_root_scope_begin(ctx: &mut FnCtx<'_>, args: &[Expr]) -> Option { - args.iter() - .any(|a| operand_needs_root(ctx, a)) +/// Emits nothing when nothing inside the scope could ever need rooting. +/// +/// #7154: `also_needed` is the caller's extra reason to open the scope beyond +/// its operands. `lower_new_impl_inner` roots the freshly-allocated *instance* +/// across the constructor body, and `new C()` with no arguments is precisely +/// the shape that would otherwise push a slot with no marker above it to cut — +/// a temp-root entry leaked per construction. +pub(crate) fn temp_root_scope_begin( + ctx: &mut FnCtx<'_>, + args: &[Expr], + also_needed: bool, +) -> Option { + (also_needed || args.iter().any(|a| operand_needs_root(ctx, a))) .then(|| temp_root_push_i64(ctx, "0")) } diff --git a/crates/perry-codegen/src/lower_call/new.rs b/crates/perry-codegen/src/lower_call/new.rs index ee5d671ef1..da0d4f7eba 100644 --- a/crates/perry-codegen/src/lower_call/new.rs +++ b/crates/perry-codegen/src/lower_call/new.rs @@ -24,6 +24,52 @@ use crate::expr::{lower_expr, lower_js_args_array, nanbox_pointer_inline, temp_r use crate::nanbox::{double_literal, POINTER_MASK_I64}; use crate::types::{DOUBLE, I32, I64, I8, PTR}; +/// Does `new (…)` run user code — an own or inherited constructor +/// body, or field initializers — between the instance allocation and the value +/// the `new` expression yields? +/// +/// That is the window #7154 is about: user code allocates, a back-edge poll +/// inside it drives an evacuating minor, and the instance moves while the +/// caller holds it only in an SSA register. A class with none of these has no +/// window at all (`js_gc_init_typed_shape_layout` is the only thing emitted in +/// between, and it does not allocate), so it keeps its pre-#7154 IR exactly. +/// +/// An unresolvable class name is `false` on purpose, not conservatively `true`: +/// the instance root is pushed only on paths that resolved the class out of +/// `ctx.classes`, so a name this returns `false` for never reaches the push and +/// would leave the scope marker as pure overhead. +fn construction_runs_user_code(ctx: &FnCtx<'_>, class_name: &str) -> bool { + ctx.classes.get(class_name).is_some_and(|class| { + class.constructor.is_some() + || !class.fields.is_empty() + || class.extends.is_some() + || class.extends_name.is_some() + || class.native_extends.is_some() + || class.extends_expr.is_some() + }) +} + +/// Re-read the freshly-constructed instance from the temp-root slot that +/// carried it across the constructor body (#7154). +/// +/// Returns `(obj_handle, obj_box)` — the bare handle and its NaN-boxed form. +/// When no root was pushed (nothing between the allocation and here can +/// collect) the original registers are handed straight back, so those sites +/// keep their old IR byte for byte. +fn reload_instance( + ctx: &mut FnCtx<'_>, + instance_root: &Option, + obj_handle: &str, + obj_box: &str, +) -> (String, String) { + let Some(idx) = instance_root.clone() else { + return (obj_handle.to_string(), obj_box.to_string()); + }; + let handle = temp_root::temp_root_get_i64(ctx, &idx); + let boxed = nanbox_pointer_inline(ctx.block(), &handle); + (handle, boxed) +} + /// Emit the `js_gc_init_typed_shape_layout` call that registers the freshly /// constructed instance's raw-f64 / pointer slot masks with the GC so the /// typed-feedback class-field fast path engages. Must run AFTER the constructor @@ -183,7 +229,14 @@ fn lower_new_impl( // here releases the group whichever path ran, instead of a // `temp_root_release` at each that reviewers and future edits must keep // balanced. - let scope = temp_root::temp_root_scope_begin(ctx, args); + // + // #7154: the body also roots the freshly-allocated instance across the + // constructor body, so the marker is required whenever construction runs + // user code — not only when an argument needs a root. `new C()` with no + // arguments is exactly the shape that would otherwise push a slot with no + // marker above it to cut. + let scope = + temp_root::temp_root_scope_begin(ctx, args, construction_runs_user_code(ctx, class_name)); let result = lower_new_impl_inner(ctx, class_name, args, caps_absent_from_args); temp_root::temp_root_scope_end(ctx, scope); result @@ -842,6 +895,29 @@ fn lower_new_impl_inner( ], ) }; + // #7154: root the instance for the duration of the constructor body. + // + // Until now the instance existed ONLY as an SSA register while that body + // ran, and a constructor body allocates. Under back-edge polls + // (`PERRY_GC_MOVING_LOOP_POLLS=1`) an evacuating minor inside the + // constructor RELOCATES it: the callee's own `this` shadow slot roots it, + // so it survives and moves, and the collector rewrites the callee's root — + // but not the caller's register, which is not a root at all. Every + // subsequent use in this function then names from-space memory, and + // `js_ctor_return_override` publishes that dead address straight into the + // caller's shadow slot. The result is a *rooted* slot holding a dangling + // pointer, which is why #7154's from-space scan only ever saw offenders + // one or more cycles after the target died, with correct layout coverage. + // + // This is #7184's sibling: there the root store landed outside the pushed + // frame, here it lands after a collection point. Same invariant — the root + // store must dominate every site that can collect — and the same symptom + // ("value is not a function" on a stale closure/instance field). + // + // The slot is released by the scope cut in `lower_new_impl`, which covers + // all ~20 return paths below. + let instance_root = construction_runs_user_code(ctx, class_name) + .then(|| temp_root::temp_root_push_i64(ctx, &obj_handle)); let obj_box = nanbox_pointer_inline(ctx.block(), &obj_handle); // #6969: the instance allocation has run, so refresh every argument before // the constructor consumes them. @@ -940,6 +1016,13 @@ fn lower_new_impl_inner( ctx.block() .call(DOUBLE, "js_new_target_set", &[(DOUBLE, prev)]); } + // #7154: the constructor body has run, so every register holding + // the instance is potentially pre-move. Re-read it from its root + // before anything else touches it — `emit_typed_shape_layout_init` + // would otherwise install the layout descriptor on the abandoned + // from-space copy, and `js_ctor_return_override` would hand the + // caller that copy's address. + let (obj_handle, obj_box) = reload_instance(ctx, &instance_root, &obj_handle, &obj_box); // The constructor body has run and set the declared fields; register // the typed raw-f64/pointer slot layout so class-field accesses hit // the slot-direct fast path instead of the by-name hashmap fallback. @@ -1780,6 +1863,11 @@ fn lower_new_impl_inner( apply_field_initializers_recursive(ctx, class_name, FieldInitMode::AfterRoot)?; } } + // #7154: same re-read as the standalone-symbol path above. The inlined + // constructor body (field initializers, `super(...)`, nested `new`s) can + // reach a back-edge poll, and the evacuating minor there relocates the + // instance out from under `obj_handle`/`obj_box`. + let (obj_handle, obj_box) = reload_instance(ctx, &instance_root, &obj_handle, &obj_box); emit_typed_shape_layout_init(ctx, class_name, &obj_handle); // Close the inline-constructor return: fall through (or branch) to the diff --git a/crates/perry-codegen/tests/temp_root_operand_temporaries.rs b/crates/perry-codegen/tests/temp_root_operand_temporaries.rs index 41310a2e9a..de25117738 100644 --- a/crates/perry-codegen/tests/temp_root_operand_temporaries.rs +++ b/crates/perry-codegen/tests/temp_root_operand_temporaries.rs @@ -729,3 +729,129 @@ fn wtf8_literal_operand_is_rooted_not_merely_reused() { in a register, which is #7114 for lone-surrogate literals:\n{f}" ); } + +// ---------------------------------------------------------------- #7154 ---- + +/// `Pair` with an instance field, so construction runs user code (the field +/// initializer) between the allocation and the value `new` yields. +fn module_with_new_running_ctor(name: &str) -> Module { + let mut module = module_with_new(name, Vec::new()); + module.classes[0].fields = vec![perry_hir::ClassField { + name: "v".to_string(), + key_expr: None, + ty: perry_hir::types::Type::Any, + // An object literal: a real collection point inside the constructor. + init: Some(Expr::Object(Vec::new())), + is_private: false, + is_readonly: false, + decorators: Vec::new(), + }]; + module +} + +/// #7154, the sibling of #7184: the freshly-allocated instance must be ROOTED +/// across the constructor body and RE-READ afterwards. +/// +/// The constructor body allocates, and under `PERRY_GC_MOVING_LOOP_POLLS=1` a +/// back-edge poll inside it drives an evacuating minor. The instance survives — +/// the callee's own `this` shadow slot roots it — which means it *moves*, and +/// the collector rewrites the callee's root but not the caller's SSA register. +/// Everything downstream in the caller (`js_gc_init_typed_shape_layout`, the +/// capture write-back, `js_ctor_return_override`) then names from-space memory, +/// and the override publishes that dead address into the caller's shadow slot: +/// a *rooted* slot holding a dangling pointer, read back later as +/// "TypeError: value is not a function". +/// +/// Sabotage check: drop the `reload_instance` call in `lower_new_impl_inner` +/// and the re-read disappears from between the allocation and the override. +#[test] +fn the_new_instance_is_rooted_across_the_constructor_body() { + let ir = String::from_utf8( + compile_module( + &module_with_new_running_ctor("new_inst_rooted.ts"), + entry_opts(), + ) + .unwrap(), + ) + .expect("LLVM IR should be UTF-8"); + let f = init_ir(&ir); + + // Assertions are on the DEF-USE chain, not on textual order: the override + // is emitted into the `ctor.return.after` block, which the writer appends + // below the block that re-reads the root. + let def_of = |reg: &str| -> String { + let needle = format!(" %{reg} = "); + f.lines() + .find(|l| l.starts_with(&needle)) + .unwrap_or_else(|| panic!("no definition of %{reg} in:\n{f}")) + .to_string() + }; + let first_operand_reg = |line: &str| -> String { + line.split("%") + .nth(1) + .and_then(|s| { + s.split(|c: char| !(c.is_alphanumeric() || c == '_' || c == '.')) + .next() + }) + .unwrap_or_else(|| panic!("no register operand in `{line}`")) + .to_string() + }; + + // 1. The allocation's result is pushed as a temp root immediately. + let alloc_line = f + .lines() + .find(|l| l.contains("call i64 @js_object_alloc_class")) + .unwrap_or_else(|| panic!("the instance allocation:\n{f}")); + let inst_reg = alloc_line + .trim_start() + .trim_start_matches('%') + .split(' ') + .next() + .unwrap() + .to_string(); + assert!( + f.contains(&format!("call i32 @js_gc_temp_root_push(i64 %{inst_reg})")), + "the instance %{inst_reg} must be rooted as soon as it is allocated, \ + before the constructor body runs (#7154):\n{f}" + ); + + // 2. The value `js_ctor_return_override` publishes is re-derived from that + // root, not carried across the constructor in the original register. + let override_line = f + .lines() + .find(|l| l.contains("call double @js_ctor_return_override")) + .unwrap_or_else(|| panic!("the return-override:\n{f}")); + let mut reg = first_operand_reg( + override_line + .split_once("js_ctor_return_override") + .expect("split") + .1, + ); + let mut chain = vec![reg.clone()]; + for _ in 0..8 { + let d = def_of(®); + if d.contains("@js_gc_temp_root_get") { + return; + } + reg = first_operand_reg(d.split_once(" = ").expect("assignment").1); + chain.push(reg.clone()); + } + panic!( + "the instance handed to js_ctor_return_override must be re-read from \ + its temp root after the constructor body — walked {chain:?} without \ + reaching a js_gc_temp_root_get (#7154):\n{f}" + ); +} + +/// The gate: a class with no constructor, no fields and no heritage runs no +/// user code between the allocation and the `new` value, so it must keep its +/// pre-#7154 IR — no instance root, and no scope marker either. +#[test] +fn a_class_that_runs_no_user_code_emits_no_instance_root() { + let ir = ir_for_new("new_inst_no_ctor.ts", vec![Expr::Number(1.0)]); + assert!( + !ir.contains("call i32 @js_gc_temp_root_push"), + "nothing can collect between the allocation and the `new` value, so \ + rooting the instance would be pure TLS traffic:\n{ir}" + ); +} diff --git a/scripts/gc_root_dominance_check.py b/scripts/gc_root_dominance_check.py new file mode 100755 index 0000000000..b5266bcec4 --- /dev/null +++ b/scripts/gc_root_dominance_check.py @@ -0,0 +1,692 @@ +#!/usr/bin/env python3 +"""Static GC root-dominance checker for perry-emitted LLVM IR. + +Invariant checked +----------------- +For any GC-managed value materialized in a function, the shadow-slot root +store that makes it visible to the precise-root collector must DOMINATE (in +the CFG sense: be on every path before) any subsequent site that can trigger +a collection. + +A violation is a triple (alloc A, activating bind B, collecting call C) where +there exists a CFG path A -> ... -> C -> ... -> B, i.e. the value is live in +an SSA register / an unrooted alloca while a collection can run. + +Why this exists (#7154) +----------------------- +Two bugs of this exact class have shipped. #7184: the root store was emitted +but its slot index fell outside the pushed shadow frame, so +`js_shadow_slot_bind` bounds-checked it and silently no-opped. #7186: the root +store was emitted in-frame but *after* a call that allocates, so a back-edge +poll inside that call ran an evacuating minor while the value was live only in +an SSA register. Both present identically — a rooted slot holding a dangling +pointer, surfacing cycles later as "TypeError: value is not a function" — and +neither is visible to any runtime GC probe, because at the moment of the +collection there is nothing for the collector to find. + +This checker is the instrument that finds them statically, over a whole corpus, +before they reach a crash. + +Usage +----- + # dump the IR (writes .perry-trace/llvm/*.ll) + PERRY_GC_MOVING_LOOP_POLLS=1 PERRY_INLINE_SHADOW_SLOT=0 \ + perry compile app.ts -o /tmp/app --trace llvm + + python3 scripts/gc_root_dominance_check.py .perry-trace/llvm [-v] + python3 scripts/gc_root_dominance_check.py .perry-trace/llvm --any-def + python3 scripts/gc_root_dominance_check.py .perry-trace/llvm --moving-only + +`PERRY_INLINE_SHADOW_SLOT=0` makes every root store the `@js_shadow_slot_bind` +call form; the inline #7088 diamond is equivalent but harder to anchor on. +`PERRY_GC_MOVING_LOOP_POLLS=1` is what puts `js_gc_loop_safepoint` in the IR, +which is what the `MOVING` classification keys on. + +Modes +----- +`--any-def` (default off) anchors on ANY call whose result reaches the root +store, not just the known allocation intrinsics — broader, noisier. +`--moving-only` keeps only violations whose window contains a call that can +transitively reach a moving-minor safepoint. + +Soundness +--------- +One-sided by design. `NONCOLLECTING` is the only place a call is declared safe, +and every entry names the runtime source line that proves it; anything +unrecognised counts as a collection point. A missing entry costs a false +positive, never a missed bug. Dominance is real CFG dominance (Cooper/Harvey/ +Kennedy) and the window is path-based, so a loop back edge is not mistaken for +an intra-iteration path — a naive line-order scan produced 8 false positives +where this reports none. + +Exit code is 1 when any violation is reported, so it can gate. +""" +import os +import re +import sys +from collections import defaultdict, deque + +# ---------------------------------------------------------------- IR parsing + +DEFINE_RE = re.compile(r"^define\s+.*?@([\w.$]+)\(") +LABEL_RE = re.compile(r"^([\w.$][\w.$]*):\s*$") +ASSIGN_RE = re.compile(r"^\s*%([\w.$]+)\s*=\s*(.*)$") +CALL_RE = re.compile(r"\bcall\s+[^@]*@([\w.$]+)\(") +BIND_RE = re.compile(r"call void @js_shadow_slot_bind\(i32 (\d+), ptr %([\w.$]+)\)") +CLEAR_RE = re.compile(r"call void @js_shadow_slot_set\(i32 (\d+), i64 0\)") +STORE_RE = re.compile(r"^\s*store\s+([\w\[\]x* ]+?)\s+([^,]+),\s*ptr %([\w.$]+)") +BR_UNCOND_RE = re.compile(r"^\s*br label %([\w.$]+)") +BR_COND_RE = re.compile(r"^\s*br i1 [^,]+, label %([\w.$]+), label %([\w.$]+)") +SWITCH_LABEL_RE = re.compile(r"label %([\w.$]+)") + + +class Insn: + __slots__ = ("text", "block", "idx", "result", "callee") + + def __init__(self, text, block, idx): + self.text = text + self.block = block + self.idx = idx + m = ASSIGN_RE.match(text) + self.result = m.group(1) if m else None + c = CALL_RE.search(text) + self.callee = c.group(1) if c else None + + +class Func: + def __init__(self, name): + self.name = name + self.blocks = [] # ordered block labels + self.insns = defaultdict(list) # label -> [Insn] + self.succs = defaultdict(set) + self.preds = defaultdict(set) + + +def parse_file(path): + funcs = [] + cur = None + curblk = None + with open(path, "r", errors="replace") as fh: + for raw in fh: + line = raw.rstrip("\n") + m = DEFINE_RE.match(line) + if m: + cur = Func(m.group(1)) + funcs.append(cur) + curblk = None + continue + if cur is None: + continue + if line.startswith("}"): + cur = None + curblk = None + continue + lm = LABEL_RE.match(line) + if lm: + curblk = lm.group(1) + if curblk not in cur.insns: + cur.blocks.append(curblk) + cur.insns[curblk] = [] + continue + if curblk is None: + continue + if not line.strip(): + continue + cur.insns[curblk].append(Insn(line, curblk, len(cur.insns[curblk]))) + for f in funcs: + build_cfg(f) + return funcs + + +def build_cfg(f): + for b in f.blocks: + for ins in f.insns[b]: + t = ins.text + m = BR_COND_RE.match(t) + if m: + f.succs[b].add(m.group(1)) + f.succs[b].add(m.group(2)) + continue + m = BR_UNCOND_RE.match(t) + if m: + f.succs[b].add(m.group(1)) + continue + if t.strip().startswith("switch"): + for lbl in SWITCH_LABEL_RE.findall(t): + f.succs[b].add(lbl) + for b, ss in list(f.succs.items()): + for s in ss: + f.preds[s].add(b) + + +# ------------------------------------------------------- collection-site model + +# Runtime helpers that provably cannot allocate, run user code, or poll. +NONCOLLECTING = { + # shadow stack / roots + "js_shadow_slot_bind", "js_shadow_slot_set", "js_shadow_frame_enter", + "js_shadow_frame_push", "js_shadow_frame_pop", "js_shadow_state_addr", + "js_gc_temp_root_push", "js_gc_temp_root_get", "js_gc_temp_root_set", + "js_gc_temp_root_truncate", + # layout / barrier bookkeeping (no allocation) + "js_gc_init_typed_shape_layout", "js_gc_layout_note_slot", + "js_write_barrier_root_nanbox", "js_write_barrier_slot", + "js_runtime_write_barrier_slot", "js_gc_register_global_root", + # pure value predicates / bit twiddling + "js_is_truthy", "js_nanbox_get_pointer", "js_value_is_object", + "js_value_is_string", "js_typeof_tag", + # inline-cache guards: pure reads + "js_typed_feedback_closure_direct_call_guard", + "js_typed_feedback_shape_guard", "js_typed_feedback_note", + # ctor identity selection + "js_ctor_return_override", + "llvm.lifetime.start.p0", "llvm.lifetime.end.p0", + # verified non-allocating bookkeeping stores/reads (perry-runtime) + "js_closure_set_capture_bits", # closure/alloc.rs:477 raw slot write + layout note + "js_closure_get_capture_bits", # closure/alloc.rs:463 raw slot read + "js_closure_set_capture_ptr", "js_closure_get_capture_ptr", + "js_box_set_bits", "js_box_get_bits", # box.rs:317 raw cell write + "js_i32_box_set", "js_bool_box_set", + "js_write_barrier", # gc/barrier.rs:930 + "js_tdz_suppress_begin", "js_tdz_suppress_end", # box.rs:242/248 counter + "js_array_note_numeric_write", # array/header.rs:1443 + "js_array_length", # array/indexing.rs:537 + "js_object_mark_class", "js_class_object_pin_parent", + "js_new_target_get", "js_new_target_set", + # object/this_binding.rs:160 -- a thread-local cell swap + "js_implicit_this_set", "js_implicit_this_get", + "js_gc_note_slot_layout", "js_string_addref_if_heap_string", +} + +# The single site where an evacuating (moving) minor runs. +MOVING_POLL = "js_gc_loop_safepoint" + +# Result-producing calls that materialize a fresh GC object. +ALLOC_RE = re.compile( + r"^js_(" + r"object_alloc\w*|array_alloc\w*|closure_alloc\w*|box_alloc\w*|" + r"string_alloc\w*|string_concat\w*|string_coerce|string_from\w*|" + r"map_alloc\w*|set_alloc\w*|promise_alloc\w*|bigint_alloc\w*|" + r"typed_array_alloc\w*|buffer_alloc\w*|regexp_alloc\w*|" + r"object_create\w*|array_from\w*|build_class_keys_array" + r")$" +) + +# Bit-level / identity producers a heap address flows through unchanged. +TRANSPARENT_OPS = ("or i64", "and i64", "bitcast", "inttoptr", "ptrtoint", + "select", "phi", "add i64", "sub i64") +TRANSPARENT_CALLS = {"js_ctor_return_override"} +# Calls that ROOT their argument (protecting it from that point on). +# `js_box_set_bits` publishes into a mutable-capture box, which `BOX_REGISTRY` +# / `scan_box_roots_mut` marks AND rewrites (gc/mod.rs:547). +ROOTING_CALLS = {"js_gc_temp_root_push", "js_gc_temp_root_set", + "js_box_set_bits", "js_i32_box_set", "js_bool_box_set"} + + +def operand_regs(text): + """SSA registers referenced on the right-hand side of an instruction.""" + body = text.split(" = ", 1)[-1] if " = " in text else text + return set(re.findall(r"%([\w.$]+)", body)) + + +def is_transparent(ins): + if ins.callee in TRANSPARENT_CALLS: + return True + if ins.callee is not None: + return False + return any(op in ins.text for op in TRANSPARENT_OPS) + + +def provenance(def_of, reg, limit=64): + """Walk back from `reg` through bit-level/identity producers to the + instructions that actually MATERIALIZE the value (calls and loads).""" + origins = [] + seen = set() + q = deque([reg]) + while q and len(seen) < limit: + r = q.popleft() + if r in seen: + continue + seen.add(r) + d = def_of.get(r) + if d is None: + continue + if is_transparent(d): + q.extend(operand_regs(d.text)) + continue + if d.callee is not None or " load " in d.text or d.text.strip().startswith("%") and "= load" in d.text: + origins.append(d) + return origins + + +def uses(text, regs): + """Does `text` reference any of `regs` as a whole SSA operand? + (`%r1` must NOT match `%r16` -- a substring test silently taints the + entire rest of the function.)""" + for r in regs: + if re.search(r"%" + re.escape(r) + r"(?![\w.$])", text): + return True + return False + + +def is_collecting(callee): + if callee is None: + return False + if callee in NONCOLLECTING: + return False + if callee.startswith("llvm."): + return False + return True + + +# ------------------------------------------------- interprocedural poll reach + +# Runtime helpers that re-enter compiled JS (and therefore its back-edge polls). +POLL_CAPABLE_RUNTIME = { + "js_call_function", "js_call_closure", "js_invoke_closure", + "js_call_value", "js_apply_function", "js_function_call", + "js_object_get_property", "js_object_set_property", + "js_object_get_field_by_name", "js_object_set_field_by_name", + "js_array_sort", "js_array_map", "js_array_filter", "js_array_for_each", + "js_array_reduce", "js_json_stringify", "js_string_replace", + "js_promise_run_microtasks", "js_gc_loop_safepoint", +} + + +def compute_poll_reaching(all_funcs): + """Names of compiled functions that can transitively reach a moving-minor + safepoint (`js_gc_loop_safepoint`) or a runtime helper that re-enters JS.""" + callees = {} + for f in all_funcs: + cs = set() + for b in f.blocks: + for ins in f.insns[b]: + if ins.callee: + cs.add(ins.callee) + callees[f.name] = cs + polls = set() + for name, cs in callees.items(): + if cs & POLL_CAPABLE_RUNTIME: + polls.add(name) + changed = True + while changed: + changed = False + for name, cs in callees.items(): + if name in polls: + continue + if cs & polls: + polls.add(name) + changed = True + return polls, set(callees) + + +# ------------------------------------------------------- dominance & windows + +def dominators(f): + """Cooper/Harvey/Kennedy iterative dominators. Returns idom map.""" + if not f.blocks: + return {} + entry = f.blocks[0] + # reverse postorder + order = [] + seen = set() + + def dfs(b): + stack = [(b, iter(sorted(f.succs[b])))] + seen.add(b) + while stack: + node, it = stack[-1] + advanced = False + for s in it: + if s in f.insns and s not in seen: + seen.add(s) + stack.append((s, iter(sorted(f.succs[s])))) + advanced = True + break + if not advanced: + order.append(stack.pop()[0]) + + dfs(entry) + rpo = list(reversed(order)) + pos = {b: i for i, b in enumerate(rpo)} + idom = {entry: entry} + + def intersect(a, b): + while a != b: + while pos[a] > pos[b]: + a = idom[a] + while pos[b] > pos[a]: + b = idom[b] + return a + + changed = True + while changed: + changed = False + for b in rpo: + if b == entry: + continue + new = None + for p in f.preds[b]: + if p not in pos or p not in idom: + continue + new = p if new is None else intersect(p, new) + if new is not None and idom.get(b) != new: + idom[b] = new + changed = True + return idom + + +def dominates(idom, a, b): + if a == b: + return True + cur = b + while cur in idom and idom[cur] != cur: + cur = idom[cur] + if cur == a: + return True + return False + + +def between_blocks(f, a_blk, b_blk): + """Blocks strictly between a_blk and b_blk on some path that does NOT + re-enter a_blk (so a loop back-edge round trip is not counted -- that is a + different dynamic instance of the value).""" + if a_blk == b_blk: + return set() + fwd = set() + q = deque(s for s in f.succs[a_blk] if s in f.insns and s != a_blk) + while q: + x = q.popleft() + if x in fwd: + continue + fwd.add(x) + if x == b_blk: + continue # sink: do not expand past the bind + for s in f.succs[x]: + if s in f.insns and s != a_blk: + q.append(s) + bwd = set() + q = deque(p for p in f.preds[b_blk] if p in f.insns and p != a_blk) + while q: + x = q.popleft() + if x in bwd: + continue + bwd.add(x) + if x == b_blk: + continue + for p in f.preds[x]: + if p in f.insns and p != a_blk: + q.append(p) + return (fwd & bwd) - {a_blk, b_blk} + + +# ---------------------------------------------------------- slot activity (must) + +def must_active_slots(f): + """Forward must-dataflow: which shadow slots are provably ACTIVE on entry + to each block. gen = js_shadow_slot_bind(N,..), kill = js_shadow_slot_set(N,0).""" + all_slots = set() + for b in f.blocks: + for ins in f.insns[b]: + m = BIND_RE.search(ins.text) + if m: + all_slots.add(int(m.group(1))) + if not all_slots: + return {} + entry = f.blocks[0] if f.blocks else None + IN = {b: set(all_slots) for b in f.blocks} + IN[entry] = set() + changed = True + while changed: + changed = False + for b in f.blocks: + if b == entry: + new = set() + else: + ps = [p for p in f.preds[b] if p in f.insns] + if not ps: + new = set() + else: + new = set(all_slots) + for p in ps: + new &= transfer(f, p, IN[p]) + if new != IN[b]: + IN[b] = new + changed = True + return IN + + +def transfer(f, b, incoming): + cur = set(incoming) + for ins in f.insns[b]: + m = BIND_RE.search(ins.text) + if m: + cur.add(int(m.group(1))) + continue + m = CLEAR_RE.search(ins.text) + if m: + cur.discard(int(m.group(1))) + return cur + + +def active_at(f, IN, block, idx): + cur = set(IN.get(block, set())) + for ins in f.insns[block][:idx]: + m = BIND_RE.search(ins.text) + if m: + cur.add(int(m.group(1))) + continue + m = CLEAR_RE.search(ins.text) + if m: + cur.discard(int(m.group(1))) + return cur + + +# -------------------------------------------------------------- the check + +class Violation: + def __init__(self, module, func, alloc, store, bind, collectors, slot, + poll_reaching=frozenset()): + self.module = module + self.func = func + self.alloc = alloc + self.store = store + self.bind = bind + self.collectors = collectors + self.slot = slot + self.poll_reaching = poll_reaching + + @property + def movers(self): + return sorted({c.callee for c in self.collectors + if c.callee == MOVING_POLL or c.callee in self.poll_reaching + or c.callee in POLL_CAPABLE_RUNTIME}) + + @property + def moving(self): + return bool(self.movers) + + +def check_func(module, f, want_moving_only=False, poll_reaching=frozenset(), + anchor_mode="alloc"): + if not f.blocks: + return [] + order = {b: i for i, b in enumerate(f.blocks)} + IN = must_active_slots(f) + # map: alloca reg -> slot idx (from binds) + slot_of_alloca = {} + binds = [] # (Insn, slot, alloca) + for b in f.blocks: + for ins in f.insns[b]: + m = BIND_RE.search(ins.text) + if m: + slot, alloca = int(m.group(1)), m.group(2) + slot_of_alloca[alloca] = slot + binds.append((ins, slot, alloca)) + + if not binds: + return [] + + # index instructions by result register + def_of = {} + for b in f.blocks: + for ins in f.insns[b]: + if ins.result: + def_of[ins.result] = ins + + idom = dominators(f) + violations = [] + + def window_hits(A, B): + """Collecting calls on some CFG path from just after A to B.""" + hits = [] + if A.block == B.block: + for c in f.insns[A.block]: + if is_collecting(c.callee) and A.idx < c.idx < B.idx: + hits.append(c) + return hits + for c in f.insns[A.block]: + if is_collecting(c.callee) and c.idx > A.idx: + hits.append(c) + for c in f.insns[B.block]: + if is_collecting(c.callee) and c.idx < B.idx: + hits.append(c) + for m_blk in between_blocks(f, A.block, B.block): + for c in f.insns[m_blk]: + if is_collecting(c.callee): + hits.append(c) + return hits + + def protected(A, B, chain): + """Is the value rooted some other way inside the window? A temp-root + push or a mutable-capture box store of any register in the value's + provenance chain roots it (both are scanned AND rewritten).""" + def scan(blk, lo, hi): + for c in f.insns[blk][lo:hi]: + if c.callee in ROOTING_CALLS and uses(c.text, chain): + return True + return False + if A.block == B.block: + return scan(A.block, A.idx + 1, B.idx) + if scan(A.block, A.idx + 1, len(f.insns[A.block])): + return True + if scan(B.block, 0, B.idx): + return True + for m_blk in between_blocks(f, A.block, B.block): + if scan(m_blk, 0, len(f.insns[m_blk])): + return True + return False + + for bind_ins, slot, alloca in binds: + # The store this bind activates: nearest preceding store to `alloca`. + store_ins = None + for j in range(bind_ins.idx - 1, -1, -1): + c = f.insns[bind_ins.block][j] + sm = STORE_RE.match(c.text) + if sm and sm.group(3) == alloca: + store_ins = c + break + if store_ins is None: + continue + # Already-active slot bound to this alloca: the store itself publishes + # the value through `bound_ptr`, so no window exists. + if slot in active_at(f, IN, store_ins.block, store_ins.idx): + continue + val = STORE_RE.match(store_ins.text).group(2).strip() + if not val.startswith("%"): + continue + reg = val[1:] + chain = set() + q = deque([reg]) + while q: + r = q.popleft() + if r in chain: + continue + chain.add(r) + d = def_of.get(r) + if d is not None and is_transparent(d): + q.extend(operand_regs(d.text)) + for origin in provenance(def_of, reg): + if anchor_mode == "alloc": + if origin.callee is None or not ALLOC_RE.match(origin.callee): + continue + else: + # A load of a constant/global handle is not a materialization + # worth anchoring in "any" mode either; keep calls only. + if origin.callee is None or origin.callee in NONCOLLECTING: + continue + # Real CFG dominance: the value bound must be the value this + # instruction produced on every path reaching the bind. + if not dominates(idom, origin.block, bind_ins.block): + continue + if origin.block == bind_ins.block and origin.idx >= bind_ins.idx: + continue + hits = window_hits(origin, bind_ins) + if not hits: + continue + if protected(origin, bind_ins, chain): + continue + v = Violation(module, f.name, origin, store_ins, bind_ins, hits, + slot, poll_reaching) + if want_moving_only and not v.moving: + continue + violations.append(v) + break # one report per bind: the widest window + return violations + + +def main(): + args = [a for a in sys.argv[1:] if not a.startswith("-")] + moving_only = "--moving-only" in sys.argv + anchor = "any" if "--any-def" in sys.argv else "alloc" + verbose = "-v" in sys.argv + paths = [] + for a in args: + if os.path.isdir(a): + for root, _dirs, files in os.walk(a): + for fn in files: + if fn.endswith(".ll"): + paths.append(os.path.join(root, fn)) + else: + paths.append(a) + + parsed = [] + for p in sorted(paths): + parsed.append((os.path.basename(p), parse_file(p))) + poll_reaching, _known = compute_poll_reaching( + [f for _m, fs in parsed for f in fs]) + + total = 0 + moving_total = 0 + per_kind = defaultdict(int) + per_kind_moving = defaultdict(int) + out = [] + for mod, fs in parsed: + for f in fs: + for v in check_func(mod, f, moving_only, poll_reaching, anchor): + total += 1 + per_kind[v.alloc.callee] += 1 + if v.moving: + moving_total += 1 + per_kind_moving[v.alloc.callee] += 1 + cs = sorted({c.callee for c in v.collectors}) + out.append( + f"{mod}::{v.func}\n" + f" alloc : {v.alloc.text.strip()}\n" + f" store : {v.store.text.strip()}\n" + f" bind : slot {v.slot} {v.bind.text.strip()}\n" + f" between: {', '.join(cs[:8])}" + f"{' (+%d more)' % (len(cs) - 8) if len(cs) > 8 else ''}\n" + f" MOVING : {('YES via ' + ', '.join(v.movers[:3])) if v.moving else 'no'}\n" + ) + if verbose: + print("\n".join(out)) + print(f"=== files: {len(paths)} violations: {total}" + f" (moving-minor reachable: {moving_total})") + for k, n in sorted(per_kind.items(), key=lambda kv: -kv[1]): + print(f" {n:6d} ({per_kind_moving.get(k, 0):5d} moving) {k}") + return 1 if total else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/test-files/test_gap_gc_new_instance_rooting.ts b/test-files/test_gap_gc_new_instance_rooting.ts new file mode 100644 index 0000000000..ff63b94fd9 --- /dev/null +++ b/test-files/test_gap_gc_new_instance_rooting.ts @@ -0,0 +1,49 @@ +// #7154: the instance of `new C(...)` must be ROOTED across the constructor +// body, not carried in an SSA register. +// +// `new C(n)` lowers to: allocate the instance, call `_constructor(inst, n)`, +// then `js_gc_init_typed_shape_layout(inst, ...)` and +// `js_ctor_return_override(inst, ret, derived)`. The constructor body allocates, +// and under `PERRY_GC_MOVING_LOOP_POLLS=1` a loop back-edge poll inside it runs +// an evacuating minor. The instance SURVIVES that minor — the callee's own +// `this` parameter has a shadow slot — which means it MOVES, and the collector +// rewrites the callee's root but not the caller's register. Every use after the +// call then names from-space memory, and the return-override publishes that +// dead address into the caller's shadow slot: a rooted slot holding a dangling +// pointer, read back later as garbage or "value is not a function". +// +// This is #7184's sibling. There the root store landed OUTSIDE the pushed frame +// and silently no-opped; here it lands AFTER a collection point. Same +// invariant: a value's root store must dominate every site that can collect. +// +// LIVE BY CONSTRUCTION. The constructor allocates hard enough to reach the +// collector, and the caller READS a field of the instance right after the call +// — a non-moving collection cannot expose this, so the evacuating arms are the +// ones that bite. + +class Node1 { + payload: any; + constructor(n: number) { + const bits: any[] = []; + for (let i = 0; i < 600; 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 < 400; r++) { + const node = new Node1(r); + const p = node.payload; + if (p === null || p === undefined) { + bad++; + } else if ((p.n as number) !== r) { + bad++; + } + } + return bad; +} + +console.log("bad", run());