linux-kernel - Re: [PATCH v4 9/9] perf annotate: Add dso_

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAP-5=fXj5pcuut9dTVqZfmipTb-sRkZXUwjhJ41rTDNOKDJPQQ@mail.gmail.com>
Date: Fri, 25 Jul 2025 17:46:48 -0700
From: Ian Rogers <irogers@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>, Kan Liang <kan.liang@...ux.intel.com>, 
	Jiri Olsa <jolsa@...nel.org>, Adrian Hunter <adrian.hunter@...el.com>, 
	Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...nel.org>, 
	LKML <linux-kernel@...r.kernel.org>, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v4 9/9] perf annotate: Add dso__debuginfo() helper

On Fri, Jul 25, 2025 at 12:38 PM Namhyung Kim <namhyung@...nel.org> wrote:
>
> It'd be great if it can get the correct debug information using DSO
> build-Id not just the path name.  Instead of adding new callsites of
> debuginfo__new(), let's add dso__debuginfo() which can hide the access
> using the pathname and help the future conversion.
>
> Suggested-by: Ian Rogers <irogers@...gle.com>
> Signed-off-by: Namhyung Kim <namhyung@...nel.org>

:-) and my prior review comments now make less sense. I think putting
the ui__warning into dso__debugingo makes sense, wdyt?

Thanks,
Ian

> ---
>  tools/perf/ui/browsers/annotate.c |  4 ++--
>  tools/perf/util/annotate.c        |  6 +++---
>  tools/perf/util/dso.h             | 10 ++++++++++
>  3 files changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index 2a4db5bdcdb7e9d8..54610621c5f910fe 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -1042,7 +1042,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
>                 case 'T':
>                         annotate_opts.code_with_type ^= 1;
>                         if (browser->dbg == NULL)
> -                               browser->dbg = debuginfo__new(dso__long_name(map__dso(ms->map)));
> +                               browser->dbg = dso__debuginfo(map__dso(ms->map));
>                         annotate_browser__show(&browser->b, title, help);
>                         annotate_browser__debuginfo_warning(browser);
>                         continue;
> @@ -1128,7 +1128,7 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
>         ui_helpline__push("Press ESC to exit");
>
>         if (annotate_opts.code_with_type)
> -               browser.dbg = debuginfo__new(dso__long_name(dso));
> +               browser.dbg = dso__debuginfo(dso);
>
>         browser.b.width = notes->src->widths.max_line_len;
>         browser.b.nr_entries = notes->src->nr_entries;
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 6fc07971631ac8a3..05eb19b110ab7dcf 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1270,7 +1270,7 @@ int hist_entry__annotate_printf(struct hist_entry *he, struct evsel *evsel)
>         apd.addr_fmt_width = annotated_source__addr_fmt_width(&notes->src->source,
>                                                               apd.start);
>         evsel__get_arch(evsel, &apd.arch);
> -       apd.dbg = debuginfo__new(filename);
> +       apd.dbg = dso__debuginfo(dso);
>
>         list_for_each_entry(pos, &notes->src->source, node) {
>                 int err;
> @@ -1375,7 +1375,7 @@ static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp,
>
>         if (annotate_opts.code_with_type) {
>                 evsel__get_arch(apd->evsel, &apd->arch);
> -               apd->dbg = debuginfo__new(dso__long_name(map__dso(apd->he->ms.map)));
> +               apd->dbg = dso__debuginfo(map__dso(apd->he->ms.map));
>         }
>
>         list_for_each_entry(al, &notes->src->source, node) {
> @@ -2888,7 +2888,7 @@ struct annotated_data_type *hist_entry__get_data_type(struct hist_entry *he)
>                 di_cache.dso = dso__get(map__dso(ms->map));
>
>                 debuginfo__delete(di_cache.dbg);
> -               di_cache.dbg = debuginfo__new(dso__long_name(di_cache.dso));
> +               di_cache.dbg = dso__debuginfo(di_cache.dso);
>         }
>
>         if (di_cache.dbg == NULL) {
> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
> index 7df1673f08d3ddb4..fd8e95de77f78dfc 100644
> --- a/tools/perf/util/dso.h
> +++ b/tools/perf/util/dso.h
> @@ -10,6 +10,7 @@
>  #include <stdio.h>
>  #include <linux/bitops.h>
>  #include "build-id.h"
> +#include "debuginfo.h"
>  #include "mutex.h"
>  #include <internal/rc_check.h>
>
> @@ -914,4 +915,13 @@ u64 dso__findnew_global_type(struct dso *dso, u64 addr, u64 offset);
>  bool perf_pid_map_tid(const char *dso_name, int *tid);
>  bool is_perf_pid_map_name(const char *dso_name);
>
> +/*
> + * In the future, we may get debuginfo using build-ID (w/o path).
> + * Add this helper is for the smooth conversion.
> + */
> +static inline struct debuginfo *dso__debuginfo(struct dso *dso)
> +{
> +       return debuginfo__new(dso__long_name(dso));
> +}
> +
>  #endif /* __PERF_DSO */
> --
> 2.50.1
>