All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Ian Rogers" <irogers@google.com>
Cc: linux-perf-users@vger.kernel.org, bpf@vger.kernel.org
Subject: Re: [PATCH v3 6/7] perf annotate: Support BPF JIT disassembly via genelf
Date: Tue, 09 Jun 2026 18:49:04 +0000	[thread overview]
Message-ID: <20260609184905.2B4681F00893@smtp.kernel.org> (raw)
In-Reply-To: <20260609182108.975586-7-irogers@google.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] JIT ELF generation uses host-native endianness and ELF class, breaking cross-platform analysis of BPF programs.
- [High] Mismatch between ELF text section address and DWARF debug line base address for JIT profiling.
--

commit 3c8720c4a004bb87cca5176252e00dcb2ade47e7
Author: Ian Rogers <irogers@google.com>

perf annotate: Support BPF JIT disassembly via genelf

For in-memory BPF DSOs (DSO_BINARY_TYPE__BPF_PROG_INFO), write the
JITted instruction buffer to a temporary ELF file on disk using the
existing genelf framework (jit_write_elf). Reroute disassembly to
this temporary ELF file, allowing objdump and libasm to disassemble it
natively. Clean up the temporary file afterward.

> diff --git a/tools/perf/util/genelf.c b/tools/perf/util/genelf.c
> index 14882def9704c..c3af040121eef 100644
> --- a/tools/perf/util/genelf.c
> +++ b/tools/perf/util/genelf.c
[ ... ]
> @@ -218,9 +218,9 @@ jit_write_elf(int fd, uint64_t load_addr __maybe_unused, const char *sym,
>  
>  	ehdr->e_ident[EI_DATA] = GEN_ELF_ENDIAN;
>  	ehdr->e_ident[EI_CLASS] = GEN_ELF_CLASS;
> -	ehdr->e_machine = GEN_ELF_ARCH;
> +	ehdr->e_machine = e_machine;

[Severity: High]
Does this hardcoded endianness and class break cross-platform trace analysis?

Since e_machine is now dynamically set, if a 64-bit x86 host analyzes a
32-bit or big-endian BPF trace (like ARM32 or s390x), wouldn't this generate a
corrupt ELF container (e.g., a 64-bit little-endian ELF claiming to be
EM_S390) and cause objdump to reject the file or decode with the wrong
endianness?

[ ... ]
> @@ -230,8 +230,8 @@ jit_write_elf(int fd, uint64_t load_addr __maybe_unused, const char *sym,
>  	phdr = elf_newphdr(e, 1);
>  	phdr[0].p_type = PT_LOAD;
>  	phdr[0].p_offset = GEN_ELF_TEXT_OFFSET;
> -	phdr[0].p_vaddr = GEN_ELF_TEXT_OFFSET;
> -	phdr[0].p_paddr = GEN_ELF_TEXT_OFFSET;
> +	phdr[0].p_vaddr = load_addr;
> +	phdr[0].p_paddr = load_addr;

[Severity: High]
Does changing the ELF virtual addresses to use load_addr break DWARF mapping?

In tools/perf/util/genelf_debug.c:jit_process_debug_info(), the code still
passes the hardcoded GEN_ELF_TEXT_OFFSET to add_debug_line():

	add_debug_line(dl, debug, nr_debug_entries, GEN_ELF_TEXT_OFFSET);

If the ELF is generated with load_addr but the DWARF .debug_line information
uses a base address of GEN_ELF_TEXT_OFFSET, will perf annotate and standard
debuggers fail to map source code lines to the disassembled instructions?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609182108.975586-1-irogers@google.com?part=6

  reply	other threads:[~2026-06-09 18:49 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-09  7:07 [PATCH v2 0/7] perf annotate: Add elfutils libasm disassembler support Ian Rogers
2026-06-09  7:07 ` [PATCH v2 1/7] tools build: Add feature check for elfutils libasm Ian Rogers
2026-06-09  7:21   ` sashiko-bot
2026-06-09  7:07 ` [PATCH v2 2/7] perf build: Add build support and capability " Ian Rogers
2026-06-09  7:19   ` sashiko-bot
2026-06-09  7:07 ` [PATCH v2 3/7] perf annotate: Implement elfutils libasm disassembler backend Ian Rogers
2026-06-09  7:07 ` [PATCH v2 4/7] perf annotate: Add --disassembler command-line option Ian Rogers
2026-06-09  7:07 ` [PATCH v2 5/7] perf test: Enhance annotate test coverage and isolate config Ian Rogers
2026-06-09  7:15   ` sashiko-bot
2026-06-09  7:07 ` [PATCH v2 6/7] perf annotate: Support BPF JIT disassembly via genelf Ian Rogers
2026-06-09  7:22   ` sashiko-bot
2026-06-09  7:07 ` [PATCH v2 7/7] perf test: Add BPF JIT annotation test coverage for all disassemblers Ian Rogers
2026-06-09  7:18   ` sashiko-bot
2026-06-09 18:21 ` [PATCH v3 0/7] perf annotate: Add elfutils libasm disassembler and BPF JIT disassembly support Ian Rogers
2026-06-09 18:21   ` [PATCH v3 1/7] tools build: Add feature check for elfutils libasm Ian Rogers
2026-06-09 18:46     ` sashiko-bot
2026-06-09 18:21   ` [PATCH v3 2/7] perf build: Add build support and capability " Ian Rogers
2026-06-09 18:21   ` [PATCH v3 3/7] perf annotate: Implement elfutils libasm disassembler backend Ian Rogers
2026-06-09 18:52     ` sashiko-bot
2026-06-09 18:21   ` [PATCH v3 4/7] perf annotate: Add --disassembler command-line option Ian Rogers
2026-06-09 18:21   ` [PATCH v3 5/7] perf test: Enhance annotate test coverage and isolate config Ian Rogers
2026-06-09 18:46     ` sashiko-bot
2026-06-09 18:21   ` [PATCH v3 6/7] perf annotate: Support BPF JIT disassembly via genelf Ian Rogers
2026-06-09 18:49     ` sashiko-bot [this message]
2026-06-09 18:21   ` [PATCH v3 7/7] perf test: Add BPF JIT annotation test coverage for all disassemblers Ian Rogers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260609184905.2B4681F00893@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=irogers@google.com \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.