From 22018e02016de03aaf6f0a2f92e1b22e2112acd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 2 Aug 2026 06:02:08 +0200 Subject: [PATCH 1/4] fix(gc): root the callback and every staged argument of the timer family MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `setTimeout(cb, 0, {…}, churn())` had two unrooted windows, and the callback's is the one that crashes. `cb_box` was lowered first and read at `js_timer_validate_callback` — after the delay's `lower_expr` and after every trailing argument's. A freshly-allocated closure therefore sat in a bare SSA register across a user call carrying loop back-edge polls, and the moving minor inside it left the register naming from-space: TypeError [ERR_INVALID_ARG_TYPE]: The "callback" argument must be of type function. Received an instance of Object deterministically, 3/3 at base under `PERRY_GC_MOVING_LOOP_POLLS=1 PERRY_GC_HEAP_LIMIT=8`, and identically on the `evac_minor` arm. The trailing-argument staging buffer is the second window and is worse in kind. Argument *i* sits in a bare `alloca_entry_array` — storage the precise root walk never visits — while argument *i+1* is lowered. That is not staleness: at the moment the collection runs, nothing anywhere refers to the object, so it is a premature SWEEP. Both close with one change: lower the whole argument list through `expr::temp_root::lower_exprs_rooted` and fill the buffer in a second, lowering-free pass. The values the stores see are re-read below the last lowering, so they are post-collection addresses; the guard is released only after the consuming call, which reads the buffer. Cost is zero when nothing in the list can collect (`OperandProtection::Reuse`) — the `setTimeout(fn, 0, someLocal)` case emits byte-identical IR. Covered: `setTimeout`/`setInterval`/`setImmediate` (global forms) and their `timers`-namespace siblings, plus `process.nextTick`. The namespace forms carried a third window of their own: `cb_handle` was `unbox_to_i64`'d — a RAW heap address, not even NaN-boxed — before the trailing arguments were lowered; the unbox now happens below them. `extern_func.rs` crossed the 2000-line cap, so the timer arms move to a new `lower_call/extern_timers.rs` (pure mechanical move; dispatched immediately above the match so arm order is unchanged). Witness `test-files/test_gap_gc_staging_args_rooting.ts`, registered in `test-parity/gc_repsel_corpus.txt`: red 3/3 at base, `bad 0` 5/5 after, clean 3/3 on the shipped default, byte-exact against node 26.5.1. Refs #7210, #7154, #6951, #7161. --- .../perry-codegen/src/expr/instance_misc1.rs | 19 +- .../src/lower_call/extern_func.rs | 219 +------------- .../src/lower_call/extern_timers.rs | 275 ++++++++++++++++++ crates/perry-codegen/src/lower_call/mod.rs | 1 + .../src/lower_call/namespace_call.rs | 75 +++-- .../test_gap_gc_staging_args_rooting.ts | 79 +++++ test-parity/gc_repsel_corpus.txt | 11 + 7 files changed, 443 insertions(+), 236 deletions(-) create mode 100644 crates/perry-codegen/src/lower_call/extern_timers.rs create mode 100644 test-files/test_gap_gc_staging_args_rooting.ts diff --git a/crates/perry-codegen/src/expr/instance_misc1.rs b/crates/perry-codegen/src/expr/instance_misc1.rs index c68ca6b52d..7c6d179058 100644 --- a/crates/perry-codegen/src/expr/instance_misc1.rs +++ b/crates/perry-codegen/src/expr/instance_misc1.rs @@ -1015,8 +1015,8 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // the varargs runtime entry; the no-args form goes through the // simpler `js_queue_next_tick` to avoid the alloca cost. Expr::ProcessNextTick { callback, args } => { - let cb_box = lower_expr(ctx, callback)?; if args.is_empty() { + let cb_box = lower_expr(ctx, callback)?; let blk = ctx.block(); // #3046: validate the callback (non-callable → Node's // `ERR_INVALID_ARG_TYPE` "callback" message) before queueing. @@ -1031,13 +1031,23 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { blk.call_void("js_queue_next_tick", &[(I64, &cb_handle)]); return Ok(double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED))); } + // #7210: the callback and every trailing argument go through + // `lower_exprs_rooted` together. Two windows closed at once — the + // callback register, held across every argument's `lower_expr`, and + // the staging buffer, in which argument *i* has no root at all + // while argument *i+1* is lowered. See `lower_call/extern_func.rs`'s + // `setTimeout` arm for the full argument. let n = args.len(); + let mut arg_refs: Vec<&Expr> = Vec::with_capacity(n + 1); + arg_refs.push(callback); + arg_refs.extend(args.iter()); + let (vals, guard) = super::temp_root::lower_exprs_rooted(ctx, &arg_refs)?; + let cb_box = vals[0].clone(); let buf = ctx.func.alloca_entry_array(DOUBLE, n); - for (i, a) in args.iter().enumerate() { - let v = lower_expr(ctx, a)?; + for (i, v) in vals.iter().skip(1).enumerate() { let blk = ctx.block(); let slot = blk.gep(DOUBLE, &buf, &[(I64, &format!("{}", i))]); - blk.store(DOUBLE, &v, &slot); + blk.store(DOUBLE, v, &slot); } let ptr_reg = ctx.block().next_reg(); ctx.block().emit_raw(format!( @@ -1055,6 +1065,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { "js_queue_next_tick_args", &[(I64, &cb_handle), (PTR, &ptr_reg), (I32, &n.to_string())], ); + super::temp_root::temp_root_release(ctx, guard); Ok(double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED))) } diff --git a/crates/perry-codegen/src/lower_call/extern_func.rs b/crates/perry-codegen/src/lower_call/extern_func.rs index f38173d875..7a0edd2d1b 100644 --- a/crates/perry-codegen/src/lower_call/extern_func.rs +++ b/crates/perry-codegen/src/lower_call/extern_func.rs @@ -1095,214 +1095,19 @@ pub fn try_lower_extern_func_call( if ctx.import_function_node_submodule.contains_key(name) { return Ok(None); } + // Timers (`setTimeout`/`setInterval`/`setImmediate` and their `clear*` + // siblings) live in `extern_timers.rs`. Split out under #7210, when the GC + // rooting fix for their trailing-argument staging buffers pushed this file + // over the 2000-line cap (`scripts/check_file_size.sh`). Dispatched HERE, + // immediately above the match, because they were its first arms: arm order + // is observable (`"setTimeout" if args.len() == 1` must still beat the + // generic consumer-prefix path at the bottom), and every guard they carry + // is on `args.len()`, so a non-matching timer name or arity falls through + // to exactly the arm it fell through to before. + if let Some(v) = super::extern_timers::try_lower_extern_timer_call(ctx, name.as_str(), args)? { + return Ok(Some(v)); + } match name.as_str() { - // #1671: `setTimeout(fn)` with no explicit delay. Node treats a - // missing/undefined delay as 0 (fires on the next timer tick). - // Without this arm a 1-arg `setTimeout` falls through to the - // catch-all below, which emits a bare LLVM call to `@setTimeout` - // and the linker fails with `Undefined symbols: _setTimeout` - // (hit by hono/jsx's `hooks/index.js`, which schedules a re-render - // via `setTimeout(() => { … })`). Route it to the same runtime - // entry as the 2-arg form with a zero delay. - "setTimeout" if args.len() == 1 => { - let cb_box = lower_expr(ctx, &args[0])?; - let blk = ctx.block(); - // #2013 — validate the callback type before unboxing the - // pointer. `js_timer_validate_callback` throws - // ERR_INVALID_ARG_TYPE for any non-callable value and - // returns the raw closure pointer otherwise; the second - // arg `0` is the type-name index for "setTimeout". - let zero_idx = "0"; - let cb_handle = blk.call( - I64, - "js_timer_validate_callback", - &[(DOUBLE, &cb_box), (I32, zero_idx)], - ); - let zero = double_literal(0.0); - let id = blk.call( - I64, - "js_set_timeout_callback", - &[(I64, &cb_handle), (DOUBLE, &zero)], - ); - return Ok(Some(nanbox_pointer_inline(blk, &id))); - } - "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))); - } - "setImmediate" if !args.is_empty() => { - let cb_box = lower_expr(ctx, &args[0])?; - if args.len() == 1 { - 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", &[(I64, &cb_handle)]); - return Ok(Some(nanbox_pointer_inline(blk, &id))); - } - - let n = args.len() - 1; - let buf = ctx.func.alloca_entry_array(DOUBLE, n); - for (i, a) in args.iter().skip(1).enumerate() { - let v = lower_expr(ctx, a)?; - 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())], - ); - return Ok(Some(nanbox_pointer_inline(blk, &id))); - } - // Refs #665: `setTimeout(fn, delay, ...args)` — JS spec forwards - // the trailing args to `fn` when the timer fires. Pack them into - // a stack buffer of doubles and hand off to the varargs runtime - // entry. Used by Promise-executor patterns like - // `setTimeout(resolve, delay, res)` (rate-limiter-flexible's - // `RateLimiterMemory.consume` is the discovering call site). - "setTimeout" if args.len() >= 3 => { - let cb_box = lower_expr(ctx, &args[0])?; - let delay_box = lower_expr(ctx, &args[1])?; - let n = args.len() - 2; - let buf = ctx.func.alloca_entry_array(DOUBLE, n); - for (i, a) in args.iter().skip(2).enumerate() { - let v = lower_expr(ctx, a)?; - 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()), - ], - ); - return Ok(Some(nanbox_pointer_inline(blk, &id))); - } - "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))); - } - "setInterval" if args.len() >= 3 => { - let cb_box = lower_expr(ctx, &args[0])?; - let delay_box = lower_expr(ctx, &args[1])?; - let n = args.len() - 2; - let buf = ctx.func.alloca_entry_array(DOUBLE, n); - for (i, a) in args.iter().skip(2).enumerate() { - let v = lower_expr(ctx, a)?; - 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()), - ], - ); - return Ok(Some(nanbox_pointer_inline(blk, &id))); - } - "clearTimeout" if args.len() == 1 => { - // Pass the raw NaN-boxed arg so the runtime accepts both the - // handle and its primitive numeric id (`clearTimeout(+t)`, #1213). - let id_box = lower_expr(ctx, &args[0])?; - ctx.block() - .call_void("js_clear_timeout_value", &[(DOUBLE, &id_box)]); - return Ok(Some(double_literal(f64::from_bits( - crate::nanbox::TAG_UNDEFINED, - )))); - } - "clearInterval" if args.len() == 1 => { - let id_box = lower_expr(ctx, &args[0])?; - ctx.block() - .call_void("js_clear_interval_value", &[(DOUBLE, &id_box)]); - return Ok(Some(double_literal(f64::from_bits( - crate::nanbox::TAG_UNDEFINED, - )))); - } - "clearImmediate" if args.len() == 1 => { - let id_box = lower_expr(ctx, &args[0])?; - ctx.block() - .call_void("js_clear_immediate_value", &[(DOUBLE, &id_box)]); - return Ok(Some(double_literal(f64::from_bits( - crate::nanbox::TAG_UNDEFINED, - )))); - } "gc" => { ctx.block().call_void("js_gc_collect", &[]); return Ok(Some(double_literal(f64::from_bits( diff --git a/crates/perry-codegen/src/lower_call/extern_timers.rs b/crates/perry-codegen/src/lower_call/extern_timers.rs new file mode 100644 index 0000000000..a157a31034 --- /dev/null +++ b/crates/perry-codegen/src/lower_call/extern_timers.rs @@ -0,0 +1,275 @@ +//! Timer lowerings for `Expr::ExternFuncRef` — `setTimeout`, `setInterval`, +//! `setImmediate` and their `clear*` siblings. +//! +//! Split out of `extern_func.rs` (#7210) when that file crossed the 2000-line +//! cap. Pure mechanical move: every arm body below is a verbatim copy of the +//! arm it replaced, reached from `try_lower_extern_func_call`'s dispatch. +//! +//! 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. + +use anyhow::Result; +use perry_hir::Expr; + +use crate::expr::{lower_expr, nanbox_pointer_inline, FnCtx}; +use crate::nanbox::double_literal; +use crate::types::{DOUBLE, I32, I64, PTR}; + +/// Lower a timer builtin, or `Ok(None)` if `name` is not one. +pub fn try_lower_extern_timer_call( + ctx: &mut FnCtx<'_>, + name: &str, + args: &[Expr], +) -> Result> { + match name { + // #1671: `setTimeout(fn)` with no explicit delay. Node treats a + // missing/undefined delay as 0 (fires on the next timer tick). + // Without this arm a 1-arg `setTimeout` falls through to the + // catch-all below, which emits a bare LLVM call to `@setTimeout` + // and the linker fails with `Undefined symbols: _setTimeout` + // (hit by hono/jsx's `hooks/index.js`, which schedules a re-render + // via `setTimeout(() => { … })`). Route it to the same runtime + // entry as the 2-arg form with a zero delay. + "setTimeout" if args.len() == 1 => { + let cb_box = lower_expr(ctx, &args[0])?; + let blk = ctx.block(); + // #2013 — validate the callback type before unboxing the + // pointer. `js_timer_validate_callback` throws + // ERR_INVALID_ARG_TYPE for any non-callable value and + // returns the raw closure pointer otherwise; the second + // arg `0` is the type-name index for "setTimeout". + let zero_idx = "0"; + let cb_handle = blk.call( + I64, + "js_timer_validate_callback", + &[(DOUBLE, &cb_box), (I32, zero_idx)], + ); + let zero = double_literal(0.0); + let id = blk.call( + I64, + "js_set_timeout_callback", + &[(I64, &cb_handle), (DOUBLE, &zero)], + ); + return Ok(Some(nanbox_pointer_inline(blk, &id))); + } + "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))); + } + "setImmediate" if !args.is_empty() => { + if args.len() == 1 { + let cb_box = lower_expr(ctx, &args[0])?; + 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", &[(I64, &cb_handle)]); + return Ok(Some(nanbox_pointer_inline(blk, &id))); + } + + // #7210: same treatment as `setTimeout` below — see the comment + // 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 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); + return Ok(Some(boxed)); + } + // Refs #665: `setTimeout(fn, delay, ...args)` — JS spec forwards + // the trailing args to `fn` when the timer fires. Pack them into + // a stack buffer of doubles and hand off to the varargs runtime + // entry. Used by Promise-executor patterns like + // `setTimeout(resolve, delay, res)` (rate-limiter-flexible's + // `RateLimiterMemory.consume` is the discovering call site). + "setTimeout" if args.len() >= 3 => { + // #7210: lower the WHOLE argument list through `lower_exprs_rooted`, + // then fill the buffer in a second, lowering-free pass. + // + // The previous shape had two unrooted windows, and the callback's + // was the one that crashed. `cb_box` was lowered first and read at + // `js_timer_validate_callback` — after `lower_expr(delay)` and after + // every trailing argument's `lower_expr`. `setTimeout(cb, 0, {…}, + // churn())` therefore held a freshly-allocated closure in an SSA + // register across a user call with loop back-edge polls, and the + // moving minor inside `churn` left the register naming from-space: + // `TypeError: The "callback" argument must be of type function. + // Received an instance of Object`, deterministically, at base. + // + // The staging buffer is the second window and is the worse of the + // two in kind: argument *i* sits in a bare `alloca_entry_array`, + // which the precise root walk never visits, while argument *i+1* is + // 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 + // 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. + 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 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); + return Ok(Some(boxed)); + } + "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))); + } + "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 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); + return Ok(Some(boxed)); + } + "clearTimeout" if args.len() == 1 => { + // Pass the raw NaN-boxed arg so the runtime accepts both the + // handle and its primitive numeric id (`clearTimeout(+t)`, #1213). + let id_box = lower_expr(ctx, &args[0])?; + ctx.block() + .call_void("js_clear_timeout_value", &[(DOUBLE, &id_box)]); + return Ok(Some(double_literal(f64::from_bits( + crate::nanbox::TAG_UNDEFINED, + )))); + } + "clearInterval" if args.len() == 1 => { + let id_box = lower_expr(ctx, &args[0])?; + ctx.block() + .call_void("js_clear_interval_value", &[(DOUBLE, &id_box)]); + return Ok(Some(double_literal(f64::from_bits( + crate::nanbox::TAG_UNDEFINED, + )))); + } + "clearImmediate" if args.len() == 1 => { + let id_box = lower_expr(ctx, &args[0])?; + ctx.block() + .call_void("js_clear_immediate_value", &[(DOUBLE, &id_box)]); + return Ok(Some(double_literal(f64::from_bits( + crate::nanbox::TAG_UNDEFINED, + )))); + } + _ => {} + } + Ok(None) +} diff --git a/crates/perry-codegen/src/lower_call/mod.rs b/crates/perry-codegen/src/lower_call/mod.rs index f542a13e6b..7913ca5e04 100644 --- a/crates/perry-codegen/src/lower_call/mod.rs +++ b/crates/perry-codegen/src/lower_call/mod.rs @@ -42,6 +42,7 @@ mod dataview_intrinsic; mod early_branches; mod event_target; mod extern_func; +mod extern_timers; mod field_init; mod func_ref; mod jsx; diff --git a/crates/perry-codegen/src/lower_call/namespace_call.rs b/crates/perry-codegen/src/lower_call/namespace_call.rs index f462daeec3..173e9c899b 100644 --- a/crates/perry-codegen/src/lower_call/namespace_call.rs +++ b/crates/perry-codegen/src/lower_call/namespace_call.rs @@ -47,30 +47,41 @@ pub fn try_lower_namespace_member_call( .is_some_and(|submod| submod == "timers") { match property.as_str() { + // #7210: the `timers` namespace forms carry the same two unrooted + // windows as the global `setTimeout`/`setInterval`/`setImmediate` + // lowerings in `extern_func.rs` (see the comment there), plus one + // of their own: `cb_handle` is `unbox_to_i64`'d — a RAW heap + // address, not even NaN-boxed — before the trailing arguments are + // lowered. Lower the whole list through `lower_exprs_rooted` and + // unbox below it, so the handle is derived from a post-collection + // value. "setTimeout" if !args.is_empty() => { - let cb_box = lower_expr(ctx, &args[0])?; + 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 { - lower_expr(ctx, &args[1])? + vals[1].clone() } else { double_literal(0.0) }; - let blk = ctx.block(); - let cb_handle = unbox_to_i64(blk, &cb_box); if args.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(Some(nanbox_pointer_inline(blk, &id))); + 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, a) in args.iter().skip(2).enumerate() { - let v = lower_expr(ctx, a)?; + 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); + blk.store(DOUBLE, v, &slot); } let ptr_reg = ctx.block().next_reg(); ctx.block().emit_raw(format!( @@ -78,6 +89,7 @@ pub fn try_lower_namespace_member_call( 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", @@ -88,28 +100,33 @@ pub fn try_lower_namespace_member_call( (I32, &n.to_string()), ], ); - return Ok(Some(nanbox_pointer_inline(blk, &id))); + let boxed = nanbox_pointer_inline(ctx.block(), &id); + crate::expr::temp_root::temp_root_release(ctx, guard); + return Ok(Some(boxed)); } "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 cb_handle = unbox_to_i64(blk, &cb_box); + 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 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(Some(nanbox_pointer_inline(blk, &id))); + 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, a) in args.iter().skip(2).enumerate() { - let v = lower_expr(ctx, a)?; + 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); + blk.store(DOUBLE, v, &slot); } let ptr_reg = ctx.block().next_reg(); ctx.block().emit_raw(format!( @@ -117,6 +134,7 @@ pub fn try_lower_namespace_member_call( 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", @@ -127,23 +145,27 @@ pub fn try_lower_namespace_member_call( (I32, &n.to_string()), ], ); - return Ok(Some(nanbox_pointer_inline(blk, &id))); + let boxed = nanbox_pointer_inline(ctx.block(), &id); + crate::expr::temp_root::temp_root_release(ctx, guard); + return Ok(Some(boxed)); } "setImmediate" if !args.is_empty() => { - let cb_box = lower_expr(ctx, &args[0])?; - let blk = ctx.block(); - let cb_handle = unbox_to_i64(blk, &cb_box); if args.len() == 1 { + let cb_box = lower_expr(ctx, &args[0])?; + let blk = ctx.block(); + let cb_handle = unbox_to_i64(blk, &cb_box); let id = blk.call(I64, "js_set_immediate_callback", &[(I64, &cb_handle)]); 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, a) in args.iter().skip(1).enumerate() { - let v = lower_expr(ctx, a)?; + for (i, v) in vals.iter().skip(1).enumerate() { let blk = ctx.block(); let slot = blk.gep(DOUBLE, &buf, &[(I64, &format!("{}", i))]); - blk.store(DOUBLE, &v, &slot); + blk.store(DOUBLE, v, &slot); } let ptr_reg = ctx.block().next_reg(); ctx.block().emit_raw(format!( @@ -151,12 +173,15 @@ pub fn try_lower_namespace_member_call( 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())], ); - return Ok(Some(nanbox_pointer_inline(blk, &id))); + let boxed = nanbox_pointer_inline(ctx.block(), &id); + crate::expr::temp_root::temp_root_release(ctx, guard); + return Ok(Some(boxed)); } "clearTimeout" | "clearInterval" | "clearImmediate" if !args.is_empty() => { let id_box = lower_expr(ctx, &args[0])?; diff --git a/test-files/test_gap_gc_staging_args_rooting.ts b/test-files/test_gap_gc_staging_args_rooting.ts new file mode 100644 index 0000000000..db30f37730 --- /dev/null +++ b/test-files/test_gap_gc_staging_args_rooting.ts @@ -0,0 +1,79 @@ +// #7210 §2: an argument staging buffer filled INTERLEAVED with lowering. +// +// `setTimeout(cb, 0, {…}, churn())` lowers to +// +// %buf = alloca [2 x double] ; a bare entry alloca +// %v0 = ; a fresh heap object +// store double %v0, ptr %buf[0] ; ← now the ONLY reference +// %v1 = call double @churn() ; loop back-edge poll -> moving minor +// store double %v1, ptr %buf[1] +// call void @js_set_timeout_callback_args(…, ptr %buf, …) +// +// `%buf` is neither a shadow slot nor a temp root, so the precise root walk +// never sees `%v0`. The window is not staleness — it is a premature SWEEP: at +// the moment `churn()` collects, nothing anywhere refers to the object. +// +// The same shape covers setImmediate / setInterval / process.nextTick / the +// `timers` namespace forms / a spread call's regular-argument buffer. + +function churn(n: number): number { + let acc = 0; + for (let i = 0; i < n; i++) { + const o = { i, pad: "x".repeat(8) }; + acc += o.i; + } + return acc; +} + +let bad = 0; + +function expect(label: string, got: unknown, want: unknown): void { + if (got !== want) { + bad++; + console.log("BAD " + label + ": got " + String(got) + " want " + String(want)); + } +} + +// --- setTimeout with trailing args ----------------------------------------- +setTimeout( + (a: { tag: string; n: number }, b: number) => { + expect("setTimeout.a.tag", a.tag, "alpha"); + expect("setTimeout.a.n", a.n, 11); + expect("setTimeout.b", b, 4950); + }, + 0, + { tag: "alpha", n: 11 }, + churn(100), +); + +// --- setImmediate with trailing args --------------------------------------- +setImmediate( + (a: { tag: string }, b: { tag: string }, c: number) => { + expect("setImmediate.a", a.tag, "beta"); + expect("setImmediate.b", b.tag, "gamma"); + expect("setImmediate.c", c, 4950); + }, + { tag: "beta" }, + { tag: "gamma" }, + churn(100), +); + +// --- process.nextTick with trailing args ----------------------------------- +process.nextTick( + (a: { tag: string }, b: number) => { + expect("nextTick.a", a.tag, "delta"); + expect("nextTick.b", b, 4950); + }, + { tag: "delta" }, + churn(100), +); + +// --- a spread call's regular-argument buffer ------------------------------- +const rest = [7, 8]; +const sink = (a: { tag: string }, b: number, ...more: number[]): string => + a.tag + ":" + String(b) + ":" + more.join(","); +expect("spread", sink({ tag: "eps" }, churn(100), ...rest), "eps:4950:7,8"); + +setTimeout(() => { + console.log("bad " + String(bad)); +}, 1); diff --git a/test-parity/gc_repsel_corpus.txt b/test-parity/gc_repsel_corpus.txt index ff1164da4f..d7f6e900a7 100644 --- a/test-parity/gc_repsel_corpus.txt +++ b/test-parity/gc_repsel_corpus.txt @@ -339,3 +339,14 @@ test_gap_gc_closure_call_prev_this_rooting # fix in this PR is verified on the safepoint route and is NOT claimed on the # allocation-point route, which is #7217's open defect class at a second site. test_gap_gc_regexp_receiver_rooting +# --- #7210: argument staging buffers filled interleaved with lowering ------- +# `setTimeout(cb, 0, {…}, churn())` kept the CALLBACK closure in a bare SSA +# register across every argument's lowering, and each trailing argument in a +# bare `alloca_entry_array` the precise root walk never visits, while the next +# one was lowered. Measured at base (`7d1dc9ca2`), compiled and run under +# `PERRY_GC_MOVING_LOOP_POLLS=1 PERRY_GC_HEAP_LIMIT=8`: 5/5 +# `TypeError: The "callback" argument must be of type function. Received an +# instance of Object` — deterministic, and identical on the `evac_minor` arm. +# Clean 5/5 on the shipped default (no polls, no pressure) both before and +# after, so this is a `requires=move`-only witness like the #7207 trio. +test_gap_gc_staging_args_rooting From cc91790b159f88d991d82f3e91adf212aed5a3b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 2 Aug 2026 06:03:20 +0200 Subject: [PATCH 2/4] docs: changelog fragment for #7230 --- changelog.d/7230-timer-arg-staging-rooting.md | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 changelog.d/7230-timer-arg-staging-rooting.md diff --git a/changelog.d/7230-timer-arg-staging-rooting.md b/changelog.d/7230-timer-arg-staging-rooting.md new file mode 100644 index 0000000000..6679632022 --- /dev/null +++ b/changelog.d/7230-timer-arg-staging-rooting.md @@ -0,0 +1,55 @@ +### Fixed + +- **GC: the timer family's callback and trailing arguments are rooted across their own argument list (#7210).** + `setTimeout(cb, 0, {…}, churn())` had two unrooted windows, and the callback's + was a live deterministic crash on `main`, not the staleness the issue predicted. + + `cb_box` was lowered first and read at `js_timer_validate_callback` — after the + delay's `lower_expr` and after every trailing argument's. A freshly-allocated + closure therefore sat in a bare SSA register across a user call carrying loop + back-edge polls, and the moving minor inside it left the register naming + from-space: `TypeError [ERR_INVALID_ARG_TYPE]: The "callback" argument must be + of type function. Received an instance of Object`, 3/3 under + `PERRY_GC_MOVING_LOOP_POLLS=1 PERRY_GC_HEAP_LIMIT=8` and identically on the + `evac_minor` arm. + + The trailing-argument staging buffer is the second window and is worse in kind: + argument *i* sits in a bare `alloca_entry_array`, storage the precise root walk + never visits, while argument *i+1* is lowered. Nothing anywhere refers to the + object at that moment, so it is a premature **sweep**, not a stale address. + + The `timers`-namespace forms carried a third: `cb_handle` was `unbox_to_i64`'d + — a raw heap address, not even NaN-boxed — above the trailing arguments. + + All three close with one change: lower the whole argument list through + `expr::temp_root::lower_exprs_rooted`, then fill the buffer in a second, + lowering-free pass, and release the guard only after the consuming call (which + reads the buffer). Cost is zero when nothing in the list can collect + (`OperandProtection::Reuse`) — `setTimeout(fn, 0, someLocal)` emits + byte-identical IR. Covers global `setTimeout`/`setInterval`/`setImmediate`, + their `timers`-namespace siblings, and `process.nextTick`. + + `crates/perry-codegen/src/lower_call/extern_func.rs` crossed the 2000-line cap, + so the timer arms moved to a new `lower_call/extern_timers.rs` — a pure + mechanical move, dispatched immediately above the match so arm order is + unchanged. Timer routing after the split was re-verified against node 26.5.1 on + a program exercising every moved arm. + + Witness `test-files/test_gap_gc_staging_args_rooting.ts`, registered in + `test-parity/gc_repsel_corpus.txt`: red 3/3 at base, `bad 0` 5/5 after, clean + 3/3 on the shipped default, byte-exact against node. + +### Notes + +- The `#7210` triage that motivated this change found that the **66** + moving-reachable `gc_root_dominance_check.py --unrooted-allocas` reports are + **all false positives**, and says so rather than adding roots: 64 are the + `@perry_class_keys_*` pointer cache, whose array is allocated by + `js_array_alloc_with_length_longlived` into the old arena and is therefore + moved only by old-page defrag — which is **off by default** since #6206 + (`PERRY_GC_OLD_DEFRAG=1` opt-in); the other 2 are `js_box_alloc_bits` results, + which come from `std::alloc::alloc`, are never freed and are never relocated + (`scan_box_roots_mut` rewrites the JSValue *inside* the box, not its address). + The checker's `_is_heap_source` conflates "a location the collector rewrites" + with "an object the collector can move"; the sites it names are the former and + not the latter. From 12bf05baf0c4c7ced430fd7c5f94154002e8aeee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 2 Aug 2026 06:44:59 +0200 Subject: [PATCH 3/4] fix(gc): scan setInterval's trailing arguments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `scan_timer_roots_mut` walked `timer.args` for CALLBACK_TIMERS and for both MOCK_TIMERS lists, and never for INTERVAL_TIMERS. The incremental twin `scan_interval_timers_step` had the same hole, and cycle-based collections run ONLY the step scanner. So `setInterval(fn, delay, { … })` left the object in a table nothing scanned: swept at the first collection, then handed to the callback as a dangling pointer on the next tick. Different class from a stale register — it does not need a collection to land in a narrow window; it goes wrong at collection #0. A partially-correct scanner is worse than an absent one: it reads as covered. This is the runtime half of the codegen fix in the preceding commit. Rooting an argument across its own lowering buys nothing if the table it then lands in is not a root. Refs #7210, #7231. --- crates/perry-runtime/src/timer.rs | 15 +++++ crates/perry-runtime/src/timer/gc_scan.rs | 21 ++++++- .../test_gap_gc_interval_args_rooting.ts | 55 +++++++++++++++++++ test-parity/gc_repsel_corpus.txt | 8 +++ 4 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 test-files/test_gap_gc_interval_args_rooting.ts diff --git a/crates/perry-runtime/src/timer.rs b/crates/perry-runtime/src/timer.rs index 9540c68cd2..7c84324fcd 100644 --- a/crates/perry-runtime/src/timer.rs +++ b/crates/perry-runtime/src/timer.rs @@ -1652,6 +1652,21 @@ pub fn scan_timer_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { if !timer.cleared && timer.callback != 0 { visitor.visit_i64_slot(&mut timer.callback); } + // #7210: `setInterval(fn, delay, ...args)` stores the trailing + // arguments in `IntervalTimer.args`, and this was the only one of + // the four blocks in this function that never walked them — the + // `CALLBACK_TIMERS` block above and both `MOCK_TIMERS` blocks below + // do. So `setInterval(fn, d, { … })` left the object in a table + // nothing scanned: swept at the first collection, then handed to the + // callback as a dangling pointer on the next tick. + // + // A partially-correct scanner is worse than an absent one — it reads + // as covered. This is also the runtime half of the codegen fix in + // the same change: rooting an argument across its own lowering buys + // nothing if the table it then lands in is not a root. + for arg in &mut timer.args { + visitor.visit_nanbox_f64_slot(arg); + } crate::async_context::scan_snapshot_roots_mut(&mut timer.context, visitor); } } diff --git a/crates/perry-runtime/src/timer/gc_scan.rs b/crates/perry-runtime/src/timer/gc_scan.rs index 6e6dc3bb27..1166ada873 100644 --- a/crates/perry-runtime/src/timer/gc_scan.rs +++ b/crates/perry-runtime/src/timer/gc_scan.rs @@ -174,6 +174,23 @@ fn scan_interval_timers_step( state.slot = 1; } if state.slot == 1 { + // #7210: the step twin of the `INTERVAL_TIMERS` args pass added to + // `scan_timer_roots_mut`. Cycle-based collections run ONLY the step + // scanner, so updating the stop-the-world function alone would leave + // `setInterval(fn, d, { … })` unrooted on exactly the incremental + // path — the same "only one of the two twins was updated" gap this + // file's `scan_mock_timers_step` comment already records. + while state.arg_index < timer.args.len() { + if !consume_timer_root_work(remaining) { + return false; + } + visitor.visit_nanbox_f64_slot(&mut timer.args[state.arg_index]); + state.arg_index += 1; + } + state.slot = 2; + state.arg_index = 0; + } + if state.slot == 2 { if !crate::async_context::scan_snapshot_roots_mut_step( &mut timer.context, visitor, @@ -183,7 +200,9 @@ fn scan_interval_timers_step( ) { return false; } - state.slot = 2; + state.slot = 3; + state.context_entry = 0; + state.context_store = 0; } state.index += 1; state.finish_timer(); diff --git a/test-files/test_gap_gc_interval_args_rooting.ts b/test-files/test_gap_gc_interval_args_rooting.ts new file mode 100644 index 0000000000..521d934e19 --- /dev/null +++ b/test-files/test_gap_gc_interval_args_rooting.ts @@ -0,0 +1,55 @@ +// #7210, runtime half: `setInterval(fn, delay, ...args)` stores its trailing +// arguments in `IntervalTimer.args`, and `scan_timer_roots_mut` was the only +// one of its four blocks that never walked them — `CALLBACK_TIMERS` and both +// `MOCK_TIMERS` lists do. The incremental twin `scan_interval_timers_step` had +// the same hole, and cycle-based collections run ONLY the step scanner. +// +// So the object below lived in a table nothing scanned: swept at the first +// collection, then handed to the callback as a dangling pointer on the next +// tick. This is a different failure class from a stale register — it does not +// need a collection to land in a narrow window, it goes wrong at collection #0 +// and stays wrong. +// +// It is also the other half of the codegen fix that ships with it: rooting an +// argument across its own lowering buys nothing if the table it lands in is not +// a root. + +function churn(n: number): number { + let acc = 0; + for (let i = 0; i < n; i++) { + const o = { i, pad: "y".repeat(12) }; + acc += o.i; + } + return acc; +} + +let bad = 0; +let ticks = 0; + +const payload = { tag: "interval", n: 3 }; + +const handle = setInterval( + (a: { tag: string; n: number }, b: string) => { + ticks++; + // Read the staged arguments back on every tick. The first tick can precede + // the first collection; the later ones cannot. + if (a === null || typeof a !== "object" || a.tag !== "interval" || a.n !== 3) { + bad++; + console.log("BAD interval.a tick " + String(ticks)); + } + if (b !== "second") { + bad++; + console.log("BAD interval.b tick " + String(ticks)); + } + churn(200); + if (ticks >= 4) { + clearInterval(handle); + console.log("bad " + String(bad)); + } + }, + 1, + payload, + "second", +); + +churn(400); diff --git a/test-parity/gc_repsel_corpus.txt b/test-parity/gc_repsel_corpus.txt index d7f6e900a7..8c681db114 100644 --- a/test-parity/gc_repsel_corpus.txt +++ b/test-parity/gc_repsel_corpus.txt @@ -350,3 +350,11 @@ test_gap_gc_regexp_receiver_rooting # Clean 5/5 on the shipped default (no polls, no pressure) both before and # after, so this is a `requires=move`-only witness like the #7207 trio. test_gap_gc_staging_args_rooting + +# --- #7210 runtime half: setInterval trailing args were in no root ---------- +# `scan_timer_roots_mut` walked `args` for CALLBACK_TIMERS and for both +# MOCK_TIMERS lists, and NOT for INTERVAL_TIMERS; the incremental twin +# `scan_interval_timers_step` had the same hole, and cycle-based collections run +# only the step scanner. A partially-correct scanner is worse than an absent one +# — it reads as covered. Measured at base: `BAD interval.a` from tick 2 onward. +test_gap_gc_interval_args_rooting From a8a6d3b8ab1a657b9eeecabc7d9c68652ad17ea8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 2 Aug 2026 06:58:37 +0200 Subject: [PATCH 4/4] docs: changelog for the setInterval args scanner fix --- changelog.d/7230-timer-arg-staging-rooting.md | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/changelog.d/7230-timer-arg-staging-rooting.md b/changelog.d/7230-timer-arg-staging-rooting.md index 6679632022..58a8894ab4 100644 --- a/changelog.d/7230-timer-arg-staging-rooting.md +++ b/changelog.d/7230-timer-arg-staging-rooting.md @@ -53,3 +53,26 @@ The checker's `_is_heap_source` conflates "a location the collector rewrites" with "an object the collector can move"; the sites it names are the former and not the latter. + +- **GC: `setInterval`'s trailing arguments are now scanned (#7210, runtime half).** + `scan_timer_roots_mut` walked `timer.args` for `CALLBACK_TIMERS` and for both + `MOCK_TIMERS` lists, and never for `INTERVAL_TIMERS`; the incremental twin + `scan_interval_timers_step` had the same hole, and cycle-based collections run + **only** the step scanner. So `setInterval(fn, delay, { … })` left the object + in a table nothing scanned — swept at the first collection, then handed to the + callback as a dangling pointer on the next tick. + + This is a different failure class from a stale register: it does not need a + collection to land in a narrow window, it goes wrong at collection #0 and + stays wrong, and no static IR checker can see it (the table is not in the + emitted IR). A partially-correct scanner is worse than an absent one, because + it reads as covered. + + It is also the other half of the codegen fix above — rooting an argument + across its own lowering buys nothing if the table it then lands in is not a + root. Witness `test-files/test_gap_gc_interval_args_rooting.ts`, registered in + the corpus: `BAD interval.a`/`BAD interval.b` from tick 1, 3/3 at base under + `PERRY_GC_MOVING_LOOP_POLLS=1 PERRY_GC_HEAP_LIMIT=8`; `bad 0` 5/5 after; clean + 3/3 on the shipped default; byte-exact against node 26.5.1. + + The wider population this came from is enumerated in #7231.