arch/riscv: Fix RISC-V backtrace#19439
Open
hitHuang wants to merge 3 commits into
Open
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
hitHuang
requested review from
gustavonihei,
jerpelea,
pussuw,
tmedicci and
xiaoxiang781216
as code owners
github-actions
Bot
added
Arch: risc-v
|
xiaoxiang781216
approved these changes
Jul 15, 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
2 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 underCONFIG_BUILD_KERNEL. This PR fixes three independent, compounding problems found while debuggingdumpstackonrv-virt:knsh_romfs.Problem 1:
xcp.ustkptris stale and no longer maintainedustkptrwas introduced in77e90d9c875("RISC-V: Include support for kernel stack"). The intent was: on syscall entry, updateustkptrto the user-mode SP; on syscall exit, reset it back toNULL.76e5204a806("risc-v/backtrace: correct stack pointer if enable ARCH_KERNEL_STACK backtrace") then started consumingustkptrinup_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,
e6973c764cd("riscv/syscall: Optimize user service call performance") appears to have dropped the maintenance ofustkptron the syscall entry/exit path. As a result,ustkptris only ever set once, at task creation time, and never updated afterwards — the!= NULLcheck inup_backtrace()is effectively dead, and the value it reads is always the stale "initial" one.This alone causes
dumpstackto return zero frames for any user-mode task, becauseustkptr + 1no 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 how76e5204a806was 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 existingTCB_FLAG_SYSCALLflag together withxcp.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
dumpstacktargets a different task's TCB (CONFIG_ARCH_ADDRENV, e.g.knsh_romfs),backtrace()'sfp/rachain lives in the target task's address space, whilebuffer(where results are written) belongs to the caller. The RISC-Vbacktrace()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 optionaladdrenvparameter and wraps only the two frame-chain reads (ra = *(fp - 1),next_fp = *(fp - 2)) inaddrenv_select()/addrenv_restore(). The write intobuffer[i++] = rastays outside that window, sincebufferis 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
raon the stackEven with problems 1 and 2 fixed, backtraces crossing a syscall boundary can still be broken.
sys_call0()..sys_call6()inarch/risc-v/include/syscall.hare leaf functions as far as the compiler can tell (they never call another C function themselves), so even withCONFIG_FRAME_POINTER(-fno-omit-frame-pointer), the compiler only bothers to saves0(the frame pointer) around theecall— it never allocates or writes a stack slot forra, because as far as the compiler's leaf-function analysis is concerned,rais never clobbered inside the function body andret(which isjalr x0, 0(x1)) works correctly straight out of the register.The problem is that
backtrace()'s fp-chain walk assumes the standard prologue layout (raatfp - 1, saveds0atfp - 2). In these leaf wrappers,s0ends up atfp - 1instead (since there's noraslot to begin with), andfp - 2is simply never written — uninitialized stack garbage. Disassembly ofwaitpid()before the fix:Note
s0is saved atsp+12(i.e.fp - 1), andrais never spilled at all.backtrace()reading*(fp - 1)here gets the caller's olds0, not a return address;*(fp - 2)gets whatever garbage was left on the stack from a previous call.Fix: force
rato be spilled/reloaded around theecallby adding it to the clobber list, gated behind a newRISCV_ECALL_CLOBBERSmacro:This only takes effect when both
CONFIG_FRAME_POINTER(the fp-chain is structurally meaningful at all) andCONFIG_SCHED_BACKTRACE(the only consumer ofup_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 ofwaitpid()after the fix:raands0are now both correctly saved atfp - 1/fp - 2, matching whatbacktrace()expects. This adds two instructions and one stack word of overhead to eachsys_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()underCONFIG_BUILD_KERNELappears 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
CONFIG_ARCH_KERNEL_STACKand/orCONFIG_ARCH_ADDRENVare enabled (i.e.CONFIG_BUILD_KERNEL-style configurations such asknsh_romfs/knsh64_romfs). No change in behavior for FLAT/PROTECTED builds that don't use these options.syscall.hclobber change only adds instructions when bothCONFIG_FRAME_POINTERandCONFIG_SCHED_BACKTRACEare enabled; otherwise it's a no-op (RISCV_ECALL_CLOBBERSexpands to the original"memory").backtrace()(static, file-local) gained an extra parameter, butup_backtrace()'s external signature is unchanged.dumpstack/sched_backtrace()output; does not change scheduling, syscall semantics, or any other runtime behavior.Testing
Tested on
rv-virt:knsh_romfs(QEMU), withCONFIG_SCHED_BACKTRACE,CONFIG_FRAME_POINTERandCONFIG_SYSTEM_DUMPSTACKmanually enabled. Also verified okay onnshandknsh64_romfs.(Not focusing here on why the printed timestamp is an obviously bogus value — that looks like a separate, pre-existing issue unrelated to this fix.)
Before fix 1
For kernel threads,
dumpstackworks fine. For a user-mode application,dumpstackproduces no output at all:dumpstacking itself also produces no output:After fix 1
User-mode
init(TID 3) now produces some output, but it's still incomplete — this is expected, since problem 2 (addrenv) hasn't been fixed yet:After fix 2
addr2lineon the TID 3 (init) addresses — noticeably better than before, but still broken partway through because of problem 3:And on TID 4 (
dumpstackitself):After fix 3 (all three fixes applied)
This time we finally get a full, correct backtrace.
addr2lineon TID 3 (init):And on TID 4 (
dumpstackitself), also complete now:Also re-verified okay on
nshandknsh64_romfs.