From 48b19e490f9076e7d83698d7818248924096c3d4 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Fri, 12 Jun 2026 17:25:41 +0200 Subject: [PATCH] [Prong A] Three 32-bit ABI PPCBUG siblings corrected to canary semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second differential audit, lead prong: hunt siblings of PPCBUG-020 (the word-form ALU truncation fixed in 341196a, whose "32-bit ABI / MSR.SF=0" premise was false — Xenon is a 64-bit core). Found three more band-aids of the same class, each verified against the canary oracle. All three are genuine oracle/ISA divergences but INERT on Sylpheed's lockstep trace (sylpheed_n50m golden digest unchanged; no re-baseline). Fixed + directed tests anyway to close the band-aid class (per audit decision). 1. slw/srw shift-count mask (PPCBUG-044 site). Ours tested the full u32 count `< 32`; canary InstrEmit_slwx/srwx mask `rb & 0x3F` then test bit 5. A count like 0x40 (low-6-bits 0) must pass the value through, not zero it. Fixed both to `& 0x3F`. The 32-bit CR0 i32-view is unchanged (genuinely 32-bit). 2. sraw/srawi result extension (PPCBUG-041/042/043 "writeback truncation"). Ours zero-extended the 32-bit arithmetic-shift result (`result as u32 as u64`); PowerISA + canary InstrEmit_srawx/srawix SIGN-extend it (`f.SignExtend`, the `(i64.s)&¬m` fill). 0x80000000>>1 is now 0xFFFFFFFF_C0000000, not 0x00000000_C0000000. CA math and CR0 view byte-identical. 3. mtspr CTR width (PPCBUG-054). Ours stored `val as u32 as u64`, dropping the upper 32 bits; CTR is a 64-bit SPR and canary InstrEmit_mtspr stores the full GPR (`f.StoreCTR(rt)`). A later `mfspr rX, CTR` now round-trips correctly. bdnz/bcctr still consume only CTR's low 32 bits (the bcx zero-TEST truncation at line ~922 MATCHES canary's `f.Truncate(ctr, INT32_TYPE)` — left untouched). Tests: updated srawx_negative_value_sign_extends_upper, srawix_high_count_negative_input_sign_extends_all_ones, and mtspr_ctr_keeps_full_64_bits (formerly premise-defending the bugs — reading-error #24). Added slwx/srwx 6-bit-mask tests, mfspr_ctr round-trip, and the rlwinm MB>ME wraparound-mask test (plan-requested gap closure). 665/665. Left correct (re-confirmed vs canary, do NOT touch): bcx/bclr CTR 32-bit test, divw/divwu zero-extend quotient (canary f.ZeroExtend, ISA upper undefined), extsb/extsh, logical-NOT chain, mulhw/mulhwu, srawx 0x3F mask, pixel pack/unpack. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/xenia-cpu/src/interpreter.rs | 173 ++++++++++++++++++++++------ 1 file changed, 140 insertions(+), 33 deletions(-) diff --git a/crates/xenia-cpu/src/interpreter.rs b/crates/xenia-cpu/src/interpreter.rs index 7d832ce..29288a3 100644 --- a/crates/xenia-cpu/src/interpreter.rs +++ b/crates/xenia-cpu/src/interpreter.rs @@ -626,7 +626,12 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - PpcOpcode::slwx => { // PPCBUG-044: 32-bit ABI CR0 view. A result with bit 31 set // (e.g. 0x80000000) is negative in i32 view but positive in i64. - let sh = ctx.gpr[instr.rb()] as u32; + // Shift amount is RB[58:63] (6 bits): if >=32 the result is zeroed, + // else shift by the low bits. Matches canary InstrEmit_slwx, which + // masks `rb & 0x3F` then tests bit 5 — NOT a full-u32 `< 32` test + // (a count like 0x40 has low-6-bits 0 and must pass the value + // through, not zero it). + let sh = ctx.gpr[instr.rb()] as u32 & 0x3F; ctx.gpr[instr.ra()] = if sh < 32 { ((ctx.gpr[instr.rs()] as u32) << sh) as u64 } else { 0 }; @@ -636,7 +641,9 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - PpcOpcode::srwx => { // PPCBUG-044: 32-bit ABI CR0 view (zero-extended right shift can never // have bit 31 set, but use the canonical form for consistency). - let sh = ctx.gpr[instr.rb()] as u32; + // Shift amount masked to RB[58:63] (6 bits) to match canary + // InstrEmit_srwx (`rb & 0x3F`, test bit 5). + let sh = ctx.gpr[instr.rb()] as u32 & 0x3F; ctx.gpr[instr.ra()] = if sh < 32 { ((ctx.gpr[instr.rs()] as u32) >> sh) as u64 } else { 0 }; @@ -644,37 +651,46 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - ctx.pc += 4; } PpcOpcode::srawx => { - // PPCBUG-041+043 coupled: 32-bit ABI writeback truncation + CR0 i32. - // CA logic is independently correct (uses u32 shifted-out test). + // sraw: 32-bit arithmetic shift right. Per PowerISA the 32-bit result + // is SIGN-extended into the full 64-bit RA (`RA <- r&m | (i64.s)&¬m`), + // matching canary InstrEmit_srawx (`v = f.SignExtend(v, INT64_TYPE)`). + // Earlier ours zero-extended (`result as u32 as u64`) — the PPCBUG-041 + // "writeback truncation" band-aid — which corrupts any negative shift + // result consumed as a 64-bit value. CA logic is independently correct + // (uses the u32 shifted-out test) and the CR0 view is unchanged (the + // sign-extended i64 has the same i32 view). let rs = ctx.gpr[instr.rs()] as i32; let sh = ctx.gpr[instr.rb()] as u32 & 0x3F; - if sh == 0 { - ctx.gpr[instr.ra()] = rs as u32 as u64; + let result: i32 = if sh == 0 { ctx.xer_ca = 0; + rs } else if sh < 32 { - let result = rs >> sh; ctx.xer_ca = if rs < 0 && (rs as u32) << (32 - sh) != 0 { 1 } else { 0 }; - ctx.gpr[instr.ra()] = result as u32 as u64; + rs >> sh } else { - ctx.gpr[instr.ra()] = if rs < 0 { 0xFFFF_FFFFu64 } else { 0 }; + // sh >= 32: result is all sign bits of rs. ctx.xer_ca = if rs < 0 { 1 } else { 0 }; - } - if instr.rc_bit() { ctx.update_cr_signed(0, ctx.gpr[instr.ra()] as u32 as i32 as i64); } + rs >> 31 + }; + ctx.gpr[instr.ra()] = result as i64 as u64; + if instr.rc_bit() { ctx.update_cr_signed(0, result as i64); } ctx.pc += 4; } PpcOpcode::srawix => { - // PPCBUG-042+043 coupled: same shape as srawx for the sh-immediate form. + // srawi: same as srawx for the sh-immediate form (sh in 0..31). + // Sign-extend the 32-bit result into the full 64-bit RA per PowerISA / + // canary InstrEmit_srawix. let rs = ctx.gpr[instr.rs()] as i32; let sh = instr.sh(); - if sh == 0 { - ctx.gpr[instr.ra()] = rs as u32 as u64; + let result: i32 = if sh == 0 { ctx.xer_ca = 0; + rs } else { - let result = rs >> sh; ctx.xer_ca = if rs < 0 && (rs as u32) << (32 - sh) != 0 { 1 } else { 0 }; - ctx.gpr[instr.ra()] = result as u32 as u64; - } - if instr.rc_bit() { ctx.update_cr_signed(0, ctx.gpr[instr.ra()] as u32 as i32 as i64); } + rs >> sh + }; + ctx.gpr[instr.ra()] = result as i64 as u64; + if instr.rc_bit() { ctx.update_cr_signed(0, result as i64); } ctx.pc += 4; } PpcOpcode::sldx => { @@ -1611,7 +1627,12 @@ 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 as u32 as u64, + // CTR is a 64-bit SPR — store the full GPR, matching canary + // InstrEmit_mtspr (`f.StoreCTR(rt)`, no truncation). The PPCBUG-054 + // `val as u32 as u64` band-aid dropped the upper 32 bits, which a + // later `mfspr rX, CTR` would read back wrong. (bdnz/bcctr only + // ever consume CTR's low 32 bits, so branching is unaffected.) + crate::context::spr::CTR => ctx.ctr = val, 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); @@ -5545,9 +5566,74 @@ mod tests { } #[test] - fn srawx_negative_value_zero_extends_upper() { - // PPCBUG-041+043: srawx of negative i32 by 1 produces a negative i32; - // writeback must zero-extend to u64 (not sign-extend). + fn slwx_shift_count_masks_to_6_bits() { + // slw masks the shift count to RB[58:63] (6 bits): a count of 0x40 has + // low-6-bits 0, so the value passes through unchanged — it must NOT be + // zeroed by a naive full-u32 `>= 32` test. Matches canary InstrEmit_slwx. + let mut ctx = PpcContext::new(); + let mut mem = TestMem::new(); + ctx.gpr[3] = 0x0000_1234u64; + ctx.gpr[4] = 0x40; // count & 0x3F == 0 → shift by 0 + // slwx r5, r3, r4 (XO=24) + let raw = (31u32 << 26) | (3 << 21) | (5 << 16) | (4 << 11) | (24 << 1); + write_instr(&mut mem, 0, raw); + ctx.pc = 0; + step(&mut ctx, &mut mem); + assert_eq!(ctx.gpr[5], 0x0000_1234u64, "0x40 masks to 0 → passthrough"); + } + + #[test] + fn slwx_count_32_to_63_zeroes() { + // A masked count in [32,63] (bit 5 set) zeroes the result. + let mut ctx = PpcContext::new(); + let mut mem = TestMem::new(); + ctx.gpr[3] = 0xFFFF_FFFFu64; + ctx.gpr[4] = 0x60; // & 0x3F = 0x20 (32) → zero + let raw = (31u32 << 26) | (3 << 21) | (5 << 16) | (4 << 11) | (24 << 1); + write_instr(&mut mem, 0, raw); + ctx.pc = 0; + step(&mut ctx, &mut mem); + assert_eq!(ctx.gpr[5], 0); + } + + #[test] + fn srwx_shift_count_masks_to_6_bits() { + // srw, same 6-bit mask. Count 0x48 → low-6-bits = 8 → logical >> 8. + let mut ctx = PpcContext::new(); + let mut mem = TestMem::new(); + ctx.gpr[3] = 0x0000_FF00u64; + ctx.gpr[4] = 0x48; // & 0x3F = 8 + // srwx r5, r3, r4 (XO=536) + let raw = (31u32 << 26) | (3 << 21) | (5 << 16) | (4 << 11) | (536 << 1); + write_instr(&mut mem, 0, raw); + ctx.pc = 0; + step(&mut ctx, &mut mem); + assert_eq!(ctx.gpr[5], 0x0000_00FFu64, "0x48 masks to 8 → >>8"); + } + + #[test] + fn rlwinm_mb_greater_than_me_wraparound_mask() { + // rlwinm with MB > ME produces a wraparound mask covering bits + // [0..ME] ∪ [MB..31] (a "split" mask). PowerISA MASK(mb,me) wraps when + // mb > me. Here rotate by 0, MB=28, ME=3 → mask = 0xF000000F. + let mut ctx = PpcContext::new(); + let mut mem = TestMem::new(); + ctx.gpr[3] = 0xFFFF_FFFFu64; + // rlwinm r5, r3, SH=0, MB=28, ME=3 (opcode 21) + let raw = (21u32 << 26) | (3 << 21) | (5 << 16) | (0 << 11) | (28 << 6) | (3 << 1); + write_instr(&mut mem, 0, raw); + ctx.pc = 0; + step(&mut ctx, &mut mem); + assert_eq!(ctx.gpr[5], 0x0000_0000_F000_000Fu64, + "MB>ME wraparound mask = bits [0..3] | [28..31]"); + } + + #[test] + fn srawx_negative_value_sign_extends_upper() { + // sraw of negative i32 by 1 produces a negative i32 result that PowerISA + // SIGN-extends into the full 64-bit RA (canary InstrEmit_srawx uses + // `f.SignExtend`). 0x80000000 >> 1 = 0xC0000000 (i32) → 0xFFFFFFFF_C0000000. + // (Was 0x00000000_C0000000 under the PPCBUG-041 zero-extend band-aid.) let mut ctx = PpcContext::new(); let mut mem = TestMem::new(); ctx.gpr[3] = 0x8000_0000u64; // i32::MIN @@ -5557,14 +5643,15 @@ mod tests { write_instr(&mut mem, 0, raw); ctx.pc = 0; step(&mut ctx, &mut mem); - assert_eq!(ctx.gpr[5], 0x0000_0000_C000_0000u64); + assert_eq!(ctx.gpr[5], 0xFFFF_FFFF_C000_0000u64); assert!(ctx.cr[0].lt); } #[test] - fn srawix_high_count_negative_input_yields_low32_all_ones() { - // PPCBUG-042+043: srawi with count=31 on negative input → low 32 bits - // all ones (0xFFFFFFFF), upper 32 zero (was u64::MAX before fix). + fn srawix_high_count_negative_input_sign_extends_all_ones() { + // srawi count=31 on negative input → result is -1 (0xFFFFFFFF as i32), + // sign-extended to the full 64-bit RA: 0xFFFFFFFF_FFFFFFFF (canary + // InstrEmit_srawix). Was 0x00000000_FFFFFFFF under the zero-extend band-aid. let mut ctx = PpcContext::new(); let mut mem = TestMem::new(); ctx.gpr[3] = 0x8000_0000u64; @@ -5573,7 +5660,7 @@ mod tests { write_instr(&mut mem, 0, raw); ctx.pc = 0; step(&mut ctx, &mut mem); - assert_eq!(ctx.gpr[5], 0x0000_0000_FFFF_FFFFu64); + assert_eq!(ctx.gpr[5], 0xFFFF_FFFF_FFFF_FFFFu64); } #[test] @@ -6549,12 +6636,13 @@ mod tests { 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. + // CTR is a 64-bit SPR. mtspr CTR stores the full GPR (canary + // InstrEmit_mtspr: `f.StoreCTR(rt)`, no truncation). The bdnz/bclr zero-TEST + // still truncates to 32 bits (separate, canary-faithful — see the bcx tests + // above); the earlier PPCBUG-054 store-side truncation was a band-aid that a + // later `mfspr rX, CTR` would read back wrong. #[test] - fn mtspr_ctr_truncates_to_32_bits() { + fn mtspr_ctr_keeps_full_64_bits() { let mut ctx = PpcContext::new(); let mut mem = TestMem::new(); ctx.gpr[3] = 0xFFFF_FFFF_8000_0001; @@ -6564,7 +6652,26 @@ mod tests { write_instr(&mut mem, 0, raw); ctx.pc = 0; step(&mut ctx, &mut mem); - assert_eq!(ctx.ctr, 0x8000_0001); + assert_eq!(ctx.ctr, 0xFFFF_FFFF_8000_0001); + } + + // mfspr rX, CTR must read back the full 64-bit CTR (round-trips the value + // mtspr stored). This is the observable consequence of the mtspr fix. + #[test] + fn mfspr_ctr_reads_full_64_bits() { + let mut ctx = PpcContext::new(); + let mut mem = TestMem::new(); + ctx.gpr[3] = 0xFFFF_FFFF_8000_0001; + // mtspr CTR, r3 then mfspr r5, CTR + let spr_swapped = ((9u32 & 0x1F) << 5) | ((9u32 >> 5) & 0x1F); + let mt = (31u32 << 26) | (3 << 21) | (spr_swapped << 11) | (467 << 1); + let mf = (31u32 << 26) | (5 << 21) | (spr_swapped << 11) | (339 << 1); + write_instr(&mut mem, 0, mt); + write_instr(&mut mem, 4, mf); + ctx.pc = 0; + step(&mut ctx, &mut mem); + step(&mut ctx, &mut mem); + assert_eq!(ctx.gpr[5], 0xFFFF_FFFF_8000_0001); } // ───────────────────────────────────────────────────────────────────────