From e2873342163ca9f873f4179cd3b9d4fc2a0b51bd Mon Sep 17 00:00:00 2001 From: Robert Allan James Date: Sun, 2 Aug 2026 07:30:27 -0400 Subject: [PATCH] Fix riscv64 asm register-reuse hazard in vm_pop_asm/vm_rpop_asm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- include/vm_asm_opt_riscv64.h | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/include/vm_asm_opt_riscv64.h b/include/vm_asm_opt_riscv64.h index 4190298..2ab14be 100644 --- a/include/vm_asm_opt_riscv64.h +++ b/include/vm_asm_opt_riscv64.h @@ -122,10 +122,23 @@ static inline cell_t vm_pop_asm(VM *vm) { "lw t0, %[dsp]\n\t" "bltz t0, 1f\n\t" /* branch if dsp < 0 (underflow) */ "slli t1, t0, 3\n\t" - "add t1, %[stack], t1\n\t" - "ld %[val], 0(t1)\n\t" + "add t1, %[stack], t1\n\t" /* t1 = &data_stack[dsp] (old dsp) */ "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" "j 2f\n\t" "1:\n\t" @@ -185,10 +198,11 @@ static inline cell_t vm_rpop_asm(VM *vm) { "lw t0, %[rsp]\n\t" "bltz t0, 1f\n\t" "slli t1, t0, 3\n\t" - "add t1, %[stack], t1\n\t" - "ld %[val], 0(t1)\n\t" + "add t1, %[stack], t1\n\t" /* t1 = &return_stack[rsp] (old rsp) */ "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" "j 2f\n\t" "1:\n\t"