Fix riscv64 asm register-reuse hazard in vm_pop_asm/vm_rpop_asm

Completes the highest-priority item from
docs/working/archive/session-logs/2026-07-24-punch-list.md (item #1),
intentionally deferred out of commit 4485c38 as a separate, more careful
change.

vm_pop_asm/vm_rpop_asm's inline asm referenced the dsp/rsp memory operand
twice (read near the top, write-back near the bottom) while also writing
a plain register output operand (%[val]) in between. Nothing pinned the
address register computed for the memory operand across that gap, so
clang's allocator could reuse it for %[val], corrupting the write-back.
GCC happened to pick different registers and never hit it — this repo's
kernel build uses GCC and USE_ASM_OPT is never defined there, so the bug
was latent, not live, prior to this fix.

Fixed by reordering: write dsp/rsp back before loading the popped value,
so the memory operand's final use has already happened by the time the
output register is live. Same fix as the old pre-split monorepo's master
commit 4db9946a, re-derived here since that commit lives in a different
repository post-split.

Verified empirically, not just theoretically: rebuilding the riscv64
hosted binary with the documented clang -O3 -DUSE_ASM_OPT=1 acceptance
recipe went from 955 passed / 10 failed (all CASE.* control-flow tests —
exactly what stack-pop corruption would hit) to 965 passed / 0 failed,
"ALL IMPLEMENTED TESTS PASSED!", with nothing else changed. All three
Makefile.starkernel kernel builds still compile clean; the change is
inert there since USE_ASM_OPT is never defined for the kernel build.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Robert Allan James
2026-08-02 07:30:27 -04:00
co-authored by Claude Sonnet 5
parent 4485c3893b
commit e287334216
+20 -6
View File
@@ -122,10 +122,23 @@ static inline cell_t vm_pop_asm(VM *vm) {
"lw t0, %[dsp]\n\t" "lw t0, %[dsp]\n\t"
"bltz t0, 1f\n\t" /* branch if dsp < 0 (underflow) */ "bltz t0, 1f\n\t" /* branch if dsp < 0 (underflow) */
"slli t1, t0, 3\n\t" "slli t1, t0, 3\n\t"
"add t1, %[stack], t1\n\t" "add t1, %[stack], t1\n\t" /* t1 = &data_stack[dsp] (old dsp) */
"ld %[val], 0(t1)\n\t"
"addi t0, t0, -1\n\t" "addi t0, t0, -1\n\t"
"sw t0, %[dsp]\n\t" "sw t0, %[dsp]\n\t" /* write dsp back BEFORE loading the
value: %[dsp] is a memory operand
referenced twice in this template
(read above, write here); loading
%[val] (a register output) before
this write-back let clang's
allocator reuse %[dsp]'s address
register for %[val], corrupting
the write-back — GCC happened to
pick different registers and
never hit it. Loading %[val] last
means %[dsp]'s final use has
already happened by the time the
output register is live. */
"ld %[val], 0(t1)\n\t"
"sw zero, %[err]\n\t" "sw zero, %[err]\n\t"
"j 2f\n\t" "j 2f\n\t"
"1:\n\t" "1:\n\t"
@@ -185,10 +198,11 @@ static inline cell_t vm_rpop_asm(VM *vm) {
"lw t0, %[rsp]\n\t" "lw t0, %[rsp]\n\t"
"bltz t0, 1f\n\t" "bltz t0, 1f\n\t"
"slli t1, t0, 3\n\t" "slli t1, t0, 3\n\t"
"add t1, %[stack], t1\n\t" "add t1, %[stack], t1\n\t" /* t1 = &return_stack[rsp] (old rsp) */
"ld %[val], 0(t1)\n\t"
"addi t0, t0, -1\n\t" "addi t0, t0, -1\n\t"
"sw t0, %[rsp]\n\t" "sw t0, %[rsp]\n\t" /* write rsp back before loading the
value — see vm_pop_asm for why */
"ld %[val], 0(t1)\n\t"
"sw zero, %[err]\n\t" "sw zero, %[err]\n\t"
"j 2f\n\t" "j 2f\n\t"
"1:\n\t" "1:\n\t"