From f61e5539e0358f7b5554ffaca1b39e0714e441e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 3 Aug 2026 10:19:29 +0200 Subject: [PATCH 1/3] fix(gc): root the spread-array accumulator across its elements (#7280) Claude-Session: https://claude.ai/code/session_018ZFER8EEg8K7ez2n6oDrT9 (cherry picked from commit 6c23c4035c874cfd1b1a58bc968b853edbd15c4f) --- .../src/expr/objects_arrays_lit.rs | 45 ++++++++++++++++--- 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/crates/perry-codegen/src/expr/objects_arrays_lit.rs b/crates/perry-codegen/src/expr/objects_arrays_lit.rs index 6c5d87fec0..a7f86921c9 100644 --- a/crates/perry-codegen/src/expr/objects_arrays_lit.rs +++ b/crates/perry-codegen/src/expr/objects_arrays_lit.rs @@ -35,34 +35,67 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { .call(I64, "js_array_clone_for_spread", &[(DOUBLE, &src_box)]); return Ok(nanbox_pointer_inline(ctx.block(), &cloned)); } + // #7280: unlike `lower_array_literal` — which lowers every element + // FIRST (each into a temp root) and only then allocates — this path + // allocates the accumulator UP FRONT and lowers the elements into + // it. The half-built array is therefore live across every element + // expression, and for a spread literal those are arbitrary user + // code: `[a, ...gen(), b]` runs an iterator protocol between two + // pushes. It is live across the lowering's OWN calls too — + // `js_array_push_f64`, `js_array_push_hole` and + // `js_array_spread_append` all allocate. + // + // Threading `current_arr` through each call's RETURN value already + // handles REALLOCATION (`js_array_push_f64` hands back a new + // pointer when it grows). It does nothing for RELOCATION: nothing + // rooted the accumulator, so a minor between two elements finds an + // array reachable from no root at all — it is reclaimed, not merely + // moved, and the remaining appends write into recycled memory. + // 29 of the 77 fatal moving stale uses on the #7280 reproducer are + // this shape. + // + // `temp_root_set_i64` rather than a fixed `RootedHandle`: the + // accumulator's address legitimately CHANGES on every append, so + // the slot must be rewritten, not just re-read. Same contract as + // the string-concat accumulator (#6971). let cap_str = (elements.len() as u32).to_string(); let mut current_arr = ctx.block().call(I64, "js_array_alloc", &[(I32, &cap_str)]); + let root = super::temp_root::temp_root_push_i64(ctx, ¤t_arr); for elem in elements { match elem { ArrayElement::Expr(e) => { let v = lower_expr(ctx, e)?; + let arr = super::temp_root::temp_root_get_i64(ctx, &root); current_arr = ctx.block().call( I64, "js_array_push_f64", - &[(I64, ¤t_arr), (DOUBLE, &v)], + &[(I64, &arr), (DOUBLE, &v)], ); } ArrayElement::Hole => { - current_arr = - ctx.block() - .call(I64, "js_array_push_hole", &[(I64, ¤t_arr)]); + let arr = super::temp_root::temp_root_get_i64(ctx, &root); + current_arr = ctx.block().call(I64, "js_array_push_hole", &[(I64, &arr)]); } ArrayElement::Spread(e) => { let src_box = lower_expr(ctx, e)?; + let arr = super::temp_root::temp_root_get_i64(ctx, &root); current_arr = ctx.block().call( I64, "js_array_spread_append", - &[(I64, ¤t_arr), (DOUBLE, &src_box)], + &[(I64, &arr), (DOUBLE, &src_box)], ); } } + // The append may have grown the array (a new address) and may + // have run a collection that moved it again. The returned + // pointer is the live one; republish it before the next + // element's lowering can collect. + super::temp_root::temp_root_set_i64(ctx, &root, ¤t_arr); } - Ok(nanbox_pointer_inline(ctx.block(), ¤t_arr)) + let current_arr = super::temp_root::temp_root_get_i64(ctx, &root); + let boxed = nanbox_pointer_inline(ctx.block(), ¤t_arr); + super::temp_root::temp_root_truncate(ctx, &root); + Ok(boxed) } // `arr[i]` index access. INLINE FAST PATH for typed-Number arrays: From c664b436c3cd1c3ef4e5501c75f5bf91ec3708bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 3 Aug 2026 10:45:38 +0200 Subject: [PATCH 2/3] fix(gc): root the namespace-import object across its member materialization (#7280) Claude-Session: https://claude.ai/code/session_018ZFER8EEg8K7ez2n6oDrT9 (cherry picked from commit f536cbf9693ee676f3b32d7553d159a8873fbe99) --- .../perry-codegen/src/expr/dyn_extern_i18n.rs | 45 ++++++++++++++++++- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/crates/perry-codegen/src/expr/dyn_extern_i18n.rs b/crates/perry-codegen/src/expr/dyn_extern_i18n.rs index c89ce82ee0..a73ff6713c 100644 --- a/crates/perry-codegen/src/expr/dyn_extern_i18n.rs +++ b/crates/perry-codegen/src/expr/dyn_extern_i18n.rs @@ -908,6 +908,41 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { "js_object_alloc", &[(I32, &zero_str), (I32, &n_str)], ); + // #7280: root the half-built namespace object. + // + // Every other lowering that allocates an object and then + // fills it in carries this contract — `Expr::Object` since + // #6951, `Expr::ObjectSpread`, the class-object lowering + // since #7211. This one was added for a different reason + // (#629, Drizzle/Stripe namespace enumeration) and never + // got it, and it builds by far the LARGEST object in a + // dependency-scale program: one property per export of the + // imported module, materialized at every use site. + // + // Both halves of the loop are collection points, on every + // iteration: + // + // * `lower_expr(member_get)` is a full `ns.member` + // PropertyGet. For a const export that is a CALL into + // the exporting module's accessor — arbitrary user + // code; for a function it allocates a closure + // singleton; for a class it resolves a class ref. + // * `js_object_set_field_by_name` performs the keys-array + // transition, which allocates. + // + // With `handle` in a bare SSA register the object is + // reachable from NO root for the whole build, so a minor + // does not merely relocate it — it reclaims it, and the + // remaining stores land in recycled memory. The caller then + // receives a namespace whose members read back as garbage, + // which surfaces as `TypeError: is not a function` at + // the first member call, arbitrarily far away. + // + // Measured on #7280's stock-zod reproducer: `import * as + // core` materializes 269 members here, and the emitted IR + // carried ZERO `js_gc_temp_root_*` calls beside its 269 + // allocating stores. + let rooted = super::temp_root::rooted_handle_begin(ctx, &handle, true); for member in &members { let member_get = Expr::PropertyGet { byte_offset: 0, @@ -922,6 +957,10 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { let key_idx = ctx.strings.intern(member); let key_handle_global = format!("@{}", ctx.strings.entry(key_idx).handle_global); + // Re-read AFTER the member resolution: that is the + // collection point, so a register captured before it is + // the stale one. + let 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); @@ -931,8 +970,10 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { &[(I64, &handle), (I64, &key_raw), (DOUBLE, &v_box)], ); } - let blk = ctx.block(); - return Ok(nanbox_pointer_inline(blk, &handle)); + let handle = super::temp_root::rooted_handle_get(ctx, &rooted); + let boxed = nanbox_pointer_inline(ctx.block(), &handle); + super::temp_root::rooted_handle_release(ctx, rooted); + return Ok(boxed); } return Ok(ctx .block() From 0a00075871fb37f47c3732eb14cdaaea1920f12e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 3 Aug 2026 11:11:26 +0200 Subject: [PATCH 3/3] docs: changelog fragment for #7299 Claude-Session: https://claude.ai/code/session_018ZFER8EEg8K7ez2n6oDrT9 --- .../7299-gc-allocate-then-fill-rooting.md | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 changelog.d/7299-gc-allocate-then-fill-rooting.md diff --git a/changelog.d/7299-gc-allocate-then-fill-rooting.md b/changelog.d/7299-gc-allocate-then-fill-rooting.md new file mode 100644 index 0000000000..00a296af25 --- /dev/null +++ b/changelog.d/7299-gc-allocate-then-fill-rooting.md @@ -0,0 +1,81 @@ +### Fixed + +**GC rooting: a parameter's caller write, and two allocate-then-fill lowerings +that never got a rooting contract (#7280, #7154).** + +Three violations of the invariant at the top of +`docs/src/internals/gc-rooting-invariant.md` — a GC-managed value live across a +collection point must be reachable from a root before that point — all found by +running `scripts/gc_root_dominance_check.py --stale-registers --moving-only` +over a **dependency-scale** corpus rather than the 25-file curated one. #7284's +correction to `POLL_CAPABLE_RUNTIME` is what made the property-GET half of that +report readable at all: on the gate corpus `--moving-only` went 65 → 115, and +101 of the 115 are a single unaddressed rule (a shadow-slot load cached in a +register across a collection point). + +**A parameter's incoming argument is a write the analysis never saw.** +`collect_pointer_typed_locals`' refinement fixpoint proves a local non-pointer +from its `writes`, which is collected by walking the body — so for a *parameter* +it reasons from a strict subset of the local's definitions. The +optional-parameter desugaring then supplies, for free and on every optional +parameter in the program, the one write that completes the false proof: +`if (p === undefined) { p = undefined; }` is `Type::Void`, definitely +non-pointer, so `all_non_pointer` held and the parameter lost its shadow slot +while its declared type said `Object`. Measured on zod's +`clone(inst, def?, params?)`: `js_shadow_frame_enter(2)` with slot 0 ← `inst` +and no bind at all for `def` or `params`, so LLVM promoted `params` into +callee-saved `d8`. Fixed by seeding the caller's write +(`LocalWrite::Incoming(declared_ty)`) rather than special-casing parameters at +each conclusion, so every consumer of the fixpoint accounts for it without +having to know it must; a `number` parameter is still provably non-pointer, +because its declared type is the one thing that does constrain the caller. Same +defect as #7291's site (1), found independently and behaviourally equivalent to +it (identical frame size and bind set on the same probe). + +**The spread-array accumulator** (`Expr::ArraySpread`) allocates up front and +lowers the elements *into* the array — the opposite order from +`lower_array_literal`, which lowers every element first into a temp root and +only then allocates. Threading the accumulator through each +`js_array_push_f64` return value handles *reallocation* and does nothing for +*relocation*: for `[a, ...gen(), b]` the half-built array was live across an +iterator protocol while reachable from no root at all, so a minor reclaimed it +rather than moving it, and the remaining appends wrote into recycled memory. +Rooted with `temp_root_set_i64`, because the accumulator's address legitimately +changes on every append (the string-concat contract from #6971). + +**The namespace-import object** (`import * as ns` used as a value) materializes +one property per export of the source module. Every other allocate-then-fill +lowering has carried a rooting contract since #6951 (`Expr::Object`) or #7211 +(class objects); this one, added for #629's Drizzle/Stripe namespace +enumeration, never got one — and it builds the largest object in a +dependency-scale program. Both halves of its loop are collection points: +resolving `ns.member` for a `const` export is a call into the exporting module's +accessor, and `js_object_set_field_by_name` performs the allocating keys-array +transition. Stock zod's `import * as core` materializes 269 members here, and +the emitted IR carried zero `js_gc_temp_root_*` calls beside its 269 allocating +stores. + +`gc/policy.rs`'s "sound by construction" claim about the loop-polls route is +also corrected (#7280 ask 2): deferring to a precise safepoint makes the +*collector* precise and says nothing about whether the mutator's live values are +reachable from the root set that safepoint scans. + +**These do not fix #7280, and #7280 stays open.** Its reproducer is unchanged at +**0/30** on the revert configuration (`PERRY_GC_MOVING_LOOP_POLLS=1` compiled +*and* run) and **0/10** on the allocation-point arm with movement asserted +(10/10 runs `copied_objects>0`); the stock-zod control reads 31/40 against a +34/40 baseline, which is inside the noise band that configuration has shown +(31, 32, 34, 35, 35 across five independent builds). The shipped default is +unaffected: 30/30 and 40/40. #7291 measures 0/30 and 32/40 on the same harness, +so it does not close the acceptance test either. After the parameter fix the +`PERRY_GC_PROTECT_FROMSPACE_DEPTH=800` quarantine fault moves off zod's `clone` +and lands in `js_native_call_method` called from a compiled module-init body — a +receiver going stale in a *runtime* frame, which is #7249's blind spot. + +No witness ships with this change. One was written for the two accumulator +fixes and confirmed by IR inspection to exercise both lowerings, then discarded: +it is 20/20 clean on the parent under `loop_polls` and 10/10 clean on the +allocation-point arm with movement asserted, so it does not discriminate. A test +that passes on the parent is a dark test with a witness's name on it, which is +what #7278 exists to stop. The only artifact that discriminates for this class +remains the dependency-scale reproducer #7280 preserved.