[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20230609235419.204624-2-irogers@google.com>
Date: Fri, 9 Jun 2023 16:54:19 -0700
From: Ian Rogers <irogers@...gle.com>
To: Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Ian Rogers <irogers@...gle.com>,
Adrian Hunter <adrian.hunter@...el.com>,
Nathan Chancellor <nathan@...nel.org>,
Nick Desaulniers <ndesaulniers@...gle.com>,
Tom Rix <trix@...hat.com>, linux-perf-users@...r.kernel.org,
linux-kernel@...r.kernel.org, llvm@...ts.linux.dev,
Changbin Du <changbin.du@...wei.com>
Subject: [PATCH v1 2/2] perf srcline: Make sentinel reading for binutils
addr2line more robust
The addr2line process is sent an address then multiple function,
filename:line "records" are read. To detect the end of output a ',' is
sent and for llvm-addr2line a ',' is then read back showing the end of
addrline's output. For binutils addr2line the ',' translates to
address 0 and we expect the bogus filename marker "??:0" (see
filename_split) to be sent from addr2line. For some kernels address 0
may have a mapping and so a seemingly valid inline output is given and
breaking the sentinel discovery:
```
$ addr2line -e vmlinux -f -i
,
__per_cpu_start
./arch/x86/kernel/cpu/common.c:1850
```
To avoid this problem enable the address dumping for addr2line (the -a
option). If an address of 0x0000000000000000 is read then this is the
sentinel value working around the problem above. The filename_split
still needs to check for "??:0" as bogus non-zero addresses also need
handling.
Reported-by: Changbin Du <changbin.du@...wei.com>
Signed-off-by: Ian Rogers <irogers@...gle.com>
---
tools/perf/util/srcline.c | 61 ++++++++++++++++++++++++++++++++++-----
1 file changed, 53 insertions(+), 8 deletions(-)
diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c
index fc85cdd6c8f9..c99a001453b4 100644
--- a/tools/perf/util/srcline.c
+++ b/tools/perf/util/srcline.c
@@ -406,7 +406,7 @@ static struct child_process *addr2line_subprocess_init(const char *addr2line_pat
const char *argv[] = {
addr2line_path ?: "addr2line",
"-e", binary_path,
- "-i", "-f", NULL
+ "-a", "-i", "-f", NULL
};
struct child_process *a2l = zalloc(sizeof(*a2l));
int start_command_status = 0;
@@ -461,10 +461,10 @@ static enum a2l_style addr2line_configure(struct child_process *a2l, const char
style = LLVM;
cached = true;
lines = 1;
- } else if (ch == '?') {
+ } else if (ch == '0') {
style = GNU_BINUTILS;
cached = true;
- lines = 2;
+ lines = 3;
} else {
if (!symbol_conf.disable_add2line_warn) {
char *output;
@@ -516,20 +516,64 @@ static int read_addr2line_record(struct io *io,
if (line_nr != NULL)
*line_nr = 0;
+ /*
+ * Read the first line. Without an error this will be either an address
+ * like 0x1234 or for llvm-addr2line the sentinal ',' character.
+ */
if (io__getline(io, &line, &line_len) < 0 || !line_len)
goto error;
- if (style == LLVM && line_len == 2 && line[0] == ',') {
- zfree(&line);
- return 0;
+ if (style == LLVM) {
+ if (line_len == 2 && line[0] == ',') {
+ zfree(&line);
+ return 0;
+ }
+ } else {
+ int zero_count = 0, non_zero_count = 0;
+
+ /* The address should always start 0x. */
+ if (line_len < 2 || line[0] != '0' || line[1] != 'x')
+ goto error;
+
+ for (size_t i = 2; i < line_len; i++) {
+ if (line[i] == '0')
+ zero_count++;
+ else
+ non_zero_count++;
+ }
+ if (!non_zero_count) {
+ int ch;
+
+ if (!zero_count) {
+ /* Line was erroneous just '0x'. */
+ goto error;
+ }
+ /*
+ * Line was 0x0..0, the sentinel for binutils. Remove
+ * the function and filename lines.
+ */
+ zfree(&line);
+ do {
+ ch = io__get_char(io);
+ } while (ch > 0 && ch != '\n');
+ do {
+ ch = io__get_char(io);
+ } while (ch > 0 && ch != '\n');
+ return 0;
+ }
}
+ /* Read the second function name line. */
+ if (io__getline(io, &line, &line_len) < 0 || !line_len)
+ goto error;
+
if (function != NULL)
*function = strdup(strim(line));
zfree(&line);
line_len = 0;
+ /* Read the third filename and line number line. */
if (io__getline(io, &line, &line_len) < 0 || !line_len)
goto error;
@@ -633,8 +677,9 @@ static int addr2line(const char *dso_name, u64 addr,
goto out;
case 0:
/*
- * The first record was invalid, so return failure, but first read another
- * record, since we asked a junk question and have to clear the answer out.
+ * The first record was invalid, so return failure, but first
+ * read another record, since we sent a sentinel ',' for the
+ * sake of detected the last inlined function.
*/
switch (read_addr2line_record(&io, a2l_style, NULL, NULL, NULL)) {
case -1:
--
2.41.0.162.gfafddb0af9-goog
Powered by blists - more mailing lists