From f1166d0f751721081764e9ef9491dbc004c6b0a6 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Sat, 2 May 2026 14:43:47 +0200 Subject: [PATCH] =?UTF-8?q?fix(cpu):=20revert=20PPCBUG-105=20=E2=80=94=20l?= =?UTF-8?q?wa/lwax/lwaux=20sign-extend=20per=20PowerISA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Post-P8 end-to-end review caught an ISA deviation introduced by P4 batch 5. The original code used `as i32 as i64 as u64` (correct PowerISA sign-extension; canary's `SignExtend(INT64_TYPE)`). My P4 batch 5 commit (20a730d) changed all three to `as u64` (zero-extend), citing the audit's "32-bit-ABI hazard" note for PPCBUG-105. This deviation is wrong per PowerISA and any 64-bit-mode kernel code that uses `lwa rT, off(rA)` will silently produce the wrong rT for negative words (e.g. memory 0x80000000 should yield 0xFFFFFFFF_80000000 but was yielding 0x00000000_80000000). Restore ISA-spec sign-extension for all three forms (lwa, lwax, lwaux). The audit's 32-bit-ABI hazard concern was speculative — there's no evidence that Xbox 360 user code emits `lwa` (compilers use `lwz`). If a real bug surfaces from a 32-bit-ABI consumer that feeds an `lwa`-loaded value into a u64 unsigned compare, that's a separate issue to debug at the consumer site. Test renamed: lwa_high_bit_set_zero_extends_upper → lwa_sign_extends_to_i64 with assertion flipped to expect 0xFFFFFFFF_80000000. Co-Authored-By: Claude Sonnet 4.6 --- crates/xenia-cpu/src/interpreter.rs | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/crates/xenia-cpu/src/interpreter.rs b/crates/xenia-cpu/src/interpreter.rs index 26f84d6..29d1ebb 100644 --- a/crates/xenia-cpu/src/interpreter.rs +++ b/crates/xenia-cpu/src/interpreter.rs @@ -1106,20 +1106,28 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - ctx.pc += 4; } PpcOpcode::lwa => { + // ISA "Load Word and Algebraic" — sign-extend to 64 bits per + // PowerISA. lwa is a 64-bit-mode opcode; canary uses + // `SignExtend(..., INT64_TYPE)`. P4-batch-5 mistakenly converted + // this to zero-extend "for 32-bit-ABI hazard mitigation"; the + // post-P8 end-to-end review caught the deviation. Restored to + // ISA-spec behavior (PPCBUG-105 reverted). let ea = if instr.ra() == 0 { 0u64 } else { ctx.gpr[instr.ra()] }; let ea = ea.wrapping_add(instr.ds() as i64 as u64) as u32; - ctx.gpr[instr.rd()] = mem.read_u32(ea) as u64; + ctx.gpr[instr.rd()] = mem.read_u32(ea) as i32 as i64 as u64; ctx.pc += 4; } PpcOpcode::lwax => { + // See PpcOpcode::lwa above: ISA sign-extend. let ea = if instr.ra() == 0 { 0u64 } else { ctx.gpr[instr.ra()] }; let ea = ea.wrapping_add(ctx.gpr[instr.rb()]) as u32; - ctx.gpr[instr.rd()] = mem.read_u32(ea) as u64; + ctx.gpr[instr.rd()] = mem.read_u32(ea) as i32 as i64 as u64; ctx.pc += 4; } PpcOpcode::lwaux => { + // See PpcOpcode::lwa above: ISA sign-extend. let ea = ctx.gpr[instr.ra()].wrapping_add(ctx.gpr[instr.rb()]) as u32; - ctx.gpr[instr.rd()] = mem.read_u32(ea) as u64; + ctx.gpr[instr.rd()] = mem.read_u32(ea) as i32 as i64 as u64; ctx.gpr[instr.ra()] = ea as u64; ctx.pc += 4; } @@ -5477,9 +5485,11 @@ mod tests { } #[test] - fn lwa_high_bit_set_zero_extends_upper() { - // PPCBUG-105: memory 0x80000000 must yield rD = 0x00000000_80000000 - // under 32-bit ABI (no sign extension to bits 32-63). + fn lwa_sign_extends_to_i64() { + // ISA "Load Word and Algebraic" — sign-extend the loaded i32 to i64. + // memory 0x80000000 (i32 = -2147483648) → rD = 0xFFFFFFFF_80000000. + // PPCBUG-105 was reverted post-P8 review: my earlier zero-extend + // change deviated from PowerISA; canary uses SignExtend(INT64_TYPE). let mut ctx = PpcContext::new(); let mem = TestMem::new(); mem.write_u32(0x100, 0x8000_0000); @@ -5489,7 +5499,8 @@ mod tests { write_instr(&mem, 0, raw); ctx.pc = 0; step(&mut ctx, &mem); - assert_eq!(ctx.gpr[5], 0x0000_0000_8000_0000u64); + assert_eq!(ctx.gpr[5], 0xFFFF_FFFF_8000_0000u64, + "lwa must sign-extend negative word to i64 per PowerISA"); } #[test]