[PATCH 0/5] perf symbol: Speed up demangling of long Rust symbols

[PATCH 0/5] perf symbol: Speed up demangling of long Rust symbols

[Michal Pluta]

dso__demangle_sym() demangles Rust v0 symbols itself instead of using
a helper like the other demanglers. When a demangled name doesn't fit
the buffer, it grows the buffer by a fixed 32 bytes and reformats the
whole name from scratch. The number of attempts grows with the name's
length, so the total work is quadratic. Rust programs with deeply
nested generics can have very long demangled names, and I noticed this
as real time spent in perf report on perf.data recorded from launching
Bevy and Typst.

Patch 3 is the actual optimisation, doubling the size of the buffer
instead.  Patch 5 moves the Rust-specific logic into its own function,
matching how the other demanglers are called.  I think the cleanup is
worthwhile but I understand it comes down to personal taste and the
patch can be skipped if maintainers would rather keep
dso__demangle_sym() as is.

I used an LLM to review my patches and find the issues with the
demangle tests and the unterminated buffer bug.

Testing:
- 'perf test' passes, including the demangle Java, OCaml and Rust
  suites (patch 3 adds a case for a symbol that exceeds the buffer
  limit).
- 'perf report' output is unchanged before and after this series on
  every Rust binary I tried it on.
- Built and tested on x86-64 with a 7.0 kernel.

The series is based on perf-tools-next, commit 0ae6fc78c5ce ("perf
timechart: Remove the unused use_old_power_events variable").

Michal Pluta (5):
  perf test demangle: Fail when demangling fails
  perf symbol: Don't return an unterminated Rust demangle buffer
  perf symbol: Grow the Rust demangle buffer geometrically
  perf symbol: Shrink the demangled Rust buffer to fit
  perf symbol: Move Rust demangling into its own function

 tools/perf/tests/demangle-java-test.c    |  1 +
 tools/perf/tests/demangle-rust-v0-test.c | 13 +++++
 tools/perf/util/symbol.c                 | 60 +++++++++++++++---------
 3 files changed, 52 insertions(+), 22 deletions(-)


base-commit: 0ae6fc78c5ce0dfd18d8712a50f0fd4602eff103
-- 
2.43.0

[PATCH 1/5] perf test demangle: Fail when demangling fails

[Michal Pluta]

The Rust and Java demangle tests only log a NULL result from
dso__demangle_sym() and carry on, so a symbol that fails to demangle
doesn't fail the test.  Set TEST_FAIL.

Assisted-by: LLM
Signed-off-by: Michal Pluta <michalpl2003@gmail.com>
---
 tools/perf/tests/demangle-java-test.c    | 1 +
 tools/perf/tests/demangle-rust-v0-test.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/tools/perf/tests/demangle-java-test.c b/tools/perf/tests/demangle-java-test.c
index 0fb3e5a4a0ed..a8eb0f4effad 100644
--- a/tools/perf/tests/demangle-java-test.c
+++ b/tools/perf/tests/demangle-java-test.c
@@ -33,6 +33,7 @@ static int test__demangle_java(struct test_suite *test __maybe_unused, int subte
 		if (!buf) {
 			pr_debug("FAILED to demangle: \"%s\"\n \"%s\"\n", test_cases[i].mangled,
 				 test_cases[i].demangled);
+			ret = TEST_FAIL;
 			continue;
 		}
 		if (strcmp(buf, test_cases[i].demangled)) {
diff --git a/tools/perf/tests/demangle-rust-v0-test.c b/tools/perf/tests/demangle-rust-v0-test.c
index 904f966c65d7..ee4ddb61174b 100644
--- a/tools/perf/tests/demangle-rust-v0-test.c
+++ b/tools/perf/tests/demangle-rust-v0-test.c
@@ -58,6 +58,7 @@ static int test__demangle_rust(struct test_suite *test __maybe_unused, int subte
 		if (!buf) {
 			pr_debug("FAILED to demangle: \"%s\"\n \"%s\"\n", test_cases[i].mangled,
 				 test_cases[i].demangled);
+			ret = TEST_FAIL;
 			continue;
 		}
 		if (strcmp(buf, test_cases[i].demangled)) {
-- 
2.43.0

[PATCH 2/5] perf symbol: Don't return an unterminated Rust demangle buffer

[Michal Pluta]

When dso__demangle_sym() runs out of retries or fails to grow the
output buffer for a Rust v0 symbol, it returns the buffer and callers
blindly use it as the demangled symbol name.  However,
rust_demangle_display_demangle() doesn't NUL terminate its output when
it reports OverflowOverflow, so the buffer isn't a string.

Free the buffer and return NULL in both cases.  Callers already fall back
to the mangled name when no demangled name is returned.

Fixes: e20848c317b5 ("perf symbol-elf: Integrate rust-v0 demangling")
Assisted-by: LLM
Signed-off-by: Michal Pluta <michalpl2003@gmail.com>
---
 tools/perf/util/symbol.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 163652f071c6..3cb42805a82f 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -2762,17 +2762,16 @@ char *dso__demangle_sym(struct dso *dso, int kmodule, const char *elf_name)
 		     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;
-			}
+			if (!tmp)
+				break;
 			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;
+		/* Failure to grow output buffer or buffer exceeded sensible bounds. */
+		free(demangled);
+		return NULL;
 	}
 
 	demangled = cxx_demangle_sym(elf_name, verbose > 0, verbose > 0);
-- 
2.43.0

[PATCH 3/5] perf symbol: Grow the Rust demangle buffer geometrically

[Michal Pluta]

When a demangled Rust name doesn't fit in the provided buffer,
dso__demangle_sym() adds 32 bytes to it and formats the name again from
the start.  The number of attempts grows with the length of the output,
so the total work is quadratic.  Names with deeply nested generic types
need many attempts, leading to noticeable slowdowns in larger programs.

Double the buffer instead, reaching the maximum buffer limit exactly
rather than stopping 32 bytes early.  The demangled names are
unchanged.

Add a test for a symbol whose expansion exceeds the bound.

Signed-off-by: Michal Pluta <michalpl2003@gmail.com>
---
 tools/perf/tests/demangle-rust-v0-test.c | 12 ++++++++++++
 tools/perf/util/symbol.c                 | 10 ++++++++--
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/tools/perf/tests/demangle-rust-v0-test.c b/tools/perf/tests/demangle-rust-v0-test.c
index ee4ddb61174b..d1e0636d73bc 100644
--- a/tools/perf/tests/demangle-rust-v0-test.c
+++ b/tools/perf/tests/demangle-rust-v0-test.c
@@ -69,6 +69,18 @@ static int test__demangle_rust(struct test_suite *test __maybe_unused, int subte
 		free(buf);
 	}
 
+	/*
+	 * A symbol with more lifetimes bound than fit in the largest buffer
+	 * must fail to demangle rather than give a truncated name.
+	 */
+	buf = dso__demangle_sym(/*dso=*/NULL, /*kmodule=*/0, "_RINvC1a1fFGZZZZZZ_EuE");
+	if (buf) {
+		pr_debug("FAILED: symbol larger than the buffer limit demangled to %zu bytes\n",
+			 strlen(buf));
+		ret = TEST_FAIL;
+		free(buf);
+	}
+
 	return ret;
 }
 
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 3cb42805a82f..1a52bc2980a0 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -2737,29 +2737,35 @@ char *cxx_demangle_sym(const char *str __maybe_unused, bool params __maybe_unuse
 }
 #endif /* !HAVE_CXA_DEMANGLE_SUPPORT */
 
+/* Buffer limit for a demangled Rust symbol name. */
+#define RUST_DEMANGLE_MAX_LEN (1024 * 1024)
+
 char *dso__demangle_sym(struct dso *dso, int kmodule, const char *elf_name)
 {
 	struct demangle rust_demangle = {
 		.style = DemangleStyleUnknown,
 	};
 	char *demangled = NULL;
+	size_t buf_len;
 
 	/*
 	 * 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) {
+		for (buf_len = min_t(size_t, roundup_pow_of_two(rust_demangle.mangled_len * 2),
+				     RUST_DEMANGLE_MAX_LEN);
+		     buf_len <= RUST_DEMANGLE_MAX_LEN;
+		     buf_len *= 2) {
 			char *tmp = realloc(demangled, buf_len);
 
 			if (!tmp)
-- 
2.43.0

[PATCH 4/5] perf symbol: Shrink the demangled Rust buffer to fit

[Michal Pluta]

The buffer a demangled Rust name is returned in is sized to whatever
attempt succeeded, which is usually larger than the string it holds.
Shrink it to the string's exact length before returning it.

Signed-off-by: Michal Pluta <michalpl2003@gmail.com>
---
 tools/perf/util/symbol.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 1a52bc2980a0..858a14a91f79 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -2772,8 +2772,10 @@ char *dso__demangle_sym(struct dso *dso, int kmodule, const char *elf_name)
 				break;
 			demangled = tmp;
 			if (rust_demangle_display_demangle(&rust_demangle, demangled, buf_len,
-							   /*alternate=*/true) == OverflowOk)
-				return demangled;
+							   /*alternate=*/true) == OverflowOk) {
+				tmp = realloc(demangled, strlen(demangled) + 1);
+				return tmp ?: demangled;
+			}
 		}
 		/* Failure to grow output buffer or buffer exceeded sensible bounds. */
 		free(demangled);
-- 
2.43.0

[PATCH 5/5] perf symbol: Move Rust demangling into its own function

[Michal Pluta]

dso__demangle_sym() is the only place that implements demangling logic
inline.  The other three are one call each to a helper.  Move the Rust
parsing and buffer growth into rust_demangle_sym(), with the same
signature as the other three.

A Rust name that fails to demangle now falls through to the other
demanglers, matching how the other demanglers chain into each other.
A v0 name can't be misread by any of them, since none accepts its "_R"
prefix.  A legacy name can only fail here on allocation failure or
hitting the output bound, neither of which is language-specific.

Assisted-by: LLM
Signed-off-by: Michal Pluta <michalpl2003@gmail.com>
---
 tools/perf/util/symbol.c | 61 +++++++++++++++++++++++-----------------
 1 file changed, 35 insertions(+), 26 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 858a14a91f79..89db9a670833 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -2740,47 +2740,56 @@ char *cxx_demangle_sym(const char *str __maybe_unused, bool params __maybe_unuse
 /* Buffer limit for a demangled Rust symbol name. */
 #define RUST_DEMANGLE_MAX_LEN (1024 * 1024)
 
-char *dso__demangle_sym(struct dso *dso, int kmodule, const char *elf_name)
+static char *rust_demangle_sym(const char *elf_name)
 {
 	struct demangle rust_demangle = {
 		.style = DemangleStyleUnknown,
 	};
 	char *demangled = NULL;
 	size_t buf_len;
 
+	rust_demangle_demangle(elf_name, &rust_demangle);
+	if (!rust_demangle_is_known(&rust_demangle))
+		return NULL;
+
+	if (rust_demangle.mangled_len == 0)
+		return NULL;
+
+	for (buf_len = min_t(size_t, roundup_pow_of_two(rust_demangle.mangled_len * 2),
+			     RUST_DEMANGLE_MAX_LEN);
+	     buf_len <= RUST_DEMANGLE_MAX_LEN;
+	     buf_len *= 2) {
+		char *tmp = realloc(demangled, buf_len);
+
+		if (!tmp)
+			break;
+		demangled = tmp;
+		if (rust_demangle_display_demangle(&rust_demangle, demangled, buf_len,
+						   /*alternate=*/true) == OverflowOk) {
+			tmp = realloc(demangled, strlen(demangled) + 1);
+			return tmp ?: demangled;
+		}
+	}
+	/* Failure to grow output buffer or buffer exceeded sensible bounds. */
+	free(demangled);
+	return NULL;
+}
+
+char *dso__demangle_sym(struct dso *dso, int kmodule, const char *elf_name)
+{
+	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 (buf_len = min_t(size_t, roundup_pow_of_two(rust_demangle.mangled_len * 2),
-				     RUST_DEMANGLE_MAX_LEN);
-		     buf_len <= RUST_DEMANGLE_MAX_LEN;
-		     buf_len *= 2) {
-			char *tmp = realloc(demangled, buf_len);
-
-			if (!tmp)
-				break;
-			demangled = tmp;
-			if (rust_demangle_display_demangle(&rust_demangle, demangled, buf_len,
-							   /*alternate=*/true) == OverflowOk) {
-				tmp = realloc(demangled, strlen(demangled) + 1);
-				return tmp ?: demangled;
-			}
-		}
-		/* Failure to grow output buffer or buffer exceeded sensible bounds. */
-		free(demangled);
-		return NULL;
-	}
+	demangled = rust_demangle_sym(elf_name);
+	if (demangled)
+		return demangled;
 
 	demangled = cxx_demangle_sym(elf_name, verbose > 0, verbose > 0);
 	if (demangled)
-- 
2.43.0