From e62c15eadcefbf7630b0b3f6a35a0da4d5561552 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 13 Jul 2026 07:33:27 +0200 Subject: [PATCH] fix(codegen): an integer literal past 2^31 must not seed the i32 fast path (#6319) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third face of #6072 (`i32 fast-path locals silently wrap at 2^31`), reached through the *copy* path rather than `x++` (#6258) or a dynamic loop bound (#6318): let x = 3000000000; let y = 0; y = x; console.log(y); // node: 3000000000 perry: -1294967296 `Expr::Integer` carries an `i64`, but every judgment in the i32 chain accepted `Expr::Integer(_)` regardless of magnitude. `x` itself stayed correct — its own `Let`-site slot is gated on `init_in_i32_range` — yet it was still admitted to `integer_locals`. `is_strictly_i32_bounded_expr`'s `LocalGet` arm then answered `known_int_locals.contains(id)`, so `y = x` counted as a strictly-i32-bounded write, `y` got an i32 shadow at its `Let` site, and the store truncated the 3e9 double. Reads prefer the shadow (issue #48), so that is what `console.log` saw. Denying the shadow is the only cure: the f64 fallback in `LocalSet` mirrors into the i32 slot with `fptosi`+`trunc` anyway (literals_vars.rs:634), so rejecting the value in `can_lower_expr_as_i32` would not have helped. Narrow all four judgments through one shared predicate, `integer_literal_fits_i32`: * `collect_integer_let_ids` — the syntactic seed * `is_int32_producing_expr` — the forward-closure admission * `int32_producing_deps` — the disqualification judgment * `is_strictly_i32_bounded_expr` — the write-is-i32-bounded proof All four are load-bearing. Narrowing only the seed does not hold: the forward closure re-admits from the `Let` init and the judgment re-admits from every `LocalSet` rhs. And `is_strictly_i32_bounded_expr` is an independent hole — `let y = 0; y = 3000000000;` wrapped without any copy at all. Two latent poison `fptosi`s go with it: the `arr.length`-hoist (loops.rs:2454) and the static `i < n` bound (loops.rs:2516) both install a counter's i32 shadow from `integer_locals` membership alone, seeding it with a bare `fptosi` of the counter's double — poison in LLVM for a counter past 2^31. Bitwise-coerced constants are untouched, so the imul32/FNV i32 chain that `compiler-output-regression` watches is unaffected: image_conv's two out-of-range constants are written `0x9E3779B9 >>> 0` and `0x811c9dc5 | 0`, which reach `integer_locals` through the `>>> 0` / `| 0` arms, not the bare `Expr::Integer` arm. `| 0`, `>>> 0`, bitwise ops and `Math.imul` genuinely produce int32 by spec; a bare literal does not. --- .../src/collectors/i32_locals.rs | 32 ++++++- .../src/collectors/integer_locals.rs | 16 +++- .../test_gap_6319_i32_literal_magnitude.ts | 95 +++++++++++++++++++ 3 files changed, 138 insertions(+), 5 deletions(-) create mode 100644 test-files/test_gap_6319_i32_literal_magnitude.ts diff --git a/crates/perry-codegen/src/collectors/i32_locals.rs b/crates/perry-codegen/src/collectors/i32_locals.rs index 97c3d5418e..8b47f12232 100644 --- a/crates/perry-codegen/src/collectors/i32_locals.rs +++ b/crates/perry-codegen/src/collectors/i32_locals.rs @@ -2,6 +2,30 @@ use std::collections::HashSet; use super::*; +/// An integer literal only proves an int32 value when it *is* one (#6319). +/// +/// `Expr::Integer` carries an `i64`, so it happily holds `3000000000` or +/// `9007199254740991` — values that are integral but do NOT fit in i32. Every +/// judgment in the i32 fast-path chain used to accept `Expr::Integer(_)` +/// unconditionally, which is how a literal past 2^31 reached an i32 shadow +/// slot and got silently `ToInt32`-wrapped (`3000000000` → `-1294967296`). +/// +/// This is the one predicate all four of those judgments now share: +/// `is_strictly_i32_bounded_expr` (the write-is-i32-bounded proof), +/// `collect_integer_let_ids` (the syntactic seed), `is_int32_producing_expr` +/// (the forward-closure admission) and `int32_producing_deps` (the +/// disqualification judgment). Narrowing only the seed would not have held: +/// the forward closure re-admits from the init, and the judgment re-admits +/// from every `LocalSet` rhs. +/// +/// Deliberately *not* widened to "any value ToInt32 can wrap". `| 0`, `>>> 0`, +/// bitwise ops and `Math.imul` all wrap by spec, so their results are genuinely +/// int32 and keep their own arms below; a bare literal does not wrap — `let x = +/// 3000000000` is the Number 3000000000 and must stay a double. +pub(crate) fn integer_literal_fits_i32(n: i64) -> bool { + i32::try_from(n).is_ok() +} + pub fn is_strictly_i32_bounded_expr( e: &perry_hir::Expr, known_int_locals: &HashSet, @@ -11,7 +35,11 @@ pub fn is_strictly_i32_bounded_expr( ) -> bool { use perry_hir::{BinaryOp, Expr}; match e { - Expr::Integer(_) => true, + // #6319: an out-of-i32-range literal is NOT i32-bounded. Without the + // magnitude check, `let y = 0; y = 3000000000;` counted every write to + // `y` as strict, `y` got an i32 shadow at its `Let` site, and the store + // truncated to -1294967296. + Expr::Integer(n) => integer_literal_fits_i32(*n), // `x++`/`x--` is i32-bounded ONLY when the updated local is itself a // known integer (a loop counter). The previous unconditional `true` // truncated `var y = x++` to an i32 slot even when `x` held a @@ -627,7 +655,7 @@ pub fn collect_integer_let_ids( init: Some(init), mutable, .. - } if matches!(init, Expr::Integer(_)) + } if matches!(init, Expr::Integer(n) if integer_literal_fits_i32(*n)) || is_flat_const_indexget(init, flat_const_ids, flat_row_alias_ids) || is_clamp_call(init, clamp_fn_ids) // Seed immutable (const) Lets whose init is a bitwise expression diff --git a/crates/perry-codegen/src/collectors/integer_locals.rs b/crates/perry-codegen/src/collectors/integer_locals.rs index 9d2ba9025a..d17d90a4b5 100644 --- a/crates/perry-codegen/src/collectors/integer_locals.rs +++ b/crates/perry-codegen/src/collectors/integer_locals.rs @@ -347,7 +347,10 @@ fn int32_producing_deps( ) }; match e { - Expr::Integer(_) => true, + // #6319: only a literal that actually fits in i32 proves an int32 + // value. `let x = 3000000000` is integral but not int32, and admitting + // it here let every copy of `x` inherit an unearned i32 shadow. + Expr::Integer(n) => super::i32_locals::integer_literal_fits_i32(*n), Expr::Update { id, .. } if candidates.contains(id) => { deps.insert(*id); true @@ -623,7 +626,11 @@ pub fn collect_flat_row_aliases( /// `int32_producing_deps` during disqualification — see the module comment. /// /// Accepted shapes: -/// - `Expr::Integer(_)`: trivially integer. +/// - `Expr::Integer(n)` **when `n` fits in i32** (#6319). `Expr::Integer` +/// carries an `i64`, so `let x = 3000000000` and `let x = +/// Number.MAX_SAFE_INTEGER` parse into it too — integral, but not int32. +/// Admitting them let a plain copy (`let y = 0; y = x`) be judged +/// i32-bounded and take a wrapping i32 shadow. /// - `(expr) | 0` and `(expr) >>> 0`: the JS ToInt32 / ToUint32 idiom — /// always yields a 32-bit integer regardless of the inner expression. /// - Pure bitwise ops (`&`, `|`, `^`, `<<`, `>>`, `>>>`): per JS spec @@ -653,7 +660,10 @@ pub fn is_int32_producing_expr( ) -> bool { use perry_hir::{BinaryOp, Expr}; match e { - Expr::Integer(_) => true, + // #6319: an `Expr::Integer` holds an i64. Only accept it when it really + // is an int32 — otherwise the forward-closure pass re-admits the very + // `let x = 3000000000` that the syntactic seed now rejects. + Expr::Integer(n) => super::i32_locals::integer_literal_fits_i32(*n), Expr::Update { .. } => true, Expr::Binary { op, right, .. } if matches!(op, BinaryOp::BitOr | BinaryOp::UShr) diff --git a/test-files/test_gap_6319_i32_literal_magnitude.ts b/test-files/test_gap_6319_i32_literal_magnitude.ts new file mode 100644 index 0000000000..48017ed107 --- /dev/null +++ b/test-files/test_gap_6319_i32_literal_magnitude.ts @@ -0,0 +1,95 @@ +// #6319 (third face of #6072): an integer literal past 2^31 still seeded the +// i32 fast path, so *copying* such a local wrapped it. +// +// `let x = 3000000000` kept the correct double in its own slot (the `Let`-site +// gate checks the literal's magnitude), but it was still admitted to +// `integer_locals`. `let y = 0; y = x;` then counted as a "strictly i32-bounded" +// write, `y` got an i32 shadow, and the store truncated it — `y` read back as +// -1294967296. Same for a direct out-of-range store, a copy chain, and a +// counter seeded past 2^31 under an `arr.length` / `i < n` loop. + +// ---- the issue's repro ---- +let x = 3000000000; +let y = 0; +y = x; +console.log("copy:", y); + +let z = 0; +z = x + 4; +console.log("add:", z); + +// ---- direct out-of-range store into an otherwise-i32 local ---- +let d = 0; +d = 3000000000; +console.log("direct store:", d); + +// ---- boundary neighbourhood, each copied through a fresh local ---- +function copyOf(v: number): number { + let out = 0; + out = v; + return out; +} +const a = 2147483647; // 2^31 - 1 — fits, stays on the i32 path +const b = 2147483648; // 2^31 +const c = 2147483649; // 2^31 + 1 +const e = 4294967296; // 2^32 +const f = -2147483648; // -2^31 — fits +const g = -2147483649; // -2^31 - 1 +const h = 9007199254740991; // Number.MAX_SAFE_INTEGER +console.log("2^31-1:", copyOf(a)); +console.log("2^31:", copyOf(b)); +console.log("2^31+1:", copyOf(c)); +console.log("2^32:", copyOf(e)); +console.log("-2^31:", copyOf(f)); +console.log("-2^31-1:", copyOf(g)); +console.log("MAX_SAFE:", copyOf(h)); + +// ---- copy chain ---- +const c1 = 5000000000; +const c2 = c1; +const c3 = c2; +console.log("chain:", c3); + +let m1 = 0; +let m2 = 0; +let m3 = 0; +m1 = 6000000000; +m2 = m1; +m3 = m2; +console.log("mut chain:", m3); + +// ---- a counter seeded past 2^31 under an `arr.length` bound ---- +// The length-hoist path used to install the counter's i32 shadow from +// `integer_locals` membership alone, seeding it with a poison `fptosi`. +const arr = [11, 22, 33]; +let big = 3000000000; +const hits: number[] = []; +for (let i = 0; i < arr.length; i++) { + hits.push(big + arr[i]); +} +console.log("length-hoist:", hits.join(","), "big:", big); + +// ---- the i32 chain that must stay exact: FNV-1a over bytes ---- +function imul32(p: number, q: number): number { + return Math.imul(p, q); +} +let fnv = 0x811c9dc5 | 0; +const bytes = [1, 2, 3, 4, 5, 250, 251, 252]; +for (let i = 0; i < bytes.length; i++) { + fnv = (fnv ^ bytes[i]) | 0; + fnv = imul32(fnv, 0x01000193); +} +const hash = fnv >>> 0; +console.log("fnv:", hash.toString(16).padStart(8, "0")); + +// A `>>> 0` seed above INT32_MAX stays unsigned and must not be truncated. +const SEED = 0x9e3779b9 >>> 0; +console.log("seed:", SEED, (SEED | 0) === -1640531527); + +// ---- the array-index fast path must survive ---- +const nums = [10, 20, 30, 40, 50]; +let acc = 0; +for (let i = 0; i < nums.length; i++) { + acc = acc + nums[i]; +} +console.log("arrsum:", acc, "nums[2]:", nums[2]);