Em Thu, Apr 12, 2018 at 10:41:28PM +0530, Sandipan Das escreveu:
> For powerpc64, if a probe is added for a function without specifying
> a line number, the corresponding trap instruction is placed at offset
> 0 (for big endian) or 8 (for little endian) from the start address of
> the function. This address is in the function prologue and the trap
> instruction preceeds the instructions to set up the stack frame.

So, the author for the fixed-by patch here is Sukadev Bhattiprolu
<suka...@linux.vnet.ibm.com>, and the reporter for the problem that
patch fixed is Maynard Johnson <mayn...@us.ibm.com>, who also tested
that patch, I think it'd be better if they get CCed to have the
opportunity to ack/comment, but since everybody is at IBM, perhaps
those guys are not anymore involved with ppc at IBM?

I'm CCing anyway :-)

- Arnaldo
 
> Therefore, at this point during execution, the return address for the
> function is yet to be written to its caller's stack frame. So, the LR
> value at index 2 of the callchain ips provided by the kernel is still
> valid and must not be skipped.
> 
> This can be observed on a powerpc64le system running Fedora 27 as
> shown below.
> 
>  # perf probe -x /usr/lib64/libc-2.26.so -a inet_pton
>  # perf record -e probe_libc:inet_pton/max-stack=3/ ping -6 -c 1 ::1
>  # perf script
> 
> Without this patch, the output is:
> 
>  ping 27909 [007] 532219.943481: probe_libc:inet_pton: (7fff99b0af28)
>                    15af28 __GI___inet_pton (/usr/lib64/libc-2.26.so)
>                    1105b4 getaddrinfo (/usr/lib64/libc-2.26.so)
> 
> With this patch applied, the output is:
> 
>  ping 27909 [007] 532219.943481: probe_libc:inet_pton: (7fff99b0af28)
>                    15af28 __GI___inet_pton (/usr/lib64/libc-2.26.so)
>                    10fa54 gaih_inet.constprop.7 (/usr/lib64/libc-2.26.so)
>                    1105b4 getaddrinfo (/usr/lib64/libc-2.26.so)
> 
> Fixes: a60335ba3298 ("perf tools powerpc: Adjust callchain based on DWARF 
> debug info")
> Signed-off-by: Sandipan Das <sandi...@linux.vnet.ibm.com>
> ---
>  tools/perf/arch/powerpc/util/skip-callchain-idx.c | 58 
> ++++++++++++++++-------
>  1 file changed, 41 insertions(+), 17 deletions(-)
> 
> diff --git a/tools/perf/arch/powerpc/util/skip-callchain-idx.c 
> b/tools/perf/arch/powerpc/util/skip-callchain-idx.c
> index 0c370f81e002..f5179f5bb306 100644
> --- a/tools/perf/arch/powerpc/util/skip-callchain-idx.c
> +++ b/tools/perf/arch/powerpc/util/skip-callchain-idx.c
> @@ -212,6 +212,37 @@ static int check_return_addr(struct dso *dso, u64 
> map_start, Dwarf_Addr pc)
>       return rc;
>  }
>  
> +/*
> + * Return:
> + *   0 if return address for the program counter @pc is on stack
> + *   1 if return address is in LR and no new stack frame was allocated
> + *   2 if return address is in LR and a new frame was allocated (but not
> + *           yet used)
> + *   -1 in case of errors
> + */
> +static int get_return_addr(struct thread *thread, u64 ip)
> +{
> +     struct addr_location al;
> +     struct dso *dso = NULL;
> +     int rc = -1;
> +
> +     thread__find_addr_location(thread, PERF_RECORD_MISC_USER,
> +                     MAP__FUNCTION, ip, &al);
> +
> +     if (!al.map || !al.map->dso) {
> +             pr_debug("%" PRIx64 " dso is NULL\n", ip);
> +             return rc;
> +     }
> +
> +     dso = al.map->dso;
> +     rc = check_return_addr(dso, al.map->start, ip);
> +
> +     pr_debug("[DSO %s, sym %s, ip 0x%" PRIx64 "] rc %d\n",
> +                             dso->long_name, al.sym->name, ip, rc);
> +
> +     return rc;
> +}
> +
>  /*
>   * The callchain saved by the kernel always includes the link register (LR).
>   *
> @@ -237,32 +268,25 @@ static int check_return_addr(struct dso *dso, u64 
> map_start, Dwarf_Addr pc)
>   */
>  int arch_skip_callchain_idx(struct thread *thread, struct ip_callchain 
> *chain)
>  {
> -     struct addr_location al;
> -     struct dso *dso = NULL;
>       int rc;
> -     u64 ip;
>       u64 skip_slot = -1;
>  
>       if (chain->nr < 3)
>               return skip_slot;
>  
> -     ip = chain->ips[2];
> +     rc = get_return_addr(thread, chain->ips[1]);
>  
> -     thread__find_addr_location(thread, PERF_RECORD_MISC_USER,
> -                     MAP__FUNCTION, ip, &al);
> -
> -     if (al.map)
> -             dso = al.map->dso;
> -
> -     if (!dso) {
> -             pr_debug("%" PRIx64 " dso is NULL\n", ip);
> +     if (rc == 1)
> +             /* Return address is still in LR and has not been updated
> +              * in caller's stack frame. This is because the probe was
> +              * placed at an offset from the start of the function that
> +              * comes before the prologue code to set up the stack frame.
> +              * So, an attempt to skip an entry based on chain->ips[2],
> +              * i.e. the LR value, should not be made.
> +              */
>               return skip_slot;
> -     }
>  
> -     rc = check_return_addr(dso, al.map->start, ip);
> -
> -     pr_debug("[DSO %s, sym %s, ip 0x%" PRIx64 "] rc %d\n",
> -                             dso->long_name, al.sym->name, ip, rc);
> +     rc = get_return_addr(thread, chain->ips[2]);
>  
>       if (rc == 0) {
>               /*
> -- 
> 2.14.3

Reply via email to