Re: [PATCH -tip V2] [BUGFIX] perf probe: Fix to find lineinformation for probe list

From: Arnaldo Carvalho de Melo
Date: Mon Oct 07 2013 - 08:25:38 EST


Em Mon, Oct 07, 2013 at 12:53:40PM +0900, Masami Hiramatsu escreveu:
> Ping? :)

I'll send a perf/urgent pull req later today, this patch is there.

- Arnaldo

> (2013/09/30 18:21), Masami Hiramatsu wrote:
> > Fix to find the correct (as much as possible) line information
> > for listing probes. Without this fix, perf probe --list action
> > will show incorrect line information as below;
> >
> > # perf probe getname_flags
> > # perf probe -l
> > probe:getname_flags (on getname_flags@ksrc/linux-3/fs/namei.c)
> > probe:getname_flags_1 (on getname:-89@x86/include/asm/current.h)
> > probe:getname_flags_2 (on user_path_at_empty:-2054@x86/include/asm/current.h)
> >
> > The minus line number is obviously wrong, and current.h is not
> > related to the probe point. Deeper investigation discovered
> > that there were 2 issues related to this bug, and minor typos too.
> >
> > 1st issue is the rack of considering about nested inlined
> > functions, which causes the wrong (relative) line number.
> > 2nd issue is that the dwarf line info is not correct at
> > those points. It points 14th line of current.h.
> >
> > Since it seems that the line info includes somewhat unreliable
> > information, this fixes perf to try to find correct line information
> > from both of debuginfo and line info as below.
> >
> > 1) Probe address is the entry of a function instance
> > In this case, the line is set as the function declared line.
> >
> > 2) Probe address is the entry of an expanded inline function block
> > In this case, the line is set as the function call-site line.
> > This means that the line number is relative from the entry line
> > of caller function (which can be an inlined function if nested)
> >
> > 3) Probe address is inside a function instance or an expanded
> > inline function block
> > In this case, perf probe queries the line number from lineinfo
> > and verify the function declared file is same as the file name
> > queried from lineinfo.
> > If the file name is different, it is a failure case. The probe
> > address is shown as symbol+offset.
> >
> > 4) Probe address is not in the any function instance
> > This is a failure case, the probe address is shown as
> > symbol+offset.
> >
> > With this fix, perf probe -l shows correct probe lines as below;
> >
> > # perf probe -l
> > probe:getname_flags (on getname_flags@ksrc/linux-3/fs/namei.c)
> > probe:getname_flags_1 (on getname:2@ksrc/linux-3/fs/namei.c)
> > probe:getname_flags_2 (on user_path_at_empty:4@ksrc/linux-3/fs/namei.c)
> >
> > Changes at v2:
> > - Fix typos in the function comments. (Thanks to Namhyung Kim)
> > - Use die_find_top_inlinefunc instead of die_find_inlinefunc_next.
> >
> > Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@xxxxxxxxxxx>
> > Cc: Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>
> > Cc: Paul Mackerras <paulus@xxxxxxxxx>
> > Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> > Cc: Arnaldo Carvalho de Melo <acme@xxxxxxxxxxxxxxxxxx>
> > Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
> > ---
> > tools/perf/util/dwarf-aux.c | 25 +++++++++++++++++---
> > tools/perf/util/dwarf-aux.h | 6 ++++-
> > tools/perf/util/probe-finder.c | 49 +++++++++++++++++++++++++++-------------
> > 3 files changed, 59 insertions(+), 21 deletions(-)
> >
> > diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> > index e23bde1..7defd77 100644
> > --- a/tools/perf/util/dwarf-aux.c
> > +++ b/tools/perf/util/dwarf-aux.c
> > @@ -426,7 +426,7 @@ static int __die_search_func_cb(Dwarf_Die *fn_die, void *data)
> > * @die_mem: a buffer for result DIE
> > *
> > * Search a non-inlined function DIE which includes @addr. Stores the
> > - * DIE to @die_mem and returns it if found. Returns NULl if failed.
> > + * DIE to @die_mem and returns it if found. Returns NULL if failed.
> > */
> > Dwarf_Die *die_find_realfunc(Dwarf_Die *cu_die, Dwarf_Addr addr,
> > Dwarf_Die *die_mem)
> > @@ -454,15 +454,32 @@ static int __die_find_inline_cb(Dwarf_Die *die_mem, void *data)
> > }
> >
> > /**
> > + * die_find_top_inlinefunc - Search the top inlined function at given address
> > + * @sp_die: a subprogram DIE which including @addr
> > + * @addr: target address
> > + * @die_mem: a buffer for result DIE
> > + *
> > + * Search an inlined function DIE which includes @addr. Stores the
> > + * DIE to @die_mem and returns it if found. Returns NULL if failed.
> > + * Even if several inlined functions are expanded recursively, this
> > + * doesn't trace it down, and returns the topmost one.
> > + */
> > +Dwarf_Die *die_find_top_inlinefunc(Dwarf_Die *sp_die, Dwarf_Addr addr,
> > + Dwarf_Die *die_mem)
> > +{
> > + return die_find_child(sp_die, __die_find_inline_cb, &addr, die_mem);
> > +}
> > +
> > +/**
> > * die_find_inlinefunc - Search an inlined function at given address
> > - * @cu_die: a CU DIE which including @addr
> > + * @sp_die: a subprogram DIE which including @addr
> > * @addr: target address
> > * @die_mem: a buffer for result DIE
> > *
> > * Search an inlined function DIE which includes @addr. Stores the
> > - * DIE to @die_mem and returns it if found. Returns NULl if failed.
> > + * DIE to @die_mem and returns it if found. Returns NULL if failed.
> > * If several inlined functions are expanded recursively, this trace
> > - * it and returns deepest one.
> > + * it down and returns deepest one.
> > */
> > Dwarf_Die *die_find_inlinefunc(Dwarf_Die *sp_die, Dwarf_Addr addr,
> > Dwarf_Die *die_mem)
> > diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
> > index 8658d41..b4fe90c 100644
> > --- a/tools/perf/util/dwarf-aux.h
> > +++ b/tools/perf/util/dwarf-aux.h
> > @@ -79,7 +79,11 @@ extern Dwarf_Die *die_find_child(Dwarf_Die *rt_die,
> > extern Dwarf_Die *die_find_realfunc(Dwarf_Die *cu_die, Dwarf_Addr addr,
> > Dwarf_Die *die_mem);
> >
> > -/* Search an inlined function including given address */
> > +/* Search the top inlined function including given address */
> > +extern Dwarf_Die *die_find_top_inlinefunc(Dwarf_Die *sp_die, Dwarf_Addr addr,
> > + Dwarf_Die *die_mem);
> > +
> > +/* Search the deepest inlined function including given address */
> > extern Dwarf_Die *die_find_inlinefunc(Dwarf_Die *sp_die, Dwarf_Addr addr,
> > Dwarf_Die *die_mem);
> >
> > diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> > index 371476c..c09e0a9 100644
> > --- a/tools/perf/util/probe-finder.c
> > +++ b/tools/perf/util/probe-finder.c
> > @@ -1327,8 +1327,8 @@ int debuginfo__find_probe_point(struct debuginfo *self, unsigned long addr,
> > struct perf_probe_point *ppt)
> > {
> > Dwarf_Die cudie, spdie, indie;
> > - Dwarf_Addr _addr, baseaddr;
> > - const char *fname = NULL, *func = NULL, *tmp;
> > + Dwarf_Addr _addr = 0, baseaddr = 0;
> > + const char *fname = NULL, *func = NULL, *basefunc = NULL, *tmp;
> > int baseline = 0, lineno = 0, ret = 0;
> >
> > /* Adjust address with bias */
> > @@ -1349,27 +1349,36 @@ int debuginfo__find_probe_point(struct debuginfo *self, unsigned long addr,
> > /* Find a corresponding function (name, baseline and baseaddr) */
> > if (die_find_realfunc(&cudie, (Dwarf_Addr)addr, &spdie)) {
> > /* Get function entry information */
> > - tmp = dwarf_diename(&spdie);
> > - if (!tmp ||
> > + func = basefunc = dwarf_diename(&spdie);
> > + if (!func ||
> > dwarf_entrypc(&spdie, &baseaddr) != 0 ||
> > - dwarf_decl_line(&spdie, &baseline) != 0)
> > + dwarf_decl_line(&spdie, &baseline) != 0) {
> > + lineno = 0;
> > goto post;
> > - func = tmp;
> > + }
> >
> > - if (addr == (unsigned long)baseaddr)
> > + if (addr == (unsigned long)baseaddr) {
> > /* Function entry - Relative line number is 0 */
> > lineno = baseline;
> > - else if (die_find_inlinefunc(&spdie, (Dwarf_Addr)addr,
> > - &indie)) {
> > + fname = dwarf_decl_file(&spdie);
> > + goto post;
> > + }
> > +
> > + /* Track down the inline functions step by step */
> > + while (die_find_top_inlinefunc(&spdie, (Dwarf_Addr)addr,
> > + &indie)) {
> > + /* There is an inline function */
> > if (dwarf_entrypc(&indie, &_addr) == 0 &&
> > - _addr == addr)
> > + _addr == addr) {
> > /*
> > * addr is at an inline function entry.
> > * In this case, lineno should be the call-site
> > - * line number.
> > + * line number. (overwrite lineinfo)
> > */
> > lineno = die_get_call_lineno(&indie);
> > - else {
> > + fname = die_get_call_file(&indie);
> > + break;
> > + } else {
> > /*
> > * addr is in an inline function body.
> > * Since lineno points one of the lines
> > @@ -1377,19 +1386,27 @@ int debuginfo__find_probe_point(struct debuginfo *self, unsigned long addr,
> > * be the entry line of the inline function.
> > */
> > tmp = dwarf_diename(&indie);
> > - if (tmp &&
> > - dwarf_decl_line(&spdie, &baseline) == 0)
> > - func = tmp;
> > + if (!tmp ||
> > + dwarf_decl_line(&indie, &baseline) != 0)
> > + break;
> > + func = tmp;
> > + spdie = indie;
> > }
> > }
> > + /* Verify the lineno and baseline are in a same file */
> > + tmp = dwarf_decl_file(&spdie);
> > + if (!tmp || strcmp(tmp, fname) != 0)
> > + lineno = 0;
> > }
> >
> > post:
> > /* Make a relative line number or an offset */
> > if (lineno)
> > ppt->line = lineno - baseline;
> > - else if (func)
> > + else if (basefunc) {
> > ppt->offset = addr - (unsigned long)baseaddr;
> > + func = basefunc;
> > + }
> >
> > /* Duplicate strings */
> > if (func) {
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
> >
>
>
> --
> Masami HIRAMATSU
> IT Management Research Dept. Linux Technology Center
> Hitachi, Ltd., Yokohama Research Laboratory
> E-mail: masami.hiramatsu.pt@xxxxxxxxxxx
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/