Re: [PATCH 1/1] arm: backtrace-clang: fix wrong sp usage for unwinding

["Ard Biesheuvel" <ardb@kernel.org>]

Hello all,

On Mon, 3 Aug 2026, at 21:22, Nathan Chancellor wrote:
> Hi Maninder,
>
> On Wed, Jul 29, 2026 at 09:11:01AM +0530, Maninder Singh wrote:
>> Ping! Any comments?
>
> I am not that familiar with this code but maybe Nick (whose address I
> have now updated to point to a better one) or Ard could help take a
> look? Original patch is at
>
>   https://lore.kernel.org/20260624054916.1571701-1-maninder1.s@samsung.com/
>
> but I have left it inline as well.
>
>> > show_stack() can be called for any task, however c_backtrace always unwinds
>> > frames based on the "sp" register. This results in printing the backtrace of
>> > the current task instead of the target task.
>> > 
>> > Try with normal TC:
>> > ==================
>> >        for_each_process(p) {
>> >                sched_show_task(p);
>> >        }
>> > 
>> > [7.433271] task:kthreadd        state:S stack:0     pid:2     tgid:2     ppid:0      task_flags:0x208040 flags:0x00000000
>> > [7.433633] Call trace:
>> > [7.433640] [<80113418>] (dump_backtrace) from [<80113510>] (show_stack+0x14/0x18)
>> > ..
>> > [7.433676] [<8016749c>] (sched_show_task) from [<803cb324>] (meminfo_proc_show+0x6c/0x930)
>> > [7.434019]  r5:8158e300 r4:8b4882d0
>> > [7.434024] [<803cb324>] (meminfo_proc_show) from [<80365788>] (seq_read_iter+0x148/0x4bc)
>> > [7.434045] [<80365788>] (seq_read_iter) from [<803c044c>] (proc_reg_read_iter+0xb8/0xc4)
>> > [7.434060] [<803c044c>] (proc_reg_read_iter) from [<80377104>] (copy_splice_read+0x228/0x308)
>> > 
>> > It should unwind frame based on passed "fp".
>> > (CONFIG_UNWINDER_FRAME_POINTER=y)
>> > 
>> > With fix:
>> > =========
>> > [13.933732] task:kthreadd        state:S stack:0     pid:2     tgid:2     ppid:0      task_flags:0x208040 flags:0x00000000
>> > [13.934165] Call trace:
>> > [13.934604] [<80afa1a8>] (schedule) from [<8015426c>] (kthreadd+0x124/0x208)
>> > [13.934654]  r10:8100bbf0 r4:8116f440
>> > [13.934664] [<8015426c>] (kthreadd) from [<8010010c>] (ret_from_fork+0x14/0x28)
>> > [13.934691] Exception stack(0xf081df9c to 0xf081dfe4)
>> > 
>> > Additionally, the extra manipulation of "sp" register appears unnecessary in the "current"
>> > task also since the "fp" register is already provided.
>> > 
>> > Signed-off-by: Onkarnath 
>> > Signed-off-by: Maninder Singh 
>> > ---
>> >  arch/arm/lib/backtrace-clang.S | 4 ----
>> >  1 file changed, 4 deletions(-)
>> > 
>> > diff --git a/arch/arm/lib/backtrace-clang.S b/arch/arm/lib/backtrace-clang.S
>> > index 290c52a60fc6..993410a6afd1 100644
>> > --- a/arch/arm/lib/backtrace-clang.S
>> > +++ b/arch/arm/lib/backtrace-clang.S
>> > @@ -105,10 +105,6 @@ ENDPROC(c_backtrace)
>> >  		moveq	mask, #0xfc000003
>> >  		movne	mask, #0		@ mask for 32-bit
>> >  
>> > -/*
>> > - * Switches the current frame to be the frame for dump_stack.
>> > - */
>> > -		add	frame, sp, #24		@ switch to false frame
>> >  for_each_frame:	tst	frame, mask		@ Check for address exceptions
>> >  		bne	no_frame
>> >  

The existing code does seem to be broken, as the provided frame pointer
value is ignored entirely.

However, by removing this add, you are skipping the false frame, which
was added to ensure that calling c_backtrace() from dump_backtrace()
does not miss any frames when regs == NULL.

So I think the correct approach here is to make the add conditional
on whether frame == fp, which will be the case when c_backtrace()
is called from dump_backtrace() with a NULL regs argument.

I.e.,

  cmp    frame, fp
  addeq  frame, sp, #24