runtime: fix multicore RISC-V GC synchronization

[jakebailey]

Fixes #5679

See commit descriptions; the gist is that scheduler waking and GC were sharing the same signals, causing confusion, so split them up. Other platforms do not seem to have this problem.

[Copilot]

🟡 Changes recommended

The renamed assembly symbol makes RISC-V stack-size reports omit the context-switch frame.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR separates RISC-V scheduler and GC signals to prevent multicore deadlocks.

Changes:

• Adds per-hart GC and scheduler signals.
• Makes RISC-V stack switching interrupt-safe.
• Adds a multicore GC regression test.

File summaries

File	Description
testdata/gcmulticore.txt	Defines expected test output.
testdata/gcmulticore.go	Exercises repeated multicore GC cycles.
src/runtime/runtime_tinygoriscv_qemu.go	Separates interrupt signals and improves diagnostics.
src/runtime/runtime_rp2.go	Adds the shared stack-state helper.
src/runtime/gc_stack_cores.go	Uses target-specific stack-state detection.
src/internal/task/task_stack_tinygoriscv.S	Protects RISC-V stack switches from interrupts.
src/internal/task/task_stack_tinygoriscv.go	Updates the RISC-V stack-switch interface.
main_test.go	Registers and configures the regression test.

Review details

• Files reviewed: 8/8 changed files
• Comments generated: 1
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

The low-level interrupt, assembly, and cross-core synchronization changes need final human validation.

Review details

• Files reviewed: 6/6 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[deadprogram]

Thank you @jakebailey for this fix.

The following items are an edited version of an automated review.

1. The test skip is not removed. main_test.go on dev skips finalizerinvariants.go for riscv-qemu. Issue runtime: finalizers deadlock or fault on multicore RISC-V (scheduler=cores) #5679 says to remove the skip when the fault is corrected. Should we revert that commit in this PR so it can detect the actual thing that was failing?

2. t0 crosses the stack switch in tinygo_swapTaskRISC. The csrrci t0, mstatus, 8 runs on the old stack, but the restore runs on the new stack, and t0 is not in the 52-byte saved frame. Each task thus gets the MIE state of the task that started the switch, not its own. This is correct now, because resume() and pause() always run with interrupts on, but the condition is not stated. Put the bit in the callee-saved frame, or add a comment.

3. gcOnSystemStack has two different meanings. runtime_tinygoriscv_qemu.go returns task.SystemStack() == 0, and runtime_rp2.go returns task.OnSystemStack(). A reader cannot see that the difference is intentional. An override of OnSystemStack() in task_stack_tinygoriscv.go keeps the condition in package task, next to the stack switch, and does not add a hook that each new scheduler=cores target must supply. Also, is RP2 safe, or only not tested? Current() == nil has the same non-atomic behavior there.

4. The wake consume can discard a wakeup. In handleInterrupt, schedulerWakePending[hartID].Swap(0) != 0 && checkpoint.Saved() always does the Swap. If the flag is set and no checkpoint is saved, the wakeup is lost. This is probably not possible, because schedulerWake signals only harts in sleepingHarts and runs with the scheduler lock held. Please confirm and add a comment, because the safety depends on a lock that a different function holds.

5. Duplication in gcInterruptHandler. The exit check if hartID == 0 && exitCodePlusOne.Load() != 0 occurs four times, and the two wait loops are the same 12 lines. A gcWaitForSignal(hartID) helper and an exit-check helper remove approximately 20 lines.

The fence in signalHart with the specification reference is correct and necessary, because the MSIP write is MMIO and RVWMO does not order it against normal memory. The mtval addition is useful.

[deadprogram]

Thank you @jakebailey for the update. All 5 items from the last review are corrected.

The following items are an edited version of an automated review.

1. The receive side has no fence. signalHart does a fence before the MSIP write. The opposite order is necessary on the receive side, but it is not there. In handleInterrupt, the MSIP[hartID].Set(0) is an MMIO store and the flag reads are normal loads. RVWMO does not order these, so a hart can read gcPauseRequest before the MSIP clear is visible. If the sender sets the flag and the MSIP in that window, the clear removes the notification and the request stays unprocessed. gcWaitForSignal has the same pattern. Its comment says "Check state after the clear so a concurrent signal is not lost", but the hardware does not give that order. Please add a fence after each MSIP.Set(0) that a flag read follows. See RISC-V Unprivileged ISA, section 2.7.

2. gcWaitForSignal tests the exit code two times. Only aclintMSWI.MSIP[hartID].Set(0) is between the two if hartID == 0 && exitCodePlusOne.Load() != 0 blocks. That write cannot change exitCodePlusOne. Please remove the first block. The second exitCodePlusOne test in handleInterrupt, after gcInterruptHandler returns, is also covered by the test in gcWaitForSignal.

3. The new comment in task_stack_tinygoriscv.S disagrees with the code. The comment says "Callers enter with interrupts enabled". If that is always true, then t0 & 8 is always 8, the beqz t0, 2f branch never occurs, and csrsi mstatus, 8 alone is sufficient. Please remove the sentence and keep the conditional restore, or keep the sentence and make the restore unconditional.

4. //go:export is different from the neighbour. swapTaskRISC uses //go:export. swapTask in task_stack.go uses //export. Both are correct. Please use //export for consistency.

I assembled task_stack_tinygoriscv.S for rv32imac. tinygo_swapTask and tinygo_swapTaskRISC are both FUNC symbols with size 0 at the same address. stacksize.CallGraph thus finds one node for tinygo_swapTask, and the lookup in builder/build.go continues to operate.

[Copilot]

🔵 Needs a closer look

The change modifies multicore interrupt synchronization and assembly stack switching.

Review details

• Files reviewed: 7/7 changed files
• Comments generated: 0 new
• Review effort level: Balanced