From 3d8e2ced2e2ffd96401c071f7534d2689f6cf6e7 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Sat, 2 May 2026 10:38:18 +0200 Subject: [PATCH] fix(cpu): PPCBUG-053+054 32-bit CTR semantics in bcx/bclrx + mtspr CTR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PPCBUG-053: bcx and bclrx tested `ctx.ctr != 0` against the full 64-bit register, but the Xbox 360 ABI runs CTR as a 32-bit counter (canary explicitly truncates: `f.Truncate(ctr, INT32_TYPE)`). When upstream 64-bit GPR pollution flowed through `mtspr CTR, rN`, the upper 32 bits stayed non-zero forever; bdnz then looped past the intended 32-bit zero point because the 64-bit comparison still saw the high bits. PPCBUG-054: `mtspr CTR` writeback wrote the full 64-bit GPR value, acting as a firewall gap that fed PPCBUG-053. Defensive truncation prevents CTR from ever acquiring non-zero upper 32 bits independently of the GPR-pollution source. Fixes: - interpreter.rs:849, 879: ctr_ok now uses `(ctx.ctr as u32) != 0` - interpreter.rs:1523: mtspr CTR writes `val as u32 as u64` Tests: - bcx_bdnz_uses_32bit_ctr_compare: bdnz with CTR=0x0000_0001_0000_0001 decrements to 0x0000_0001_0000_0000 and exits (low 32 bits = 0). - bclrx_uses_32bit_ctr_compare: same coverage for bdnzlr. - mtspr_ctr_truncates_to_32_bits: gpr=0xFFFF_FFFF_8000_0001 → ctr=0x8000_0001. Coupled fix per the audit: PPCBUG-053 and PPCBUG-054 land together because either alone is necessary-but-not-sufficient — the truncation prevents new pollution, the 32-bit compare protects against any pollution that slipped in via routes other than mtspr (e.g. mfctr-mtctr roundtrips). Co-Authored-By: Claude Sonnet 4.6 --- crates/xenia-cpu/src/interpreter.rs | 59 +++++++++++++++++++++++++++-- 1 file changed, 56 insertions(+), 3 deletions(-) diff --git a/crates/xenia-cpu/src/interpreter.rs b/crates/xenia-cpu/src/interpreter.rs index eff89de..09800b8 100644 --- a/crates/xenia-cpu/src/interpreter.rs +++ b/crates/xenia-cpu/src/interpreter.rs @@ -846,7 +846,7 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - } let ctr_ok = (bo & 0b00100) != 0 - || ((ctx.ctr != 0) ^ ((bo & 0b00010) != 0)); + || (((ctx.ctr as u32) != 0) ^ ((bo & 0b00010) != 0)); let cond_ok = (bo & 0b10000) != 0 || (ctx.get_cr_bit(bi) == ((bo & 0b01000) != 0)); @@ -876,7 +876,7 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - } let ctr_ok = (bo & 0b00100) != 0 - || ((ctx.ctr != 0) ^ ((bo & 0b00010) != 0)); + || (((ctx.ctr as u32) != 0) ^ ((bo & 0b00010) != 0)); let cond_ok = (bo & 0b10000) != 0 || (ctx.get_cr_bit(bi) == ((bo & 0b01000) != 0)); @@ -1520,7 +1520,7 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - match spr { crate::context::spr::XER => ctx.set_xer(val as u32), crate::context::spr::LR => ctx.lr = val, - crate::context::spr::CTR => ctx.ctr = val, + crate::context::spr::CTR => ctx.ctr = val as u32 as u64, crate::context::spr::DEC => ctx.dec = val as u32, crate::context::spr::TBL_WRITE => { ctx.timebase = (ctx.timebase & 0xFFFF_FFFF_0000_0000) | (val & 0xFFFF_FFFF); @@ -5834,6 +5834,59 @@ mod tests { assert_eq!(ctx.timebase >> 32, 0xAAAA_BBBB); } + // PPCBUG-053: bcx CTR zero-test must use 32-bit comparison. When prior + // 64-bit pollution (e.g. via negx → mtctr) leaves CTR upper 32 bits + // non-zero, the 64-bit `ctx.ctr != 0` would loop forever even when the + // 32-bit counter has decremented to zero. + #[test] + fn bcx_bdnz_uses_32bit_ctr_compare() { + let mut ctx = PpcContext::new(); + let mut mem = TestMem::new(); + ctx.ctr = 0x0000_0001_0000_0001; + // bdnz +8: BO=16 (decrement, branch if CTR!=0, ignore CR), BI=0, BD/4=2 + let raw = (16u32 << 26) | (16 << 21) | (0 << 16) | (2 << 2); + write_instr(&mut mem, 0, raw); + ctx.pc = 0; + step(&mut ctx, &mut mem); + // After decrement: low 32 = 0, high 32 = 1. 32-bit test says zero → no branch. + assert_eq!(ctx.ctr, 0x0000_0001_0000_0000); + assert_eq!(ctx.pc, 4); + } + + #[test] + fn bclrx_uses_32bit_ctr_compare() { + let mut ctx = PpcContext::new(); + let mut mem = TestMem::new(); + ctx.ctr = 0x0000_0001_0000_0001; + ctx.lr = 0x100; + // bdnzlr: opcode 19, BO=16 (decrement, branch if CTR!=0), BI=0, XO=16 + let raw = (19u32 << 26) | (16 << 21) | (0 << 16) | (16 << 1); + write_instr(&mut mem, 0, raw); + ctx.pc = 0; + step(&mut ctx, &mut mem); + // 32-bit CTR=0 after decrement → don't branch to LR. + assert_eq!(ctx.ctr, 0x0000_0001_0000_0000); + assert_eq!(ctx.pc, 4); + } + + // PPCBUG-054: mtspr CTR must truncate the source GPR to 32 bits, matching + // canary's `f.Truncate(ctr, INT32_TYPE)`. Prevents upstream 64-bit GPR + // pollution from poisoning the 32-bit CTR counter independently of the + // bcx zero-test fix. + #[test] + fn mtspr_ctr_truncates_to_32_bits() { + let mut ctx = PpcContext::new(); + let mut mem = TestMem::new(); + ctx.gpr[3] = 0xFFFF_FFFF_8000_0001; + // mtspr CTR (9), r3 + let spr_swapped = ((9u32 & 0x1F) << 5) | ((9u32 >> 5) & 0x1F); + let raw = (31u32 << 26) | (3 << 21) | (spr_swapped << 11) | (467 << 1); + write_instr(&mut mem, 0, raw); + ctx.pc = 0; + step(&mut ctx, &mut mem); + assert_eq!(ctx.ctr, 0x8000_0001); + } + // ---------- Block-cache parity tests ---------- // // These confirm that running a program through the basic-block