Re: [PATCH v1 00/11] perf annotate arch clean up

From: James Clark <james.clark@linaro.org>
To: Ian Rogers <irogers@google.com>
Cc: "Peter Zijlstra" <peterz@infradead.org>,
	"Ingo Molnar" <mingo@redhat.com>,
	"Arnaldo Carvalho de Melo" <acme@kernel.org>,
	"Namhyung Kim" <namhyung@kernel.org>,
	"Alexander Shishkin" <alexander.shishkin@linux.intel.com>,
	"Jiri Olsa" <jolsa@kernel.org>,
	"Adrian Hunter" <adrian.hunter@intel.com>,
	"John Garry" <john.g.garry@oracle.com>,
	"Will Deacon" <will@kernel.org>, "Leo Yan" <leo.yan@linux.dev>,
	"Guo Ren" <guoren@kernel.org>, "Paul Walmsley" <pjw@kernel.org>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Albert Ou" <aou@eecs.berkeley.edu>,
	"Alexandre Ghiti" <alex@ghiti.fr>,
	"Nathan Chancellor" <nathan@kernel.org>,
	"Nick Desaulniers" <nick.desaulniers+lkml@gmail.com>,
	"Bill Wendling" <morbo@google.com>,
	"Justin Stitt" <justinstitt@google.com>,
	"Zecheng Li" <zecheng@google.com>,
	"Tianyou Li" <tianyou.li@intel.com>,
	"Thomas Falcon" <thomas.falcon@intel.com>,
	"Julia Lawall" <Julia.Lawall@inria.fr>,
	"Suchit Karunakaran" <suchitkarunakaran@gmail.com>,
	"Athira Rajeev" <atrajeev@linux.ibm.com>,
	"Aditya Bodkhe" <aditya.b1@linux.ibm.com>,
	"Howard Chu" <howardchu95@gmail.com>,
	"Krzysztof Łopatowski" <krzysztof.m.lopatowski@gmail.com>,
	"Dr. David Alan Gilbert" <linux@treblig.org>,
	"Shimin Guo" <shimin.guo@skydio.com>,
	"Sergei Trofimovich" <slyich@gmail.com>,
	linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-csky@vger.kernel.org,
	linux-riscv@lists.infradead.org
Subject: Re: [PATCH v1 00/11] perf annotate arch clean up
Date: Thu, 22 Jan 2026 10:43:38 +0000	[thread overview]
Message-ID: <9fd8375c-2de7-4305-ac3e-42ff8f919964@linaro.org> (raw)
In-Reply-To: <20260122073127.375139-1-irogers@google.com>

On 22/01/2026 7:31 am, Ian Rogers wrote:
> 2 initial memory leak fix patches for perf annotate.
> 
> Move the disasm.c #included arch files into a new util/annotate-arch
> directory. Make the code compile standalone. Try to better encapsulate
> architecture specific code within the files and with a
> arch__new_<arch> function that allows additional data to be associated
> with the struct arch using the standard container_of trick.
> 
> Make the lookup of struct arch use the ELF machine gather from a
> thread rather than an evsel. For example, the evsel can't determine
> the difference between EM_386 and EM_X86_64 so generally the latter
> has currently been used. This means the dwarf registers are off as
> register 1 is CX with EM_386 and DX with EM_X86_64. To facilitate this
> change map_symbol is refactored to use a struct thread rather than a
> struct maps, the maps being obtainable from the thread.
> 
> To make in and out arguments clearer, for safety, etc. try to constify
> struct arch and its related structs.
> 

Hi Ian,

I get this crash, not sure which commit causes it exactly but it 
happened 2/2 times on both x86 and arm:

  $ perf record -- true
  $ perf report

  < Select first sample >
  < Select annotate >

perf: Segmentation fault
-------- backtrace --------
     #0 0x5df8f58ba973 in ui__signal_backtrace setup.c:110
     #1 0x74373a642520 in __restore_rt libc_sigaction.c:0
     #2 0x5df8f5963f0d in dso__kernel dso.h:532
     #3 0x5df8f5965d0a in __map__const_kmap map.c:627
     #4 0x5df8f5965a36 in map__rip_2objdump map.c:531
     #5 0x5df8f58e5bef in symbol__parse_objdump_line disasm.c:1032
     #6 0x5df8f58e498b in symbol__disassemble_objdump disasm.c:1503
     #7 0x5df8f58e3852 in symbol__disassemble disasm.c:1621
     #8 0x5df8f58c638d in symbol__annotate annotate.c:1055
     #9 0x5df8f58c8628 in symbol__annotate2 annotate.c:2209
     #10 0x5df8f58a36b7 in __hist_entry__tui_annotate annotate.c:1189
     #11 0x5df8f58adf86 in do_annotate hists.c:2488
     #12 0x5df8f58ad762 in evsel__hists_browse hists.c:3441
     #13 0x5df8f58b78f2 in perf_evsel_menu__run hists.c:3556
     #14 0x5df8f58adabc in __evlist__tui_browse_hists hists.c:3643
     #15 0x5df8f58abf3b in evlist__tui_browse_hists hists.c:3691
     #16 0x5df8f571ac3a in report__browse_hists builtin-report.c:688
     #17 0x5df8f5719943 in __cmd_report builtin-report.c:1177
     #18 0x5df8f57181bc in cmd_report builtin-report.c:1890
     #19 0x5df8f57b5c3a in run_builtin perf.c:348
     #20 0x5df8f57b53b3 in handle_internal_command perf.c:398
     #21 0x5df8f57b5b2f in run_argv perf.c:445
     #22 0x5df8f57b5091 in main perf.c:551
     #23 0x74373a629d90 in __libc_start_call_main libc_start_call_main.h:58
     #24 0x74373a629e40 in __libc_start_main@@GLIBC_2.34 libc-start.c:128
     #25 0x5df8f56ead35 in _start perf[55d35]

> Ian Rogers (11):
>    perf maps: Fix reference count leak in maps__find_ams
>    perf annotate: Fix args leak of map_symbol
>    perf disasm: Constify use of struct arch
>    perf disasm: Constify use of struct ins_op
>    perf disasm: Constify use of struct ins
>    perf disasm: Rework the string arch__is to use the ELF machine
>    perf disasm: Don't include C files from the arch directory
>    perf disasm: Refactor ins__is_call/jump to avoid exposing arch ins_ops
>    perf map_symbol: Switch from holding maps to holding thread
>    perf disasm: Refactor arch__find and initialization of arch structs
>    perf disasm: Minor layout tweaks for struct arch
> 
>   tools/perf/arch/arc/annotate/instructions.c   |  11 -
>   .../perf/arch/riscv64/annotate/instructions.c |  36 --
>   tools/perf/ui/browsers/annotate.c             |   6 +-
>   tools/perf/ui/browsers/hists.c                |   3 +-
>   tools/perf/util/Build                         |   1 +
>   tools/perf/util/addr_location.c               |   4 -
>   tools/perf/util/addr_location.h               |   1 -
>   tools/perf/util/annotate-arch/Build           |  11 +
>   tools/perf/util/annotate-arch/annotate-arc.c  |  18 +
>   .../annotate-arch/annotate-arm.c}             |  48 +-
>   .../annotate-arch/annotate-arm64.c}           |  59 +--
>   .../annotate-arch/annotate-csky.c}            |  29 +-
>   .../annotate-arch/annotate-loongarch.c}       |  57 ++-
>   .../annotate-arch/annotate-mips.c}            |  29 +-
>   .../annotate-arch/annotate-powerpc.c}         | 125 +++++-
>   .../util/annotate-arch/annotate-riscv64.c     |  41 ++
>   .../annotate-arch/annotate-s390.c}            |  59 ++-
>   .../annotate-arch/annotate-sparc.c}           |  26 +-
>   .../annotate-arch/annotate-x86.c}             |  89 ++--
>   tools/perf/util/annotate-data.c               |  12 +-
>   tools/perf/util/annotate-data.h               |   2 +-
>   tools/perf/util/annotate.c                    |  88 ++--
>   tools/perf/util/annotate.h                    |  10 +-
>   tools/perf/util/callchain.c                   |  12 +-
>   tools/perf/util/capstone.c                    |  18 +-
>   tools/perf/util/db-export.c                   |   1 -
>   tools/perf/util/disasm.c                      | 424 +++++++-----------
>   tools/perf/util/disasm.h                      |  95 ++--
>   tools/perf/util/event.c                       |   2 -
>   tools/perf/util/hist.c                        |  18 +-
>   tools/perf/util/llvm.c                        |   8 +-
>   tools/perf/util/machine.c                     |  13 +-
>   tools/perf/util/map_symbol.c                  |   5 +-
>   tools/perf/util/map_symbol.h                  |   3 +-
>   tools/perf/util/maps.c                        |   1 +
>   tools/perf/util/sort.c                        |   2 +-
>   tools/perf/util/unwind-libdw.c                |   2 +-
>   tools/perf/util/unwind-libunwind-local.c      |   2 +-
>   38 files changed, 752 insertions(+), 619 deletions(-)
>   delete mode 100644 tools/perf/arch/arc/annotate/instructions.c
>   delete mode 100644 tools/perf/arch/riscv64/annotate/instructions.c
>   create mode 100644 tools/perf/util/annotate-arch/Build
>   create mode 100644 tools/perf/util/annotate-arch/annotate-arc.c
>   rename tools/perf/{arch/arm/annotate/instructions.c => util/annotate-arch/annotate-arm.c} (59%)
>   rename tools/perf/{arch/arm64/annotate/instructions.c => util/annotate-arch/annotate-arm64.c} (70%)
>   rename tools/perf/{arch/csky/annotate/instructions.c => util/annotate-arch/annotate-csky.c} (67%)
>   rename tools/perf/{arch/loongarch/annotate/instructions.c => util/annotate-arch/annotate-loongarch.c} (66%)
>   rename tools/perf/{arch/mips/annotate/instructions.c => util/annotate-arch/annotate-mips.c} (59%)
>   rename tools/perf/{arch/powerpc/annotate/instructions.c => util/annotate-arch/annotate-powerpc.c} (73%)
>   create mode 100644 tools/perf/util/annotate-arch/annotate-riscv64.c
>   rename tools/perf/{arch/s390/annotate/instructions.c => util/annotate-arch/annotate-s390.c} (70%)
>   rename tools/perf/{arch/sparc/annotate/instructions.c => util/annotate-arch/annotate-sparc.c} (85%)
>   rename tools/perf/{arch/x86/annotate/instructions.c => util/annotate-arch/annotate-x86.c} (93%)
> 

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv