diff --git a/CLAUDE.md b/CLAUDE.md index fd42f01d0d..3c31081f70 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,7 +8,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co Perry is a native TypeScript compiler written in Rust that compiles TypeScript source code directly to native executables. It uses SWC for TypeScript parsing and LLVM for code generation. -**Current Version:** 0.5.1367 +**Current Version:** 0.5.1368 ## TypeScript Parity Status diff --git a/Cargo.toml b/Cargo.toml index b842b8f410..63d00bd43d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -315,7 +315,7 @@ codegen-units = 16 codegen-units = 16 [workspace.package] -version = "0.5.1367" +version = "0.5.1368" edition = "2021" license = "MIT" repository = "https://github.com/PerryTS/perry" diff --git a/changelog.d/7648-layer1-slice5-lower-call.md b/changelog.d/7648-layer1-slice5-lower-call.md new file mode 100644 index 0000000000..09b0e1ddfd --- /dev/null +++ b/changelog.d/7648-layer1-slice5-lower-call.md @@ -0,0 +1,136 @@ +### Layer 1 rooting migration, slice 5 — the timer and namespace-call lowerings (#7615) + +`lower_call/extern_timers.rs` and `lower_call/namespace_call.rs` now make every +rooting decision through `crate::rooting`, and are listed in the +`MIGRATED_MODULES` ledger. Both lines are load-bearing on the committed source: +each named `lower_exprs_rooted` / `temp_root_release` before the migration. The +sabotage arm was run per module — a compiling `temp_root_push_double` / +`temp_root_truncate` pair planted in each, the ledger test confirmed red and +naming both lines, with the build output checked for `error[` (**0** in both +arms, so neither plant was a build failure scored as a successful sabotage). + +**Four live bugs, found by the audit rather than by the translation.** All four +are demonstrated against node 26.5.1 (the `.node-version` oracle) with a +baseline compiler built from `main` in a separate target directory, and verified +fixed. + +1. **Two-argument `setTimeout` / `setInterval` held the callback unrooted across + the delay expression.** #7210 rooted the *trailing-argument* forms and left + their two-argument siblings alone, although the comment it wrote names the + window exactly. The delay is an arbitrary expression, so + `setTimeout(() => …, churn())` emits + + ```llvm + %r6 = call i64 @js_closure_alloc(...) ; the callback + %r8 = bitcast i64 %r7 to double ; a BARE register + %r9 = call double @perry_fn_mod__churn() ; the delay. User code, polls. + %r10 = call i64 @js_timer_validate_callback(double %r8, i32 0) ; stale + ``` + + Compiled with `PERRY_GC_MOVING_LOOP_POLLS=1` and run under `PERRY_GC_ZEAL=1`, + the baseline throws #7210's own symptom text — `The "callback" argument must + be of type function. Received an instance of Object` — for both timers, where + node prints the scheduled timer. + + **Two preconditions, and both are easy to miss.** `PERRY_GC_MOVING_LOOP_POLLS=1` + is a **compile**-time flag: without it `churn()` gets no loop back-edge polls, + zeal only fires at event-loop boundaries, a compute-only loop never collects, + and the identical binary prints the correct answer at exit 0. Check it took + effect with `--trace llvm`: the module must contain at least one + `call … @js_gc_loop_safepoint` (1 with the flag, 0 without). And the callback + must genuinely **capture** — a callback that closes over nothing but module + globals allocates a closure whose contents nothing relocates, so the stale + register keeps working. `PERRY_GC_PROTECT_FROMSPACE` and + `…_DEPTH` are *not* needed here: this fault is a use-after-move that lands on + recycled memory, not a from-space `SIGSEGV`, so zeal alone surfaces it. + + **The runtime fault is arrangement-dependent; the IR window is not.** Review + could not reproduce the `TypeError` on a `main` baseline built during the + audit, using these files and these commands — it printed the correct answer. + What review *did* confirm, directly in `--trace llvm` output, is the window + itself: `%r9 = bitcast i64 %r8 to double` (the callback), then + `call double @perry_fn_…__churn()`, then + `js_timer_validate_callback(double %r9, …)` reading the register defined above + the call — and the fixed arm storing `%r8` into a root slot and reloading it. + Whether a stale pointer is *observably* wrong depends on what gets recycled + into those bytes, which is why the acceptance tests assert the IR ordering + rather than a runtime outcome. **A window that does not fault today is not a + window that is absent** — it is one whose victim happened to survive. + + Conversely, a window that does not fault is not a window that is absent — + whether a stale pointer is *observably* wrong depends on what gets recycled + into those bytes. The IR above is the evidence that the bug is there; the + thrown `TypeError` is only evidence that it is reachable in one arrangement. + +2. **The var-shaped namespace export dispatched through a stale closure.** + `import * as ns from "./m"; ns.arrow(churn())` fetches the closure from its + zero-arg getter first (spec order — the callee reference is evaluated before + the arguments), holds it in a bare register across every argument's lowering, + and only then `unbox_to_i64`s it into a raw heap address. This is #7280 + taxonomy (a) and (c) at once: `root_reload` could not have repaired it, because + the pointer is derived *below* the window from a register captured *above* it. + Baseline throws `TypeError: value is not a function`; node and the fix print + `a:1` / `b:2`. Same two preconditions as bug 1 (compile-time polls; zeal alone + suffices). + + The reproducer that faults imports a **`.ts`** sibling and calls a two-argument + export. A `.mjs` sibling with a one-argument export emits the identical window + — getter, `churn`, then `bitcast`/`and` of the pre-call register into + `js_closure_call1` — and does **not** fault, for the recycling reason above. + Worth knowing before concluding from one non-faulting arrangement that the arm + is clean. + +3. **The `has_rest` namespace direct call lost every rest element — silently, on + the default build, with no GC instrumentation at all.** This is the most + serious of the four and the only one that needs nothing special to see. + The #7154 accumulator shape verbatim: `current` was a raw `*mut ArrayHeader` + threaded through a push loop while the next argument's expression ran, holding + the only reference to everything pushed so far. `lower_rest_call_args_rooted` + was written for exactly this and this path never adopted it. For + `lib.joinRest(churn("head"), churn("r1"), churn("r2"), churn("r3"))` node + prints `head|r1,r2,r3` and the baseline prints `head|`. Independently + reproduced during review with a different repro shape, where the baseline + prints **nothing at all and exits 0** — no zeal, no protect, no + `PERRY_GC_MOVING_LOOP_POLLS`, just the plain compiler: the argument churn + allocates enough to guarantee a collection inside the window, so unlike the + other three this one needs no instrumentation to arrange. A wrong answer, not + a crash. Delegating to the audited helper also pads the fixed parameters to + the declared arity, which the hand-rolled loop did not. + +4. **Three more unprotected windows**, repaired in passing and reachable by + inspection rather than by a reproducer that faults today: the `fs/promises` + `writeFile` / `appendFile` / `rmdir` arms (operand-to-operand — `path` held + across `content` and `options`), both V8-bridge arms (a bare + `for a in args { lower_expr }`, #7240's shape in a path that post-dates the + fix), and the plain namespace direct-call argument loop. + +**Cost is zero where the window cannot collect**, and that is now pinned rather +than asserted. A literal delay routes the callback to `OperandProtection::Reuse`, +and the emitted module for `setTimeout(fn, 5)` / `setInterval(fn, 5)` contains no +temp-root traffic at all — the callback register feeds +`js_timer_validate_callback` directly, exactly as before. + +**Tests.** `lower_call/timer_rooting_tests.rs` asserts on emitted IR rather than +on runtime behaviour, because the runtime fault needs a capturing callback, a +polling delay *and* the compile-time `PERRY_GC_MOVING_LOOP_POLLS=1` (off by +default since #7161) — a gap test would be green on the default build whether or +not the fix is present, which is hazard 4. The assertion is an *ordering*, not a +slot count: with an allocating delay the register `js_timer_validate_callback` +reads must be defined below the delay's allocation, and with a literal delay it +must be the original register with zero temp-root traffic. Checked against the +pre-fix source: the two ordering tests fail, the two zero-cost tests pass, so +neither is vacuous. + +**Four modules of the family were deliberately not migrated**, and the reason is +recorded in the ledger comment because it is a statement about the API: three of +them need a re-read at more than one point and every `with_operands_rooted*` form +has exactly one. `mod.rs` returns a guard on purpose (its consumers in +`func_ref.rs` are block-splitting specialized-ABI diamonds whose release must sit +in a merge block ~200 lines below); `new.rs` re-reads one operand group at three +caller-chosen points under a scope marker spanning ~20 return paths; +`console_promise.rs`'s `lower_dynamic_closure_call` re-reads in two stages; and +`early_branches.rs`'s only escape-hatch uses are the already-paired +`implicit_this_save`/`restore`, so migrating it would be a rename that made the +ledger line look substantive while asserting nothing. The concrete missing +combinator is the variadic/rest shape: per-element re-reads between allocating +pushes. diff --git a/crates/perry-codegen/src/lower_call/extern_timers.rs b/crates/perry-codegen/src/lower_call/extern_timers.rs index a157a31034..f8604f38f3 100644 --- a/crates/perry-codegen/src/lower_call/extern_timers.rs +++ b/crates/perry-codegen/src/lower_call/extern_timers.rs @@ -7,15 +7,75 @@ //! //! The three trailing-argument forms share one GC contract, documented on the //! `setTimeout` arm: the whole argument list is lowered through -//! `lower_exprs_rooted`, and only then stored into the stack buffer. +//! [`crate::rooting::with_operands_rooted`], and only then stored into the +//! stack buffer. +//! +//! # Layer 1 migration (#7615) +//! +//! Every rooting decision in this file goes through `crate::rooting`. The three +//! `lower_exprs_rooted` + `temp_root_release` pairs became +//! [`with_operands_rooted`], which owns the release on every path out instead of +//! handing it back as a guard the arm has to remember to drop. +//! +//! **The migration found a live bug, and it is the reason this file was worth +//! migrating rather than merely relisting.** #7210 rooted the *trailing-argument* +//! forms of `setTimeout`/`setInterval` and left their **two-argument** siblings +//! alone, even though the comment it wrote names the exact window those siblings +//! have: +//! +//! ```text +//! %r6 = call i64 @js_closure_alloc(...) ; the callback +//! %r8 = bitcast i64 %r7 to double ; cb_box -- a BARE register +//! %r9 = call double @perry_fn_mod__churn() ; the DELAY. User code. +//! %r10 = call i64 @js_timer_validate_callback(double %r8, i32 0) ; stale +//! ``` +//! +//! `setTimeout(() => …, churn())` is legal JS — the delay is an arbitrary +//! expression — so `%r8` crosses a real user call with back-edge polls while +//! nothing roots it. Under `PERRY_GC_ZEAL=1 PERRY_GC_PROTECT_FROMSPACE=1` the +//! baseline throws #7210's own symptom text, `The "callback" argument must be of +//! type function. Received an instance of Object`, where node prints the +//! scheduled timer. +//! +//! The one-argument and `clear*` arms are deliberately NOT routed through the +//! API. Their operand is consumed by the very next emission, so the window is +//! empty — counted in EMISSIONS, not in source lines — and +//! `with_operands_rooted` over a one-element list provably emits nothing +//! (`any_may_trigger_gc` over an empty tail is `false`). Routing them through it +//! would buy uniformity and no protection. use anyhow::Result; use perry_hir::Expr; use crate::expr::{lower_expr, nanbox_pointer_inline, FnCtx}; use crate::nanbox::double_literal; +use crate::rooting::with_operands_rooted; use crate::types::{DOUBLE, I32, I64, PTR}; +/// Fill an entry-block `[n x double]` staging buffer from `vals`, and yield the +/// `ptr` to its first element. +/// +/// Shared by the four trailing-argument arms. The buffer is NOT a GC root — +/// nothing in the precise walk visits an `alloca_entry_array` — so every caller +/// fills it from values that have already been re-read below the last collection +/// point, and emits nothing that can collect between the fill and the consuming +/// call. +pub(super) fn fill_arg_buffer(ctx: &mut FnCtx<'_>, vals: &[String]) -> String { + let n = vals.len(); + let buf = ctx.func.alloca_entry_array(DOUBLE, n); + for (i, v) in vals.iter().enumerate() { + let blk = ctx.block(); + let slot = blk.gep(DOUBLE, &buf, &[(I64, &format!("{}", i))]); + blk.store(DOUBLE, v, &slot); + } + let ptr_reg = ctx.block().next_reg(); + ctx.block().emit_raw(format!( + "{} = getelementptr [{} x double], ptr {}, i64 0, i64 0", + ptr_reg, n, buf + )); + ptr_reg +} + /// Lower a timer builtin, or `Ok(None)` if `name` is not one. pub fn try_lower_extern_timer_call( ctx: &mut FnCtx<'_>, @@ -53,22 +113,30 @@ pub fn try_lower_extern_timer_call( ); return Ok(Some(nanbox_pointer_inline(blk, &id))); } + // The delay is an arbitrary expression, so lowering it is a collection + // point sitting between the callback's allocation and the + // `js_timer_validate_callback` that reads it — see the module header for + // the emitted IR and the reproducer. `setTimeout(fn, 100)` emits exactly + // the IR it emitted before: a literal delay cannot collect, so + // `operand_protection` routes the callback to `Reuse` and nothing is + // pushed. "setTimeout" if args.len() == 2 => { - let cb_box = lower_expr(ctx, &args[0])?; - let delay_box = lower_expr(ctx, &args[1])?; - let blk = ctx.block(); - let zero_idx = "0"; - let cb_handle = blk.call( - I64, - "js_timer_validate_callback", - &[(DOUBLE, &cb_box), (I32, zero_idx)], - ); - let id = blk.call( - I64, - "js_set_timeout_callback", - &[(I64, &cb_handle), (DOUBLE, &delay_box)], - ); - return Ok(Some(nanbox_pointer_inline(blk, &id))); + let boxed = with_operands_rooted(ctx, &[&args[0], &args[1]], |ctx, vals| { + let (cb_box, delay_box) = (vals[0].clone(), vals[1].clone()); + let blk = ctx.block(); + let cb_handle = blk.call( + I64, + "js_timer_validate_callback", + &[(DOUBLE, &cb_box), (I32, "0")], + ); + let id = blk.call( + I64, + "js_set_timeout_callback", + &[(I64, &cb_handle), (DOUBLE, &delay_box)], + ); + Ok(nanbox_pointer_inline(blk, &id)) + })?; + return Ok(Some(boxed)); } "setImmediate" if !args.is_empty() => { if args.len() == 1 { @@ -88,34 +156,23 @@ pub fn try_lower_extern_timer_call( // there for why the callback register and the staging buffer are // one fix, not two. let arg_refs: Vec<&Expr> = args.iter().collect(); - let (vals, guard) = crate::expr::temp_root::lower_exprs_rooted(ctx, &arg_refs)?; - let cb_box = vals[0].clone(); let n = args.len() - 1; - let buf = ctx.func.alloca_entry_array(DOUBLE, n); - for (i, v) in vals.iter().skip(1).enumerate() { + let boxed = with_operands_rooted(ctx, &arg_refs, |ctx, vals| { + let cb_box = vals[0].clone(); + let ptr_reg = fill_arg_buffer(ctx, &vals[1..]); let blk = ctx.block(); - let slot = blk.gep(DOUBLE, &buf, &[(I64, &format!("{}", i))]); - blk.store(DOUBLE, v, &slot); - } - let ptr_reg = ctx.block().next_reg(); - ctx.block().emit_raw(format!( - "{} = getelementptr [{} x double], ptr {}, i64 0, i64 0", - ptr_reg, n, buf - )); - let blk = ctx.block(); - let two_idx = "2"; - let cb_handle = blk.call( - I64, - "js_timer_validate_callback", - &[(DOUBLE, &cb_box), (I32, two_idx)], - ); - let id = blk.call( - I64, - "js_set_immediate_callback_args", - &[(I64, &cb_handle), (PTR, &ptr_reg), (I32, &n.to_string())], - ); - let boxed = nanbox_pointer_inline(ctx.block(), &id); - crate::expr::temp_root::temp_root_release(ctx, guard); + let cb_handle = blk.call( + I64, + "js_timer_validate_callback", + &[(DOUBLE, &cb_box), (I32, "2")], + ); + let id = blk.call( + I64, + "js_set_immediate_callback_args", + &[(I64, &cb_handle), (PTR, &ptr_reg), (I32, &n.to_string())], + ); + Ok(nanbox_pointer_inline(blk, &id)) + })?; return Ok(Some(boxed)); } // Refs #665: `setTimeout(fn, delay, ...args)` — JS spec forwards @@ -144,103 +201,83 @@ pub fn try_lower_extern_timer_call( // lowered. That is not staleness — nothing anywhere refers to the // object, so it is a premature SWEEP. // - // `lower_exprs_rooted` closes both at once: it protects each value + // `with_operands_rooted` closes both at once: it protects each value // as soon as it is produced and re-reads them all below the last // one, so the stores below observe post-collection addresses. Cost // is zero when nothing in the list can collect (`OperandProtection:: - // Reuse`), which is the `setTimeout(fn, 0, someLocal)` case. + // Reuse`), which is the `setTimeout(fn, 0, someLocal)` case. The + // release happens after `body` returns — below the consuming call, + // which reads the buffer — and on the error path too. let arg_refs: Vec<&Expr> = args.iter().collect(); - let (vals, guard) = crate::expr::temp_root::lower_exprs_rooted(ctx, &arg_refs)?; - let cb_box = vals[0].clone(); - let delay_box = vals[1].clone(); let n = args.len() - 2; - let buf = ctx.func.alloca_entry_array(DOUBLE, n); - for (i, v) in vals.iter().skip(2).enumerate() { + let boxed = with_operands_rooted(ctx, &arg_refs, |ctx, vals| { + let (cb_box, delay_box) = (vals[0].clone(), vals[1].clone()); + let ptr_reg = fill_arg_buffer(ctx, &vals[2..]); let blk = ctx.block(); - let slot = blk.gep(DOUBLE, &buf, &[(I64, &format!("{}", i))]); - blk.store(DOUBLE, v, &slot); - } - let ptr_reg = ctx.block().next_reg(); - ctx.block().emit_raw(format!( - "{} = getelementptr [{} x double], ptr {}, i64 0, i64 0", - ptr_reg, n, buf - )); - let blk = ctx.block(); - let zero_idx = "0"; - let cb_handle = blk.call( - I64, - "js_timer_validate_callback", - &[(DOUBLE, &cb_box), (I32, zero_idx)], - ); - let id = blk.call( - I64, - "js_set_timeout_callback_args", - &[ - (I64, &cb_handle), - (DOUBLE, &delay_box), - (crate::types::PTR, &ptr_reg), - (I32, &n.to_string()), - ], - ); - let boxed = nanbox_pointer_inline(ctx.block(), &id); - // Released only after the consuming call: it reads the buffer. - crate::expr::temp_root::temp_root_release(ctx, guard); + let cb_handle = blk.call( + I64, + "js_timer_validate_callback", + &[(DOUBLE, &cb_box), (I32, "0")], + ); + let id = blk.call( + I64, + "js_set_timeout_callback_args", + &[ + (I64, &cb_handle), + (DOUBLE, &delay_box), + (PTR, &ptr_reg), + (I32, &n.to_string()), + ], + ); + Ok(nanbox_pointer_inline(blk, &id)) + })?; return Ok(Some(boxed)); } + // The 2-arg twin of the `setTimeout` arm above, and it carried the same + // live bug — see the module header. "setInterval" if args.len() == 2 => { - let cb_box = lower_expr(ctx, &args[0])?; - let delay_box = lower_expr(ctx, &args[1])?; - let blk = ctx.block(); - let one_idx = "1"; - let cb_handle = blk.call( - I64, - "js_timer_validate_callback", - &[(DOUBLE, &cb_box), (I32, one_idx)], - ); - let id = blk.call( - I64, - "setInterval", - &[(I64, &cb_handle), (DOUBLE, &delay_box)], - ); - return Ok(Some(nanbox_pointer_inline(blk, &id))); + let boxed = with_operands_rooted(ctx, &[&args[0], &args[1]], |ctx, vals| { + let (cb_box, delay_box) = (vals[0].clone(), vals[1].clone()); + let blk = ctx.block(); + let cb_handle = blk.call( + I64, + "js_timer_validate_callback", + &[(DOUBLE, &cb_box), (I32, "1")], + ); + let id = blk.call( + I64, + "setInterval", + &[(I64, &cb_handle), (DOUBLE, &delay_box)], + ); + Ok(nanbox_pointer_inline(blk, &id)) + })?; + return Ok(Some(boxed)); } "setInterval" if args.len() >= 3 => { // #7210: same treatment as `setTimeout` above. let arg_refs: Vec<&Expr> = args.iter().collect(); - let (vals, guard) = crate::expr::temp_root::lower_exprs_rooted(ctx, &arg_refs)?; - let cb_box = vals[0].clone(); - let delay_box = vals[1].clone(); let n = args.len() - 2; - let buf = ctx.func.alloca_entry_array(DOUBLE, n); - for (i, v) in vals.iter().skip(2).enumerate() { + let boxed = with_operands_rooted(ctx, &arg_refs, |ctx, vals| { + let (cb_box, delay_box) = (vals[0].clone(), vals[1].clone()); + let ptr_reg = fill_arg_buffer(ctx, &vals[2..]); let blk = ctx.block(); - let slot = blk.gep(DOUBLE, &buf, &[(I64, &format!("{}", i))]); - blk.store(DOUBLE, v, &slot); - } - let ptr_reg = ctx.block().next_reg(); - ctx.block().emit_raw(format!( - "{} = getelementptr [{} x double], ptr {}, i64 0, i64 0", - ptr_reg, n, buf - )); - let blk = ctx.block(); - let one_idx = "1"; - let cb_handle = blk.call( - I64, - "js_timer_validate_callback", - &[(DOUBLE, &cb_box), (I32, one_idx)], - ); - let id = blk.call( - I64, - "js_set_interval_callback_args", - &[ - (I64, &cb_handle), - (DOUBLE, &delay_box), - (crate::types::PTR, &ptr_reg), - (I32, &n.to_string()), - ], - ); - let boxed = nanbox_pointer_inline(ctx.block(), &id); - crate::expr::temp_root::temp_root_release(ctx, guard); + let cb_handle = blk.call( + I64, + "js_timer_validate_callback", + &[(DOUBLE, &cb_box), (I32, "1")], + ); + let id = blk.call( + I64, + "js_set_interval_callback_args", + &[ + (I64, &cb_handle), + (DOUBLE, &delay_box), + (PTR, &ptr_reg), + (I32, &n.to_string()), + ], + ); + Ok(nanbox_pointer_inline(blk, &id)) + })?; return Ok(Some(boxed)); } "clearTimeout" if args.len() == 1 => { diff --git a/crates/perry-codegen/src/lower_call/mod.rs b/crates/perry-codegen/src/lower_call/mod.rs index 74a06f57b7..b58d9e12d5 100644 --- a/crates/perry-codegen/src/lower_call/mod.rs +++ b/crates/perry-codegen/src/lower_call/mod.rs @@ -62,6 +62,10 @@ mod options; /// the same condition that routes it (#7592). pub(crate) mod property_get; mod scalar_method; +/// Rooting coverage for the two-argument timer arms slice 5 repaired — see the +/// module header for why the default build cannot fault on them. +#[cfg(test)] +mod timer_rooting_tests; /// #7510: which of the two typed-shape layout entry points a `new` site emits, /// and where. Split out of `new.rs` to keep it under the 2000-line cap. mod typed_shape_init; diff --git a/crates/perry-codegen/src/lower_call/namespace_call.rs b/crates/perry-codegen/src/lower_call/namespace_call.rs index 173e9c899b..b4b73524c4 100644 --- a/crates/perry-codegen/src/lower_call/namespace_call.rs +++ b/crates/perry-codegen/src/lower_call/namespace_call.rs @@ -1,14 +1,69 @@ //! Issue #636: namespace member call — //! `Call { callee: PropertyGet { ExternFuncRef(ns), method }, args }` //! where `ns ∈ namespace_imports`. +//! +//! # Layer 1 migration (#7615) +//! +//! Every rooting decision this file makes goes through `crate::rooting`. The +//! three `lower_exprs_rooted` + `temp_root_release` pairs in the `timers` arms +//! became [`with_operands_rooted`], which owns the release on every path out. +//! +//! **The audit that earned the ledger line matters more than the translation.** +//! Only the three `timers` arms were rooted at all; every other arm in this file +//! lowered its operands into bare SSA registers and then held them across more +//! user code. Five distinct windows, all of them the shapes +//! `docs/src/internals/gc-rooting-invariant.md` names: +//! +//! 1. **`fs/promises` `writeFile` / `appendFile` / `rmdir`** — operand-to-operand. +//! `path` is lowered, then `content` and `options` lower arbitrary user +//! code, then `path` is read by the consuming call. +//! 2. **Both V8-bridge arms** — `for a in args { lowered.push(lower_expr(a)?) }` +//! with no protection at all. This is #7240's shape verbatim, in a loop that +//! post-dates the fix. +//! 3. **The var-shaped namespace export** (`imported_vars`) — the worst of the +//! five. The closure is fetched from its zero-arg getter FIRST (spec order: +//! the callee reference is evaluated before the arguments), sits in a bare +//! register across every argument's lowering, and is only then `unbox_to_i64`'d +//! into a RAW heap address and dispatched. It is #7280 taxonomy (a) and (c) at +//! once: `root_reload` could not have repaired it even if it had been reached, +//! because the pointer is derived below the window from a register captured +//! above it. +//! 4. **The `has_rest` direct-call arm** — the #7154 accumulator shape, verbatim: +//! `current` is a raw `*mut ArrayHeader` threaded through a push loop while +//! the NEXT argument's expression is lowered, holding the only reference to +//! everything pushed so far. `super::lower_rest_call_args_rooted` was written +//! for exactly this and this path never adopted it. +//! 5. **The non-rest direct-call arm** — the plain unprotected argument loop. +//! +//! Windows are counted in **emissions**, not source lines. The `clear*` arm and +//! the one-argument `setImmediate` arm lower a single operand and consume it in +//! the very next emission, so they have no window and are deliberately left on +//! bare `lower_expr`; routing them through the API would emit nothing anyway +//! (`any_may_trigger_gc` over an empty tail is `false`) and would only suggest a +//! protection that is not there to give. +//! +//! One boundary is stated rather than hidden: the `has_rest` arm delegates to +//! `super::lower_rest_call_args_rooted`, which still names the raw API because +//! `crate::rooting` cannot yet express the variadic shape — the rest array's +//! per-element pushes each allocate, so every OTHER operand must be re-read +//! between them, and `with_operands_rooted` has exactly one re-read point. That +//! is a gap in the API, filed with the slice, not a decision this file makes. +//! Delegating to an audited helper is the same posture as calling `lower_expr`. use anyhow::{bail, Result}; use perry_hir::Expr; +use super::extern_timers::fill_arg_buffer; use crate::expr::{lower_expr, nanbox_pointer_inline, unbox_to_i64, FnCtx}; use crate::nanbox::double_literal; +use crate::rooting::with_operands_rooted; use crate::types::{DOUBLE, I32, I64, PTR}; +/// The NaN-boxed `undefined` literal — this file's most repeated expression. +fn undefined_lit() -> String { + double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)) +} + pub fn try_lower_namespace_member_call( ctx: &mut FnCtx<'_>, callee: &Expr, @@ -57,99 +112,73 @@ pub fn try_lower_namespace_member_call( // value. "setTimeout" if !args.is_empty() => { let arg_refs: Vec<&Expr> = args.iter().collect(); - let (vals, guard) = crate::expr::temp_root::lower_exprs_rooted(ctx, &arg_refs)?; - let cb_box = vals[0].clone(); - let delay_box = if args.len() >= 2 { - vals[1].clone() - } else { - double_literal(0.0) - }; - if args.len() <= 2 { + let boxed = with_operands_rooted(ctx, &arg_refs, |ctx, vals| { + let cb_box = vals[0].clone(); + let delay_box = vals.get(1).cloned().unwrap_or_else(|| double_literal(0.0)); + if vals.len() <= 2 { + let blk = ctx.block(); + let cb_handle = unbox_to_i64(blk, &cb_box); + let id = blk.call( + I64, + "js_set_timeout_callback", + &[(I64, &cb_handle), (DOUBLE, &delay_box)], + ); + return Ok(nanbox_pointer_inline(blk, &id)); + } + let n = vals.len() - 2; + let ptr_reg = fill_arg_buffer(ctx, &vals[2..]); let blk = ctx.block(); let cb_handle = unbox_to_i64(blk, &cb_box); let id = blk.call( I64, - "js_set_timeout_callback", - &[(I64, &cb_handle), (DOUBLE, &delay_box)], + "js_set_timeout_callback_args", + &[ + (I64, &cb_handle), + (DOUBLE, &delay_box), + (PTR, &ptr_reg), + (I32, &n.to_string()), + ], ); - let boxed = nanbox_pointer_inline(ctx.block(), &id); - crate::expr::temp_root::temp_root_release(ctx, guard); - return Ok(Some(boxed)); - } - let n = args.len() - 2; - let buf = ctx.func.alloca_entry_array(DOUBLE, n); - for (i, v) in vals.iter().skip(2).enumerate() { - let blk = ctx.block(); - let slot = blk.gep(DOUBLE, &buf, &[(I64, &format!("{}", i))]); - blk.store(DOUBLE, v, &slot); - } - let ptr_reg = ctx.block().next_reg(); - ctx.block().emit_raw(format!( - "{} = getelementptr [{} x double], ptr {}, i64 0, i64 0", - ptr_reg, n, buf - )); - let blk = ctx.block(); - let cb_handle = unbox_to_i64(blk, &cb_box); - let id = blk.call( - I64, - "js_set_timeout_callback_args", - &[ - (I64, &cb_handle), - (DOUBLE, &delay_box), - (PTR, &ptr_reg), - (I32, &n.to_string()), - ], - ); - let boxed = nanbox_pointer_inline(ctx.block(), &id); - crate::expr::temp_root::temp_root_release(ctx, guard); + Ok(nanbox_pointer_inline(blk, &id)) + })?; return Ok(Some(boxed)); } "setInterval" if args.len() >= 2 => { let arg_refs: Vec<&Expr> = args.iter().collect(); - let (vals, guard) = crate::expr::temp_root::lower_exprs_rooted(ctx, &arg_refs)?; - let cb_box = vals[0].clone(); - let delay_box = vals[1].clone(); - if args.len() == 2 { + let boxed = with_operands_rooted(ctx, &arg_refs, |ctx, vals| { + let (cb_box, delay_box) = (vals[0].clone(), vals[1].clone()); + if vals.len() == 2 { + let blk = ctx.block(); + let cb_handle = unbox_to_i64(blk, &cb_box); + let id = blk.call( + I64, + "setInterval", + &[(I64, &cb_handle), (DOUBLE, &delay_box)], + ); + return Ok(nanbox_pointer_inline(blk, &id)); + } + let n = vals.len() - 2; + let ptr_reg = fill_arg_buffer(ctx, &vals[2..]); let blk = ctx.block(); let cb_handle = unbox_to_i64(blk, &cb_box); let id = blk.call( I64, - "setInterval", - &[(I64, &cb_handle), (DOUBLE, &delay_box)], + "js_set_interval_callback_args", + &[ + (I64, &cb_handle), + (DOUBLE, &delay_box), + (PTR, &ptr_reg), + (I32, &n.to_string()), + ], ); - let boxed = nanbox_pointer_inline(ctx.block(), &id); - crate::expr::temp_root::temp_root_release(ctx, guard); - return Ok(Some(boxed)); - } - let n = args.len() - 2; - let buf = ctx.func.alloca_entry_array(DOUBLE, n); - for (i, v) in vals.iter().skip(2).enumerate() { - let blk = ctx.block(); - let slot = blk.gep(DOUBLE, &buf, &[(I64, &format!("{}", i))]); - blk.store(DOUBLE, v, &slot); - } - let ptr_reg = ctx.block().next_reg(); - ctx.block().emit_raw(format!( - "{} = getelementptr [{} x double], ptr {}, i64 0, i64 0", - ptr_reg, n, buf - )); - let blk = ctx.block(); - let cb_handle = unbox_to_i64(blk, &cb_box); - let id = blk.call( - I64, - "js_set_interval_callback_args", - &[ - (I64, &cb_handle), - (DOUBLE, &delay_box), - (PTR, &ptr_reg), - (I32, &n.to_string()), - ], - ); - let boxed = nanbox_pointer_inline(ctx.block(), &id); - crate::expr::temp_root::temp_root_release(ctx, guard); + Ok(nanbox_pointer_inline(blk, &id)) + })?; return Ok(Some(boxed)); } "setImmediate" if !args.is_empty() => { + // One argument: the callback is consumed by the very next + // emission, so there is no window to protect. Counted in + // emissions, not source lines. if args.len() == 1 { let cb_box = lower_expr(ctx, &args[0])?; let blk = ctx.block(); @@ -158,29 +187,19 @@ pub fn try_lower_namespace_member_call( return Ok(Some(nanbox_pointer_inline(blk, &id))); } let arg_refs: Vec<&Expr> = args.iter().collect(); - let (vals, guard) = crate::expr::temp_root::lower_exprs_rooted(ctx, &arg_refs)?; - let cb_box = vals[0].clone(); - let n = args.len() - 1; - let buf = ctx.func.alloca_entry_array(DOUBLE, n); - for (i, v) in vals.iter().skip(1).enumerate() { + let boxed = with_operands_rooted(ctx, &arg_refs, |ctx, vals| { + let cb_box = vals[0].clone(); + let n = vals.len() - 1; + let ptr_reg = fill_arg_buffer(ctx, &vals[1..]); let blk = ctx.block(); - let slot = blk.gep(DOUBLE, &buf, &[(I64, &format!("{}", i))]); - blk.store(DOUBLE, v, &slot); - } - let ptr_reg = ctx.block().next_reg(); - ctx.block().emit_raw(format!( - "{} = getelementptr [{} x double], ptr {}, i64 0, i64 0", - ptr_reg, n, buf - )); - let blk = ctx.block(); - let cb_handle = unbox_to_i64(blk, &cb_box); - let id = blk.call( - I64, - "js_set_immediate_callback_args", - &[(I64, &cb_handle), (PTR, &ptr_reg), (I32, &n.to_string())], - ); - let boxed = nanbox_pointer_inline(ctx.block(), &id); - crate::expr::temp_root::temp_root_release(ctx, guard); + let cb_handle = unbox_to_i64(blk, &cb_box); + let id = blk.call( + I64, + "js_set_immediate_callback_args", + &[(I64, &cb_handle), (PTR, &ptr_reg), (I32, &n.to_string())], + ); + Ok(nanbox_pointer_inline(blk, &id)) + })?; return Ok(Some(boxed)); } "clearTimeout" | "clearInterval" | "clearImmediate" if !args.is_empty() => { @@ -204,56 +223,31 @@ pub fn try_lower_namespace_member_call( .get(ns_name) .is_some_and(|submod| submod == "fs/promises") { - match property.as_str() { - "writeFile" if args.len() >= 2 => { - let path = lower_expr(ctx, &args[0])?; - let content = lower_expr(ctx, &args[1])?; - let options = if args.len() >= 3 { - lower_expr(ctx, &args[2])? - } else { - double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)) - }; - let promise = ctx.block().call( - DOUBLE, - "js_fs_promises_write_file", - &[(DOUBLE, &path), (DOUBLE, &content), (DOUBLE, &options)], - ); - return Ok(Some(promise)); - } - "appendFile" if args.len() >= 2 => { - let path = lower_expr(ctx, &args[0])?; - let content = lower_expr(ctx, &args[1])?; - let options = if args.len() >= 3 { - lower_expr(ctx, &args[2])? - } else { - double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)) - }; - let promise = ctx.block().call( - DOUBLE, - "js_fs_promises_append_file", - &[(DOUBLE, &path), (DOUBLE, &content), (DOUBLE, &options)], - ); - return Ok(Some(promise)); - } - "rmdir" => { - let path = if let Some(path) = args.first() { - lower_expr(ctx, path)? - } else { - double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)) - }; - let options = if args.len() >= 2 { - lower_expr(ctx, &args[1])? - } else { - double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)) - }; - let promise = ctx.block().call( - DOUBLE, - "js_fs_promises_rmdir", - &[(DOUBLE, &path), (DOUBLE, &options)], - ); - return Ok(Some(promise)); - } - _ => {} + // Each of these lowered operand 0 into a bare register and then lowered + // operands 1 and 2 — arbitrary user code — before the consuming call + // read it. `fs.writeFile(namePath(), await body(), opts())` is the + // shape. Routing the whole list through the API closes every + // operand-to-operand window at once, and costs nothing when the later + // operands provably cannot collect (`OperandProtection::Reuse`), which + // is the `writeFile("out.txt", data)` case. + let fs_promises_helper = match property.as_str() { + "writeFile" if args.len() >= 2 => Some(("js_fs_promises_write_file", 3usize)), + "appendFile" if args.len() >= 2 => Some(("js_fs_promises_append_file", 3)), + "rmdir" => Some(("js_fs_promises_rmdir", 2)), + _ => None, + }; + if let Some((helper, arity)) = fs_promises_helper { + // Only the operands the user actually wrote are lowered; the rest + // are `undefined` literals, exactly as before. + let arg_refs: Vec<&Expr> = args.iter().take(arity).collect(); + let promise = with_operands_rooted(ctx, &arg_refs, |ctx, vals| { + let mut passed: Vec = vals.to_vec(); + passed.resize(arity, undefined_lit()); + let slices: Vec<(crate::types::LlvmType, &str)> = + passed.iter().map(|v| (DOUBLE, v.as_str())).collect(); + Ok(ctx.block().call(DOUBLE, helper, &slices)) + })?; + return Ok(Some(promise)); } } // Issue #678 followup (namespace branch): wildcard-namespace @@ -267,13 +261,9 @@ pub fn try_lower_namespace_member_call( // jose / effect wildcard members fell to the // `double_literal(0.0)` stub. if let Some(specifier) = ctx.namespace_v8_specifiers.get(ns_name).cloned() { - let mut lowered: Vec = Vec::with_capacity(args.len()); - for a in args { - lowered.push(lower_expr(ctx, a)?); - } - return Ok(Some(crate::expr::emit_v8_export_call( - ctx, &specifier, property, &lowered, - ))); + return Ok(Some(emit_v8_export_call_rooted( + ctx, &specifier, property, args, + )?)); } // Issue #680: prefer the per-namespace map so // `random.make` and `tracer.make` resolve to their own @@ -295,13 +285,9 @@ pub fn try_lower_namespace_member_call( // runtime bridge — no `perry_fn___` symbol // exists for the linker to bind to. if let Some(specifier) = ctx.import_function_v8_specifiers.get(property).cloned() { - let mut lowered: Vec = Vec::with_capacity(args.len()); - for a in args { - lowered.push(lower_expr(ctx, a)?); - } - return Ok(Some(crate::expr::emit_v8_export_call( - ctx, &specifier, property, &lowered, - ))); + return Ok(Some(emit_v8_export_call_rooted( + ctx, &specifier, property, args, + )?)); } // Issue #678/#5924: re-exported names (e.g. `export { default as // render }`) emit `perry_fn___default` in the origin — @@ -319,26 +305,61 @@ pub fn try_lower_namespace_member_call( if ctx.imported_vars.contains(property) { // Var-shaped export: fetch closure via zero-arg // getter, then closure-call with the user args. - ctx.pending_declares.push((symbol.clone(), DOUBLE, vec![])); - let closure_box = ctx.block().call(DOUBLE, &symbol, &[]); - let mut lowered: Vec = Vec::with_capacity(args.len()); - for a in args { - lowered.push(lower_expr(ctx, a)?); - } - if lowered.len() > 16 { + if args.len() > 16 { bail!( "perry-codegen: namespace closure call with {} args (max 16)", - lowered.len() + args.len() ); } - let blk = ctx.block(); - let closure_handle = unbox_to_i64(blk, &closure_box); - let runtime_fn = format!("js_closure_call{}", lowered.len()); - let mut call_args: Vec<(crate::types::LlvmType, &str)> = vec![(I64, &closure_handle)]; - for v in &lowered { - call_args.push((DOUBLE, v.as_str())); - } - return Ok(Some(blk.call(DOUBLE, &runtime_fn, &call_args))); + ctx.pending_declares.push((symbol.clone(), DOUBLE, vec![])); + // The getter runs FIRST and must keep running first: it is the callee + // reference, and the spec evaluates that before the arguments. Sinking + // it below them would read a value an argument had reassigned, which is + // a miscompile rather than a rooting fix. So the closure is the thing + // that has to survive the window, not the thing that can be re-derived. + let closure_box = ctx.block().call(DOUBLE, &symbol, &[]); + let arg_refs: Vec<&Expr> = args.iter().collect(); + // Rooted as `Boxed`, not `Ptr`, and that is a correctness choice rather + // than a stylistic one. `unbox_to_i64` masks the tag off unconditionally, + // so on a namespace export that is NOT callable the masked word is not an + // address at all — pushing it as `Repr::Ptr` would publish garbage into a + // slot the collector *traces*. Rooting the NaN-boxed value keeps the tag, + // which is what the scanner reads. The unbox then happens in `finish`, + // below the window; it is a bitcast and a mask, emits no call, and so + // cannot itself collect. + let lowered_args = std::cell::RefCell::new(Vec::::new()); + let result = crate::rooting::with_rooted_accumulator( + ctx, + crate::rooting::Repr::Boxed, + &closure_box, + crate::rooting::any_operand_may_collect(ctx, args.iter()), + |ctx, _closure| { + // The arguments are lowered INSIDE the closure's protected + // window and rooted against each other by the inner group. + // `temp_root_truncate` is a stack CUT, and the inner group sits + // strictly above the closure's own slot, so releasing it leaves + // the closure rooted for `finish`. + with_operands_rooted(ctx, &arg_refs, |_ctx, vals| { + *lowered_args.borrow_mut() = vals.to_vec(); + Ok(()) + }) + }, + |ctx, closure_box| { + // Below every collection point: the inner group's re-reads, then + // the accumulator's own re-read, then nothing that allocates. + let lowered = lowered_args.borrow(); + let blk = ctx.block(); + let closure_handle = unbox_to_i64(blk, closure_box); + let runtime_fn = format!("js_closure_call{}", lowered.len()); + let mut call_args: Vec<(crate::types::LlvmType, &str)> = + vec![(I64, &closure_handle)]; + for v in lowered.iter() { + call_args.push((DOUBLE, v.as_str())); + } + Ok(blk.call(DOUBLE, &runtime_fn, &call_args)) + }, + )?; + return Ok(Some(result)); } // Function-decl-shaped export: direct call with rest bundling. let declared_count = ctx @@ -347,37 +368,73 @@ pub fn try_lower_namespace_member_call( .copied() .unwrap_or(args.len()); let has_rest = ctx.imported_func_has_rest.contains(property); - let mut lowered: Vec = Vec::with_capacity(declared_count); if has_rest { + // #7154's accumulator shape, verbatim: `current` was a raw + // `*mut ArrayHeader` in a bare SSA register holding the only reference + // to every argument pushed so far, while the NEXT argument's expression + // — arbitrary user code — was lowered. The fixed parameters were + // unprotected across the same push loop. + // + // `super::lower_rest_call_args_rooted` was written for exactly this and + // this path never adopted it. Delegating is the same posture as calling + // `lower_expr`: the helper's contract is documented and audited, and no + // ordering decision is made here. It also pads the fixed parameters to + // the declared arity, which the hand-rolled loop did not — a call with + // fewer arguments than the callee declares used to emit a call of the + // wrong arity. let fixed_count = declared_count.saturating_sub(1); - for a in args.iter().take(fixed_count) { - lowered.push(lower_expr(ctx, a)?); - } - let rest_count = args.len().saturating_sub(fixed_count); - let cap = (rest_count as u32).to_string(); - let mut current = ctx.block().call(I64, "js_array_alloc", &[(I32, &cap)]); - for a in args.iter().skip(fixed_count) { - let v = lower_expr(ctx, a)?; - let blk = ctx.block(); - current = blk.call(I64, "js_array_push_f64", &[(I64, ¤t), (DOUBLE, &v)]); - } - let rest_box = nanbox_pointer_inline(ctx.block(), ¤t); - lowered.push(rest_box); - } else { - for a in args { - lowered.push(lower_expr(ctx, a)?); - } - // Pad missing trailing args with TAG_UNDEFINED. - let undef_lit = double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)); - while lowered.len() < declared_count { - lowered.push(undef_lit.clone()); - } + let (lowered, guard) = super::lower_rest_call_args_rooted( + ctx, + args, + fixed_count, + &[super::RestBundle { + from: fixed_count, + mark_arguments_object: false, + }], + )?; + let arg_types: Vec = + std::iter::repeat_n(DOUBLE, lowered.len()).collect(); + ctx.pending_declares + .push((symbol.clone(), DOUBLE, arg_types)); + return Ok(Some(super::emit_rooted_call(ctx, &symbol, &lowered, guard))); } - let arg_types: Vec = - std::iter::repeat_n(DOUBLE, lowered.len()).collect(); - ctx.pending_declares - .push((symbol.clone(), DOUBLE, arg_types)); - let arg_slices: Vec<(crate::types::LlvmType, &str)> = - lowered.iter().map(|s| (DOUBLE, s.as_str())).collect(); - Ok(Some(ctx.block().call(DOUBLE, &symbol, &arg_slices))) + // The plain argument loop had no protection at all — #7240's shape, in a + // path that post-dates the fix. Zero cost when nothing in the list can + // collect. + let arg_refs: Vec<&Expr> = args.iter().collect(); + let call = with_operands_rooted(ctx, &arg_refs, |ctx, vals| { + let mut lowered: Vec = vals.to_vec(); + // Pad missing trailing args with TAG_UNDEFINED. + lowered.resize(lowered.len().max(declared_count), undefined_lit()); + let arg_types: Vec = + std::iter::repeat_n(DOUBLE, lowered.len()).collect(); + ctx.pending_declares + .push((symbol.clone(), DOUBLE, arg_types)); + let arg_slices: Vec<(crate::types::LlvmType, &str)> = + lowered.iter().map(|s| (DOUBLE, s.as_str())).collect(); + Ok(ctx.block().call(DOUBLE, &symbol, &arg_slices)) + })?; + Ok(Some(call)) +} + +/// Lower a V8-bridge export call's arguments with each one rooted across the +/// evaluation of the ones that follow, then emit the bridge call. +/// +/// Both bridge arms used a bare `for a in args { lowered.push(lower_expr(a)?) }` +/// — argument 0 sits in an SSA register while arguments 1..n run arbitrary user +/// code, and `emit_v8_export_call` marshals them all afterwards. That is #7240's +/// shape; the two arms differ only in which map resolved the specifier, so the +/// fix is one function rather than two edits. +fn emit_v8_export_call_rooted( + ctx: &mut FnCtx<'_>, + specifier: &str, + property: &str, + args: &[Expr], +) -> Result { + let arg_refs: Vec<&Expr> = args.iter().collect(); + with_operands_rooted(ctx, &arg_refs, |ctx, vals| { + Ok(crate::expr::emit_v8_export_call( + ctx, specifier, property, vals, + )) + }) } diff --git a/crates/perry-codegen/src/lower_call/timer_rooting_tests.rs b/crates/perry-codegen/src/lower_call/timer_rooting_tests.rs new file mode 100644 index 0000000000..b59e023b77 --- /dev/null +++ b/crates/perry-codegen/src/lower_call/timer_rooting_tests.rs @@ -0,0 +1,221 @@ +//! Rooting coverage for the two-argument `setTimeout` / `setInterval` arms +//! repaired by slice 5 of the Layer 1 migration (#7615). +//! +//! # Why a unit test rather than a gap test +//! +//! The bug needs three things to fault at runtime: a **capturing** callback (a +//! non-capturing arrow lowers to `js_closure_alloc_singleton`, which is never +//! moved, so the stale register keeps working), a delay expression that reaches +//! a back-edge poll, and `PERRY_GC_MOVING_LOOP_POLLS=1` at *compile* time — off +//! by default since #7161. A gap test would therefore be green on the default +//! build regardless of whether the fix is present, which is CLAUDE.md hazard 4: +//! a gate whose subject never ran. +//! +//! These assert on the emitted IR instead, where the property is unconditional. +//! +//! # What is asserted, and why it cannot pass vacuously +//! +//! Not a slot COUNT. Counting root slots across two programs that differ in an +//! operand would let the delay expression's own rooting pay for the assertion — +//! the test would stay green with the repair deleted. What is asserted is the +//! ordering that IS the bug: +//! +//! * **hot** (an allocating delay): the register `js_timer_validate_callback` +//! reads must be defined **below** the delay's last allocation. That can only +//! happen if the callback was rooted above the window and re-read below it. +//! With the repair removed the callback's register is defined above the +//! delay and the assertion fails. +//! * **cold** (a literal delay): the same register must be defined **above** +//! the consuming call with **no** temp-root traffic anywhere in the module. +//! This pins the zero-cost claim — `operand_protection` routes a +//! non-collecting window to `Reuse` — so a future change that roots +//! unconditionally turns this red rather than silently taxing every +//! `setTimeout(fn, 100)` in every program. +//! +//! Both arms first assert the arm under test was actually reached, by callee +//! name, so an IR shape measured over a lowering that never ran cannot pass. + +use perry_hir::types::Type; +use perry_hir::{Expr, Function, Module as HirModule, Stmt}; + +/// Compile a one-function module and return its LLVM IR. +fn compile_body(name: &str, body: Vec) -> String { + let mut hir = HirModule::new(name); + hir.functions.push(Function { + id: 0, + name: "build".to_string(), + type_params: Vec::new(), + params: Vec::new(), + return_type: Type::Any, + body, + is_async: false, + is_generator: false, + is_strict: true, + is_exported: false, + captures: Vec::new(), + decorators: Vec::new(), + was_plain_async: false, + was_unrolled: false, + }); + let opts = crate::CompileOptions { + emit_ir_only: true, + ..Default::default() + }; + let bytes = crate::compile_module(&hir, opts).expect("test module compiles"); + String::from_utf8(bytes).expect("LLVM IR is UTF-8") +} + +/// `timer(callback, delay)` as an `ExternFuncRef` call — the shape +/// `try_lower_extern_timer_call` dispatches on. +/// +/// The callback is an object literal rather than a closure on purpose: codegen +/// does not type-check it, an object literal is a heap value that +/// `operand_needs_root` protects exactly as a capturing closure is, and it keeps +/// the HIR small enough to read. +fn timer_call(timer: &str, delay: Expr) -> Vec { + vec![Stmt::Expr(Expr::Call { + callee: Box::new(Expr::ExternFuncRef { + name: timer.to_string(), + param_types: vec![Type::Any, Type::Number], + return_type: Type::Any, + }), + args: vec![ + Expr::Object(vec![("cb".to_string(), Expr::Number(1.0))]), + delay, + ], + type_args: Vec::new(), + byte_offset: 0, + })] +} + +/// A delay that allocates (so the window collects), and one that cannot. +fn allocating_delay() -> Expr { + Expr::Object(vec![("d".to_string(), Expr::Number(1.0))]) +} + +fn inert_delay() -> Expr { + Expr::Number(5.0) +} + +/// Line index of the `js_timer_validate_callback` **call**, or `None`. +/// +/// Excluding `declare` is load-bearing rather than tidy: the module always +/// carries `declare i64 @js_timer_validate_callback(double, i32)` whether or not +/// anything calls it, so a liveness check that accepted it would be satisfied by +/// a lowering that never ran — hazard 4 wearing the subject's name. +fn validate_call_line(ir: &str) -> Option { + ir.lines().position(|l| { + l.contains("@js_timer_validate_callback(") && !l.trim_start().starts_with("declare") + }) +} + +/// The SSA register `js_timer_validate_callback` actually reads. +fn callback_operand(ir: &str) -> String { + let idx = validate_call_line(ir).expect("no js_timer_validate_callback call"); + let line = ir.lines().nth(idx).expect("line index came from this IR"); + let after = line + .split("(double ") + .nth(1) + .expect("js_timer_validate_callback takes a double first argument"); + after + .split([',', ')']) + .next() + .expect("the first argument is comma- or paren-terminated") + .trim() + .to_string() +} + +/// Line index at which `reg` is defined. +fn definition_line(ir: &str, reg: &str) -> usize { + let prefix = format!("{reg} = "); + ir.lines() + .position(|l| l.trim_start().starts_with(&prefix)) + .unwrap_or_else(|| panic!("no definition for {reg} in:\n{ir}")) +} + +/// Line index of the LAST occurrence of `needle`. +fn last_line_containing(ir: &str, needle: &str) -> usize { + ir.lines() + .enumerate() + .filter(|(_, l)| l.contains(needle)) + .map(|(i, _)| i) + .last() + .unwrap_or_else(|| panic!("no line containing {needle} in:\n{ir}")) +} + +/// An allocating delay must leave the callback re-read BELOW the allocation. +fn assert_callback_survives_an_allocating_delay(timer: &str) { + let ir = compile_body( + &format!("{timer}_hot"), + timer_call(timer, allocating_delay()), + ); + + // Subject liveness first: an IR shape measured over a lowering that never + // ran proves nothing (CLAUDE.md hazard 4). + assert!( + validate_call_line(&ir).is_some(), + "{timer}: the two-argument arm was not reached:\n{ir}" + ); + + let operand = callback_operand(&ir); + let operand_def = definition_line(&ir, &operand); + // The delay is the SECOND object literal, so its allocation is the last one + // emitted before the timer call. + let delay_alloc = last_line_containing(&ir, "@js_object_alloc"); + assert!( + operand_def > delay_alloc, + "{timer}: the callback register {operand} is defined at line {operand_def}, ABOVE the \ + delay's allocation at line {delay_alloc}. That is the unrooted window: an evacuating \ + minor inside the delay relocates the callback and js_timer_validate_callback then reads \ + from-space. The callback must be rooted above the window and re-read below it.\n{ir}" + ); +} + +/// A literal delay cannot collect, so the repair must emit nothing at all. +fn assert_inert_delay_costs_nothing(timer: &str) { + let ir = compile_body(&format!("{timer}_cold"), timer_call(timer, inert_delay())); + + let validate = validate_call_line(&ir) + .unwrap_or_else(|| panic!("{timer}: the two-argument arm was not reached:\n{ir}")); + + let operand = callback_operand(&ir); + let operand_def = definition_line(&ir, &operand); + assert!( + operand_def < validate, + "{timer}: the callback operand must be the register the callback was lowered into" + ); + + // Temp-root traffic only ever appears as a call; `declare` lines name the + // helpers whether or not anything calls them, so they are excluded. + let pushes = ir + .lines() + .filter(|l| !l.trim_start().starts_with("declare")) + .filter(|l| l.contains("js_gc_temp_root")) + .count(); + assert_eq!( + pushes, 0, + "{timer}: a literal delay cannot collect, so operand_protection must route the callback \ + to Reuse and emit no temp-root traffic. Rooting it unconditionally taxes every \ + `{timer}(fn, 100)` in every program.\n{ir}" + ); +} + +#[test] +fn set_timeout_two_arg_roots_the_callback_across_an_allocating_delay() { + assert_callback_survives_an_allocating_delay("setTimeout"); +} + +#[test] +fn set_interval_two_arg_roots_the_callback_across_an_allocating_delay() { + assert_callback_survives_an_allocating_delay("setInterval"); +} + +#[test] +fn set_timeout_two_arg_costs_nothing_when_the_delay_is_a_literal() { + assert_inert_delay_costs_nothing("setTimeout"); +} + +#[test] +fn set_interval_two_arg_costs_nothing_when_the_delay_is_a_literal() { + assert_inert_delay_costs_nothing("setInterval"); +} diff --git a/crates/perry-codegen/src/rooting.rs b/crates/perry-codegen/src/rooting.rs index f526217b36..bbadfd9522 100644 --- a/crates/perry-codegen/src/rooting.rs +++ b/crates/perry-codegen/src/rooting.rs @@ -845,6 +845,41 @@ pub(crate) fn with_rooted_accumulator<'f, R>( /// the module cannot make an ORDERING mistake against the raw API, because it /// no longer names it. A window with no decision at all is invisible to this /// check, and the only instrument for it is reading the module. +/// +/// Slice 5 is two modules of the `lower_call/` family, both load-bearing: +/// `extern_timers.rs` and `namespace_call.rs` each named +/// `lower_exprs_rooted` / `temp_root_release` before the migration, so their +/// lines hold on the committed source and not only under sabotage. +/// +/// It is two rather than the six the family contains, and the four left out are +/// left out for one reason worth recording, because it is a **statement about +/// this API rather than about those files**: three of them need a re-read at +/// more than one point, and every `with_operands_rooted*` form has exactly one. +/// +/// * `lower_call/mod.rs` — `lower_call_args_rooted` and +/// `lower_rest_call_args_rooted` return a guard *deliberately*: their +/// consumers in `func_ref.rs` are block-splitting specialized-ABI diamonds, +/// so the release must sit in a merge block that post-dominates four +/// dispatch paths, ~200 lines below the lowering. A closure form can express +/// that only by swallowing the whole dispatch chain, in a file outside the +/// slice. +/// * `lower_call/new.rs` — `refresh_rooted_args` re-reads the SAME operand +/// group at three caller-chosen points (after the instance allocation, +/// before the field initializers, before an inlined constructor body), under +/// a `temp_root_scope_begin`/`_end` marker spanning ~20 return paths. +/// * `lower_call/console_promise.rs` — `lower_dynamic_closure_call` re-reads +/// the receiver and callee below the arguments, then re-reads the arguments +/// again below the allocating rebind unbox. Two stages, one combinator. +/// * `lower_call/early_branches.rs` — its only escape-hatch uses are +/// `implicit_this_save`/`implicit_this_restore`, which is already a paired +/// combinator rather than the raw ordering API. Migrating it means +/// re-exporting that pair through `crate::rooting`, which is a rename that +/// would make the ledger line look substantive while asserting nothing new. +/// +/// The honest reading: a module that cannot be migrated because the API cannot +/// say what it means is a gap in the API, and recording it here is what stops +/// the next slice from rediscovering it. The variadic/rest shape (per-element +/// re-reads between allocating pushes) is the concrete missing combinator. #[cfg(test)] const MIGRATED_MODULES: &[(&str, &str)] = &[ ( @@ -927,6 +962,14 @@ const MIGRATED_MODULES: &[(&str, &str)] = &[ "crates/perry-codegen/src/expr/property_set.rs", include_str!("expr/property_set.rs"), ), + ( + "crates/perry-codegen/src/lower_call/extern_timers.rs", + include_str!("lower_call/extern_timers.rs"), + ), + ( + "crates/perry-codegen/src/lower_call/namespace_call.rs", + include_str!("lower_call/namespace_call.rs"), + ), ]; /// Lines in `src` that reach past [`crate::rooting`] into the raw rooting API.