Message-ID:  <173153821454.2776880.582308516721515492.stgit@mhiramat.roam.corp.google.com>
Date: Thu, 14 Nov 2024 07:50:14 +0900
From: "Masami Hiramatsu (Google)" <mhiramat@...nel.org>
To: Arnaldo Carvalho de Melo <acme@...nel.org>,
	Namhyung Kim <namhyung@...nel.org>
Cc: Li Huafei <lihuafei1@...wei.com>,
	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>,
	Masami Hiramatsu <mhiramat@...nel.org>,
	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: [PATCH] perf-probe: Fix --line option to show correct offset line number from function

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>
---
 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