Re: [PATCH] Perf: support to unwind userspace application stacks generated with thumb.

Re: [PATCH] Perf: support to unwind userspace application stacks generated with thumb.

[Will Deacon]

On Thu, Apr 16, 2020 at 01:38:29PM +0800, Jiping Ma wrote:
> Only push sp and lr in the stack for thumb mode. it will go
> through the stack find sp and lr.
> 
> Change this to the more detailed description of the patch
> 
> Signed-off-by: Jiping Ma <jiping.ma2@windriver.com>
> ---
>  arch/arm64/kernel/perf_callchain.c | 36 +++++++++++++++++++++++++++---
>  1 file changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/kernel/perf_callchain.c b/arch/arm64/kernel/perf_callchain.c
> index bcafd7dcfe8b..97dde271c121 100644
> --- a/arch/arm64/kernel/perf_callchain.c
> +++ b/arch/arm64/kernel/perf_callchain.c
> @@ -104,6 +104,30 @@ compat_user_backtrace(struct compat_frame_tail __user *tail,
>  
>  	return (struct compat_frame_tail __user *)compat_ptr(buftail.fp) - 1;
>  }
> +
> +void
> +user_backtrace_thumb(struct perf_callchain_entry_ctx *entry,
> +		     struct pt_regs *regs)
> +{
> +	u32 sp;
> +	u32 *sp_t;
> +	/*
> +	 * Only push sp, lr to stack.
> +	 */
> +	for (sp = regs->compat_sp; (sp < current->mm->start_stack) &&
> +		(entry->nr < entry->max_stack); sp += 4) {
> +		sp_t = (u32 *)(unsigned long)sp;
> +		if ((*sp_t > regs->compat_sp) &&
> +			(*sp_t < current->mm->start_stack)) {
> +			if (*(sp_t + 1) < current->mm->end_code &&
> +				*(sp_t + 1) > current->mm->start_code) {
> +				perf_callchain_store(entry,  *(sp_t + 1)-1);
> +				sp += 4;
> +			}
> +		}
> +	}
> +}

This looks like a pile of fragile heuristics to me. Why don't you just use
libunwind in userspace, the same way you'd have to if you compiled without
framepointers?

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] Perf: support to unwind userspace application stacks generated with thumb.

[Will Deacon]

On Wed, Apr 22, 2020 at 10:56:04AM +0800, Jiping Ma wrote:
> On 04/16/2020 03:10 PM, Will Deacon wrote:
> > On Thu, Apr 16, 2020 at 01:38:29PM +0800, Jiping Ma wrote:
> > > +void
> > > +user_backtrace_thumb(struct perf_callchain_entry_ctx *entry,
> > > +		     struct pt_regs *regs)
> > > +{
> > > +	u32 sp;
> > > +	u32 *sp_t;
> > > +	/*
> > > +	 * Only push sp, lr to stack.
> > > +	 */
> > > +	for (sp = regs->compat_sp; (sp < current->mm->start_stack) &&
> > > +		(entry->nr < entry->max_stack); sp += 4) {
> > > +		sp_t = (u32 *)(unsigned long)sp;
> > > +		if ((*sp_t > regs->compat_sp) &&
> > > +			(*sp_t < current->mm->start_stack)) {
> > > +			if (*(sp_t + 1) < current->mm->end_code &&
> > > +				*(sp_t + 1) > current->mm->start_code) {
> > > +				perf_callchain_store(entry,  *(sp_t + 1)-1);
> > > +				sp += 4;
> > > +			}
> > > +		}
> > > +	}
> > > +}
> > This looks like a pile of fragile heuristics to me. Why don't you just use
> > libunwind in userspace, the same way you'd have to if you compiled without
> > framepointers?
> 
> Could you share more details for libunwind in userspace? Following is our
> build command for test app.
> bt_perf.thumb: arm-wrs-linux-gnueabi-gcc -march=armv7ve -mthumb -mfpu=neon
> -mfloat-abi=softfp -mcpu=cortex-a15
> --sysroot=sysroots/cortexa15t2-neon-wrs-linux-gnueabi -O0 -g -ggdb -Wall
> -funwind-tables -fno-omit-frame-pointer -frecord-gcc-switches -mapcs-frame
> bt_perf.c -o bt_perf.thumb

You can build the perf tool with support for libunwind, and then it should
handle the unwinding in userspace. I haven't used it for ages, but you may
need to pass '--call-graph dwarf'. Have a play (and you can also read the
code in tools/perf/).

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel