diff --git a/crates/perry-codegen/src/collectors/i32_locals.rs b/crates/perry-codegen/src/collectors/i32_locals.rs index 97c3d5418..8b47f1223 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 9d2ba9025..d17d90a4b 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 000000000..48017ed10 --- /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]);