From e9536ab46d0f522b1ad93196a68a7cd464ec5fa5 Mon Sep 17 00:00:00 2001 From: wstran Date: Sun, 3 May 2026 23:33:04 +0700 Subject: [PATCH] fix(tracer): use wrapping_add in alignment assertions `VirtualAssertWordAlignment` and `VirtualAssertHalfwordAlignment` compute the effective address via `cpu.x[rs1] + imm`. Both operands are `i64`, so the bare `+` panics in debug builds whenever the signed sum falls outside `[i64::MIN, i64::MAX]`, e.g. `rs1 = i64::MAX, imm = 1`. The panic surfaces as `attempt to add with overflow` from inside the tracer's `execute`, aborting the trace before the actual alignment check runs. RV64 specifies effective addresses as `rs1 + sign_ext(imm)` mod 2^64, and every other memory-touching tracer instruction (LB/LBU/LH/LHU/ LW/LWU/LD, SB/SH/SW/SD, VirtualLW/VirtualSW, AMO*) computes the address via `wrapping_add`. These two assertions are the only outliers. Switch them to `wrapping_add` so: - debug builds match release behavior (release silently wraps `+`), - the alignment check operates on the correctly-wrapped low bits, - the convention is consistent across the tracer. Adds a regression test in each file pinning the wrapping behavior (rs1 = i64::MAX, imm = 1 produces an aligned wrapped address and must execute without panicking). No release-mode behavior change: signed overflow already wraps in release, and the alignment check operates on the same low bits in both forms. --- .../virtual_assert_halfword_alignment.rs | 28 +++++++++++++++- .../virtual_assert_word_alignment.rs | 33 ++++++++++++++++++- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/tracer/src/instruction/virtual_assert_halfword_alignment.rs b/tracer/src/instruction/virtual_assert_halfword_alignment.rs index 6c5bbbf3cf..a785968089 100644 --- a/tracer/src/instruction/virtual_assert_halfword_alignment.rs +++ b/tracer/src/instruction/virtual_assert_halfword_alignment.rs @@ -18,7 +18,7 @@ impl VirtualAssertHalfwordAlignment { cpu: &mut Cpu, _: &mut ::RAMAccess, ) { - let address = cpu.x[self.operands.rs1 as usize] + self.operands.imm; + let address = cpu.x[self.operands.rs1 as usize].wrapping_add(self.operands.imm); assert!( address & 1 == 0, "RAM access (LH or LHU) is not halfword aligned: {address:x}" @@ -27,3 +27,29 @@ impl VirtualAssertHalfwordAlignment { } impl RISCVTrace for VirtualAssertHalfwordAlignment {} + +#[cfg(test)] +mod tests { + use super::*; + use crate::emulator::cpu::Cpu; + use crate::emulator::terminal::DummyTerminal; + use crate::instruction::format::format_assert_align::FormatAssert; + + /// See [`VirtualAssertWordAlignment`] tests for the rationale. + #[test] + fn wraps_on_overflow() { + let mut cpu = Cpu::new(Box::new(DummyTerminal::default())); + cpu.x[1] = i64::MAX; + let instr = VirtualAssertHalfwordAlignment { + address: 0, + operands: FormatAssert { rs1: 1, imm: 1 }, + virtual_sequence_remaining: None, + is_first_in_sequence: false, + is_compressed: false, + }; + let mut ram_access = (); + // i64::MAX.wrapping_add(1) == i64::MIN, whose low bit is 0, so this + // must execute without panicking. + instr.execute(&mut cpu, &mut ram_access); + } +} diff --git a/tracer/src/instruction/virtual_assert_word_alignment.rs b/tracer/src/instruction/virtual_assert_word_alignment.rs index e8b4307f7f..5b71d9b3ba 100644 --- a/tracer/src/instruction/virtual_assert_word_alignment.rs +++ b/tracer/src/instruction/virtual_assert_word_alignment.rs @@ -18,7 +18,7 @@ impl VirtualAssertWordAlignment { cpu: &mut Cpu, _: &mut ::RAMAccess, ) { - let address = cpu.x[self.operands.rs1 as usize] + self.operands.imm; + let address = cpu.x[self.operands.rs1 as usize].wrapping_add(self.operands.imm); assert!( address & 3 == 0, "RAM access (LW or LWU) is not word aligned: {address:x}" @@ -27,3 +27,34 @@ impl VirtualAssertWordAlignment { } impl RISCVTrace for VirtualAssertWordAlignment {} + +#[cfg(test)] +mod tests { + use super::*; + use crate::emulator::cpu::Cpu; + use crate::emulator::terminal::DummyTerminal; + use crate::instruction::format::format_assert_align::FormatAssert; + + /// `rs1 + imm` must use wrapping arithmetic to match RV64 semantics + /// (effective addresses are mod 2^64) and the convention used by every + /// other tracer instruction (LB/LBU/LH/LHU/LW/LWU/LD, SB/SH/SW/SD, + /// VirtualLW/VirtualSW, AMO*). Plain `+` panics on i64 overflow in + /// debug builds for any rs1/imm pair whose signed sum falls outside + /// `[i64::MIN, i64::MAX]`. + #[test] + fn wraps_on_overflow() { + let mut cpu = Cpu::new(Box::new(DummyTerminal::default())); + cpu.x[1] = i64::MAX; + let instr = VirtualAssertWordAlignment { + address: 0, + operands: FormatAssert { rs1: 1, imm: 1 }, + virtual_sequence_remaining: None, + is_first_in_sequence: false, + is_compressed: false, + }; + let mut ram_access = (); + // i64::MAX.wrapping_add(1) == i64::MIN, whose low 2 bits are 0, + // so this must execute without panicking. + instr.execute(&mut cpu, &mut ram_access); + } +}