[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aDeIWI8DH9MOTy1z@x1>
Date: Wed, 28 May 2025 19:04:08 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Ian Rogers <irogers@...gle.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Namhyung Kim <namhyung@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>,
Kan Liang <kan.liang@...ux.intel.com>,
Miguel Ojeda <ojeda@...nel.org>,
Alex Gaynor <alex.gaynor@...il.com>,
Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <benno.lossin@...ton.me>,
Andreas Hindborg <a.hindborg@...nel.org>,
Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
Danilo Krummrich <dakr@...nel.org>,
Jiapeng Chong <jiapeng.chong@...ux.alibaba.com>,
James Clark <james.clark@...aro.org>,
Howard Chu <howardchu95@...il.com>,
Weilin Wang <weilin.wang@...el.com>,
Stephen Brennan <stephen.s.brennan@...cle.com>,
Andi Kleen <ak@...ux.intel.com>, Dmitry Vyukov <dvyukov@...gle.com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] perf symbol: Move demangling code out of symbol-elf.c
On Wed, May 28, 2025 at 02:08:58PM -0700, Ian Rogers wrote:
> symbol-elf.c is used when building with libelf, symbol-minimal is used
> otherwise. There is no reason the demangling code with no dependencies
> on libelf is part of symbol-elf.c so move to symbol.c. This allows
> demangling tests to pass with NO_LIBELF=1.
>
> Structurally, while moving the functions rename demangle_sym to
> dso__demangle_sym which is already a function exposed in symbol.h and
> the only purpose of which in symbol-elf.c was to call
> demangle_sym. Change the calls to demangle_sym in symbol-elf.c to
> calls to dso__demangle_sym.
>
> Signed-off-by: Ian Rogers <irogers@...gle.com>
> ---
> v3. Separate patch as most of v2 landed already. Fix issues with
> libelf build while merging dso__demangle_sym and demangle_sym to
> remove some unneeded complexity.
> The thread comm_lock fix also isn't included:
> https://lore.kernel.org/lkml/20250528032637.198960-8-irogers@google.com/
> as discussion on that has moved to a revert by Arnaldo:
> https://lore.kernel.org/lkml/aDcyVLVpZRui1ole@x1/
Thanks, applied to perf-tools-next,
- Arnaldo
> ---
> tools/perf/util/demangle-cxx.h | 2 +
> tools/perf/util/symbol-elf.c | 92 ++------------------------------
> tools/perf/util/symbol-minimal.c | 7 ---
> tools/perf/util/symbol.c | 82 ++++++++++++++++++++++++++++
> 4 files changed, 87 insertions(+), 96 deletions(-)
>
> diff --git a/tools/perf/util/demangle-cxx.h b/tools/perf/util/demangle-cxx.h
> index 26b5b66c0b4e..9359937a881a 100644
> --- a/tools/perf/util/demangle-cxx.h
> +++ b/tools/perf/util/demangle-cxx.h
> @@ -2,6 +2,8 @@
> #ifndef __PERF_DEMANGLE_CXX
> #define __PERF_DEMANGLE_CXX 1
>
> +#include <stdbool.h>
> +
> #ifdef __cplusplus
> extern "C" {
> #endif
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 8734e8b6cf84..01818abd02df 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -13,17 +13,12 @@
> #include "maps.h"
> #include "symbol.h"
> #include "symsrc.h"
> -#include "demangle-cxx.h"
> -#include "demangle-ocaml.h"
> -#include "demangle-java.h"
> -#include "demangle-rust-v0.h"
> #include "machine.h"
> #include "vdso.h"
> #include "debug.h"
> #include "util/copyfile.h"
> #include <linux/ctype.h>
> #include <linux/kernel.h>
> -#include <linux/log2.h>
> #include <linux/zalloc.h>
> #include <linux/string.h>
> #include <symbol/kallsyms.h>
> @@ -280,82 +275,6 @@ static int elf_read_program_header(Elf *elf, u64 vaddr, GElf_Phdr *phdr)
> return -1;
> }
>
> -static bool want_demangle(bool is_kernel_sym)
> -{
> - return is_kernel_sym ? symbol_conf.demangle_kernel : symbol_conf.demangle;
> -}
> -
> -/*
> - * Demangle C++ function signature, typically replaced by demangle-cxx.cpp
> - * version.
> - */
> -#ifndef HAVE_CXA_DEMANGLE_SUPPORT
> -char *cxx_demangle_sym(const char *str __maybe_unused, bool params __maybe_unused,
> - bool modifiers __maybe_unused)
> -{
> -#ifdef HAVE_LIBBFD_SUPPORT
> - int flags = (params ? DMGL_PARAMS : 0) | (modifiers ? DMGL_ANSI : 0);
> -
> - return bfd_demangle(NULL, str, flags);
> -#elif defined(HAVE_CPLUS_DEMANGLE_SUPPORT)
> - int flags = (params ? DMGL_PARAMS : 0) | (modifiers ? DMGL_ANSI : 0);
> -
> - return cplus_demangle(str, flags);
> -#else
> - return NULL;
> -#endif
> -}
> -#endif /* !HAVE_CXA_DEMANGLE_SUPPORT */
> -
> -static char *demangle_sym(struct dso *dso, int kmodule, const char *elf_name)
> -{
> - struct demangle rust_demangle = {
> - .style = DemangleStyleUnknown,
> - };
> - char *demangled = NULL;
> -
> - /*
> - * We need to figure out if the object was created from C++ sources
> - * DWARF DW_compile_unit has this, but we don't always have access
> - * to it...
> - */
> - if (!want_demangle((dso && dso__kernel(dso)) || kmodule))
> - return demangled;
> -
> - rust_demangle_demangle(elf_name, &rust_demangle);
> - if (rust_demangle_is_known(&rust_demangle)) {
> - /* A rust mangled name. */
> - if (rust_demangle.mangled_len == 0)
> - return demangled;
> -
> - for (size_t buf_len = roundup_pow_of_two(rust_demangle.mangled_len * 2);
> - buf_len < 1024 * 1024; buf_len += 32) {
> - char *tmp = realloc(demangled, buf_len);
> -
> - if (!tmp) {
> - /* Failure to grow output buffer, return what is there. */
> - return demangled;
> - }
> - demangled = tmp;
> - if (rust_demangle_display_demangle(&rust_demangle, demangled, buf_len,
> - /*alternate=*/true) == OverflowOk)
> - return demangled;
> - }
> - /* Buffer exceeded sensible bounds, return what is there. */
> - return demangled;
> - }
> -
> - demangled = cxx_demangle_sym(elf_name, verbose > 0, verbose > 0);
> - if (demangled)
> - return demangled;
> -
> - demangled = ocaml_demangle_sym(elf_name);
> - if (demangled)
> - return demangled;
> -
> - return java_demangle_sym(elf_name, JAVA_DEMANGLE_NORET);
> -}
> -
> struct rel_info {
> u32 nr_entries;
> u32 *sorted;
> @@ -641,7 +560,7 @@ static bool get_plt_got_name(GElf_Shdr *shdr, size_t i,
> /* Get the associated symbol */
> gelf_getsym(di->dynsym_data, vr->sym_idx, &sym);
> sym_name = elf_sym__name(&sym, di->dynstr_data);
> - demangled = demangle_sym(di->dso, 0, sym_name);
> + demangled = dso__demangle_sym(di->dso, /*kmodule=*/0, sym_name);
> if (demangled != NULL)
> sym_name = demangled;
>
> @@ -839,7 +758,7 @@ int dso__synthesize_plt_symbols(struct dso *dso, struct symsrc *ss)
> gelf_getsym(syms, get_rel_symidx(&ri, idx), &sym);
>
> elf_name = elf_sym__name(&sym, symstrs);
> - demangled = demangle_sym(dso, 0, elf_name);
> + demangled = dso__demangle_sym(dso, /*kmodule=*/0, elf_name);
> if (demangled)
> elf_name = demangled;
> if (*elf_name)
> @@ -868,11 +787,6 @@ int dso__synthesize_plt_symbols(struct dso *dso, struct symsrc *ss)
> return 0;
> }
>
> -char *dso__demangle_sym(struct dso *dso, int kmodule, const char *elf_name)
> -{
> - return demangle_sym(dso, kmodule, elf_name);
> -}
> -
> /*
> * Align offset to 4 bytes as needed for note name and descriptor data.
> */
> @@ -1861,7 +1775,7 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
> }
> }
>
> - demangled = demangle_sym(dso, kmodule, elf_name);
> + demangled = dso__demangle_sym(dso, kmodule, elf_name);
> if (demangled != NULL)
> elf_name = demangled;
>
> diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
> index 36c1d3090689..c73fe2e09fe9 100644
> --- a/tools/perf/util/symbol-minimal.c
> +++ b/tools/perf/util/symbol-minimal.c
> @@ -355,13 +355,6 @@ void symbol__elf_init(void)
> {
> }
>
> -char *dso__demangle_sym(struct dso *dso __maybe_unused,
> - int kmodule __maybe_unused,
> - const char *elf_name __maybe_unused)
> -{
> - return NULL;
> -}
> -
> bool filename__has_section(const char *filename __maybe_unused, const char *sec __maybe_unused)
> {
> return false;
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index fe801880afea..8b30c6f16a9e 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -19,6 +19,11 @@
> #include "build-id.h"
> #include "cap.h"
> #include "cpumap.h"
> +#include "debug.h"
> +#include "demangle-cxx.h"
> +#include "demangle-java.h"
> +#include "demangle-ocaml.h"
> +#include "demangle-rust-v0.h"
> #include "dso.h"
> #include "util.h" // lsdir()
> #include "debug.h"
> @@ -36,6 +41,7 @@
> #include "header.h"
> #include "path.h"
> #include <linux/ctype.h>
> +#include <linux/log2.h>
> #include <linux/zalloc.h>
>
> #include <elf.h>
> @@ -2648,3 +2654,79 @@ int symbol__validate_sym_arguments(void)
> }
> return 0;
> }
> +
> +static bool want_demangle(bool is_kernel_sym)
> +{
> + return is_kernel_sym ? symbol_conf.demangle_kernel : symbol_conf.demangle;
> +}
> +
> +/*
> + * Demangle C++ function signature, typically replaced by demangle-cxx.cpp
> + * version.
> + */
> +#ifndef HAVE_CXA_DEMANGLE_SUPPORT
> +char *cxx_demangle_sym(const char *str __maybe_unused, bool params __maybe_unused,
> + bool modifiers __maybe_unused)
> +{
> +#ifdef HAVE_LIBBFD_SUPPORT
> + int flags = (params ? DMGL_PARAMS : 0) | (modifiers ? DMGL_ANSI : 0);
> +
> + return bfd_demangle(NULL, str, flags);
> +#elif defined(HAVE_CPLUS_DEMANGLE_SUPPORT)
> + int flags = (params ? DMGL_PARAMS : 0) | (modifiers ? DMGL_ANSI : 0);
> +
> + return cplus_demangle(str, flags);
> +#else
> + return NULL;
> +#endif
> +}
> +#endif /* !HAVE_CXA_DEMANGLE_SUPPORT */
> +
> +char *dso__demangle_sym(struct dso *dso, int kmodule, const char *elf_name)
> +{
> + struct demangle rust_demangle = {
> + .style = DemangleStyleUnknown,
> + };
> + char *demangled = NULL;
> +
> + /*
> + * We need to figure out if the object was created from C++ sources
> + * DWARF DW_compile_unit has this, but we don't always have access
> + * to it...
> + */
> + if (!want_demangle((dso && dso__kernel(dso)) || kmodule))
> + return demangled;
> +
> + rust_demangle_demangle(elf_name, &rust_demangle);
> + if (rust_demangle_is_known(&rust_demangle)) {
> + /* A rust mangled name. */
> + if (rust_demangle.mangled_len == 0)
> + return demangled;
> +
> + for (size_t buf_len = roundup_pow_of_two(rust_demangle.mangled_len * 2);
> + buf_len < 1024 * 1024; buf_len += 32) {
> + char *tmp = realloc(demangled, buf_len);
> +
> + if (!tmp) {
> + /* Failure to grow output buffer, return what is there. */
> + return demangled;
> + }
> + demangled = tmp;
> + if (rust_demangle_display_demangle(&rust_demangle, demangled, buf_len,
> + /*alternate=*/true) == OverflowOk)
> + return demangled;
> + }
> + /* Buffer exceeded sensible bounds, return what is there. */
> + return demangled;
> + }
> +
> + demangled = cxx_demangle_sym(elf_name, verbose > 0, verbose > 0);
> + if (demangled)
> + return demangled;
> +
> + demangled = ocaml_demangle_sym(elf_name);
> + if (demangled)
> + return demangled;
> +
> + return java_demangle_sym(elf_name, JAVA_DEMANGLE_NORET);
> +}
> --
> 2.49.0.1204.g71687c7c1d-goog
>
Powered by blists - more mailing lists