* [PATCH v1] perf addr2line: Add a libdw implementation @ 2025-11-22 9:39 Ian Rogers 2025-11-24 11:21 ` James Clark 2025-11-26 18:27 ` Namhyung Kim 0 siblings, 2 replies; 13+ messages in thread From: Ian Rogers @ 2025-11-22 9:39 UTC (permalink / raw) To: Tony Jones, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter, James Clark, Howard Chu, Stephen Brennan, linux-kernel, linux-perf-users Add an implementation of addr2line that uses libdw. Other addr2line implementations or, in the case of forking addr2line, slow. Add an implementation that caches the libdw information in the dso and uses it to find the file and line number information. Signed-off-by: Ian Rogers <irogers@google.com> --- tools/perf/util/Build | 1 + tools/perf/util/dso.c | 2 + tools/perf/util/dso.h | 11 +++ tools/perf/util/libdw.c | 136 ++++++++++++++++++++++++++++++++++++++ tools/perf/util/libdw.h | 60 +++++++++++++++++ tools/perf/util/srcline.c | 5 ++ 6 files changed, 215 insertions(+) create mode 100644 tools/perf/util/libdw.c create mode 100644 tools/perf/util/libdw.h diff --git a/tools/perf/util/Build b/tools/perf/util/Build index 1c2a43e1dc68..2bed6274e248 100644 --- a/tools/perf/util/Build +++ b/tools/perf/util/Build @@ -224,6 +224,7 @@ perf-util-$(CONFIG_LIBDW) += dwarf-regs-powerpc.o perf-util-$(CONFIG_LIBDW) += dwarf-regs-x86.o perf-util-$(CONFIG_LIBDW) += debuginfo.o perf-util-$(CONFIG_LIBDW) += annotate-data.o +perf-util-$(CONFIG_LIBDW) += libdw.o perf-util-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o perf-util-$(CONFIG_LOCAL_LIBUNWIND) += unwind-libunwind-local.o diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index 344e689567ee..06980844c014 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -32,6 +32,7 @@ #include "string2.h" #include "vdso.h" #include "annotate-data.h" +#include "libdw.h" static const char * const debuglink_paths[] = { "%.0s%s", @@ -1605,6 +1606,7 @@ void dso__delete(struct dso *dso) auxtrace_cache__free(RC_CHK_ACCESS(dso)->auxtrace_cache); dso_cache__free(dso); dso__free_a2l(dso); + dso__free_a2l_libdw(dso); dso__free_symsrc_filename(dso); nsinfo__zput(RC_CHK_ACCESS(dso)->nsinfo); mutex_destroy(dso__lock(dso)); diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h index f8ccb9816b89..4aee23775054 100644 --- a/tools/perf/util/dso.h +++ b/tools/perf/util/dso.h @@ -268,6 +268,7 @@ DECLARE_RC_STRUCT(dso) { const char *short_name; const char *long_name; void *a2l; + void *a2l_libdw; char *symsrc_filename; #if defined(__powerpc__) void *dwfl; /* DWARF debug info */ @@ -334,6 +335,16 @@ static inline void dso__set_a2l(struct dso *dso, void *val) RC_CHK_ACCESS(dso)->a2l = val; } +static inline void *dso__a2l_libdw(const struct dso *dso) +{ + return RC_CHK_ACCESS(dso)->a2l_libdw; +} + +static inline void dso__set_a2l_libdw(struct dso *dso, void *val) +{ + RC_CHK_ACCESS(dso)->a2l_libdw = val; +} + static inline unsigned int dso__a2l_fails(const struct dso *dso) { return RC_CHK_ACCESS(dso)->a2l_fails; diff --git a/tools/perf/util/libdw.c b/tools/perf/util/libdw.c new file mode 100644 index 000000000000..c4331fa8e1a3 --- /dev/null +++ b/tools/perf/util/libdw.c @@ -0,0 +1,136 @@ +// SPDX-License-Identifier: GPL-2.0 +#include "dso.h" +#include "libdw.h" +#include "srcline.h" +#include "symbol.h" +#include "dwarf-aux.h" +#include <fcntl.h> +#include <unistd.h> +#include <elfutils/libdwfl.h> + +void dso__free_a2l_libdw(struct dso *dso) +{ + Dwfl *dwfl = dso__a2l_libdw(dso); + + if (dwfl) { + dwfl_end(dwfl); + dso__set_a2l_libdw(dso, NULL); + } +} + +int libdw__addr2line(const char *dso_name, u64 addr, + char **file, unsigned int *line_nr, + struct dso *dso, bool unwind_inlines, + struct inline_node *node, struct symbol *sym) +{ + static const Dwfl_Callbacks offline_callbacks = { + .find_debuginfo = dwfl_standard_find_debuginfo, + .section_address = dwfl_offline_section_address, + .find_elf = dwfl_build_id_find_elf, + }; + Dwfl *dwfl = dso__a2l_libdw(dso); + Dwfl_Module *mod; + Dwfl_Line *dwline; + Dwarf_Addr bias; + const char *src; + int lineno; + + if (!dwfl) { + /* + * Initialize Dwfl session. + * We need to open the DSO file to report it to libdw. + */ + int fd; + + fd = open(dso_name, O_RDONLY); + if (fd < 0) + return 0; + + dwfl = dwfl_begin(&offline_callbacks); + if (!dwfl) { + close(fd); + return 0; + } + + /* + * If the report is successful, the file descriptor fd is consumed + * and closed by the Dwfl. If not, it is not closed. + */ + mod = dwfl_report_offline(dwfl, dso_name, dso_name, fd); + if (!mod) { + dwfl_end(dwfl); + close(fd); + return 0; + } + + dwfl_report_end(dwfl, /*removed=*/NULL, /*arg=*/NULL); + dso__set_a2l_libdw(dso, dwfl); + } else { + /* Dwfl session already initialized, get module for address. */ + mod = dwfl_addrmodule(dwfl, addr); + } + + if (!mod) + return 0; + + /* Find source line information for the address. */ + dwline = dwfl_module_getsrc(mod, addr); + if (!dwline) + return 0; + + /* Get line information. */ + src = dwfl_lineinfo(dwline, &addr, &lineno, /*col=*/NULL, /*mtime=*/NULL, /*length=*/NULL); + + if (file) + *file = src ? strdup(src) : NULL; + if (line_nr) + *line_nr = lineno; + + /* Optionally unwind inline function call chain. */ + if (unwind_inlines && node && src) { + Dwarf_Die *cudie = dwfl_module_addrdie(mod, addr, &bias); + Dwarf_Die *scopes = NULL; + int nscopes; + + if (!cudie) + return 1; + + nscopes = die_get_scopes(cudie, addr, &scopes); + if (nscopes > 0) { + int i; + const char *call_file = src; + unsigned int call_line = lineno; + + for (i = 0; i < nscopes; i++) { + Dwarf_Die *die = &scopes[i]; + struct symbol *inline_sym; + char *srcline = NULL; + int tag = dwarf_tag(die); + + /* We are interested in inlined subroutines. */ + if (tag != DW_TAG_inlined_subroutine && + tag != DW_TAG_subprogram) + continue; + + inline_sym = new_inline_sym(dso, sym, dwarf_diename(die)); + + if (call_file) + srcline = srcline_from_fileline(call_file, call_line); + + inline_list__append(inline_sym, srcline, node); + + /* Update call site for next level. */ + if (tag == DW_TAG_inlined_subroutine) { + call_file = die_get_call_file(die); + call_line = die_get_call_lineno(die); + } else { + /* Reached the root subprogram. */ + break; + } + } + free(scopes); + } + } + + return 1; +} diff --git a/tools/perf/util/libdw.h b/tools/perf/util/libdw.h new file mode 100644 index 000000000000..0f8d7b4a11a5 --- /dev/null +++ b/tools/perf/util/libdw.h @@ -0,0 +1,60 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef PERF_LIBDW_H +#define PERF_LIBDW_H + +#include <linux/types.h> + +struct dso; +struct inline_node; +struct symbol; + +#ifdef HAVE_LIBDW_SUPPORT +/* + * libdw__addr2line - Convert address to source location using libdw + * @dso_name: Name of the DSO + * @addr: Address to resolve + * @file: Pointer to return filename (caller must free) + * @line_nr: Pointer to return line number + * @dso: The dso struct + * @unwind_inlines: Whether to unwind inline function calls + * @node: Inline node list to append to + * @sym: The symbol associated with the address + * + * This function initializes a Dwfl context for the DSO if not already present, + * finds the source line information for the given address, and optionally + * resolves inline function call chains. + * + * Returns 1 on success (found), 0 on failure (not found). + */ +int libdw__addr2line(const char *dso_name, u64 addr, char **file, + unsigned int *line_nr, struct dso *dso, + bool unwind_inlines, struct inline_node *node, + struct symbol *sym); + +/* + * dso__free_a2l_libdw - Free libdw resources associated with the DSO + * @dso: The dso to free resources for + * + * This function cleans up the Dwfl context used for addr2line lookups. + */ +void dso__free_a2l_libdw(struct dso *dso); + +#else /* HAVE_LIBDW_SUPPORT */ + +static inline int libdw__addr2line(const char *dso_name __maybe_unused, + u64 addr __maybe_unused, char **file __maybe_unused, + unsigned int *line_nr __maybe_unused, + struct dso *dso __maybe_unused, + bool unwind_inlines __maybe_unused, + struct inline_node *node __maybe_unused, + struct symbol *sym __maybe_unused) +{ + return 0; +} + +static inline void dso__free_a2l_libdw(struct dso *dso __maybe_unused) +{ +} +#endif /* HAVE_LIBDW_SUPPORT */ + +#endif /* PERF_LIBDW_H */ diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c index 27c0966611ab..4b456c4d4138 100644 --- a/tools/perf/util/srcline.c +++ b/tools/perf/util/srcline.c @@ -6,6 +6,7 @@ #include "libbfd.h" #include "llvm.h" #include "symbol.h" +#include "libdw.h" #include <inttypes.h> #include <string.h> @@ -120,6 +121,10 @@ static int addr2line(const char *dso_name, u64 addr, char **file, unsigned int * { int ret; + ret = libdw__addr2line(dso_name, addr, file, line_nr, dso, unwind_inlines, node, sym); + if (ret > 0) + return ret; + ret = llvm__addr2line(dso_name, addr, file, line_nr, dso, unwind_inlines, node, sym); if (ret > 0) return ret; -- 2.52.0.rc2.455.g230fcf2819-goog ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v1] perf addr2line: Add a libdw implementation 2025-11-22 9:39 [PATCH v1] perf addr2line: Add a libdw implementation Ian Rogers @ 2025-11-24 11:21 ` James Clark 2025-11-26 18:27 ` Namhyung Kim 1 sibling, 0 replies; 13+ messages in thread From: James Clark @ 2025-11-24 11:21 UTC (permalink / raw) To: Ian Rogers Cc: Tony Jones, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Alexander Shishkin, Jiri Olsa, Adrian Hunter, Howard Chu, Stephen Brennan, linux-kernel, linux-perf-users On 22/11/2025 9:39 am, Ian Rogers wrote: > Add an implementation of addr2line that uses libdw. Other addr2line > implementations or, in the case of forking addr2line, slow. Add an "are slow"? Other that that: Reviewed-by: James Clark <james.clark@linaro.org> > implementation that caches the libdw information in the dso and uses > it to find the file and line number information. > > Signed-off-by: Ian Rogers <irogers@google.com> > --- > tools/perf/util/Build | 1 + > tools/perf/util/dso.c | 2 + > tools/perf/util/dso.h | 11 +++ > tools/perf/util/libdw.c | 136 ++++++++++++++++++++++++++++++++++++++ > tools/perf/util/libdw.h | 60 +++++++++++++++++ > tools/perf/util/srcline.c | 5 ++ > 6 files changed, 215 insertions(+) > create mode 100644 tools/perf/util/libdw.c > create mode 100644 tools/perf/util/libdw.h > > diff --git a/tools/perf/util/Build b/tools/perf/util/Build > index 1c2a43e1dc68..2bed6274e248 100644 > --- a/tools/perf/util/Build > +++ b/tools/perf/util/Build > @@ -224,6 +224,7 @@ perf-util-$(CONFIG_LIBDW) += dwarf-regs-powerpc.o > perf-util-$(CONFIG_LIBDW) += dwarf-regs-x86.o > perf-util-$(CONFIG_LIBDW) += debuginfo.o > perf-util-$(CONFIG_LIBDW) += annotate-data.o > +perf-util-$(CONFIG_LIBDW) += libdw.o > > perf-util-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o > perf-util-$(CONFIG_LOCAL_LIBUNWIND) += unwind-libunwind-local.o > diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c > index 344e689567ee..06980844c014 100644 > --- a/tools/perf/util/dso.c > +++ b/tools/perf/util/dso.c > @@ -32,6 +32,7 @@ > #include "string2.h" > #include "vdso.h" > #include "annotate-data.h" > +#include "libdw.h" > > static const char * const debuglink_paths[] = { > "%.0s%s", > @@ -1605,6 +1606,7 @@ void dso__delete(struct dso *dso) > auxtrace_cache__free(RC_CHK_ACCESS(dso)->auxtrace_cache); > dso_cache__free(dso); > dso__free_a2l(dso); > + dso__free_a2l_libdw(dso); > dso__free_symsrc_filename(dso); > nsinfo__zput(RC_CHK_ACCESS(dso)->nsinfo); > mutex_destroy(dso__lock(dso)); > diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h > index f8ccb9816b89..4aee23775054 100644 > --- a/tools/perf/util/dso.h > +++ b/tools/perf/util/dso.h > @@ -268,6 +268,7 @@ DECLARE_RC_STRUCT(dso) { > const char *short_name; > const char *long_name; > void *a2l; > + void *a2l_libdw; > char *symsrc_filename; > #if defined(__powerpc__) > void *dwfl; /* DWARF debug info */ > @@ -334,6 +335,16 @@ static inline void dso__set_a2l(struct dso *dso, void *val) > RC_CHK_ACCESS(dso)->a2l = val; > } > > +static inline void *dso__a2l_libdw(const struct dso *dso) > +{ > + return RC_CHK_ACCESS(dso)->a2l_libdw; > +} > + > +static inline void dso__set_a2l_libdw(struct dso *dso, void *val) > +{ > + RC_CHK_ACCESS(dso)->a2l_libdw = val; > +} > + > static inline unsigned int dso__a2l_fails(const struct dso *dso) > { > return RC_CHK_ACCESS(dso)->a2l_fails; > diff --git a/tools/perf/util/libdw.c b/tools/perf/util/libdw.c > new file mode 100644 > index 000000000000..c4331fa8e1a3 > --- /dev/null > +++ b/tools/perf/util/libdw.c > @@ -0,0 +1,136 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#include "dso.h" > +#include "libdw.h" > +#include "srcline.h" > +#include "symbol.h" > +#include "dwarf-aux.h" > +#include <fcntl.h> > +#include <unistd.h> > +#include <elfutils/libdwfl.h> > + > +void dso__free_a2l_libdw(struct dso *dso) > +{ > + Dwfl *dwfl = dso__a2l_libdw(dso); > + > + if (dwfl) { > + dwfl_end(dwfl); > + dso__set_a2l_libdw(dso, NULL); > + } > +} > + > +int libdw__addr2line(const char *dso_name, u64 addr, > + char **file, unsigned int *line_nr, > + struct dso *dso, bool unwind_inlines, > + struct inline_node *node, struct symbol *sym) > +{ > + static const Dwfl_Callbacks offline_callbacks = { > + .find_debuginfo = dwfl_standard_find_debuginfo, > + .section_address = dwfl_offline_section_address, > + .find_elf = dwfl_build_id_find_elf, > + }; > + Dwfl *dwfl = dso__a2l_libdw(dso); > + Dwfl_Module *mod; > + Dwfl_Line *dwline; > + Dwarf_Addr bias; > + const char *src; > + int lineno; > + > + if (!dwfl) { > + /* > + * Initialize Dwfl session. > + * We need to open the DSO file to report it to libdw. > + */ > + int fd; > + > + fd = open(dso_name, O_RDONLY); > + if (fd < 0) > + return 0; > + > + dwfl = dwfl_begin(&offline_callbacks); > + if (!dwfl) { > + close(fd); > + return 0; > + } > + > + /* > + * If the report is successful, the file descriptor fd is consumed > + * and closed by the Dwfl. If not, it is not closed. > + */ > + mod = dwfl_report_offline(dwfl, dso_name, dso_name, fd); > + if (!mod) { > + dwfl_end(dwfl); > + close(fd); > + return 0; > + } > + > + dwfl_report_end(dwfl, /*removed=*/NULL, /*arg=*/NULL); > + dso__set_a2l_libdw(dso, dwfl); > + } else { > + /* Dwfl session already initialized, get module for address. */ > + mod = dwfl_addrmodule(dwfl, addr); > + } > + > + if (!mod) > + return 0; > + > + /* Find source line information for the address. */ > + dwline = dwfl_module_getsrc(mod, addr); > + if (!dwline) > + return 0; > + > + /* Get line information. */ > + src = dwfl_lineinfo(dwline, &addr, &lineno, /*col=*/NULL, /*mtime=*/NULL, /*length=*/NULL); > + > + if (file) > + *file = src ? strdup(src) : NULL; > + if (line_nr) > + *line_nr = lineno; > + > + /* Optionally unwind inline function call chain. */ > + if (unwind_inlines && node && src) { > + Dwarf_Die *cudie = dwfl_module_addrdie(mod, addr, &bias); > + Dwarf_Die *scopes = NULL; > + int nscopes; > + > + if (!cudie) > + return 1; > + > + nscopes = die_get_scopes(cudie, addr, &scopes); > + if (nscopes > 0) { > + int i; > + const char *call_file = src; > + unsigned int call_line = lineno; > + > + for (i = 0; i < nscopes; i++) { > + Dwarf_Die *die = &scopes[i]; > + struct symbol *inline_sym; > + char *srcline = NULL; > + int tag = dwarf_tag(die); > + > + /* We are interested in inlined subroutines. */ > + if (tag != DW_TAG_inlined_subroutine && > + tag != DW_TAG_subprogram) > + continue; > + > + inline_sym = new_inline_sym(dso, sym, dwarf_diename(die)); > + > + if (call_file) > + srcline = srcline_from_fileline(call_file, call_line); > + > + inline_list__append(inline_sym, srcline, node); > + > + /* Update call site for next level. */ > + if (tag == DW_TAG_inlined_subroutine) { > + call_file = die_get_call_file(die); > + call_line = die_get_call_lineno(die); > + } else { > + /* Reached the root subprogram. */ > + break; > + } > + } > + free(scopes); > + } > + } > + > + return 1; > +} > diff --git a/tools/perf/util/libdw.h b/tools/perf/util/libdw.h > new file mode 100644 > index 000000000000..0f8d7b4a11a5 > --- /dev/null > +++ b/tools/perf/util/libdw.h > @@ -0,0 +1,60 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef PERF_LIBDW_H > +#define PERF_LIBDW_H > + > +#include <linux/types.h> > + > +struct dso; > +struct inline_node; > +struct symbol; > + > +#ifdef HAVE_LIBDW_SUPPORT > +/* > + * libdw__addr2line - Convert address to source location using libdw > + * @dso_name: Name of the DSO > + * @addr: Address to resolve > + * @file: Pointer to return filename (caller must free) > + * @line_nr: Pointer to return line number > + * @dso: The dso struct > + * @unwind_inlines: Whether to unwind inline function calls > + * @node: Inline node list to append to > + * @sym: The symbol associated with the address > + * > + * This function initializes a Dwfl context for the DSO if not already present, > + * finds the source line information for the given address, and optionally > + * resolves inline function call chains. > + * > + * Returns 1 on success (found), 0 on failure (not found). > + */ > +int libdw__addr2line(const char *dso_name, u64 addr, char **file, > + unsigned int *line_nr, struct dso *dso, > + bool unwind_inlines, struct inline_node *node, > + struct symbol *sym); > + > +/* > + * dso__free_a2l_libdw - Free libdw resources associated with the DSO > + * @dso: The dso to free resources for > + * > + * This function cleans up the Dwfl context used for addr2line lookups. > + */ > +void dso__free_a2l_libdw(struct dso *dso); > + > +#else /* HAVE_LIBDW_SUPPORT */ > + > +static inline int libdw__addr2line(const char *dso_name __maybe_unused, > + u64 addr __maybe_unused, char **file __maybe_unused, > + unsigned int *line_nr __maybe_unused, > + struct dso *dso __maybe_unused, > + bool unwind_inlines __maybe_unused, > + struct inline_node *node __maybe_unused, > + struct symbol *sym __maybe_unused) > +{ > + return 0; > +} > + > +static inline void dso__free_a2l_libdw(struct dso *dso __maybe_unused) > +{ > +} > +#endif /* HAVE_LIBDW_SUPPORT */ > + > +#endif /* PERF_LIBDW_H */ > diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c > index 27c0966611ab..4b456c4d4138 100644 > --- a/tools/perf/util/srcline.c > +++ b/tools/perf/util/srcline.c > @@ -6,6 +6,7 @@ > #include "libbfd.h" > #include "llvm.h" > #include "symbol.h" > +#include "libdw.h" > > #include <inttypes.h> > #include <string.h> > @@ -120,6 +121,10 @@ static int addr2line(const char *dso_name, u64 addr, char **file, unsigned int * > { > int ret; > > + ret = libdw__addr2line(dso_name, addr, file, line_nr, dso, unwind_inlines, node, sym); > + if (ret > 0) > + return ret; > + > ret = llvm__addr2line(dso_name, addr, file, line_nr, dso, unwind_inlines, node, sym); > if (ret > 0) > return ret; ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1] perf addr2line: Add a libdw implementation 2025-11-22 9:39 [PATCH v1] perf addr2line: Add a libdw implementation Ian Rogers 2025-11-24 11:21 ` James Clark @ 2025-11-26 18:27 ` Namhyung Kim 2025-11-27 11:43 ` Ian Rogers 1 sibling, 1 reply; 13+ messages in thread From: Namhyung Kim @ 2025-11-26 18:27 UTC (permalink / raw) To: Ian Rogers Cc: Tony Jones, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa, Adrian Hunter, James Clark, Howard Chu, Stephen Brennan, linux-kernel, linux-perf-users On Sat, Nov 22, 2025 at 01:39:34AM -0800, Ian Rogers wrote: > Add an implementation of addr2line that uses libdw. Other addr2line > implementations or, in the case of forking addr2line, slow. Add an > implementation that caches the libdw information in the dso and uses > it to find the file and line number information. My concern is the limit in the open file descriptors in case the data touched a lot of different libraries. The DSO code has some logic to deal with it but I'm not sure if we can share that since libdw seems to want to own the FD. Also, have you checked if this generates the exactly same output with other implementations? Thanks, Namhyung > > Signed-off-by: Ian Rogers <irogers@google.com> > --- > tools/perf/util/Build | 1 + > tools/perf/util/dso.c | 2 + > tools/perf/util/dso.h | 11 +++ > tools/perf/util/libdw.c | 136 ++++++++++++++++++++++++++++++++++++++ > tools/perf/util/libdw.h | 60 +++++++++++++++++ > tools/perf/util/srcline.c | 5 ++ > 6 files changed, 215 insertions(+) > create mode 100644 tools/perf/util/libdw.c > create mode 100644 tools/perf/util/libdw.h > > diff --git a/tools/perf/util/Build b/tools/perf/util/Build > index 1c2a43e1dc68..2bed6274e248 100644 > --- a/tools/perf/util/Build > +++ b/tools/perf/util/Build > @@ -224,6 +224,7 @@ perf-util-$(CONFIG_LIBDW) += dwarf-regs-powerpc.o > perf-util-$(CONFIG_LIBDW) += dwarf-regs-x86.o > perf-util-$(CONFIG_LIBDW) += debuginfo.o > perf-util-$(CONFIG_LIBDW) += annotate-data.o > +perf-util-$(CONFIG_LIBDW) += libdw.o > > perf-util-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o > perf-util-$(CONFIG_LOCAL_LIBUNWIND) += unwind-libunwind-local.o > diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c > index 344e689567ee..06980844c014 100644 > --- a/tools/perf/util/dso.c > +++ b/tools/perf/util/dso.c > @@ -32,6 +32,7 @@ > #include "string2.h" > #include "vdso.h" > #include "annotate-data.h" > +#include "libdw.h" > > static const char * const debuglink_paths[] = { > "%.0s%s", > @@ -1605,6 +1606,7 @@ void dso__delete(struct dso *dso) > auxtrace_cache__free(RC_CHK_ACCESS(dso)->auxtrace_cache); > dso_cache__free(dso); > dso__free_a2l(dso); > + dso__free_a2l_libdw(dso); > dso__free_symsrc_filename(dso); > nsinfo__zput(RC_CHK_ACCESS(dso)->nsinfo); > mutex_destroy(dso__lock(dso)); > diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h > index f8ccb9816b89..4aee23775054 100644 > --- a/tools/perf/util/dso.h > +++ b/tools/perf/util/dso.h > @@ -268,6 +268,7 @@ DECLARE_RC_STRUCT(dso) { > const char *short_name; > const char *long_name; > void *a2l; > + void *a2l_libdw; > char *symsrc_filename; > #if defined(__powerpc__) > void *dwfl; /* DWARF debug info */ > @@ -334,6 +335,16 @@ static inline void dso__set_a2l(struct dso *dso, void *val) > RC_CHK_ACCESS(dso)->a2l = val; > } > > +static inline void *dso__a2l_libdw(const struct dso *dso) > +{ > + return RC_CHK_ACCESS(dso)->a2l_libdw; > +} > + > +static inline void dso__set_a2l_libdw(struct dso *dso, void *val) > +{ > + RC_CHK_ACCESS(dso)->a2l_libdw = val; > +} > + > static inline unsigned int dso__a2l_fails(const struct dso *dso) > { > return RC_CHK_ACCESS(dso)->a2l_fails; > diff --git a/tools/perf/util/libdw.c b/tools/perf/util/libdw.c > new file mode 100644 > index 000000000000..c4331fa8e1a3 > --- /dev/null > +++ b/tools/perf/util/libdw.c > @@ -0,0 +1,136 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#include "dso.h" > +#include "libdw.h" > +#include "srcline.h" > +#include "symbol.h" > +#include "dwarf-aux.h" > +#include <fcntl.h> > +#include <unistd.h> > +#include <elfutils/libdwfl.h> > + > +void dso__free_a2l_libdw(struct dso *dso) > +{ > + Dwfl *dwfl = dso__a2l_libdw(dso); > + > + if (dwfl) { > + dwfl_end(dwfl); > + dso__set_a2l_libdw(dso, NULL); > + } > +} > + > +int libdw__addr2line(const char *dso_name, u64 addr, > + char **file, unsigned int *line_nr, > + struct dso *dso, bool unwind_inlines, > + struct inline_node *node, struct symbol *sym) > +{ > + static const Dwfl_Callbacks offline_callbacks = { > + .find_debuginfo = dwfl_standard_find_debuginfo, > + .section_address = dwfl_offline_section_address, > + .find_elf = dwfl_build_id_find_elf, > + }; > + Dwfl *dwfl = dso__a2l_libdw(dso); > + Dwfl_Module *mod; > + Dwfl_Line *dwline; > + Dwarf_Addr bias; > + const char *src; > + int lineno; > + > + if (!dwfl) { > + /* > + * Initialize Dwfl session. > + * We need to open the DSO file to report it to libdw. > + */ > + int fd; > + > + fd = open(dso_name, O_RDONLY); > + if (fd < 0) > + return 0; > + > + dwfl = dwfl_begin(&offline_callbacks); > + if (!dwfl) { > + close(fd); > + return 0; > + } > + > + /* > + * If the report is successful, the file descriptor fd is consumed > + * and closed by the Dwfl. If not, it is not closed. > + */ > + mod = dwfl_report_offline(dwfl, dso_name, dso_name, fd); > + if (!mod) { > + dwfl_end(dwfl); > + close(fd); > + return 0; > + } > + > + dwfl_report_end(dwfl, /*removed=*/NULL, /*arg=*/NULL); > + dso__set_a2l_libdw(dso, dwfl); > + } else { > + /* Dwfl session already initialized, get module for address. */ > + mod = dwfl_addrmodule(dwfl, addr); > + } > + > + if (!mod) > + return 0; > + > + /* Find source line information for the address. */ > + dwline = dwfl_module_getsrc(mod, addr); > + if (!dwline) > + return 0; > + > + /* Get line information. */ > + src = dwfl_lineinfo(dwline, &addr, &lineno, /*col=*/NULL, /*mtime=*/NULL, /*length=*/NULL); > + > + if (file) > + *file = src ? strdup(src) : NULL; > + if (line_nr) > + *line_nr = lineno; > + > + /* Optionally unwind inline function call chain. */ > + if (unwind_inlines && node && src) { > + Dwarf_Die *cudie = dwfl_module_addrdie(mod, addr, &bias); > + Dwarf_Die *scopes = NULL; > + int nscopes; > + > + if (!cudie) > + return 1; > + > + nscopes = die_get_scopes(cudie, addr, &scopes); > + if (nscopes > 0) { > + int i; > + const char *call_file = src; > + unsigned int call_line = lineno; > + > + for (i = 0; i < nscopes; i++) { > + Dwarf_Die *die = &scopes[i]; > + struct symbol *inline_sym; > + char *srcline = NULL; > + int tag = dwarf_tag(die); > + > + /* We are interested in inlined subroutines. */ > + if (tag != DW_TAG_inlined_subroutine && > + tag != DW_TAG_subprogram) > + continue; > + > + inline_sym = new_inline_sym(dso, sym, dwarf_diename(die)); > + > + if (call_file) > + srcline = srcline_from_fileline(call_file, call_line); > + > + inline_list__append(inline_sym, srcline, node); > + > + /* Update call site for next level. */ > + if (tag == DW_TAG_inlined_subroutine) { > + call_file = die_get_call_file(die); > + call_line = die_get_call_lineno(die); > + } else { > + /* Reached the root subprogram. */ > + break; > + } > + } > + free(scopes); > + } > + } > + > + return 1; > +} > diff --git a/tools/perf/util/libdw.h b/tools/perf/util/libdw.h > new file mode 100644 > index 000000000000..0f8d7b4a11a5 > --- /dev/null > +++ b/tools/perf/util/libdw.h > @@ -0,0 +1,60 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef PERF_LIBDW_H > +#define PERF_LIBDW_H > + > +#include <linux/types.h> > + > +struct dso; > +struct inline_node; > +struct symbol; > + > +#ifdef HAVE_LIBDW_SUPPORT > +/* > + * libdw__addr2line - Convert address to source location using libdw > + * @dso_name: Name of the DSO > + * @addr: Address to resolve > + * @file: Pointer to return filename (caller must free) > + * @line_nr: Pointer to return line number > + * @dso: The dso struct > + * @unwind_inlines: Whether to unwind inline function calls > + * @node: Inline node list to append to > + * @sym: The symbol associated with the address > + * > + * This function initializes a Dwfl context for the DSO if not already present, > + * finds the source line information for the given address, and optionally > + * resolves inline function call chains. > + * > + * Returns 1 on success (found), 0 on failure (not found). > + */ > +int libdw__addr2line(const char *dso_name, u64 addr, char **file, > + unsigned int *line_nr, struct dso *dso, > + bool unwind_inlines, struct inline_node *node, > + struct symbol *sym); > + > +/* > + * dso__free_a2l_libdw - Free libdw resources associated with the DSO > + * @dso: The dso to free resources for > + * > + * This function cleans up the Dwfl context used for addr2line lookups. > + */ > +void dso__free_a2l_libdw(struct dso *dso); > + > +#else /* HAVE_LIBDW_SUPPORT */ > + > +static inline int libdw__addr2line(const char *dso_name __maybe_unused, > + u64 addr __maybe_unused, char **file __maybe_unused, > + unsigned int *line_nr __maybe_unused, > + struct dso *dso __maybe_unused, > + bool unwind_inlines __maybe_unused, > + struct inline_node *node __maybe_unused, > + struct symbol *sym __maybe_unused) > +{ > + return 0; > +} > + > +static inline void dso__free_a2l_libdw(struct dso *dso __maybe_unused) > +{ > +} > +#endif /* HAVE_LIBDW_SUPPORT */ > + > +#endif /* PERF_LIBDW_H */ > diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c > index 27c0966611ab..4b456c4d4138 100644 > --- a/tools/perf/util/srcline.c > +++ b/tools/perf/util/srcline.c > @@ -6,6 +6,7 @@ > #include "libbfd.h" > #include "llvm.h" > #include "symbol.h" > +#include "libdw.h" > > #include <inttypes.h> > #include <string.h> > @@ -120,6 +121,10 @@ static int addr2line(const char *dso_name, u64 addr, char **file, unsigned int * > { > int ret; > > + ret = libdw__addr2line(dso_name, addr, file, line_nr, dso, unwind_inlines, node, sym); > + if (ret > 0) > + return ret; > + > ret = llvm__addr2line(dso_name, addr, file, line_nr, dso, unwind_inlines, node, sym); > if (ret > 0) > return ret; > -- > 2.52.0.rc2.455.g230fcf2819-goog > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1] perf addr2line: Add a libdw implementation 2025-11-26 18:27 ` Namhyung Kim @ 2025-11-27 11:43 ` Ian Rogers 2025-11-27 12:16 ` James Clark 2025-11-27 20:53 ` Namhyung Kim 0 siblings, 2 replies; 13+ messages in thread From: Ian Rogers @ 2025-11-27 11:43 UTC (permalink / raw) To: Namhyung Kim Cc: Tony Jones, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa, Adrian Hunter, James Clark, Howard Chu, Stephen Brennan, linux-kernel, linux-perf-users On Wed, Nov 26, 2025 at 10:27 AM Namhyung Kim <namhyung@kernel.org> wrote: > > On Sat, Nov 22, 2025 at 01:39:34AM -0800, Ian Rogers wrote: > > Add an implementation of addr2line that uses libdw. Other addr2line > > implementations or, in the case of forking addr2line, slow. Add an > > implementation that caches the libdw information in the dso and uses > > it to find the file and line number information. Thanks James and Namhyung for the reviews! I agree with James' comment about a typo in the commit message. > My concern is the limit in the open file descriptors in case the data > touched a lot of different libraries. The DSO code has some logic to > deal with it but I'm not sure if we can share that since libdw seems to > want to own the FD. The code opens the FD: + fd = open(dso_name, O_RDONLY); + if (fd < 0) + return 0; + + dwfl = dwfl_begin(&offline_callbacks); + if (!dwfl) { + close(fd); + return 0; + } It then uses the FD and closes it (the close is hidden in libdw itself): + /* + * If the report is successful, the file descriptor fd is consumed + * and closed by the Dwfl. If not, it is not closed. + */ + mod = dwfl_report_offline(dwfl, dso_name, dso_name, fd); So it is possible we exhaust all the file descriptors if there are multiple concurrent calls to libdw__addr2line and every dso has missing libdw dwfl data... but because the open/close are close together and that 1 FD is say small to the FDs needed for the cmd__addr2line, I don't think it is a problem we need to specifically handle. Were the FD kept open until the dso was deleted, I'd agree with you. > Also, have you checked if this generates the exactly same output with > other implementations? So the code passes `perf test` and I was checking functionality with perf annotate, top, etc. What I saw looked good, but it may not have been exhaustive. I didn't specifically create a test that compares the output of the different addr2line implementations. Such a test would be possible, it's not something we've done elsewhere. Thanks, Ian > Thanks, > Namhyung > > > > > Signed-off-by: Ian Rogers <irogers@google.com> > > --- > > tools/perf/util/Build | 1 + > > tools/perf/util/dso.c | 2 + > > tools/perf/util/dso.h | 11 +++ > > tools/perf/util/libdw.c | 136 ++++++++++++++++++++++++++++++++++++++ > > tools/perf/util/libdw.h | 60 +++++++++++++++++ > > tools/perf/util/srcline.c | 5 ++ > > 6 files changed, 215 insertions(+) > > create mode 100644 tools/perf/util/libdw.c > > create mode 100644 tools/perf/util/libdw.h > > > > diff --git a/tools/perf/util/Build b/tools/perf/util/Build > > index 1c2a43e1dc68..2bed6274e248 100644 > > --- a/tools/perf/util/Build > > +++ b/tools/perf/util/Build > > @@ -224,6 +224,7 @@ perf-util-$(CONFIG_LIBDW) += dwarf-regs-powerpc.o > > perf-util-$(CONFIG_LIBDW) += dwarf-regs-x86.o > > perf-util-$(CONFIG_LIBDW) += debuginfo.o > > perf-util-$(CONFIG_LIBDW) += annotate-data.o > > +perf-util-$(CONFIG_LIBDW) += libdw.o > > > > perf-util-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o > > perf-util-$(CONFIG_LOCAL_LIBUNWIND) += unwind-libunwind-local.o > > diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c > > index 344e689567ee..06980844c014 100644 > > --- a/tools/perf/util/dso.c > > +++ b/tools/perf/util/dso.c > > @@ -32,6 +32,7 @@ > > #include "string2.h" > > #include "vdso.h" > > #include "annotate-data.h" > > +#include "libdw.h" > > > > static const char * const debuglink_paths[] = { > > "%.0s%s", > > @@ -1605,6 +1606,7 @@ void dso__delete(struct dso *dso) > > auxtrace_cache__free(RC_CHK_ACCESS(dso)->auxtrace_cache); > > dso_cache__free(dso); > > dso__free_a2l(dso); > > + dso__free_a2l_libdw(dso); > > dso__free_symsrc_filename(dso); > > nsinfo__zput(RC_CHK_ACCESS(dso)->nsinfo); > > mutex_destroy(dso__lock(dso)); > > diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h > > index f8ccb9816b89..4aee23775054 100644 > > --- a/tools/perf/util/dso.h > > +++ b/tools/perf/util/dso.h > > @@ -268,6 +268,7 @@ DECLARE_RC_STRUCT(dso) { > > const char *short_name; > > const char *long_name; > > void *a2l; > > + void *a2l_libdw; > > char *symsrc_filename; > > #if defined(__powerpc__) > > void *dwfl; /* DWARF debug info */ > > @@ -334,6 +335,16 @@ static inline void dso__set_a2l(struct dso *dso, void *val) > > RC_CHK_ACCESS(dso)->a2l = val; > > } > > > > +static inline void *dso__a2l_libdw(const struct dso *dso) > > +{ > > + return RC_CHK_ACCESS(dso)->a2l_libdw; > > +} > > + > > +static inline void dso__set_a2l_libdw(struct dso *dso, void *val) > > +{ > > + RC_CHK_ACCESS(dso)->a2l_libdw = val; > > +} > > + > > static inline unsigned int dso__a2l_fails(const struct dso *dso) > > { > > return RC_CHK_ACCESS(dso)->a2l_fails; > > diff --git a/tools/perf/util/libdw.c b/tools/perf/util/libdw.c > > new file mode 100644 > > index 000000000000..c4331fa8e1a3 > > --- /dev/null > > +++ b/tools/perf/util/libdw.c > > @@ -0,0 +1,136 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +#include "dso.h" > > +#include "libdw.h" > > +#include "srcline.h" > > +#include "symbol.h" > > +#include "dwarf-aux.h" > > +#include <fcntl.h> > > +#include <unistd.h> > > +#include <elfutils/libdwfl.h> > > + > > +void dso__free_a2l_libdw(struct dso *dso) > > +{ > > + Dwfl *dwfl = dso__a2l_libdw(dso); > > + > > + if (dwfl) { > > + dwfl_end(dwfl); > > + dso__set_a2l_libdw(dso, NULL); > > + } > > +} > > + > > +int libdw__addr2line(const char *dso_name, u64 addr, > > + char **file, unsigned int *line_nr, > > + struct dso *dso, bool unwind_inlines, > > + struct inline_node *node, struct symbol *sym) > > +{ > > + static const Dwfl_Callbacks offline_callbacks = { > > + .find_debuginfo = dwfl_standard_find_debuginfo, > > + .section_address = dwfl_offline_section_address, > > + .find_elf = dwfl_build_id_find_elf, > > + }; > > + Dwfl *dwfl = dso__a2l_libdw(dso); > > + Dwfl_Module *mod; > > + Dwfl_Line *dwline; > > + Dwarf_Addr bias; > > + const char *src; > > + int lineno; > > + > > + if (!dwfl) { > > + /* > > + * Initialize Dwfl session. > > + * We need to open the DSO file to report it to libdw. > > + */ > > + int fd; > > + > > + fd = open(dso_name, O_RDONLY); > > + if (fd < 0) > > + return 0; > > + > > + dwfl = dwfl_begin(&offline_callbacks); > > + if (!dwfl) { > > + close(fd); > > + return 0; > > + } > > + > > + /* > > + * If the report is successful, the file descriptor fd is consumed > > + * and closed by the Dwfl. If not, it is not closed. > > + */ > > + mod = dwfl_report_offline(dwfl, dso_name, dso_name, fd); > > + if (!mod) { > > + dwfl_end(dwfl); > > + close(fd); > > + return 0; > > + } > > + > > + dwfl_report_end(dwfl, /*removed=*/NULL, /*arg=*/NULL); > > + dso__set_a2l_libdw(dso, dwfl); > > + } else { > > + /* Dwfl session already initialized, get module for address. */ > > + mod = dwfl_addrmodule(dwfl, addr); > > + } > > + > > + if (!mod) > > + return 0; > > + > > + /* Find source line information for the address. */ > > + dwline = dwfl_module_getsrc(mod, addr); > > + if (!dwline) > > + return 0; > > + > > + /* Get line information. */ > > + src = dwfl_lineinfo(dwline, &addr, &lineno, /*col=*/NULL, /*mtime=*/NULL, /*length=*/NULL); > > + > > + if (file) > > + *file = src ? strdup(src) : NULL; > > + if (line_nr) > > + *line_nr = lineno; > > + > > + /* Optionally unwind inline function call chain. */ > > + if (unwind_inlines && node && src) { > > + Dwarf_Die *cudie = dwfl_module_addrdie(mod, addr, &bias); > > + Dwarf_Die *scopes = NULL; > > + int nscopes; > > + > > + if (!cudie) > > + return 1; > > + > > + nscopes = die_get_scopes(cudie, addr, &scopes); > > + if (nscopes > 0) { > > + int i; > > + const char *call_file = src; > > + unsigned int call_line = lineno; > > + > > + for (i = 0; i < nscopes; i++) { > > + Dwarf_Die *die = &scopes[i]; > > + struct symbol *inline_sym; > > + char *srcline = NULL; > > + int tag = dwarf_tag(die); > > + > > + /* We are interested in inlined subroutines. */ > > + if (tag != DW_TAG_inlined_subroutine && > > + tag != DW_TAG_subprogram) > > + continue; > > + > > + inline_sym = new_inline_sym(dso, sym, dwarf_diename(die)); > > + > > + if (call_file) > > + srcline = srcline_from_fileline(call_file, call_line); > > + > > + inline_list__append(inline_sym, srcline, node); > > + > > + /* Update call site for next level. */ > > + if (tag == DW_TAG_inlined_subroutine) { > > + call_file = die_get_call_file(die); > > + call_line = die_get_call_lineno(die); > > + } else { > > + /* Reached the root subprogram. */ > > + break; > > + } > > + } > > + free(scopes); > > + } > > + } > > + > > + return 1; > > +} > > diff --git a/tools/perf/util/libdw.h b/tools/perf/util/libdw.h > > new file mode 100644 > > index 000000000000..0f8d7b4a11a5 > > --- /dev/null > > +++ b/tools/perf/util/libdw.h > > @@ -0,0 +1,60 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#ifndef PERF_LIBDW_H > > +#define PERF_LIBDW_H > > + > > +#include <linux/types.h> > > + > > +struct dso; > > +struct inline_node; > > +struct symbol; > > + > > +#ifdef HAVE_LIBDW_SUPPORT > > +/* > > + * libdw__addr2line - Convert address to source location using libdw > > + * @dso_name: Name of the DSO > > + * @addr: Address to resolve > > + * @file: Pointer to return filename (caller must free) > > + * @line_nr: Pointer to return line number > > + * @dso: The dso struct > > + * @unwind_inlines: Whether to unwind inline function calls > > + * @node: Inline node list to append to > > + * @sym: The symbol associated with the address > > + * > > + * This function initializes a Dwfl context for the DSO if not already present, > > + * finds the source line information for the given address, and optionally > > + * resolves inline function call chains. > > + * > > + * Returns 1 on success (found), 0 on failure (not found). > > + */ > > +int libdw__addr2line(const char *dso_name, u64 addr, char **file, > > + unsigned int *line_nr, struct dso *dso, > > + bool unwind_inlines, struct inline_node *node, > > + struct symbol *sym); > > + > > +/* > > + * dso__free_a2l_libdw - Free libdw resources associated with the DSO > > + * @dso: The dso to free resources for > > + * > > + * This function cleans up the Dwfl context used for addr2line lookups. > > + */ > > +void dso__free_a2l_libdw(struct dso *dso); > > + > > +#else /* HAVE_LIBDW_SUPPORT */ > > + > > +static inline int libdw__addr2line(const char *dso_name __maybe_unused, > > + u64 addr __maybe_unused, char **file __maybe_unused, > > + unsigned int *line_nr __maybe_unused, > > + struct dso *dso __maybe_unused, > > + bool unwind_inlines __maybe_unused, > > + struct inline_node *node __maybe_unused, > > + struct symbol *sym __maybe_unused) > > +{ > > + return 0; > > +} > > + > > +static inline void dso__free_a2l_libdw(struct dso *dso __maybe_unused) > > +{ > > +} > > +#endif /* HAVE_LIBDW_SUPPORT */ > > + > > +#endif /* PERF_LIBDW_H */ > > diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c > > index 27c0966611ab..4b456c4d4138 100644 > > --- a/tools/perf/util/srcline.c > > +++ b/tools/perf/util/srcline.c > > @@ -6,6 +6,7 @@ > > #include "libbfd.h" > > #include "llvm.h" > > #include "symbol.h" > > +#include "libdw.h" > > > > #include <inttypes.h> > > #include <string.h> > > @@ -120,6 +121,10 @@ static int addr2line(const char *dso_name, u64 addr, char **file, unsigned int * > > { > > int ret; > > > > + ret = libdw__addr2line(dso_name, addr, file, line_nr, dso, unwind_inlines, node, sym); > > + if (ret > 0) > > + return ret; > > + > > ret = llvm__addr2line(dso_name, addr, file, line_nr, dso, unwind_inlines, node, sym); > > if (ret > 0) > > return ret; > > -- > > 2.52.0.rc2.455.g230fcf2819-goog > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1] perf addr2line: Add a libdw implementation 2025-11-27 11:43 ` Ian Rogers @ 2025-11-27 12:16 ` James Clark 2025-11-27 13:19 ` Ian Rogers 2025-12-11 23:35 ` Tony Jones 2025-11-27 20:53 ` Namhyung Kim 1 sibling, 2 replies; 13+ messages in thread From: James Clark @ 2025-11-27 12:16 UTC (permalink / raw) To: Ian Rogers, Namhyung Kim Cc: Tony Jones, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa, Adrian Hunter, Howard Chu, Stephen Brennan, linux-kernel, linux-perf-users On 27/11/2025 11:43 am, Ian Rogers wrote: > On Wed, Nov 26, 2025 at 10:27 AM Namhyung Kim <namhyung@kernel.org> wrote: >> >> On Sat, Nov 22, 2025 at 01:39:34AM -0800, Ian Rogers wrote: >>> Add an implementation of addr2line that uses libdw. Other addr2line >>> implementations or, in the case of forking addr2line, slow. Add an >>> implementation that caches the libdw information in the dso and uses >>> it to find the file and line number information. > > Thanks James and Namhyung for the reviews! I agree with James' comment > about a typo in the commit message. > >> My concern is the limit in the open file descriptors in case the data >> touched a lot of different libraries. The DSO code has some logic to >> deal with it but I'm not sure if we can share that since libdw seems to >> want to own the FD. > > The code opens the FD: > > + fd = open(dso_name, O_RDONLY); > + if (fd < 0) > + return 0; > + > + dwfl = dwfl_begin(&offline_callbacks); > + if (!dwfl) { > + close(fd); > + return 0; > + } > > It then uses the FD and closes it (the close is hidden in libdw itself): > > + /* > + * If the report is successful, the file descriptor fd > is consumed > + * and closed by the Dwfl. If not, it is not closed. > + */ > + mod = dwfl_report_offline(dwfl, dso_name, dso_name, fd); > > So it is possible we exhaust all the file descriptors if there are > multiple concurrent calls to libdw__addr2line and every dso has > missing libdw dwfl data... but because the open/close are close > together and that 1 FD is say small to the FDs needed for the > cmd__addr2line, I don't think it is a problem we need to specifically > handle. Were the FD kept open until the dso was deleted, I'd agree > with you. > >> Also, have you checked if this generates the exactly same output with >> other implementations? > > So the code passes `perf test` and I was checking functionality with > perf annotate, top, etc. What I saw looked good, but it may not have > been exhaustive. I didn't specifically create a test that compares the > output of the different addr2line implementations. Such a test would > be possible, it's not something we've done elsewhere. > > Thanks, > Ian > I manually looked at a couple of line numbers and they looked reasonable. I think an automated test that compared dwarf decoders would be a bit of a nightmare because I'm sure there would always be subtle differences. Doing a manual side by side comparison of libdw__addr2line() and llvm__addr2line(), they seem to be quite different: $ perf record -- perf test -w leafloop $ perf script -F ip,srcline > libdw_addr2line.txt # Comment out libdw_addr2line() and rebuild $ perf script -F ip,srcline > llvm_addr2line.txt $ diff libdw_addr2line.txt llvm_addr2line.txt It gets all of the simple leafloop workload lines the same, but lots of the libc stuff is different. For example libdw gives cpu-features.c:350 where llvm gives strcmp.S. At least that one looks like inlining so llvm might be "better", but where it gives dl-cache.c:490 isntead of llvm's mmap64.c:47, it's hard to see how that can be inlining if they're in different compilation units. If the llvm addr2line implementation is also supposed to be slow, it just means we're trading speed with accuracy with this change. Hard to say what the default should be in that case. >> Thanks, >> Namhyung >> >>> >>> Signed-off-by: Ian Rogers <irogers@google.com> >>> --- >>> tools/perf/util/Build | 1 + >>> tools/perf/util/dso.c | 2 + >>> tools/perf/util/dso.h | 11 +++ >>> tools/perf/util/libdw.c | 136 ++++++++++++++++++++++++++++++++++++++ >>> tools/perf/util/libdw.h | 60 +++++++++++++++++ >>> tools/perf/util/srcline.c | 5 ++ >>> 6 files changed, 215 insertions(+) >>> create mode 100644 tools/perf/util/libdw.c >>> create mode 100644 tools/perf/util/libdw.h >>> >>> diff --git a/tools/perf/util/Build b/tools/perf/util/Build >>> index 1c2a43e1dc68..2bed6274e248 100644 >>> --- a/tools/perf/util/Build >>> +++ b/tools/perf/util/Build >>> @@ -224,6 +224,7 @@ perf-util-$(CONFIG_LIBDW) += dwarf-regs-powerpc.o >>> perf-util-$(CONFIG_LIBDW) += dwarf-regs-x86.o >>> perf-util-$(CONFIG_LIBDW) += debuginfo.o >>> perf-util-$(CONFIG_LIBDW) += annotate-data.o >>> +perf-util-$(CONFIG_LIBDW) += libdw.o >>> >>> perf-util-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o >>> perf-util-$(CONFIG_LOCAL_LIBUNWIND) += unwind-libunwind-local.o >>> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c >>> index 344e689567ee..06980844c014 100644 >>> --- a/tools/perf/util/dso.c >>> +++ b/tools/perf/util/dso.c >>> @@ -32,6 +32,7 @@ >>> #include "string2.h" >>> #include "vdso.h" >>> #include "annotate-data.h" >>> +#include "libdw.h" >>> >>> static const char * const debuglink_paths[] = { >>> "%.0s%s", >>> @@ -1605,6 +1606,7 @@ void dso__delete(struct dso *dso) >>> auxtrace_cache__free(RC_CHK_ACCESS(dso)->auxtrace_cache); >>> dso_cache__free(dso); >>> dso__free_a2l(dso); >>> + dso__free_a2l_libdw(dso); >>> dso__free_symsrc_filename(dso); >>> nsinfo__zput(RC_CHK_ACCESS(dso)->nsinfo); >>> mutex_destroy(dso__lock(dso)); >>> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h >>> index f8ccb9816b89..4aee23775054 100644 >>> --- a/tools/perf/util/dso.h >>> +++ b/tools/perf/util/dso.h >>> @@ -268,6 +268,7 @@ DECLARE_RC_STRUCT(dso) { >>> const char *short_name; >>> const char *long_name; >>> void *a2l; >>> + void *a2l_libdw; >>> char *symsrc_filename; >>> #if defined(__powerpc__) >>> void *dwfl; /* DWARF debug info */ >>> @@ -334,6 +335,16 @@ static inline void dso__set_a2l(struct dso *dso, void *val) >>> RC_CHK_ACCESS(dso)->a2l = val; >>> } >>> >>> +static inline void *dso__a2l_libdw(const struct dso *dso) >>> +{ >>> + return RC_CHK_ACCESS(dso)->a2l_libdw; >>> +} >>> + >>> +static inline void dso__set_a2l_libdw(struct dso *dso, void *val) >>> +{ >>> + RC_CHK_ACCESS(dso)->a2l_libdw = val; >>> +} >>> + >>> static inline unsigned int dso__a2l_fails(const struct dso *dso) >>> { >>> return RC_CHK_ACCESS(dso)->a2l_fails; >>> diff --git a/tools/perf/util/libdw.c b/tools/perf/util/libdw.c >>> new file mode 100644 >>> index 000000000000..c4331fa8e1a3 >>> --- /dev/null >>> +++ b/tools/perf/util/libdw.c >>> @@ -0,0 +1,136 @@ >>> +// SPDX-License-Identifier: GPL-2.0 >>> +#include "dso.h" >>> +#include "libdw.h" >>> +#include "srcline.h" >>> +#include "symbol.h" >>> +#include "dwarf-aux.h" >>> +#include <fcntl.h> >>> +#include <unistd.h> >>> +#include <elfutils/libdwfl.h> >>> + >>> +void dso__free_a2l_libdw(struct dso *dso) >>> +{ >>> + Dwfl *dwfl = dso__a2l_libdw(dso); >>> + >>> + if (dwfl) { >>> + dwfl_end(dwfl); >>> + dso__set_a2l_libdw(dso, NULL); >>> + } >>> +} >>> + >>> +int libdw__addr2line(const char *dso_name, u64 addr, >>> + char **file, unsigned int *line_nr, >>> + struct dso *dso, bool unwind_inlines, >>> + struct inline_node *node, struct symbol *sym) >>> +{ >>> + static const Dwfl_Callbacks offline_callbacks = { >>> + .find_debuginfo = dwfl_standard_find_debuginfo, >>> + .section_address = dwfl_offline_section_address, >>> + .find_elf = dwfl_build_id_find_elf, >>> + }; >>> + Dwfl *dwfl = dso__a2l_libdw(dso); >>> + Dwfl_Module *mod; >>> + Dwfl_Line *dwline; >>> + Dwarf_Addr bias; >>> + const char *src; >>> + int lineno; >>> + >>> + if (!dwfl) { >>> + /* >>> + * Initialize Dwfl session. >>> + * We need to open the DSO file to report it to libdw. >>> + */ >>> + int fd; >>> + >>> + fd = open(dso_name, O_RDONLY); >>> + if (fd < 0) >>> + return 0; >>> + >>> + dwfl = dwfl_begin(&offline_callbacks); >>> + if (!dwfl) { >>> + close(fd); >>> + return 0; >>> + } >>> + >>> + /* >>> + * If the report is successful, the file descriptor fd is consumed >>> + * and closed by the Dwfl. If not, it is not closed. >>> + */ >>> + mod = dwfl_report_offline(dwfl, dso_name, dso_name, fd); >>> + if (!mod) { >>> + dwfl_end(dwfl); >>> + close(fd); >>> + return 0; >>> + } >>> + >>> + dwfl_report_end(dwfl, /*removed=*/NULL, /*arg=*/NULL); >>> + dso__set_a2l_libdw(dso, dwfl); >>> + } else { >>> + /* Dwfl session already initialized, get module for address. */ >>> + mod = dwfl_addrmodule(dwfl, addr); >>> + } >>> + >>> + if (!mod) >>> + return 0; >>> + >>> + /* Find source line information for the address. */ >>> + dwline = dwfl_module_getsrc(mod, addr); >>> + if (!dwline) >>> + return 0; >>> + >>> + /* Get line information. */ >>> + src = dwfl_lineinfo(dwline, &addr, &lineno, /*col=*/NULL, /*mtime=*/NULL, /*length=*/NULL); >>> + >>> + if (file) >>> + *file = src ? strdup(src) : NULL; >>> + if (line_nr) >>> + *line_nr = lineno; >>> + >>> + /* Optionally unwind inline function call chain. */ >>> + if (unwind_inlines && node && src) { >>> + Dwarf_Die *cudie = dwfl_module_addrdie(mod, addr, &bias); >>> + Dwarf_Die *scopes = NULL; >>> + int nscopes; >>> + >>> + if (!cudie) >>> + return 1; >>> + >>> + nscopes = die_get_scopes(cudie, addr, &scopes); >>> + if (nscopes > 0) { >>> + int i; >>> + const char *call_file = src; >>> + unsigned int call_line = lineno; >>> + >>> + for (i = 0; i < nscopes; i++) { >>> + Dwarf_Die *die = &scopes[i]; >>> + struct symbol *inline_sym; >>> + char *srcline = NULL; >>> + int tag = dwarf_tag(die); >>> + >>> + /* We are interested in inlined subroutines. */ >>> + if (tag != DW_TAG_inlined_subroutine && >>> + tag != DW_TAG_subprogram) >>> + continue; >>> + >>> + inline_sym = new_inline_sym(dso, sym, dwarf_diename(die)); >>> + >>> + if (call_file) >>> + srcline = srcline_from_fileline(call_file, call_line); >>> + >>> + inline_list__append(inline_sym, srcline, node); >>> + >>> + /* Update call site for next level. */ >>> + if (tag == DW_TAG_inlined_subroutine) { >>> + call_file = die_get_call_file(die); >>> + call_line = die_get_call_lineno(die); >>> + } else { >>> + /* Reached the root subprogram. */ >>> + break; >>> + } >>> + } >>> + free(scopes); >>> + } >>> + } >>> + >>> + return 1; >>> +} >>> diff --git a/tools/perf/util/libdw.h b/tools/perf/util/libdw.h >>> new file mode 100644 >>> index 000000000000..0f8d7b4a11a5 >>> --- /dev/null >>> +++ b/tools/perf/util/libdw.h >>> @@ -0,0 +1,60 @@ >>> +/* SPDX-License-Identifier: GPL-2.0 */ >>> +#ifndef PERF_LIBDW_H >>> +#define PERF_LIBDW_H >>> + >>> +#include <linux/types.h> >>> + >>> +struct dso; >>> +struct inline_node; >>> +struct symbol; >>> + >>> +#ifdef HAVE_LIBDW_SUPPORT >>> +/* >>> + * libdw__addr2line - Convert address to source location using libdw >>> + * @dso_name: Name of the DSO >>> + * @addr: Address to resolve >>> + * @file: Pointer to return filename (caller must free) >>> + * @line_nr: Pointer to return line number >>> + * @dso: The dso struct >>> + * @unwind_inlines: Whether to unwind inline function calls >>> + * @node: Inline node list to append to >>> + * @sym: The symbol associated with the address >>> + * >>> + * This function initializes a Dwfl context for the DSO if not already present, >>> + * finds the source line information for the given address, and optionally >>> + * resolves inline function call chains. >>> + * >>> + * Returns 1 on success (found), 0 on failure (not found). >>> + */ >>> +int libdw__addr2line(const char *dso_name, u64 addr, char **file, >>> + unsigned int *line_nr, struct dso *dso, >>> + bool unwind_inlines, struct inline_node *node, >>> + struct symbol *sym); >>> + >>> +/* >>> + * dso__free_a2l_libdw - Free libdw resources associated with the DSO >>> + * @dso: The dso to free resources for >>> + * >>> + * This function cleans up the Dwfl context used for addr2line lookups. >>> + */ >>> +void dso__free_a2l_libdw(struct dso *dso); >>> + >>> +#else /* HAVE_LIBDW_SUPPORT */ >>> + >>> +static inline int libdw__addr2line(const char *dso_name __maybe_unused, >>> + u64 addr __maybe_unused, char **file __maybe_unused, >>> + unsigned int *line_nr __maybe_unused, >>> + struct dso *dso __maybe_unused, >>> + bool unwind_inlines __maybe_unused, >>> + struct inline_node *node __maybe_unused, >>> + struct symbol *sym __maybe_unused) >>> +{ >>> + return 0; >>> +} >>> + >>> +static inline void dso__free_a2l_libdw(struct dso *dso __maybe_unused) >>> +{ >>> +} >>> +#endif /* HAVE_LIBDW_SUPPORT */ >>> + >>> +#endif /* PERF_LIBDW_H */ >>> diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c >>> index 27c0966611ab..4b456c4d4138 100644 >>> --- a/tools/perf/util/srcline.c >>> +++ b/tools/perf/util/srcline.c >>> @@ -6,6 +6,7 @@ >>> #include "libbfd.h" >>> #include "llvm.h" >>> #include "symbol.h" >>> +#include "libdw.h" >>> >>> #include <inttypes.h> >>> #include <string.h> >>> @@ -120,6 +121,10 @@ static int addr2line(const char *dso_name, u64 addr, char **file, unsigned int * >>> { >>> int ret; >>> >>> + ret = libdw__addr2line(dso_name, addr, file, line_nr, dso, unwind_inlines, node, sym); >>> + if (ret > 0) >>> + return ret; >>> + >>> ret = llvm__addr2line(dso_name, addr, file, line_nr, dso, unwind_inlines, node, sym); >>> if (ret > 0) >>> return ret; >>> -- >>> 2.52.0.rc2.455.g230fcf2819-goog >>> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1] perf addr2line: Add a libdw implementation 2025-11-27 12:16 ` James Clark @ 2025-11-27 13:19 ` Ian Rogers 2025-11-27 13:48 ` James Clark 2025-12-11 23:35 ` Tony Jones 1 sibling, 1 reply; 13+ messages in thread From: Ian Rogers @ 2025-11-27 13:19 UTC (permalink / raw) To: James Clark Cc: Namhyung Kim, Tony Jones, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa, Adrian Hunter, Howard Chu, Stephen Brennan, linux-kernel, linux-perf-users On Thu, Nov 27, 2025 at 4:16 AM James Clark <james.clark@linaro.org> wrote: > > > > On 27/11/2025 11:43 am, Ian Rogers wrote: > > On Wed, Nov 26, 2025 at 10:27 AM Namhyung Kim <namhyung@kernel.org> wrote: > >> > >> On Sat, Nov 22, 2025 at 01:39:34AM -0800, Ian Rogers wrote: > >>> Add an implementation of addr2line that uses libdw. Other addr2line > >>> implementations or, in the case of forking addr2line, slow. Add an > >>> implementation that caches the libdw information in the dso and uses > >>> it to find the file and line number information. > > > > Thanks James and Namhyung for the reviews! I agree with James' comment > > about a typo in the commit message. > > > >> My concern is the limit in the open file descriptors in case the data > >> touched a lot of different libraries. The DSO code has some logic to > >> deal with it but I'm not sure if we can share that since libdw seems to > >> want to own the FD. > > > > The code opens the FD: > > > > + fd = open(dso_name, O_RDONLY); > > + if (fd < 0) > > + return 0; > > + > > + dwfl = dwfl_begin(&offline_callbacks); > > + if (!dwfl) { > > + close(fd); > > + return 0; > > + } > > > > It then uses the FD and closes it (the close is hidden in libdw itself): > > > > + /* > > + * If the report is successful, the file descriptor fd > > is consumed > > + * and closed by the Dwfl. If not, it is not closed. > > + */ > > + mod = dwfl_report_offline(dwfl, dso_name, dso_name, fd); > > > > So it is possible we exhaust all the file descriptors if there are > > multiple concurrent calls to libdw__addr2line and every dso has > > missing libdw dwfl data... but because the open/close are close > > together and that 1 FD is say small to the FDs needed for the > > cmd__addr2line, I don't think it is a problem we need to specifically > > handle. Were the FD kept open until the dso was deleted, I'd agree > > with you. > > > >> Also, have you checked if this generates the exactly same output with > >> other implementations? > > > > So the code passes `perf test` and I was checking functionality with > > perf annotate, top, etc. What I saw looked good, but it may not have > > been exhaustive. I didn't specifically create a test that compares the > > output of the different addr2line implementations. Such a test would > > be possible, it's not something we've done elsewhere. > > > > Thanks, > > Ian > > > > I manually looked at a couple of line numbers and they looked > reasonable. I think an automated test that compared dwarf decoders would > be a bit of a nightmare because I'm sure there would always be subtle > differences. > > Doing a manual side by side comparison of libdw__addr2line() and > llvm__addr2line(), they seem to be quite different: > > $ perf record -- perf test -w leafloop > $ perf script -F ip,srcline > libdw_addr2line.txt > # Comment out libdw_addr2line() and rebuild > $ perf script -F ip,srcline > llvm_addr2line.txt > $ diff libdw_addr2line.txt llvm_addr2line.txt > > It gets all of the simple leafloop workload lines the same, but lots of > the libc stuff is different. > > For example libdw gives cpu-features.c:350 where llvm gives strcmp.S. At > least that one looks like inlining so llvm might be "better", but where > it gives dl-cache.c:490 isntead of llvm's mmap64.c:47, it's hard to see > how that can be inlining if they're in different compilation units. > > If the llvm addr2line implementation is also supposed to be slow, it > just means we're trading speed with accuracy with this change. Hard to > say what the default should be in that case. Agreed. We could do some kind of option scheme like with the disassemblers: https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/disasm.c?h=perf-tools-next#n1715 I'm not sure there are currently any distributions using the LLVM options, this in part motivates: https://lore.kernel.org/lkml/20251007163835.3152881-1-irogers@google.com/ but the addr2line APIs are a pain there - see the extra generated .so. Because of that it could be reasonable to delete the LLVM addr2line code and just focus on libdw. I kind of wish we could also delete the precarious command forking versions too. And libbfd... Perhaps with this code, those advocating libbfd have a fast enough alternative without bringing in large dependencies like libLLVM. Thanks, Ian > >> Thanks, > >> Namhyung > >> > >>> > >>> Signed-off-by: Ian Rogers <irogers@google.com> > >>> --- > >>> tools/perf/util/Build | 1 + > >>> tools/perf/util/dso.c | 2 + > >>> tools/perf/util/dso.h | 11 +++ > >>> tools/perf/util/libdw.c | 136 ++++++++++++++++++++++++++++++++++++++ > >>> tools/perf/util/libdw.h | 60 +++++++++++++++++ > >>> tools/perf/util/srcline.c | 5 ++ > >>> 6 files changed, 215 insertions(+) > >>> create mode 100644 tools/perf/util/libdw.c > >>> create mode 100644 tools/perf/util/libdw.h > >>> > >>> diff --git a/tools/perf/util/Build b/tools/perf/util/Build > >>> index 1c2a43e1dc68..2bed6274e248 100644 > >>> --- a/tools/perf/util/Build > >>> +++ b/tools/perf/util/Build > >>> @@ -224,6 +224,7 @@ perf-util-$(CONFIG_LIBDW) += dwarf-regs-powerpc.o > >>> perf-util-$(CONFIG_LIBDW) += dwarf-regs-x86.o > >>> perf-util-$(CONFIG_LIBDW) += debuginfo.o > >>> perf-util-$(CONFIG_LIBDW) += annotate-data.o > >>> +perf-util-$(CONFIG_LIBDW) += libdw.o > >>> > >>> perf-util-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o > >>> perf-util-$(CONFIG_LOCAL_LIBUNWIND) += unwind-libunwind-local.o > >>> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c > >>> index 344e689567ee..06980844c014 100644 > >>> --- a/tools/perf/util/dso.c > >>> +++ b/tools/perf/util/dso.c > >>> @@ -32,6 +32,7 @@ > >>> #include "string2.h" > >>> #include "vdso.h" > >>> #include "annotate-data.h" > >>> +#include "libdw.h" > >>> > >>> static const char * const debuglink_paths[] = { > >>> "%.0s%s", > >>> @@ -1605,6 +1606,7 @@ void dso__delete(struct dso *dso) > >>> auxtrace_cache__free(RC_CHK_ACCESS(dso)->auxtrace_cache); > >>> dso_cache__free(dso); > >>> dso__free_a2l(dso); > >>> + dso__free_a2l_libdw(dso); > >>> dso__free_symsrc_filename(dso); > >>> nsinfo__zput(RC_CHK_ACCESS(dso)->nsinfo); > >>> mutex_destroy(dso__lock(dso)); > >>> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h > >>> index f8ccb9816b89..4aee23775054 100644 > >>> --- a/tools/perf/util/dso.h > >>> +++ b/tools/perf/util/dso.h > >>> @@ -268,6 +268,7 @@ DECLARE_RC_STRUCT(dso) { > >>> const char *short_name; > >>> const char *long_name; > >>> void *a2l; > >>> + void *a2l_libdw; > >>> char *symsrc_filename; > >>> #if defined(__powerpc__) > >>> void *dwfl; /* DWARF debug info */ > >>> @@ -334,6 +335,16 @@ static inline void dso__set_a2l(struct dso *dso, void *val) > >>> RC_CHK_ACCESS(dso)->a2l = val; > >>> } > >>> > >>> +static inline void *dso__a2l_libdw(const struct dso *dso) > >>> +{ > >>> + return RC_CHK_ACCESS(dso)->a2l_libdw; > >>> +} > >>> + > >>> +static inline void dso__set_a2l_libdw(struct dso *dso, void *val) > >>> +{ > >>> + RC_CHK_ACCESS(dso)->a2l_libdw = val; > >>> +} > >>> + > >>> static inline unsigned int dso__a2l_fails(const struct dso *dso) > >>> { > >>> return RC_CHK_ACCESS(dso)->a2l_fails; > >>> diff --git a/tools/perf/util/libdw.c b/tools/perf/util/libdw.c > >>> new file mode 100644 > >>> index 000000000000..c4331fa8e1a3 > >>> --- /dev/null > >>> +++ b/tools/perf/util/libdw.c > >>> @@ -0,0 +1,136 @@ > >>> +// SPDX-License-Identifier: GPL-2.0 > >>> +#include "dso.h" > >>> +#include "libdw.h" > >>> +#include "srcline.h" > >>> +#include "symbol.h" > >>> +#include "dwarf-aux.h" > >>> +#include <fcntl.h> > >>> +#include <unistd.h> > >>> +#include <elfutils/libdwfl.h> > >>> + > >>> +void dso__free_a2l_libdw(struct dso *dso) > >>> +{ > >>> + Dwfl *dwfl = dso__a2l_libdw(dso); > >>> + > >>> + if (dwfl) { > >>> + dwfl_end(dwfl); > >>> + dso__set_a2l_libdw(dso, NULL); > >>> + } > >>> +} > >>> + > >>> +int libdw__addr2line(const char *dso_name, u64 addr, > >>> + char **file, unsigned int *line_nr, > >>> + struct dso *dso, bool unwind_inlines, > >>> + struct inline_node *node, struct symbol *sym) > >>> +{ > >>> + static const Dwfl_Callbacks offline_callbacks = { > >>> + .find_debuginfo = dwfl_standard_find_debuginfo, > >>> + .section_address = dwfl_offline_section_address, > >>> + .find_elf = dwfl_build_id_find_elf, > >>> + }; > >>> + Dwfl *dwfl = dso__a2l_libdw(dso); > >>> + Dwfl_Module *mod; > >>> + Dwfl_Line *dwline; > >>> + Dwarf_Addr bias; > >>> + const char *src; > >>> + int lineno; > >>> + > >>> + if (!dwfl) { > >>> + /* > >>> + * Initialize Dwfl session. > >>> + * We need to open the DSO file to report it to libdw. > >>> + */ > >>> + int fd; > >>> + > >>> + fd = open(dso_name, O_RDONLY); > >>> + if (fd < 0) > >>> + return 0; > >>> + > >>> + dwfl = dwfl_begin(&offline_callbacks); > >>> + if (!dwfl) { > >>> + close(fd); > >>> + return 0; > >>> + } > >>> + > >>> + /* > >>> + * If the report is successful, the file descriptor fd is consumed > >>> + * and closed by the Dwfl. If not, it is not closed. > >>> + */ > >>> + mod = dwfl_report_offline(dwfl, dso_name, dso_name, fd); > >>> + if (!mod) { > >>> + dwfl_end(dwfl); > >>> + close(fd); > >>> + return 0; > >>> + } > >>> + > >>> + dwfl_report_end(dwfl, /*removed=*/NULL, /*arg=*/NULL); > >>> + dso__set_a2l_libdw(dso, dwfl); > >>> + } else { > >>> + /* Dwfl session already initialized, get module for address. */ > >>> + mod = dwfl_addrmodule(dwfl, addr); > >>> + } > >>> + > >>> + if (!mod) > >>> + return 0; > >>> + > >>> + /* Find source line information for the address. */ > >>> + dwline = dwfl_module_getsrc(mod, addr); > >>> + if (!dwline) > >>> + return 0; > >>> + > >>> + /* Get line information. */ > >>> + src = dwfl_lineinfo(dwline, &addr, &lineno, /*col=*/NULL, /*mtime=*/NULL, /*length=*/NULL); > >>> + > >>> + if (file) > >>> + *file = src ? strdup(src) : NULL; > >>> + if (line_nr) > >>> + *line_nr = lineno; > >>> + > >>> + /* Optionally unwind inline function call chain. */ > >>> + if (unwind_inlines && node && src) { > >>> + Dwarf_Die *cudie = dwfl_module_addrdie(mod, addr, &bias); > >>> + Dwarf_Die *scopes = NULL; > >>> + int nscopes; > >>> + > >>> + if (!cudie) > >>> + return 1; > >>> + > >>> + nscopes = die_get_scopes(cudie, addr, &scopes); > >>> + if (nscopes > 0) { > >>> + int i; > >>> + const char *call_file = src; > >>> + unsigned int call_line = lineno; > >>> + > >>> + for (i = 0; i < nscopes; i++) { > >>> + Dwarf_Die *die = &scopes[i]; > >>> + struct symbol *inline_sym; > >>> + char *srcline = NULL; > >>> + int tag = dwarf_tag(die); > >>> + > >>> + /* We are interested in inlined subroutines. */ > >>> + if (tag != DW_TAG_inlined_subroutine && > >>> + tag != DW_TAG_subprogram) > >>> + continue; > >>> + > >>> + inline_sym = new_inline_sym(dso, sym, dwarf_diename(die)); > >>> + > >>> + if (call_file) > >>> + srcline = srcline_from_fileline(call_file, call_line); > >>> + > >>> + inline_list__append(inline_sym, srcline, node); > >>> + > >>> + /* Update call site for next level. */ > >>> + if (tag == DW_TAG_inlined_subroutine) { > >>> + call_file = die_get_call_file(die); > >>> + call_line = die_get_call_lineno(die); > >>> + } else { > >>> + /* Reached the root subprogram. */ > >>> + break; > >>> + } > >>> + } > >>> + free(scopes); > >>> + } > >>> + } > >>> + > >>> + return 1; > >>> +} > >>> diff --git a/tools/perf/util/libdw.h b/tools/perf/util/libdw.h > >>> new file mode 100644 > >>> index 000000000000..0f8d7b4a11a5 > >>> --- /dev/null > >>> +++ b/tools/perf/util/libdw.h > >>> @@ -0,0 +1,60 @@ > >>> +/* SPDX-License-Identifier: GPL-2.0 */ > >>> +#ifndef PERF_LIBDW_H > >>> +#define PERF_LIBDW_H > >>> + > >>> +#include <linux/types.h> > >>> + > >>> +struct dso; > >>> +struct inline_node; > >>> +struct symbol; > >>> + > >>> +#ifdef HAVE_LIBDW_SUPPORT > >>> +/* > >>> + * libdw__addr2line - Convert address to source location using libdw > >>> + * @dso_name: Name of the DSO > >>> + * @addr: Address to resolve > >>> + * @file: Pointer to return filename (caller must free) > >>> + * @line_nr: Pointer to return line number > >>> + * @dso: The dso struct > >>> + * @unwind_inlines: Whether to unwind inline function calls > >>> + * @node: Inline node list to append to > >>> + * @sym: The symbol associated with the address > >>> + * > >>> + * This function initializes a Dwfl context for the DSO if not already present, > >>> + * finds the source line information for the given address, and optionally > >>> + * resolves inline function call chains. > >>> + * > >>> + * Returns 1 on success (found), 0 on failure (not found). > >>> + */ > >>> +int libdw__addr2line(const char *dso_name, u64 addr, char **file, > >>> + unsigned int *line_nr, struct dso *dso, > >>> + bool unwind_inlines, struct inline_node *node, > >>> + struct symbol *sym); > >>> + > >>> +/* > >>> + * dso__free_a2l_libdw - Free libdw resources associated with the DSO > >>> + * @dso: The dso to free resources for > >>> + * > >>> + * This function cleans up the Dwfl context used for addr2line lookups. > >>> + */ > >>> +void dso__free_a2l_libdw(struct dso *dso); > >>> + > >>> +#else /* HAVE_LIBDW_SUPPORT */ > >>> + > >>> +static inline int libdw__addr2line(const char *dso_name __maybe_unused, > >>> + u64 addr __maybe_unused, char **file __maybe_unused, > >>> + unsigned int *line_nr __maybe_unused, > >>> + struct dso *dso __maybe_unused, > >>> + bool unwind_inlines __maybe_unused, > >>> + struct inline_node *node __maybe_unused, > >>> + struct symbol *sym __maybe_unused) > >>> +{ > >>> + return 0; > >>> +} > >>> + > >>> +static inline void dso__free_a2l_libdw(struct dso *dso __maybe_unused) > >>> +{ > >>> +} > >>> +#endif /* HAVE_LIBDW_SUPPORT */ > >>> + > >>> +#endif /* PERF_LIBDW_H */ > >>> diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c > >>> index 27c0966611ab..4b456c4d4138 100644 > >>> --- a/tools/perf/util/srcline.c > >>> +++ b/tools/perf/util/srcline.c > >>> @@ -6,6 +6,7 @@ > >>> #include "libbfd.h" > >>> #include "llvm.h" > >>> #include "symbol.h" > >>> +#include "libdw.h" > >>> > >>> #include <inttypes.h> > >>> #include <string.h> > >>> @@ -120,6 +121,10 @@ static int addr2line(const char *dso_name, u64 addr, char **file, unsigned int * > >>> { > >>> int ret; > >>> > >>> + ret = libdw__addr2line(dso_name, addr, file, line_nr, dso, unwind_inlines, node, sym); > >>> + if (ret > 0) > >>> + return ret; > >>> + > >>> ret = llvm__addr2line(dso_name, addr, file, line_nr, dso, unwind_inlines, node, sym); > >>> if (ret > 0) > >>> return ret; > >>> -- > >>> 2.52.0.rc2.455.g230fcf2819-goog > >>> > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1] perf addr2line: Add a libdw implementation 2025-11-27 13:19 ` Ian Rogers @ 2025-11-27 13:48 ` James Clark 2025-11-28 9:00 ` Ian Rogers 0 siblings, 1 reply; 13+ messages in thread From: James Clark @ 2025-11-27 13:48 UTC (permalink / raw) To: Ian Rogers Cc: Namhyung Kim, Tony Jones, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa, Adrian Hunter, Howard Chu, Stephen Brennan, linux-kernel, linux-perf-users On 27/11/2025 1:19 pm, Ian Rogers wrote: > On Thu, Nov 27, 2025 at 4:16 AM James Clark <james.clark@linaro.org> wrote: >> >> >> >> On 27/11/2025 11:43 am, Ian Rogers wrote: >>> On Wed, Nov 26, 2025 at 10:27 AM Namhyung Kim <namhyung@kernel.org> wrote: >>>> >>>> On Sat, Nov 22, 2025 at 01:39:34AM -0800, Ian Rogers wrote: >>>>> Add an implementation of addr2line that uses libdw. Other addr2line >>>>> implementations or, in the case of forking addr2line, slow. Add an >>>>> implementation that caches the libdw information in the dso and uses >>>>> it to find the file and line number information. >>> >>> Thanks James and Namhyung for the reviews! I agree with James' comment >>> about a typo in the commit message. >>> >>>> My concern is the limit in the open file descriptors in case the data >>>> touched a lot of different libraries. The DSO code has some logic to >>>> deal with it but I'm not sure if we can share that since libdw seems to >>>> want to own the FD. >>> >>> The code opens the FD: >>> >>> + fd = open(dso_name, O_RDONLY); >>> + if (fd < 0) >>> + return 0; >>> + >>> + dwfl = dwfl_begin(&offline_callbacks); >>> + if (!dwfl) { >>> + close(fd); >>> + return 0; >>> + } >>> >>> It then uses the FD and closes it (the close is hidden in libdw itself): >>> >>> + /* >>> + * If the report is successful, the file descriptor fd >>> is consumed >>> + * and closed by the Dwfl. If not, it is not closed. >>> + */ >>> + mod = dwfl_report_offline(dwfl, dso_name, dso_name, fd); >>> >>> So it is possible we exhaust all the file descriptors if there are >>> multiple concurrent calls to libdw__addr2line and every dso has >>> missing libdw dwfl data... but because the open/close are close >>> together and that 1 FD is say small to the FDs needed for the >>> cmd__addr2line, I don't think it is a problem we need to specifically >>> handle. Were the FD kept open until the dso was deleted, I'd agree >>> with you. >>> >>>> Also, have you checked if this generates the exactly same output with >>>> other implementations? >>> >>> So the code passes `perf test` and I was checking functionality with >>> perf annotate, top, etc. What I saw looked good, but it may not have >>> been exhaustive. I didn't specifically create a test that compares the >>> output of the different addr2line implementations. Such a test would >>> be possible, it's not something we've done elsewhere. >>> >>> Thanks, >>> Ian >>> >> >> I manually looked at a couple of line numbers and they looked >> reasonable. I think an automated test that compared dwarf decoders would >> be a bit of a nightmare because I'm sure there would always be subtle >> differences. >> >> Doing a manual side by side comparison of libdw__addr2line() and >> llvm__addr2line(), they seem to be quite different: >> >> $ perf record -- perf test -w leafloop >> $ perf script -F ip,srcline > libdw_addr2line.txt >> # Comment out libdw_addr2line() and rebuild >> $ perf script -F ip,srcline > llvm_addr2line.txt >> $ diff libdw_addr2line.txt llvm_addr2line.txt >> >> It gets all of the simple leafloop workload lines the same, but lots of >> the libc stuff is different. >> >> For example libdw gives cpu-features.c:350 where llvm gives strcmp.S. At >> least that one looks like inlining so llvm might be "better", but where >> it gives dl-cache.c:490 isntead of llvm's mmap64.c:47, it's hard to see >> how that can be inlining if they're in different compilation units. >> >> If the llvm addr2line implementation is also supposed to be slow, it >> just means we're trading speed with accuracy with this change. Hard to >> say what the default should be in that case. > > Agreed. We could do some kind of option scheme like with the disassemblers: > https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/disasm.c?h=perf-tools-next#n1715 > > I'm not sure there are currently any distributions using the LLVM > options, this in part motivates: > https://lore.kernel.org/lkml/20251007163835.3152881-1-irogers@google.com/ > but the addr2line APIs are a pain there - see the extra generated .so. > Because of that it could be reasonable to delete the LLVM addr2line > code and just focus on libdw. I kind of wish we could also delete the > precarious command forking versions too. And libbfd... Perhaps with > this code, those advocating libbfd have a fast enough alternative > without bringing in large dependencies like libLLVM. > > Thanks, > Ian > Personally for debugging I would want the most accurate and detailed information possible and don't care at all about the size of dependencies. If Perf is a tool for debugging then surely that should be the default? But right now I don't actually know which of the outputs is most accurate although I'll assume that the llvm one is. Wanting the smallest application possible while trading away detail and accuracy seems like an unusual special case that should be opt in. It also seems to relate to this old discussion [1], in that if we're going to keep the llvm depencency then it doesn't make sense to re-write anything in Perf. [1]: https://lore.kernel.org/linux-perf-users/549d3812-a606-4981-83f5-0a99b0ff9f6a@linaro.org/ >>>> Thanks, >>>> Namhyung >>>> >>>>> >>>>> Signed-off-by: Ian Rogers <irogers@google.com> >>>>> --- >>>>> tools/perf/util/Build | 1 + >>>>> tools/perf/util/dso.c | 2 + >>>>> tools/perf/util/dso.h | 11 +++ >>>>> tools/perf/util/libdw.c | 136 ++++++++++++++++++++++++++++++++++++++ >>>>> tools/perf/util/libdw.h | 60 +++++++++++++++++ >>>>> tools/perf/util/srcline.c | 5 ++ >>>>> 6 files changed, 215 insertions(+) >>>>> create mode 100644 tools/perf/util/libdw.c >>>>> create mode 100644 tools/perf/util/libdw.h >>>>> >>>>> diff --git a/tools/perf/util/Build b/tools/perf/util/Build >>>>> index 1c2a43e1dc68..2bed6274e248 100644 >>>>> --- a/tools/perf/util/Build >>>>> +++ b/tools/perf/util/Build >>>>> @@ -224,6 +224,7 @@ perf-util-$(CONFIG_LIBDW) += dwarf-regs-powerpc.o >>>>> perf-util-$(CONFIG_LIBDW) += dwarf-regs-x86.o >>>>> perf-util-$(CONFIG_LIBDW) += debuginfo.o >>>>> perf-util-$(CONFIG_LIBDW) += annotate-data.o >>>>> +perf-util-$(CONFIG_LIBDW) += libdw.o >>>>> >>>>> perf-util-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o >>>>> perf-util-$(CONFIG_LOCAL_LIBUNWIND) += unwind-libunwind-local.o >>>>> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c >>>>> index 344e689567ee..06980844c014 100644 >>>>> --- a/tools/perf/util/dso.c >>>>> +++ b/tools/perf/util/dso.c >>>>> @@ -32,6 +32,7 @@ >>>>> #include "string2.h" >>>>> #include "vdso.h" >>>>> #include "annotate-data.h" >>>>> +#include "libdw.h" >>>>> >>>>> static const char * const debuglink_paths[] = { >>>>> "%.0s%s", >>>>> @@ -1605,6 +1606,7 @@ void dso__delete(struct dso *dso) >>>>> auxtrace_cache__free(RC_CHK_ACCESS(dso)->auxtrace_cache); >>>>> dso_cache__free(dso); >>>>> dso__free_a2l(dso); >>>>> + dso__free_a2l_libdw(dso); >>>>> dso__free_symsrc_filename(dso); >>>>> nsinfo__zput(RC_CHK_ACCESS(dso)->nsinfo); >>>>> mutex_destroy(dso__lock(dso)); >>>>> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h >>>>> index f8ccb9816b89..4aee23775054 100644 >>>>> --- a/tools/perf/util/dso.h >>>>> +++ b/tools/perf/util/dso.h >>>>> @@ -268,6 +268,7 @@ DECLARE_RC_STRUCT(dso) { >>>>> const char *short_name; >>>>> const char *long_name; >>>>> void *a2l; >>>>> + void *a2l_libdw; >>>>> char *symsrc_filename; >>>>> #if defined(__powerpc__) >>>>> void *dwfl; /* DWARF debug info */ >>>>> @@ -334,6 +335,16 @@ static inline void dso__set_a2l(struct dso *dso, void *val) >>>>> RC_CHK_ACCESS(dso)->a2l = val; >>>>> } >>>>> >>>>> +static inline void *dso__a2l_libdw(const struct dso *dso) >>>>> +{ >>>>> + return RC_CHK_ACCESS(dso)->a2l_libdw; >>>>> +} >>>>> + >>>>> +static inline void dso__set_a2l_libdw(struct dso *dso, void *val) >>>>> +{ >>>>> + RC_CHK_ACCESS(dso)->a2l_libdw = val; >>>>> +} >>>>> + >>>>> static inline unsigned int dso__a2l_fails(const struct dso *dso) >>>>> { >>>>> return RC_CHK_ACCESS(dso)->a2l_fails; >>>>> diff --git a/tools/perf/util/libdw.c b/tools/perf/util/libdw.c >>>>> new file mode 100644 >>>>> index 000000000000..c4331fa8e1a3 >>>>> --- /dev/null >>>>> +++ b/tools/perf/util/libdw.c >>>>> @@ -0,0 +1,136 @@ >>>>> +// SPDX-License-Identifier: GPL-2.0 >>>>> +#include "dso.h" >>>>> +#include "libdw.h" >>>>> +#include "srcline.h" >>>>> +#include "symbol.h" >>>>> +#include "dwarf-aux.h" >>>>> +#include <fcntl.h> >>>>> +#include <unistd.h> >>>>> +#include <elfutils/libdwfl.h> >>>>> + >>>>> +void dso__free_a2l_libdw(struct dso *dso) >>>>> +{ >>>>> + Dwfl *dwfl = dso__a2l_libdw(dso); >>>>> + >>>>> + if (dwfl) { >>>>> + dwfl_end(dwfl); >>>>> + dso__set_a2l_libdw(dso, NULL); >>>>> + } >>>>> +} >>>>> + >>>>> +int libdw__addr2line(const char *dso_name, u64 addr, >>>>> + char **file, unsigned int *line_nr, >>>>> + struct dso *dso, bool unwind_inlines, >>>>> + struct inline_node *node, struct symbol *sym) >>>>> +{ >>>>> + static const Dwfl_Callbacks offline_callbacks = { >>>>> + .find_debuginfo = dwfl_standard_find_debuginfo, >>>>> + .section_address = dwfl_offline_section_address, >>>>> + .find_elf = dwfl_build_id_find_elf, >>>>> + }; >>>>> + Dwfl *dwfl = dso__a2l_libdw(dso); >>>>> + Dwfl_Module *mod; >>>>> + Dwfl_Line *dwline; >>>>> + Dwarf_Addr bias; >>>>> + const char *src; >>>>> + int lineno; >>>>> + >>>>> + if (!dwfl) { >>>>> + /* >>>>> + * Initialize Dwfl session. >>>>> + * We need to open the DSO file to report it to libdw. >>>>> + */ >>>>> + int fd; >>>>> + >>>>> + fd = open(dso_name, O_RDONLY); >>>>> + if (fd < 0) >>>>> + return 0; >>>>> + >>>>> + dwfl = dwfl_begin(&offline_callbacks); >>>>> + if (!dwfl) { >>>>> + close(fd); >>>>> + return 0; >>>>> + } >>>>> + >>>>> + /* >>>>> + * If the report is successful, the file descriptor fd is consumed >>>>> + * and closed by the Dwfl. If not, it is not closed. >>>>> + */ >>>>> + mod = dwfl_report_offline(dwfl, dso_name, dso_name, fd); >>>>> + if (!mod) { >>>>> + dwfl_end(dwfl); >>>>> + close(fd); >>>>> + return 0; >>>>> + } >>>>> + >>>>> + dwfl_report_end(dwfl, /*removed=*/NULL, /*arg=*/NULL); >>>>> + dso__set_a2l_libdw(dso, dwfl); >>>>> + } else { >>>>> + /* Dwfl session already initialized, get module for address. */ >>>>> + mod = dwfl_addrmodule(dwfl, addr); >>>>> + } >>>>> + >>>>> + if (!mod) >>>>> + return 0; >>>>> + >>>>> + /* Find source line information for the address. */ >>>>> + dwline = dwfl_module_getsrc(mod, addr); >>>>> + if (!dwline) >>>>> + return 0; >>>>> + >>>>> + /* Get line information. */ >>>>> + src = dwfl_lineinfo(dwline, &addr, &lineno, /*col=*/NULL, /*mtime=*/NULL, /*length=*/NULL); >>>>> + >>>>> + if (file) >>>>> + *file = src ? strdup(src) : NULL; >>>>> + if (line_nr) >>>>> + *line_nr = lineno; >>>>> + >>>>> + /* Optionally unwind inline function call chain. */ >>>>> + if (unwind_inlines && node && src) { >>>>> + Dwarf_Die *cudie = dwfl_module_addrdie(mod, addr, &bias); >>>>> + Dwarf_Die *scopes = NULL; >>>>> + int nscopes; >>>>> + >>>>> + if (!cudie) >>>>> + return 1; >>>>> + >>>>> + nscopes = die_get_scopes(cudie, addr, &scopes); >>>>> + if (nscopes > 0) { >>>>> + int i; >>>>> + const char *call_file = src; >>>>> + unsigned int call_line = lineno; >>>>> + >>>>> + for (i = 0; i < nscopes; i++) { >>>>> + Dwarf_Die *die = &scopes[i]; >>>>> + struct symbol *inline_sym; >>>>> + char *srcline = NULL; >>>>> + int tag = dwarf_tag(die); >>>>> + >>>>> + /* We are interested in inlined subroutines. */ >>>>> + if (tag != DW_TAG_inlined_subroutine && >>>>> + tag != DW_TAG_subprogram) >>>>> + continue; >>>>> + >>>>> + inline_sym = new_inline_sym(dso, sym, dwarf_diename(die)); >>>>> + >>>>> + if (call_file) >>>>> + srcline = srcline_from_fileline(call_file, call_line); >>>>> + >>>>> + inline_list__append(inline_sym, srcline, node); >>>>> + >>>>> + /* Update call site for next level. */ >>>>> + if (tag == DW_TAG_inlined_subroutine) { >>>>> + call_file = die_get_call_file(die); >>>>> + call_line = die_get_call_lineno(die); >>>>> + } else { >>>>> + /* Reached the root subprogram. */ >>>>> + break; >>>>> + } >>>>> + } >>>>> + free(scopes); >>>>> + } >>>>> + } >>>>> + >>>>> + return 1; >>>>> +} >>>>> diff --git a/tools/perf/util/libdw.h b/tools/perf/util/libdw.h >>>>> new file mode 100644 >>>>> index 000000000000..0f8d7b4a11a5 >>>>> --- /dev/null >>>>> +++ b/tools/perf/util/libdw.h >>>>> @@ -0,0 +1,60 @@ >>>>> +/* SPDX-License-Identifier: GPL-2.0 */ >>>>> +#ifndef PERF_LIBDW_H >>>>> +#define PERF_LIBDW_H >>>>> + >>>>> +#include <linux/types.h> >>>>> + >>>>> +struct dso; >>>>> +struct inline_node; >>>>> +struct symbol; >>>>> + >>>>> +#ifdef HAVE_LIBDW_SUPPORT >>>>> +/* >>>>> + * libdw__addr2line - Convert address to source location using libdw >>>>> + * @dso_name: Name of the DSO >>>>> + * @addr: Address to resolve >>>>> + * @file: Pointer to return filename (caller must free) >>>>> + * @line_nr: Pointer to return line number >>>>> + * @dso: The dso struct >>>>> + * @unwind_inlines: Whether to unwind inline function calls >>>>> + * @node: Inline node list to append to >>>>> + * @sym: The symbol associated with the address >>>>> + * >>>>> + * This function initializes a Dwfl context for the DSO if not already present, >>>>> + * finds the source line information for the given address, and optionally >>>>> + * resolves inline function call chains. >>>>> + * >>>>> + * Returns 1 on success (found), 0 on failure (not found). >>>>> + */ >>>>> +int libdw__addr2line(const char *dso_name, u64 addr, char **file, >>>>> + unsigned int *line_nr, struct dso *dso, >>>>> + bool unwind_inlines, struct inline_node *node, >>>>> + struct symbol *sym); >>>>> + >>>>> +/* >>>>> + * dso__free_a2l_libdw - Free libdw resources associated with the DSO >>>>> + * @dso: The dso to free resources for >>>>> + * >>>>> + * This function cleans up the Dwfl context used for addr2line lookups. >>>>> + */ >>>>> +void dso__free_a2l_libdw(struct dso *dso); >>>>> + >>>>> +#else /* HAVE_LIBDW_SUPPORT */ >>>>> + >>>>> +static inline int libdw__addr2line(const char *dso_name __maybe_unused, >>>>> + u64 addr __maybe_unused, char **file __maybe_unused, >>>>> + unsigned int *line_nr __maybe_unused, >>>>> + struct dso *dso __maybe_unused, >>>>> + bool unwind_inlines __maybe_unused, >>>>> + struct inline_node *node __maybe_unused, >>>>> + struct symbol *sym __maybe_unused) >>>>> +{ >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static inline void dso__free_a2l_libdw(struct dso *dso __maybe_unused) >>>>> +{ >>>>> +} >>>>> +#endif /* HAVE_LIBDW_SUPPORT */ >>>>> + >>>>> +#endif /* PERF_LIBDW_H */ >>>>> diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c >>>>> index 27c0966611ab..4b456c4d4138 100644 >>>>> --- a/tools/perf/util/srcline.c >>>>> +++ b/tools/perf/util/srcline.c >>>>> @@ -6,6 +6,7 @@ >>>>> #include "libbfd.h" >>>>> #include "llvm.h" >>>>> #include "symbol.h" >>>>> +#include "libdw.h" >>>>> >>>>> #include <inttypes.h> >>>>> #include <string.h> >>>>> @@ -120,6 +121,10 @@ static int addr2line(const char *dso_name, u64 addr, char **file, unsigned int * >>>>> { >>>>> int ret; >>>>> >>>>> + ret = libdw__addr2line(dso_name, addr, file, line_nr, dso, unwind_inlines, node, sym); >>>>> + if (ret > 0) >>>>> + return ret; >>>>> + >>>>> ret = llvm__addr2line(dso_name, addr, file, line_nr, dso, unwind_inlines, node, sym); >>>>> if (ret > 0) >>>>> return ret; >>>>> -- >>>>> 2.52.0.rc2.455.g230fcf2819-goog >>>>> >> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1] perf addr2line: Add a libdw implementation 2025-11-27 13:48 ` James Clark @ 2025-11-28 9:00 ` Ian Rogers 2025-11-28 14:25 ` James Clark 0 siblings, 1 reply; 13+ messages in thread From: Ian Rogers @ 2025-11-28 9:00 UTC (permalink / raw) To: James Clark Cc: Namhyung Kim, Tony Jones, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa, Adrian Hunter, Howard Chu, Stephen Brennan, linux-kernel, linux-perf-users On Thu, Nov 27, 2025 at 5:48 AM James Clark <james.clark@linaro.org> wrote: > > > > On 27/11/2025 1:19 pm, Ian Rogers wrote: > > On Thu, Nov 27, 2025 at 4:16 AM James Clark <james.clark@linaro.org> wrote: > >> > >> > >> > >> On 27/11/2025 11:43 am, Ian Rogers wrote: > >>> On Wed, Nov 26, 2025 at 10:27 AM Namhyung Kim <namhyung@kernel.org> wrote: > >>>> > >>>> On Sat, Nov 22, 2025 at 01:39:34AM -0800, Ian Rogers wrote: > >>>>> Add an implementation of addr2line that uses libdw. Other addr2line > >>>>> implementations or, in the case of forking addr2line, slow. Add an > >>>>> implementation that caches the libdw information in the dso and uses > >>>>> it to find the file and line number information. > >>> > >>> Thanks James and Namhyung for the reviews! I agree with James' comment > >>> about a typo in the commit message. > >>> > >>>> My concern is the limit in the open file descriptors in case the data > >>>> touched a lot of different libraries. The DSO code has some logic to > >>>> deal with it but I'm not sure if we can share that since libdw seems to > >>>> want to own the FD. > >>> > >>> The code opens the FD: > >>> > >>> + fd = open(dso_name, O_RDONLY); > >>> + if (fd < 0) > >>> + return 0; > >>> + > >>> + dwfl = dwfl_begin(&offline_callbacks); > >>> + if (!dwfl) { > >>> + close(fd); > >>> + return 0; > >>> + } > >>> > >>> It then uses the FD and closes it (the close is hidden in libdw itself): > >>> > >>> + /* > >>> + * If the report is successful, the file descriptor fd > >>> is consumed > >>> + * and closed by the Dwfl. If not, it is not closed. > >>> + */ > >>> + mod = dwfl_report_offline(dwfl, dso_name, dso_name, fd); > >>> > >>> So it is possible we exhaust all the file descriptors if there are > >>> multiple concurrent calls to libdw__addr2line and every dso has > >>> missing libdw dwfl data... but because the open/close are close > >>> together and that 1 FD is say small to the FDs needed for the > >>> cmd__addr2line, I don't think it is a problem we need to specifically > >>> handle. Were the FD kept open until the dso was deleted, I'd agree > >>> with you. > >>> > >>>> Also, have you checked if this generates the exactly same output with > >>>> other implementations? > >>> > >>> So the code passes `perf test` and I was checking functionality with > >>> perf annotate, top, etc. What I saw looked good, but it may not have > >>> been exhaustive. I didn't specifically create a test that compares the > >>> output of the different addr2line implementations. Such a test would > >>> be possible, it's not something we've done elsewhere. > >>> > >>> Thanks, > >>> Ian > >>> > >> > >> I manually looked at a couple of line numbers and they looked > >> reasonable. I think an automated test that compared dwarf decoders would > >> be a bit of a nightmare because I'm sure there would always be subtle > >> differences. > >> > >> Doing a manual side by side comparison of libdw__addr2line() and > >> llvm__addr2line(), they seem to be quite different: > >> > >> $ perf record -- perf test -w leafloop > >> $ perf script -F ip,srcline > libdw_addr2line.txt > >> # Comment out libdw_addr2line() and rebuild > >> $ perf script -F ip,srcline > llvm_addr2line.txt > >> $ diff libdw_addr2line.txt llvm_addr2line.txt > >> > >> It gets all of the simple leafloop workload lines the same, but lots of > >> the libc stuff is different. > >> > >> For example libdw gives cpu-features.c:350 where llvm gives strcmp.S. At > >> least that one looks like inlining so llvm might be "better", but where > >> it gives dl-cache.c:490 isntead of llvm's mmap64.c:47, it's hard to see > >> how that can be inlining if they're in different compilation units. > >> > >> If the llvm addr2line implementation is also supposed to be slow, it > >> just means we're trading speed with accuracy with this change. Hard to > >> say what the default should be in that case. > > > > Agreed. We could do some kind of option scheme like with the disassemblers: > > https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/disasm.c?h=perf-tools-next#n1715 > > > > I'm not sure there are currently any distributions using the LLVM > > options, this in part motivates: > > https://lore.kernel.org/lkml/20251007163835.3152881-1-irogers@google.com/ > > but the addr2line APIs are a pain there - see the extra generated .so. > > Because of that it could be reasonable to delete the LLVM addr2line > > code and just focus on libdw. I kind of wish we could also delete the > > precarious command forking versions too. And libbfd... Perhaps with > > this code, those advocating libbfd have a fast enough alternative > > without bringing in large dependencies like libLLVM. > > > > Thanks, > > Ian > > > > Personally for debugging I would want the most accurate and detailed > information possible and don't care at all about the size of > dependencies. If Perf is a tool for debugging then surely that should be > the default? But right now I don't actually know which of the outputs is > most accurate although I'll assume that the llvm one is. I know LLVM got dwarf 5 support early. I'm not sure if accurate is the best word to compare libdw and LLVM as, if libdw supports the format, discrepancies are bugs in LLVM or libdw, .. Building around LLVM as a common base sgtm and my main use-case doesn't get impeded by the large dependency because of the dlopen patches. As libelf/libdw is pretty much a given for a perf build, as it is required for libbpf, then the code here can avoid the fork/exec fallback. I can see the sense in trying with LLVM first and then falling back to libdw. > Wanting the smallest application possible while trading away detail and > accuracy seems like an unusual special case that should be opt in. It > also seems to relate to this old discussion [1], in that if we're going > to keep the llvm depencency then it doesn't make sense to re-write > anything in Perf. So I think PE support in libdw wouldn't be a priority and so LLVM for the win there. I think distributions have been slow to pick up the LLVM dependency, I'm not sure what the issue is there. It would be nice to start removing some of the alternatives the perf build supports. The libbfd and libunwind code is only built with specific opt-ins, and I think the fork/exec addr2line is redundant after this change. For addr2line that'd bring the 2 implementations down to libdw and LLVM. As the recent libbfd fixes show [1], inadvertently we can break these largely untested dependencies by exposing latent bugs in things like improper initialization. Thanks, Ian [1] https://lore.kernel.org/lkml/20251112074311.1440101-1-irogers@google.com/ > [1]: > https://lore.kernel.org/linux-perf-users/549d3812-a606-4981-83f5-0a99b0ff9f6a@linaro.org/ > > >>>> Thanks, > >>>> Namhyung > >>>> > >>>>> > >>>>> Signed-off-by: Ian Rogers <irogers@google.com> > >>>>> --- > >>>>> tools/perf/util/Build | 1 + > >>>>> tools/perf/util/dso.c | 2 + > >>>>> tools/perf/util/dso.h | 11 +++ > >>>>> tools/perf/util/libdw.c | 136 ++++++++++++++++++++++++++++++++++++++ > >>>>> tools/perf/util/libdw.h | 60 +++++++++++++++++ > >>>>> tools/perf/util/srcline.c | 5 ++ > >>>>> 6 files changed, 215 insertions(+) > >>>>> create mode 100644 tools/perf/util/libdw.c > >>>>> create mode 100644 tools/perf/util/libdw.h > >>>>> > >>>>> diff --git a/tools/perf/util/Build b/tools/perf/util/Build > >>>>> index 1c2a43e1dc68..2bed6274e248 100644 > >>>>> --- a/tools/perf/util/Build > >>>>> +++ b/tools/perf/util/Build > >>>>> @@ -224,6 +224,7 @@ perf-util-$(CONFIG_LIBDW) += dwarf-regs-powerpc.o > >>>>> perf-util-$(CONFIG_LIBDW) += dwarf-regs-x86.o > >>>>> perf-util-$(CONFIG_LIBDW) += debuginfo.o > >>>>> perf-util-$(CONFIG_LIBDW) += annotate-data.o > >>>>> +perf-util-$(CONFIG_LIBDW) += libdw.o > >>>>> > >>>>> perf-util-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o > >>>>> perf-util-$(CONFIG_LOCAL_LIBUNWIND) += unwind-libunwind-local.o > >>>>> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c > >>>>> index 344e689567ee..06980844c014 100644 > >>>>> --- a/tools/perf/util/dso.c > >>>>> +++ b/tools/perf/util/dso.c > >>>>> @@ -32,6 +32,7 @@ > >>>>> #include "string2.h" > >>>>> #include "vdso.h" > >>>>> #include "annotate-data.h" > >>>>> +#include "libdw.h" > >>>>> > >>>>> static const char * const debuglink_paths[] = { > >>>>> "%.0s%s", > >>>>> @@ -1605,6 +1606,7 @@ void dso__delete(struct dso *dso) > >>>>> auxtrace_cache__free(RC_CHK_ACCESS(dso)->auxtrace_cache); > >>>>> dso_cache__free(dso); > >>>>> dso__free_a2l(dso); > >>>>> + dso__free_a2l_libdw(dso); > >>>>> dso__free_symsrc_filename(dso); > >>>>> nsinfo__zput(RC_CHK_ACCESS(dso)->nsinfo); > >>>>> mutex_destroy(dso__lock(dso)); > >>>>> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h > >>>>> index f8ccb9816b89..4aee23775054 100644 > >>>>> --- a/tools/perf/util/dso.h > >>>>> +++ b/tools/perf/util/dso.h > >>>>> @@ -268,6 +268,7 @@ DECLARE_RC_STRUCT(dso) { > >>>>> const char *short_name; > >>>>> const char *long_name; > >>>>> void *a2l; > >>>>> + void *a2l_libdw; > >>>>> char *symsrc_filename; > >>>>> #if defined(__powerpc__) > >>>>> void *dwfl; /* DWARF debug info */ > >>>>> @@ -334,6 +335,16 @@ static inline void dso__set_a2l(struct dso *dso, void *val) > >>>>> RC_CHK_ACCESS(dso)->a2l = val; > >>>>> } > >>>>> > >>>>> +static inline void *dso__a2l_libdw(const struct dso *dso) > >>>>> +{ > >>>>> + return RC_CHK_ACCESS(dso)->a2l_libdw; > >>>>> +} > >>>>> + > >>>>> +static inline void dso__set_a2l_libdw(struct dso *dso, void *val) > >>>>> +{ > >>>>> + RC_CHK_ACCESS(dso)->a2l_libdw = val; > >>>>> +} > >>>>> + > >>>>> static inline unsigned int dso__a2l_fails(const struct dso *dso) > >>>>> { > >>>>> return RC_CHK_ACCESS(dso)->a2l_fails; > >>>>> diff --git a/tools/perf/util/libdw.c b/tools/perf/util/libdw.c > >>>>> new file mode 100644 > >>>>> index 000000000000..c4331fa8e1a3 > >>>>> --- /dev/null > >>>>> +++ b/tools/perf/util/libdw.c > >>>>> @@ -0,0 +1,136 @@ > >>>>> +// SPDX-License-Identifier: GPL-2.0 > >>>>> +#include "dso.h" > >>>>> +#include "libdw.h" > >>>>> +#include "srcline.h" > >>>>> +#include "symbol.h" > >>>>> +#include "dwarf-aux.h" > >>>>> +#include <fcntl.h> > >>>>> +#include <unistd.h> > >>>>> +#include <elfutils/libdwfl.h> > >>>>> + > >>>>> +void dso__free_a2l_libdw(struct dso *dso) > >>>>> +{ > >>>>> + Dwfl *dwfl = dso__a2l_libdw(dso); > >>>>> + > >>>>> + if (dwfl) { > >>>>> + dwfl_end(dwfl); > >>>>> + dso__set_a2l_libdw(dso, NULL); > >>>>> + } > >>>>> +} > >>>>> + > >>>>> +int libdw__addr2line(const char *dso_name, u64 addr, > >>>>> + char **file, unsigned int *line_nr, > >>>>> + struct dso *dso, bool unwind_inlines, > >>>>> + struct inline_node *node, struct symbol *sym) > >>>>> +{ > >>>>> + static const Dwfl_Callbacks offline_callbacks = { > >>>>> + .find_debuginfo = dwfl_standard_find_debuginfo, > >>>>> + .section_address = dwfl_offline_section_address, > >>>>> + .find_elf = dwfl_build_id_find_elf, > >>>>> + }; > >>>>> + Dwfl *dwfl = dso__a2l_libdw(dso); > >>>>> + Dwfl_Module *mod; > >>>>> + Dwfl_Line *dwline; > >>>>> + Dwarf_Addr bias; > >>>>> + const char *src; > >>>>> + int lineno; > >>>>> + > >>>>> + if (!dwfl) { > >>>>> + /* > >>>>> + * Initialize Dwfl session. > >>>>> + * We need to open the DSO file to report it to libdw. > >>>>> + */ > >>>>> + int fd; > >>>>> + > >>>>> + fd = open(dso_name, O_RDONLY); > >>>>> + if (fd < 0) > >>>>> + return 0; > >>>>> + > >>>>> + dwfl = dwfl_begin(&offline_callbacks); > >>>>> + if (!dwfl) { > >>>>> + close(fd); > >>>>> + return 0; > >>>>> + } > >>>>> + > >>>>> + /* > >>>>> + * If the report is successful, the file descriptor fd is consumed > >>>>> + * and closed by the Dwfl. If not, it is not closed. > >>>>> + */ > >>>>> + mod = dwfl_report_offline(dwfl, dso_name, dso_name, fd); > >>>>> + if (!mod) { > >>>>> + dwfl_end(dwfl); > >>>>> + close(fd); > >>>>> + return 0; > >>>>> + } > >>>>> + > >>>>> + dwfl_report_end(dwfl, /*removed=*/NULL, /*arg=*/NULL); > >>>>> + dso__set_a2l_libdw(dso, dwfl); > >>>>> + } else { > >>>>> + /* Dwfl session already initialized, get module for address. */ > >>>>> + mod = dwfl_addrmodule(dwfl, addr); > >>>>> + } > >>>>> + > >>>>> + if (!mod) > >>>>> + return 0; > >>>>> + > >>>>> + /* Find source line information for the address. */ > >>>>> + dwline = dwfl_module_getsrc(mod, addr); > >>>>> + if (!dwline) > >>>>> + return 0; > >>>>> + > >>>>> + /* Get line information. */ > >>>>> + src = dwfl_lineinfo(dwline, &addr, &lineno, /*col=*/NULL, /*mtime=*/NULL, /*length=*/NULL); > >>>>> + > >>>>> + if (file) > >>>>> + *file = src ? strdup(src) : NULL; > >>>>> + if (line_nr) > >>>>> + *line_nr = lineno; > >>>>> + > >>>>> + /* Optionally unwind inline function call chain. */ > >>>>> + if (unwind_inlines && node && src) { > >>>>> + Dwarf_Die *cudie = dwfl_module_addrdie(mod, addr, &bias); > >>>>> + Dwarf_Die *scopes = NULL; > >>>>> + int nscopes; > >>>>> + > >>>>> + if (!cudie) > >>>>> + return 1; > >>>>> + > >>>>> + nscopes = die_get_scopes(cudie, addr, &scopes); > >>>>> + if (nscopes > 0) { > >>>>> + int i; > >>>>> + const char *call_file = src; > >>>>> + unsigned int call_line = lineno; > >>>>> + > >>>>> + for (i = 0; i < nscopes; i++) { > >>>>> + Dwarf_Die *die = &scopes[i]; > >>>>> + struct symbol *inline_sym; > >>>>> + char *srcline = NULL; > >>>>> + int tag = dwarf_tag(die); > >>>>> + > >>>>> + /* We are interested in inlined subroutines. */ > >>>>> + if (tag != DW_TAG_inlined_subroutine && > >>>>> + tag != DW_TAG_subprogram) > >>>>> + continue; > >>>>> + > >>>>> + inline_sym = new_inline_sym(dso, sym, dwarf_diename(die)); > >>>>> + > >>>>> + if (call_file) > >>>>> + srcline = srcline_from_fileline(call_file, call_line); > >>>>> + > >>>>> + inline_list__append(inline_sym, srcline, node); > >>>>> + > >>>>> + /* Update call site for next level. */ > >>>>> + if (tag == DW_TAG_inlined_subroutine) { > >>>>> + call_file = die_get_call_file(die); > >>>>> + call_line = die_get_call_lineno(die); > >>>>> + } else { > >>>>> + /* Reached the root subprogram. */ > >>>>> + break; > >>>>> + } > >>>>> + } > >>>>> + free(scopes); > >>>>> + } > >>>>> + } > >>>>> + > >>>>> + return 1; > >>>>> +} > >>>>> diff --git a/tools/perf/util/libdw.h b/tools/perf/util/libdw.h > >>>>> new file mode 100644 > >>>>> index 000000000000..0f8d7b4a11a5 > >>>>> --- /dev/null > >>>>> +++ b/tools/perf/util/libdw.h > >>>>> @@ -0,0 +1,60 @@ > >>>>> +/* SPDX-License-Identifier: GPL-2.0 */ > >>>>> +#ifndef PERF_LIBDW_H > >>>>> +#define PERF_LIBDW_H > >>>>> + > >>>>> +#include <linux/types.h> > >>>>> + > >>>>> +struct dso; > >>>>> +struct inline_node; > >>>>> +struct symbol; > >>>>> + > >>>>> +#ifdef HAVE_LIBDW_SUPPORT > >>>>> +/* > >>>>> + * libdw__addr2line - Convert address to source location using libdw > >>>>> + * @dso_name: Name of the DSO > >>>>> + * @addr: Address to resolve > >>>>> + * @file: Pointer to return filename (caller must free) > >>>>> + * @line_nr: Pointer to return line number > >>>>> + * @dso: The dso struct > >>>>> + * @unwind_inlines: Whether to unwind inline function calls > >>>>> + * @node: Inline node list to append to > >>>>> + * @sym: The symbol associated with the address > >>>>> + * > >>>>> + * This function initializes a Dwfl context for the DSO if not already present, > >>>>> + * finds the source line information for the given address, and optionally > >>>>> + * resolves inline function call chains. > >>>>> + * > >>>>> + * Returns 1 on success (found), 0 on failure (not found). > >>>>> + */ > >>>>> +int libdw__addr2line(const char *dso_name, u64 addr, char **file, > >>>>> + unsigned int *line_nr, struct dso *dso, > >>>>> + bool unwind_inlines, struct inline_node *node, > >>>>> + struct symbol *sym); > >>>>> + > >>>>> +/* > >>>>> + * dso__free_a2l_libdw - Free libdw resources associated with the DSO > >>>>> + * @dso: The dso to free resources for > >>>>> + * > >>>>> + * This function cleans up the Dwfl context used for addr2line lookups. > >>>>> + */ > >>>>> +void dso__free_a2l_libdw(struct dso *dso); > >>>>> + > >>>>> +#else /* HAVE_LIBDW_SUPPORT */ > >>>>> + > >>>>> +static inline int libdw__addr2line(const char *dso_name __maybe_unused, > >>>>> + u64 addr __maybe_unused, char **file __maybe_unused, > >>>>> + unsigned int *line_nr __maybe_unused, > >>>>> + struct dso *dso __maybe_unused, > >>>>> + bool unwind_inlines __maybe_unused, > >>>>> + struct inline_node *node __maybe_unused, > >>>>> + struct symbol *sym __maybe_unused) > >>>>> +{ > >>>>> + return 0; > >>>>> +} > >>>>> + > >>>>> +static inline void dso__free_a2l_libdw(struct dso *dso __maybe_unused) > >>>>> +{ > >>>>> +} > >>>>> +#endif /* HAVE_LIBDW_SUPPORT */ > >>>>> + > >>>>> +#endif /* PERF_LIBDW_H */ > >>>>> diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c > >>>>> index 27c0966611ab..4b456c4d4138 100644 > >>>>> --- a/tools/perf/util/srcline.c > >>>>> +++ b/tools/perf/util/srcline.c > >>>>> @@ -6,6 +6,7 @@ > >>>>> #include "libbfd.h" > >>>>> #include "llvm.h" > >>>>> #include "symbol.h" > >>>>> +#include "libdw.h" > >>>>> > >>>>> #include <inttypes.h> > >>>>> #include <string.h> > >>>>> @@ -120,6 +121,10 @@ static int addr2line(const char *dso_name, u64 addr, char **file, unsigned int * > >>>>> { > >>>>> int ret; > >>>>> > >>>>> + ret = libdw__addr2line(dso_name, addr, file, line_nr, dso, unwind_inlines, node, sym); > >>>>> + if (ret > 0) > >>>>> + return ret; > >>>>> + > >>>>> ret = llvm__addr2line(dso_name, addr, file, line_nr, dso, unwind_inlines, node, sym); > >>>>> if (ret > 0) > >>>>> return ret; > >>>>> -- > >>>>> 2.52.0.rc2.455.g230fcf2819-goog > >>>>> > >> > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1] perf addr2line: Add a libdw implementation 2025-11-28 9:00 ` Ian Rogers @ 2025-11-28 14:25 ` James Clark 0 siblings, 0 replies; 13+ messages in thread From: James Clark @ 2025-11-28 14:25 UTC (permalink / raw) To: Ian Rogers Cc: Namhyung Kim, Tony Jones, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa, Adrian Hunter, Howard Chu, Stephen Brennan, linux-kernel, linux-perf-users On 28/11/2025 9:00 am, Ian Rogers wrote: > On Thu, Nov 27, 2025 at 5:48 AM James Clark <james.clark@linaro.org> wrote: >> >> >> >> On 27/11/2025 1:19 pm, Ian Rogers wrote: >>> On Thu, Nov 27, 2025 at 4:16 AM James Clark <james.clark@linaro.org> wrote: >>>> >>>> >>>> >>>> On 27/11/2025 11:43 am, Ian Rogers wrote: >>>>> On Wed, Nov 26, 2025 at 10:27 AM Namhyung Kim <namhyung@kernel.org> wrote: >>>>>> >>>>>> On Sat, Nov 22, 2025 at 01:39:34AM -0800, Ian Rogers wrote: >>>>>>> Add an implementation of addr2line that uses libdw. Other addr2line >>>>>>> implementations or, in the case of forking addr2line, slow. Add an >>>>>>> implementation that caches the libdw information in the dso and uses >>>>>>> it to find the file and line number information. >>>>> >>>>> Thanks James and Namhyung for the reviews! I agree with James' comment >>>>> about a typo in the commit message. >>>>> >>>>>> My concern is the limit in the open file descriptors in case the data >>>>>> touched a lot of different libraries. The DSO code has some logic to >>>>>> deal with it but I'm not sure if we can share that since libdw seems to >>>>>> want to own the FD. >>>>> >>>>> The code opens the FD: >>>>> >>>>> + fd = open(dso_name, O_RDONLY); >>>>> + if (fd < 0) >>>>> + return 0; >>>>> + >>>>> + dwfl = dwfl_begin(&offline_callbacks); >>>>> + if (!dwfl) { >>>>> + close(fd); >>>>> + return 0; >>>>> + } >>>>> >>>>> It then uses the FD and closes it (the close is hidden in libdw itself): >>>>> >>>>> + /* >>>>> + * If the report is successful, the file descriptor fd >>>>> is consumed >>>>> + * and closed by the Dwfl. If not, it is not closed. >>>>> + */ >>>>> + mod = dwfl_report_offline(dwfl, dso_name, dso_name, fd); >>>>> >>>>> So it is possible we exhaust all the file descriptors if there are >>>>> multiple concurrent calls to libdw__addr2line and every dso has >>>>> missing libdw dwfl data... but because the open/close are close >>>>> together and that 1 FD is say small to the FDs needed for the >>>>> cmd__addr2line, I don't think it is a problem we need to specifically >>>>> handle. Were the FD kept open until the dso was deleted, I'd agree >>>>> with you. >>>>> >>>>>> Also, have you checked if this generates the exactly same output with >>>>>> other implementations? >>>>> >>>>> So the code passes `perf test` and I was checking functionality with >>>>> perf annotate, top, etc. What I saw looked good, but it may not have >>>>> been exhaustive. I didn't specifically create a test that compares the >>>>> output of the different addr2line implementations. Such a test would >>>>> be possible, it's not something we've done elsewhere. >>>>> >>>>> Thanks, >>>>> Ian >>>>> >>>> >>>> I manually looked at a couple of line numbers and they looked >>>> reasonable. I think an automated test that compared dwarf decoders would >>>> be a bit of a nightmare because I'm sure there would always be subtle >>>> differences. >>>> >>>> Doing a manual side by side comparison of libdw__addr2line() and >>>> llvm__addr2line(), they seem to be quite different: >>>> >>>> $ perf record -- perf test -w leafloop >>>> $ perf script -F ip,srcline > libdw_addr2line.txt >>>> # Comment out libdw_addr2line() and rebuild >>>> $ perf script -F ip,srcline > llvm_addr2line.txt >>>> $ diff libdw_addr2line.txt llvm_addr2line.txt >>>> >>>> It gets all of the simple leafloop workload lines the same, but lots of >>>> the libc stuff is different. >>>> >>>> For example libdw gives cpu-features.c:350 where llvm gives strcmp.S. At >>>> least that one looks like inlining so llvm might be "better", but where >>>> it gives dl-cache.c:490 isntead of llvm's mmap64.c:47, it's hard to see >>>> how that can be inlining if they're in different compilation units. >>>> >>>> If the llvm addr2line implementation is also supposed to be slow, it >>>> just means we're trading speed with accuracy with this change. Hard to >>>> say what the default should be in that case. >>> >>> Agreed. We could do some kind of option scheme like with the disassemblers: >>> https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/disasm.c?h=perf-tools-next#n1715 >>> >>> I'm not sure there are currently any distributions using the LLVM >>> options, this in part motivates: >>> https://lore.kernel.org/lkml/20251007163835.3152881-1-irogers@google.com/ >>> but the addr2line APIs are a pain there - see the extra generated .so. >>> Because of that it could be reasonable to delete the LLVM addr2line >>> code and just focus on libdw. I kind of wish we could also delete the >>> precarious command forking versions too. And libbfd... Perhaps with >>> this code, those advocating libbfd have a fast enough alternative >>> without bringing in large dependencies like libLLVM. >>> >>> Thanks, >>> Ian >>> >> >> Personally for debugging I would want the most accurate and detailed >> information possible and don't care at all about the size of >> dependencies. If Perf is a tool for debugging then surely that should be >> the default? But right now I don't actually know which of the outputs is >> most accurate although I'll assume that the llvm one is. > > I know LLVM got dwarf 5 support early. I'm not sure if accurate is the > best word to compare libdw and LLVM as, if libdw supports the format, > discrepancies are bugs in LLVM or libdw, .. Building around LLVM as a > common base sgtm and my main use-case doesn't get impeded by the large > dependency because of the dlopen patches. As libelf/libdw is pretty > much a given for a perf build, as it is required for libbpf, then the > code here can avoid the fork/exec fallback. I can see the sense in > trying with LLVM first and then falling back to libdw. > Seems like my libc does have dwarf 5 symbols. Looking a bit more I still think "accurate" is the right word. Comparing gdb, forked addr2line and llvm_addr2line(), all 3 agree on the source line for every address in my recording. The only one that disagrees is libdw_addr2line(). I don't know if there's an issue with how it's being called, or there is a bug in the library. Digging even deeper, if I put breakpoints on the lines that libdw_addr2line() gives, they aren't even hit. So it's not even like it's not de-inlining stuff, which you could argue is fine. And there is the issue that some of the results are in completely different C files. It's hard to see why it's giving such different results without debugging libdw line by line. I'm not sure if you see the same thing with a simple recording or you want me to send a perf archive? >> Wanting the smallest application possible while trading away detail and >> accuracy seems like an unusual special case that should be opt in. It >> also seems to relate to this old discussion [1], in that if we're going >> to keep the llvm depencency then it doesn't make sense to re-write >> anything in Perf. > > So I think PE support in libdw wouldn't be a priority and so LLVM for > the win there. I think distributions have been slow to pick up the > LLVM dependency, I'm not sure what the issue is there. It would be > nice to start removing some of the alternatives the perf build > supports. The libbfd and libunwind code is only built with specific > opt-ins, and I think the fork/exec addr2line is redundant after this > change. For addr2line that'd bring the 2 implementations down to libdw It might be a bit premature to drop the addr2line fork if libdw isn't mature enough. Especially as it seems to fall over on such a basic test. Hopefully it's just an issue with how it's called or the version that I have. > and LLVM. As the recent libbfd fixes show [1], inadvertently we can > break these largely untested dependencies by exposing latent bugs in > things like improper initialization. > > Thanks, > Ian > > [1] https://lore.kernel.org/lkml/20251112074311.1440101-1-irogers@google.com/ > >> [1]: >> https://lore.kernel.org/linux-perf-users/549d3812-a606-4981-83f5-0a99b0ff9f6a@linaro.org/ >> >>>>>> Thanks, >>>>>> Namhyung >>>>>> >>>>>>> >>>>>>> Signed-off-by: Ian Rogers <irogers@google.com> >>>>>>> --- >>>>>>> tools/perf/util/Build | 1 + >>>>>>> tools/perf/util/dso.c | 2 + >>>>>>> tools/perf/util/dso.h | 11 +++ >>>>>>> tools/perf/util/libdw.c | 136 ++++++++++++++++++++++++++++++++++++++ >>>>>>> tools/perf/util/libdw.h | 60 +++++++++++++++++ >>>>>>> tools/perf/util/srcline.c | 5 ++ >>>>>>> 6 files changed, 215 insertions(+) >>>>>>> create mode 100644 tools/perf/util/libdw.c >>>>>>> create mode 100644 tools/perf/util/libdw.h >>>>>>> >>>>>>> diff --git a/tools/perf/util/Build b/tools/perf/util/Build >>>>>>> index 1c2a43e1dc68..2bed6274e248 100644 >>>>>>> --- a/tools/perf/util/Build >>>>>>> +++ b/tools/perf/util/Build >>>>>>> @@ -224,6 +224,7 @@ perf-util-$(CONFIG_LIBDW) += dwarf-regs-powerpc.o >>>>>>> perf-util-$(CONFIG_LIBDW) += dwarf-regs-x86.o >>>>>>> perf-util-$(CONFIG_LIBDW) += debuginfo.o >>>>>>> perf-util-$(CONFIG_LIBDW) += annotate-data.o >>>>>>> +perf-util-$(CONFIG_LIBDW) += libdw.o >>>>>>> >>>>>>> perf-util-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o >>>>>>> perf-util-$(CONFIG_LOCAL_LIBUNWIND) += unwind-libunwind-local.o >>>>>>> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c >>>>>>> index 344e689567ee..06980844c014 100644 >>>>>>> --- a/tools/perf/util/dso.c >>>>>>> +++ b/tools/perf/util/dso.c >>>>>>> @@ -32,6 +32,7 @@ >>>>>>> #include "string2.h" >>>>>>> #include "vdso.h" >>>>>>> #include "annotate-data.h" >>>>>>> +#include "libdw.h" >>>>>>> >>>>>>> static const char * const debuglink_paths[] = { >>>>>>> "%.0s%s", >>>>>>> @@ -1605,6 +1606,7 @@ void dso__delete(struct dso *dso) >>>>>>> auxtrace_cache__free(RC_CHK_ACCESS(dso)->auxtrace_cache); >>>>>>> dso_cache__free(dso); >>>>>>> dso__free_a2l(dso); >>>>>>> + dso__free_a2l_libdw(dso); >>>>>>> dso__free_symsrc_filename(dso); >>>>>>> nsinfo__zput(RC_CHK_ACCESS(dso)->nsinfo); >>>>>>> mutex_destroy(dso__lock(dso)); >>>>>>> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h >>>>>>> index f8ccb9816b89..4aee23775054 100644 >>>>>>> --- a/tools/perf/util/dso.h >>>>>>> +++ b/tools/perf/util/dso.h >>>>>>> @@ -268,6 +268,7 @@ DECLARE_RC_STRUCT(dso) { >>>>>>> const char *short_name; >>>>>>> const char *long_name; >>>>>>> void *a2l; >>>>>>> + void *a2l_libdw; >>>>>>> char *symsrc_filename; >>>>>>> #if defined(__powerpc__) >>>>>>> void *dwfl; /* DWARF debug info */ >>>>>>> @@ -334,6 +335,16 @@ static inline void dso__set_a2l(struct dso *dso, void *val) >>>>>>> RC_CHK_ACCESS(dso)->a2l = val; >>>>>>> } >>>>>>> >>>>>>> +static inline void *dso__a2l_libdw(const struct dso *dso) >>>>>>> +{ >>>>>>> + return RC_CHK_ACCESS(dso)->a2l_libdw; >>>>>>> +} >>>>>>> + >>>>>>> +static inline void dso__set_a2l_libdw(struct dso *dso, void *val) >>>>>>> +{ >>>>>>> + RC_CHK_ACCESS(dso)->a2l_libdw = val; >>>>>>> +} >>>>>>> + >>>>>>> static inline unsigned int dso__a2l_fails(const struct dso *dso) >>>>>>> { >>>>>>> return RC_CHK_ACCESS(dso)->a2l_fails; >>>>>>> diff --git a/tools/perf/util/libdw.c b/tools/perf/util/libdw.c >>>>>>> new file mode 100644 >>>>>>> index 000000000000..c4331fa8e1a3 >>>>>>> --- /dev/null >>>>>>> +++ b/tools/perf/util/libdw.c >>>>>>> @@ -0,0 +1,136 @@ >>>>>>> +// SPDX-License-Identifier: GPL-2.0 >>>>>>> +#include "dso.h" >>>>>>> +#include "libdw.h" >>>>>>> +#include "srcline.h" >>>>>>> +#include "symbol.h" >>>>>>> +#include "dwarf-aux.h" >>>>>>> +#include <fcntl.h> >>>>>>> +#include <unistd.h> >>>>>>> +#include <elfutils/libdwfl.h> >>>>>>> + >>>>>>> +void dso__free_a2l_libdw(struct dso *dso) >>>>>>> +{ >>>>>>> + Dwfl *dwfl = dso__a2l_libdw(dso); >>>>>>> + >>>>>>> + if (dwfl) { >>>>>>> + dwfl_end(dwfl); >>>>>>> + dso__set_a2l_libdw(dso, NULL); >>>>>>> + } >>>>>>> +} >>>>>>> + >>>>>>> +int libdw__addr2line(const char *dso_name, u64 addr, >>>>>>> + char **file, unsigned int *line_nr, >>>>>>> + struct dso *dso, bool unwind_inlines, >>>>>>> + struct inline_node *node, struct symbol *sym) >>>>>>> +{ >>>>>>> + static const Dwfl_Callbacks offline_callbacks = { >>>>>>> + .find_debuginfo = dwfl_standard_find_debuginfo, >>>>>>> + .section_address = dwfl_offline_section_address, >>>>>>> + .find_elf = dwfl_build_id_find_elf, >>>>>>> + }; >>>>>>> + Dwfl *dwfl = dso__a2l_libdw(dso); >>>>>>> + Dwfl_Module *mod; >>>>>>> + Dwfl_Line *dwline; >>>>>>> + Dwarf_Addr bias; >>>>>>> + const char *src; >>>>>>> + int lineno; >>>>>>> + >>>>>>> + if (!dwfl) { >>>>>>> + /* >>>>>>> + * Initialize Dwfl session. >>>>>>> + * We need to open the DSO file to report it to libdw. >>>>>>> + */ >>>>>>> + int fd; >>>>>>> + >>>>>>> + fd = open(dso_name, O_RDONLY); >>>>>>> + if (fd < 0) >>>>>>> + return 0; >>>>>>> + >>>>>>> + dwfl = dwfl_begin(&offline_callbacks); >>>>>>> + if (!dwfl) { >>>>>>> + close(fd); >>>>>>> + return 0; >>>>>>> + } >>>>>>> + >>>>>>> + /* >>>>>>> + * If the report is successful, the file descriptor fd is consumed >>>>>>> + * and closed by the Dwfl. If not, it is not closed. >>>>>>> + */ >>>>>>> + mod = dwfl_report_offline(dwfl, dso_name, dso_name, fd); >>>>>>> + if (!mod) { >>>>>>> + dwfl_end(dwfl); >>>>>>> + close(fd); >>>>>>> + return 0; >>>>>>> + } >>>>>>> + >>>>>>> + dwfl_report_end(dwfl, /*removed=*/NULL, /*arg=*/NULL); >>>>>>> + dso__set_a2l_libdw(dso, dwfl); >>>>>>> + } else { >>>>>>> + /* Dwfl session already initialized, get module for address. */ >>>>>>> + mod = dwfl_addrmodule(dwfl, addr); >>>>>>> + } >>>>>>> + >>>>>>> + if (!mod) >>>>>>> + return 0; >>>>>>> + >>>>>>> + /* Find source line information for the address. */ >>>>>>> + dwline = dwfl_module_getsrc(mod, addr); >>>>>>> + if (!dwline) >>>>>>> + return 0; >>>>>>> + >>>>>>> + /* Get line information. */ >>>>>>> + src = dwfl_lineinfo(dwline, &addr, &lineno, /*col=*/NULL, /*mtime=*/NULL, /*length=*/NULL); >>>>>>> + >>>>>>> + if (file) >>>>>>> + *file = src ? strdup(src) : NULL; >>>>>>> + if (line_nr) >>>>>>> + *line_nr = lineno; >>>>>>> + >>>>>>> + /* Optionally unwind inline function call chain. */ >>>>>>> + if (unwind_inlines && node && src) { >>>>>>> + Dwarf_Die *cudie = dwfl_module_addrdie(mod, addr, &bias); >>>>>>> + Dwarf_Die *scopes = NULL; >>>>>>> + int nscopes; >>>>>>> + >>>>>>> + if (!cudie) >>>>>>> + return 1; >>>>>>> + >>>>>>> + nscopes = die_get_scopes(cudie, addr, &scopes); >>>>>>> + if (nscopes > 0) { >>>>>>> + int i; >>>>>>> + const char *call_file = src; >>>>>>> + unsigned int call_line = lineno; >>>>>>> + >>>>>>> + for (i = 0; i < nscopes; i++) { >>>>>>> + Dwarf_Die *die = &scopes[i]; >>>>>>> + struct symbol *inline_sym; >>>>>>> + char *srcline = NULL; >>>>>>> + int tag = dwarf_tag(die); >>>>>>> + >>>>>>> + /* We are interested in inlined subroutines. */ >>>>>>> + if (tag != DW_TAG_inlined_subroutine && >>>>>>> + tag != DW_TAG_subprogram) >>>>>>> + continue; >>>>>>> + >>>>>>> + inline_sym = new_inline_sym(dso, sym, dwarf_diename(die)); >>>>>>> + >>>>>>> + if (call_file) >>>>>>> + srcline = srcline_from_fileline(call_file, call_line); >>>>>>> + >>>>>>> + inline_list__append(inline_sym, srcline, node); >>>>>>> + >>>>>>> + /* Update call site for next level. */ >>>>>>> + if (tag == DW_TAG_inlined_subroutine) { >>>>>>> + call_file = die_get_call_file(die); >>>>>>> + call_line = die_get_call_lineno(die); >>>>>>> + } else { >>>>>>> + /* Reached the root subprogram. */ >>>>>>> + break; >>>>>>> + } >>>>>>> + } >>>>>>> + free(scopes); >>>>>>> + } >>>>>>> + } >>>>>>> + >>>>>>> + return 1; >>>>>>> +} >>>>>>> diff --git a/tools/perf/util/libdw.h b/tools/perf/util/libdw.h >>>>>>> new file mode 100644 >>>>>>> index 000000000000..0f8d7b4a11a5 >>>>>>> --- /dev/null >>>>>>> +++ b/tools/perf/util/libdw.h >>>>>>> @@ -0,0 +1,60 @@ >>>>>>> +/* SPDX-License-Identifier: GPL-2.0 */ >>>>>>> +#ifndef PERF_LIBDW_H >>>>>>> +#define PERF_LIBDW_H >>>>>>> + >>>>>>> +#include <linux/types.h> >>>>>>> + >>>>>>> +struct dso; >>>>>>> +struct inline_node; >>>>>>> +struct symbol; >>>>>>> + >>>>>>> +#ifdef HAVE_LIBDW_SUPPORT >>>>>>> +/* >>>>>>> + * libdw__addr2line - Convert address to source location using libdw >>>>>>> + * @dso_name: Name of the DSO >>>>>>> + * @addr: Address to resolve >>>>>>> + * @file: Pointer to return filename (caller must free) >>>>>>> + * @line_nr: Pointer to return line number >>>>>>> + * @dso: The dso struct >>>>>>> + * @unwind_inlines: Whether to unwind inline function calls >>>>>>> + * @node: Inline node list to append to >>>>>>> + * @sym: The symbol associated with the address >>>>>>> + * >>>>>>> + * This function initializes a Dwfl context for the DSO if not already present, >>>>>>> + * finds the source line information for the given address, and optionally >>>>>>> + * resolves inline function call chains. >>>>>>> + * >>>>>>> + * Returns 1 on success (found), 0 on failure (not found). >>>>>>> + */ >>>>>>> +int libdw__addr2line(const char *dso_name, u64 addr, char **file, >>>>>>> + unsigned int *line_nr, struct dso *dso, >>>>>>> + bool unwind_inlines, struct inline_node *node, >>>>>>> + struct symbol *sym); >>>>>>> + >>>>>>> +/* >>>>>>> + * dso__free_a2l_libdw - Free libdw resources associated with the DSO >>>>>>> + * @dso: The dso to free resources for >>>>>>> + * >>>>>>> + * This function cleans up the Dwfl context used for addr2line lookups. >>>>>>> + */ >>>>>>> +void dso__free_a2l_libdw(struct dso *dso); >>>>>>> + >>>>>>> +#else /* HAVE_LIBDW_SUPPORT */ >>>>>>> + >>>>>>> +static inline int libdw__addr2line(const char *dso_name __maybe_unused, >>>>>>> + u64 addr __maybe_unused, char **file __maybe_unused, >>>>>>> + unsigned int *line_nr __maybe_unused, >>>>>>> + struct dso *dso __maybe_unused, >>>>>>> + bool unwind_inlines __maybe_unused, >>>>>>> + struct inline_node *node __maybe_unused, >>>>>>> + struct symbol *sym __maybe_unused) >>>>>>> +{ >>>>>>> + return 0; >>>>>>> +} >>>>>>> + >>>>>>> +static inline void dso__free_a2l_libdw(struct dso *dso __maybe_unused) >>>>>>> +{ >>>>>>> +} >>>>>>> +#endif /* HAVE_LIBDW_SUPPORT */ >>>>>>> + >>>>>>> +#endif /* PERF_LIBDW_H */ >>>>>>> diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c >>>>>>> index 27c0966611ab..4b456c4d4138 100644 >>>>>>> --- a/tools/perf/util/srcline.c >>>>>>> +++ b/tools/perf/util/srcline.c >>>>>>> @@ -6,6 +6,7 @@ >>>>>>> #include "libbfd.h" >>>>>>> #include "llvm.h" >>>>>>> #include "symbol.h" >>>>>>> +#include "libdw.h" >>>>>>> >>>>>>> #include <inttypes.h> >>>>>>> #include <string.h> >>>>>>> @@ -120,6 +121,10 @@ static int addr2line(const char *dso_name, u64 addr, char **file, unsigned int * >>>>>>> { >>>>>>> int ret; >>>>>>> >>>>>>> + ret = libdw__addr2line(dso_name, addr, file, line_nr, dso, unwind_inlines, node, sym); >>>>>>> + if (ret > 0) >>>>>>> + return ret; >>>>>>> + >>>>>>> ret = llvm__addr2line(dso_name, addr, file, line_nr, dso, unwind_inlines, node, sym); >>>>>>> if (ret > 0) >>>>>>> return ret; >>>>>>> -- >>>>>>> 2.52.0.rc2.455.g230fcf2819-goog >>>>>>> >>>> >> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1] perf addr2line: Add a libdw implementation 2025-11-27 12:16 ` James Clark 2025-11-27 13:19 ` Ian Rogers @ 2025-12-11 23:35 ` Tony Jones [not found] ` <CAP-5=fUbZ1pjvcbF0do8BrC0NAzkOJeQUAozQhXKpGuosrW3ew@mail.gmail.com> 1 sibling, 1 reply; 13+ messages in thread From: Tony Jones @ 2025-12-11 23:35 UTC (permalink / raw) To: James Clark Cc: Ian Rogers, Namhyung Kim, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa, Adrian Hunter, Howard Chu, Stephen Brennan, linux-kernel, linux-perf-users On Thu, Nov 27, 2025 at 12:16:29PM +0000, James Clark wrote: > If the llvm addr2line implementation is also supposed to be slow, it just > means we're trading speed with accuracy with this change. Hard to say what > the default should be in that case. I originally inquired about this (and was going to code up a patch using libdw but Ian beat me to it) as we had a customer bug complaining about the performance of the binutils (/usr/bin/addr2line) forking solution and llvm isn't presently an option for us. Using the forked addr2line the customer data set takes real: 54m15.023s user: 40m57.719s sys: 11m22.461s for perf script -F ip,srcline (with numerous time-out errors). Versus real: 6m6.598s user: 5m32.603s sys: 0m1.768s with this proposed libdw patch. I'm also unfortunately seeing similar wrong file/line# issues. This said I think given that libdw is being used anyways and how poor the /usr/bin/addr2line performance is that it's worth digging deeper into what the issues are with libdw. I'll try to look at it some more next week. Thanks for the patch Ian! Tony --------- Tony Jones SUSE Kernel Performance Team ^ permalink raw reply [flat|nested] 13+ messages in thread
[parent not found: <CAP-5=fUbZ1pjvcbF0do8BrC0NAzkOJeQUAozQhXKpGuosrW3ew@mail.gmail.com>]
* Re: [PATCH v1] perf addr2line: Add a libdw implementation [not found] ` <CAP-5=fUbZ1pjvcbF0do8BrC0NAzkOJeQUAozQhXKpGuosrW3ew@mail.gmail.com> @ 2025-12-12 16:34 ` Tony Jones 0 siblings, 0 replies; 13+ messages in thread From: Tony Jones @ 2025-12-12 16:34 UTC (permalink / raw) To: Ian Rogers Cc: James Clark, Namhyung Kim, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa, Adrian Hunter, Howard Chu, Stephen Brennan, linux-kernel, linux-perf-users On Thu, Dec 11, 2025 at 09:20:35PM -0800, Ian Rogers wrote: > Thanks Tony, those are great performance numbers! I'm wondering now: > - do we want to land addr2line with libdw given the wrong file/line# > problems or wait for a fix? > - do we want to make the addr2line configurable like with objdump [1] to > mitigate the problem above? I think we should spend a bit more time understanding the former before doing the later. If only to provide some documentation on why there is a deviance to explain the configurability. Tony -- Tony Jones SUSE Kernel Performance Team ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1] perf addr2line: Add a libdw implementation 2025-11-27 11:43 ` Ian Rogers 2025-11-27 12:16 ` James Clark @ 2025-11-27 20:53 ` Namhyung Kim 2025-11-28 8:42 ` Ian Rogers 1 sibling, 1 reply; 13+ messages in thread From: Namhyung Kim @ 2025-11-27 20:53 UTC (permalink / raw) To: Ian Rogers Cc: Tony Jones, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa, Adrian Hunter, James Clark, Howard Chu, Stephen Brennan, linux-kernel, linux-perf-users On Thu, Nov 27, 2025 at 03:43:34AM -0800, Ian Rogers wrote: > On Wed, Nov 26, 2025 at 10:27 AM Namhyung Kim <namhyung@kernel.org> wrote: > > > > On Sat, Nov 22, 2025 at 01:39:34AM -0800, Ian Rogers wrote: > > > Add an implementation of addr2line that uses libdw. Other addr2line > > > implementations or, in the case of forking addr2line, slow. Add an > > > implementation that caches the libdw information in the dso and uses > > > it to find the file and line number information. > > Thanks James and Namhyung for the reviews! I agree with James' comment > about a typo in the commit message. > > > My concern is the limit in the open file descriptors in case the data > > touched a lot of different libraries. The DSO code has some logic to > > deal with it but I'm not sure if we can share that since libdw seems to > > want to own the FD. > > The code opens the FD: > > + fd = open(dso_name, O_RDONLY); > + if (fd < 0) > + return 0; > + > + dwfl = dwfl_begin(&offline_callbacks); > + if (!dwfl) { > + close(fd); > + return 0; > + } > > It then uses the FD and closes it (the close is hidden in libdw itself): > > + /* > + * If the report is successful, the file descriptor fd > is consumed > + * and closed by the Dwfl. If not, it is not closed. > + */ > + mod = dwfl_report_offline(dwfl, dso_name, dso_name, fd); > > So it is possible we exhaust all the file descriptors if there are > multiple concurrent calls to libdw__addr2line and every dso has > missing libdw dwfl data... but because the open/close are close > together and that 1 FD is say small to the FDs needed for the > cmd__addr2line, I don't think it is a problem we need to specifically > handle. Were the FD kept open until the dso was deleted, I'd agree > with you. Oh, I thought libdwfl keeps the FD open until you call dwfl_end(). Are you sure the dwfl function would close the FD? I've quickly checked the code, but it doesn't seem to close if you pass an FD manually. Thanks, Namhyung > > > Also, have you checked if this generates the exactly same output with > > other implementations? > > So the code passes `perf test` and I was checking functionality with > perf annotate, top, etc. What I saw looked good, but it may not have > been exhaustive. I didn't specifically create a test that compares the > output of the different addr2line implementations. Such a test would > be possible, it's not something we've done elsewhere. > > Thanks, > Ian > > > Thanks, > > Namhyung > > > > > > > > Signed-off-by: Ian Rogers <irogers@google.com> > > > --- > > > tools/perf/util/Build | 1 + > > > tools/perf/util/dso.c | 2 + > > > tools/perf/util/dso.h | 11 +++ > > > tools/perf/util/libdw.c | 136 ++++++++++++++++++++++++++++++++++++++ > > > tools/perf/util/libdw.h | 60 +++++++++++++++++ > > > tools/perf/util/srcline.c | 5 ++ > > > 6 files changed, 215 insertions(+) > > > create mode 100644 tools/perf/util/libdw.c > > > create mode 100644 tools/perf/util/libdw.h > > > > > > diff --git a/tools/perf/util/Build b/tools/perf/util/Build > > > index 1c2a43e1dc68..2bed6274e248 100644 > > > --- a/tools/perf/util/Build > > > +++ b/tools/perf/util/Build > > > @@ -224,6 +224,7 @@ perf-util-$(CONFIG_LIBDW) += dwarf-regs-powerpc.o > > > perf-util-$(CONFIG_LIBDW) += dwarf-regs-x86.o > > > perf-util-$(CONFIG_LIBDW) += debuginfo.o > > > perf-util-$(CONFIG_LIBDW) += annotate-data.o > > > +perf-util-$(CONFIG_LIBDW) += libdw.o > > > > > > perf-util-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o > > > perf-util-$(CONFIG_LOCAL_LIBUNWIND) += unwind-libunwind-local.o > > > diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c > > > index 344e689567ee..06980844c014 100644 > > > --- a/tools/perf/util/dso.c > > > +++ b/tools/perf/util/dso.c > > > @@ -32,6 +32,7 @@ > > > #include "string2.h" > > > #include "vdso.h" > > > #include "annotate-data.h" > > > +#include "libdw.h" > > > > > > static const char * const debuglink_paths[] = { > > > "%.0s%s", > > > @@ -1605,6 +1606,7 @@ void dso__delete(struct dso *dso) > > > auxtrace_cache__free(RC_CHK_ACCESS(dso)->auxtrace_cache); > > > dso_cache__free(dso); > > > dso__free_a2l(dso); > > > + dso__free_a2l_libdw(dso); > > > dso__free_symsrc_filename(dso); > > > nsinfo__zput(RC_CHK_ACCESS(dso)->nsinfo); > > > mutex_destroy(dso__lock(dso)); > > > diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h > > > index f8ccb9816b89..4aee23775054 100644 > > > --- a/tools/perf/util/dso.h > > > +++ b/tools/perf/util/dso.h > > > @@ -268,6 +268,7 @@ DECLARE_RC_STRUCT(dso) { > > > const char *short_name; > > > const char *long_name; > > > void *a2l; > > > + void *a2l_libdw; > > > char *symsrc_filename; > > > #if defined(__powerpc__) > > > void *dwfl; /* DWARF debug info */ > > > @@ -334,6 +335,16 @@ static inline void dso__set_a2l(struct dso *dso, void *val) > > > RC_CHK_ACCESS(dso)->a2l = val; > > > } > > > > > > +static inline void *dso__a2l_libdw(const struct dso *dso) > > > +{ > > > + return RC_CHK_ACCESS(dso)->a2l_libdw; > > > +} > > > + > > > +static inline void dso__set_a2l_libdw(struct dso *dso, void *val) > > > +{ > > > + RC_CHK_ACCESS(dso)->a2l_libdw = val; > > > +} > > > + > > > static inline unsigned int dso__a2l_fails(const struct dso *dso) > > > { > > > return RC_CHK_ACCESS(dso)->a2l_fails; > > > diff --git a/tools/perf/util/libdw.c b/tools/perf/util/libdw.c > > > new file mode 100644 > > > index 000000000000..c4331fa8e1a3 > > > --- /dev/null > > > +++ b/tools/perf/util/libdw.c > > > @@ -0,0 +1,136 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +#include "dso.h" > > > +#include "libdw.h" > > > +#include "srcline.h" > > > +#include "symbol.h" > > > +#include "dwarf-aux.h" > > > +#include <fcntl.h> > > > +#include <unistd.h> > > > +#include <elfutils/libdwfl.h> > > > + > > > +void dso__free_a2l_libdw(struct dso *dso) > > > +{ > > > + Dwfl *dwfl = dso__a2l_libdw(dso); > > > + > > > + if (dwfl) { > > > + dwfl_end(dwfl); > > > + dso__set_a2l_libdw(dso, NULL); > > > + } > > > +} > > > + > > > +int libdw__addr2line(const char *dso_name, u64 addr, > > > + char **file, unsigned int *line_nr, > > > + struct dso *dso, bool unwind_inlines, > > > + struct inline_node *node, struct symbol *sym) > > > +{ > > > + static const Dwfl_Callbacks offline_callbacks = { > > > + .find_debuginfo = dwfl_standard_find_debuginfo, > > > + .section_address = dwfl_offline_section_address, > > > + .find_elf = dwfl_build_id_find_elf, > > > + }; > > > + Dwfl *dwfl = dso__a2l_libdw(dso); > > > + Dwfl_Module *mod; > > > + Dwfl_Line *dwline; > > > + Dwarf_Addr bias; > > > + const char *src; > > > + int lineno; > > > + > > > + if (!dwfl) { > > > + /* > > > + * Initialize Dwfl session. > > > + * We need to open the DSO file to report it to libdw. > > > + */ > > > + int fd; > > > + > > > + fd = open(dso_name, O_RDONLY); > > > + if (fd < 0) > > > + return 0; > > > + > > > + dwfl = dwfl_begin(&offline_callbacks); > > > + if (!dwfl) { > > > + close(fd); > > > + return 0; > > > + } > > > + > > > + /* > > > + * If the report is successful, the file descriptor fd is consumed > > > + * and closed by the Dwfl. If not, it is not closed. > > > + */ > > > + mod = dwfl_report_offline(dwfl, dso_name, dso_name, fd); > > > + if (!mod) { > > > + dwfl_end(dwfl); > > > + close(fd); > > > + return 0; > > > + } > > > + > > > + dwfl_report_end(dwfl, /*removed=*/NULL, /*arg=*/NULL); > > > + dso__set_a2l_libdw(dso, dwfl); > > > + } else { > > > + /* Dwfl session already initialized, get module for address. */ > > > + mod = dwfl_addrmodule(dwfl, addr); > > > + } > > > + > > > + if (!mod) > > > + return 0; > > > + > > > + /* Find source line information for the address. */ > > > + dwline = dwfl_module_getsrc(mod, addr); > > > + if (!dwline) > > > + return 0; > > > + > > > + /* Get line information. */ > > > + src = dwfl_lineinfo(dwline, &addr, &lineno, /*col=*/NULL, /*mtime=*/NULL, /*length=*/NULL); > > > + > > > + if (file) > > > + *file = src ? strdup(src) : NULL; > > > + if (line_nr) > > > + *line_nr = lineno; > > > + > > > + /* Optionally unwind inline function call chain. */ > > > + if (unwind_inlines && node && src) { > > > + Dwarf_Die *cudie = dwfl_module_addrdie(mod, addr, &bias); > > > + Dwarf_Die *scopes = NULL; > > > + int nscopes; > > > + > > > + if (!cudie) > > > + return 1; > > > + > > > + nscopes = die_get_scopes(cudie, addr, &scopes); > > > + if (nscopes > 0) { > > > + int i; > > > + const char *call_file = src; > > > + unsigned int call_line = lineno; > > > + > > > + for (i = 0; i < nscopes; i++) { > > > + Dwarf_Die *die = &scopes[i]; > > > + struct symbol *inline_sym; > > > + char *srcline = NULL; > > > + int tag = dwarf_tag(die); > > > + > > > + /* We are interested in inlined subroutines. */ > > > + if (tag != DW_TAG_inlined_subroutine && > > > + tag != DW_TAG_subprogram) > > > + continue; > > > + > > > + inline_sym = new_inline_sym(dso, sym, dwarf_diename(die)); > > > + > > > + if (call_file) > > > + srcline = srcline_from_fileline(call_file, call_line); > > > + > > > + inline_list__append(inline_sym, srcline, node); > > > + > > > + /* Update call site for next level. */ > > > + if (tag == DW_TAG_inlined_subroutine) { > > > + call_file = die_get_call_file(die); > > > + call_line = die_get_call_lineno(die); > > > + } else { > > > + /* Reached the root subprogram. */ > > > + break; > > > + } > > > + } > > > + free(scopes); > > > + } > > > + } > > > + > > > + return 1; > > > +} > > > diff --git a/tools/perf/util/libdw.h b/tools/perf/util/libdw.h > > > new file mode 100644 > > > index 000000000000..0f8d7b4a11a5 > > > --- /dev/null > > > +++ b/tools/perf/util/libdw.h > > > @@ -0,0 +1,60 @@ > > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > +#ifndef PERF_LIBDW_H > > > +#define PERF_LIBDW_H > > > + > > > +#include <linux/types.h> > > > + > > > +struct dso; > > > +struct inline_node; > > > +struct symbol; > > > + > > > +#ifdef HAVE_LIBDW_SUPPORT > > > +/* > > > + * libdw__addr2line - Convert address to source location using libdw > > > + * @dso_name: Name of the DSO > > > + * @addr: Address to resolve > > > + * @file: Pointer to return filename (caller must free) > > > + * @line_nr: Pointer to return line number > > > + * @dso: The dso struct > > > + * @unwind_inlines: Whether to unwind inline function calls > > > + * @node: Inline node list to append to > > > + * @sym: The symbol associated with the address > > > + * > > > + * This function initializes a Dwfl context for the DSO if not already present, > > > + * finds the source line information for the given address, and optionally > > > + * resolves inline function call chains. > > > + * > > > + * Returns 1 on success (found), 0 on failure (not found). > > > + */ > > > +int libdw__addr2line(const char *dso_name, u64 addr, char **file, > > > + unsigned int *line_nr, struct dso *dso, > > > + bool unwind_inlines, struct inline_node *node, > > > + struct symbol *sym); > > > + > > > +/* > > > + * dso__free_a2l_libdw - Free libdw resources associated with the DSO > > > + * @dso: The dso to free resources for > > > + * > > > + * This function cleans up the Dwfl context used for addr2line lookups. > > > + */ > > > +void dso__free_a2l_libdw(struct dso *dso); > > > + > > > +#else /* HAVE_LIBDW_SUPPORT */ > > > + > > > +static inline int libdw__addr2line(const char *dso_name __maybe_unused, > > > + u64 addr __maybe_unused, char **file __maybe_unused, > > > + unsigned int *line_nr __maybe_unused, > > > + struct dso *dso __maybe_unused, > > > + bool unwind_inlines __maybe_unused, > > > + struct inline_node *node __maybe_unused, > > > + struct symbol *sym __maybe_unused) > > > +{ > > > + return 0; > > > +} > > > + > > > +static inline void dso__free_a2l_libdw(struct dso *dso __maybe_unused) > > > +{ > > > +} > > > +#endif /* HAVE_LIBDW_SUPPORT */ > > > + > > > +#endif /* PERF_LIBDW_H */ > > > diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c > > > index 27c0966611ab..4b456c4d4138 100644 > > > --- a/tools/perf/util/srcline.c > > > +++ b/tools/perf/util/srcline.c > > > @@ -6,6 +6,7 @@ > > > #include "libbfd.h" > > > #include "llvm.h" > > > #include "symbol.h" > > > +#include "libdw.h" > > > > > > #include <inttypes.h> > > > #include <string.h> > > > @@ -120,6 +121,10 @@ static int addr2line(const char *dso_name, u64 addr, char **file, unsigned int * > > > { > > > int ret; > > > > > > + ret = libdw__addr2line(dso_name, addr, file, line_nr, dso, unwind_inlines, node, sym); > > > + if (ret > 0) > > > + return ret; > > > + > > > ret = llvm__addr2line(dso_name, addr, file, line_nr, dso, unwind_inlines, node, sym); > > > if (ret > 0) > > > return ret; > > > -- > > > 2.52.0.rc2.455.g230fcf2819-goog > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1] perf addr2line: Add a libdw implementation 2025-11-27 20:53 ` Namhyung Kim @ 2025-11-28 8:42 ` Ian Rogers 0 siblings, 0 replies; 13+ messages in thread From: Ian Rogers @ 2025-11-28 8:42 UTC (permalink / raw) To: Namhyung Kim Cc: Tony Jones, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa, Adrian Hunter, James Clark, Howard Chu, Stephen Brennan, linux-kernel, linux-perf-users On Thu, Nov 27, 2025 at 12:53 PM Namhyung Kim <namhyung@kernel.org> wrote: > > On Thu, Nov 27, 2025 at 03:43:34AM -0800, Ian Rogers wrote: > > On Wed, Nov 26, 2025 at 10:27 AM Namhyung Kim <namhyung@kernel.org> wrote: > > > > > > On Sat, Nov 22, 2025 at 01:39:34AM -0800, Ian Rogers wrote: > > > > Add an implementation of addr2line that uses libdw. Other addr2line > > > > implementations or, in the case of forking addr2line, slow. Add an > > > > implementation that caches the libdw information in the dso and uses > > > > it to find the file and line number information. > > > > Thanks James and Namhyung for the reviews! I agree with James' comment > > about a typo in the commit message. > > > > > My concern is the limit in the open file descriptors in case the data > > > touched a lot of different libraries. The DSO code has some logic to > > > deal with it but I'm not sure if we can share that since libdw seems to > > > want to own the FD. > > > > The code opens the FD: > > > > + fd = open(dso_name, O_RDONLY); > > + if (fd < 0) > > + return 0; > > + > > + dwfl = dwfl_begin(&offline_callbacks); > > + if (!dwfl) { > > + close(fd); > > + return 0; > > + } > > > > It then uses the FD and closes it (the close is hidden in libdw itself): > > > > + /* > > + * If the report is successful, the file descriptor fd > > is consumed > > + * and closed by the Dwfl. If not, it is not closed. > > + */ > > + mod = dwfl_report_offline(dwfl, dso_name, dso_name, fd); > > > > So it is possible we exhaust all the file descriptors if there are > > multiple concurrent calls to libdw__addr2line and every dso has > > missing libdw dwfl data... but because the open/close are close > > together and that 1 FD is say small to the FDs needed for the > > cmd__addr2line, I don't think it is a problem we need to specifically > > handle. Were the FD kept open until the dso was deleted, I'd agree > > with you. > > Oh, I thought libdwfl keeps the FD open until you call dwfl_end(). > Are you sure the dwfl function would close the FD? I've quickly checked > the code, but it doesn't seem to close if you pass an FD manually. So in the documentation there is: https://sourceware.org/git/?p=elfutils.git;a=blob;f=libdwfl/libdwfl.h;h=90523283c9359b09a16709a8422917a44257932f;hb=HEAD#l155 ``` /* ...On success, FD is consumed by the library, ... */ extern Dwfl_Module *dwfl_report_elf (Dwfl *dwfl, const char *name, const char *file_name, int fd, GElf_Addr base, bool add_p_vaddr); /* Similar, but report the module for offline use. ... */ extern Dwfl_Module *dwfl_report_offline (Dwfl *dwfl, const char *name, const char *file_name, int fd); ``` Running with gdb I see: ``` $ perf record perf test -w noploop Lowering default frequency rate from 4000 to 2500. Please consider tweaking /proc/sys/kernel/perf_event_max_sample_rate. [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.611 MB perf.data (2534 samples) ] $ gdb --args perf --no-pager annotate -l --stdio ... Reading symbols from /tmp/perf/perf... (gdb) b libdw__addr2line Breakpoint 1 at 0x392b98: file util/libdw.c, line 25. (gdb) r Starting program: /tmp/perf/perf --no-pager annotate -l --stdio ... Breakpoint 1, libdw__addr2line (dso_name=0x555557b41270 "/root/.debug/.build-id/4c/72a04e484e8636631219f4454cc64a674a5f57/elf", addr=1903613, file=0x7fffffffa830, line_nr=0x7fffffffa82c, dso=0x5555569eba20, unwind_inlines=false, node=0x0, sym=0x0) at util/libdw.c:25 25 { (gdb) n 31 Dwfl *dwfl = dso__a2l_libdw(dso); (gdb) 38 if (!dwfl) { (gdb) 45 fd = open(dso_name, O_RDONLY); (gdb) 46 if (fd < 0) (gdb) p fd $1 = 5 (gdb) n 49 dwfl = dwfl_begin(&offline_callbacks); (gdb) b close Breakpoint 2 at 0x7fffedf09760: close. (15 locations) (gdb) c Continuing. Breakpoint 2.1, __GI___close (fd=5) at ../sysdeps/unix/sysv/linux/close.c:27 warning: 27 ../sysdeps/unix/sysv/linux/close.c: No such file or directory (gdb) bt #0 __GI___close (fd=5) at ../sysdeps/unix/sysv/linux/close.c:27 #1 0x00007ffff7e0e122 in ?? () from /lib/x86_64-linux-gnu/libdw.so.1 #2 0x00007ffff7e0e6f7 in ?? () from /lib/x86_64-linux-gnu/libdw.so.1 #3 0x00005555558e6c3c in libdw__addr2line (dso_name=0x555557b41270 "/root/.debug/.build-id/4c/72a04e484e8636631219f4454cc64a674a5f57/elf", addr=1903613, file=0x7fffffffa830, line_nr=0x7fffffffa82c, dso=0x5555569eba20, unwind_inlines=false, node=0x0, sym=0x0) at util/libdw.c:59 #4 0x000055555584c861 in addr2line (dso_name=0x555557b41270 "/root/.debug/.build-id/4c/72a04e484e8636631219f4454cc64a674a5f57/elf", addr=1903613, file=0x7fffffffa830, line_nr=0x7fffffffa82c, dso=0x5555569eba20, unwind_inlines=false, node=0x0, sym=0x0) at util/srcline.c:124 #5 0x000055555584ca9c in __get_srcline (dso=0x5555569eba20, addr=1903613, sym=0x0, show_sym=false, show_addr=true, unwind_inlines=false, ip=1903613) at util/srcline.c:181 #6 0x000055555584ce21 in get_srcline (dso=0x5555569eba20, addr=1903613, sym=0x0, show_sym=false, show_addr=true, ip=1903613) at util/srcline.c:259 #7 0x0000555555754a63 in annotation__calc_lines (notes=0x5555586d02d0, ms=0x555558665600, root=0x7fffffffa9a0) at util/annotate.c:1651 #8 0x0000555555754b20 in symbol__calc_lines (ms=0x555558665600, root=0x7fffffffa9a0) at util/annotate.c:1663 #9 0x0000555555754f22 in hist_entry__tty_annotate (he=0x555558665560, evsel=0x5555561dc6e0) at util/annotate.c:1728 #10 0x00005555555a829a in hist_entry__stdio_annotate (he=0x555558665560, evsel=0x5555561dc6e0, ann=0x7fffffffcc90) at builtin-annotate.c:332 #11 0x00005555555a8ed4 in hists__find_annotations (hists=0x5555561dc998, evsel=0x5555561dc6e0, ann=0x7fffffffcc90) at builtin-annotate.c:545 #12 0x00005555555a9404 in __cmd_annotate (ann=0x7fffffffcc90) at builtin-annotate.c:655 #13 0x00005555555aa5c6 in cmd_annotate (argc=0, argv=0x7fffffffe318) at builtin-annotate.c:949 #14 0x0000555555655e11 in run_builtin (p=0x555556128650 <commands+432>, argc=3, argv=0x7fffffffe318) at perf.c:349 #15 0x00005555556560a9 in handle_internal_command (argc=3, argv=0x7fffffffe318) at perf.c:401 #16 0x0000555555656202 in run_argv (argcp=0x7fffffffe10c, argv=0x7fffffffe100) at perf.c:445 #17 0x000055555565654b in main (argc=3, argv=0x7fffffffe318) at perf.c:553 (gdb) frame 3 #3 0x00005555558e6c3c in libdw__addr2line (dso_name=0x555557b41270 "/root/.debug/.build-id/4c/72a04e484e8636631219f4454cc64a674a5f57/elf", addr=1903613, file=0x7fffffffa830, line_nr=0x7fffffffa82c, dso=0x5555569eba20, unwind_inlines=false, node=0x0, sym=0x0) at util/libdw.c:59 59 mod = dwfl_report_offline(dwfl, dso_name, dso_name, fd); ``` So I think everything is working as explained. The open happens returning fd 5, dwfl_report_offline is called and on success it closes fd 5. Were the dwfl_report_offline to fail then the error path in libdw__addr2line closes fd. Looking at older versions of the code (10 years old) the header file documentation is the same, so I'm not sure why you are seeing a different behavior. Thanks, Ian > Thanks, > Namhyung > > > > > > Also, have you checked if this generates the exactly same output with > > > other implementations? > > > > So the code passes `perf test` and I was checking functionality with > > perf annotate, top, etc. What I saw looked good, but it may not have > > been exhaustive. I didn't specifically create a test that compares the > > output of the different addr2line implementations. Such a test would > > be possible, it's not something we've done elsewhere. > > > > Thanks, > > Ian > > > > > Thanks, > > > Namhyung > > > > > > > > > > > Signed-off-by: Ian Rogers <irogers@google.com> > > > > --- > > > > tools/perf/util/Build | 1 + > > > > tools/perf/util/dso.c | 2 + > > > > tools/perf/util/dso.h | 11 +++ > > > > tools/perf/util/libdw.c | 136 ++++++++++++++++++++++++++++++++++++++ > > > > tools/perf/util/libdw.h | 60 +++++++++++++++++ > > > > tools/perf/util/srcline.c | 5 ++ > > > > 6 files changed, 215 insertions(+) > > > > create mode 100644 tools/perf/util/libdw.c > > > > create mode 100644 tools/perf/util/libdw.h > > > > > > > > diff --git a/tools/perf/util/Build b/tools/perf/util/Build > > > > index 1c2a43e1dc68..2bed6274e248 100644 > > > > --- a/tools/perf/util/Build > > > > +++ b/tools/perf/util/Build > > > > @@ -224,6 +224,7 @@ perf-util-$(CONFIG_LIBDW) += dwarf-regs-powerpc.o > > > > perf-util-$(CONFIG_LIBDW) += dwarf-regs-x86.o > > > > perf-util-$(CONFIG_LIBDW) += debuginfo.o > > > > perf-util-$(CONFIG_LIBDW) += annotate-data.o > > > > +perf-util-$(CONFIG_LIBDW) += libdw.o > > > > > > > > perf-util-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o > > > > perf-util-$(CONFIG_LOCAL_LIBUNWIND) += unwind-libunwind-local.o > > > > diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c > > > > index 344e689567ee..06980844c014 100644 > > > > --- a/tools/perf/util/dso.c > > > > +++ b/tools/perf/util/dso.c > > > > @@ -32,6 +32,7 @@ > > > > #include "string2.h" > > > > #include "vdso.h" > > > > #include "annotate-data.h" > > > > +#include "libdw.h" > > > > > > > > static const char * const debuglink_paths[] = { > > > > "%.0s%s", > > > > @@ -1605,6 +1606,7 @@ void dso__delete(struct dso *dso) > > > > auxtrace_cache__free(RC_CHK_ACCESS(dso)->auxtrace_cache); > > > > dso_cache__free(dso); > > > > dso__free_a2l(dso); > > > > + dso__free_a2l_libdw(dso); > > > > dso__free_symsrc_filename(dso); > > > > nsinfo__zput(RC_CHK_ACCESS(dso)->nsinfo); > > > > mutex_destroy(dso__lock(dso)); > > > > diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h > > > > index f8ccb9816b89..4aee23775054 100644 > > > > --- a/tools/perf/util/dso.h > > > > +++ b/tools/perf/util/dso.h > > > > @@ -268,6 +268,7 @@ DECLARE_RC_STRUCT(dso) { > > > > const char *short_name; > > > > const char *long_name; > > > > void *a2l; > > > > + void *a2l_libdw; > > > > char *symsrc_filename; > > > > #if defined(__powerpc__) > > > > void *dwfl; /* DWARF debug info */ > > > > @@ -334,6 +335,16 @@ static inline void dso__set_a2l(struct dso *dso, void *val) > > > > RC_CHK_ACCESS(dso)->a2l = val; > > > > } > > > > > > > > +static inline void *dso__a2l_libdw(const struct dso *dso) > > > > +{ > > > > + return RC_CHK_ACCESS(dso)->a2l_libdw; > > > > +} > > > > + > > > > +static inline void dso__set_a2l_libdw(struct dso *dso, void *val) > > > > +{ > > > > + RC_CHK_ACCESS(dso)->a2l_libdw = val; > > > > +} > > > > + > > > > static inline unsigned int dso__a2l_fails(const struct dso *dso) > > > > { > > > > return RC_CHK_ACCESS(dso)->a2l_fails; > > > > diff --git a/tools/perf/util/libdw.c b/tools/perf/util/libdw.c > > > > new file mode 100644 > > > > index 000000000000..c4331fa8e1a3 > > > > --- /dev/null > > > > +++ b/tools/perf/util/libdw.c > > > > @@ -0,0 +1,136 @@ > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > +#include "dso.h" > > > > +#include "libdw.h" > > > > +#include "srcline.h" > > > > +#include "symbol.h" > > > > +#include "dwarf-aux.h" > > > > +#include <fcntl.h> > > > > +#include <unistd.h> > > > > +#include <elfutils/libdwfl.h> > > > > + > > > > +void dso__free_a2l_libdw(struct dso *dso) > > > > +{ > > > > + Dwfl *dwfl = dso__a2l_libdw(dso); > > > > + > > > > + if (dwfl) { > > > > + dwfl_end(dwfl); > > > > + dso__set_a2l_libdw(dso, NULL); > > > > + } > > > > +} > > > > + > > > > +int libdw__addr2line(const char *dso_name, u64 addr, > > > > + char **file, unsigned int *line_nr, > > > > + struct dso *dso, bool unwind_inlines, > > > > + struct inline_node *node, struct symbol *sym) > > > > +{ > > > > + static const Dwfl_Callbacks offline_callbacks = { > > > > + .find_debuginfo = dwfl_standard_find_debuginfo, > > > > + .section_address = dwfl_offline_section_address, > > > > + .find_elf = dwfl_build_id_find_elf, > > > > + }; > > > > + Dwfl *dwfl = dso__a2l_libdw(dso); > > > > + Dwfl_Module *mod; > > > > + Dwfl_Line *dwline; > > > > + Dwarf_Addr bias; > > > > + const char *src; > > > > + int lineno; > > > > + > > > > + if (!dwfl) { > > > > + /* > > > > + * Initialize Dwfl session. > > > > + * We need to open the DSO file to report it to libdw. > > > > + */ > > > > + int fd; > > > > + > > > > + fd = open(dso_name, O_RDONLY); > > > > + if (fd < 0) > > > > + return 0; > > > > + > > > > + dwfl = dwfl_begin(&offline_callbacks); > > > > + if (!dwfl) { > > > > + close(fd); > > > > + return 0; > > > > + } > > > > + > > > > + /* > > > > + * If the report is successful, the file descriptor fd is consumed > > > > + * and closed by the Dwfl. If not, it is not closed. > > > > + */ > > > > + mod = dwfl_report_offline(dwfl, dso_name, dso_name, fd); > > > > + if (!mod) { > > > > + dwfl_end(dwfl); > > > > + close(fd); > > > > + return 0; > > > > + } > > > > + > > > > + dwfl_report_end(dwfl, /*removed=*/NULL, /*arg=*/NULL); > > > > + dso__set_a2l_libdw(dso, dwfl); > > > > + } else { > > > > + /* Dwfl session already initialized, get module for address. */ > > > > + mod = dwfl_addrmodule(dwfl, addr); > > > > + } > > > > + > > > > + if (!mod) > > > > + return 0; > > > > + > > > > + /* Find source line information for the address. */ > > > > + dwline = dwfl_module_getsrc(mod, addr); > > > > + if (!dwline) > > > > + return 0; > > > > + > > > > + /* Get line information. */ > > > > + src = dwfl_lineinfo(dwline, &addr, &lineno, /*col=*/NULL, /*mtime=*/NULL, /*length=*/NULL); > > > > + > > > > + if (file) > > > > + *file = src ? strdup(src) : NULL; > > > > + if (line_nr) > > > > + *line_nr = lineno; > > > > + > > > > + /* Optionally unwind inline function call chain. */ > > > > + if (unwind_inlines && node && src) { > > > > + Dwarf_Die *cudie = dwfl_module_addrdie(mod, addr, &bias); > > > > + Dwarf_Die *scopes = NULL; > > > > + int nscopes; > > > > + > > > > + if (!cudie) > > > > + return 1; > > > > + > > > > + nscopes = die_get_scopes(cudie, addr, &scopes); > > > > + if (nscopes > 0) { > > > > + int i; > > > > + const char *call_file = src; > > > > + unsigned int call_line = lineno; > > > > + > > > > + for (i = 0; i < nscopes; i++) { > > > > + Dwarf_Die *die = &scopes[i]; > > > > + struct symbol *inline_sym; > > > > + char *srcline = NULL; > > > > + int tag = dwarf_tag(die); > > > > + > > > > + /* We are interested in inlined subroutines. */ > > > > + if (tag != DW_TAG_inlined_subroutine && > > > > + tag != DW_TAG_subprogram) > > > > + continue; > > > > + > > > > + inline_sym = new_inline_sym(dso, sym, dwarf_diename(die)); > > > > + > > > > + if (call_file) > > > > + srcline = srcline_from_fileline(call_file, call_line); > > > > + > > > > + inline_list__append(inline_sym, srcline, node); > > > > + > > > > + /* Update call site for next level. */ > > > > + if (tag == DW_TAG_inlined_subroutine) { > > > > + call_file = die_get_call_file(die); > > > > + call_line = die_get_call_lineno(die); > > > > + } else { > > > > + /* Reached the root subprogram. */ > > > > + break; > > > > + } > > > > + } > > > > + free(scopes); > > > > + } > > > > + } > > > > + > > > > + return 1; > > > > +} > > > > diff --git a/tools/perf/util/libdw.h b/tools/perf/util/libdw.h > > > > new file mode 100644 > > > > index 000000000000..0f8d7b4a11a5 > > > > --- /dev/null > > > > +++ b/tools/perf/util/libdw.h > > > > @@ -0,0 +1,60 @@ > > > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > > +#ifndef PERF_LIBDW_H > > > > +#define PERF_LIBDW_H > > > > + > > > > +#include <linux/types.h> > > > > + > > > > +struct dso; > > > > +struct inline_node; > > > > +struct symbol; > > > > + > > > > +#ifdef HAVE_LIBDW_SUPPORT > > > > +/* > > > > + * libdw__addr2line - Convert address to source location using libdw > > > > + * @dso_name: Name of the DSO > > > > + * @addr: Address to resolve > > > > + * @file: Pointer to return filename (caller must free) > > > > + * @line_nr: Pointer to return line number > > > > + * @dso: The dso struct > > > > + * @unwind_inlines: Whether to unwind inline function calls > > > > + * @node: Inline node list to append to > > > > + * @sym: The symbol associated with the address > > > > + * > > > > + * This function initializes a Dwfl context for the DSO if not already present, > > > > + * finds the source line information for the given address, and optionally > > > > + * resolves inline function call chains. > > > > + * > > > > + * Returns 1 on success (found), 0 on failure (not found). > > > > + */ > > > > +int libdw__addr2line(const char *dso_name, u64 addr, char **file, > > > > + unsigned int *line_nr, struct dso *dso, > > > > + bool unwind_inlines, struct inline_node *node, > > > > + struct symbol *sym); > > > > + > > > > +/* > > > > + * dso__free_a2l_libdw - Free libdw resources associated with the DSO > > > > + * @dso: The dso to free resources for > > > > + * > > > > + * This function cleans up the Dwfl context used for addr2line lookups. > > > > + */ > > > > +void dso__free_a2l_libdw(struct dso *dso); > > > > + > > > > +#else /* HAVE_LIBDW_SUPPORT */ > > > > + > > > > +static inline int libdw__addr2line(const char *dso_name __maybe_unused, > > > > + u64 addr __maybe_unused, char **file __maybe_unused, > > > > + unsigned int *line_nr __maybe_unused, > > > > + struct dso *dso __maybe_unused, > > > > + bool unwind_inlines __maybe_unused, > > > > + struct inline_node *node __maybe_unused, > > > > + struct symbol *sym __maybe_unused) > > > > +{ > > > > + return 0; > > > > +} > > > > + > > > > +static inline void dso__free_a2l_libdw(struct dso *dso __maybe_unused) > > > > +{ > > > > +} > > > > +#endif /* HAVE_LIBDW_SUPPORT */ > > > > + > > > > +#endif /* PERF_LIBDW_H */ > > > > diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c > > > > index 27c0966611ab..4b456c4d4138 100644 > > > > --- a/tools/perf/util/srcline.c > > > > +++ b/tools/perf/util/srcline.c > > > > @@ -6,6 +6,7 @@ > > > > #include "libbfd.h" > > > > #include "llvm.h" > > > > #include "symbol.h" > > > > +#include "libdw.h" > > > > > > > > #include <inttypes.h> > > > > #include <string.h> > > > > @@ -120,6 +121,10 @@ static int addr2line(const char *dso_name, u64 addr, char **file, unsigned int * > > > > { > > > > int ret; > > > > > > > > + ret = libdw__addr2line(dso_name, addr, file, line_nr, dso, unwind_inlines, node, sym); > > > > + if (ret > 0) > > > > + return ret; > > > > + > > > > ret = llvm__addr2line(dso_name, addr, file, line_nr, dso, unwind_inlines, node, sym); > > > > if (ret > 0) > > > > return ret; > > > > -- > > > > 2.52.0.rc2.455.g230fcf2819-goog > > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-12-12 16:34 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-22 9:39 [PATCH v1] perf addr2line: Add a libdw implementation Ian Rogers
2025-11-24 11:21 ` James Clark
2025-11-26 18:27 ` Namhyung Kim
2025-11-27 11:43 ` Ian Rogers
2025-11-27 12:16 ` James Clark
2025-11-27 13:19 ` Ian Rogers
2025-11-27 13:48 ` James Clark
2025-11-28 9:00 ` Ian Rogers
2025-11-28 14:25 ` James Clark
2025-12-11 23:35 ` Tony Jones
[not found] ` <CAP-5=fUbZ1pjvcbF0do8BrC0NAzkOJeQUAozQhXKpGuosrW3ew@mail.gmail.com>
2025-12-12 16:34 ` Tony Jones
2025-11-27 20:53 ` Namhyung Kim
2025-11-28 8:42 ` Ian Rogers
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).