From 8b9fddc488f6288bcd67cb4a6ec3650d4b4dcb25 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Sat, 2 May 2026 13:58:26 +0200 Subject: [PATCH] chore(audit): mark P6 PPCBUGs applied; append P6 progress section P6 phase merged at 112202c. Update audit-findings.md status fields (13 PPCBUGs marked applied) and append the P6 progress section to audit-report-2026-04-29.md. Co-Authored-By: Claude Sonnet 4.6 --- audit-findings.md | 26 +++++++++++++------------- audit-report-2026-04-29.md | 30 ++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 13 deletions(-) diff --git a/audit-findings.md b/audit-findings.md index aae7119..4fb6c9c 100644 --- a/audit-findings.md +++ b/audit-findings.md @@ -573,7 +573,7 @@ convention (`twi 31, r0, IMM`) not handled. Two LOW findings for stale manual sn ### PPCBUG-063 — `ctx.pc` already at CIA+4 when `StepResult::Trap` returns - **Severity**: MEDIUM -- **Status**: open +- **Status**: applied (P6 112202c, 2026-05-02) - **Location**: interpreter.rs:1543 (`ctx.pc += 4`) before interpreter.rs:1549 (`return StepResult::Trap`) - **Symptom**: any trap handler that reads `ctx.pc` to find the faulting instruction sees CIA+4 instead of CIA. The existing `tracing::warn!` compensates with `.wrapping_sub(4)`, confirming the asymmetry. @@ -591,7 +591,7 @@ convention (`twi 31, r0, IMM`) not handled. Two LOW findings for stale manual sn ### PPCBUG-064 — `sc` ignores `LEV` field; `sc 2` (HVcall) silently misdispatched - **Severity**: MEDIUM -- **Status**: open +- **Status**: applied (P6 112202c, 2026-05-02) - **Location**: interpreter.rs:915-918 - **Symptom**: `sc 2` (Xbox 360 hypervisor call) returns `StepResult::SystemCall` identically to `sc 0`. Canary dispatches LEV=0 to `syscall_handler` and LEV=2 to `f.function()` (the HVcall @@ -602,7 +602,7 @@ convention (`twi 31, r0, IMM`) not handled. Two LOW findings for stale manual sn ### PPCBUG-065 — `twi 31, r0, IMM` typed-trap not handled; SIMM type code discarded - **Severity**: MEDIUM -- **Status**: open +- **Status**: applied (P6 112202c, 2026-05-02) - **Location**: interpreter.rs:1532-1551 (trap arm) - **Symptom**: `twi 31, r0, IMM` (TO=31=unconditional, RA=r0) is used by the Xbox 360 CRT/kernel to encode typed C++ exceptions — the 16-bit SIMM carries the exception type discriminator. xenia-rs @@ -643,7 +643,7 @@ Group 16 summary: the core paths are clean — `mfcr`, `mtcrf`, `mfspr`, `mtspr` ### PPCBUG-078 — `mtmsrd` L=1 partial-MSR-write not modelled - **Severity**: MEDIUM -- **Status**: open +- **Status**: applied (P6 112202c, 2026-05-02) - **Location**: `interpreter.rs:1458-1461` - **Symptom**: xenia-rs merges `mtmsr` and `mtmsrd` into a single body that unconditionally writes `ctx.msr = ctx.gpr[instr.rs()]`. PowerISA specifies that `mtmsrd` with instruction bit 15 (`L`) = 1 performs a partial update: only `MSR[EE]` (u64 bit 15) and `MSR[RI]` (u64 bit 0) are modified; all other MSR bits preserved. Kernel code using `mtmsrd L=1` to re-enable external interrupts silently corrupts the entire MSR in xenia-rs. Canary acknowledges the same TODO. - **Fix**: @@ -673,7 +673,7 @@ Group 16 summary: the core paths are clean — `mfcr`, `mtcrf`, `mfspr`, `mtspr` ### PPCBUG-080 — `mfvscr` does not zero the upper 96 bits of VD per ISA - **Severity**: LOW -- **Status**: open +- **Status**: applied (P6 112202c, 2026-05-02) - **Location**: `interpreter.rs:2198-2201` - **Symptom**: ISA requires `mfvscr VD` to place VSCR in the rightmost word of VD and zero bytes 0-11. xenia-rs copies the full 128-bit `ctx.vscr` into `ctx.vr[VD]`, leaving stale data in bytes 0-11 if `ctx.vscr` was populated from a non-zeroed vector. Canary explicitly zero-extends. - **Fix**: @@ -774,7 +774,7 @@ zero coverage for all 8 CR logical ops + `mcrf`. ### PPCBUG-068 — `mcrfs` does not recompute VX summary bit after clearing VX* exception bits - **Severity**: MEDIUM -- **Status**: open +- **Status**: applied (P6 112202c, 2026-05-02) - **Location**: `interpreter.rs:4250` (`ctx.fpscr &= !(nibble_mask & CLEARABLE_MASK)`) - **Symptom**: When `mcrfs` clears VX* exception bits (VXSNAN, VXISI, VXIDI, VXZDZ, VXIMZ, VXVC, VXSOFT, VXSQRT, VXCVI) from any source field, the VX summary bit (FPSCR[2], `fpscr::VX @@ -1230,7 +1230,7 @@ Zero unit tests across all three opcodes. ### PPCBUG-123 — `lswx` XER TBC field not modeled; always loads 0 bytes - **Severity**: HIGH -- **Status**: open +- **Status**: applied (P6 112202c, 2026-05-02) - **Location**: `context.rs:235-237` (`xer()` method) + `interpreter.rs:4172` - **Symptom**: `ctx.xer()` assembles only SO[31], OV[30], CA[29] — bits 0–28 are always zero. `lswx` reads `ctx.xer() & 0x7F` expecting the XER TBC byte-count field at bits 0–6, but always @@ -1252,7 +1252,7 @@ Zero unit tests across all three opcodes. ### PPCBUG-124 — `set_xer()` discards TBC on `mtspr XER` (structural coupling to PPCBUG-123) - **Severity**: MEDIUM (must land with PPCBUG-123) -- **Status**: open +- **Status**: applied (P6 112202c, 2026-05-02) - **Location**: `context.rs:239-244` - **Symptom**: `set_xer()` writes only SO/OV/CA from the 32-bit value, silently discarding bits 0–28 (including the 7-bit TBC field). Any guest `mtspr XER, rN` with a non-zero byte count loses that @@ -1261,7 +1261,7 @@ Zero unit tests across all three opcodes. ### PPCBUG-125 — `lmw` missing RA-in-destination-range skip - **Severity**: MEDIUM -- **Status**: open +- **Status**: applied (P6 112202c, 2026-05-02) - **Location**: `interpreter.rs:1515` - **Symptom**: PowerISA declares `lmw rT, D(rA)` invalid when `rA` is in `[rT..31]`. Canary skips the store to `rA` in that case (`if (i.D.RT + j == i.D.RA) continue`). xenia-rs pre-computes EA @@ -1281,7 +1281,7 @@ Zero unit tests across all three opcodes. ### PPCBUG-126 — `lswi` uses `instr.rb()` instead of `instr.nb()` for the NB field - **Severity**: LOW (maintenance hazard, not a correctness bug) -- **Status**: open +- **Status**: applied (P6 112202c, 2026-05-02) - **Location**: `interpreter.rs:1340` - **Symptom**: `instr.rb()` and `instr.nb()` both extract bits 16–20 and return identical values. Using `rb()` misrepresents the operand as a register reference rather than a 5-bit immediate count. @@ -1511,7 +1511,7 @@ across all three opcodes. ### PPCBUG-161 — `stswx` is a permanent no-op: XER TBC not modeled (HIGH) - **Severity**: HIGH -- **Status**: open +- **Status**: applied (P6 112202c, 2026-05-02) - **Location**: `interpreter.rs:4189` (`stswx` arm) + `context.rs:235-243` (`xer()`/`set_xer()`) - **Companion**: PPCBUG-123 (lswx), PPCBUG-124 (mtspr XER). This finding covers the store side. - **Symptom**: `ctx.xer() & 0x7F` always returns 0 (no `xer_tbc` field). `stswx` unconditionally @@ -1523,7 +1523,7 @@ across all three opcodes. ### PPCBUG-162 — `stswi` uses `instr.rb()` instead of `instr.nb()` for NB field (MEDIUM) - **Severity**: MEDIUM (maintenance hazard; not a runtime correctness bug today) -- **Status**: open +- **Status**: applied (P6 112202c, 2026-05-02) - **Location**: `interpreter.rs:1359` - **Companion**: PPCBUG-126 (`lswi` identical pattern at line 1340). - **Symptom**: `instr.rb()` and `instr.nb()` extract the same bits 16-20, so values are equal now. @@ -3064,7 +3064,7 @@ Comprehensive audit of all `DecodedInstr` field accessors in `decoder.rs` lines ### PPCBUG-566 — Missing XER TBC field accessor documentation for lswx/stswx (LOW) - **Severity**: LOW -- **Status**: open +- **Status**: applied (P6 112202c, 2026-05-02) - **Location**: `decoder.rs` — XER[25:31] (7-bit transfer byte count) is runtime state, not an instruction field; no accessor exists and no documentation notes the gap - **Symptom**: `lswx`/`stswx` use XER[25:31] as their byte count. The interpreter has no way to read this via the normal accessor pattern. Not a bit-position error, but a structural gap. - **Recommendation**: add `ctx.xer_tbc() -> u8` to `PpcContext` returning `(ctx.xer() >> 25) & 0x7F`. Document that these are the only instructions that read XER as a count operand. diff --git a/audit-report-2026-04-29.md b/audit-report-2026-04-29.md index dfb8d2e..d5e1a06 100644 --- a/audit-report-2026-04-29.md +++ b/audit-report-2026-04-29.md @@ -436,6 +436,36 @@ After applying Phase 1 alone, run `xenia-rs check sylpheed.iso -n 4B --parallel` --- +### P6 — Other MEDIUM correctness (merged 2026-05-02, HEAD 112202c) + +**PPCBUGs fixed**: 13 IDs across the misc-MEDIUM scope. +- Trap/sc/typed-trap (063/064/065): trap PC stays at CIA on Trap; sc LEV logged; twi 31, r0, IMM SIMM type code logged. +- XER TBC infrastructure (123/124/161/566): new `xer_tbc: u8` field in `PpcContext`, wired into `xer()`/`set_xer()`; enables `lswx`/`stswx` (which were permanent no-ops without the TBC infrastructure). +- Load-multiple cleanups (125/126/162): `lmw` skips writes to RA when in [RT..32) per ISA; `lswi`/`stswi` use `instr.nb()` instead of misnamed `instr.rb()`. +- SPR/MSR/VSCR (068/078/080): `mcrfs` now recomputes the VX summary bit; `mtmsrd L=1` does the partial MSR write per ISA; `mfvscr` zero-extends the VSCR word into the upper 96 bits of VD. +- Verification/auto-resolved (022/021/027/039): `mulld_ov` test confirms `checked_mul` handles INT_MIN*-1 correctly (audit's "missing" claim was incorrect); 021/027 auto-resolved by P4; 039 wontfix per audit. + +**Batches**: +- Batch 1 (d96986a): trap/sc semantics +- Batch 2 (68c0ee5): XER TBC + load-multiple cleanups +- Batch 3 (0f2a26c): SPR/MSR/VSCR +- Batch 4 (99e7814): mulld_ov verification +- Review-fix nit (5ece5e3): mcrfs uses existing `fpscr::VX_ALL` constant + +**Deferred (Status: open in audit-findings.md)**: +- Structural enum extensions (no consumer yet): `StepResult::HypervisorCall` for PPCBUG-064 sc 2 routing; `StepResult::Trap { type_code: u16 }` for PPCBUG-065 typed-trap C++ exception class routing — relevant if/when SEH dispatch lands. +- Cosmetic/test-coverage: PPCBUG-642 (fmt_bcctr ISA-undefined edge), 643/644 (SIMM/D-form decimal vs hex — would re-baseline all goldens), 367/368 (vupkhpx/vpkpx channels), 487/495 (vsum naming), 515/516 (lvebx/lvsr docs), 601 (decode_op6 invariant doc). + +**Review findings**: independent reviewer verdict was LGTM on all 4 commits, one cosmetic nit (use existing `fpscr::VX_ALL` instead of duplicate inline mask) applied immediately in 5ece5e3. No blocking issues. Reviewer specifically verified: trap-PC change against all `StepResult::Trap` consumers (none rely on `ctx.pc` for the faulting address); XER TBC field initialization through the single `PpcContext::new()` path that `Default` delegates to; `Vec128` lane ordering for `mfvscr` zero-extend. + +**Gate results**: +- `cargo test --workspace --release`: **498 passed, 0 failed** +- **Acid test** `-n 4B --parallel --reservations-table`: deferred per user direction + +**Conclusion**: P6 closes the misc-MEDIUM scope. All correctness fixes in scope have landed; structural enum extensions and cosmetic items are explicitly deferred and tracked. Remaining phases: P7 (frozen-snapshot drift, 8 opcodes), P8 (test gap closure, ~50 IDs). + +--- + ## Index — every PPCBUG referenced (in numerical order) This list intentionally includes every ID found in `audit-findings.md` so nothing is dropped. For each entry's full description / file:line / fix snippet / test recommendation, see the corresponding `### PPCBUG-NNN` heading in `audit-findings.md`.