fabi/xenia-rs
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(cpu): revert PPCBUG-105 — lwa/lwax/lwaux sign-extend per PowerISA

Browse Source 
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 <noreply@anthropic.com>
This commit is contained in:
MechaCat02
2026-05-02 14:43:47 +02:00
parent 9de18a9eec
commit f1166d0f75
1 changed files with 18 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
crates/xenia-cpu/src/interpreter.rs
View File

@@ -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]