Skip to content

fix(tracer): use wrapping_add in alignment assertions#1492

Merged
moodlezoup merged 1 commit into
a16z:mainfrom
wstran:fix-align-wrapping-add
May 7, 2026
Merged

fix(tracer): use wrapping_add in alignment assertions#1492
moodlezoup merged 1 commit into
a16z:mainfrom
wstran:fix-align-wrapping-add

Conversation

@wstran
Copy link
Copy Markdown
Contributor

@wstran wstran commented May 3, 2026

Summary

VirtualAssertWordAlignment::exec and VirtualAssertHalfwordAlignment::exec
compute the effective address via cpu.x[rs1] + imm. Both operands are
i64, so the bare + panics in debug builds when the signed sum falls
outside [i64::MIN, i64::MAX]. The other 30+ memory-touching tracer
instructions (LB/LBU/LH/LHU/LW/LWU/LD, SB/SH/SW/SD, VirtualLW/VirtualSW,
AMO*) all use wrapping_add; these two are the only outliers.

Reproducer

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 = ();
instr.execute(&mut cpu, &mut ram_access);
// thread '...' panicked at virtual_assert_word_alignment.rs:21:23:
// attempt to add with overflow

Why this is correct

  • RV64 spec: effective addresses are rs1 + sign_ext(imm) mod 2^64
    (per the audit Finding 1 framing in Fix audit findings from 2026-04-15 report #1442).
  • Codebase convention: every other memory-touching tracer instruction
    uses wrapping_add for the same operation. Verified via
    grep -rn 'as usize\] +' tracer/src/instruction/ — only these two
    files match before this fix.
  • No release-mode change: signed + already wraps in release, and
    the alignment check operates on the same low bits in both forms. The
    fix only stops the spurious debug-build panic on overflow.

Test plan

  • cargo fmt -q
  • cargo clippy -p jolt-core --features host --message-format=short -q --all-targets -- -D warnings
  • cargo clippy -p jolt-core --features host,zk --message-format=short -q --all-targets -- -D warnings
  • cargo nextest run -p tracer — 114/114 pass (includes 2 new regression tests, both red on the un-fixed code)
  • cargo nextest run -p jolt-core muldiv --cargo-quiet --features host — pass
  • cargo nextest run -p jolt-core muldiv --cargo-quiet --features host,zk — pass

@wstran wstran requested a review from moodlezoup as a code owner May 3, 2026 17:02
@github-actions github-actions Bot added the no-spec PR has no spec file label May 3, 2026
`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.
@wstran wstran force-pushed the fix-align-wrapping-add branch from 2a8a526 to e9536ab Compare May 6, 2026 17:24
Copy link
Copy Markdown
Collaborator

@moodlezoup moodlezoup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@moodlezoup moodlezoup merged commit 4aed07f into a16z:main May 7, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-spec PR has no spec file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants