Skip to content

fix(riscv): check the store address on failed SC.W/SC.D - #241

Closed
carlosqwqqwq wants to merge 2 commits into
HexRaysSA:masterfrom
carlosqwqqwq:fix-riscv-sc-store-check
Closed

fix(riscv): check the store address on failed SC.W/SC.D#241
carlosqwqqwq wants to merge 2 commits into
HexRaysSA:masterfrom
carlosqwqqwq:fix-riscv-sc-store-check

Conversation

@carlosqwqqwq

Copy link
Copy Markdown
Contributor

fix(riscv): check the store address on failed SC.W/SC.D

Closes #240

Summary

RAX retires a failed store-conditional instruction without checking whether its address is valid for a store: an aligned SC.W to an unmapped address returns failure status and advances the PC instead of raising a store access fault. The same omission exists in the interpreter and in the production JIT helper. The RISC-V A-extension contract requires a failed SC to pass memory-permission checks; it may fail without writing, but it cannot retire against an invalid store address.

The fix probes the target range without modifying memory after clearing the reservation and before writing the failure status; a valid failed SC still returns nonzero and does not modify memory. The failed SC.W/SC.D branches read-probe the 4-byte or 8-byte range and raise the store access fault when unmapped, and the JIT helper performs the same probe on a failed reservation.

Validation

  • Fixed: the targeted regression test for the store-conditional path passes, and the full library test suite passes with no regressions.
  • Reference: the control ELF (aligned SC.W at address zero with no preceding LR) exits with an access-fault status on QEMU and on the native RISC-V hardware reference; the A-extension specification is the normative anchor.
  • cargo test --lib: full library test suite passes with no regressions.

No new upstream test files are added; this is a source-only change.

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.

19h commented Aug 11, 2026

Copy link
Copy Markdown
Member

Thank you for reporting and proposing this fix. I independently reimplemented and validated failed SC.W/SC.D store-range validation, exact trap/retirement behavior, and a non-mutating permission probe for MMIO-safe direct and native execution in the consolidation PR #231; no commits or code from this branch were taken. The rollup carries @carlosqwqqwq as co-author on its commits and includes direct and native regression coverage. I’m closing this PR as superseded by #231—please review the consolidated implementation there.

@19h 19h closed this Aug 11, 2026
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.

Failed store-conditional retires without checking the store address

2 participants