Messages in this thread | | | From | Namhyung Kim <> | Date | Wed, 29 Mar 2023 17:15:12 -0700 | Subject | Re: [PATCH v1 6/6] perf symbol: Add command line support for addr2line path |
| |
Hi ian,
On Tue, Mar 28, 2023 at 4:56 PM Ian Rogers <irogers@google.com> wrote: > > Allow addr2line to be set either on the command line or via the > perfconfig file. This doesn't currently work with llvm-addr2line as > the addr2line code emits two things: > 1) the address to decode, > 2) a bogus ',' value. > The expectation is the bogus value will generate: > ?? > ??:0 > that terminates the addr2line reading. However, the output from > llvm-addr2line is a single line with just the input ',' locking up the > addr2line reading that is expecting a second line.
Yeah, I remember that when I tried to llvm-addr2line support. Thanks for pointing it out. Have you reported it to the LLVM community or even sent a fix? :)
Thanks, Namhyung
> > Signed-off-by: Ian Rogers <irogers@google.com> > --- > tools/perf/Documentation/perf-annotate.txt | 3 +++ > tools/perf/Documentation/perf-config.txt | 3 +++ > tools/perf/Documentation/perf-report.txt | 3 +++ > tools/perf/Documentation/perf-top.txt | 6 +++++ > tools/perf/builtin-annotate.c | 9 +++++++- > tools/perf/builtin-report.c | 9 +++++++- > tools/perf/builtin-top.c | 10 +++++++-- > tools/perf/util/annotate.c | 6 +++++ > tools/perf/util/srcline.c | 26 +++++++++++++--------- > tools/perf/util/symbol_conf.h | 1 + > 10 files changed, 62 insertions(+), 14 deletions(-) > > diff --git a/tools/perf/Documentation/perf-annotate.txt b/tools/perf/Documentation/perf-annotate.txt > index 980fe2c29275..fe168e8165c8 100644 > --- a/tools/perf/Documentation/perf-annotate.txt > +++ b/tools/perf/Documentation/perf-annotate.txt > @@ -116,6 +116,9 @@ include::itrace.txt[] > -M:: > --disassembler-style=:: Set disassembler style for objdump. > > +--addr2line=<path>:: > + Path to addr2line binary. > + > --objdump=<path>:: > Path to objdump binary. > > diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt > index 697f7f924545..e56ae54805a8 100644 > --- a/tools/perf/Documentation/perf-config.txt > +++ b/tools/perf/Documentation/perf-config.txt > @@ -250,6 +250,9 @@ annotate.*:: > These are in control of addresses, jump function, source code > in lines of assembly code from a specific program. > > + annotate.addr2line:: > + addr2line binary to use for file names and line numbers. > + > annotate.objdump:: > objdump binary to use for disassembly and annotations. > > diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt > index cfd502f7e6da..af068b4f1e5a 100644 > --- a/tools/perf/Documentation/perf-report.txt > +++ b/tools/perf/Documentation/perf-report.txt > @@ -381,6 +381,9 @@ OPTIONS > This allows to examine the path the program took to each sample. > The data collection must have used -b (or -j) and -g. > > +--addr2line=<path>:: > + Path to addr2line binary. > + > --objdump=<path>:: > Path to objdump binary. > > diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt > index c60e615b7183..619cc8143ad5 100644 > --- a/tools/perf/Documentation/perf-top.txt > +++ b/tools/perf/Documentation/perf-top.txt > @@ -161,6 +161,12 @@ Default is to monitor all CPUS. > -M:: > --disassembler-style=:: Set disassembler style for objdump. > > +--addr2line=<path>:: > + Path to addr2line binary. > + > +--objdump=<path>:: > + Path to objdump binary. > + > --prefix=PREFIX:: > --prefix-strip=N:: > Remove first N entries from source file path names in executables > diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c > index d781639b644f..ec276db3e5dd 100644 > --- a/tools/perf/builtin-annotate.c > +++ b/tools/perf/builtin-annotate.c > @@ -516,7 +516,7 @@ int cmd_annotate(int argc, const char **argv) > struct itrace_synth_opts itrace_synth_opts = { > .set = 0, > }; > - const char *disassembler_style = NULL, *objdump_path = NULL; > + const char *disassembler_style = NULL, *objdump_path = NULL, *addr2line_path = NULL; > struct option options[] = { > OPT_STRING('i', "input", &input_name, "file", > "input file name"), > @@ -569,6 +569,8 @@ int cmd_annotate(int argc, const char **argv) > "Strip first N entries of source file path name in programs (with --prefix)"), > OPT_STRING(0, "objdump", &objdump_path, "path", > "objdump binary to use for disassembly and annotations"), > + OPT_STRING(0, "addr2line", &addr2line_path, "path", > + "addr2line binary to use for line numbers"), > OPT_BOOLEAN(0, "demangle", &symbol_conf.demangle, > "Enable symbol demangling"), > OPT_BOOLEAN(0, "demangle-kernel", &symbol_conf.demangle_kernel, > @@ -628,6 +630,11 @@ int cmd_annotate(int argc, const char **argv) > if (!annotate.opts.objdump_path) > return -ENOMEM; > } > + if (addr2line_path) { > + symbol_conf.addr2line_path = strdup(addr2line_path); > + if (!symbol_conf.addr2line_path) > + return -ENOMEM; > + } > > if (annotate_check_args(&annotate.opts) < 0) > return -EINVAL; > diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c > index 15b0cf649e1a..4011abc03d0d 100644 > --- a/tools/perf/builtin-report.c > +++ b/tools/perf/builtin-report.c > @@ -1226,7 +1226,7 @@ int cmd_report(int argc, const char **argv) > }; > char *sort_order_help = sort_help("sort by key(s):"); > char *field_order_help = sort_help("output field(s): overhead period sample "); > - const char *disassembler_style = NULL, *objdump_path = NULL; > + const char *disassembler_style = NULL, *objdump_path = NULL, *addr2line_path = NULL; > const struct option options[] = { > OPT_STRING('i', "input", &input_name, "file", > "input file name"), > @@ -1344,6 +1344,8 @@ int cmd_report(int argc, const char **argv) > "add last branch records to call history"), > OPT_STRING(0, "objdump", &objdump_path, "path", > "objdump binary to use for disassembly and annotations"), > + OPT_STRING(0, "addr2line", &addr2line_path, "path", > + "addr2line binary to use for line numbers"), > OPT_BOOLEAN(0, "demangle", &symbol_conf.demangle, > "Disable symbol demangling"), > OPT_BOOLEAN(0, "demangle-kernel", &symbol_conf.demangle_kernel, > @@ -1430,6 +1432,11 @@ int cmd_report(int argc, const char **argv) > if (!report.annotation_opts.objdump_path) > return -ENOMEM; > } > + if (addr2line_path) { > + symbol_conf.addr2line_path = strdup(addr2line_path); > + if (!symbol_conf.addr2line_path) > + return -ENOMEM; > + } > > if (annotate_check_args(&report.annotation_opts) < 0) { > ret = -EINVAL; > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c > index 57a273cd03de..82c6c065830d 100644 > --- a/tools/perf/builtin-top.c > +++ b/tools/perf/builtin-top.c > @@ -1439,7 +1439,7 @@ int cmd_top(int argc, const char **argv) > }; > struct record_opts *opts = &top.record_opts; > struct target *target = &opts->target; > - const char *disassembler_style = NULL, *objdump_path = NULL; > + const char *disassembler_style = NULL, *objdump_path = NULL, *addr2line_path = NULL; > const struct option options[] = { > OPT_CALLBACK('e', "event", &top.evlist, "event", > "event selector. use 'perf list' to list available events", > @@ -1527,6 +1527,8 @@ int cmd_top(int argc, const char **argv) > OPT_BOOLEAN(0, "no-bpf-event", &top.record_opts.no_bpf_event, "do not record bpf events"), > OPT_STRING(0, "objdump", &objdump_path, "path", > "objdump binary to use for disassembly and annotations"), > + OPT_STRING(0, "addr2line", &addr2line_path, "path", > + "addr2line binary to use for line numbers"), > OPT_STRING('M', "disassembler-style", &disassembler_style, "disassembler style", > "Specify disassembler style (e.g. -M intel for intel syntax)"), > OPT_STRING(0, "prefix", &top.annotation_opts.prefix, "prefix", > @@ -1629,7 +1631,11 @@ int cmd_top(int argc, const char **argv) > if (!top.annotation_opts.objdump_path) > return -ENOMEM; > } > - > + if (addr2line_path) { > + symbol_conf.addr2line_path = strdup(addr2line_path); > + if (!symbol_conf.addr2line_path) > + return -ENOMEM; > + } > > status = symbol__validate_sym_arguments(); > if (status) > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c > index 3eaa9b2df6c4..3e5df90e11f8 100644 > --- a/tools/perf/util/annotate.c > +++ b/tools/perf/util/annotate.c > @@ -3217,6 +3217,12 @@ static int annotation__config(const char *var, const char *value, void *data) > pr_err("Not enough memory for annotate.objdump\n"); > return -1; > } > + } else if (!strcmp(var, "annotate.addr2line")) { > + symbol_conf.addr2line_path = strdup(value); > + if (!symbol_conf.addr2line_path) { > + pr_err("Not enough memory for annotate.addr2line\n"); > + return -1; > + } > } else if (!strcmp(var, "annotate.demangle")) { > symbol_conf.demangle = perf_config_bool("demangle", value); > } else if (!strcmp(var, "annotate.demangle_kernel")) { > diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c > index 33321867416b..f0a96a834e4b 100644 > --- a/tools/perf/util/srcline.c > +++ b/tools/perf/util/srcline.c > @@ -414,9 +414,14 @@ static void addr2line_subprocess_cleanup(struct a2l_subprocess *a2l) > free(a2l); > } > > -static struct a2l_subprocess *addr2line_subprocess_init(const char *path) > -{ > - const char *argv[] = { "addr2line", "-e", path, "-i", "-f", NULL }; > +static struct a2l_subprocess *addr2line_subprocess_init(const char *addr2line_path, > + const char *binary_path) > +{ > + const char *argv[] = { > + addr2line_path ?: "addr2line", > + "-e", binary_path, > + "-i", "-f", NULL > + }; > struct a2l_subprocess *a2l = zalloc(sizeof(*a2l)); > int start_command_status = 0; > > @@ -436,21 +441,22 @@ static struct a2l_subprocess *addr2line_subprocess_init(const char *path) > a2l->addr2line.argv = NULL; /* it's not used after start_command; avoid dangling pointers */ > > if (start_command_status != 0) { > - pr_warning("could not start addr2line for %s: start_command return code %d\n", > - path, > - start_command_status); > + pr_warning("could not start addr2line (%s) for %s: start_command return code %d\n", > + addr2line_path, binary_path, start_command_status); > goto out; > } > > a2l->to_child = fdopen(a2l->addr2line.in, "w"); > if (a2l->to_child == NULL) { > - pr_warning("could not open write-stream to addr2line of %s\n", path); > + pr_warning("could not open write-stream to addr2line (%s) of %s\n", > + addr2line_path, binary_path); > goto out; > } > > a2l->from_child = fdopen(a2l->addr2line.out, "r"); > if (a2l->from_child == NULL) { > - pr_warning("could not open read-stream from addr2line of %s\n", path); > + pr_warning("could not open read-stream from addr2line (%s) of %s\n", > + addr2line_path, binary_path); > goto out; > } > > @@ -490,7 +496,6 @@ static int read_addr2line_record(struct a2l_subprocess *a2l, > > if (getline(&line, &line_len, a2l->from_child) < 0 || !line_len) > goto error; > - > if (function != NULL) > *function = strdup(strim(line)); > > @@ -553,7 +558,8 @@ static int addr2line(const char *dso_name, u64 addr, > if (!filename__has_section(dso_name, ".debug_line")) > goto out; > > - dso->a2l = addr2line_subprocess_init(dso_name); > + dso->a2l = addr2line_subprocess_init(symbol_conf.addr2line_path, > + dso_name); > a2l = dso->a2l; > } > > diff --git a/tools/perf/util/symbol_conf.h b/tools/perf/util/symbol_conf.h > index bc3d046fbb63..5accd8e69ad2 100644 > --- a/tools/perf/util/symbol_conf.h > +++ b/tools/perf/util/symbol_conf.h > @@ -61,6 +61,7 @@ struct symbol_conf { > *sym_list_str, > *col_width_list_str, > *bt_stop_list_str; > + char *addr2line_path; > unsigned long time_quantum; > struct strlist *dso_list, > *comm_list, > -- > 2.40.0.348.gf938b09366-goog >
| |