From 5c6d869e5575368493d3d03557f1bba78ad80657 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 18 Sep 2026 13:26:54 +0000 Subject: [PATCH 1/2] fix(codegen): refresh local_types on var redeclaration A hoisted var reaches lower_let as two Stmt::Lets sharing one local id (a body-entry predefine, then the real declaration); the second takes the #1803 redeclaration early return before ctx.local_types is updated. proven_local_types (consulted by is_numeric_expr) IS refreshed on redeclaration, but local_types (consulted by expr_may_return_boxed_value_from_raw_f64_fallback) was not, so the two predicates disagreed about the same local: a strict-equality compare against an out-of-bounds/hole read of a var-declared number array took the bare-fcmp numeric fast path, which cannot represent the NaN-boxed undefined tag such a read can produce. Refresh local_types on the redeclaration path too. --- crates/perry-codegen/src/stmt/let_stmt.rs | 16 +++ .../src/stmt/let_stmt_var_redeclare_tests.rs | 119 ++++++++++++++++++ crates/perry-codegen/src/stmt/mod.rs | 2 + .../test_gap_10488_var_array_void_compare.ts | 101 +++++++++++++++ 4 files changed, 238 insertions(+) create mode 100644 crates/perry-codegen/src/stmt/let_stmt_var_redeclare_tests.rs create mode 100644 test-files/test_gap_10488_var_array_void_compare.ts diff --git a/crates/perry-codegen/src/stmt/let_stmt.rs b/crates/perry-codegen/src/stmt/let_stmt.rs index 251f4bbd8a..d805e919d2 100644 --- a/crates/perry-codegen/src/stmt/let_stmt.rs +++ b/crates/perry-codegen/src/stmt/let_stmt.rs @@ -383,6 +383,22 @@ pub(crate) fn lower_let( ctx, &perry_hir::Expr::LocalSet(id, Box::new(init_expr.clone())), )?; + // #10488: a hoisted `var`'s real declaration reaches this + // redeclaration branch (#1803 predefine-then-declare shape) and + // returns below before the fresh-declaration path's + // `ctx.local_types.insert` ever runs. `proven_local_types` a few + // lines up IS refreshed per-site, so `is_numeric_expr` (which + // consults it via `stable_local_type_proof`) sees this + // declaration's more specific type — but `local_types` keeps + // whatever the FIRST (predefine) site declared, normally `Any`. + // That desyncs `is_numeric_expr` from `static_type_of` / + // `expr_may_return_boxed_value_from_raw_f64_fallback`, which both + // read `local_types`: a strict-equality compare against an + // out-of-bounds/hole array read was treated as definitely-numeric + // (a bare `fcmp`, which cannot represent `undefined`) instead of + // falling back to a boxed compare. Refresh `local_types` here too + // so both predicates agree on this local's current type. + ctx.local_types.insert(id, refined_ty.clone()); } else if ctx.tdz_boxes.remove(&id) { // No-init reuse (`let x;`) of a TDZ-seeded box must still end the // dead zone by clearing the sentinel to `undefined`; otherwise a diff --git a/crates/perry-codegen/src/stmt/let_stmt_var_redeclare_tests.rs b/crates/perry-codegen/src/stmt/let_stmt_var_redeclare_tests.rs new file mode 100644 index 0000000000..20a6e56d33 --- /dev/null +++ b/crates/perry-codegen/src/stmt/let_stmt_var_redeclare_tests.rs @@ -0,0 +1,119 @@ +//! #10488: a hoisted `var` reaches `lower_let` as TWO `Stmt::Let`s sharing one +//! local id — a body-entry predefine (`Any = undefined`), then the real +//! declaration (here, `Array(Number) = [0]`) — and the second one takes the +//! #1803 redeclaration early return. `ctx.local_types` must be refreshed on +//! that path too, or `expr_may_return_boxed_value_from_raw_f64_fallback` +//! (which reads it via `static_type_of`) stays desynced from +//! `is_numeric_expr` (which reads the separately-refreshed +//! `proven_local_types`), and a strict-equality compare against an +//! out-of-bounds array read wrongly takes the bare-`fcmp` numeric fast path. + +use perry_hir::types::Type; +use perry_hir::{CompareOp, Expr, Stmt}; + +use crate::temp_root_coverage::main_ir_for as ir_for; + +const ARR: u32 = 1; +const R: u32 = 2; + +/// A CALL to the boxed comparison helper, not the unconditional `declare` +/// line — mirrors `compare_tests.rs`'s `JS_EQ_CALL`. +const JS_EQ_CALL: &str = "call i64 @js_eq("; + +/// Hand-build the exact HIR shape a hoisted `var arr = [0]; ...; arr[1] === +/// void 0;` lowers to: TWO `Let`s sharing id `ARR` (the predefine, then the +/// real array-literal declaration), followed by a strict-equality compare of +/// an out-of-bounds index read against `Expr::Void`. +#[test] +fn var_redeclared_numeric_array_compare_against_void_stays_boxed() { + let ir = ir_for( + "var_redeclare_void_compare", + vec![ + // Body-entry predefine: `var arr;` before the real declaration + // runs — declared `Any`, matching what hoisting emits. + Stmt::Let { + id: ARR, + name: "arr".to_string(), + ty: Type::Any, + mutable: true, + init: Some(Expr::Undefined), + }, + // The real declaration: same id, now a proven `Array(Number)`. + // This is the REDECLARATION path in `lower_let` (#1803). + Stmt::Let { + id: ARR, + name: "arr".to_string(), + ty: Type::Array(Box::new(Type::Number)), + mutable: true, + init: Some(Expr::Array(vec![Expr::Integer(0)])), + }, + Stmt::Let { + id: R, + name: "r".to_string(), + ty: Type::Any, + mutable: true, + init: Some(Expr::Compare { + op: CompareOp::Eq, + left: Box::new(Expr::IndexGet { + object: Box::new(Expr::LocalGet(ARR)), + index: Box::new(Expr::Integer(1)), + }), + right: Box::new(Expr::Void(Box::new(Expr::Integer(0)))), + }), + }, + ], + ); + assert!( + ir.contains(JS_EQ_CALL), + "an out-of-bounds read of a var-redeclared Array(Number) compared \ + against `void 0` must fall back to the boxed js_eq helper (the \ + element read can be undefined, which the STATIC numeric fast path \ + can't represent); got IR:\n{ir}" + ); + // NOT a blanket "no fcmp anywhere" check: `js_eq`'s own dynamic + // comparison lowering has an internal, RUNTIME-guarded fast path (an + // `icmp` range check on the raw bits proves both operands are genuine + // untagged doubles before it dares an `fcmp`) that is safe and expected + // to appear here too — that is a property of the boxed path, not the + // STATIC always-numeric bug this test guards against. The call to + // `js_eq` above is what proves this comparison did NOT take the static + // fast path (`lower_strict_eq_against_number`, which emits an + // UNGUARDED `fcmp` with no dynamic dispatch at all). +} + +/// Control: the SAME shape through a `let` (no redeclaration ambiguity) +/// already took the boxed path before this fix and must keep doing so. +#[test] +fn let_numeric_array_compare_against_void_stays_boxed() { + let ir = ir_for( + "let_void_compare", + vec![ + Stmt::Let { + id: ARR, + name: "arr".to_string(), + ty: Type::Array(Box::new(Type::Number)), + mutable: false, + init: Some(Expr::Array(vec![Expr::Integer(0)])), + }, + Stmt::Let { + id: R, + name: "r".to_string(), + ty: Type::Any, + mutable: true, + init: Some(Expr::Compare { + op: CompareOp::Eq, + left: Box::new(Expr::IndexGet { + object: Box::new(Expr::LocalGet(ARR)), + index: Box::new(Expr::Integer(1)), + }), + right: Box::new(Expr::Void(Box::new(Expr::Integer(0)))), + }), + }, + ], + ); + assert!( + ir.contains(JS_EQ_CALL), + "control case (let, no redeclaration) must already take the boxed \ + path; got IR:\n{ir}" + ); +} diff --git a/crates/perry-codegen/src/stmt/mod.rs b/crates/perry-codegen/src/stmt/mod.rs index a6ca584e6a..f6a334d8f1 100644 --- a/crates/perry-codegen/src/stmt/mod.rs +++ b/crates/perry-codegen/src/stmt/mod.rs @@ -30,6 +30,8 @@ mod let_buffer_views; mod let_object_facts; mod let_stmt; mod let_stmt_facts; +#[cfg(test)] +mod let_stmt_var_redeclare_tests; mod loops; mod masked_window_region; #[cfg(test)] diff --git a/test-files/test_gap_10488_var_array_void_compare.ts b/test-files/test_gap_10488_var_array_void_compare.ts new file mode 100644 index 0000000000..e5d62a7175 --- /dev/null +++ b/test-files/test_gap_10488_var_array_void_compare.ts @@ -0,0 +1,101 @@ +// #10488: `arr[i] === void 0` (and other undefined-valued comparisons) +// against an out-of-bounds/hole read of a `var`-declared number-literal +// array always compiled `false` (and `!==` always `true`). +// +// A hoisted `var` lowers to TWO HIR `Let`s sharing one LocalId: a body-entry +// predefine (`Any = undefined`) and the real declaration (`Array(Number) = +// [0]`, say). `lower_let` (crates/perry-codegen/src/stmt/let_stmt.rs) +// refreshes `ctx.proven_local_types` on every `Let`, but a redeclaration +// (the second `Let`, since the predefine already allocated the slot) took +// an early return that never touched `ctx.local_types`. That left +// `is_numeric_expr` (which consults `proven_local_types`) and +// `expr_may_return_boxed_value_from_raw_f64_fallback`/`static_type_of` +// (which read the now-stale `local_types`, still `Any`) disagreeing about +// the very same local: the fallback-hazard guard never fired, so a +// strict-equality compare against the array element compiled to a bare +// `fcmp` — which can't represent the NaN-boxed `undefined` tag a hole or +// out-of-bounds read actually produces. + +function varVoid(): boolean { + var arr = [0]; + return arr[1] === void 0; +} +function varNe(): boolean { + var arr = [0]; + return arr[1] !== void 0; +} +function varUndefVar(): boolean { + var arr = [0]; + var u; + return arr[1] === u; +} +function varTwoReads(): boolean { + var arr = [0]; + return arr[1] === arr[2]; +} +function varHole(): boolean { + var arr = [0, 1]; + arr.length = 5; + return arr[3] === void 0; +} +function varNegIdx(): boolean { + var arr = [0]; + return arr[-1] === void 0; +} +function varPropCompare(): boolean { + var arr = [0]; + var o: any = {}; + return arr[1] === o.v; +} +function varParamIdx(i: number): boolean { + var arr = [0]; + return arr[i] === void 0; +} +// Controls: already correct in Perry before this fix; must stay correct. +function varUndefined(): boolean { + var arr = [0]; + return arr[1] === undefined; +} +function letVoid(): boolean { + let arr = [0]; + return arr[1] === void 0; +} +function varInBoundsVoid(): boolean { + var arr = [0]; + return arr[0] === void 0; +} + +var NUMERALS = "0123456789abcdef"; +// decimal.js convertBase('255', 10, 16) (decimal.mjs:2608), verbatim. +function convertBase(str: string, baseIn: number, baseOut: number) { + var j, arr = [0], arrL, i = 0, strL = str.length; + for (; i < strL; ) { + for (arrL = arr.length; arrL--; ) arr[arrL] *= baseIn; + arr[0] += NUMERALS.indexOf(str.charAt(i++)); + for (j = 0; j < arr.length; j++) { + if (arr[j] > baseOut - 1) { + if (arr[j + 1] === void 0) arr[j + 1] = 0; + arr[j + 1] += (arr[j] / baseOut) | 0; + arr[j] %= baseOut; + } + } + } + return arr.reverse(); +} + +for (const f of [ + varVoid, + varNe, + varUndefVar, + varTwoReads, + varHole, + varNegIdx, + varPropCompare, + varUndefined, + letVoid, + varInBoundsVoid, +]) { + console.log(f.name, f()); +} +console.log("varParamIdx", varParamIdx(1)); +console.log("convertBase", JSON.stringify(convertBase("255", 10, 16))); From a8f85070594059d564a795675886ae50a55a700e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 18 Sep 2026 13:28:25 +0000 Subject: [PATCH 2/2] changelog: fragment for #10627 --- changelog.d/10627-var-array-void-compare.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 changelog.d/10627-var-array-void-compare.md diff --git a/changelog.d/10627-var-array-void-compare.md b/changelog.d/10627-var-array-void-compare.md new file mode 100644 index 0000000000..d1b1cae40c --- /dev/null +++ b/changelog.d/10627-var-array-void-compare.md @@ -0,0 +1,15 @@ +### Fixed + +- **`arr[i] === void 0` (and other undefined-valued comparisons) against an + out-of-bounds or hole read of a `var`-declared number array always + compiled `false`, and `!==` always `true`.** A hoisted `var` lowers to two + HIR declarations sharing one local id (a body-entry predefine, then the + real declaration); the codegen redeclaration path refreshed the numeric + type PROOF used by `is_numeric_expr` but left the declared-type map used + by the boxed-fallback hazard guard stale at `Any`. The two disagreed about + the same local, so the hazard guard never caught the case and a strict + equality compare against the array element compiled to a bare `fcmp` — + which cannot represent the NaN-boxed `undefined` tag a hole/out-of-bounds + read actually produces. Both are now kept in sync on every `var` + redeclaration. This was blocking `decimal.js`'s `toHexadecimal`/ + `toBinary`/`toOctal` (`convertBase`'s carry-slot initialization check).