Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions crates/perry-codegen/src/collectors/i32_locals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<u32>,
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
16 changes: 13 additions & 3 deletions crates/perry-codegen/src/collectors/integer_locals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
95 changes: 95 additions & 0 deletions test-files/test_gap_6319_i32_literal_magnitude.ts
Original file line number Diff line number Diff line change
@@ -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]);
Loading