Skip to content

Fix MIPS lifter bugs: overflow detection and MSUB/MSUBU subtraction order#126

Merged
endeav0r merged 4 commits into
masterfrom
worktree-mips-tests
Mar 24, 2026
Merged

Fix MIPS lifter bugs: overflow detection and MSUB/MSUBU subtraction order#126
endeav0r merged 4 commits into
masterfrom
worktree-mips-tests

Conversation

@endeav0r

@endeav0r endeav0r commented Mar 24, 2026

Copy link
Copy Markdown
Member

Summary

  • Fix ADD/ADDI/SUB overflow detection to match MIPS32 manual (sign-extend to 64 bits, check bit32 vs bit31)
  • Fix MSUB/MSUBU reversed subtraction: was computing product - accumulator, manual specifies accumulator - product
  • Add comprehensive MIPS instruction test suite: fix incorrect test assertions, add edge cases, add 8 new test functions (TEQ, LL, SC, RDHWR, PREF, SYNC, ADDIU negative immediate, SLTIU negative immediate)

Test plan

  • All 82 MIPS translator tests pass (cargo test --lib translator::mips::test)
  • Verify no regressions in other test suites

🤖 Generated with Claude Code

endeavor and others added 4 commits March 24, 2026 15:23
Fix tests asserting wrong behavior per MIPS32 manual:
- ADD: -1 + 1 = 0 is not overflow, add MIN_INT + (-1) overflow case
- SUB: 0 - 1 = -1 is not overflow, add MAX_INT - (-1) overflow case
- MSUB/MSUBU: fix reversed subtraction (manual: accumulator - product)

Add edge cases to existing tests:
- CLZ(0)=32, CLO(0xFFFFFFFF)=32, DIV neg/neg, SRA shift-31
- BGEZAL/BLTZAL: assert $ra written unconditionally

Add new test functions:
- TEQ, LL, SC, RDHWR, PREF, SYNC, ADDIU negative imm, SLTIU negative imm

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ction order

- ADD/ADDI: Overflow check compared result sign to lhs sign only, which
  falsely triggered on e.g. -1 + 1 = 0. Now uses manual-specified 33-bit
  check: sign-extend to 64 bits, add, overflow if bit32 != bit31.
- SUB: Overflow used incorrect cmpltu formula. Now uses same 33-bit
  approach as ADD but with subtraction.
- MSUB/MSUBU: Computed product - accumulator instead of manual-specified
  accumulator - product. Swapped operand order.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- actions/checkout@v2 → v4
- actions-rs/toolchain@v1 → dtolnay/rust-toolchain@stable
- actions-rs/cargo@v1 → direct cargo commands
- Consolidate rustfmt + clippy components in single toolchain step

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@endeav0r
endeav0r merged commit 00a76f2 into master Mar 24, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant