From dff77a3389c248a878e766301cdf0b3b2fb4dd3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 5 Aug 2026 18:01:14 +0200 Subject: [PATCH 1/2] fix(codegen): root the third URLSearchParams operand, and release on both paths Corrects two defects in #7462. INCOMPLETE. Has/Set/Append/Delete take a third operand, and #7462 rooted only params+name -- so `value` still lowered AFTER the receiver was unboxed, leaving p_ptr and n_v crossing exactly the window the change was meant to close. All operands are now lowered together, `value` included when present. LEAKED. The automated release placement in #7462 landed inside Delete's `else` branch only, so the with-value path pushed two temp roots per execution and never truncated them -- unbounded growth in a loop. It compiled with no warning, which is why the arms are now audited programmatically for a top-level release rather than by reading. All six arms verified: one release each, at arm top level, reachable on every path. 12/12 URL + URLSearchParams gap tests byte-identical to node. A three-operand repro (set/append/has/delete, each argument a fresh allocating call) matches node and is clean under PERRY_GC_HEAP_LIMIT=8 PERRY_GC_FORCE_EVACUATE=1. --- crates/perry-codegen/src/expr/url_main.rs | 55 +++++++++++++++++------ 1 file changed, 41 insertions(+), 14 deletions(-) diff --git a/crates/perry-codegen/src/expr/url_main.rs b/crates/perry-codegen/src/expr/url_main.rs index b728c0fede..45b0ba58b7 100644 --- a/crates/perry-codegen/src/expr/url_main.rs +++ b/crates/perry-codegen/src/expr/url_main.rs @@ -335,14 +335,22 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // reloaded receiver, so neither crosses the window in a register. // The guard lives to the end of the arm: every use below is a use // of one of the two rooted values. - let (vals, operand_guard) = super::temp_root::lower_exprs_rooted(ctx, &[params, name])?; + // All operands rooted together, `value` included when present, so + // nothing crosses its lowering in a register. #7462 rooted only + // `params`+`name` and left this path with the window it was meant + // to close. + let mut operand_exprs: Vec<&Expr> = vec![params, name]; + if let Some(v_expr) = value { + operand_exprs.push(v_expr); + } + let (vals, operand_guard) = super::temp_root::lower_exprs_rooted(ctx, &operand_exprs)?; let (p_v, n_v) = (vals[0].clone(), vals[1].clone()); let p_ptr = unbox_to_i64(ctx.block(), &p_v); // Runtime returns 0.0 / 1.0 as a plain f64 — not NaN-boxed. // Translate to TAG_TRUE / TAG_FALSE so `typeof` and strict-eq // behave correctly. - let raw = if let Some(v_expr) = value { - let v_v = lower_expr(ctx, v_expr)?; + let raw = if value.is_some() { + let v_v = vals[2].clone(); ctx.block().call( DOUBLE, "js_url_search_params_has2", @@ -380,10 +388,14 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // reloaded receiver, so neither crosses the window in a register. // The guard lives to the end of the arm: every use below is a use // of one of the two rooted values. - let (vals, operand_guard) = super::temp_root::lower_exprs_rooted(ctx, &[params, name])?; - let (p_v, n_v) = (vals[0].clone(), vals[1].clone()); + // All operands are rooted together: `value` is lowered too, so + // nothing crosses it in a register. #7462 rooted only + // `params`+`name`, which left the three-operand path with the same + // window it was meant to close. + let (vals, operand_guard) = + super::temp_root::lower_exprs_rooted(ctx, &[params, name, value])?; + let (p_v, n_v, val_v) = (vals[0].clone(), vals[1].clone(), vals[2].clone()); let p_ptr = unbox_to_i64(ctx.block(), &p_v); - let val_v = lower_expr(ctx, value)?; ctx.block().call_void( "js_url_search_params_set", &[(I64, &p_ptr), (DOUBLE, &n_v), (DOUBLE, &val_v)], @@ -406,10 +418,14 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // reloaded receiver, so neither crosses the window in a register. // The guard lives to the end of the arm: every use below is a use // of one of the two rooted values. - let (vals, operand_guard) = super::temp_root::lower_exprs_rooted(ctx, &[params, name])?; - let (p_v, n_v) = (vals[0].clone(), vals[1].clone()); + // All operands are rooted together: `value` is lowered too, so + // nothing crosses it in a register. #7462 rooted only + // `params`+`name`, which left the three-operand path with the same + // window it was meant to close. + let (vals, operand_guard) = + super::temp_root::lower_exprs_rooted(ctx, &[params, name, value])?; + let (p_v, n_v, val_v) = (vals[0].clone(), vals[1].clone(), vals[2].clone()); let p_ptr = unbox_to_i64(ctx.block(), &p_v); - let val_v = lower_expr(ctx, value)?; ctx.block().call_void( "js_url_search_params_append", &[(I64, &p_ptr), (DOUBLE, &n_v), (DOUBLE, &val_v)], @@ -432,11 +448,19 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // reloaded receiver, so neither crosses the window in a register. // The guard lives to the end of the arm: every use below is a use // of one of the two rooted values. - let (vals, operand_guard) = super::temp_root::lower_exprs_rooted(ctx, &[params, name])?; + // All operands rooted together, `value` included when present, so + // nothing crosses its lowering in a register. #7462 rooted only + // `params`+`name` and left this path with the window it was meant + // to close. + let mut operand_exprs: Vec<&Expr> = vec![params, name]; + if let Some(v_expr) = value { + operand_exprs.push(v_expr); + } + let (vals, operand_guard) = super::temp_root::lower_exprs_rooted(ctx, &operand_exprs)?; let (p_v, n_v) = (vals[0].clone(), vals[1].clone()); let p_ptr = unbox_to_i64(ctx.block(), &p_v); - if let Some(v_expr) = value { - let v_v = lower_expr(ctx, v_expr)?; + if value.is_some() { + let v_v = vals[2].clone(); ctx.block().call_void( "js_url_search_params_delete2", &[(I64, &p_ptr), (DOUBLE, &n_v), (DOUBLE, &v_v)], @@ -446,9 +470,12 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { "js_url_search_params_delete", &[(I64, &p_ptr), (DOUBLE, &n_v)], ); - // Released after the consuming call, which itself allocates. - super::temp_root::temp_root_release(ctx, operand_guard); } + // Released after the consuming call on BOTH arms. #7462's automated + // placement put this inside the `else` only, so the with-value path + // pushed two temp roots per execution and never truncated them — + // unbounded growth in a loop, and it compiled without a warning. + super::temp_root::temp_root_release(ctx, operand_guard); Ok(ctx .block() .bitcast_i64_to_double(crate::nanbox::TAG_UNDEFINED_I64)) From d9fffbee3f15b62b3cd768727074983c9ce5d042 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 5 Aug 2026 18:01:44 +0200 Subject: [PATCH 2/2] docs: changelog fragment for 7463 --- changelog.d/7463-searchparams-third-operand.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/7463-searchparams-third-operand.md diff --git a/changelog.d/7463-searchparams-third-operand.md b/changelog.d/7463-searchparams-third-operand.md new file mode 100644 index 0000000000..efb6454fdb --- /dev/null +++ b/changelog.d/7463-searchparams-third-operand.md @@ -0,0 +1 @@ +- **Corrects #7462 on the `URLSearchParams` arms that take a value.** `Has`, `Set`, `Append` and `Delete` have a third operand, and #7462 rooted only `params`+`name` — so `value` still lowered after the receiver was unboxed, leaving the pointer crossing the window the change was meant to close. All operands are now rooted together. It also fixes a leak that change introduced: the automated release placement landed inside `Delete`'s `else` branch only, so the with-value path pushed two temp roots per execution and never truncated them — unbounded growth in a loop, and it compiled without a warning. All six arms are now audited programmatically for exactly one top-level release reachable on every path. 12/12 gap tests byte-identical to node; a three-operand repro with allocating arguments is clean under forced evacuation. (#7463)