[PATCH] perf: Extract is_mapping_symbol() helper for mapping symbol filtering

[PATCH] perf: Extract is_mapping_symbol() helper for mapping symbol filtering

[Rui Qi]

The perf tool currently has ad-hoc logic to filter out ELF mapping
symbols scattered across multiple files and architectures.  ARM,
AArch64 and RISC-V each have their own inline checks in
dso__load_sym_internal(), and kallsym processing in map__process_kallsym_symbol()
has yet another check for ARM module symbols.

This is fragile: adding support for a new architecture or adjusting
which prefixes are considered mapping symbols requires touching
multiple places, and it is easy for the checks to diverge.  It also
does not match the kernel's own is_mapping_symbol() logic, which
additionally covers x86 local symbols ('.L*' and 'L0*').

Introduce a single is_mapping_symbol() inline helper in symbol.h and
convert all call sites to use it.  The helper covers the existing
'$' prefix used by ARM, AArch64 and RISC-V, and also adds the x86
local symbol prefixes so that perf stays consistent with the kernel.
This makes future maintenance simpler and reduces code duplication.

Signed-off-by: Rui Qi <qirui.001@bytedance.com>
---
 tools/perf/util/libbfd.c     |  4 ++++
 tools/perf/util/machine.c    |  6 ++++++
 tools/perf/util/symbol-elf.c | 17 +++--------------
 tools/perf/util/symbol.c     |  8 ++++++--
 tools/perf/util/symbol.h     | 12 ++++++++++++
 5 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/tools/perf/util/libbfd.c b/tools/perf/util/libbfd.c
index c1c12308cc12..a8474d287fc1 100644
--- a/tools/perf/util/libbfd.c
+++ b/tools/perf/util/libbfd.c
@@ -383,6 +383,10 @@ int dso__load_bfd_symbols(struct dso *dso, const char *debugfile)
 		if (bfd2elf_binding(sym) < 0)
 			continue;
 
+		/* Ignore mapping symbols in BFD symbols */
+		if (is_mapping_symbol(bfd_asymbol_name(sym)))
+			continue;
+
 		while (i + 1 < symbols_count &&
 		       bfd_asymbol_section(symbols[i + 1]) == section &&
 		       bfd2elf_binding(symbols[i + 1]) < 0)
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index e76f8c86e62a..cce3faa70b32 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -769,6 +769,12 @@ static int machine__process_ksymbol_register(struct machine *machine,
 		dso = dso__get(map__dso(map));
 	}
 
+	/* Ignore mapping symbols in ksymbol events */
+	if (is_mapping_symbol(event->ksymbol.name)) {
+		err = 0;
+		goto out;
+	}
+
 	sym = symbol__new(map__map_ip(map, map__start(map)),
 			  event->ksymbol.len,
 			  0, 0, event->ksymbol.name);
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 7afa8a117139..ddf5a9c4d6c9 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1592,20 +1592,9 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
 		if (!is_label && !elf_sym__filter(&sym))
 			continue;
 
-		/* Reject ARM ELF "mapping symbols": these aren't unique and
-		 * don't identify functions, so will confuse the profile
-		 * output: */
-		if (ehdr.e_machine == EM_ARM || ehdr.e_machine == EM_AARCH64) {
-			if (elf_name[0] == '$' && strchr("adtx", elf_name[1])
-			    && (elf_name[2] == '\0' || elf_name[2] == '.'))
-				continue;
-		}
-
-		/* Reject RISCV ELF "mapping symbols" */
-		if (ehdr.e_machine == EM_RISCV) {
-			if (elf_name[0] == '$' && strchr("dx", elf_name[1]))
-				continue;
-		}
+		/* Ignore mapping symbols in ELF files */
+		if (is_mapping_symbol(elf_name))
+			continue;
 
 		if (runtime_ss->opdsec && sym.st_shndx == runtime_ss->opdidx) {
 			u32 offset = sym.st_value - syms_ss->opdshdr.sh_addr;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index fcaeeddbbb6b..1e7ffeb6a539 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -770,8 +770,8 @@ static int map__process_kallsym_symbol(void *arg, const char *name,
 	if (!symbol_type__filter(type))
 		return 0;
 
-	/* Ignore local symbols for ARM modules */
-	if (name[0] == '$')
+	/* Ignore mapping symbols */
+	if (is_mapping_symbol(name))
 		return 0;
 
 	/*
@@ -1566,6 +1566,10 @@ static int dso__load_perf_map(const char *map_path, struct dso *dso)
 		if (len + 2 >= line_len)
 			continue;
 
+		/* Ignore mapping symbols in perf map */
+		if (is_mapping_symbol(line + len))
+			continue;
+
 		sym = symbol__new(start, size, STB_GLOBAL, STT_FUNC, line + len);
 
 		if (sym == NULL)
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index bd6eb90c8668..9528fd777b87 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -28,6 +28,18 @@ struct maps;
 struct option;
 struct build_id;
 
+/*
+ * Ignore mapping symbols, matching kernel is_mapping_symbol() logic.
+ */
+static inline bool is_mapping_symbol(const char *str)
+{
+	if (str[0] == '.' && str[1] == 'L')
+		return true;
+	if (str[0] == 'L' && str[1] == '0')
+		return true;
+	return str[0] == '$';
+}
+
 /*
  * libelf 0.8.x and earlier do not support ELF_C_READ_MMAP;
  * for newer versions we can use mmap to reduce memory usage:
-- 
2.20.1

Re: [PATCH] perf: Extract is_mapping_symbol() helper for mapping symbol filtering

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Destructive empty map insertion in `machine__process_ksymbol_register()` corrupts the kernel map tree.
- [High] Over-broad filtering unconditionally drops valid user-space and JIT symbols.
--

commit ae672508d7c2692194bb670cadab94e218fa13f7
Author: Rui Qi <qirui.001@bytedance.com>

perf: Extract is_mapping_symbol() helper for mapping symbol filtering

This commit extracts ad-hoc ELF mapping symbol filtering logic into a unified
is_mapping_symbol() helper in symbol.h. It replaces architecture-specific
checks with a single check that ignores any symbol starting with '$', '.L', or
'L0', and applies it across multiple symbol loading paths.

> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -769,6 +769,12 @@ static int machine__process_ksymbol_register(struct machine *machine,
>  		dso = dso__get(map__dso(map));
>  	}
>  
> +	/* Ignore mapping symbols in ksymbol events */
> +	if (is_mapping_symbol(event->ksymbol.name)) {
> +		err = 0;
> +		goto out;
> +	}
> +

Could this early return corrupt the kernel map tree? If a new map was just
allocated and inserted via maps__fixup_overlap_and_insert() earlier in this
function, taking this goto out leaves the empty, unpopulated map in the
rbtree. 

Since maps__fixup_overlap_and_insert() actively splits or deletes
existing overlapping maps, does this permanently punch a hole in the address
space layout?

Would it be safer to perform the is_mapping_symbol() check at the very
beginning of the function before any state mutations occur?

[ ... ]

> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -1592,20 +1592,9 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
>  		if (!is_label && !elf_sym__filter(&sym))
>  			continue;
>  
> -		/* Reject ARM ELF "mapping symbols": these aren't unique and
> -		 * don't identify functions, so will confuse the profile
> -		 * output: */
> -		if (ehdr.e_machine == EM_ARM || ehdr.e_machine == EM_AARCH64) {
> -			if (elf_name[0] == '$' && strchr("adtx", elf_name[1])
> -			    && (elf_name[2] == '\0' || elf_name[2] == '.'))
> -				continue;
> -		}
> -
> -		/* Reject RISCV ELF "mapping symbols" */
> -		if (ehdr.e_machine == EM_RISCV) {
> -			if (elf_name[0] == '$' && strchr("dx", elf_name[1]))
> -				continue;
> -		}
> +		/* Ignore mapping symbols in ELF files */
> +		if (is_mapping_symbol(elf_name))
> +			continue;

The previous logic carefully restricted filtering to exact ARM/RISC-V mapping
prefixes (like '$a', '$d', '$x') on those specific architectures. 

Will unconditionally dropping all symbols starting with '$' across all
architectures silently discard legitimate user-space functions? GCC permits
'$' in identifiers, and it is frequently used in user-space code.

[ ... ]

> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -1566,6 +1566,10 @@ static int dso__load_perf_map(const char *map_path, struct dso *dso)
>  		if (len + 2 >= line_len)
>  			continue;
>  
> +		/* Ignore mapping symbols in perf map */
> +		if (is_mapping_symbol(line + len))
> +			continue;
> +

Similarly, does applying this kernel-centric heuristic here discard valid
symbols for user-space JIT environments? The '$' character is a valid and
common prefix for function names in languages like Java and Scala.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260504090609.1801880-1-qirui.001@bytedance.com?part=1