diff --git a/changelog.d/7116-string-literal-operand-stale-after-evacuation.md b/changelog.d/7116-string-literal-operand-stale-after-evacuation.md new file mode 100644 index 0000000000..2d37cde866 --- /dev/null +++ b/changelog.d/7116-string-literal-operand-stale-after-evacuation.md @@ -0,0 +1,56 @@ +### Fixed + +- **A string-literal operand was reused from a register across an allocating + sibling, so an evacuating collection silently truncated the result (#7114).** + `console.log("acc:" + run(10_000_000))` printed an **empty line and exited 0** + — no crash, no diagnostic. The corruption was allocation-count dependent + (correct at 100 000 iterations, prefix replaced by a garbage byte or gone + entirely at 10⁷), which is the signature of a stale heap address rather than a + logic error. Hoisting the call into its own statement made it correct, which + localised it to operand evaluation order rather than to `run`, to + `js_string_concat_value`, or to the arithmetic. + + In the emitted IR the literal's `__perry_init_strings_*` handle was loaded + *above* the call and masked to a pointer *below* it: + + ```llvm + %r1 = load double, ptr @m_ts_.str.1.handle ; read BEFORE + %r2 = call double @perry_fn_m_ts__run__spec_i32(i32 10000000) + %r3 = bitcast double %r1 to i64 ; STALE + %r4 = and i64 %r3, 281474976710655 + %r5 = call i64 @js_string_concat_value(i64 %r4, double %r2) + ``` + + The handle global *is* a registered GC root, so the string was never swept and + the global was rewritten when the copying minor relocated it. The register + taken beforehand was not. + + **Root cause: two implementations of one contract, drifted.** + `crates/perry-codegen/src/expr/temp_root.rs` suppresses a string literal from + temp rooting — correctly; it is already a root and cannot be swept — and + compensates by re-deriving it below the collection point. `RootedOperands` + (`new C(a, b)`, native collection methods) did both halves. `lower_exprs_rooted` + — behind `lower_operand_pair_rooted`, the array-literal element list and the + string-concat chain — did only the suppression. Its own comment said the + staleness "is not the hazard #6951 is about"; it was #7114. + + **The invariant this establishes**, now stated in the module header: *no + operand register may outlive 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, never reused.* A root buys three things + and they are not the same: liveness, a rewritten location, and **the value the + consuming call actually observes**. The third is the one #7114 dropped. + + The fix routes both helper families through one `operand_protection()` + decision (`Root` / `Reload` / `Reuse`) so the pair cannot drift again. This is + the codegen shadow-stack/temp-root mechanism, not `RuntimeHandleScope`: the + stale value lived in an LLVM SSA register in generated code, so no runtime + helper's handle scope could have seen it. + + Cost is zero runtime calls — `Reload` re-emits the `load` that was going to be + emitted anyway, and only when a later operand can collect, so `"user_" + i` + and every other non-collecting concat keep their previous IR byte for byte. + + Verified on an M1 at `--release` against Node 26.5.1: the new gap probe prints + `!119999700000` on `ff85fd483` and `acc:119999700000` after, with 60 GC cycles + and 2 255 476 objects relocated by the copying minor in the same run. diff --git a/crates/perry-codegen/src/expr/temp_root.rs b/crates/perry-codegen/src/expr/temp_root.rs index 0934f0aba6..d41fabf63f 100644 --- a/crates/perry-codegen/src/expr/temp_root.rs +++ b/crates/perry-codegen/src/expr/temp_root.rs @@ -20,6 +20,30 @@ //! evacuating cycle rewrites it and the register pushed beforehand is stale. //! That is also why this is preferable to widening conservative scanning — //! conservative roots have to pin, precise ones can move. +//! +//! # The invariant (#7114) +//! +//! **No operand register may outlive 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 — never reused.** +//! +//! A root buys three things, and they are not the same thing: +//! +//! 1. **liveness** — the object is marked instead of swept; +//! 2. **a rewritten location** — a slot evacuation updates to the new address; +//! 3. **the value the consuming call observes** — which is (2) only if the +//! code that resumes after the safepoint *reads that location again*. +//! +//! #7114 is what dropping (3) on its own looks like. A string literal is a load +//! from a `__perry_init_strings_*` handle global that +//! `js_gc_register_global_root` registered, so it has (1) and (2) for free — +//! and `console.log("acc:" + run(1e7))` still printed an empty line, because +//! the register loaded *before* `run` held the pre-move address. Exit code 0, +//! no diagnostic, no crash. +//! +//! [`operand_protection`] is the single place that decides which of the three +//! strategies an operand needs. Both helper families in this module route +//! through it; before #7114 they answered it separately and disagreed. use perry_hir::types::Type as HirType; use perry_hir::Expr; @@ -262,10 +286,12 @@ fn any_later_ref_may_trigger_gc(ctx: &FnCtx<'_>, exprs: &[&Expr], i: usize) -> b /// Lower `exprs` left to right, keeping each already-evaluated value precisely /// rooted across the evaluation of the ones that follow (#6951). /// -/// Returns the lowered values — **re-read from their roots**, so they are -/// valid after an evacuating cycle — and the guard index the caller must pass -/// to [`temp_root_release`] once the consuming call has run. `None` means -/// nothing needed protecting and no runtime calls were emitted. +/// Returns the lowered values — **re-read from their roots, or re-derived from +/// their immutable storage** ([`OperandProtection`]), so they are valid after +/// an evacuating cycle — and the guard index the caller must pass to +/// [`temp_root_release`] once the consuming call has run. `None` means nothing +/// needed a temp-root slot; it does NOT mean nothing was re-read, because the +/// [`OperandProtection::Reload`] half emits no runtime call at all. pub(crate) fn lower_exprs_rooted( ctx: &mut FnCtx<'_>, exprs: &[&Expr], @@ -273,38 +299,43 @@ pub(crate) fn lower_exprs_rooted( let mut values = Vec::with_capacity(exprs.len()); let mut slots: Vec> = Vec::with_capacity(exprs.len()); let mut guard: Option = None; + let mut reload: Vec = Vec::with_capacity(exprs.len()); for (i, expr) in exprs.iter().enumerate() { let value = super::lower_expr(ctx, expr)?; - // A value that provably cannot be a heap reference roots nothing, so a - // slot for it is pure TLS traffic. This is the gate that keeps - // `total + s.length` and other numeric operand pairs at their old IR. - // - // A string literal is skipped for the opposite reason: it is a load - // from a module global that `__perry_init_strings_*` registered with - // `js_gc_register_global_root`, so it already has a precise root and - // the sweep can never take it. (A register loaded from that global is - // still stale after an *evacuating* cycle — but that is true of every - // `Expr::String` use in the compiler, not something this site - // introduces, and it is not the hazard #6951 is about.) Template - // literals are mostly literal parts, so this matters. - let needs_root = !super::expr_is_known_non_pointer_shadow_value(ctx, expr) - && !matches!(expr, Expr::String(_)); - if needs_root && any_later_ref_may_trigger_gc(ctx, exprs, i) { - let idx = temp_root_push_double(ctx, &value); - // The FIRST slot pushed is the guard: truncating it drops every - // slot above it too, so one call releases the whole group. - if guard.is_none() { - guard = Some(idx.clone()); + // `any_later_ref_may_trigger_gc` is the *window*: can anything between + // this operand and the consuming call collect? [`operand_protection`] + // turns that window into the one strategy this operand needs. + let collects = any_later_ref_may_trigger_gc(ctx, exprs, i); + match operand_protection(ctx, expr, collects) { + OperandProtection::Root => { + let idx = temp_root_push_double(ctx, &value); + // The FIRST slot pushed is the guard: truncating it drops every + // slot above it too, so one call releases the whole group. + if guard.is_none() { + guard = Some(idx.clone()); + } + slots.push(Some(idx)); + reload.push(false); + } + OperandProtection::Reload => { + slots.push(None); + reload.push(true); + } + OperandProtection::Reuse => { + slots.push(None); + reload.push(false); } - slots.push(Some(idx)); - } else { - slots.push(None); } values.push(value); } - for (value, slot) in values.iter_mut().zip(slots.iter()) { - if let Some(idx) = slot { - *value = temp_root_get_double(ctx, idx); + for (i, value) in values.iter_mut().enumerate() { + if let Some(idx) = slots[i].clone() { + *value = temp_root_get_double(ctx, &idx); + } else if reload[i] { + // #7114: no runtime call — just the load that was already emitted, + // emitted again below the collection point so it observes the + // address evacuation wrote back into the handle global. + *value = super::lower_expr(ctx, exprs[i])?; } } Ok((values, guard)) @@ -362,6 +393,25 @@ pub(crate) struct RootedOperands { /// /// This is the same staleness #6981 reports one layer in (a raw typed-array /// pointer passed under the specialized ABI). +/// +/// # Why the sibling literal forms are deliberately absent +/// +/// `Expr::WtfString` (a lone-surrogate literal) and `Expr::I18nString` lower to +/// exactly the same thing as `Expr::String` — one load of a +/// `__perry_init_strings_*` handle global, registered with +/// `js_gc_register_global_root` by the same loop, `is_wtf8` or not +/// (`codegen/string_pool.rs`). They would be sound here. They are not listed +/// because [`operand_needs_root`] does not suppress them either, so they take a +/// real temp root — and **`Root` is strictly stronger than `Reload`**: it +/// supplies liveness, a rewritten location and the call-time value on its own, +/// where `Reload` borrows the first from the handle global. +/// +/// The failure mode to guard against is not the asymmetry, it is *half*-closing +/// it: adding a literal form to [`operand_needs_root`]'s suppression list +/// without adding it here leaves it on `Reuse`, which is #7114 for that form. +/// `wtf8_literal_operand_is_rooted_not_merely_reused` in +/// `tests/temp_root_operand_temporaries.rs` pins the current answer so that edit +/// goes red instead of shipping another silent wrong answer. pub(crate) fn operand_is_reloadable(expr: &Expr) -> bool { // ONLY provably immutable sources. A string literal always re-lowers to a // load of the same `__perry_init_strings_*` handle, so re-reading it can @@ -426,8 +476,8 @@ impl RootedOperands { value: &str, collects: bool, ) { - let needs_root = collects && operand_needs_root(ctx, operand); - if needs_root { + let protection = operand_protection(ctx, operand, collects); + if protection == OperandProtection::Root { let idx = temp_root_push_double(ctx, value); // The FIRST slot pushed is the guard: truncating it drops every // slot above it too, so one call releases the whole group. @@ -439,7 +489,7 @@ impl RootedOperands { self.slots.push(None); } self.reloadable - .push(!needs_root && collects && operand_is_reloadable(operand)); + .push(protection == OperandProtection::Reload); self.values.push(value.to_string()); } @@ -602,6 +652,69 @@ pub(crate) fn operand_needs_root(ctx: &FnCtx<'_>, expr: &Expr) -> bool { !matches!(expr, Expr::String(_)) } +/// What an already-lowered operand needs so that the consuming call observes a +/// valid, current address across a following collection point. +/// +/// See the module header for the three properties a root buys. Each variant is +/// the cheapest strategy that supplies all three for its class of operand: +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub(crate) enum OperandProtection { + /// Push a temp-root slot and re-read it. The only strategy that gives + /// liveness *and* a rewritten location *and* the call-time value, so it is + /// what every operand with no other root gets — and also what a local or a + /// module global gets, because those are mutable and re-deriving them would + /// observe a later assignment instead of the value the call was given. + Root, + /// Emit no runtime call; re-derive the operand below the collection point. + /// For operands whose storage is a registered root the collector rewrites + /// **and** which are immutable, so re-lowering provably yields the same + /// value at the corrected address. Only [`operand_is_reloadable`] answers + /// yes here, and only for a string literal. + Reload, + /// Reuse the register. Correct in exactly two cases: nothing between this + /// operand and its consumer can collect, or the value provably is not a + /// heap reference and relocation cannot invalidate it. + Reuse, +} + +/// THE decision. Every operand-protection helper in this module routes through +/// it, so "root, re-derive, or reuse?" is answered in exactly one place. +/// +/// It used to be answered in two, and they disagreed. [`RootedOperands`] paired +/// its suppression of string literals with the compensating re-load; +/// [`lower_exprs_rooted`] suppressed them and reused the register. That is +/// #7114: `"acc:" + run(1e7)` lowers through `lower_string_coerce_concat` → +/// `lower_operand_pair_rooted` → `lower_exprs_rooted`, the literal's handle was +/// loaded before the call and masked to a pointer after it, and once `run` drove +/// an evacuating minor the concat read the string's *old* address — printing an +/// empty line and exiting 0. +/// +/// Keeping the two predicates but calling them from two places is what let the +/// pair drift, so the fix is the single call site, not a second copy of the +/// re-load. +pub(crate) fn operand_protection( + ctx: &FnCtx<'_>, + expr: &Expr, + collects: bool, +) -> OperandProtection { + if !collects { + // Nothing can be swept and nothing can move before the consumer runs, + // so the register still holds the value the call observes. This is the + // gate that keeps `total + s.length`, `f(x, y)` and `[1, 2, 3]` at + // exactly the IR they emitted before #6951. + return OperandProtection::Reuse; + } + if operand_needs_root(ctx, expr) { + return OperandProtection::Root; + } + if operand_is_reloadable(expr) { + return OperandProtection::Reload; + } + // Suppressed by `expr_is_known_non_pointer_shadow_value`: not a heap + // reference, so there is nothing for the collector to move. + OperandProtection::Reuse +} + /// Open an expression-scope temp-root barrier for a call/constructor whose /// operands are `args`. /// diff --git a/crates/perry-codegen/src/lower_call/new.rs b/crates/perry-codegen/src/lower_call/new.rs index e1fafd2bd4..ee5d671ef1 100644 --- a/crates/perry-codegen/src/lower_call/new.rs +++ b/crates/perry-codegen/src/lower_call/new.rs @@ -138,11 +138,14 @@ pub(crate) fn lower_new_member_captured( /// - a **rooted** argument is re-read from its slot, because the slot is a /// *mutable* root that an evacuating cycle rewrites in place, leaving the /// register pushed beforehand stale; -/// - an argument that was NOT rooted because it reads a registered root (a -/// shadow-slotted local, a module global, a string literal) is **re-loaded**. -/// Those are never swept, but evacuation rewrote their storage too, so the -/// cached register points at where the value used to be. Re-lowering emits -/// the load again and costs no runtime call. +/// - an argument that was NOT rooted because it reads an *immutable* registered +/// root — a string literal, the only `temp_root::operand_is_reloadable` case +/// — is **re-loaded**. It is never swept, but evacuation rewrote its handle +/// global too, so the cached register points at where the string used to be. +/// Re-lowering emits the load again and costs no runtime call. (A +/// shadow-slotted local or a module global is a registered root as well, but +/// a *mutable* one, so it takes a temp-root slot instead: re-deriving it +/// would observe an assignment made after the call-time value was taken.) /// /// Called after the instance allocation and again before the late consumers /// that sit behind further arbitrary lowering (field initializers, an inlined diff --git a/crates/perry-codegen/tests/temp_root_operand_temporaries.rs b/crates/perry-codegen/tests/temp_root_operand_temporaries.rs index 695a36715b..c1fd8c1b3d 100644 --- a/crates/perry-codegen/tests/temp_root_operand_temporaries.rs +++ b/crates/perry-codegen/tests/temp_root_operand_temporaries.rs @@ -431,3 +431,182 @@ fn registered_root_operands_are_reloaded_rather_than_rooted() { the literal must not get a slot of its own, got {pushes}:\n{ir}" ); } + +// ---------------------------------------------------------------- #7114 ---- +// +// The other half of the same claim, and the one that was missing. `new C(...)` +// (above) suppressed the string literal AND re-loaded it. `lower_exprs_rooted` +// — the helper behind `lower_operand_pair_rooted`, the array-literal element +// list and the string-concat chain — suppressed it and reused the register. +// +// So `console.log("acc:" + run(1e7))` loaded the literal's handle before the +// call and masked the cached register to a pointer after it. The handle global +// is a registered root that evacuation rewrites, the register is not, and the +// concat read the string's pre-move address: an empty line, exit code 0. + +/// An operand that is statically NUMERIC *and* can collect, so `"lit" + it` +/// takes the fused `js_string_concat_value` path — the exact expression form +/// #7114 was reported against. +/// +/// A non-`Add` `Expr::Binary` is numeric by construction (`is_numeric_expr`), +/// and it is GC-capable whenever an operand is not an inert primitive, which an +/// object literal is not. That is the same pairing as the reported repro, where +/// the sibling was a `number`-returning call whose body allocated. +fn allocating_numeric() -> Expr { + Expr::Binary { + op: perry_hir::BinaryOp::Sub, + left: Box::new(allocating()), + right: Box::new(Expr::Number(0.0)), + } +} + +/// `"acc:" + ` — the reported shape. +/// +/// The invariant: **no operand register may outlive a collection point.** The +/// literal is not rooted (it does not need to be — it is already a registered +/// root and can never be swept), so the only thing that makes it correct is +/// that its `load` is emitted BELOW the sibling that collects. +#[test] +fn string_literal_concat_operand_is_re_derived_below_the_allocating_sibling() { + let ir = ir_for( + "concat_reload.ts", + vec![Stmt::Expr(Expr::Binary { + op: perry_hir::BinaryOp::Add, + left: Box::new(Expr::String("acc:".to_string())), + right: Box::new(allocating_numeric()), + })], + ); + + let concat = ir + .find("call i64 @js_string_concat_value(") + .unwrap_or_else(|| panic!("expected the fused string+value concat:\n{ir}")); + let alloc = ir[..concat] + .rfind("call i64 @js_object_alloc(") + .unwrap_or_else(|| panic!("the sibling operand must allocate:\n{ir}")); + let handle_load = ir[..concat] + .rfind("load double, ptr @concat_reload_ts_.str.") + .unwrap_or_else(|| panic!("the literal must come from its handle global:\n{ir}")); + + assert!( + handle_load > alloc, + "#7114: the handle load that feeds the concat must sit BELOW the \ + allocating sibling. Loading it above and masking the cached register \ + below is what made `console.log(\"acc:\" + run(1e7))` print an empty \ + line — the handle global is a registered root that an evacuating \ + cycle REWRITES, and the pre-call register keeps the pre-move \ + address:\n{ir}" + ); + + assert_eq!( + ir.matches("call i32 @js_gc_temp_root_push").count(), + 0, + "and it must cost no runtime call. A registered root already has \ + liveness; all it was missing is the re-derivation, which is the load \ + that was going to be emitted anyway:\n{ir}" + ); +} + +/// The gate. Nothing after the literal can collect, so nothing may move, so the +/// register is still the value the call observes — and the IR must be exactly +/// what it was before #6951 and before #7114: ONE load, no second one. +/// +/// Without this half the "fix" could be an unconditional re-load, which would +/// pay for the reported bug on every `"user_" + i` in the codebase. +#[test] +fn string_literal_concat_operand_is_not_re_derived_when_nothing_collects() { + let ir = ir_for( + "concat_noreload.ts", + vec![Stmt::Expr(Expr::Binary { + op: perry_hir::BinaryOp::Add, + left: Box::new(Expr::String("acc:".to_string())), + right: Box::new(Expr::Compare { + op: perry_hir::CompareOp::Lt, + left: Box::new(Expr::Number(1.0)), + right: Box::new(Expr::Number(2.0)), + }), + })], + ); + + assert_eq!( + ir.matches("load double, ptr @concat_noreload_ts_.str.") + .count(), + 1, + "a comparison over two immediates runs no user code and allocates \ + nothing, so the literal must be loaded exactly once:\n{ir}" + ); + assert!( + !ir.contains("call i32 @js_gc_temp_root_push"), + "and no rooting at all:\n{ir}" + ); +} + +/// The same helper, reached from its other caller: an array literal's element +/// list. `["lit", allocating()]` holds the literal across the element that +/// collects, and the array's own store must receive the re-derived address. +#[test] +fn string_literal_array_element_is_re_derived_below_an_allocating_element() { + let ir = ir_for( + "array_reload.ts", + vec![Stmt::Expr(Expr::Array(vec![ + Expr::String("lit".to_string()), + allocating(), + ]))], + ); + + let handle_pat = "load double, ptr @array_reload_ts_.str."; + let loads: Vec = ir.match_indices(handle_pat).map(|(i, _)| i).collect(); + let element_alloc = ir + .find("call i64 @js_object_alloc(") + .unwrap_or_else(|| panic!("element 1 must allocate:\n{ir}")); + + assert!( + loads.iter().any(|&l| l > element_alloc), + "#7114: element 0's handle must be re-derived below element 1's \ + allocation before it is stored into the array:\n{ir}" + ); +} + +/// The sibling literal forms, and why the asymmetry in `operand_is_reloadable` +/// is deliberate rather than an oversight (raised on #7116). +/// +/// A WTF-8 literal — a lone surrogate — lowers to exactly the same thing as +/// `Expr::String`: one load of a `__perry_init_strings_*` handle global that +/// `js_gc_register_global_root` registered. It is nonetheless **not** in the +/// suppression list, so it takes a real temp root rather than a re-load, and +/// that is strictly stronger: `Root` supplies liveness, a rewritten location +/// and the call-time value on its own. +/// +/// The hazard worth gating is not the asymmetry but *half*-closing it. Adding a +/// literal form to `operand_needs_root`'s suppression list without also adding +/// it to `operand_is_reloadable` drops it to `Reuse` — #7114, for that form — +/// and no other test in this file would notice. This one goes red. +#[test] +fn wtf8_literal_operand_is_rooted_not_merely_reused() { + // 0xED 0xA0 0x80 is U+D800 in WTF-8: a lone surrogate, which is what routes + // a literal to `Expr::WtfString` instead of `Expr::String`. + let ir = ir_for( + "wtf8_operand.ts", + vec![Stmt::Expr(Expr::Binary { + op: perry_hir::BinaryOp::Add, + left: Box::new(Expr::WtfString(vec![0xED, 0xA0, 0x80])), + right: Box::new(allocating_numeric()), + })], + ); + + let push = ir + .find("call i32 @js_gc_temp_root_push") + .unwrap_or_else(|| panic!("a WTF-8 literal operand must be ROOTED:\n{ir}")); + let alloc = ir + .find("call i64 @js_object_alloc(") + .unwrap_or_else(|| panic!("the sibling operand must allocate:\n{ir}")); + let get = ir + .find("call i64 @js_gc_temp_root_get") + .unwrap_or_else(|| panic!("and re-read after it:\n{ir}")); + + assert!( + push < alloc && alloc < get, + "order must be push -> allocating sibling -> re-read. Anything else \ + means the WTF-8 literal is being carried across the collection point \ + in a register, which is #7114 for lone-surrogate literals:\n{ir}" + ); +} diff --git a/test-files/test_gap_gc_string_literal_operand_rooting.ts b/test-files/test_gap_gc_string_literal_operand_rooting.ts new file mode 100644 index 0000000000..8bf1edc521 --- /dev/null +++ b/test-files/test_gap_gc_string_literal_operand_rooting.ts @@ -0,0 +1,104 @@ +// #7114 — a string-literal operand held across an allocating sibling. +// +// `console.log("acc:" + run(n))` loads the literal's `__perry_init_strings_*` +// handle BEFORE the call and masks it to a pointer AFTER it. The handle global +// is a registered GC root, so the string is never swept and the global is +// rewritten on evacuation — but the register taken beforehand still holds the +// pre-move address. Under enough allocation to drive an evacuating minor the +// concatenation silently dropped or corrupted its prefix and the program exited +// 0. On main this printed an EMPTY LINE where `acc:74999992500000` belongs. +// +// ***WHY THE FIRST LINE IS THE ONE THAT MATTERS.*** +// Every string literal in the module is materialized once, together, by +// `__perry_init_strings_*` before any user code runs — so they are all young at +// the same moment and the first evacuating cycle relocates all of them at once. +// After that they live in the old generation and a nursery scavenge cannot move +// them again. So a program gets exactly ONE chance to observe this bug, and it +// is the first literal-plus-allocating-call expression it evaluates. That is +// why `stale` runs first, before anything else has had a chance to collect, and +// why the checks below it are shape coverage rather than a second live probe. +// The exhaustive per-shape coverage is the codegen contract test +// (crates/perry-codegen/tests/temp_root_operand_temporaries.rs); this file is +// the end-to-end proof that the shape really corrupts under a real collection. +// +// Registered in test-parity/gc_repsel_corpus.txt so the GC matrix runs it under +// the evacuating arms and reports whether anything actually moved. A run in +// which nothing moved proves nothing here — a non-moving collection cannot +// produce a stale pointer. + +class Rec { + id: number; + score: number; + constructor(id: number, score: number) { + this.id = id; + this.score = score; + } +} + +// Escaping churn: the records go into a module-level sink and are dropped in +// batches, so the arena genuinely grows and the survivors are genuinely +// relocatable. Without the sink the allocations are dead on arrival and the +// collector has nothing to move. +let sink: Rec[] = []; +let dropped = 0; + +function make(i: number): Rec { + const r = new Rec(i, 0); + r.score = r.id * 1.5; + sink.push(r); + if (sink.length > 8192) { + dropped = dropped + sink.length; + sink = []; + } + return r; +} + +function run(n: number): number { + let acc = 0; + for (let i = 0; i < n; i++) { + const r = make(i); + acc = acc + r.score; + } + return acc; +} + +// Measured on main (M1, `--release`, DEFAULT GC settings, no env at all): +// N = 50 000 drives 0 collections and passes vacuously; N = 100 000 drives 7 +// cycles / 590 472 scavenged and still passes; N = 150 000 is the first size at +// which the first `run` reaches an evacuating minor while `"acc:"` is still in +// the nursery, and it prints an empty line. N below is ~3x that threshold so a +// collector retune degrades this to UNVERIFIED (the harness's honest state) +// rather than to a silent pass. +const N = 400000; + +// THE PROBE. First statement, first literal use: `"acc:"` is loaded before +// `run` and consumed after it. +console.log("acc:" + run(N)); + +// The same value with the call hoisted into its own statement — the literal is +// loaded after the collection, which is why this line was always correct and +// the line above was not. Keeping both makes a regression report the +// difference rather than just "the number is wrong". +const hoisted = run(N); +console.log("hoisted:" + hoisted); + +// Shape coverage. All four route through `lower_exprs_rooted`, the helper that +// suppressed the literal without re-deriving it; each pairs a literal operand +// with an allocating sibling in a different lowering path. They are NOT live +// here (see the note above), so they do not re-prove the bug — but each was +// promoted to first position in its own file and A/B'd on main at N = 400 000: +// the array-element list (`["arr", run(N)].join("|")`) and the template literal +// (`` `tpl:${run(N)}` ``) both printed an empty line on ff85fd483 and match +// after the fix; `run(N) + ":right"` and `"a" + "b" + run(N)` were already +// correct there, because in those orders the literal is loaded after the call. +// literal on the RIGHT -> js_value_concat_string +console.log(run(1000) + ":right"); +// literal + literal, allocating sibling further along the chain +console.log("chain:" + run(1000) + ":end"); +// template literal (the parts list is mostly literals) +console.log(`tpl:${run(1000)}:done`); +// literal argument next to an allocating argument +console.log("args", run(1000), "tail"); + +console.log("dropped", dropped > 0); +console.log("sink", sink.length > 0); diff --git a/test-parity/gc_repsel_corpus.txt b/test-parity/gc_repsel_corpus.txt index 4501ad364f..91ed6f8535 100644 --- a/test-parity/gc_repsel_corpus.txt +++ b/test-parity/gc_repsel_corpus.txt @@ -68,6 +68,26 @@ test_gap_repsel_proven_this_frozen # per-PR when `evac_minor` joins PR_ARMS. test_gap_gc_ta_ctor_source_rooting +# --- String-literal operand re-derivation (#7114) ---------------------------- +# Not a representation file either, so registered explicitly per the header +# rule. `"acc:" + run(n)` loads the literal's `__perry_init_strings_*` handle +# BEFORE the call and masks the cached register AFTER it. The handle global is a +# registered root that evacuation REWRITES, so the register keeps the pre-move +# address and the concat silently produced an empty string -- exit code 0, no +# diagnostic. Verified on main (ff85fd483) to FAIL at DEFAULT GC settings, with +# no arm env at all, so it bites on `default` and `shipped_default` as well as +# on the %E% arms. +# +# LIVE BY CONSTRUCTION, AND ONLY ONCE PER RUN. Every literal in a module is +# materialized together by `__perry_init_strings_*` before user code runs, so +# the first evacuating cycle relocates all of them at once and afterwards they +# are old-gen and a nursery scavenge cannot move them again. The probe is +# therefore the FIRST statement of the file -- keep it first. Measured on main, +# `--release`, default settings: N=100 000 passes (7 cycles, 590 472 scavenged +# -- it collected and it moved, just not before the concat), N=150 000 is the +# first failing size, and the file ships at N=400 000 for margin. +test_gap_gc_string_literal_operand_rooting + # --- Scalar-replaced object/array locals (#6968) ----------------------------- # Not a representation of its own: escape analysis DELETES the object and keeps # one entry-block alloca per field/element. Those allocas belong to no HIR