Repository navigation
i32 fast path: a >>> 0 uint32 copied into a local with a signed literal seed takes a SIGNED i32 slot and reads back negative #6359
Description
Severity: P1 (silent data corruption)
Found while fixing: #6339
What
A >>> 0 result (a uint32, range 0 .. 2^32-1) assigned into a local whose other write is a plain signed integer literal gets a signed i32 shadow slot. Reads sitofp the bit pattern, so any value ≥ 2^31 comes back negative.
const SEED = 0x9e3779b9 >>> 0;
let seedCopy = 0;
seedCopy = SEED >>> 0;
console.log(seedCopy); // node: 2654435769 perry: -1640531527
let d = 0;
d = 4000000000 >>> 0;
console.log(d); // node: 4000000000 perry: -294967296Reproduced against origin/main, against the #6319 branch (#6340) and against the #6339 branch — all three print the same wrong values, so it is independent of that family's fixes.
What still works (so this is narrow)
const SEED = 0x9e3779b9 >>> 0;
console.log(SEED); // 2654435769 ✓ (const, no i32 slot at all)
let s = SEED >>> 0; // EVERY write is `>>> 0`
s = (s ^ ((s << 13) >>> 0)) >>> 0;
s = (s ^ (s >>> 17)) >>> 0;
console.log(s); // ✓ correct — unsigned_i32_locals pathimage_convolution's xorshift s is exactly the second shape, which is why the compiler-output-regression gate never caught this.
Why
Two sets disagree about signedness.
collect_unsigned_i32_locals(crates/perry-codegen/src/collectors/i32_locals.rs) requires every write to beexpr >>> 0. Those locals get the unsigned slot and are read back withuitofp.seedCopy'sLetinit is0— a plainExpr::Integer, not a>>> 0— so it is disqualified.is_strictly_i32_bounded_expracceptsexpr >>> 0as strictly-i32-bounded:
Expr::Binary { op, right, .. }
if matches!(op, BinaryOp::BitOr | BinaryOp::UShr)
&& matches!(right.as_ref(), Expr::Integer(0)) => true,But | 0 is ToInt32 (signed i32 — genuinely bounded) while >>> 0 is ToUint32 (unsigned). Both writes to seedCopy therefore pass, seedCopy lands in strictly_i32_bounded_locals, needs_i32_slot gives it the signed i32 shadow at its Let site, and every read sitofps 0x9E3779B9 to -1640531527.
So the local falls between the two stools: too mixed for the unsigned slot, wrongly admitted to the signed one.
Also suspect
The general bitwise arm just below returns true for any UShr:
Expr::Binary { op, .. } => matches!(op, BitAnd | BitOr | BitXor | Shl | Shr | UShr),x >>> k for a constant k >= 1 really does fit in signed i32 (max 2^31 - 1). But x >>> k with a variable k can be a shift by 0 at runtime, which is again a uint32 — same corruption, no literal 0 in the HIR to key on. Worth covering in the same fix.
Fix sketch
Treat "fits in signed i32" and "is a 32-bit bit pattern" as different properties:
- Drop
BinaryOp::UShrfrom theright == Integer(0)arm ofis_strictly_i32_bounded_expr—>>> 0does not prove a signed i32. - In the general bitwise arm, admit
UShronly when the shift amount is a literal>= 1. - Locals that lose the signed slot either qualify for
unsigned_i32_localsalready or fall back to the f64 slot — correct, marginally slower.
Alternatively, widen collect_unsigned_i32_locals so a local whose writes are a mix of in-range signed literals and >>> 0 casts can still take the unsigned slot. That keeps the fast path but needs the read side to pick uitofp consistently.
Either way it wants the compiler-output-regression gate run: image_convolution's xorshift s and FNV h both live in this machinery.
Confidence: high (probe + source; A/B'd against three separate builds).
Activity
fix: `>>> 0` uint32 copied over a signed-literal seed took a signed i…