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());