From: sashiko-bot@kernel.org
To: "Ian Rogers" <irogers@google.com>
Cc: linux-perf-users@vger.kernel.org
Subject: Re: [PATCH v8 03/17] perf capstone: Determine architecture from e_machine
Date: Sat, 02 May 2026 07:58:50 +0000 [thread overview]
Message-ID: <20260502075851.7303EC19425@smtp.kernel.org> (raw)
In-Reply-To: <20260502065935.291960-4-irogers@google.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Hardcoded endianness in e_machine_to_capstone() breaks disassembly for bi-endian architectures.
- [High] thread__e_machine() fallback to EM_HOST breaks cross-architecture disassembly for kernel code and BPF programs.
--
commit a3ff746537f2f9677ae9d6427555ff27588e61a4
Author: Ian Rogers <irogers@google.com>
perf capstone: Determine architecture from e_machine
This commit refactors capstone initialization to use the e_machine value
from the ELF header instead of the architecture string, adding support
for MIPS and RISCV translations.
Note: The format of this review has been altered to provide a free-form
summary of the findings without directly quoting the patch code, due to
recitation restrictions.
The new e_machine_to_capstone() function hardcodes CS_MODE_BIG_ENDIAN for
EM_PPC, EM_PPC64, and EM_MIPS, and implicitly assumes little-endian for
EM_AARCH64 and EM_ARM.
Does this approach break disassembly for bi-endian architectures?
Linux fully supports variants like ppc64le, mipsel, armeb, and aarch64_be
under these same e_machine IDs. Because the initialization now succeeds
with the hardcoded endianness rather than returning -1, it actively bypasses
the functional fallback to objdump. Since objdump natively reads EI_DATA
from the ELF header to determine the correct endianness, skipping it leaves
these architectures with incorrect disassembly output.
Could we read EI_DATA alongside e_machine to set the endianness mode correctly?
Additionally, the updated code replaces machine__normalized_is() with
thread__e_machine() to determine the target architecture.
Will this break cross-architecture disassembly for kernel code and BPF
programs?
Previously, machine__normalized_is() correctly determined the architecture
from machine->env->arch, which is populated by the perf.data file header.
The new approach searches DSOs and falls back to dso__e_machine(), which
is hardcoded to return EM_HOST (the compile-time host architecture of the
perf binary) for kernel components like DSO_BINARY_TYPE__KALLSYMS,
DSO_BINARY_TYPE__VMLINUX, and DSO_BINARY_TYPE__BPF_PROG_INFO.
If a user performs cross-architecture analysis, such as analyzing an ARM64
perf.data file on an x86_64 host, won't thread__e_machine() return
EM_X86_64? This would cause Capstone to decode ARM64 kernel or JIT
instructions as x86_64 instructions.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260502065935.291960-1-irogers@google.com?part=3
next prev parent reply other threads:[~2026-05-02 7:58 UTC|newest]
Thread overview: 106+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-19 11:38 [PATCH v2] perf symbol: Remove psw_idle() from list of idle symbols Thomas Richter
2026-02-19 11:55 ` Jan Polensky
2026-02-23 21:46 ` Namhyung Kim
2026-02-23 23:14 ` Arnaldo Melo
2026-03-02 18:43 ` Arnaldo Carvalho de Melo
2026-03-02 19:44 ` Ian Rogers
2026-03-04 14:34 ` Arnaldo Carvalho de Melo
2026-03-02 23:43 ` [PATCH v1] perf symbol: Lazily compute idle and use the perf_env Ian Rogers
2026-03-24 17:14 ` Ian Rogers
2026-03-25 6:58 ` Namhyung Kim
2026-03-25 15:58 ` Ian Rogers
2026-03-25 16:18 ` [PATCH v2] " Ian Rogers
2026-03-26 7:20 ` Honglei Wang
2026-03-26 15:11 ` Ian Rogers
2026-03-26 17:45 ` [PATCH v3 0/2] perf symbol/env: ELF machine clean up and lazy idle computation Ian Rogers
2026-03-26 17:45 ` [PATCH v3 1/2] perf env: Add perf_env__e_machine helper and use in perf_env__arch Ian Rogers
2026-03-26 17:45 ` [PATCH v3 2/2] perf symbol: Lazily compute idle and use the perf_env Ian Rogers
2026-03-27 6:56 ` Honglei Wang
2026-03-27 4:50 ` [PATCH v4 0/2] perf symbol/env: ELF machine clean up and lazy idle computation Ian Rogers
2026-03-27 4:50 ` [PATCH v4 1/2] perf env: Add perf_env__e_machine helper and use in perf_env__arch Ian Rogers
2026-04-06 5:05 ` Namhyung Kim
2026-04-06 15:36 ` Ian Rogers
2026-03-27 4:50 ` [PATCH v4 2/2] perf symbol: Lazily compute idle and use the perf_env Ian Rogers
2026-04-06 5:10 ` Namhyung Kim
2026-04-06 16:11 ` Ian Rogers
2026-04-06 17:09 ` [PATCH v5 0/3] perf symbol/env: ELF machine clean up and lazy idle computation Ian Rogers
2026-04-06 17:09 ` [PATCH v5 1/3] perf env: Add perf_env__e_machine helper and use in perf_env__arch Ian Rogers
2026-04-06 17:09 ` [PATCH v5 2/3] perf env: Add helper to lazily compute the os_release Ian Rogers
2026-04-06 17:09 ` [PATCH v5 3/3] perf symbol: Lazily compute idle and use the perf_env Ian Rogers
2026-04-09 23:06 ` [PATCH v6 0/3] perf symbol/env: ELF machine clean up and lazy idle computation Ian Rogers
2026-04-09 23:06 ` [PATCH v6 1/3] perf env: Add perf_env__e_machine helper and use in perf_env__arch Ian Rogers
2026-04-09 23:37 ` sashiko-bot
2026-05-01 18:20 ` [PATCH v7 0/4] perf symbol/env: ELF machine clean up and lazy idle computation Ian Rogers
2026-05-01 18:20 ` [PATCH v7 1/4] perf env: Add perf_env__e_machine helper and use in perf_env__arch Ian Rogers
2026-05-01 18:56 ` sashiko-bot
2026-05-01 18:20 ` [PATCH v7 2/4] perf env: Add helper to lazily compute the os_release Ian Rogers
2026-05-01 19:20 ` sashiko-bot
2026-05-01 18:20 ` [PATCH v7 3/4] perf symbol: Add setters for bitfields sharing a byte to avoid concurrent update issues Ian Rogers
2026-05-01 19:42 ` sashiko-bot
2026-05-01 18:20 ` [PATCH v7 4/4] perf symbol: Lazily compute idle and use a global lock for updates Ian Rogers
2026-05-01 20:13 ` sashiko-bot
2026-05-02 6:59 ` [PATCH v8 00/17] perf symbol/env: ELF machine clean up and lazy idle computation Ian Rogers
2026-05-02 6:59 ` [PATCH v8 01/17] perf env: Add perf_env__e_machine helper and use in perf_env__arch Ian Rogers
2026-05-02 7:56 ` sashiko-bot
2026-05-02 6:59 ` [PATCH v8 02/17] perf tests topology: Switch env->arch use to env->e_machine Ian Rogers
2026-05-02 6:59 ` [PATCH v8 03/17] perf capstone: Determine architecture from e_machine Ian Rogers
2026-05-02 7:58 ` sashiko-bot [this message]
2026-05-02 6:59 ` [PATCH v8 04/17] perf print_insn: Use e_machine for fallback IP length check Ian Rogers
2026-05-02 7:55 ` sashiko-bot
2026-05-02 6:59 ` [PATCH v8 05/17] perf machine: Use perf_env e_machine rather than arch Ian Rogers
2026-05-02 7:11 ` sashiko-bot
2026-05-02 6:59 ` [PATCH v8 06/17] perf sample-raw: " Ian Rogers
2026-05-02 6:59 ` [PATCH v8 07/17] perf sort: " Ian Rogers
2026-05-02 6:59 ` [PATCH v8 08/17] perf symbol: Avoid use of machine__is Ian Rogers
2026-05-02 7:17 ` sashiko-bot
2026-05-02 6:59 ` [PATCH v8 09/17] perf arch common: Use perf_env e_machine rather than arch Ian Rogers
2026-05-02 7:59 ` sashiko-bot
2026-05-02 6:59 ` [PATCH v8 10/17] perf header: In print_pmu_caps use perf_env e_machine Ian Rogers
2026-05-02 6:59 ` [PATCH v8 11/17] perf c2c: Use perf_env e_machine rather than arch Ian Rogers
2026-05-02 7:44 ` sashiko-bot
2026-05-02 6:59 ` [PATCH v8 12/17] perf lock-contention: " Ian Rogers
2026-05-02 6:59 ` [PATCH v8 13/17] perf env: Refactor perf_env__arch_strerrno Ian Rogers
2026-05-02 6:59 ` [PATCH v8 14/17] perf env: Remove unused perf_env__raw_arch Ian Rogers
2026-05-02 6:59 ` [PATCH v8 15/17] perf env: Add helper to lazily compute the os_release Ian Rogers
2026-05-02 7:53 ` sashiko-bot
2026-05-02 6:59 ` [PATCH v8 16/17] perf symbol: Add setters for bitfields sharing a byte to avoid concurrent update issues Ian Rogers
2026-05-02 7:55 ` sashiko-bot
2026-05-02 6:59 ` [PATCH v8 17/17] perf symbol: Lazily compute idle and use a global lock for updates Ian Rogers
2026-05-03 0:22 ` [PATCH v9 00/18] perf symbol/env: ELF machine clean up and lazy idle computation Ian Rogers
2026-05-03 0:22 ` [PATCH v9 01/18] perf env: Add perf_env__e_machine helper and use in perf_env__arch Ian Rogers
2026-05-03 0:52 ` sashiko-bot
2026-05-04 1:35 ` Namhyung Kim
2026-05-03 0:22 ` [PATCH v9 02/18] perf tests topology: Switch env->arch use to env->e_machine Ian Rogers
2026-05-03 0:22 ` [PATCH v9 03/18] perf env, dso, thread: Add _endian variants for e_machine helpers Ian Rogers
2026-05-03 0:39 ` sashiko-bot
2026-05-03 0:22 ` [PATCH v9 04/18] perf capstone: Determine architecture from e_machine Ian Rogers
2026-05-03 0:50 ` sashiko-bot
2026-05-03 0:22 ` [PATCH v9 05/18] perf print_insn: Use e_machine for fallback IP length check Ian Rogers
2026-05-03 0:22 ` [PATCH v9 06/18] perf symbol: Avoid use of machine__is Ian Rogers
2026-05-03 0:51 ` sashiko-bot
2026-05-03 0:22 ` [PATCH v9 07/18] perf machine: Use perf_env e_machine rather than arch Ian Rogers
2026-05-03 1:00 ` sashiko-bot
2026-05-03 0:22 ` [PATCH v9 08/18] perf sample-raw: " Ian Rogers
2026-05-03 0:22 ` [PATCH v9 09/18] perf sort: " Ian Rogers
2026-05-03 0:22 ` [PATCH v9 10/18] perf arch common: " Ian Rogers
2026-05-03 0:38 ` sashiko-bot
2026-05-03 0:22 ` [PATCH v9 11/18] perf header: In print_pmu_caps use perf_env e_machine Ian Rogers
2026-05-03 0:22 ` [PATCH v9 12/18] perf c2c: Use perf_env e_machine rather than arch Ian Rogers
2026-05-03 0:22 ` [PATCH v9 13/18] perf lock-contention: " Ian Rogers
2026-05-03 0:22 ` [PATCH v9 14/18] perf env: Refactor perf_env__arch_strerrno Ian Rogers
2026-05-03 1:11 ` sashiko-bot
2026-05-03 0:22 ` [PATCH v9 15/18] perf env: Remove unused perf_env__raw_arch Ian Rogers
2026-05-03 0:22 ` [PATCH v9 16/18] perf env: Add helper to lazily compute the os_release Ian Rogers
2026-05-03 1:00 ` sashiko-bot
2026-05-03 0:22 ` [PATCH v9 17/18] perf symbol: Add setters for bitfields sharing a byte to avoid concurrent update issues Ian Rogers
2026-05-03 0:59 ` sashiko-bot
2026-05-03 0:22 ` [PATCH v9 18/18] perf symbol: Lazily compute idle Ian Rogers
2026-05-03 1:11 ` sashiko-bot
2026-04-09 23:06 ` [PATCH v6 2/3] perf env: Add helper to lazily compute the os_release Ian Rogers
2026-04-09 23:50 ` sashiko-bot
2026-04-09 23:06 ` [PATCH v6 3/3] perf symbol: Lazily compute idle and use the perf_env Ian Rogers
2026-04-10 0:11 ` sashiko-bot
2026-03-27 6:00 ` [PATCH v2] perf tests task-analyzer: Write test files to tmpdir Ian Rogers
2026-03-31 7:22 ` Namhyung Kim
2026-03-31 17:58 ` Ian Rogers
2026-04-01 3:41 ` Namhyung Kim
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=20260502075851.7303EC19425@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=irogers@google.com \
--cc=linux-perf-users@vger.kernel.org \
--cc=sashiko@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.