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

[Matthew Wood <thepacketgeek@gmail.com>]

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