[PATCH v2 0/3] perf unwind: Don't load symbols when the caller doesn't need them

[PATCH v2 0/3] perf unwind: Don't load symbols when the caller doesn't need them

[Matthew Wood]

When perf only needs to know which DSOs a callchain passes through, as
build-id marking does at the end of 'perf record' and in 'perf inject
-b', it resolves the callchain without symbols. The DWARF unwinders
ignore that and look up the symbol of every frame anyway, which loads
and demangles the whole symbol table of each DSO they visit.

On arm64 every frame pointer sample also gets a short DWARF
unwind to find the leaf function's caller; since v6.17 (commit
29be60c93d2d, "perf build-id: Mark DSO in sample callchains") the end of
'perf record -a --call-graph fp' loads the symbols of every user binary
it sampled. On a 128-CPU arm64 host,

  perf record -a -e cycles:u --call-graph fp -- sleep 2

never finishes writing perf.data, with over 1GB of heap growth in 5
seconds.

Just skipping the symbol lookup isn't enough, because loading the
symbols had two side effects that the unwinders depended on:

 - it set dso__symsrc_filename(), which is how both unwinders find a
   separate debuginfo file, for instance to use its .debug_frame;

 - it read a missing build-id from inside the DSO's mount namespace,
   which libdw needs to fall back to the build-id cache.

Patches 1 and 2 provide both without loading symbols, so the frames
that come back don't change:

  1: move dso__read_build_id() from 'perf inject' to dso.c
  2: add dso__find_debuginfo() and use it in both unwinders
  3: pass the symbols flag down to the unwinders and honour it, which
     also makes the arm64 leaf frame caller unwind skip symbols

Testing compared v7.3-rc7 with this series, on x86 and natively on a
CentOS Stream 10 arm64 host:

 - The 'perf test' dwarf unwind and 'perf inject' DWARF callchain
   conversion tests pass on both. The libdw multi-threaded unwind test
   passes on x86 and is skipped on the arm64 host.

 - 'perf inject -b' on an x86 'perf record --call-graph dwarf' profile
   of a Python workload goes from 0.24s and 58.5MB max RSS to 0.04s and
   15.5MB, with the same build-ids injected.

 - A test library built with -fno-asynchronous-unwind-tables, so that
   its CFI is only in .debug_frame, then split with 'objcopy
   --only-keep-debug', sits between two other DSOs in the sampled
   stacks. With libdw, on both architectures, and with the debuginfo
   found either through .gnu_debuglink or only in perf's build-id cache,
   'perf script' output for --call-graph dwarf profiles, and on arm64
   for fp profiles, is unchanged, and 'perf inject -b' still unwinds
   past the library and marks the same DSOs. An earlier version that
   only passed the flag down lost every frame past the library in all
   but 5 of 4045 samples in 'perf script' on x86, and unwound 10127
   instead of 33738 frames in 'perf inject -b'.

 - libunwind on arm64 returns no frames with or without this series:
   since v7.2, libunwind-arm64.c checks HAVE_LIBUNWIND_ARM64_SUPPORT,
   but Makefile.config defines HAVE_LIBUNWIND_AARCH64_SUPPORT. With that
   macro defined by hand, and a cast to make its .debug_frame code
   build, libunwind results for the test library are the same with and
   without this series. Neither gets past the library: the .debug_frame
   code looks broken, as the table that dwarf_find_debug_frame() returns
   is passed back to libunwind as a remote table. On x86, EL9's
   libunwind lacks .debug_frame support, so only the lookup could be
   exercised there: libunwind now finds the debuginfo from the first
   sample, instead of once the DSO's symbols have been loaded. These
   libunwind problems are independent of this series.

Not tested: the build-id cache fallback for DSOs in another mount
namespace when the MMAP2 events don't carry build-ids.

Changes since v1:
  - Complete re-work on approach based on feedback
  - Link: https://lore.kernel.org/linux-perf-users/20261014151326.4142140-1-thepacketgeek@gmail.com/

Matthew Wood (3):
  perf dso: Move dso__read_build_id() from perf inject to dso.c
  perf unwind: Find separate debuginfo without loading symbols
  perf unwind: Don't look up symbols when the caller doesn't want them

 tools/perf/builtin-inject.c                   | 25 --------
 tools/perf/tests/dwarf-unwind.c               |  3 +-
 .../util/arm64-frame-pointer-unwind-support.c |  7 ++-
 tools/perf/util/dso.c                         | 60 +++++++++++++++++++
 tools/perf/util/dso.h                         |  2 +
 tools/perf/util/machine.c                     |  7 +--
 tools/perf/util/unwind-libdw.c                | 43 ++++++++++---
 tools/perf/util/unwind-libdw.h                |  1 +
 tools/perf/util/unwind-libunwind.c            | 43 +++++++------
 tools/perf/util/unwind.c                      |  7 ++-
 tools/perf/util/unwind.h                      | 11 +++-
 11 files changed, 140 insertions(+), 69 deletions(-)


base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
--
2.53.0-Meta

[PATCH v2 1/3] perf dso: Move dso__read_build_id() from perf inject to dso.c

[Matthew Wood]

dso__read_build_id() reads a DSO's build-id from inside its mount
namespace when it isn't known yet. The libdw unwinder is going to need
it, so move it out of builtin-inject.c and make it available to the rest
of perf.

No functional change.

Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>
---
 tools/perf/builtin-inject.c | 25 -------------------------
 tools/perf/util/dso.c       | 29 +++++++++++++++++++++++++++++
 tools/perf/util/dso.h       |  1 +
 3 files changed, 30 insertions(+), 25 deletions(-)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 70bbfad5653e..7e4b45a5062c 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -892,31 +892,6 @@ static int perf_event__repipe_tracing_data(const struct perf_tool *tool,
 }
 #endif
 
-static int dso__read_build_id(struct dso *dso)
-{
-	struct nscookie nsc;
-	struct build_id bid = { .size = 0, };
-
-	if (dso__has_build_id(dso))
-		return 0;
-
-	mutex_lock(dso__lock(dso));
-	nsinfo__mountns_enter(dso__nsinfo(dso), &nsc);
-	if (filename__read_build_id(dso__long_name(dso), &bid) > 0)
-		dso__set_build_id(dso, &bid);
-	else if (dso__nsinfo(dso)) {
-		char *new_name = dso__filename_with_chroot(dso, dso__long_name(dso));
-
-		if (new_name && filename__read_build_id(new_name, &bid) > 0)
-			dso__set_build_id(dso, &bid);
-		free(new_name);
-	}
-	nsinfo__mountns_exit(&nsc);
-	mutex_unlock(dso__lock(dso));
-
-	return dso__has_build_id(dso) ? 0 : -1;
-}
-
 static struct strlist *perf_inject__parse_known_build_ids(
 	const char *known_build_ids_string)
 {
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 42bfe30a3b51..e03e2cb9212b 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -569,6 +569,35 @@ char *dso__filename_with_chroot(const struct dso *dso, const char *filename)
 	return filename_with_chroot(nsinfo__pid(dso__nsinfo_const(dso)), filename);
 }
 
+/*
+ * Read the build-id from the DSO's file, inside its mount namespace, if it
+ * isn't known yet.
+ */
+int dso__read_build_id(struct dso *dso)
+{
+	struct nscookie nsc;
+	struct build_id bid = { .size = 0, };
+
+	if (dso__has_build_id(dso))
+		return 0;
+
+	mutex_lock(dso__lock(dso));
+	nsinfo__mountns_enter(dso__nsinfo(dso), &nsc);
+	if (filename__read_build_id(dso__long_name(dso), &bid) > 0)
+		dso__set_build_id(dso, &bid);
+	else if (dso__nsinfo(dso)) {
+		char *new_name = dso__filename_with_chroot(dso, dso__long_name(dso));
+
+		if (new_name && filename__read_build_id(new_name, &bid) > 0)
+			dso__set_build_id(dso, &bid);
+		free(new_name);
+	}
+	nsinfo__mountns_exit(&nsc);
+	mutex_unlock(dso__lock(dso));
+
+	return dso__has_build_id(dso) ? 0 : -1;
+}
+
 static char *dso__get_filename(struct dso *dso, const char *root_dir,
 			       bool *decomp)
 {
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 55c4aaa53c38..ece54b9b8552 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -805,6 +805,7 @@ static inline bool dso__has_symbols(const struct dso *dso)
 }
 
 char *dso__filename_with_chroot(const struct dso *dso, const char *filename);
+int dso__read_build_id(struct dso *dso);
 
 bool dso__sorted_by_name(const struct dso *dso);
 void dso__set_sorted_by_name(struct dso *dso);
-- 
2.53.0-Meta

Re: [PATCH v2 1/3] perf dso: Move dso__read_build_id() from perf inject to dso.c

[sashiko-bot]

> dso__read_build_id() reads a DSO's build-id from inside its mount
> namespace when it isn't known yet. The libdw unwinder is going to need
> it, so move it out of builtin-inject.c and make it available to the rest
> of perf.
> 
> No functional change.
> 
> Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261017000158.1178269-1-thepacketgeek@gmail.com?part=1

[PATCH v2 2/3] perf unwind: Find separate debuginfo without loading symbols

[Matthew Wood]

Both DWARF unwinders find the .debug_frame in a separate debuginfo file
through dso__symsrc_filename(), which only dso__load() sets, when it
finds a file with a symbol table. libdw's __find_debuginfo() hands out
nothing else, and libdw doesn't ask again for a module after a failed
lookup. libunwind's read_unwind_spec_debug_frame() falls back to
following .gnu_debuglink, which misses debuginfo that is only reachable
by build-id, such as under /usr/lib/debug/.build-id/ or in perf's
build-id cache.

The unwinders load the symbols of every frame they visit today, so this
mostly works, although libunwind can't use such a debuginfo file until
an earlier sample has loaded the DSO's symbols. A following patch lets
callers that don't need symbols skip loading them, and then nothing
would set dso__symsrc_filename().

Add dso__find_debuginfo(), which looks for a separate debuginfo file in
the places dso__load() would, reading the build-id first so that the
build-id based locations can be checked, but without loading symbols.
Use it in __find_debuginfo() when dso__symsrc_filename() isn't set, and
in read_unwind_spec_debug_frame() instead of only following
.gnu_debuglink. Don't open() a NULL dso__symsrc_filename(), which
becomes the common case once symbols aren't loaded.

Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>
---
 tools/perf/util/dso.c              | 31 ++++++++++++++++++++++++++++++
 tools/perf/util/dso.h              |  1 +
 tools/perf/util/unwind-libdw.c     | 16 ++++++++++++---
 tools/perf/util/unwind-libunwind.c | 28 ++++++++++-----------------
 4 files changed, 55 insertions(+), 21 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index e03e2cb9212b..36f5ab94c7e7 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -598,6 +598,37 @@ int dso__read_build_id(struct dso *dso)
 	return dso__has_build_id(dso) ? 0 : -1;
 }
 
+/*
+ * Look for a separate debuginfo file for a user space DSO in the places
+ * dso__load() would, without loading its symbols. Returns an allocated path,
+ * or NULL if none was found.
+ */
+char *dso__find_debuginfo(struct dso *dso)
+{
+	static const enum dso_binary_type debuginfo_types[] = {
+		DSO_BINARY_TYPE__DEBUGLINK,
+		DSO_BINARY_TYPE__BUILD_ID_CACHE_DEBUGINFO,
+		DSO_BINARY_TYPE__FEDORA_DEBUGINFO,
+		DSO_BINARY_TYPE__UBUNTU_DEBUGINFO,
+		DSO_BINARY_TYPE__BUILDID_DEBUGINFO,
+		DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO,
+		DSO_BINARY_TYPE__MIXEDUP_UBUNTU_DEBUGINFO,
+	};
+	char filename[PATH_MAX];
+
+	/* The build-id cache and /usr/lib/debug/.build-id need the build-id. */
+	dso__read_build_id(dso);
+
+	for (size_t i = 0; i < ARRAY_SIZE(debuginfo_types); i++) {
+		if (dso__read_binary_type_filename(dso, debuginfo_types[i], /*root_dir=*/"",
+						   filename, sizeof(filename)))
+			continue;
+		if (is_regular_file(filename))
+			return strdup(filename);
+	}
+	return NULL;
+}
+
 static char *dso__get_filename(struct dso *dso, const char *root_dir,
 			       bool *decomp)
 {
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index ece54b9b8552..88d8674f6247 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -806,6 +806,7 @@ static inline bool dso__has_symbols(const struct dso *dso)
 
 char *dso__filename_with_chroot(const struct dso *dso, const char *filename);
 int dso__read_build_id(struct dso *dso);
+char *dso__find_debuginfo(struct dso *dso);
 
 bool dso__sorted_by_name(const struct dso *dso);
 void dso__set_sorted_by_name(struct dso *dso);
diff --git a/tools/perf/util/unwind-libdw.c b/tools/perf/util/unwind-libdw.c
index 63a5c2253174..a2e70e4f206f 100644
--- a/tools/perf/util/unwind-libdw.c
+++ b/tools/perf/util/unwind-libdw.c
@@ -5,6 +5,7 @@
 #include <elfutils/libdwfl.h>
 #include <inttypes.h>
 #include <errno.h>
+#include <string.h>
 #include "debug.h"
 #include "dso.h"
 #include <dwarf-regs.h>
@@ -39,11 +40,20 @@ static int __find_debuginfo(Dwfl_Module *mod __maybe_unused, void **userdata,
 			    const char *file_name, const char *debuglink_file __maybe_unused,
 			    GElf_Word debuglink_crc __maybe_unused, char **debuginfo_file_name)
 {
-	const struct dso *dso = *userdata;
+	struct dso *dso = *userdata;
 
 	assert(dso);
-	if (dso__symsrc_filename(dso) && strcmp(file_name, dso__symsrc_filename(dso)))
-		*debuginfo_file_name = strdup(dso__symsrc_filename(dso));
+	if (dso__symsrc_filename(dso)) {
+		if (strcmp(file_name, dso__symsrc_filename(dso)))
+			*debuginfo_file_name = strdup(dso__symsrc_filename(dso));
+	} else {
+		char *debuginfo = dso__find_debuginfo(dso);
+
+		if (debuginfo && strcmp(file_name, debuginfo))
+			*debuginfo_file_name = debuginfo;
+		else
+			free(debuginfo);
+	}
 	return -1;
 }
 
diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c
index 73d191ce51a5..d92f1d485285 100644
--- a/tools/perf/util/unwind-libunwind.c
+++ b/tools/perf/util/unwind-libunwind.c
@@ -283,8 +283,9 @@ static int read_unwind_spec_debug_frame(struct dso *dso,
 	/* debug_frame can reside in:
 	 *  - dso
 	 *  - debug pointed by symsrc_filename
-	 *  - gnu_debuglink, which doesn't necessary
-	 *    has to be pointed by symsrc_filename
+	 *  - a separate debuginfo file, which symsrc_filename doesn't
+	 *    necessarily point to, or isn't set yet when the symbols
+	 *    haven't been loaded
 	 */
 	if (ofs == 0) {
 		if (dso__data_get_fd(dso, machine, &fd)) {
@@ -292,7 +293,7 @@ static int read_unwind_spec_debug_frame(struct dso *dso,
 			dso__data_put_fd(dso);
 		}
 
-		if (ofs <= 0) {
+		if (ofs <= 0 && dso__symsrc_filename(dso)) {
 			fd = open(dso__symsrc_filename(dso), O_RDONLY);
 			if (fd >= 0) {
 				ofs = elf_section_offset(fd, ".debug_frame");
@@ -301,19 +302,10 @@ static int read_unwind_spec_debug_frame(struct dso *dso,
 		}
 
 		if (ofs <= 0) {
-			char *debuglink = malloc(PATH_MAX);
-			int ret = 0;
-
-			if (debuglink == NULL) {
-				pr_err("unwind: Can't read unwind spec debug frame.\n");
-				return -ENOMEM;
-			}
+			char *debuginfo = dso__find_debuginfo(dso);
 
-			ret = dso__read_binary_type_filename(
-				dso, DSO_BINARY_TYPE__DEBUGLINK,
-				machine->root_dir, debuglink, PATH_MAX);
-			if (!ret) {
-				fd = open(debuglink, O_RDONLY);
+			if (debuginfo) {
+				fd = open(debuginfo, O_RDONLY);
 				if (fd >= 0) {
 					ofs = elf_section_offset(fd,
 							".debug_frame");
@@ -326,12 +318,12 @@ static int read_unwind_spec_debug_frame(struct dso *dso,
 						"%s: overwrite symsrc(%s,%s)\n",
 							__func__,
 							dso__symsrc_filename(dso),
-							debuglink);
+							debuginfo);
 					dso__free_symsrc_filename(dso);
 				}
-				dso__set_symsrc_filename(dso, debuglink);
+				dso__set_symsrc_filename(dso, debuginfo);
 			} else {
-				free(debuglink);
+				free(debuginfo);
 			}
 		}
 
-- 
2.53.0-Meta

Re: [PATCH v2 2/3] perf unwind: Find separate debuginfo without loading symbols

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] perf dso: dso__find_debuginfo() returns unverified first regular file
- [Medium] perf dso: dso__find_debuginfo() ignores symfs when reading build-id
- [Medium] perf dso: dso__find_debuginfo() skips mountns and chroot lookup
- [Medium] perf unwind: __find_debuginfo() skips fallback when symsrc is main binary
--

commit c3719700a5c64b73424eb58a041d6711fca9b8d4
Author: Matthew Wood <thepacketgeek@gmail.com>

perf unwind: Find separate debuginfo without loading symbols

This commit adds dso__find_debuginfo() to locate separate debuginfo files
for user space DSOs without loading symbol tables, and wires it into the
libdw and libunwind DWARF unwinders.

> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index e03e2cb9212b1..36f5ab94c7e70 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -598,6 +598,37 @@ int dso__read_build_id(struct dso *dso)
>  	return dso__has_build_id(dso) ? 0 : -1;
>  }
>  
> +/*
> + * Look for a separate debuginfo file for a user space DSO in the places
> + * dso__load() would, without loading its symbols. Returns an allocated path,
> + * or NULL if none was found.
> + */
> +char *dso__find_debuginfo(struct dso *dso)
> +{
> +	static const enum dso_binary_type debuginfo_types[] = {
> +		DSO_BINARY_TYPE__DEBUGLINK,
> +		DSO_BINARY_TYPE__BUILD_ID_CACHE_DEBUGINFO,
> +		DSO_BINARY_TYPE__FEDORA_DEBUGINFO,
> +		DSO_BINARY_TYPE__UBUNTU_DEBUGINFO,
> +		DSO_BINARY_TYPE__BUILDID_DEBUGINFO,
> +		DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO,
> +		DSO_BINARY_TYPE__MIXEDUP_UBUNTU_DEBUGINFO,
> +	};
> +	char filename[PATH_MAX];
> +
> +	/* The build-id cache and /usr/lib/debug/.build-id need the build-id. */
> +	dso__read_build_id(dso);

[Severity: Medium]
Does calling dso__read_build_id() here ignore symbol_conf.symfs when
--symfs is used and the DSO does not yet have a recorded build-id?

In tools/perf/util/symbol.c:dso__load(), perf joins symfs before reading
the build-id:

	if (!dso__has_build_id(dso)) {
		struct build_id bid = { .size = 0, };

		__symbol__join_symfs(name, PATH_MAX, dso__long_name(dso));
		if (filename__read_build_id(name, &bid) > 0)
			dso__set_build_id(dso, &bid);
	}

In contrast, tools/perf/util/dso.c:dso__read_build_id() passes
dso__long_name(dso) directly to filename__read_build_id() without
__symbol__join_symfs():

	if (filename__read_build_id(dso__long_name(dso), &bid) > 0)
		dso__set_build_id(dso, &bid);

If the binary only exists under symfs, dso__read_build_id() fails to read
the build-id. If a different version of the binary exists at
dso__long_name(dso) on the host root filesystem, dso__read_build_id()
populates the DSO with the host binary's build-id, causing subsequent
dso__load() calls to reject the symfs binary with
DSO_LOAD_ERRNO__MISMATCHING_BUILDID.

> +
> +	for (size_t i = 0; i < ARRAY_SIZE(debuginfo_types); i++) {
> +		if (dso__read_binary_type_filename(dso, debuginfo_types[i], /*root_dir=*/"",
> +						   filename, sizeof(filename)))
> +			continue;
> +		if (is_regular_file(filename))
> +			return strdup(filename);
> +	}

[Severity: Medium]
Can this loop return the stripped DSO itself or a file with a mismatched
build-id before reaching the remaining candidates in debuginfo_types[]?

When checking DSO_BINARY_TYPE__DEBUGLINK,
tools/perf/util/dso.c:dso__read_binary_type_filename() tests
debuglink_paths[] using is_regular_file():

		for (i = 0; i < ARRAY_SIZE(debuglink_paths); i++) {
			snprintf(filename, size,
					debuglink_paths[i], dso_dir, symfile);
			if (is_regular_file(filename)) {
				ret = 0;
				break;
			}
		}

When .gnu_debuglink has the same basename as the DSO itself (such as on
Debian/Ubuntu where /usr/lib/libfoo.so has separate debuginfo at
/usr/lib/debug/usr/lib/libfoo.so), debuglink_paths[1] ("%s/%s") resolves to
the stripped DSO binary itself. Similarly, for
DSO_BINARY_TYPE__BUILD_ID_CACHE_DEBUGINFO, __dso__build_id_filename()
ignores is_debug=true for vDSOs or flat build-id cache links and returns
the cached main binary.

In dso__load(), each candidate is opened via symsrc__init(), which checks
that the candidate's build-id matches the DSO:

	if (dso__has_build_id(dso) && !symbol_conf.ignore_vmlinux_buildid) {
		...
		if (!dso__build_id_equal(dso, &bid)) {
			pr_debug("%s: build id mismatch for %s.\n", __func__, name);
			*dso__load_errno(dso) = DSO_LOAD_ERRNO__MISMATCHING_BUILDID;
			goto out_elf_end;
		}
	}

and verifies symsrc__has_symtab() before stopping, allowing the loop to
continue to BUILD_ID_CACHE_DEBUGINFO, UBUNTU_DEBUGINFO, and
BUILDID_DEBUGINFO.

Because dso__find_debuginfo() returns the very first regular file without
checking if it is distinct from the DSO, if its build-id matches, or if it
contains debug/unwind sections:
- In __find_debuginfo(), if the returned path equals file_name, it is freed
  and libdw receives no debuginfo without trying later debuginfo_types[]
  entries.
- If a path-based candidate (DEBUGLINK, FEDORA_DEBUGINFO, UBUNTU_DEBUGINFO)
  on the host has a mismatched build-id while the matching debuginfo is in
  BUILD_ID_CACHE_DEBUGINFO or BUILDID_DEBUGINFO, libdwfl rejects the
  mismatched file and never calls __find_debuginfo() again for that module,
  while libunwind's read_unwind_spec_debug_frame() reads .debug_frame from
  the mismatched file and overwrites dso->symsrc_filename with it.

[Severity: Medium]
Also in this loop, how does dso__find_debuginfo() find debuginfo files
located inside a target process's mount namespace or chroot?

In tools/perf/util/symbol.c:dso__load(), perf enters dso__nsinfo(dso)'s
mount namespace (exiting for the build-id cache types) and falls back to
dso__filename_with_chroot() when is_regular_file() fails with ENOENT:

		if (nsexit)
			nsinfo__mountns_exit(&nsc);

		is_reg = is_regular_file(name);
		if (!is_reg && errno == ENOENT && dso__nsinfo(dso)) {
			char *new_name = dso__filename_with_chroot(dso, name);
			if (new_name) {
				is_reg = is_regular_file(new_name);
				strlcpy(name, new_name, PATH_MAX);
				free(new_name);
			}
		}

Here, dso__read_build_id(dso) enters and immediately exits the mount
namespace, so the debuginfo_types[] loop runs in perf's own mount namespace
without mountns or chroot handling.

> +	return NULL;
> +}
> +
>  static char *dso__get_filename(struct dso *dso, const char *root_dir,
>  			       bool *decomp)
>  {

[ ... ]

> diff --git a/tools/perf/util/unwind-libdw.c b/tools/perf/util/unwind-libdw.c
> index 63a5c2253174f..a2e70e4f206f7 100644
> --- a/tools/perf/util/unwind-libdw.c
> +++ b/tools/perf/util/unwind-libdw.c

[ ... ]

> @@ -39,11 +40,20 @@ static int __find_debuginfo(Dwfl_Module *mod __maybe_unused, void **userdata,
>  			    const char *file_name, const char *debuglink_file __maybe_unused,
>  			    GElf_Word debuglink_crc __maybe_unused, char **debuginfo_file_name)
>  {
> -	const struct dso *dso = *userdata;
> +	struct dso *dso = *userdata;
>  
>  	assert(dso);
> -	if (dso__symsrc_filename(dso) && strcmp(file_name, dso__symsrc_filename(dso)))
> -		*debuginfo_file_name = strdup(dso__symsrc_filename(dso));
> +	if (dso__symsrc_filename(dso)) {
> +		if (strcmp(file_name, dso__symsrc_filename(dso)))
> +			*debuginfo_file_name = strdup(dso__symsrc_filename(dso));
> +	} else {
> +		char *debuginfo = dso__find_debuginfo(dso);
> +
> +		if (debuginfo && strcmp(file_name, debuginfo))
> +			*debuginfo_file_name = debuginfo;
> +		else
> +			free(debuginfo);
> +	}
>  	return -1;
>  }

[Severity: Medium]
Should __find_debuginfo() also fall back to dso__find_debuginfo(dso) when
dso__symsrc_filename(dso) is equal to file_name?

If symbols were loaded from the main binary itself (for example, when the
binary retains a .symtab after strip --strip-debug, has .gnu_debugdata
MiniDebugInfo, or matched DSO_BINARY_TYPE__BUILD_ID_CACHE in dso__load()),
dso__symsrc_filename(dso) is non-NULL and equal to file_name while
.debug_frame is in a separate debuginfo file.

With the if/else structure above, strcmp(file_name, dso__symsrc_filename(dso))
is 0, so *debuginfo_file_name is not set and the else branch calling
dso__find_debuginfo(dso) is skipped.

[ ... ]

> diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c
> index 73d191ce51a55..d92f1d4852851 100644
> --- a/tools/perf/util/unwind-libunwind.c
> +++ b/tools/perf/util/unwind-libunwind.c

[ ... ]

> @@ -301,19 +302,10 @@ static int read_unwind_spec_debug_frame(struct dso *dso,
>  		}
>  
>  		if (ofs <= 0) {
> -			char *debuglink = malloc(PATH_MAX);
> -			int ret = 0;
> -
> -			if (debuglink == NULL) {
> -				pr_err("unwind: Can't read unwind spec debug frame.\n");
> -				return -ENOMEM;
> -			}
> +			char *debuginfo = dso__find_debuginfo(dso);
>  
> -			ret = dso__read_binary_type_filename(
> -				dso, DSO_BINARY_TYPE__DEBUGLINK,
> -				machine->root_dir, debuglink, PATH_MAX);
> -			if (!ret) {
> -				fd = open(debuglink, O_RDONLY);
> +			if (debuginfo) {
> +				fd = open(debuginfo, O_RDONLY);
>  				if (fd >= 0) {
>  					ofs = elf_section_offset(fd,
>  							".debug_frame");
> @@ -326,12 +318,12 @@ static int read_unwind_spec_debug_frame(struct dso *dso,
>  						"%s: overwrite symsrc(%s,%s)\n",
>  							__func__,
>  							dso__symsrc_filename(dso),
> -							debuglink);
> +							debuginfo);
>  					dso__free_symsrc_filename(dso);
>  				}
> -				dso__set_symsrc_filename(dso, debuglink);
> +				dso__set_symsrc_filename(dso, debuginfo);
>  			} else {
> -				free(debuglink);
> +				free(debuginfo);
>  			}
>  		}
>  

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261017000158.1178269-1-thepacketgeek@gmail.com?part=2

[PATCH v2 3/3] perf unwind: Don't look up symbols when the caller doesn't want them

[Matthew Wood]

Commit 02b2705017d2 ("perf callchain: Allow symbols to be optional when
resolving a callchain") let callers that only need maps, such as
build-id marking in 'perf record' and 'perf inject', resolve callchains
without loading symbols. The DWARF unwinders were left out:
thread__resolve_callchain_unwind() just notes that "Not resolving
symbols with an unwinder isn't currently supported", and both libdw and
libunwind look up the symbol of every frame they visit. The first lookup
in a DSO loads its whole symbol table and demangles every name in it.

On arm64 this happens even for frame pointer callchains, because
get_leaf_frame_caller_aarch64() runs a two frame DWARF unwind for every
sample to recover the leaf function's caller. Since commit 29be60c93d2d
("perf build-id: Mark DSO in sample callchains"), process_buildids()
does that for every sample at the end of 'perf record', so with -a it
loads and demangles the symbol tables of every user binary sampled on
the system. A C++ binary with a pathological mangled name sent
__cxa_demangle() into exponential time and memory, and

  perf record -a -e cycles:u --call-graph fp -- sleep 2

never finished writing perf.data after the workload exited.

Pass a symbols flag down to the unwinders. When it is false, look up
only the map of each frame and leave entry->ms.sym NULL, which
unwind_entry() already handles just as add_callchain_ip() does for
symbols=false. get_leaf_frame_caller_aarch64() uses nothing but the
addresses, and its caller looks up the leaf frame caller's symbol
through add_callchain_ip() when it wants it, so it never asks for
symbols.

Loading the symbols also had side effects that the unwinders relied on.
In libdw, both report_module() and entry() follow the flag, so with
symbols the module's symbols are still loaded before libdw first asks
__find_debuginfo() for its debuginfo, which it does while unwinding the
frame to find out whether it is an activation. Without symbols, libdw's
__find_debuginfo() and libunwind's read_unwind_spec_debug_frame() find
the debuginfo file themselves since the previous patch, and
__report_module() reads a missing build-id with dso__read_build_id()
before trying the build-id cache. The frames returned are unchanged.

With libdw, 'perf inject -b' on an x86 'perf record --call-graph dwarf'
profile of a Python workload goes from 0.24s and 58.5MB max RSS to 0.04s
and 15.5MB, with the same build-ids injected.

Fixes: 02b2705017d2 ("perf callchain: Allow symbols to be optional when resolving a callchain")
Fixes: 29be60c93d2d ("perf build-id: Mark DSO in sample callchains")
Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>
---
 tools/perf/tests/dwarf-unwind.c               |  3 ++-
 .../util/arm64-frame-pointer-unwind-support.c |  7 ++++-
 tools/perf/util/machine.c                     |  7 ++---
 tools/perf/util/unwind-libdw.c                | 27 ++++++++++++++-----
 tools/perf/util/unwind-libdw.h                |  1 +
 tools/perf/util/unwind-libunwind.c            | 15 +++++++----
 tools/perf/util/unwind.c                      |  7 +++--
 tools/perf/util/unwind.h                      | 11 +++++---
 8 files changed, 55 insertions(+), 23 deletions(-)

diff --git a/tools/perf/tests/dwarf-unwind.c b/tools/perf/tests/dwarf-unwind.c
index 9ed78d00fb87..1d46c629eeee 100644
--- a/tools/perf/tests/dwarf-unwind.c
+++ b/tools/perf/tests/dwarf-unwind.c
@@ -104,7 +104,8 @@ NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__thread(struct thread *thr
 	}
 
 	err = unwind__get_entries(unwind_entry, &cnt, thread,
-				  &sample, MAX_STACK, false);
+				  &sample, MAX_STACK, /*symbols=*/true,
+				  /*best_effort=*/false);
 	if (err)
 		pr_debug("unwind failed\n");
 	else if (cnt != MAX_STACK) {
diff --git a/tools/perf/util/arm64-frame-pointer-unwind-support.c b/tools/perf/util/arm64-frame-pointer-unwind-support.c
index 3af8c7a466e0..c16eeee34211 100644
--- a/tools/perf/util/arm64-frame-pointer-unwind-support.c
+++ b/tools/perf/util/arm64-frame-pointer-unwind-support.c
@@ -67,7 +67,12 @@ u64 get_leaf_frame_caller_aarch64(struct perf_sample *sample, struct thread *thr
 		regs->cache_regs[PERF_REG_ARM64_SP] = 0;
 	}
 
-	ret = unwind__get_entries(add_entry, &entries, thread, sample, 2, true);
+	/*
+	 * Only the addresses are used. The caller looks up the leaf frame
+	 * caller's symbol itself if it wants it, so don't load symbols here.
+	 */
+	ret = unwind__get_entries(add_entry, &entries, thread, sample, 2,
+				  /*symbols=*/false, /*best_effort=*/true);
 	memcpy(regs, &old_regs, sizeof(*regs));
 
 	if (ret || entries.length != 2)
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index f86b3b7df742..16847e26ea09 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -3061,11 +3061,8 @@ static int thread__resolve_callchain_unwind(struct thread *thread,
 	    !sample->user_stack.size)
 		return 0;
 
-	if (!symbols)
-		pr_debug("Not resolving symbols with an unwinder isn't currently supported\n");
-
-	return unwind__get_entries(unwind_entry, cursor,
-				   thread, sample, max_stack, false);
+	return unwind__get_entries(unwind_entry, cursor, thread, sample,
+				   max_stack, symbols, /*best_effort=*/false);
 }
 
 int __thread__resolve_callchain(struct thread *thread,
diff --git a/tools/perf/util/unwind-libdw.c b/tools/perf/util/unwind-libdw.c
index a2e70e4f206f..c8675484625b 100644
--- a/tools/perf/util/unwind-libdw.c
+++ b/tools/perf/util/unwind-libdw.c
@@ -78,16 +78,22 @@ static const Dwfl_Callbacks offline_callbacks = {
 };
 
 static int __report_module(struct addr_location *al, u64 ip,
-			    struct unwind_info *ui)
+			    struct unwind_info *ui, bool symbols)
 {
 	Dwfl_Module *mod;
 	struct dso *dso = NULL;
 	Dwarf_Addr base;
+
 	/*
-	 * Some callers will use al->sym, so we can't just use the
-	 * cheaper thread__find_map() here.
+	 * Some callers will use al->sym. Loading the symbols also reads a
+	 * missing build-id, needed below, and sets dso__symsrc_filename(),
+	 * needed by __find_debuginfo(). Without symbols, both are found
+	 * separately.
 	 */
-	thread__find_symbol(ui->thread, PERF_RECORD_MISC_USER, ip, al);
+	if (symbols)
+		thread__find_symbol(ui->thread, PERF_RECORD_MISC_USER, ip, al);
+	else
+		thread__find_map(ui->thread, PERF_RECORD_MISC_USER, ip, al);
 
 	if (al->map)
 		dso = map__dso(al->map);
@@ -129,6 +135,13 @@ static int __report_module(struct addr_location *al, u64 ip,
 	if (!mod) {
 		char filename[PATH_MAX];
 
+		/*
+		 * Finding the DSO in the build-id cache needs its build-id.
+		 * Loading the symbols reads it, so read it here if they
+		 * weren't loaded.
+		 */
+		if (!symbols)
+			dso__read_build_id(dso);
 		if (dso__build_id_filename(dso, filename, sizeof(filename), false))
 			mod = dwfl_report_elf(ui->dwfl, dso__short_name(dso), filename, -1,
 					      base, false);
@@ -150,7 +163,7 @@ static int report_module(u64 ip, struct unwind_info *ui)
 	int res;
 
 	addr_location__init(&al);
-	res = __report_module(&al, ip, ui);
+	res = __report_module(&al, ip, ui, ui->symbols);
 	addr_location__exit(&al);
 	return res;
 }
@@ -166,7 +179,7 @@ static int entry(u64 ip, struct unwind_info *ui)
 	struct addr_location al;
 
 	addr_location__init(&al);
-	if (__report_module(&al, ip, ui)) {
+	if (__report_module(&al, ip, ui, ui->symbols)) {
 		addr_location__exit(&al);
 		return -1;
 	}
@@ -371,6 +384,7 @@ int libdw__get_entries(unwind_entry_cb_t cb, void *arg,
 			struct thread *thread,
 			struct perf_sample *data,
 			int max_stack,
+			bool symbols,
 			bool best_effort)
 {
 	struct maps *maps = thread__maps(thread);
@@ -399,6 +413,7 @@ int libdw__get_entries(unwind_entry_cb_t cb, void *arg,
 		.max_stack	= max_stack,
 		.e_machine	= e_machine,
 		.e_flags	= e_flags,
+		.symbols	= symbols,
 		.best_effort    = best_effort
 	};
 
diff --git a/tools/perf/util/unwind-libdw.h b/tools/perf/util/unwind-libdw.h
index 6423bf5a2492..ab37bf5cb105 100644
--- a/tools/perf/util/unwind-libdw.h
+++ b/tools/perf/util/unwind-libdw.h
@@ -22,6 +22,7 @@ struct unwind_info {
 	int			idx;
 	uint32_t		e_flags;
 	uint16_t		e_machine;
+	bool			symbols;
 	bool			best_effort;
 	struct unwind_entry	entries[];
 };
diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c
index d92f1d485285..30d3c494f45a 100644
--- a/tools/perf/util/unwind-libunwind.c
+++ b/tools/perf/util/unwind-libunwind.c
@@ -581,14 +581,19 @@ void unwind__finish_access(struct maps *maps)
 	libunwind_arch__finish_access(maps);
 }
 
-static int entry(uint64_t ip, struct thread *thread, unwind_entry_cb_t cb, void *arg)
+static int entry(uint64_t ip, struct thread *thread, bool symbols,
+		 unwind_entry_cb_t cb, void *arg)
 {
 	struct unwind_entry e;
 	struct addr_location al;
 	int ret;
 
 	addr_location__init(&al);
-	e.ms.sym = thread__find_symbol(thread, PERF_RECORD_MISC_USER, ip, &al);
+	if (symbols)
+		thread__find_symbol(thread, PERF_RECORD_MISC_USER, ip, &al);
+	else
+		thread__find_map(thread, PERF_RECORD_MISC_USER, ip, &al);
+	e.ms.sym = al.sym;
 	e.ip     = ip;
 	e.ms.map = al.map;
 	e.ms.thread = thread__get(al.thread);
@@ -606,7 +611,7 @@ static int entry(uint64_t ip, struct thread *thread, unwind_entry_cb_t cb, void
 int libunwind__get_entries(unwind_entry_cb_t cb, void *arg,
 			 struct thread *thread,
 			 struct perf_sample *sample, int max_stack,
-			 bool best_effort)
+			 bool symbols, bool best_effort)
 {
 	struct unwind_info *ui;
 	uint64_t first_ip;
@@ -632,7 +637,7 @@ int libunwind__get_entries(unwind_entry_cb_t cb, void *arg,
 
 	if (max_stack == 1) {
 		/* Special case for a single entry. */
-		ret = entry(first_ip, thread, cb, arg);
+		ret = entry(first_ip, thread, symbols, cb, arg);
 		return ret ? (ret == -ENOMEM ? -ENOMEM : 0) : 1;
 	}
 
@@ -654,7 +659,7 @@ int libunwind__get_entries(unwind_entry_cb_t cb, void *arg,
 		int j = callchain_param.order == ORDER_CALLEE ? i : ui->cur_ip - i - 1;
 
 		if (ui->ips[j]) {
-			ret = entry(ui->ips[j], thread, cb, arg);
+			ret = entry(ui->ips[j], thread, symbols, cb, arg);
 			if (ret)
 				break;
 			entries++;
diff --git a/tools/perf/util/unwind.c b/tools/perf/util/unwind.c
index 4ed4b1d55c69..8555ece4b6f3 100644
--- a/tools/perf/util/unwind.c
+++ b/tools/perf/util/unwind.c
@@ -10,6 +10,7 @@ int unwind__get_entries(unwind_entry_cb_t cb __maybe_unused, void *arg __maybe_u
 			struct thread *thread __maybe_unused,
 			struct perf_sample *data __maybe_unused,
 			int max_stack __maybe_unused,
+			bool symbols __maybe_unused,
 			bool best_effort __maybe_unused)
 {
 	int ret = 0;
@@ -29,10 +30,12 @@ int unwind__get_entries(unwind_entry_cb_t cb __maybe_unused, void *arg __maybe_u
 	for (size_t i = 0; i < ARRAY_SIZE(symbol_conf.unwind_style); i++) {
 		switch (symbol_conf.unwind_style[i]) {
 		case UNWIND_STYLE_LIBDW:
-			ret = libdw__get_entries(cb, arg, thread, data, max_stack, best_effort);
+			ret = libdw__get_entries(cb, arg, thread, data, max_stack,
+						 symbols, best_effort);
 			break;
 		case UNWIND_STYLE_LIBUNWIND:
-			ret = libunwind__get_entries(cb, arg, thread, data, max_stack, best_effort);
+			ret = libunwind__get_entries(cb, arg, thread, data, max_stack,
+						     symbols, best_effort);
 			break;
 		case UNWIND_STYLE_UNKNOWN:
 		default:
diff --git a/tools/perf/util/unwind.h b/tools/perf/util/unwind.h
index a2d4af694fde..59dfb234cbea 100644
--- a/tools/perf/util/unwind.h
+++ b/tools/perf/util/unwind.h
@@ -23,6 +23,9 @@ int unwind__configure(const char *var, const char *value, void *cb);
 int unwind__option(const struct option *opt, const char *arg, int unset);
 
 /*
+ * When symbols is false, only the map of each frame is looked up and
+ * entry->ms.sym is left NULL, so that no symbol tables are loaded.
+ *
  * When best_effort is set, don't report errors and fail silently. This could
  * be expanded in the future to be more permissive about things other than
  * error messages.
@@ -30,19 +33,20 @@ int unwind__option(const struct option *opt, const char *arg, int unset);
 int unwind__get_entries(unwind_entry_cb_t cb, void *arg,
 			struct thread *thread,
 			struct perf_sample *data, int max_stack,
-			bool best_effort);
+			bool symbols, bool best_effort);
 
 #ifdef HAVE_LIBDW_SUPPORT
 int libdw__get_entries(unwind_entry_cb_t cb, void *arg,
 		       struct thread *thread,
 		       struct perf_sample *data, int max_stack,
-		       bool best_effort);
+		       bool symbols, bool best_effort);
 #else
 #include "debug.h"
 static inline int libdw__get_entries(unwind_entry_cb_t cb __maybe_unused, void *arg __maybe_unused,
 				     struct thread *thread __maybe_unused,
 				     struct perf_sample *data __maybe_unused,
 				     int max_stack __maybe_unused,
+				     bool symbols __maybe_unused,
 				     bool best_effort __maybe_unused)
 {
 	pr_warning_once("Error: libdw dwarf unwinding not built into perf\n");
@@ -55,7 +59,7 @@ static inline int libdw__get_entries(unwind_entry_cb_t cb __maybe_unused, void *
 int libunwind__get_entries(unwind_entry_cb_t cb, void *arg,
 			   struct thread *thread,
 			   struct perf_sample *data, int max_stack,
-			   bool best_effort);
+			   bool symbols, bool best_effort);
 int unwind__prepare_access(struct maps *maps, uint16_t e_machine);
 void unwind__flush_access(struct maps *maps);
 void unwind__finish_access(struct maps *maps);
@@ -66,6 +70,7 @@ static inline int libunwind__get_entries(unwind_entry_cb_t cb __maybe_unused,
 					 struct thread *thread __maybe_unused,
 					 struct perf_sample *data __maybe_unused,
 					 int max_stack __maybe_unused,
+					 bool symbols __maybe_unused,
 					 bool best_effort __maybe_unused)
 {
 	pr_warning_once("Error: libunwind dwarf unwinding not built into perf\n");
-- 
2.53.0-Meta

Re: [PATCH v2 3/3] perf unwind: Don't look up symbols when the caller doesn't want them

[sashiko-bot]

> Commit 02b2705017d2 ("perf callchain: Allow symbols to be optional when
> resolving a callchain") let callers that only need maps, such as
> build-id marking in 'perf record' and 'perf inject', resolve callchains
> without loading symbols. The DWARF unwinders were left out:
> thread__resolve_callchain_unwind() just notes that "Not resolving
> symbols with an unwinder isn't currently supported", and both libdw and
> libunwind look up the symbol of every frame they visit. The first lookup
> in a DSO loads its whole symbol table and demangles every name in it.
> 
> On arm64 this happens even for frame pointer callchains, because
> get_leaf_frame_caller_aarch64() runs a two frame DWARF unwind for every
> sample to recover the leaf function's caller. Since commit 29be60c93d2d
> ("perf build-id: Mark DSO in sample callchains"), process_buildids()
> does that for every sample at the end of 'perf record', so with -a it
> loads and demangles the symbol tables of every user binary sampled on
> [ ... ]
> and 15.5MB, with the same build-ids injected.
> 
> Fixes: 02b2705017d2 ("perf callchain: Allow symbols to be optional when resolving a callchain")
> Fixes: 29be60c93d2d ("perf build-id: Mark DSO in sample callchains")
> Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261017000158.1178269-1-thepacketgeek@gmail.com?part=3