fix(gc): make argument temporaries precise roots (#6951)

changelog.d/6972-precise-root-argument-temporaries.md

@@ -0,0 +1,66 @@
+**fix(gc): argument temporaries are precise roots (#6951)**
+
+With the conservative native-stack scan disabled — precise/shadow-stack roots
+only — a collection landing during argument evaluation dropped `console.log`'s
+string-literal argument, with no crash and no diagnostic. Harder shapes
+(`fresh() + "/" + f()`, `new C(fresh(), f())`, `s.concat("|" + f())`) segfaulted.
+
+**Root cause.** The shadow stack roots *named locals*: one slot per pointer-typed
+local, bound to that local's alloca. It has no slot for the values that exist
+only between two instructions, and an LLVM SSA register is not a GC root.
+`console.log("alpha", churn())` lowers to `js_array_alloc(2)` plus one
+`js_array_push_f64` per argument, with the accumulator threaded through an SSA
+register. That register held the ONLY reference to everything already pushed —
+argument 0 included — across argument 1's evaluation. The sweep freed the
+half-built array, `churn` recycled the block, and the next push wrote the number
+into a header whose `length` had been reset to 0: a one-element array, and the
+label gone. Conservative stack scanning hid it, because `gc_check_trigger` forces
+a full conservative scan on both automatic arms while `gc/roots.rs`'s nominal
+production default is `Auto -> SkipDisabled` — the scan was doing load-bearing
+correctness work, not acting as a safety net.
+
+**Mechanism.** `crates/perry-runtime/src/gc/roots/temp_roots.rs` adds a
+per-thread temp-root *stack* callable from generated code, registered in
+`gc_init` as a budgeted mutable root scanner, so slots are marked AND rewritten
+rather than pinned. Slots are visited through `visit_heap_word_u64_slot`, the
+same decoder the shadow stack uses, so a slot may hold either word form the
+`gc::root_words` contract admits: a NaN-boxed value or a bare heap address (the
+raw `i64` array pointers threaded through `js_array_alloc`). Generated code
+pushes before the collection point, **re-reads** after (mandatory — an
+evacuating cycle rewrites the slot, so the pushed register is stale), and
+truncates after the consuming call. Truncate is a stack cut, not a pop, so a
+missed release is bounded by the next one. `ShadowSavepoint` now carries the
+temp-root depth, so the `longjmp` unwind that already restores the shadow stack
+restores this stack with it — no change to `crate::exception`.
+
+**Rooted sites.** The variadic argument accumulator (`console.log` / `info` /
+`warn` / `error` / `debug` / `trace` / `assert` / `timeLog`); the string-concat
+operand pair and the n-way concat chain (template literals, log lines), plus the
+intermediate `js_jsvalue_to_string` handle in the both-non-string fallback; the
+object-literal handle across its initializers (all three lowering paths); and
+array-literal element values.
+
+**Cost.** Emission is gated three ways, any one of which suppresses it: nothing
+after the value reaches a collection point; the value provably cannot be a heap
+reference; or the value is a string literal (already a registered global root).
+`"user_" + i`, `[1, 2, 3]`, `{a: i, b: total}` and all-local argument lists emit
+byte-identical IR to before. On a hot loop doing a concat, an array literal, an
+object literal and a template literal per iteration the gates take emitted
+rooting calls from 32 to 12.
+
+**Verification.** `scripts/gc_repsel_matrix.sh --arms all` against pinned Node
+26.5.0: 361/361 cells byte-exact, FAIL=0, XFAIL=0 —
+`test_gap_repsel_gc_stress × cons_scan_off` and `× cons_scan_off_force` move from
+XFAIL to PASS with the arm measurably live (17 completed cycles), so both entries
+are removed from `test-parity/gc_repsel_triage.txt` and `cons_scan_off` (a PR
+arm) becomes a hard gate on this shape. A 431-file gap-corpus A/B against
+`origin/main` produced identical result sets. Four new unit tests in
+`gc::tests::temp_roots`, every one pinning `ConservativeStackScanMode::Disabled`
+(with the scan on the bug is invisible), plus three codegen IR tests pinning the
+emission contract and the no-cost gate.
+
+**Still open**, filed with reproducers: #6968 (scalar-replaced object/array
+locals), #6969 (`new C(a, b)` constructor arguments), #6970 (native-method-call
+arguments), #6971 (string-method receiver + arguments). `moved_objects` remains 0
+in every arm and 333 matrix cells remain UNVERIFIED — that is #6950, which this
+change unblocks rather than fixes.

crates/perry-codegen/src/expr/array_literal.rs

@@ -4,8 +4,9 @@
 use anyhow::Result;
 use perry_hir::Expr;
 
+use super::temp_root::{lower_exprs_rooted, temp_root_release};
 use super::{
-    emit_jsvalue_slot_store_on_block, expr_produces_non_pointer_bits_by_construction, lower_expr,
+    emit_jsvalue_slot_store_on_block, expr_produces_non_pointer_bits_by_construction,
     nanbox_pointer_inline, FnCtx,
 };
 use crate::type_analysis::is_numeric_expr;
@@ -33,8 +34,15 @@ use crate::types::{DOUBLE, I32, I64, I8, PTR};
 /// allocated slot (offset hasn't advanced past the `fits` check) or a
 /// header with `length == capacity` and uninitialized elements. No
 /// allocator call runs between the header write and the element stores,
-/// so GC can't run in that window. Element expressions with their own
-/// allocations lower to SSA values pinned by conservative stack scanning.
+/// so GC can't run in that window.
+///
+/// #6951: element values themselves are a different matter. They are lowered
+/// before the allocation and each one then sits in an SSA register across
+/// every later element's evaluation — which is not a root, and was only ever
+/// covered by conservative native-stack scanning. `[freshString(), f()]` lost
+/// its first element as soon as `f` collected. `lower_exprs_rooted` roots each
+/// value that has an allocating element after it, and emits nothing for the
+/// all-literal / all-local shapes.
 pub(crate) fn lower_array_literal(ctx: &mut FnCtx<'_>, elements: &[Expr]) -> Result<String> {
     let n = elements.len();
     let all_numeric_elements = elements.iter().all(|e| is_numeric_expr(ctx, e));
@@ -45,18 +53,18 @@ pub(crate) fn lower_array_literal(ctx: &mut FnCtx<'_>, elements: &[Expr]) -> Res
         return Ok(nanbox_pointer_inline(ctx.block(), &arr));
     }
 
-    // Evaluate all element expressions *before* allocating. This keeps each
-    // value in an SSA register (spilled to stack if needed; reachable by the
-    // conservative stack scanner) so nested allocations inside element
-    // expressions don't see a half-initialized outer array.
-    let mut vals = Vec::with_capacity(n);
+    // Evaluate all element expressions *before* allocating, so nested
+    // allocations inside element expressions don't see a half-initialized
+    // outer array. Each evaluated value is kept in a temp root until the last
+    // element has been lowered (#6951).
     let mut layout_notes_needed = Vec::with_capacity(n);
     for value_expr in elements {
         layout_notes_needed.push(!expr_produces_non_pointer_bits_by_construction(
             ctx, value_expr,
         ));
-        vals.push(lower_expr(ctx, value_expr)?);
     }
+    let element_refs: Vec<&Expr> = elements.iter().collect();
+    let (vals, element_guard) = lower_exprs_rooted(ctx, &element_refs)?;
 
     // #5391: oversized modules outline array-literal construction. The inline
     // bump-alloc + N×(store + layout-note + barrier) sequence makes minified
@@ -75,7 +83,9 @@ pub(crate) fn lower_array_literal(ctx: &mut FnCtx<'_>, elements: &[Expr]) -> Res
         let arr = ctx
             .block()
             .call(I64, "js_array_from_values", &[(PTR, &buf), (I32, &n_str)]);
-        return Ok(nanbox_pointer_inline(ctx.block(), &arr));
+        let boxed = nanbox_pointer_inline(ctx.block(), &arr);
+        temp_root_release(ctx, element_guard);
+        return Ok(boxed);
     }
 
     // Inline bump-allocator path for small literals. Size threshold matches
@@ -211,7 +221,9 @@ pub(crate) fn lower_array_literal(ctx: &mut FnCtx<'_>, elements: &[Expr]) -> Res
             );
         }
 
-        return Ok(nanbox_pointer_inline(ctx.block(), &user_ptr_as_i64));
+        let boxed = nanbox_pointer_inline(ctx.block(), &user_ptr_as_i64);
+        temp_root_release(ctx, element_guard);
+        return Ok(boxed);
     }
 
     // Fallback for N > INLINE_MAX_ELEMENTS: keep the extern call + N inline
@@ -250,5 +262,7 @@ pub(crate) fn lower_array_literal(ctx: &mut FnCtx<'_>, elements: &[Expr]) -> Res
             .call(I32, "js_array_mark_numeric_f64_layout", &[(I64, &arr)]);
     }
 
-    Ok(nanbox_pointer_inline(ctx.block(), &arr))
+    let boxed = nanbox_pointer_inline(ctx.block(), &arr);
+    temp_root_release(ctx, element_guard);
+    Ok(boxed)
 }

crates/perry-codegen/src/expr/binary.rs

@@ -22,6 +22,7 @@ use crate::type_analysis::{
 };
 use crate::types::{DOUBLE, I1, I128, I32, I64};
 
+use super::temp_root::{lower_operand_pair_rooted, temp_root_release};
 use super::{is_known_finite, lower_expr, FnCtx};
 
 fn lower_arithmetic_operand(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<(String, bool)> {
@@ -411,13 +412,14 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
                     if other_known_primitive {
                         return lower_string_coerce_concat(ctx, left, right, l_is_str, r_is_str);
                     }
-                    let l = lower_expr(ctx, left)?;
-                    let r = lower_expr(ctx, right)?;
-                    return Ok(ctx.block().call(
+                    let (l, r, guard) = lower_operand_pair_rooted(ctx, left, right)?;
+                    let sum = ctx.block().call(
                         DOUBLE,
                         "js_dynamic_string_or_number_add",
                         &[(DOUBLE, &l), (DOUBLE, &r)],
-                    ));
+                    );
+                    temp_root_release(ctx, guard);
+                    return Ok(sum);
                 }
                 if is_bigint_expr(ctx, left) && is_bigint_expr(ctx, right) {
                     if let Some(value) = try_lower_small_bigint_literal_binary(
@@ -428,13 +430,12 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
                     ) {
                         return Ok(value);
                     }
-                    let l = lower_expr(ctx, left)?;
-                    let r = lower_expr(ctx, right)?;
-                    return Ok(ctx.block().call(
-                        DOUBLE,
-                        "js_dynamic_add",
-                        &[(DOUBLE, &l), (DOUBLE, &r)],
-                    ));
+                    let (l, r, guard) = lower_operand_pair_rooted(ctx, left, right)?;
+                    let sum =
+                        ctx.block()
+                            .call(DOUBLE, "js_dynamic_add", &[(DOUBLE, &l), (DOUBLE, &r)]);
+                    temp_root_release(ctx, guard);
+                    return Ok(sum);
                 }
                 // Refs #486: neither operand is statically known. Per JS
                 // spec for `+`, if EITHER side is a string at runtime, the
@@ -454,13 +455,14 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
                     && crate::type_analysis::is_numeric_expr(ctx, right))
                     || add_operands_have_pod_materialization_hazard(ctx, left, right)
                 {
-                    let l = lower_expr(ctx, left)?;
-                    let r = lower_expr(ctx, right)?;
-                    return Ok(ctx.block().call(
+                    let (l, r, guard) = lower_operand_pair_rooted(ctx, left, right)?;
+                    let sum = ctx.block().call(
                         DOUBLE,
                         "js_dynamic_string_or_number_add",
                         &[(DOUBLE, &l), (DOUBLE, &r)],
-                    ));
+                    );
+                    temp_root_release(ctx, guard);
+                    return Ok(sum);
                 }
             }
             // BigInt arithmetic fast path. NaN-tagged bigints compare
@@ -483,11 +485,12 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
                 {
                     return Ok(value);
                 }
-                let l = lower_expr(ctx, left)?;
-                let r = lower_expr(ctx, right)?;
-                return Ok(ctx
+                let (l, r, guard) = lower_operand_pair_rooted(ctx, left, right)?;
+                let value = ctx
                     .block()
-                    .call(DOUBLE, fname, &[(DOUBLE, &l), (DOUBLE, &r)]));
+                    .call(DOUBLE, fname, &[(DOUBLE, &l), (DOUBLE, &r)]);
+                temp_root_release(ctx, guard);
+                return Ok(value);
             }
             // A non-primitive operand may `ToNumeric` to a BigInt at runtime
             // (`Object(1n)`, or an object with a BigInt-returning

crates/perry-codegen/src/expr/mod.rs

@@ -133,6 +133,7 @@ mod dispatch;
 mod record_value;
 mod shadow_slot;
 mod slot_rep;
+pub(crate) mod temp_root;
 pub(crate) use slot_rep::{
     canonical_i32_locals_enabled, canonical_local_i32_slot, canonical_str_locals_enabled,
     collect_canonical_str_ineligible_locals, collect_closure_referenced_locals,

crates/perry-codegen/src/expr/object_literal.rs

@@ -5,6 +5,9 @@ use anyhow::Result;
 use perry_hir::types::Type as HirType;
 use perry_hir::Expr;
 
+use super::temp_root::{
+    any_may_trigger_gc, rooted_handle_begin, rooted_handle_get, rooted_handle_release,
+};
 use super::{lower_expr, nanbox_pointer_inline, FnCtx};
 use crate::nanbox::POINTER_MASK_I64;
 use crate::type_analysis::{compute_auto_captures, is_numeric_expr};
@@ -308,6 +311,12 @@ pub(crate) fn lower_object_literal(
     props: &[(String, Expr)],
     expected_ty: Option<&HirType>,
 ) -> Result<String> {
+    // #6951: the object handle is allocated BEFORE the property values are
+    // lowered and lives in an SSA register across all of them. `{ a: s, b: f() }`
+    // therefore had its half-built object swept by `f`'s collection, and the
+    // remaining field stores landed in recycled memory. Root the handle when any
+    // initializer can collect; literals of plain locals emit no extra IR.
+    let protect_handle = any_may_trigger_gc(props.iter().map(|(_, v)| v));
     let field_count = props.len() as u32;
     let zero_str = "0".to_string();
     let n_str = field_count.to_string();
@@ -365,16 +374,21 @@ pub(crate) fn lower_object_literal(
             ],
         );
 
+        let rooted = rooted_handle_begin(ctx, &obj_handle, protect_handle);
         for (i, (_, value_expr)) in props.iter().enumerate() {
             let v = lower_expr(ctx, value_expr)?;
             let idx_str = i.to_string();
+            let obj_handle = rooted_handle_get(ctx, &rooted);
             ctx.block().call_void(
                 "js_object_set_unboxed_f64_field",
                 &[(I64, &obj_handle), (I32, &idx_str), (DOUBLE, &v)],
             );
         }
+        let obj_handle = rooted_handle_get(ctx, &rooted);
         emit_unboxed_object_layout_init(ctx, &obj_handle);
-        return Ok(nanbox_pointer_inline(ctx.block(), &obj_handle));
+        let boxed = nanbox_pointer_inline(ctx.block(), &obj_handle);
+        rooted_handle_release(ctx, rooted);
+        return Ok(boxed);
     }
 
     if !any_method_closure && field_count > 0 {
@@ -416,9 +430,11 @@ pub(crate) fn lower_object_literal(
             ],
         );
 
+        let rooted = rooted_handle_begin(ctx, &obj_handle, protect_handle);
         for (i, (_, value_expr)) in props.iter().enumerate() {
             let v = lower_expr(ctx, value_expr)?;
             let idx_str = i.to_string();
+            let obj_handle = rooted_handle_get(ctx, &rooted);
             // Issue #448: the runtime `js_object_set_field` takes its
             // value as `JSValue` (`#[repr(transparent)] u64`), which the
             // System V / AArch64 / Win64 ABIs all pass in a *general*-
@@ -439,15 +455,19 @@ pub(crate) fn lower_object_literal(
                 &[(I64, &obj_handle), (I32, &idx_str), (I64, &v_bits)],
             );
         }
+        let obj_handle = rooted_handle_get(ctx, &rooted);
         if let Some(layout) = typed_layout.as_ref() {
             emit_object_typed_shape_init(ctx, &obj_handle, layout);
         }
-        return Ok(nanbox_pointer_inline(ctx.block(), &obj_handle));
+        let boxed = nanbox_pointer_inline(ctx.block(), &obj_handle);
+        rooted_handle_release(ctx, rooted);
+        return Ok(boxed);
     }
 
     let obj_handle = ctx
         .block()
         .call(I64, "js_object_alloc", &[(I32, &zero_str), (I32, &n_str)]);
+    let rooted = rooted_handle_begin(ctx, &obj_handle, protect_handle);
 
     // Track `(closure_value_double, reserved_this_slot_idx)` for each
     // method closure that needs `this` patched after the object is
@@ -472,6 +492,7 @@ pub(crate) fn lower_object_literal(
             let v = lower_expr(ctx, value_expr)?;
             this_patches.push((v.clone(), this_idx));
 
+            let obj_handle = 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);
@@ -484,6 +505,7 @@ pub(crate) fn lower_object_literal(
         }
 
         let v = lower_expr(ctx, value_expr)?;
+        let obj_handle = 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);
@@ -497,6 +519,7 @@ pub(crate) fn lower_object_literal(
     // Patch each method closure's reserved `this` slot with the object
     // pointer (NaN-boxed). Done AFTER all fields are set so every
     // method sees the fully-initialized object.
+    let obj_handle = rooted_handle_get(ctx, &rooted);
     if !this_patches.is_empty() {
         let blk = ctx.block();
         let obj_tagged = {
@@ -519,5 +542,7 @@ pub(crate) fn lower_object_literal(
         emit_object_typed_shape_init(ctx, &obj_handle, layout);
     }
 
-    Ok(nanbox_pointer_inline(ctx.block(), &obj_handle))
+    let boxed = nanbox_pointer_inline(ctx.block(), &obj_handle);
+    rooted_handle_release(ctx, rooted);
+    Ok(boxed)
 }