From bf8bede45e25f42fc27fa36030976a7847a2bdfb Mon Sep 17 00:00:00 2001 From: jdalton Date: Sat, 1 Aug 2026 12:05:08 -0400 Subject: [PATCH 1/3] fix(codegen): root a call receiver and a computed-read base across their sibling operands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two more sites of #7192's root-store-dominance class, found by extending its checker with the general stale-register invariant and each reproduced in ~30 lines of TypeScript. `recv.m(f())` and `o[f()]` evaluate the reference first and the second operand after — spec order, and codegen follows it. That left the reference in a bare SSA register while `f()` was lowered, and `f()` allocates. Under PERRY_GC_MOVING_LOOP_POLLS=1 a loop back-edge poll inside it runs an evacuating minor. The reference SURVIVES that minor — the closure capture cell, shadow slot or module global holding it is a root — so it MOVES: the collector rewrites that location and the register keeps naming from-space. Same "property (2), a rewritten location, is worthless without property (3), reading that location again below the collection point" that expr/temp_root.rs's module header describes, and the same fix #7192 applied to the property/element STORE receiver. * lower_call/console_promise.rs — the js_native_call_method_by_id dispatch. The stale receiver makes the method lookup resolve against abandoned memory, so the call throws "TypeError: value is not a function". In sfw-registry this is zod classic/schemas.ts:301, `inst.regex = (...args) => inst.check(checks.regex(...args))`: `inst` is read out of the arrow's capture cell, held across `checks.regex(...args)` (a real user call, so it polls), then used as `.check`'s receiver. The arguments are rooted too — each before the NEXT one is lowered, per RootedOperands' incremental contract — so an earlier argument cannot go stale across a later one either. * expr/index_get.rs — the dynamic-string-key arm and the last-resort runtime-tag-check arm: the READ counterpart of #7192's index_set / property_set guard, which only covered the store side. The stale base makes the field read walk the keys array of from-space memory — a SIGSEGV inside get_field_by_name_object_tail, or a silently wrong value. In sfw-registry this is zod core/checks.ts:68, `numericOriginMap[typeof def.value]`: a module-global base with a key expression that reads a property and therefore can collect. Both use temp_root::guard_store_operand / reread_store_operand / release_store_operand (#7198's generalized naming). A temp root, not a re-lower: re-lowering the reference would observe an assignment made by the second operand itself, which is a miscompile rather than a rooting fix. The guard emits nothing when the sibling expression cannot collect, so an inert argument list or key keeps its previous IR exactly, and it is released AFTER the dispatch because the dispatcher allocates while reading these values. Verified by two new gap tests, each red on the parent commit and green after, and each clean under a non-moving collector so the failure is proven to track collector mode rather than luck: test_gap_gc_method_receiver_rooting.ts POLLS=1 parent: TypeError 5/5 this: bad 0 10/10 test_gap_gc_index_get_receiver_rooting.ts POLLS=1 parent: TypeError 4/4 this: bad 0 10/10 both, POLLS=1 + PERRY_GEN_GC=0 bad 0 both, default (no polls) bad 0 5/5 scripts/gc_root_dominance_check.py grows --stale-registers. The shipped check anchors on a shadow-slot bind, so it can only see values that are eventually rooted; neither site above is. The new mode classifies every heap-value source (an allocation, or a read of a collector-rewritten location: a shadow-slot load, a closure capture cell, a temp-root slot, a module global, a mutable-capture box), follows it forward through bit-level identity ops, and reports the first real use below a collecting call. --fatal-sinks narrows to uses that DEREFERENCE the value (a call receiver or callee), where a relocation is fatal rather than merely wrong. Over the 141-module sfw-registry corpus the fatal-sink slice went 986 -> 729 with this change and the entire js_typed_feedback_native_call_method_by_id class (257) is gone. The remaining 593 are js_closure_callN — the generic dynamic-value-call lowering, which holds the callee AND the `this` receiver AND each argument in registers across the argument list. That is the next site of this class and it is NOT fixed here. That mode is a diagnostic, so it EXITS 0 and reports counts. It is not calibrated to zero — the raw count is dominated by values the checker cannot prove are pointers, and even the --fatal-sinks slice still carries the js_closure_callN class above — so returning 1 on any hit would be a check that can never pass, the mirror image of CLAUDE.md's four "a gate that cannot fail" hazards and just as reliably ignored. Gating is opt-in via --max-stale N, which exits 1 above the budget, so a calibrated slice can become a ratchet later without the raw mode pretending to be one. --max-stale or --fatal-sinks without --stale-registers is a usage error (exit 2) rather than a silently ignored budget or an ignored filter. --self-test asserts all of it: the default must report 2 uses on the planted fixture and still exit 0, --max-stale 0 must exit 1, --max-stale 2 must exit 0, and the control fixture must report zero. The bind-anchored gate gc-root-dominance.yml actually runs is untouched and still exits non-zero on any violation. sfw-registry --help under PERRY_GC_MOVING_LOOP_POLLS=1 (compiled AND run with the flag) is still red — 8/10 SIGSEGV before these fixes surfaced past the TypeError, 5/10 after — so #7161's stopgap stays. Its default arm is clean 10/10. cargo test -p perry-codegen: the 6 loop_safepoint_purity failures from #7161's default flip, identical to main. cargo test -p perry-runtime: unchanged — this commit touches only perry-codegen, which perry-runtime does not depend on. Refs #7154, #7192, #7198, #7184, #7161, #7114, #6951. --- changelog.d/7206-stale-receiver-registers.md | 108 ++++++ crates/perry-codegen/src/expr/index_get.rs | 35 +- .../src/lower_call/console_promise.rs | 55 ++- scripts/gc_root_dominance_check.py | 314 ++++++++++++++++++ .../test_gap_gc_index_get_receiver_rooting.ts | 58 ++++ .../test_gap_gc_method_receiver_rooting.ts | 61 ++++ 6 files changed, 622 insertions(+), 9 deletions(-) create mode 100644 changelog.d/7206-stale-receiver-registers.md create mode 100644 test-files/test_gap_gc_index_get_receiver_rooting.ts create mode 100644 test-files/test_gap_gc_method_receiver_rooting.ts diff --git a/changelog.d/7206-stale-receiver-registers.md b/changelog.d/7206-stale-receiver-registers.md new file mode 100644 index 0000000000..0cea8a53a1 --- /dev/null +++ b/changelog.d/7206-stale-receiver-registers.md @@ -0,0 +1,108 @@ +### Fixed + +- **A method call's receiver and a computed read's base are now rooted across + the expressions lowered between them and the dispatch.** Two more sites of + #7192's root-store-dominance class, both found by extending its checker and + both reproduced in ~30 lines of TypeScript. + + `recv.m(f())` and `o[f()]` evaluate the reference first and the second + operand after — spec order, and codegen follows it. That left the reference + in a bare SSA register while `f()` was lowered, and `f()` allocates. Under + `PERRY_GC_MOVING_LOOP_POLLS=1` a loop back-edge poll inside it runs an + evacuating minor. The reference *survives* that minor — the closure capture + cell, shadow slot or module global holding it is a root — so it **moves**: + the collector rewrites that location and the register keeps naming + from-space. This is the same "property (2) — a rewritten location — is + worthless without property (3), reading that location again below the + collection point" that `expr/temp_root.rs`'s module header describes, and the + same fix #7192 applied to the property/element STORE receiver. + + - **`lower_call/console_promise.rs`** — the `js_native_call_method_by_id` + dispatch. The stale receiver makes the method lookup resolve against + abandoned memory, so the call throws `TypeError: value is not a function`. + In `sfw-registry` this is zod's `classic/schemas.ts:301`, + `inst.regex = (...args) => inst.check(checks.regex(...args))`: `inst` is + read out of the arrow's capture cell, held across `checks.regex(...args)` + (a real user call, so it polls), then used as `.check`'s receiver. The + arguments are now rooted too — each before the *next* one is lowered, per + `RootedOperands`' incremental contract — so an earlier argument cannot go + stale across a later one either. + - **`expr/index_get.rs`** — the dynamic-string-key arm and the last-resort + runtime-tag-check arm, i.e. the READ counterpart of #7192's `index_set` / + `property_set` guard, which only covered the store side. The stale base + makes the field read walk the keys array of from-space memory: a SIGSEGV + inside `get_field_by_name_object_tail`, or a silently wrong value. In + `sfw-registry` this is zod's `core/checks.ts:68`, + `numericOriginMap[typeof def.value]` — a module-global base with a key + expression that reads a property and therefore can collect. + + Both use `temp_root::guard_store_operand` / `reread_store_operand` / + `release_store_operand` (#7198's generalized naming). A temp root, not a + re-lower: re-lowering the reference would observe an assignment made by the + second operand itself, which is a miscompile rather than a rooting fix. The + guard emits nothing when the sibling expression cannot collect, so an inert + argument list or key keeps its previous IR exactly, and it is released + *after* the dispatch because the dispatcher allocates while reading these + values. + + Verified by two new gap tests, each red on the parent commit and green after, + and each clean under a non-moving collector so the failure is proven to track + collector mode rather than luck: + + | | parent | this change | + |---|---|---| + | `test_gap_gc_method_receiver_rooting.ts`, `POLLS=1` | `TypeError: value is not a function` 5/5 | `bad 0` **10/10** | + | `test_gap_gc_index_get_receiver_rooting.ts`, `POLLS=1` | `TypeError: Cannot read properties of undefined` 4/4 | `bad 0` **10/10** | + | both, `POLLS=1` + `PERRY_GEN_GC=0` | `bad 0` | `bad 0` | + | both, default (no polls) | `bad 0` | `bad 0` 5/5 | + +### Changed + +- **`scripts/gc_root_dominance_check.py` grows a `--stale-registers` mode.** + The shipped check anchors on a shadow-slot bind, so it can only see values + that are eventually rooted; neither site above is. The new mode checks the + more general invariant the module header already states — *no register + holding a GC pointer may be used below a collection point without being + re-read from a root* — by classifying every **heap-value source** (an + allocation, or a read of a collector-rewritten location: a shadow-slot load, + a closure capture cell, a temp-root slot, a module global, a mutable-capture + box), following it forward through bit-level identity ops, and reporting the + first real use that sits below a collecting call. `--fatal-sinks` narrows to + uses that *dereference* the value (a call receiver or callee), where a + relocation is fatal rather than merely wrong. + + It reproduces both sites above independently of any runtime probe, and it is + how they were found. Over the 141-module `sfw-registry` corpus the + fatal-sink slice went **986 → 729** with this change; the entire + `js_typed_feedback_native_call_method_by_id` class (257) is gone. The + remaining 593 are `js_closure_callN` — the generic dynamic-value-call + lowering, which holds the callee AND the `this` receiver AND each argument in + registers across the argument list. That is the next site of this class and + it is not fixed here. + + Like the bind-anchored check the mode is one-sided: `NONCOLLECTING` is the + only place a call is declared safe. It is a diagnostic, not a gate — the raw + (non-`--fatal-sinks`) count is dominated by values the checker cannot prove + are pointers, so it is a ranked lead list rather than a pass/fail number. + + **The exit status says so.** `--stale-registers` prints its counts and exits + `0`; it is not calibrated to zero, and a mode that returned `1` on any hit + would be a check that can never pass — the mirror image of CLAUDE.md's four + "a gate that cannot fail" hazards, and just as reliably ignored. Gating is + opt-in through the new **`--max-stale N`**, which exits `1` when more than + `N` uses are reported, so a slice that *has* been calibrated (say the + `--fatal-sinks` count once `js_closure_call*` is fixed) can become a ratchet + without the raw mode pretending to be one. Passing `--max-stale` or + `--fatal-sinks` without `--stale-registers` is a usage error (exit 2) rather + than a silently ignored budget or an ignored filter — either one would run + the bind-anchored check while looking like it did something else. + Misconfiguration keeps its own status: `--min-files` still makes an + empty corpus exit 2 in this mode too, and the bind-anchored gate that + `gc-root-dominance.yml` actually runs is untouched — it still exits non-zero + on any violation. + + `--self-test` grew four arms for this, so the exit status is asserted from + both ends rather than assumed: over the planted fixture the default mode + must report 2 uses and still exit `0`, `--max-stale 0` must exit `1`, + `--max-stale 2` must exit `0`, and the control fixture must report zero. + Reverting the default to `return 1 if total else 0` fails three of them. diff --git a/crates/perry-codegen/src/expr/index_get.rs b/crates/perry-codegen/src/expr/index_get.rs index 63fc244487..f48609273a 100644 --- a/crates/perry-codegen/src/expr/index_get.rs +++ b/crates/perry-codegen/src/expr/index_get.rs @@ -1987,7 +1987,23 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { let preserve_class_ref_bits = index_object_is_class_or_proto_ref(ctx, object.as_ref()); let obj_box = lower_expr(ctx, object)?; + // #7154: `o[f()]` evaluates the base first and the key second, + // leaving the base in a bare SSA register while `f()` runs. An + // evacuating minor inside `f()` relocates the base; the location + // it was read from is a root and gets rewritten, the register is + // not, and the field read then dereferences from-space memory. + // + // This is the READ counterpart of #7192's `index_set` / + // `property_set` receiver guard. zod's `core/checks.ts:68` + // (`numericOriginMap[typeof def.value]`) is the instance that + // SIGSEGV'd `sfw-registry --help` under + // `PERRY_GC_MOVING_LOOP_POLLS=1`: `numericOriginMap` is a module + // global (a registered root the collector rewrites) and the key + // `typeof def.value` is a property get that can collect. + let recv_guard = + super::temp_root::guard_store_operand(ctx, object, &obj_box, index); let key_box = lower_expr(ctx, index)?; + let obj_box = super::temp_root::reread_store_operand(ctx, &recv_guard, &obj_box); let blk = ctx.block(); let obj_bits = blk.bitcast_double_to_i64(&obj_box); let obj_handle = @@ -1999,11 +2015,13 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { "object[index]", TypedFeedbackContract::object_get_by_name(), ); - return Ok(ctx.block().call( + let out = ctx.block().call( DOUBLE, "js_typed_feedback_object_get_field_by_name_f64", &[(I64, &site_id), (I64, &obj_handle), (I64, &key_handle)], - )); + ); + super::temp_root::release_store_operand(ctx, recv_guard); + return Ok(out); } // Last-resort fallback with runtime tag checks on the index. // First runtime-check whether the index is a Symbol; if so, @@ -2011,7 +2029,12 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // IndexSet branch. Otherwise fall through to string/numeric. let preserve_class_ref_bits = index_object_is_class_or_proto_ref(ctx, object.as_ref()); let obj_box = lower_expr(ctx, object)?; + // #7154: same window as the dynamic-string-key arm above — the base + // is live in a register while the key expression is lowered, and an + // evacuating minor inside the key relocates it. + let recv_guard = super::temp_root::guard_store_operand(ctx, object, &obj_box, index); let idx_box = lower_expr(ctx, index)?; + let obj_box = super::temp_root::reread_store_operand(ctx, &recv_guard, &obj_box); // RequireObjectCoercible(base): `null[k]` / `undefined[k]` must throw // a TypeError per spec, NOT silently return undefined. The dotted // PropertyGet path already guards nullish receivers; the computed @@ -2114,14 +2137,18 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { ctx.block().br(&merge_lbl); // Merge. ctx.current_block = merge_idx; - Ok(ctx.block().phi( + let merged = ctx.block().phi( DOUBLE, &[ (&v_sym, &sym_end_lbl), (&v_str, &str_end_lbl), (&v_num, &num_end_lbl), ], - )) + ); + // Released in the merge block so every arm's getter (any of which + // can run a user getter and therefore collect) is still covered. + super::temp_root::release_store_operand(ctx, recv_guard); + Ok(merged) } // Phase H err: `agg.errors.length` — receiver is diff --git a/crates/perry-codegen/src/lower_call/console_promise.rs b/crates/perry-codegen/src/lower_call/console_promise.rs index dc49d5cebd..4bc662bb1f 100644 --- a/crates/perry-codegen/src/lower_call/console_promise.rs +++ b/crates/perry-codegen/src/lower_call/console_promise.rs @@ -852,11 +852,52 @@ pub fn try_lower_native_method_str_dispatch( return Ok(Some(reg)); } } + // #7154: `recv.m(f())` evaluates the receiver first and the + // arguments second — spec order, and codegen follows it. That left + // the receiver in a bare SSA register while every argument was + // lowered, and an argument that allocates reaches a back-edge poll + // and an evacuating minor. The minor RELOCATES the receiver: the + // location it was read from (a closure capture cell, a shadow slot, + // a module global) is a root and gets rewritten, but the register is + // not a root and keeps naming from-space. The dispatch below then + // resolves the method against abandoned memory and throws + // `TypeError: value is not a function`. + // + // This is the site that kept `sfw-registry --help` red under + // `PERRY_GC_MOVING_LOOP_POLLS=1` after #7192. zod's + // `classic/schemas.ts:301` builds + // `inst.regex = (...args) => inst.check(checks.regex(...args))`; + // `inst` is read out of the arrow's capture cell, held across + // `checks.regex(...args)` (a real user call, so it polls), and then + // used as the receiver of `.check`. + // + // Same shape, same fix as #7192's property/element STORE receiver: + // a temp root, not a re-lower. Re-lowering `object` would observe an + // assignment made by an argument, which is a miscompile rather than + // a rooting fix (see `temp_root::operand_is_reloadable`). Each + // argument is likewise rooted before the NEXT one is lowered, so an + // earlier argument cannot go stale across a later one. + let arg_collects: Vec = args + .iter() + .map(|a| crate::expr::temp_root::expr_may_trigger_gc(ctx, a)) + .collect(); + let any_arg_collects = arg_collects.iter().any(|&c| c); + let operand_exprs: Vec<&Expr> = std::iter::once(object.as_ref()) + .chain(args.iter()) + .collect(); + let mut roots = crate::expr::temp_root::root_operands_begin(args.len() + 1); let recv_box = lower_expr(ctx, object)?; - let mut lowered_args: Vec = Vec::with_capacity(args.len()); - for a in args { - lowered_args.push(lower_expr(ctx, a)?); + roots.push(ctx, object.as_ref(), &recv_box, any_arg_collects); + for (i, a) in args.iter().enumerate() { + let v = lower_expr(ctx, a)?; + roots.push(ctx, a, &v, arg_collects[i + 1..].iter().any(|&c| c)); } + // Re-read below the last collection point. Mandatory, not + // defensive: the temp-root slot is a MUTABLE root, so an evacuating + // cycle rewrites it and the register pushed beforehand is stale. + let rereads = roots.reread(ctx, &operand_exprs)?; + let recv_box = rereads[0].clone(); + let lowered_args: Vec = rereads[1..].to_vec(); // Pass a tagged pointer to the immutable StringPool dispatch // descriptor. A GC-backed string handle belongs to the main // thread's arena and cannot be resolved safely by a @@ -893,7 +934,7 @@ pub fn try_lower_native_method_str_dispatch( // call's location no longer shadows this one. crate::expr::calls::emit_call_location_at(ctx, call_byte_offset); let blk = ctx.block(); - return Ok(Some(blk.call( + let result = blk.call( DOUBLE, "js_typed_feedback_native_call_method_by_id", &[ @@ -903,7 +944,11 @@ pub fn try_lower_native_method_str_dispatch( (PTR, &args_ptr), (I64, &args_len_str), ], - ))); + ); + // Release AFTER the dispatch, not before: the dispatcher allocates + // while it reads these values. + roots.release(ctx); + return Ok(Some(result)); } } Ok(None) diff --git a/scripts/gc_root_dominance_check.py b/scripts/gc_root_dominance_check.py index be3e39e537..1d3ff6b06b 100755 --- a/scripts/gc_root_dominance_check.py +++ b/scripts/gc_root_dominance_check.py @@ -85,6 +85,8 @@ real corpus. """ import argparse +import contextlib +import io import os import re import sys @@ -758,6 +760,251 @@ def scan(blk, lo, hi): """ +# ------------------------------------------------- stale-register invariant +# +# The check above anchors on a shadow-slot BIND, so it can only see values that +# are eventually rooted. The residual #7154 offender is not one of those: the +# receiver of `inst.check(f())` is read out of a closure capture cell, kept in a +# bare register across `f()`, and never rooted at all. `--stale-registers` +# checks the more general invariant the module header states: +# +# No register holding a GC pointer may be USED below a collection point. +# After the last thing that can collect, every operand is either re-read from +# a root the collector rewrote, or re-derived from immutable storage. +# +# A "heap-value source" is an instruction that yields a value which may be a GC +# pointer. Two kinds: +# +# * an ALLOCATION (ALLOC_RE) -- unambiguously a fresh heap object; +# * a ROOT READ -- a load of a location the collector rewrites (a shadow-slot +# alloca, a closure capture cell, a temp-root slot, a module global, a +# mutable-capture box). Reading one produces a register that is correct +# *now* and stale after the next evacuation, which is precisely the +# "property (2) without property (3)" failure `temp_root.rs` describes. +# +# `js_closure_get_capture_bits` is the load-bearing one and it is deliberately +# in NONCOLLECTING (closure/alloc.rs:463 is a raw slot read that cannot +# allocate) -- being non-collecting is exactly what makes it a root READ rather +# than a collection point. + +# Loads of these globals are root reads: module-level variables live in +# `@perry_global_*` and are registered roots that evacuation rewrites. +GLOBAL_ROOT_RE = re.compile(r"@perry_global_[\w.$]+") + +# Calls that READ a collector-rewritten location into a register. +ROOT_READ_CALLS = { + "js_closure_get_capture_bits", # closure/alloc.rs:463 capture cell read + "js_closure_get_capture_ptr", + "js_gc_temp_root_get", # a MUTABLE root: rewritten on evacuation + "js_box_get_bits", # box.rs mutable-capture cell read + "js_implicit_this_get", # object/this_binding.rs:160 thread-local + "js_new_target_get", +} + +# Argument positions that make a stale pointer FATAL rather than merely wrong: +# the value is dereferenced as an object. Used only for ranking, never to +# suppress -- the check stays one-sided. +RECEIVER_SINKS = re.compile( + r"^js_(" + r"typed_feedback_native_call_method\w*|native_call_method\w*|" + r"native_call_value|object_get_field\w*|object_set_field\w*|" + r"object_get_property|object_set_property|put_value_set_dyn_ic|" + r"get_value_dyn_ic|closure_call\w*|call_closure|call_function|" + r"call_value|invoke_closure|apply_function|" + r"array_\w+|map_\w+|set_\w+|typed_feedback_\w*call\w*" + r")$" +) + + +def heap_source_kind(ins, slot_of_alloca): + """Classify `ins` as a producer of a possibly-GC-pointer register.""" + if ins.result is None: + return None + if ins.callee is not None: + if ALLOC_RE.match(ins.callee): + return "alloc" + if ins.callee in ROOT_READ_CALLS: + return "capture" if "capture" in ins.callee else "rootread" + return None + if "= load " in ins.text: + if GLOBAL_ROOT_RE.search(ins.text): + return "global" + m = re.search(r"load\s+(?:i64|double)\s*,\s*ptr %([\w.$]+)", ins.text) + if m and m.group(1) in slot_of_alloca: + return "slotload" + return None + + +class StaleUse: + def __init__(self, module, func, src, kind, use, collectors, reg, + poll_reaching=frozenset()): + self.module = module + self.func = func + self.src = src + self.kind = kind + self.use = use + self.collectors = collectors + self.reg = reg + 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) + + @property + def fatal_sink(self): + return bool(self.use.callee and RECEIVER_SINKS.match(self.use.callee)) + + +def check_func_stale(module, f, poll_reaching=frozenset(), moving_only=False): + """Report registers holding a heap value that are USED below a collection + point without being re-read from a root.""" + if not f.blocks: + return [] + slot_of_alloca = {} + for b in f.blocks: + for ins in f.insns[b]: + m = BIND_RE.search(ins.text) + if m: + slot_of_alloca[m.group(2)] = int(m.group(1)) + + def_of = {} + for b in f.blocks: + for ins in f.insns[b]: + if ins.result: + def_of[ins.result] = ins + + idom = dominators(f) + out = [] + + for b in f.blocks: + for src in f.insns[b]: + kind = heap_source_kind(src, slot_of_alloca) + if kind is None: + continue + # Forward closure over bit-level/identity ops: the untagged + # pointer, the nanbox and the bitcast are all the same address. + chain = {src.result} + grew = True + while grew: + grew = False + for bb in f.blocks: + for ins in f.insns[bb]: + if (ins.result and ins.result not in chain + and is_transparent(ins) + and uses(ins.text, chain)): + chain.add(ins.result) + grew = True + # First real (non-transparent) use of any register in the chain + # that sits below a collection point. + for bb in f.blocks: + if not dominates(idom, src.block, bb): + continue + for use in f.insns[bb]: + if use.result in chain or is_transparent(use): + continue + if not uses(use.text, chain): + continue + if use.block == src.block and use.idx <= src.idx: + continue + # `js_shadow_slot_bind(N, ptr %alloca)` names the alloca, + # not the value; a store THROUGH the chain is a use of the + # pointer operand only when the chain is the stored value. + hits = window_hits_generic(f, src, use) + if not hits: + continue + v = StaleUse(module, f.name, src, kind, use, hits, + src.result, poll_reaching) + if moving_only and not v.moving: + continue + out.append(v) + break + else: + continue + break + return out + + +def window_hits_generic(f, A, B): + """Collecting calls on some CFG path from just after A to just before 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 run_stale(parsed, poll_reaching, verbose, moving_only, fatal_only, + max_stale=None): + total = 0 + per_kind = defaultdict(int) + per_sink = defaultdict(int) + out = [] + for mod, fs in parsed: + for f in fs: + for v in check_func_stale(mod, f, poll_reaching, moving_only): + if fatal_only and not v.fatal_sink: + continue + total += 1 + per_kind[v.kind] += 1 + per_sink[v.use.callee or "store"] += 1 + cs = sorted({c.callee for c in v.collectors}) + out.append( + f"{mod}::{f.name}\n" + f" source ({v.kind}): {v.src.text.strip()}\n" + f" stale use : {v.use.text.strip()}\n" + f" between : {', '.join(cs[:6])}" + f"{' (+%d more)' % (len(cs) - 6) if len(cs) > 6 else ''}\n" + f" MOVING : " + f"{('YES via ' + ', '.join(v.movers[:3])) if v.moving else 'no'}\n" + ) + if verbose: + print("\n".join(out)) + print(f"=== stale-register uses: {total}") + for k, n in sorted(per_kind.items(), key=lambda kv: -kv[1]): + print(f" {n:6d} source={k}") + for k, n in sorted(per_sink.items(), key=lambda kv: -kv[1])[:15]: + print(f" {n:6d} sink={k}") + + # This mode is a DIAGNOSTIC, and unlike the bind-anchored check it is not + # calibrated to zero. The count is dominated by values the checker cannot + # prove are pointers, and even the `--fatal-sinks` slice still carries a + # known-unfixed class (`js_closure_call*`, #7154), so `!= 0` is the normal + # state of a healthy tree. Exiting 1 on any hit would make this a check + # that can never pass, which is the same failure as the four "a gate that + # cannot fail" hazards in CLAUDE.md read backwards: every caller learns to + # ignore the exit status, and the day it means something nobody is looking. + # Ranked leads are the product. Gating is opt-in and explicit via + # --max-stale, which is how a calibrated slice becomes a ratchet later. + if max_stale is not None: + if total > max_stale: + print(f"error: {total} stale-register use(s), budget is " + f"{max_stale}. Lower the count or raise --max-stale " + "deliberately.", file=sys.stderr) + return 1 + print(f"within budget: {total} <= {max_stale}") + return 0 + + + def _scan(paths, moving_only, anchor): """(violations, n_binds) over `paths`.""" parsed = [(os.path.basename(p), parse_file(p)) for p in sorted(paths)] @@ -780,6 +1027,20 @@ def _scan(paths, moving_only, anchor): return found, binds +def _stale_probe(path, max_stale): + """(reported uses, exit status) from --stale-registers over one file.""" + parsed = [(os.path.basename(path), parse_file(path))] + poll_reaching, _known = compute_poll_reaching( + [f for _m, fs in parsed for f in fs]) + buf = io.StringIO() + # Both streams: an over-budget probe is *expected* to print its error, and + # that line in the self-test transcript reads like a real failure. + with contextlib.redirect_stdout(buf), contextlib.redirect_stderr(buf): + rc = run_stale(parsed, poll_reaching, False, False, False, max_stale) + m = re.search(r"stale-register uses: (\d+)", buf.getvalue()) + return (int(m.group(1)) if m else -1), rc + + def self_test(): """Assert the checker reports the planted violation and clears the control. @@ -824,6 +1085,32 @@ def self_test(): file=sys.stderr) ok = False + # --stale-registers is a diagnostic, so its exit status is asserted + # from both ends: the default must NOT go red on a corpus that has + # hits, and --max-stale must actually be able to. A budget nobody has + # watched fail is a budget that has not been shown to work. + n_planted, rc_planted = _stale_probe(planted, None) + if n_planted != 2: + print(f"self-test FAIL: --stale-registers over the planted fixture " + f"-> {n_planted} uses, expected 2", file=sys.stderr) + ok = False + if rc_planted != 0: + print("self-test FAIL: --stale-registers without --max-stale must " + f"exit 0 (diagnostic), got {rc_planted}", file=sys.stderr) + ok = False + if _stale_probe(planted, 0)[1] != 1: + print("self-test FAIL: --max-stale 0 over the planted fixture must " + "exit 1; the budget cannot fail", file=sys.stderr) + ok = False + if _stale_probe(planted, 2)[1] != 0: + print("self-test FAIL: --max-stale 2 over the planted fixture must " + "exit 0; the budget is off by one", file=sys.stderr) + ok = False + if _stale_probe(clean, 0) != (0, 0): + print("self-test FAIL: --max-stale 0 over the control fixture must " + "report 0 uses and exit 0", file=sys.stderr) + ok = False + try: _scan([broken], False, "alloc") except MalformedIR: @@ -851,6 +1138,21 @@ def main(): ap.add_argument("-v", "--verbose", action="store_true") ap.add_argument("--self-test", action="store_true", help="run the built-in planted/clean fixtures and exit") + ap.add_argument("--stale-registers", action="store_true", + help="check the general stale-register invariant instead of " + "the bind-anchored one: report every register holding a " + "GC value that is USED below a collection point without " + "being re-read from a root (#7154). Diagnostic: this " + "mode exits 0 and reports counts unless --max-stale " + "gives it a budget") + ap.add_argument("--fatal-sinks", action="store_true", + help="with --stale-registers, keep only uses that " + "DEREFERENCE the stale value (a call receiver/callee), " + "where a relocation is fatal rather than merely wrong") + ap.add_argument("--max-stale", type=int, default=None, metavar="N", + help="with --stale-registers, exit 1 when more than N uses " + "are reported. Without it the mode is a ranked lead " + "list, not a pass/fail number, so it exits 0.") ap.add_argument("--min-files", type=int, default=1, metavar="N", help="fail unless at least N .ll files were scanned (default 1)") ap.add_argument("--min-binds", type=int, default=1, metavar="N", @@ -858,6 +1160,15 @@ def main(): "A clean verdict over zero root stores proves nothing.") ns = ap.parse_args() + # A knob that is silently ignored is a disarmed knob: `--max-stale 0` + # without `--stale-registers` would run the bind-anchored check and look + # like it enforced a budget, and `--fatal-sinks` alone would look like it + # narrowed a report it never reached. Refuse instead (argparse exits 2). + if ns.max_stale is not None and not ns.stale_registers: + ap.error("--max-stale requires --stale-registers") + if ns.fatal_sinks and not ns.stale_registers: + ap.error("--fatal-sinks requires --stale-registers") + if ns.self_test: return self_test() @@ -891,6 +1202,9 @@ def main(): 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]) + if ns.stale_registers: + return run_stale(parsed, poll_reaching, verbose, moving_only, + ns.fatal_sinks, ns.max_stale) n_binds = sum( 1 for _m, fs in parsed diff --git a/test-files/test_gap_gc_index_get_receiver_rooting.ts b/test-files/test_gap_gc_index_get_receiver_rooting.ts new file mode 100644 index 0000000000..3fb1ddfe0b --- /dev/null +++ b/test-files/test_gap_gc_index_get_receiver_rooting.ts @@ -0,0 +1,58 @@ +// #7154: the BASE of a computed property READ must be rooted across the +// evaluation of the key expression. +// +// `o[f()]` evaluates the base first and the key second — spec order, and +// codegen follows it — which left the base in a bare SSA register while `f()` +// was lowered. `f()` allocates, and under `PERRY_GC_MOVING_LOOP_POLLS=1` a loop +// back-edge poll inside it runs an evacuating minor. The base SURVIVES (the +// closure capture cell / module global holding it is a root), so it MOVES: the +// collector rewrites that location but not the register. The field read then +// walks the keys array of abandoned from-space memory — a SIGSEGV inside +// `get_field_by_name_object_tail`, or a silently wrong value. +// +// This is the READ counterpart of #7192's `index_set` / `property_set` receiver +// guard, which fixed only the STORE side (`o[f()] = v`). In the registry it is +// zod's `core/checks.ts:68`, +// `numericOriginMap[typeof def.value as "number" | "bigint" | "object"]` — +// a module-global base with a key expression that reads a property, and +// therefore can collect. +// +// LIVE BY CONSTRUCTION. The base is read out of a closure capture, so it is a +// young movable object; the key expression allocates long enough that the minor +// runs EARLY in it and the abandoned from-space copy is then reused by the rest +// of the key's own allocation; and the value read back is a heap object whose +// field is dereferenced, so a stale read is observable rather than latent. +// Clean under a non-moving collector, so the evacuating arms are the ones that +// bite. + +function keyOf(v: number): string { + const bits: any[] = []; + for (let i = 0; i < 4000; i++) { + bits.push({ i: i, s: "x", pad: [i, i + 1, i + 2] }); + } + return bits.length === 4000 ? "hit" : "miss"; +} + +function make(tag: number): (k: number) => number { + const originMap: any = { + hit: { v: tag, name: "hit" }, + miss: { v: -1, name: "miss" }, + other: { v: -2, name: "other" }, + spare: { v: -3, name: "spare" }, + }; + return (k: number) => (originMap[keyOf(k)] as any).v as number; +} + +function run(): number { + let bad = 0; + for (let r = 0; r < 200; r++) { + const f = make(r); + const got = f(r); + if (got !== r) { + bad++; + } + } + return bad; +} + +console.log("bad", run()); diff --git a/test-files/test_gap_gc_method_receiver_rooting.ts b/test-files/test_gap_gc_method_receiver_rooting.ts new file mode 100644 index 0000000000..72bb1d8dda --- /dev/null +++ b/test-files/test_gap_gc_method_receiver_rooting.ts @@ -0,0 +1,61 @@ +// #7154: the RECEIVER of a dynamic method call must be rooted across the +// evaluation of the call's arguments. +// +// `recv.m(f())` evaluates the receiver first and the arguments second — spec +// order, and codegen follows it — which left the receiver in a bare SSA +// register while `f()` was lowered. `f()` allocates, and under +// `PERRY_GC_MOVING_LOOP_POLLS=1` a loop back-edge poll inside it runs an +// evacuating minor. The receiver SURVIVES that minor (the closure capture cell +// holding it is a root), which means it MOVES: the collector rewrites the +// capture cell but not the caller's register. The dispatch then resolves `m` +// against abandoned from-space memory and throws +// `TypeError: value is not a function`. +// +// This is the site that kept `sfw-registry --help` red under +// `PERRY_GC_MOVING_LOOP_POLLS=1` after #7192 fixed the four alloc-anchored +// sites. In the registry it is zod's `classic/schemas.ts:301`, +// `inst.regex = (...args) => inst.check(checks.regex(...args))` — `inst` read +// from the arrow's capture cell, held across a real user call, then used as the +// receiver of `.check`. +// +// Same invariant as #7192, #7184 and #7114: a GC value's root must dominate +// every subsequent collection point, and a rewritten location is worthless +// unless the code below the collection point READS that location again. +// +// LIVE BY CONSTRUCTION. `inst` is a plain object literal read out of a closure +// capture, so the call takes the dynamic by-name dispatch rather than a static +// class-method call, and the argument allocates hard enough to reach the +// collector. A non-moving collection cannot expose this, so the evacuating arms +// are the ones that bite. + +function churn(x: number): number { + const bits: any[] = []; + for (let i = 0; i < 600; i++) { + bits.push({ i: i, s: "x" }); + } + return x + bits.length - 600; +} + +function make(t: number): (p: number) => number { + const inst: any = { + tag: t, + check(v: number): number { + return this.tag + v; + }, + }; + return (p: number) => inst.check(churn(p)); +} + +function run(): number { + let bad = 0; + for (let r = 0; r < 400; r++) { + const f = make(r); + const got = f(1); + if (got !== r + 1) { + bad++; + } + } + return bad; +} + +console.log("bad", run()); From 1f9d710480844b0f1bb0c3a10ec553465ce4ad6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 1 Aug 2026 21:42:33 +0200 Subject: [PATCH 2/3] fix(codegen): adopt #7207's reread_store_operand signature at the two #7206 sites Merge-time fix, not a change of intent. #7207 (`1679e22b4`, merged after this PR branched) widened `reread_store_operand` to take the operand expression and return `anyhow::Result` so its new `Reload` arm can re-lower a string-literal base instead of reusing a register taken before the collection point. The two call sites added here still passed the old three-argument form. git merged both files cleanly because the change is in a DIFFERENT file (`temp_root.rs`) from the call sites (`index_get.rs`) -- a textually clean merge that does not compile. Adopting the new signature also gives these two sites #7207's strictly better treatment of a literal base (`"abc"[k]`). --- crates/perry-codegen/src/expr/index_get.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/perry-codegen/src/expr/index_get.rs b/crates/perry-codegen/src/expr/index_get.rs index f48609273a..7f2ba03be3 100644 --- a/crates/perry-codegen/src/expr/index_get.rs +++ b/crates/perry-codegen/src/expr/index_get.rs @@ -2003,7 +2003,8 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { let recv_guard = super::temp_root::guard_store_operand(ctx, object, &obj_box, index); let key_box = lower_expr(ctx, index)?; - let obj_box = super::temp_root::reread_store_operand(ctx, &recv_guard, &obj_box); + let obj_box = + super::temp_root::reread_store_operand(ctx, &recv_guard, object, &obj_box)?; let blk = ctx.block(); let obj_bits = blk.bitcast_double_to_i64(&obj_box); let obj_handle = @@ -2034,7 +2035,8 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // evacuating minor inside the key relocates it. let recv_guard = super::temp_root::guard_store_operand(ctx, object, &obj_box, index); let idx_box = lower_expr(ctx, index)?; - let obj_box = super::temp_root::reread_store_operand(ctx, &recv_guard, &obj_box); + let obj_box = + super::temp_root::reread_store_operand(ctx, &recv_guard, object, &obj_box)?; // RequireObjectCoercible(base): `null[k]` / `undefined[k]` must throw // a TypeError per spec, NOT silently return undefined. The dotted // PropertyGet path already guards nullish receivers; the computed From 08a665d50a9eac78635582f5097514b14978c3e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 1 Aug 2026 23:10:43 +0200 Subject: [PATCH 3/3] test(gc): register #7206's two witnesses in the GC x repsel corpus Both are moving-only: clean on the shipped default on both sides of the fix, so they certify nothing on the `default` arm and belong with the `requires=move` rows. Measured red-then-green numbers are in the comment block. --- test-parity/gc_repsel_corpus.txt | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test-parity/gc_repsel_corpus.txt b/test-parity/gc_repsel_corpus.txt index 77725895cb..2b953831e1 100644 --- a/test-parity/gc_repsel_corpus.txt +++ b/test-parity/gc_repsel_corpus.txt @@ -209,3 +209,26 @@ test_gap_gc_inline_ctor_this_rooting # `requires=move` arms. test_gap_gc_catch_param_rooting test_gap_gc_closure_this_capture_rooting + +# --- Stale REGISTERS: a receiver and a computed base (#7206) ----------------- +# A different failure from every entry above. The sites above are root stores +# that are late, mis-indexed or missing; these two values are never rooted at +# all. Spec order evaluates the object first and the sibling operand second, so +# `recv.m(f())` and `o[f()]` leave the base in a bare SSA register while `f()` +# runs. It SURVIVES the minor -- the capture cell or module global it was read +# from is a root -- and therefore MOVES, the collector rewrites that location, +# and the register keeps naming from-space. A bare register is not a root, so +# no runtime GC probe can see it. +# +# Measured on `origin/main` (91170973c), compiled AND run with +# `PERRY_GC_MOVING_LOOP_POLLS=1`, oracle node 26.5.1 (`bad 0` for both): +# method_receiver_rooting `TypeError: value is not a function`, and +# exit=139 (SIGSEGV) under PERRY_GC_ZEAL=1 + +# PERRY_GC_PROTECT_FROMSPACE=1 +# index_get_receiver_rooting `TypeError: Cannot read properties of undefined +# (reading 'v')` +# Both are `bad 0` with #7206 applied, and `bad 0` on the shipped default on +# BOTH sides -- the default cannot express the bug, which is why these belong on +# the `requires=move` arms and prove nothing on `default`. +test_gap_gc_method_receiver_rooting +test_gap_gc_index_get_receiver_rooting