| 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
| ||
|
Message-ID: <CAP-5=fUUOT4-1DU=Lx7OTayLssUq_ZT-bdGuqRzj_2jMqBx_FQ@mail.gmail.com>
Date: Thu, 26 Jun 2025 08:23:30 -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 v3] perf annotate: Fix source code annotate with objdump
On Wed, Jun 25, 2025 at 4:03 PM Namhyung Kim <namhyung@...nel.org> wrote:
>
> Recently it uses llvm and capstone to speed up annotation or disassembly
> of instructions. But they don't support source code view yet. Until it
> fixed, we can force to use objdump for source code annotation.
>
> To prevent performance loss, it's disabled by default and turned it on
> when user requests it in TUI by pressing 's' key.
>
> Link: https://lore.kernel.org/r/20250608004613.238291-1-namhyung@kernel.org
> Reported-by: Ingo Molnar <mingo@...nel.org>
> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
Can we move this series forward:
https://lore.kernel.org/lkml/20250417230740.86048-1-irogers@google.com/
There was some build testing by Arnaldo but then things became
stalled. I'm guessing I'll need to send a rebase.
Thanks,
Ian
> ---
> v3)
> * show warning when there's no source code
>
> tools/perf/ui/browsers/annotate.c | 86 +++++++++++++++++++++++++++++--
> tools/perf/util/annotate.c | 2 +
> tools/perf/util/annotate.h | 1 +
> tools/perf/util/disasm.c | 7 +++
> 4 files changed, 93 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index ab776b1ed2d5b4ba..183902dac042ecb0 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -345,6 +345,23 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser,
> browser->curr_hot = rb_last(&browser->entries);
> }
>
> +static struct annotation_line *annotate_browser__find_new_asm_line(
> + struct annotate_browser *browser,
> + int idx_asm)
> +{
> + struct annotation_line *al;
> + struct list_head *head = browser->b.entries;
> +
> + /* find an annotation line in the new list with the same idx_asm */
> + list_for_each_entry(al, head, node) {
> + if (al->idx_asm == idx_asm)
> + return al;
> + }
> +
> + /* There are no asm lines */
> + return NULL;
> +}
> +
> static struct annotation_line *annotate_browser__find_next_asm_line(
> struct annotate_browser *browser,
> struct annotation_line *al)
> @@ -368,7 +385,31 @@ static struct annotation_line *annotate_browser__find_next_asm_line(
> return NULL;
> }
>
> -static bool annotate_browser__toggle_source(struct annotate_browser *browser)
> +static bool annotation__has_source(struct annotation *notes)
> +{
> + struct annotation_line *al;
> + bool found_asm = false;
> +
> + /* Let's skip the first non-asm lines which present regardless of source. */
> + list_for_each_entry(al, ¬es->src->source, node) {
> + if (al->offset >= 0) {
> + found_asm = true;
> + break;
> + }
> + }
> +
> + if (found_asm) {
> + /* After assembly lines, any line without offset means source. */
> + list_for_each_entry_continue(al, ¬es->src->source, node) {
> + if (al->offset == -1)
> + return true;
> + }
> + }
> + return false;
> +}
> +
> +static bool annotate_browser__toggle_source(struct annotate_browser *browser,
> + struct evsel *evsel)
> {
> struct annotation *notes = browser__annotation(&browser->b);
> struct annotation_line *al;
> @@ -377,6 +418,39 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
> browser->b.seek(&browser->b, offset, SEEK_CUR);
> al = list_entry(browser->b.top, struct annotation_line, node);
>
> + if (!annotate_opts.annotate_src)
> + annotate_opts.annotate_src = true;
> +
> + /*
> + * It's about to get source code annotation for the first time.
> + * Drop the existing annotation_lines and get the new one with source.
> + * And then move to the original line at the same asm index.
> + */
> + if (annotate_opts.hide_src_code && !notes->src->tried_source) {
> + struct map_symbol *ms = browser->b.priv;
> + int orig_idx_asm = al->idx_asm;
> +
> + /* annotate again with source code info */
> + annotate_opts.hide_src_code = false;
> + annotated_source__purge(notes->src);
> + symbol__annotate2(ms, evsel, &browser->arch);
> + annotate_opts.hide_src_code = true;
> +
> + /* should be after annotated_source__purge() */
> + notes->src->tried_source = true;
> +
> + if (!annotation__has_source(notes))
> + ui__warning("Annotation has no source code.");
> +
> + browser->b.entries = ¬es->src->source;
> + al = annotate_browser__find_new_asm_line(browser, orig_idx_asm);
> + if (unlikely(al == NULL)) {
> + al = list_first_entry(¬es->src->source,
> + struct annotation_line, node);
> + }
> + browser->b.seek(&browser->b, al->idx_asm, SEEK_SET);
> + }
> +
> if (annotate_opts.hide_src_code) {
> if (al->idx_asm < offset)
> offset = al->idx;
> @@ -833,7 +907,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
> nd = browser->curr_hot;
> break;
> case 's':
> - if (annotate_browser__toggle_source(browser))
> + if (annotate_browser__toggle_source(browser, evsel))
> ui_helpline__puts(help);
> annotate__scnprintf_title(hists, title, sizeof(title));
> annotate_browser__show(&browser->b, title, help);
> @@ -1011,6 +1085,12 @@ int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
> ui__error("Couldn't annotate %s:\n%s", sym->name, msg);
> return -1;
> }
> +
> + if (!annotate_opts.hide_src_code) {
> + notes->src->tried_source = true;
> + if (!annotation__has_source(notes))
> + ui__warning("Annotation has no source code.");
> + }
> }
>
> ui_helpline__push("Press ESC to exit");
> @@ -1025,7 +1105,7 @@ int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
>
> ret = annotate_browser__run(&browser, evsel, hbt);
>
> - if(not_annotated)
> + if (not_annotated && !notes->src->tried_source)
> annotated_source__purge(notes->src);
>
> return ret;
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 264a212b47df850c..0dd475a744b6dfac 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1451,6 +1451,7 @@ void annotated_source__purge(struct annotated_source *as)
> list_del_init(&al->node);
> disasm_line__free(disasm_line(al));
> }
> + as->tried_source = false;
> }
>
> static size_t disasm_line__fprintf(struct disasm_line *dl, FILE *fp)
> @@ -2280,6 +2281,7 @@ void annotation_options__init(void)
> opt->annotate_src = true;
> opt->offset_level = ANNOTATION__OFFSET_JUMP_TARGETS;
> opt->percent_type = PERCENT_PERIOD_LOCAL;
> + opt->hide_src_code = true;
> opt->hide_src_code_on_title = true;
> }
>
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index bbb89b32f398b3c9..8b5131d257b01e3e 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -294,6 +294,7 @@ struct annotated_source {
> int nr_entries;
> int nr_asm_entries;
> int max_jump_sources;
> + bool tried_source;
> u64 start;
> struct {
> u8 addr;
> diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
> index 8f0eb56c6fc66a96..ff475a239f4b0db4 100644
> --- a/tools/perf/util/disasm.c
> +++ b/tools/perf/util/disasm.c
> @@ -2284,6 +2284,13 @@ int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
> }
> }
>
> + /* FIXME: LLVM and CAPSTONE should support source code */
> + if (options->annotate_src && !options->hide_src_code) {
> + err = symbol__disassemble_objdump(symfs_filename, sym, args);
> + if (err == 0)
> + goto out_remove_tmp;
> + }
> +
> err = -1;
> for (u8 i = 0; i < ARRAY_SIZE(options->disassemblers) && err != 0; i++) {
> enum perf_disassembler dis = options->disassemblers[i];
> --
> 2.50.0.727.gbf7dc18ff4-goog
>
Powered by blists - more mailing lists