Re: [PATCH 6/8] ARM: clang: Do not relay on lr register for stacktrace

[Nick Desaulniers]

On Tue, Oct 12, 2021 at 7:18 AM Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote:
>
> Hi Nick,
>
> On Mon, 11 Oct 2021 11:45:22 -0700
> Nick Desaulniers <ndesaulniers@xxxxxxxxxx> wrote:
>
> >
> > If I change from CONFIG_UNWINDER_ARM=y to
> > CONFIG_UNWINDER_FRAME_POINTER=y, before:
> >
> > # cat /proc/self/stack
> > [<0>] stack_trace_save_tsk+0x50/0x6c
> > [<0>] proc_pid_stack+0xa0/0xf8
> > [<0>] proc_single_show+0x50/0xbc
> > [<0>] seq_read_iter+0x178/0x4ec
> > [<0>] seq_read+0x138/0x15c
> > [<0>] vfs_read+0xd0/0x304
> > [<0>] ksys_read+0x78/0xd4
> > [<0>] sys_read+0xc/0x10
> >
> > after:
> > # cat /proc/self/stack
> > [<0>] proc_pid_stack+0xa0/0xf8
> > [<0>] proc_single_show+0x50/0xbc
> > [<0>] seq_read_iter+0x178/0x4ec
> > [<0>] seq_read+0x138/0x15c
> > [<0>] vfs_read+0xd0/0x304
> > [<0>] ksys_read+0x78/0xd4
> > [<0>] sys_read+0xc/0x10
> > [<0>] __entry_text_start+0x14/0x14
> > [<0>] 0xffffffff
> >
> > So I guess this helps the CONFIG_UNWINDER_FRAME_POINTER=y case? (That
> > final frame address looks wrong, but is potentially yet another bug;
> > perhaps for clang we need to manually store the previous frame's pc at
> > a different offset before jumping to __entry_text_start).
>
> Ah, yes. I didn't touch the UNWINDER_ARM. As you may know the
> unwind_frame()@arch/arm/kernel/stacktrace.c is compiled only
> CONFIG_UNWINDER_FRAME_POINTER=y.

Ah, right, thanks for pointing that out.

> > so likely this fixes/improves CONFIG_UNWINDER_FRAME_POINTER=y? Is that correct?
>
> Yes, that is correct.

In that case, additionally:
Reviewed-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>

-- 
Thanks,
~Nick Desaulniers