[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <6d88ae99-c624-c08e-2272-5866105b10f1@huawei.com>
Date: Thu, 14 Nov 2024 17:16:17 +0800
From: Li Huafei <lihuafei1@...wei.com>
To: "Masami Hiramatsu (Google)" <mhiramat@...nel.org>, Arnaldo Carvalho de
Melo <acme@...nel.org>, Namhyung Kim <namhyung@...nel.org>
CC: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Mark Rutland <mark.rutland@....com>, Alexander Shishkin
<alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>, Ian
Rogers <irogers@...gle.com>, Adrian Hunter <adrian.hunter@...el.com>, Liang
Kan <kan.liang@...ux.intel.com>, James Clark <james.clark@...aro.org>, Yury
Norov <yury.norov@...il.com>, Przemek Kitszel <przemyslaw.kitszel@...el.com>,
Alexander Lobakin <aleksander.lobakin@...el.com>, "David S . Miller"
<davem@...emloft.net>, <linux-perf-users@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] perf-probe: Fix --line option to show correct offset line
number from function
On 2024/11/14 6:50, Masami Hiramatsu (Google) wrote:
> From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
>
> Fix --line option to show correct offset if the DWARF line info of the
> probe address has the statement lines in differnt functions.
> Currently debuginfo__find_probe_point() does
>
> (1) Get the line and file from CU's lineinfo
> (2) Get the real function(function instance) of the address
> (use this function's decl_line/decl_file as basement)
> (2-1) Search the inlined function scope in the real function
> for the given address.
> (2-2) if there is inlined function, update basement line/file.
> (2-3) verify the filename is same as basement filename.
> (3) calculate the relative line number from basement.
>
> The problem is in (1). Since we have no basement file/line info,
> we can not verify that the file/line info from CU's lineinfo.
> Li Huafei reported[1], the lineinfo may have several different lines
> for one address. We need to find most appropriate one based on
> the basement file/line.
>
> [1] https://lore.kernel.org/all/20241108181909.3515716-2-lihuafei1@huawei.com/
>
> This basically does
>
> - Introduce cu_find_lineinfo_after() which find the line after the
> basement file/line information so that it can choose correct one.
> - Swap the order of (1) and (2) so that we can pass the basement
> file/line when searching lineinfo.
>
> Reported-by: Li Huafei <lihuafei1@...wei.com>
> Closes: https://lore.kernel.org/all/20241108181909.3515716-2-lihuafei1@huawei.com/
> Fixes: 57f95bf5f882 ("perf probe: Show correct statement line number by perf probe -l")
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@...nel.org>
Tested-by: Li Huafei <lihuafei1@...wei.com>
Thanks,
Huafei
> ---
> tools/perf/util/dwarf-aux.c | 78 ++++++++++++++++++++++++++++++++++------
> tools/perf/util/dwarf-aux.h | 7 ++++
> tools/perf/util/probe-finder.c | 44 ++++++++++++++++-------
> 3 files changed, 105 insertions(+), 24 deletions(-)
>
> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index 92eb9c8dc3e5..4e3bf41746c1 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -60,14 +60,33 @@ const char *cu_get_comp_dir(Dwarf_Die *cu_die)
> return dwarf_formstring(&attr);
> }
>
> +static Dwarf_Line *get_next_statement_line(Dwarf_Lines *lines, size_t *idx, Dwarf_Addr addr)
> +{
> + bool is_statement = false;
> + Dwarf_Line *line;
> + Dwarf_Addr laddr;
> + size_t l = *idx;
> +
> + while (!is_statement) {
> + line = dwarf_onesrcline(lines, ++l);
> + if (!line || dwarf_lineaddr(line, &laddr) != 0 ||
> + dwarf_linebeginstatement(line, &is_statement) != 0)
> + return NULL;
> + if (laddr > addr)
> + return NULL;
> + }
> + *idx = l;
> + return line;
> +}
> +
> /* Unlike dwarf_getsrc_die(), cu_getsrc_die() only returns statement line */
> -static Dwarf_Line *cu_getsrc_die(Dwarf_Die *cu_die, Dwarf_Addr addr)
> +static Dwarf_Line *cu_getsrc_die(Dwarf_Die *cu_die, Dwarf_Addr addr,
> + Dwarf_Lines **lines_p, size_t *idx)
> {
> Dwarf_Addr laddr;
> Dwarf_Lines *lines;
> Dwarf_Line *line;
> size_t nlines, l, u, n;
> - bool flag;
>
> if (dwarf_getsrclines(cu_die, &lines, &nlines) != 0 ||
> nlines == 0)
> @@ -91,16 +110,14 @@ static Dwarf_Line *cu_getsrc_die(Dwarf_Die *cu_die, Dwarf_Addr addr)
> if (!line || dwarf_lineaddr(line, &laddr) != 0)
> return NULL;
> } while (laddr == addr);
> - l++;
> /* Going forward to find the statement line */
> - do {
> - line = dwarf_onesrcline(lines, l++);
> - if (!line || dwarf_lineaddr(line, &laddr) != 0 ||
> - dwarf_linebeginstatement(line, &flag) != 0)
> - return NULL;
> - if (laddr > addr)
> - return NULL;
> - } while (!flag);
> + line = get_next_statement_line(lines, &l, addr);
> + if (!line)
> + return NULL;
> + if (lines_p)
> + *lines_p = lines;
> + if (idx)
> + *idx = l;
>
> return line;
> }
> @@ -129,7 +146,7 @@ int cu_find_lineinfo(Dwarf_Die *cu_die, Dwarf_Addr addr,
> goto out;
> }
>
> - line = cu_getsrc_die(cu_die, addr);
> + line = cu_getsrc_die(cu_die, addr, NULL, NULL);
> if (line && dwarf_lineno(line, lineno) == 0) {
> *fname = dwarf_linesrc(line, NULL, NULL);
> if (!*fname)
> @@ -141,6 +158,43 @@ int cu_find_lineinfo(Dwarf_Die *cu_die, Dwarf_Addr addr,
> return (*lineno && *fname) ? *lineno : -ENOENT;
> }
>
> +/**
> + * cu_find_lineinfo_after - Get a line number after file:line for given address
> + * @cu_die: a CU DIE
> + * @addr: An address
> + * @lineno: a pointer which returns the line number
> + * @fname: the filename where searching the line
> + * @baseline: the basement line number
> + *
> + * Find a line number after @baseline in @fname for @addr in @cu_die.
> + * Return the found line number, or -ENOENT if not found.
> + */
> +int cu_find_lineinfo_after(Dwarf_Die *cu_die, Dwarf_Addr addr,
> + int *lineno, const char *fname, int baseline)
> +{
> + const char *line_fname;
> + Dwarf_Lines *lines;
> + Dwarf_Line *line;
> + size_t idx = 0;
> +
> + if (cu_find_lineinfo(cu_die, addr, &line_fname, lineno) < 0)
> + return -ENOENT;
> +
> + if (!strcmp(line_fname, fname) && baseline <= *lineno)
> + return *lineno;
> +
> + line = cu_getsrc_die(cu_die, addr, &lines, &idx);
> +
> + while (line && dwarf_lineno(line, lineno) == 0) {
> + line_fname = dwarf_linesrc(line, NULL, NULL);
> + if (line_fname && !strcmp(line_fname, fname) && baseline <= *lineno)
> + return *lineno;
> + line = get_next_statement_line(lines, &idx, addr);
> + }
> +
> + return -ENOENT;
> +}
> +
> static int __die_find_inline_cb(Dwarf_Die *die_mem, void *data);
>
> /**
> diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
> index bd7505812569..19edf21e2f78 100644
> --- a/tools/perf/util/dwarf-aux.h
> +++ b/tools/perf/util/dwarf-aux.h
> @@ -23,6 +23,13 @@ const char *cu_get_comp_dir(Dwarf_Die *cu_die);
> int cu_find_lineinfo(Dwarf_Die *cudie, Dwarf_Addr addr,
> const char **fname, int *lineno);
>
> +/*
> + * Get the most likely line number for given address in given filename
> + * and basement line number.
> + */
> +int cu_find_lineinfo_after(Dwarf_Die *cudie, Dwarf_Addr addr,
> + int *lineno, const char *fname, int baseline);
> +
> /* Walk on functions at given address */
> int cu_walk_functions_at(Dwarf_Die *cu_die, Dwarf_Addr addr,
> int (*callback)(Dwarf_Die *, void *), void *data);
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index 13ff45d3d6a4..efcacb5568e5 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -1578,7 +1578,8 @@ int debuginfo__find_probe_point(struct debuginfo *dbg, u64 addr,
> {
> Dwarf_Die cudie, spdie, indie;
> Dwarf_Addr _addr = 0, baseaddr = 0;
> - const char *fname = NULL, *func = NULL, *basefunc = NULL, *tmp;
> + const char *fname = NULL, *func = NULL, *basefunc = NULL;
> + const char *basefname = NULL, *tmp;
> int baseline = 0, lineno = 0, ret = 0;
>
> /* We always need to relocate the address for aranges */
> @@ -1592,11 +1593,7 @@ int debuginfo__find_probe_point(struct debuginfo *dbg, u64 addr,
> goto end;
> }
>
> - /* Find a corresponding line (filename and lineno) */
> - cu_find_lineinfo(&cudie, (Dwarf_Addr)addr, &fname, &lineno);
> - /* Don't care whether it failed or not */
> -
> - /* Find a corresponding function (name, baseline and baseaddr) */
> + /* Find the basement function (name, baseline and baseaddr) */
> if (die_find_realfunc(&cudie, (Dwarf_Addr)addr, &spdie)) {
> /* Get function entry information */
> func = basefunc = dwarf_diename(&spdie);
> @@ -1607,10 +1604,16 @@ int debuginfo__find_probe_point(struct debuginfo *dbg, u64 addr,
> goto post;
> }
>
> - fname = die_get_decl_file(&spdie);
> + basefname = die_get_decl_file(&spdie);
> + if (!basefname) {
> + lineno = 0;
> + goto post;
> + }
> +
> if (addr == baseaddr) {
> /* Function entry - Relative line number is 0 */
> lineno = baseline;
> + fname = basefname;
> goto post;
> }
>
> @@ -1627,7 +1630,9 @@ int debuginfo__find_probe_point(struct debuginfo *dbg, u64 addr,
> */
> lineno = die_get_call_lineno(&indie);
> fname = die_get_call_file(&indie);
> - break;
> + if (!fname || strcmp(fname, basefname))
> + lineno = 0;
> + goto post;
> } else {
> /*
> * addr is in an inline function body.
> @@ -1636,20 +1641,35 @@ int debuginfo__find_probe_point(struct debuginfo *dbg, u64 addr,
> * be the entry line of the inline function.
> */
> tmp = dwarf_diename(&indie);
> - if (!tmp ||
> - dwarf_decl_line(&indie, &baseline) != 0)
> - break;
> + basefname = die_get_decl_file(&indie);
> + if (!tmp || !basefname ||
> + dwarf_decl_line(&indie, &baseline) != 0) {
> + lineno = 0;
> + goto post;
> + }
> func = tmp;
> spdie = indie;
> }
> }
> + }
> +
> + if (!lineno) {
> + /* Find a corresponding line (filename and lineno) */
> + if (cu_find_lineinfo_after(&cudie, (Dwarf_Addr)addr, &lineno,
> + basefname, baseline) < 0)
> + lineno = 0;
> + else
> + fname = basefname;
> + }
> +
> +post:
> + if (lineno) {
> /* Verify the lineno and baseline are in a same file */
> tmp = die_get_decl_file(&spdie);
> if (!tmp || (fname && strcmp(tmp, fname) != 0))
> lineno = 0;
> }
>
> -post:
> /* Make a relative line number or an offset */
> if (lineno)
> ppt->line = lineno - baseline;
>
>
> .
>
Powered by blists - more mailing lists