2023-06-13 04:17:13

by Ian Rogers

[permalink] [raw]
Subject: [PATCH v2 0/2] Fix srcline addr2line issue with the ',' sentinel

addr2line makes it hard to detect when inline output has finished. To
detect it srcline issues a bogus ',' symbol and expects to see a
filename:line of "??:0". For binutils addr2line ',' gets turned into
address 0 and if there is a symbol there then the sentinel isn't
detected, this problem has been occurring with kernel binaries.

The issue was reported by and debugged by Changbin Du <[email protected]>.

v2. Incorporate feedback from Changbin.

Ian Rogers (2):
perf srcline: Make addr2line configuration failure more verbose
perf srcline: Make sentinel reading for binutils addr2line more robust

tools/perf/util/srcline.c | 93 ++++++++++++++++++++++++++++++---------
1 file changed, 73 insertions(+), 20 deletions(-)

--
2.41.0.162.gfafddb0af9-goog



2023-06-13 04:22:28

by Changbin Du

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] Fix srcline addr2line issue with the ',' sentinel

The problem is fixed, thanks!

Tested-by: Changbin Du <[email protected]>

On Mon, Jun 12, 2023 at 08:48:15PM -0700, Ian Rogers wrote:
> addr2line makes it hard to detect when inline output has finished. To
> detect it srcline issues a bogus ',' symbol and expects to see a
> filename:line of "??:0". For binutils addr2line ',' gets turned into
> address 0 and if there is a symbol there then the sentinel isn't
> detected, this problem has been occurring with kernel binaries.
>
> The issue was reported by and debugged by Changbin Du <[email protected]>.
>
> v2. Incorporate feedback from Changbin.
>
> Ian Rogers (2):
> perf srcline: Make addr2line configuration failure more verbose
> perf srcline: Make sentinel reading for binutils addr2line more robust
>
> tools/perf/util/srcline.c | 93 ++++++++++++++++++++++++++++++---------
> 1 file changed, 73 insertions(+), 20 deletions(-)
>
> --
> 2.41.0.162.gfafddb0af9-goog
>

--
Cheers,
Changbin Du

2023-06-13 04:22:28

by Ian Rogers

[permalink] [raw]
Subject: [PATCH v2 1/2] perf srcline: Make addr2line configuration failure more verbose

To aid debugging why it fails. Also, combine the loops for reading a
line for the llvm/binutils cases.

Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/srcline.c | 34 +++++++++++++++++++++-------------
1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c
index aec596a0b0bb..d477332586b2 100644
--- a/tools/perf/util/srcline.c
+++ b/tools/perf/util/srcline.c
@@ -443,7 +443,7 @@ enum a2l_style {
LLVM,
};

-static enum a2l_style addr2line_configure(struct child_process *a2l)
+static enum a2l_style addr2line_configure(struct child_process *a2l, const char *dso_name)
{
static bool cached;
static enum a2l_style style;
@@ -452,6 +452,7 @@ static enum a2l_style addr2line_configure(struct child_process *a2l)
char buf[128];
struct io io;
int ch;
+ int lines;

if (write(a2l->in, ",\n", 2) != 2)
return BROKEN;
@@ -461,19 +462,29 @@ static enum a2l_style addr2line_configure(struct child_process *a2l)
if (ch == ',') {
style = LLVM;
cached = true;
+ lines = 1;
} else if (ch == '?') {
style = GNU_BINUTILS;
cached = true;
+ lines = 2;
} else {
- style = BROKEN;
+ if (!symbol_conf.disable_add2line_warn) {
+ char *output = NULL;
+ size_t output_len;
+
+ io__getline(&io, &output, &output_len);
+ pr_warning("%s %s: addr2line configuration failed\n",
+ __func__, dso_name);
+ pr_warning("\t%c%s", ch, output);
+ }
+ return BROKEN;
}
- do {
+ while (lines) {
ch = io__get_char(&io);
- } while (ch > 0 && ch != '\n');
- if (style == GNU_BINUTILS) {
- do {
- ch = io__get_char(&io);
- } while (ch > 0 && ch != '\n');
+ if (ch <= 0)
+ break;
+ if (ch == '\n')
+ lines--;
}
/* Ignore SIGPIPE in the event addr2line exits. */
signal(SIGPIPE, SIG_IGN);
@@ -593,12 +604,9 @@ static int addr2line(const char *dso_name, u64 addr,
pr_warning("%s %s: addr2line_subprocess_init failed\n", __func__, dso_name);
goto out;
}
- a2l_style = addr2line_configure(a2l);
- if (a2l_style == BROKEN) {
- if (!symbol_conf.disable_add2line_warn)
- pr_warning("%s: addr2line configuration failed\n", __func__);
+ a2l_style = addr2line_configure(a2l, dso_name);
+ if (a2l_style == BROKEN)
goto out;
- }

/*
* Send our request and then *deliberately* send something that can't be interpreted as
--
2.41.0.162.gfafddb0af9-goog