perf symbol: remove Rust symbol workarounds

From: Gary Guo <[email protected]>

Due to an off-by-one error introduced in commit 73bbb94466fd ("kallsyms:
support "big" kernel symbols"), long symbols (which are currently only produced by Rust) can have their symbol type being wrongly parsed by kernel/kallsyms.c.

This has been fixed in commit f3f9f42232dee5 ("kallsyms: Fix wrong "big" kernel symbol type read from procfs"), and these symbols are now reported correctly.

Drop the workaround in perf symbol that filter out these symbol types.

Specifically, '1' and 'l' can never be generated by nm -- 'u' does indicate GNU unique, however such symbols are only generated by G++ for C++ templates, and are never generated by LLVM (LLVM generates weak symbols in such cases instead). 'N' can appear if symbols exist inside debug sections, and 'n' may appear for symbols inside note sections, however these sections do not typically have symbol (and they're explicitly filtered out by kallsyms).

Therefore, the previous occurrence of these symbols types must be due to the off-by-one error and can be safely removed.

Cc Arnaldo Carvalho de Melo <[email protected]>

Acked-by Miguel Ojeda <[email protected]>

Signed-off-by Gary Guo <[email protected]>

---
v2:
- Removed the "__" change as suggested by Miguel
- Expanded the commit message to mention why "N" and "n" cannot appear
- Link to v1: https://lore.kernel.org/rust-for-linux/[email protected]/#t

---
 tools/perf/util/symbol.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

tools/perf/util/symbol.c +4 -15

index 814f960fa8f8d..8662001e1e25b 100644
@@ -104,21 +104,10 @@ static enum dso_binary_type binary_type_symtab[] = {
 
 #define DSO_BINARY_TYPE__SYMTAB_CNT ARRAY_SIZE(binary_type_symtab)
 
-static bool symbol_type__filter(char __symbol_type)
-{
-	// Since 'U' == undefined and 'u' == unique global symbol, we can't use toupper there
-	// 'N' is for debugging symbols, 'n' is a non-data, non-code, non-debug read-only section.
-	// According to 'man nm'.
-	// 'N' first seen in:
-	// ffffffff9b35d130 N __pfx__RNCINvNtNtNtCsbDUBuN8AbD4_4core4iter8adapters3map12map_try_foldjNtCs6vVzKs5jPr6_12drm_panic_qr7VersionuINtNtNtBa_3ops12control_flow11ControlFlowB10_ENcB10_0NCINvNvNtNtNtB8_6traits8iterator8Iterator4find5checkB10_NCNvMB12_B10_13from_segments0E0E0B12_
-	// a seemingly Rust mangled name
-	// Ditto for '1':
-	// root@x1:~# grep ' 1 ' /proc/kallsyms
-	// ffffffffb098bc00 1 __pfx__RNCINvNtNtNtCsfwaGRd4cjqE_4core4iter8adapters3map12map_try_foldjNtCskFudTml27HW_12drm_panic_qr7VersionuINtNtNtBa_3ops12control_flow11ControlFlowB10_ENcB10_0NCINvNvNtNtNtB8_6traits8iterator8Iterator4find5checkB10_NCNvMB12_B10_13from_segments0E0E0B12_
-	// ffffffffb098bc10 1 _RNCINvNtNtNtCsfwaGRd4cjqE_4core4iter8adapters3map12map_try_foldjNtCskFudTml27HW_12drm_panic_qr7VersionuINtNtNtBa_3ops12control_flow11ControlFlowB10_ENcB10_0NCINvNvNtNtNtB8_6traits8iterator8Iterator4find5checkB10_NCNvMB12_B10_13from_segments0E0E0B12_
-	char symbol_type = toupper(__symbol_type);
-	return symbol_type == 'T' || symbol_type == 'W' || symbol_type == 'D' || symbol_type == 'B' ||
-	       __symbol_type == 'u' || __symbol_type == 'l' || __symbol_type == 'N' || __symbol_type == '1';
+static bool symbol_type__filter(char symbol_type)
+{
+	symbol_type = toupper(symbol_type);
+	return symbol_type == 'T' || symbol_type == 'W' || symbol_type == 'D' || symbol_type == 'B';
 }
 
 static int prefix_underscores_count(const char *str)

base-commit: 3609fa95fb0f2c1b099e69e56634edb8fc03f87c

Signature · 1 line

2.51.2

Quoted 6 lines · Gary Guo

From: Gary Guo <[email protected]>

Due to an off-by-one error introduced in commit 73bbb94466fd ("kallsyms:
support "big" kernel symbols"), long symbols (which are currently only produced by Rust) can have their symbol type being wrongly parsed by kernel/kallsyms.c.

Looks sane, applied to perf-tools-next,

Quoted 63 lines

This has been fixed in commit f3f9f42232dee5 ("kallsyms: Fix wrong "big" kernel symbol type read from procfs"), and these symbols are now reported correctly.

Drop the workaround in perf symbol that filter out these symbol types.

Specifically, '1' and 'l' can never be generated by nm -- 'u' does indicate GNU unique, however such symbols are only generated by G++ for C++ templates, and are never generated by LLVM (LLVM generates weak symbols in such cases instead). 'N' can appear if symbols exist inside debug sections, and 'n' may appear for symbols inside note sections, however these sections do not typically have symbol (and they're explicitly filtered out by kallsyms).

Therefore, the previous occurrence of these symbols types must be due to the off-by-one error and can be safely removed.

Cc Arnaldo Carvalho de Melo <[email protected]>

Acked-by Miguel Ojeda <[email protected]>

Signed-off-by Gary Guo <[email protected]>

---
v2:
- Removed the "__" change as suggested by Miguel
- Expanded the commit message to mention why "N" and "n" cannot appear
- Link to v1: https://lore.kernel.org/rust-for-linux/[email protected]/#t

---
 tools/perf/util/symbol.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

tools/perf/util/symbol.c +4 -15

index 814f960fa8f8d..8662001e1e25b 100644
@@ -104,21 +104,10 @@ static enum dso_binary_type binary_type_symtab[] = {
 
 #define DSO_BINARY_TYPE__SYMTAB_CNT ARRAY_SIZE(binary_type_symtab)
 
-static bool symbol_type__filter(char __symbol_type)
-{
-	// Since 'U' == undefined and 'u' == unique global symbol, we can't use toupper there
-	// 'N' is for debugging symbols, 'n' is a non-data, non-code, non-debug read-only section.
-	// According to 'man nm'.
-	// 'N' first seen in:
-	// ffffffff9b35d130 N __pfx__RNCINvNtNtNtCsbDUBuN8AbD4_4core4iter8adapters3map12map_try_foldjNtCs6vVzKs5jPr6_12drm_panic_qr7VersionuINtNtNtBa_3ops12control_flow11ControlFlowB10_ENcB10_0NCINvNvNtNtNtB8_6traits8iterator8Iterator4find5checkB10_NCNvMB12_B10_13from_segments0E0E0B12_
-	// a seemingly Rust mangled name
-	// Ditto for '1':
-	// root@x1:~# grep ' 1 ' /proc/kallsyms
-	// ffffffffb098bc00 1 __pfx__RNCINvNtNtNtCsfwaGRd4cjqE_4core4iter8adapters3map12map_try_foldjNtCskFudTml27HW_12drm_panic_qr7VersionuINtNtNtBa_3ops12control_flow11ControlFlowB10_ENcB10_0NCINvNvNtNtNtB8_6traits8iterator8Iterator4find5checkB10_NCNvMB12_B10_13from_segments0E0E0B12_
-	// ffffffffb098bc10 1 _RNCINvNtNtNtCsfwaGRd4cjqE_4core4iter8adapters3map12map_try_foldjNtCskFudTml27HW_12drm_panic_qr7VersionuINtNtNtBa_3ops12control_flow11ControlFlowB10_ENcB10_0NCINvNvNtNtNtB8_6traits8iterator8Iterator4find5checkB10_NCNvMB12_B10_13from_segments0E0E0B12_
-	char symbol_type = toupper(__symbol_type);
-	return symbol_type == 'T' || symbol_type == 'W' || symbol_type == 'D' || symbol_type == 'B' ||
-	       __symbol_type == 'u' || __symbol_type == 'l' || __symbol_type == 'N' || __symbol_type == '1';
+static bool symbol_type__filter(char symbol_type)
+{
+	symbol_type = toupper(symbol_type);
+	return symbol_type == 'T' || symbol_type == 'W' || symbol_type == 'D' || symbol_type == 'B';
 }
 
 static int prefix_underscores_count(const char *str)

base-commit: 3609fa95fb0f2c1b099e69e56634edb8fc03f87c

Signature · 1 line

2.51.2

I'm afraid there may be some outdated kernels which don't have the fix yet. Maybe make sense to wait some more time for them to disappear?

Quoted 3 lines · Namhyung Kim

I'm afraid there may be some outdated kernels which don't have the fix yet. Maybe make sense to wait some more time for them to disappear?

Hmm... Do you mean that someone would use a new perf with an old kernel? Is that supported etc.?

In any case, the fix is getting backported all the way to 6.1.y, i.e. all the supported LTSs:

761e965d2c61 ("kallsyms: Fix wrong "big" kernel symbol type read
from procfs")
d5fe0c115bbc ("kallsyms: Fix wrong "big" kernel symbol type read
from procfs")
d46ef2bf91ef ("kallsyms: Fix wrong "big" kernel symbol type read
from procfs")

And the one in this thread could in principle be backported too to 6.18.y now that the fix is there, no?

Yep, perf userspace can be released separate with the kernel and it's recommended to run the latest version regardless of kernel versions. So from the tools perspective, it should support any kernels.

Thanks for your work! I think it should be fine once the backports land to distro kernels.

Quoted 7 lines · Arnaldo Carvalho de Melo

Quoted 4 lines · Gary Guo

Due to an off-by-one error introduced in commit 73bbb94466fd ("kallsyms:
support "big" kernel symbols"), long symbols (which are currently only produced by Rust) can have their symbol type being wrongly parsed by kernel/kallsyms.c.

Looks sane, applied to perf-tools-next,

I'm afraid there may be some outdated kernels which don't have the fix yet. Maybe make sense to wait some more time for them to disappear?

Quoted 11 lines · Namhyung Kim

Quoted 7 lines · Arnaldo Carvalho de Melo

Quoted 4 lines · Gary Guo

Due to an off-by-one error introduced in commit 73bbb94466fd ("kallsyms:
support "big" kernel symbols"), long symbols (which are currently only produced by Rust) can have their symbol type being wrongly parsed by kernel/kallsyms.c.

Looks sane, applied to perf-tools-next,

I'm afraid there may be some outdated kernels which don't have the fix yet. Maybe make sense to wait some more time for them to disappear?

Right, I took it more as it seems to be just a blip in time, something that came and went quickly and that most likely will not cause problems, i.e. just a handful of extra symbols in the lookup data structures in those particular kernels and a new 'perf test' tool with these reverts, if run on those particular kernels would then fail for that particular test case, for a known reason.

So with the above rationale you still feel strongly about having it applied?