lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ