diff --git a/crates/perry-codegen/src/collectors/i32_locals.rs b/crates/perry-codegen/src/collectors/i32_locals.rs index 5d98c708c8..d5e0fdf02a 100644 --- a/crates/perry-codegen/src/collectors/i32_locals.rs +++ b/crates/perry-codegen/src/collectors/i32_locals.rs @@ -68,23 +68,35 @@ pub fn is_strictly_i32_bounded_expr( } ok } - // `expr | 0` / `expr >>> 0` ToInt32/ToUint32 idioms — explicit i32 - // coercion, hard-bounded. - Expr::Binary { op, right, .. } - if matches!(op, BinaryOp::BitOr | BinaryOp::UShr) - && matches!(right.as_ref(), Expr::Integer(0)) => - { - true - } - // Pure bitwise — always i32 per JS spec. + // `expr | 0` ToInt32 idiom — explicit *signed* i32 coercion, + // hard-bounded. `>>> 0` is deliberately NOT here (#6359): it is + // ToUint32, whose result (0..2^32-1) does NOT round-trip through a + // signed i32 slot. A write like `seedCopy = 0x9e3779b9 >>> 0` was + // admitted as strict, `seedCopy` took a signed i32 shadow, and the + // read `sitofp`'d it back to -1640531527. `>>> 0` falls to the UShr + // arm below, which rejects it. + Expr::Binary { + op: BinaryOp::BitOr, + right, + .. + } if matches!(right.as_ref(), Expr::Integer(0)) => true, + // `x >>> k` is ToUint32 (0..2^32-1). It fits in a *signed* i32 slot + // only when the effective shift `k & 31` is nonzero — that drops the + // top bit, capping the result at 2^31-1. `>>> 0` (a u32 cast), + // `>>> 32` (== `>>> 0`), and `>>> ` (which can be a + // shift-by-0 at runtime) can all exceed 2^31-1 and must NOT take a + // signed i32 slot (#6359). + Expr::Binary { + op: BinaryOp::UShr, + right, + .. + } => { + matches!(right.as_ref(), Expr::Integer(k) if k & 31 != 0) + } + // Pure bitwise — always signed i32 per JS spec. Expr::Binary { op, .. } => matches!( op, - BinaryOp::BitAnd - | BinaryOp::BitOr - | BinaryOp::BitXor - | BinaryOp::Shl - | BinaryOp::Shr - | BinaryOp::UShr + BinaryOp::BitAnd | BinaryOp::BitOr | BinaryOp::BitXor | BinaryOp::Shl | BinaryOp::Shr ), Expr::Call { callee, .. } => { if let Expr::FuncRef(fid) = callee.as_ref() { diff --git a/test-files/test_gap_6359_ushr_uint32_signed_slot.ts b/test-files/test_gap_6359_ushr_uint32_signed_slot.ts new file mode 100644 index 0000000000..cbdb609600 --- /dev/null +++ b/test-files/test_gap_6359_ushr_uint32_signed_slot.ts @@ -0,0 +1,91 @@ +// #6359 (fifth face of the #6072 i32 fast-path family): a `>>> 0` uint32 +// copied into a local whose *other* write is a signed integer literal took a +// SIGNED i32 slot and read back negative. +// +// `>>> 0` is ToUint32 (result 0..2^32-1), NOT ToInt32. But +// `is_strictly_i32_bounded_expr` used to accept `expr >>> 0` in the same arm as +// `expr | 0`, and it accepted *any* `UShr` in the general bitwise arm. So a +// local mixing a plain `0` init with a `>>> 0` write (too mixed for the +// unsigned-i32 slot, wrongly admitted to the signed one) landed in +// `strictly_i32_bounded_locals`, got a signed i32 shadow at its `Let`, and +// every read `sitofp`'d the bit pattern: `0x9e3779b9` → -1640531527. +// +// Fix: `>>> 0` no longer proves a signed i32. `x >>> k` is admitted to a signed +// slot only when the effective shift `k & 31` is a nonzero literal (drops the +// top bit, capping the value at 2^31-1). `>>> 0`, `>>> 32`, and `>>> ` +// (which can be a shift-by-0 at runtime) all fall back to the f64 slot. + +// ---- the issue's exact repro ---- +const SEED = 0x9e3779b9 >>> 0; + +let seedCopy = 0; +seedCopy = SEED >>> 0; +console.log("seedCopy:", seedCopy); // 2654435769 + +let d = 0; +d = 4000000000 >>> 0; +console.log("d:", d); // 4000000000 + +// ---- a whole boundary matrix of `>>> 0` copied over a signed-literal seed ---- +function ushrCopy(v: number): number { + let out = 0; // signed literal seed — the poison + out = v >>> 0; // uint32 write + return out; +} +console.log("2^31-1:", ushrCopy(2147483647)); // 2147483647 +console.log("2^31:", ushrCopy(2147483648)); // 2147483648 +console.log("2^31+1:", ushrCopy(2147483649)); // 2147483649 +console.log("2^32-1:", ushrCopy(4294967295)); // 4294967295 +console.log("neg wraps unsigned:", ushrCopy(-1)); // 4294967295 +console.log("small stays small:", ushrCopy(42)); // 42 + +// ---- the "also suspect" case: `x >>> k` with a VARIABLE k ---- +// k can be 0 at runtime, making the result a uint32 with no literal `0` in the +// HIR to key on. The variable-shift write must not confer a signed i32 slot. +function ushrVar(x: number, k: number): number { + let out = 0; + out = x >>> k; + return out; +} +console.log("ushrVar k=0:", ushrVar(0x9e3779b9, 0)); // 2654435769 +console.log("ushrVar k=1:", ushrVar(0x9e3779b9, 1)); // 1327217884 +console.log("ushrVar k=8:", ushrVar(0xffffffff, 8)); // 16777215 + +// ---- copy chain: disqualification must propagate transitively ---- +let a1 = 3000000000 >>> 0; // 3000000000 +let a2 = 0; +let a3 = 0; +a2 = a1; +a3 = a2; +console.log("copy chain:", a3); // 3000000000 + +// ==== what MUST still work (fast path preserved) ==== + +// A `>>> k` with a literal k>=1 genuinely fits signed i32; copying it stays +// correct whether or not it keeps the fast i32 slot. +function ushrLit(v: number): number { + let out = 0; + out = v >>> 1; // <= 2^31-1, signed-safe + return out; +} +console.log("ushr>>1:", ushrLit(0xffffffff)); // 2147483647 +console.log("ushr>>16:", 0xffffffff >>> 16); // 65535 + +// `| 0` (ToInt32) still proves a signed i32 slot and still wraps mod 2^32. +let h = 0x811c9dc5 | 0; +const bytes = [1, 2, 3, 4, 5, 250, 251, 252]; +for (let i = 0; i < bytes.length; i++) { + h = (h ^ bytes[i]) | 0; + h = Math.imul(h, 0x01000193); +} +console.log("fnv:", (h >>> 0).toString(16).padStart(8, "0")); + +// An all-`>>> 0` xorshift keeps its (unsigned) i32 slot and wraps mod 2^32. +let s = SEED >>> 0; +s = (s ^ ((s << 13) >>> 0)) >>> 0; +s = (s ^ (s >>> 17)) >>> 0; +s = (s ^ ((s << 5) >>> 0)) >>> 0; +console.log("xorshift:", s); + +// A const `>>> 0` seed above INT32_MAX prints unsigned (no i32 slot at all). +console.log("SEED:", SEED, (SEED | 0) === -1640531527);