On 10/10/26 9:27 PM, Karl Mehltretter wrote:
stack_trace_save() uses skipnr + 1 because arch_stack_walk() is expected
to first report the return into stack_trace_save(). With FRAME_POINTER,
the RISC-V arch_stack_walk() calls walk_stackframe(), which starts from
its own frame and first reports the return into arch_stack_walk().
stack_trace_save() therefore starts one frame too early. For example,
page_owner records begin at __set_page_owner() instead of the allocation
site.
Move the walker into an always-inlined helper and call it directly from
arch_stack_walk(). Keep walk_stackframe() as a wrapper for its existing
callers.
return_address() skips level + 3 entries, while the arm64 implementation
skips level + 2. After removing the extra frame, use level + 2 so
ftrace_return_address() and CALLER_ADDR results do not shift by one caller.
Fixes: 6a00ef449370 ("riscv: eliminate unreliable __builtin_frame_address(1)")
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <[email protected]>
---
Hi, Karl,
Tested with the KUnit suite from patch 2 on QEMU's virt machine, clang 22
(LLVM=1), on af32da41b032:
mainline stack_trace_save 0/3, return_address 2/2
stacktrace.c change alone stack_trace_save 3/3, return_address 0/2
this patch stack_trace_save 3/3, return_address 2/2
A BeagleV Ahead (TH1520) booted with this patch passes all five cases.
The stack_trace_save() result on mainline (0/3) was also reproduced with
GCC 15.2 and QEMU 10.2.1.
Without CONFIG_FRAME_POINTER the walker scans the stack for anything that
looks like a text address and its start frame is not exact before or
after this patch. What return_address() returns there is unchanged: the
walk loses one leading entry and the skip count goes down by one.
Hi Karl,
As promised in the other thread, here are test results for this series.
Short version: the fix is correct, the KUnit suite reproduces the bug
precisely, and the whole series passes here.
Testing
-------
Your numbers were collected with clang 22, plus the mainline
reproduction with GCC 15.2 you noted in the commit message, so I
re-ran the full matrix with gcc: riscv64 cross-gcc 16.2.0 for the
first three configurations below and a native gcc 14.3 for the last
one, all under QEMU (qemu-system-riscv64 11.1.1, -machine virt,
-cpu rv64) on top of your base af32da41b032.
A. base + 2/2 only (tests, no fix): pass:2 fail:3, i.e. exactly the
"stack_trace_save 0/3, return_address 2/2" you reported for
mainline. The failures show the off-by-one shift nicely; with
skip = 0 the first recorded symbol is the walker itself:
# riscv_stacktrace_skip_zero: skip=0 first=stack_trace_save count=11
# symbol == "stack_trace_save"
# expected[skip] == "riscv_stacktrace_leaf"
The two return_address() cases still pass on unpatched mainline,
which is consistent with the analysis: the current "level + 3" is
calibrated for the buggy frame layout, so ftrace_return_address()
keeps working while stack_trace_save() - and everything built on
top of it - records one bogus leading entry.
B. base + 1/2 + 2/2: pass:5 fail:0.
C. My v6 reliable stack unwinding series [1] + your 2/2, with the
level constant in return_address.c adjusted to the same
unconditional "level + 2" as in your 1/2: pass:5 fail:0, with
per-case results identical to B (skip=0/1/2 first =
leaf/middle/outer, count = 10/9/8).
D. Cross-toolchain check: I rebuilt the C tree with a native riscv64
gcc 14.3 inside an Anolis OS 23.4 riscv64 guest - no cross
toolchain involved at any step - using the same Kconfig options,
and booted the resulting Image under the same QEMU: pass:5 fail:0
again, identical output. The result does not depend on one
particular compiler's frame layout.
Please feel free to add
Tested-by: Shuai Xue <[email protected]>
Reviewed-by: Shuai Xue <[email protected]>
Thanks.
Shuai
