Skip to content

fix(riscv): reject reserved JALR funct3 in the lifter - #220

Closed
carlosqwqqwq wants to merge 2 commits into
HexRaysSA:masterfrom
carlosqwqqwq:fix-riscv-lifter-jalr-funct3
Closed

fix(riscv): reject reserved JALR funct3 in the lifter#220
carlosqwqqwq wants to merge 2 commits into
HexRaysSA:masterfrom
carlosqwqqwq:fix-riscv-lifter-jalr-funct3

Conversation

@carlosqwqqwq

Copy link
Copy Markdown
Contributor

fix(riscv): reject reserved JALR funct3 in the lifter

Closes #219

Summary

JALR's funct3 field is reserved and must be 0; the SMIR lifter lifted any funct3 value as a JALR. It now returns InvalidEncoding for the reserved values.

Validation

  • Regression covers funct3=001 (must be rejected) and funct3=000 (must lift).
  • cargo test --lib: 1 new test passes.

Notes

Independent encoding-validation root cause, distinct from the no-C alignment handling.

cpu.rs exceeded the AGENTS.md hard split triggers (2000 lines / 150 kB). Move the vector (RVV) element access and data-path execution group, including set_vtype, into cpu/vector.rs with no behavior change.
JALR's funct3 field is reserved and must be 0; the SMIR lifter now returns InvalidEncoding for other values instead of lifting the word as a JALR.

19h commented Aug 11, 2026

Copy link
Copy Markdown
Member

I reviewed the focused commit 2a1dddb6 against issue #219. Checking funct3 == 0 at the JALR lifter boundary and returning InvalidEncoding for the reserved values matches the direct decoder and the base ISA encoding.

Two changes are needed before this is merge-ready:

  1. This PR still contains 55043ddc, the closed refactor(riscv): split vector semantics out of cpu.rs #202 vector-module split. Please rebase onto current master and drop that commit so the PR contains only the focused JALR change.
  2. The new regression has duplicate adjacent #[test] attributes. The focused filter runs the same test twice (2/2 with the identical name); please remove one marker.

The legal/reserved controls otherwise pass. This fix is non-RVV and should remain separate from #203.

19h commented Aug 11, 2026

Copy link
Copy Markdown
Member

Thank you @carlosqwqqwq for identifying the reserved JALR funct3 gap and proposing a fix. We independently reimplemented and validated all eight field values in #231, where this is consolidated with #218, #222, and #224. No commits from this branch were taken. To keep review and CI on one maintained branch, I’m closing this PR as superseded by #231. You are credited as co-author on the consolidation commit—please review #231.

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.

SMIR lifts JALR with reserved funct3

2 participants