{bp-19439} arch/riscv: Fix RISC-V backtrace - #19621
Merged
Merged
Conversation
xcp.ustkptr is only assigned once in up_initial_state() when a task is created and, since the syscall fast path was optimized in e6973c7, is never updated afterwards. up_backtrace() used "ustkptr != NULL" to decide whether a task is currently blocked inside a syscall, and *(ustkptr + 1) as the frame pointer to resume tracing from. Since ustkptr is now a dead value fixed at task creation time, the check is always true and the "frame pointer" it derives points at stale data near the initial stack top, unrelated to where the task is actually blocked. Use rtcb->flags & TCB_FLAG_SYSCALL together with xcp.sregs, which dispatch_syscall() maintains precisely across the entire syscall execution window (including any nested context switches caused by blocking), to locate the frame pointer/return address saved at syscall entry instead. Signed-off-by: liang.huang <liang.huang@houmo.ai>
up_backtrace() on a different tcb dereferences that task's own stack to walk its frame pointer chain, but never selected that task's address environment first. Under CONFIG_ARCH_ADDRENV each task's stack lives behind its own page tables mapped at the same fixed virtual range, so reading tcb->stack_base_ptr without first switching to that task's addrenv reads whatever physical page the caller's own mapping of that virtual range happens to point to, not the target task's real stack. A cross-tid dumpstack of a task running in a different address environment therefore returns garbage or an all-zero backtrace instead of failing cleanly or resolving the real call chain. Add an addrenv parameter to backtrace() and select the target tcb's addrenv_own only around the two dereferences that read the target's saved ra/fp (ra = *(fp - 1), next_fp = *(fp - 2)), then restore the caller's own addrenv before writing the result into buffer. buffer belongs to the caller, not the target tcb, so it must always be written back in the caller's own address environment; writing it while the target's addrenv is still selected would corrupt the access instead of fixing it. Signed-off-by: liang.huang <liang.huang@houmo.ai>
sys_callN() wraps a bare ecall and calls no other function, so the compiler treats it as a leaf function: with frame pointers enabled, it only needs to spill the caller's s0, which it places in what up_backtrace()'s fp-chain walk assumes is ra's stack slot, while the real ra slot is never written. sched_backtrace() then misreads that slot as the return address for this frame, either resolving to a bogus symbol or, if the adjacent garbage happens to look out-of-range, terminating the backtrace early. Add "ra" to the ecall clobber list so the compiler spills/reloads ra around the ecall like a normal call site, keeping ra and the saved s0 in their expected slots. Gate this on CONFIG_FRAME_POINTER && CONFIG_SCHED_BACKTRACE, the only combination where up_backtrace()'s fp-chain walk is both valid (FRAME_POINTER) and actually exercised (SCHED_BACKTRACE); other configurations keep the original "memory"-only clobber and pay no extra cost. This only fixes the syscall boundary. Leaf functions that do not cross a syscall (e.g. up_idle()) can still lose their ra slot the same way and are not addressed here. Signed-off-by: liang.huang <liang.huang@houmo.ai>
jerpelea
requested review from
acassis,
cederom,
linguini1,
lupyuen and
xiaoxiang781216
August 3, 2026 08:22
jerpelea
requested review from
gustavonihei,
pussuw and
tmedicci
as code owners
August 3, 2026 08:22
github-actions
Bot
added
Arch: risc-v
xiaoxiang781216
approved these changes
Aug 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The RISC-V implementation of up_backtrace() (arch/risc-v/src/common/riscv_backtrace.c) has several correctness issues under CONFIG_BUILD_KERNEL. This PR fixes three independent, compounding problems found while debugging dumpstack on rv-virt:knsh_romfs.
Problem 1: xcp.ustkptr is stale and no longer maintained
ustkptr was introduced in 77e90d9 ("RISC-V: Include support for kernel stack"). The intent was: on syscall entry, update ustkptr to the user-mode SP; on syscall exit, reset it back to NULL.
76e5204 ("risc-v/backtrace: correct stack pointer if enable ARCH_KERNEL_STACK backtrace") then started consuming ustkptr in up_backtrace(), using *(ustkptr + 1) as the starting frame pointer for a task that is currently inside a syscall (running on the kernel stack instead of the user stack).
However, e6973c7 ("riscv/syscall: Optimize user service call performance") appears to have dropped the maintenance of ustkptr on the syscall entry/exit path. As a result, ustkptr is only ever set once, at task creation time, and never updated afterwards — the != NULL check in up_backtrace() is effectively dead, and the value it reads is always the stale "initial" one.
This alone causes dumpstack to return zero frames for any user-mode task, because ustkptr + 1 no longer points anywhere near a valid frame — it's out of range of the user stack entirely.
Separately, I have to admit I don't fully understand what *(ustkptr + 1) was supposed to compute in the first place — it doesn't correspond to any ABI-defined frame pointer slot I can find. If I'm missing something about how 76e5204 was verified at the time, I'd appreciate clarification; otherwise this looks like it was never exercised against a real out-of-range case.
Fix: instead of relying on ustkptr, use the existing TCB_FLAG_SYSCALL flag together with xcp.sregs[REG_FP] / xcp.sregs[REG_EPC] (the syscall entry's saved register context, already maintained elsewhere) to detect "this task is currently inside a syscall" and get its correct frame pointer/PC.
Problem 2: cross-tcb backtrace reads across the wrong address environment
Independently of problem 1, when dumpstack targets a different task's TCB (CONFIG_ARCH_ADDRENV, e.g. knsh_romfs), backtrace()'s fp/ra chain lives in the target task's address space, while buffer (where results are written) belongs to the caller. The RISC-V backtrace() never switched address environments at all, so the fp-chain walk dereferenced the target task's stack addresses using the caller's page tables — reading whatever physical page happens to be mapped at that virtual address in the caller's own environment, not the target's actual stack contents.
Fix: backtrace() now takes an optional addrenv parameter and wraps only the two frame-chain reads (ra = *(fp - 1), next_fp = *(fp - 2)) in addrenv_select()/addrenv_restore(). The write into buffer[i++] = ra stays outside that window, since buffer is the caller's own memory and must be written using the caller's own address environment — switching addrenv around that write as well would corrupt/lose the result (confirmed by hardware testing; an earlier, wider-scoped attempt that included the buffer write inside the switch produced all-zero backtraces).
Problem 3: leaf syscall wrapper functions don't save ra on the stack
Even with problems 1 and 2 fixed, backtraces crossing a syscall boundary can still be broken. sys_call0()..sys_call6() in arch/risc-v/include/syscall.h are leaf functions as far as the compiler can tell (they never call another C function themselves), so even with CONFIG_FRAME_POINTER (-fno-omit-frame-pointer), the compiler only bothers to save s0 (the frame pointer) around the ecall — it never allocates or writes a stack slot for ra, because as far as the compiler's leaf-function analysis is concerned, ra is never clobbered inside the function body and ret (which is jalr x0, 0(x1)) works correctly straight out of the register.
The problem is that backtrace()'s fp-chain walk assumes the standard prologue layout (ra at fp - 1, saved s0 at fp - 2). In these leaf wrappers, s0 ends up at fp - 1 instead (since there's no ra slot to begin with), and fp - 2 is simply never written — uninitialized stack garbage. Disassembly of waitpid() before the fix:
c000ba08 :
c000ba08: 1141 addi sp,sp,-16
c000ba0a: c622 sw s0,12(sp)
c000ba0c: 872a mv a4,a0
c000ba0e: 0800 addi s0,sp,16
c000ba10: 87ae mv a5,a1
c000ba12: 86b2 mv a3,a2
c000ba14: 02300513 li a0,35
c000ba18: 85ba mv a1,a4
c000ba1a: 863e mv a2,a5
c000ba1c: 00000073 ecall
c000ba20: 0001 nop
c000ba22: 4432 lw s0,12(sp)
c000ba24: 0141 addi sp,sp,16
c000ba26: 8082 ret
Note s0 is saved at sp+12 (i.e. fp - 1), and ra is never spilled at all. backtrace() reading *(fp - 1) here gets the caller's old s0, not a return address; *(fp - 2) gets whatever garbage was left on the stack from a previous call.
Fix: force ra to be spilled/reloaded around the ecall by adding it to the clobber list, gated behind a new RISCV_ECALL_CLOBBERS macro:
#if defined(CONFIG_FRAME_POINTER) && defined(CONFIG_SCHED_BACKTRACE)
define RISCV_ECALL_CLOBBERS "memory", "ra"
#else
define RISCV_ECALL_CLOBBERS "memory"
#endif
This only takes effect when both CONFIG_FRAME_POINTER (the fp-chain is structurally meaningful at all) and CONFIG_SCHED_BACKTRACE (the only consumer of up_backtrace()'s fp-chain walk) are enabled — otherwise there is no fp-chain consumer to fix up for, and the extra spill/reload would be pure overhead. Disassembly of waitpid() after the fix:
c000ba9c :
c000ba9c: 1141 addi sp,sp,-16
c000ba9e: c422 sw s0,8(sp)
c000baa0: c606 sw ra,12(sp)
c000baa2: 0800 addi s0,sp,16
c000baa4: 872a mv a4,a0
c000baa6: 87ae mv a5,a1
c000baa8: 86b2 mv a3,a2
c000baaa: 02300513 li a0,35
c000baae: 85ba mv a1,a4
c000bab0: 863e mv a2,a5
c000bab2: 00000073 ecall
c000bab6: 0001 nop
c000bab8: 40b2 lw ra,12(sp)
c000baba: 4422 lw s0,8(sp)
c000babc: 0141 addi sp,sp,16
c000babe: 8082 ret
ra and s0 are now both correctly saved at fp - 1/fp - 2, matching what backtrace() expects. This adds two instructions and one stack word of overhead to each sys_callN() wrapper, but only in configurations where both gating options above are enabled.
Known follow-up (not part of this PR)
While debugging this, I found that AArch64's up_backtrace() under CONFIG_BUILD_KERNEL appears to have a similar class of problem. I'd like to hold off on that until this PR is reviewed/confirmed, and will submit it as a separate follow-up PR.
Impact
RELEASE
Testing
CI