[PATCH v1 0/8] perf tools: Cleanup dso debug info access

[PATCH v1 0/8] perf tools: Cleanup dso debug info access

[Namhyung Kim]

Hello,

This is a preparation for DSO file access cleanup work.  IIUC DSO
access has three types - symbol table, binary data and debug info.
Those information can be placed separately like stripped binaries and
split debug files.  I've added dbginfo_type to DSO to track location
of debug info like others.

I'm sending this out before it grows too much.  This patchset focuses
on debuginfo open path.  Once it looks good to others, I'll update
other places accessing DWARF info to use this info and cleanup other
file type accesses too.

Thanks,
Namhyung


Namhyung Kim (8):
  perf tools: Add dso->dbginfo_type field
  perf tools: Factor out dso__find_dbginfo_type()
  perf tools: Check system path when check debuginfo
  perf tools: Export dso__get_filename() with type argument
  perf tools: Add dso__put_filename()
  perf tools: Avoid repeated failing search for debuginfo
  perf tools: Looks symbol file first when check debuginfo
  perf tools: Update debuginfo__new() to take DSO

 tools/perf/util/debuginfo.c   | 51 +++++++++------------
 tools/perf/util/debuginfo.h   | 13 +++++-
 tools/perf/util/dso.c         | 86 ++++++++++++++++++++++++++---------
 tools/perf/util/dso.h         | 33 ++++++++++++++
 tools/perf/util/probe-event.c |  4 +-
 5 files changed, 133 insertions(+), 54 deletions(-)

-- 
2.55.0

[PATCH v1 1/8] perf tools: Add dso->dbginfo_type field

[Namhyung Kim]

It saves where the debug info is available for the given DSO.  It's
sometimes different than where binary data or symbol table is located.
So let's add a new field to save the info.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/dso.c |  1 +
 tools/perf/util/dso.h | 16 ++++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 11db461f2032bc41..90a6f661278a9c81 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1721,6 +1721,7 @@ struct dso *dso__new_id(const char *name, const struct dso_id *id)
 		dso->data.status = DSO_DATA_STATUS_UNKNOWN;
 		dso->symtab_type = DSO_BINARY_TYPE__NOT_FOUND;
 		dso->binary_type = DSO_BINARY_TYPE__NOT_FOUND;
+		dso->dbginfo_type = DSO_BINARY_TYPE__NOT_FOUND;
 		dso->is_64_bit = (sizeof(void *) == 8);
 		dso->loaded = 0;
 		dso->rel = 0;
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index e7d5f4bbf89497c0..34d187d1fc00efdc 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -328,8 +328,14 @@ DECLARE_RC_STRUCT(dso) {
 	enum dso_load_errno	load_errno;
 	u16		 long_name_len;
 	u16		 short_name_len;
+	/*
+	 * DSO file data can be spread into multiple places.  Distros usually
+	 * place debug info to a separate file.  Symbol tables may exist in
+	 * the binary or the debug file.  Let's separate them.
+	 */
 	enum dso_binary_type	symtab_type:8;
 	enum dso_binary_type	binary_type:8;
+	enum dso_binary_type	dbginfo_type:8;
 	enum dso_space_type	kernel:2;
 	enum dso_swap_type	needs_swap:2;
 	bool			is_kmod:1;
@@ -997,6 +1003,16 @@ static inline bool dso__is_kallsyms(const struct dso *dso)
 	return is_guest_kallsyms_pid_name(name);
 }
 
+static inline enum dso_binary_type dso__dbginfo_type(const struct dso *dso)
+{
+	return RC_CHK_ACCESS(dso)->dbginfo_type;
+}
+
+static inline void dso__set_dbginfo_type(struct dso *dso, enum dso_binary_type bt)
+{
+	RC_CHK_ACCESS(dso)->dbginfo_type = bt;
+}
+
 bool dso__is_object_file(const struct dso *dso);
 
 void dso__free_a2l(struct dso *dso);
-- 
2.55.0

[PATCH v1 2/8] perf tools: Factor out dso__find_dbginfo_type()

[Namhyung Kim]

It tries to find a file with working debug info by checking all possible
debug types.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/debuginfo.c | 23 ++++++-----------------
 tools/perf/util/dso.c       | 28 ++++++++++++++++++++++++++++
 tools/perf/util/dso.h       |  2 ++
 3 files changed, 36 insertions(+), 17 deletions(-)

diff --git a/tools/perf/util/debuginfo.c b/tools/perf/util/debuginfo.c
index 84a78b30ceac1066..d6bd0fcc22648f0e 100644
--- a/tools/perf/util/debuginfo.c
+++ b/tools/perf/util/debuginfo.c
@@ -93,15 +93,6 @@ static struct debuginfo *__debuginfo__new(const char *path)
 
 struct debuginfo *debuginfo__new(const char *path)
 {
-	static const enum dso_binary_type distro_dwarf_types[] = {
-		DSO_BINARY_TYPE__FEDORA_DEBUGINFO,
-		DSO_BINARY_TYPE__UBUNTU_DEBUGINFO,
-		DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO,
-		DSO_BINARY_TYPE__BUILDID_DEBUGINFO,
-		DSO_BINARY_TYPE__MIXEDUP_UBUNTU_DEBUGINFO,
-		DSO_BINARY_TYPE__NOT_FOUND,
-	};
-	const enum dso_binary_type *type;
 	char buf[PATH_MAX], nil = '\0';
 	struct dso *dso;
 	struct debuginfo *dinfo = NULL;
@@ -120,14 +111,12 @@ struct debuginfo *debuginfo__new(const char *path)
 	if (filename__read_build_id(path, &bid) > 0)
 		dso__set_build_id(dso, &bid);
 
-	for (type = distro_dwarf_types;
-	     !dinfo && *type != DSO_BINARY_TYPE__NOT_FOUND;
-	     type++) {
-		if (dso__read_binary_type_filename(dso, *type, &nil,
-						   buf, PATH_MAX) < 0)
-			continue;
-		dinfo = __debuginfo__new(buf);
-	}
+	dso__find_dbginfo_type(dso);
+	if (dso__read_binary_type_filename(dso, dso__dbginfo_type(dso), &nil,
+					   buf, PATH_MAX) < 0)
+		goto out;
+
+	dinfo = __debuginfo__new(buf);
 	dso__put(dso);
 
 out:
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 90a6f661278a9c81..10c4cd35b6d9464b 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -2108,3 +2108,31 @@ void dso__set_symsrc_filename(struct dso *dso, char *val)
 	dso__set_has_srcline(dso, true);
 	dso__set_a2l_fails(dso, 0);
 }
+
+void dso__find_dbginfo_type(struct dso *dso)
+{
+	static const enum dso_binary_type dbginfo_types[] = {
+		DSO_BINARY_TYPE__FEDORA_DEBUGINFO,
+		DSO_BINARY_TYPE__UBUNTU_DEBUGINFO,
+		DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO,
+		DSO_BINARY_TYPE__BUILDID_DEBUGINFO,
+		DSO_BINARY_TYPE__MIXEDUP_UBUNTU_DEBUGINFO,
+		DSO_BINARY_TYPE__NOT_FOUND,
+	};
+	const enum dso_binary_type *type;
+	char buf[PATH_MAX];
+
+	if (dso__dbginfo_type(dso) != DSO_BINARY_TYPE__NOT_FOUND)
+		return;
+
+	for (type = dbginfo_types; *type != DSO_BINARY_TYPE__NOT_FOUND; type++) {
+		if (dso__read_binary_type_filename(dso, *type, "",
+						   buf, PATH_MAX) < 0)
+			continue;
+
+		if (filename__has_section(buf, ".debug_info"))
+			break;
+	}
+
+	dso__set_dbginfo_type(dso, *type);
+}
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 34d187d1fc00efdc..a152957322f6b31c 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -1013,6 +1013,8 @@ static inline void dso__set_dbginfo_type(struct dso *dso, enum dso_binary_type b
 	RC_CHK_ACCESS(dso)->dbginfo_type = bt;
 }
 
+void dso__find_dbginfo_type(struct dso *dso);
+
 bool dso__is_object_file(const struct dso *dso);
 
 void dso__free_a2l(struct dso *dso);
-- 
2.55.0

[PATCH v1 3/8] perf tools: Check system path when check debuginfo

[Namhyung Kim]

The debuginfo__new() has a fallback to try the given path, but it seems
that it can check SYSTEM_PATH_DSO type in the dso__find_dbginfo_type()
instead.

One corner case would be when dso__new() fails in debuginfo__new() but
it should be rare and memory is exhausted.  I think it's fine to fail
for that case.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/debuginfo.c | 12 +++---------
 tools/perf/util/dso.c       |  1 +
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/debuginfo.c b/tools/perf/util/debuginfo.c
index d6bd0fcc22648f0e..2669d5f5ee6121d4 100644
--- a/tools/perf/util/debuginfo.c
+++ b/tools/perf/util/debuginfo.c
@@ -101,7 +101,7 @@ struct debuginfo *debuginfo__new(const char *path)
 	/* Try to open distro debuginfo files */
 	dso = dso__new(path);
 	if (!dso)
-		goto out;
+		return NULL;
 
 	/*
 	 * Set the build id for DSO_BINARY_TYPE__BUILDID_DEBUGINFO. Don't block
@@ -114,18 +114,12 @@ struct debuginfo *debuginfo__new(const char *path)
 	dso__find_dbginfo_type(dso);
 	if (dso__read_binary_type_filename(dso, dso__dbginfo_type(dso), &nil,
 					   buf, PATH_MAX) < 0)
-		goto out;
+		return NULL;
 
 	dinfo = __debuginfo__new(buf);
 	dso__put(dso);
 
-out:
-	if (dinfo)
-		return dinfo;
-
-	/* if failed to open all distro debuginfo, open given binary */
-	symbol__join_symfs(buf, path);
-	return __debuginfo__new(buf);
+	return dinfo;
 }
 
 void debuginfo__delete(struct debuginfo *dbg)
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 10c4cd35b6d9464b..512f2bb61e8f2a2f 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -2117,6 +2117,7 @@ void dso__find_dbginfo_type(struct dso *dso)
 		DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO,
 		DSO_BINARY_TYPE__BUILDID_DEBUGINFO,
 		DSO_BINARY_TYPE__MIXEDUP_UBUNTU_DEBUGINFO,
+		DSO_BINARY_TYPE__SYSTEM_PATH_DSO,
 		DSO_BINARY_TYPE__NOT_FOUND,
 	};
 	const enum dso_binary_type *type;
-- 
2.55.0

[PATCH v1 4/8] perf tools: Export dso__get_filename() with type argument

[Namhyung Kim]

The dso__get_filename() returns a new string of filepath for the given
binary type.  It handles chroot and decompressed kernel modules so that
callers can read the file contents.  Thus it'd be better to use it to
check available binary types.

Add a type argument to handle a different path for symbol or debug info.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/debuginfo.c | 17 +++++++++++------
 tools/perf/util/dso.c       | 27 +++++++++++++++++----------
 tools/perf/util/dso.h       |  2 ++
 3 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/tools/perf/util/debuginfo.c b/tools/perf/util/debuginfo.c
index 2669d5f5ee6121d4..38e9299765182bd7 100644
--- a/tools/perf/util/debuginfo.c
+++ b/tools/perf/util/debuginfo.c
@@ -93,10 +93,11 @@ static struct debuginfo *__debuginfo__new(const char *path)
 
 struct debuginfo *debuginfo__new(const char *path)
 {
-	char buf[PATH_MAX], nil = '\0';
 	struct dso *dso;
-	struct debuginfo *dinfo = NULL;
-	struct build_id bid = { .size = 0};
+	struct debuginfo *dinfo;
+	struct build_id bid = { .size = 0 };
+	char *filename;
+	bool decomp = false;
 
 	/* Try to open distro debuginfo files */
 	dso = dso__new(path);
@@ -112,11 +113,15 @@ struct debuginfo *debuginfo__new(const char *path)
 		dso__set_build_id(dso, &bid);
 
 	dso__find_dbginfo_type(dso);
-	if (dso__read_binary_type_filename(dso, dso__dbginfo_type(dso), &nil,
-					   buf, PATH_MAX) < 0)
+	filename = dso__get_filename(dso, "", &decomp, dso__dbginfo_type(dso));
+	if (filename == NULL)
 		return NULL;
 
-	dinfo = __debuginfo__new(buf);
+	dinfo = __debuginfo__new(filename);
+
+	if (decomp)
+		unlink(filename);
+	free(filename);
 	dso__put(dso);
 
 	return dinfo;
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 512f2bb61e8f2a2f..52f3e78c37220780 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -590,8 +590,8 @@ char *dso__filename_with_chroot(const struct dso *dso, const char *filename)
 	return filename_with_chroot(nsinfo__pid(dso__nsinfo_const(dso)), filename);
 }
 
-static char *dso__get_filename(struct dso *dso, const char *root_dir,
-			       bool *decomp)
+char *dso__get_filename(struct dso *dso, const char *root_dir, bool *decomp,
+			enum dso_binary_type type)
 {
 	char *name = malloc(PATH_MAX);
 
@@ -600,8 +600,7 @@ static char *dso__get_filename(struct dso *dso, const char *root_dir,
 	if (name == NULL)
 		return NULL;
 
-	if (dso__read_binary_type_filename(dso, dso__binary_type(dso),
-					    root_dir, name, PATH_MAX))
+	if (dso__read_binary_type_filename(dso, type, root_dir, name, PATH_MAX))
 		goto out;
 
 	if (!is_regular_file(name)) {
@@ -671,7 +670,8 @@ static int __open_dso(struct dso *dso, struct machine *machine)
 
 	mutex_lock(dso__lock(dso));
 
-	name = dso__get_filename(dso, machine ? machine->root_dir : "", &decomp);
+	name = dso__get_filename(dso, machine ? machine->root_dir : "", &decomp,
+				 dso__binary_type(dso));
 	if (name) {
 		fd = do_open(name);
 	} else {
@@ -2087,7 +2087,7 @@ struct debuginfo *dso__debuginfo(struct dso *dso)
 
 	mutex_lock(dso__lock(dso));
 
-	name = dso__get_filename(dso, "", &decomp);
+	name = dso__get_filename(dso, "", &decomp, dso__dbginfo_type(dso));
 	if (name)
 		dinfo = debuginfo__new(name);
 
@@ -2121,17 +2121,24 @@ void dso__find_dbginfo_type(struct dso *dso)
 		DSO_BINARY_TYPE__NOT_FOUND,
 	};
 	const enum dso_binary_type *type;
-	char buf[PATH_MAX];
+	char *path;
+	bool found, decomp = false;
 
 	if (dso__dbginfo_type(dso) != DSO_BINARY_TYPE__NOT_FOUND)
 		return;
 
 	for (type = dbginfo_types; *type != DSO_BINARY_TYPE__NOT_FOUND; type++) {
-		if (dso__read_binary_type_filename(dso, *type, "",
-						   buf, PATH_MAX) < 0)
+		path = dso__get_filename(dso, "", &decomp, *type);
+		if (path == NULL)
 			continue;
 
-		if (filename__has_section(buf, ".debug_info"))
+		found = filename__has_section(path, ".debug_info");
+
+		if (decomp)
+			unlink(path);
+		free(path);
+
+		if (found)
 			break;
 	}
 
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index a152957322f6b31c..94cc7ea89ceb5bd9 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -841,6 +841,8 @@ int dso__kernel_module_get_build_id(struct dso *dso, const char *root_dir);
 char dso__symtab_origin(const struct dso *dso);
 int dso__read_binary_type_filename(const struct dso *dso, enum dso_binary_type type,
 				   const char *root_dir, char *filename, size_t size);
+char *dso__get_filename(struct dso *dso, const char *root_dir, bool *decomp,
+			enum dso_binary_type type);
 bool is_kernel_module(const char *pathname, int cpumode);
 bool dso__needs_decompress(struct dso *dso);
 int dso__decompress_kmodule_fd(struct dso *dso, const char *name);
-- 
2.55.0

[PATCH v1 5/8] perf tools: Add dso__put_filename()

[Namhyung Kim]

It's a counterpart of the dso__get_filename() as the returned string
should be free.  Optionally it needs to remove a temporary file if it's
decompressed on the fly.

Currently the dso argument is not used, but it's better to pair with the
matching dso.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/debuginfo.c |  5 +----
 tools/perf/util/dso.c       | 28 +++++++++++++---------------
 tools/perf/util/dso.h       |  2 ++
 3 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/tools/perf/util/debuginfo.c b/tools/perf/util/debuginfo.c
index 38e9299765182bd7..fc2640dcd29d3aaa 100644
--- a/tools/perf/util/debuginfo.c
+++ b/tools/perf/util/debuginfo.c
@@ -118,10 +118,7 @@ struct debuginfo *debuginfo__new(const char *path)
 		return NULL;
 
 	dinfo = __debuginfo__new(filename);
-
-	if (decomp)
-		unlink(filename);
-	free(filename);
+	dso__put_filename(dso, filename, decomp);
 	dso__put(dso);
 
 	return dinfo;
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 52f3e78c37220780..0c76441fba82c413 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -661,6 +661,14 @@ char *dso__get_filename(struct dso *dso, const char *root_dir, bool *decomp,
 	return NULL;
 }
 
+void dso__put_filename(struct dso *dso __maybe_unused, char *filename, bool decomp)
+{
+	if (decomp)
+		unlink(filename);
+
+	free(filename);
+}
+
 static int __open_dso(struct dso *dso, struct machine *machine)
 	EXCLUSIVE_LOCKS_REQUIRED(_dso__data_open_lock)
 {
@@ -680,11 +688,8 @@ static int __open_dso(struct dso *dso, struct machine *machine)
 		fd = -errno;
 	}
 
-	if (decomp)
-		unlink(name);
-
+	dso__put_filename(dso, name, decomp);
 	mutex_unlock(dso__lock(dso));
-	free(name);
 	return fd;
 }
 
@@ -2088,14 +2093,11 @@ struct debuginfo *dso__debuginfo(struct dso *dso)
 	mutex_lock(dso__lock(dso));
 
 	name = dso__get_filename(dso, "", &decomp, dso__dbginfo_type(dso));
-	if (name)
+	if (name) {
 		dinfo = debuginfo__new(name);
-
-	if (decomp)
-		unlink(name);
-
+		dso__put_filename(dso, name, decomp);
+	}
 	mutex_unlock(dso__lock(dso));
-	free(name);
 	return dinfo;
 }
 
@@ -2133,11 +2135,7 @@ void dso__find_dbginfo_type(struct dso *dso)
 			continue;
 
 		found = filename__has_section(path, ".debug_info");
-
-		if (decomp)
-			unlink(path);
-		free(path);
-
+		dso__put_filename(dso, path, decomp);
 		if (found)
 			break;
 	}
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 94cc7ea89ceb5bd9..45c4954d5eaf9ffa 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -841,8 +841,10 @@ int dso__kernel_module_get_build_id(struct dso *dso, const char *root_dir);
 char dso__symtab_origin(const struct dso *dso);
 int dso__read_binary_type_filename(const struct dso *dso, enum dso_binary_type type,
 				   const char *root_dir, char *filename, size_t size);
+/* returned filename should be freed by dso__put_filename() */
 char *dso__get_filename(struct dso *dso, const char *root_dir, bool *decomp,
 			enum dso_binary_type type);
+void dso__put_filename(struct dso *dso, char *filename, bool decomp);
 bool is_kernel_module(const char *pathname, int cpumode);
 bool dso__needs_decompress(struct dso *dso);
 int dso__decompress_kmodule_fd(struct dso *dso, const char *name);
-- 
2.55.0

[PATCH v1 6/8] perf tools: Avoid repeated failing search for debuginfo

[Namhyung Kim]

If dso has no debuginfo, dso__find_dbginfo_type() may repeat the same
search for available types.  Let's add a new field (debuginfo_searched)
not to do it.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/dso.c |  4 ++++
 tools/perf/util/dso.h | 11 +++++++++++
 2 files changed, 15 insertions(+)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 0c76441fba82c413..c36192e7ea557b47 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -2129,6 +2129,10 @@ void dso__find_dbginfo_type(struct dso *dso)
 	if (dso__dbginfo_type(dso) != DSO_BINARY_TYPE__NOT_FOUND)
 		return;
 
+	if (dso__debuginfo_searched(dso))
+		return;
+	dso__set_debuginfo_searched(dso);
+
 	for (type = dbginfo_types; *type != DSO_BINARY_TYPE__NOT_FOUND; type++) {
 		path = dso__get_filename(dso, "", &decomp, *type);
 		if (path == NULL)
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 45c4954d5eaf9ffa..3f08d45e7f53a508 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -349,6 +349,7 @@ DECLARE_RC_STRUCT(dso) {
 	u8		 short_name_allocated:1;
 	u8		 long_name_allocated:1;
 	u8		 is_64_bit:1;
+	u8		 debuginfo_searched:1;
 	bool		 sorted_by_name;
 	bool		 loaded;
 	u8		 rel;
@@ -1019,6 +1020,16 @@ static inline void dso__set_dbginfo_type(struct dso *dso, enum dso_binary_type b
 
 void dso__find_dbginfo_type(struct dso *dso);
 
+static inline bool dso__debuginfo_searched(const struct dso *dso)
+{
+	return RC_CHK_ACCESS(dso)->debuginfo_searched;
+}
+
+static inline void dso__set_debuginfo_searched(struct dso *dso)
+{
+	RC_CHK_ACCESS(dso)->debuginfo_searched = 1;
+}
+
 bool dso__is_object_file(const struct dso *dso);
 
 void dso__free_a2l(struct dso *dso);
-- 
2.55.0

[PATCH v1 7/8] perf tools: Looks symbol file first when check debuginfo

[Namhyung Kim]

There's a high chance that the file containing the symbol table also has
debug information.  Let's check that first before other locations.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/dso.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index c36192e7ea557b47..9c8b963906a7e88c 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -2133,6 +2133,18 @@ void dso__find_dbginfo_type(struct dso *dso)
 		return;
 	dso__set_debuginfo_searched(dso);
 
+	/* Maybe debug info is in the same file with the symbol table */
+	path = dso__get_filename(dso, "", &decomp, dso__symtab_type(dso));
+	if (path) {
+		found = filename__has_section(path, ".debug_info");
+		dso__put_filename(dso, path, decomp);
+		if (found) {
+			dso__set_dbginfo_type(dso, dso__symtab_type(dso));
+			return;
+		}
+	}
+
+	/* Otherwise check distro debug locations */
 	for (type = dbginfo_types; *type != DSO_BINARY_TYPE__NOT_FOUND; type++) {
 		path = dso__get_filename(dso, "", &decomp, *type);
 		if (path == NULL)
-- 
2.55.0

[PATCH v1 8/8] perf tools: Update debuginfo__new() to take DSO

[Namhyung Kim]

It should be the common case to get debuginfo from existing DSOs.
Now dso__debuginfo() calls it with its debuginfo type to avoid the
unnecessary loop to find the location of debug file.

Update existing function to debuginfo__from_path() for clarity.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/debuginfo.c   | 28 ++++++++++++++++++----------
 tools/perf/util/debuginfo.h   | 13 +++++++++++--
 tools/perf/util/dso.c         | 11 ++---------
 tools/perf/util/probe-event.c |  4 ++--
 4 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/tools/perf/util/debuginfo.c b/tools/perf/util/debuginfo.c
index fc2640dcd29d3aaa..db188c397d63880c 100644
--- a/tools/perf/util/debuginfo.c
+++ b/tools/perf/util/debuginfo.c
@@ -91,13 +91,27 @@ static struct debuginfo *__debuginfo__new(const char *path)
 	return dbg;
 }
 
-struct debuginfo *debuginfo__new(const char *path)
+struct debuginfo *debuginfo__new(struct dso *dso)
+{
+	struct debuginfo *dinfo;
+	char *path;
+	bool decomp = false;
+
+	dso__find_dbginfo_type(dso);
+	path = dso__get_filename(dso, "", &decomp, dso__dbginfo_type(dso));
+	if (path == NULL)
+		return NULL;
+
+	dinfo = __debuginfo__new(path);
+	dso__put_filename(dso, path, decomp);
+	return dinfo;
+}
+
+struct debuginfo *debuginfo__from_path(const char *path)
 {
 	struct dso *dso;
 	struct debuginfo *dinfo;
 	struct build_id bid = { .size = 0 };
-	char *filename;
-	bool decomp = false;
 
 	/* Try to open distro debuginfo files */
 	dso = dso__new(path);
@@ -112,13 +126,7 @@ struct debuginfo *debuginfo__new(const char *path)
 	if (filename__read_build_id(path, &bid) > 0)
 		dso__set_build_id(dso, &bid);
 
-	dso__find_dbginfo_type(dso);
-	filename = dso__get_filename(dso, "", &decomp, dso__dbginfo_type(dso));
-	if (filename == NULL)
-		return NULL;
-
-	dinfo = __debuginfo__new(filename);
-	dso__put_filename(dso, filename, decomp);
+	dinfo = debuginfo__new(dso);
 	dso__put(dso);
 
 	return dinfo;
diff --git a/tools/perf/util/debuginfo.h b/tools/perf/util/debuginfo.h
index a52d69932815cd72..ade40d54ea125988 100644
--- a/tools/perf/util/debuginfo.h
+++ b/tools/perf/util/debuginfo.h
@@ -18,8 +18,12 @@ struct debuginfo {
 	const unsigned char	*build_id;
 };
 
+struct dso;
+
 /* This also tries to open distro debuginfo */
-struct debuginfo *debuginfo__new(const char *path);
+struct debuginfo *debuginfo__from_path(const char *path);
+/* Create a debuginfo using information in DSO */
+struct debuginfo *debuginfo__new(struct dso *dso);
 void debuginfo__delete(struct debuginfo *dbg);
 
 int debuginfo__get_text_offset(struct debuginfo *dbg, Dwarf_Addr *offs,
@@ -31,7 +35,12 @@ int debuginfo__get_text_offset(struct debuginfo *dbg, Dwarf_Addr *offs,
 struct debuginfo {
 };
 
-static inline struct debuginfo *debuginfo__new(const char *path __maybe_unused)
+static inline struct debuginfo *debuginfo__from_path(const char *path __maybe_unused)
+{
+	return NULL;
+}
+
+static inline struct debuginfo *debuginfo__new(struct dso *dso __maybe_unused)
 {
 	return NULL;
 }
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 9c8b963906a7e88c..f8b5528427765b02 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -2086,17 +2086,10 @@ const u8 *dso__read_symbol(struct dso *dso, const char *symfs_filename,
 
 struct debuginfo *dso__debuginfo(struct dso *dso)
 {
-	char *name;
-	bool decomp = false;
-	struct debuginfo *dinfo = NULL;
+	struct debuginfo *dinfo;
 
 	mutex_lock(dso__lock(dso));
-
-	name = dso__get_filename(dso, "", &decomp, dso__dbginfo_type(dso));
-	if (name) {
-		dinfo = debuginfo__new(name);
-		dso__put_filename(dso, name, decomp);
-	}
+	dinfo = debuginfo__new(dso);
 	mutex_unlock(dso__lock(dso));
 	return dinfo;
 }
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 11ae4a09412c9bd9..1534280c0c1d59cb 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -519,7 +519,7 @@ static struct debuginfo *open_from_debuginfod(struct dso *dso, struct nsinfo *ns
 		pr_debug("Load debuginfo from debuginfod (%s)\n", path);
 
 	nsinfo__mountns_enter(nsi, &nsc);
-	ret = debuginfo__new((const char *)path);
+	ret = debuginfo__from_path((const char *)path);
 	nsinfo__mountns_exit(&nsc);
 	return ret;
 }
@@ -567,7 +567,7 @@ static struct debuginfo *open_debuginfo(const char *module, struct nsinfo *nsi,
 		path = dso__long_name(dso);
 	}
 	nsinfo__mountns_enter(nsi, &nsc);
-	ret = debuginfo__new(path);
+	ret = debuginfo__from_path(path);
 	if (!ret && !silent) {
 		pr_warning("The %s file has no debug information.\n", path);
 		if (!module || !strtailcmp(path, ".ko"))
-- 
2.55.0

Re: [PATCH v1 5/8] perf tools: Add dso__put_filename()

[sashiko-bot]

> It's a counterpart of the dso__get_filename() as the returned string
> should be free.  Optionally it needs to remove a temporary file if it's
> decompressed on the fly.
> 
> Currently the dso argument is not used, but it's better to pair with the
> matching dso.
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928055055.95144-1-namhyung@kernel.org?part=5

Re: [PATCH v1 1/8] perf tools: Add dso->dbginfo_type field

[sashiko-bot]

> It saves where the debug info is available for the given DSO.  It's
> sometimes different than where binary data or symbol table is located.
> So let's add a new field to save the info.
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928055055.95144-1-namhyung@kernel.org?part=1

Re: [PATCH v1 2/8] perf tools: Factor out dso__find_dbginfo_type()

[sashiko-bot]

> It tries to find a file with working debug info by checking all possible
> debug types.
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928055055.95144-1-namhyung@kernel.org?part=2

Re: [PATCH v1 3/8] perf tools: Check system path when check debuginfo

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] perf tools: Loss of DWARF support for .gnu_debugdata and .zdebug_info
--

commit 945ce7f07042383e50c7db270f3808ce02dd95b3
Author: Namhyung Kim <namhyung@kernel.org>

perf tools: Check system path when check debuginfo

This commit removes the fallback to open the given binary path in
debuginfo__new() and instead relies on checking the SYSTEM_PATH_DSO
type within dso__find_dbginfo_type().

> diff --git a/tools/perf/util/debuginfo.c b/tools/perf/util/debuginfo.c
> index d6bd0fcc22648..2669d5f5ee612 100644
> --- a/tools/perf/util/debuginfo.c
> +++ b/tools/perf/util/debuginfo.c
[ ... ]
> @@ -114,18 +114,12 @@ struct debuginfo *debuginfo__new(const char *path)
>  	dso__find_dbginfo_type(dso);
>  	if (dso__read_binary_type_filename(dso, dso__dbginfo_type(dso), &nil,
>  					   buf, PATH_MAX) < 0)
> -		goto out;
> +		return NULL;
>  
>  	dinfo = __debuginfo__new(buf);
>  	dso__put(dso);
>  
> -out:
> -	if (dinfo)
> -		return dinfo;
> -
> -	/* if failed to open all distro debuginfo, open given binary */
> -	symbol__join_symfs(buf, path);
> -	return __debuginfo__new(buf);

[Severity: High]
Does removing this fallback break DWARF support for files using alternative
debug sections like .gnu_debugdata (mini-debuginfo) or .zdebug_info?

The new logic entirely relies on dso__find_dbginfo_type() successfully
returning a binary type. However, dso__find_dbginfo_type() strictly
validates the file by exact string match:

tools/perf/util/dso.c:dso__find_dbginfo_type() {
	...
		if (filename__has_section(buf, ".debug_info"))
			break;
	...
}

Because files relying on .gnu_debugdata or .zdebug_info do not have a section
named exactly ".debug_info", this check fails. The type then defaults to
DSO_BINARY_TYPE__NOT_FOUND, and debuginfo__new() returns NULL.

Could this cause features like inline callchain resolution to silently fail on
these binaries since we completely bypass libdw's native support for parsing
these alternative formats?

> +	return dinfo;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928055055.95144-1-namhyung@kernel.org?part=3

Re: [PATCH v1 8/8] perf tools: Update debuginfo__new() to take DSO

[sashiko-bot]

> It should be the common case to get debuginfo from existing DSOs.
> Now dso__debuginfo() calls it with its debuginfo type to avoid the
> unnecessary loop to find the location of debug file.
> 
> Update existing function to debuginfo__from_path() for clarity.
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928055055.95144-1-namhyung@kernel.org?part=8

Re: [PATCH v1 6/8] perf tools: Avoid repeated failing search for debuginfo

[sashiko-bot]

> If dso has no debuginfo, dso__find_dbginfo_type() may repeat the same
> search for available types.  Let's add a new field (debuginfo_searched)
> not to do it.
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928055055.95144-1-namhyung@kernel.org?part=6

Re: [PATCH v1 7/8] perf tools: Looks symbol file first when check debuginfo

[sashiko-bot]

> There's a high chance that the file containing the symbol table also has
> debug information.  Let's check that first before other locations.
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928055055.95144-1-namhyung@kernel.org?part=7

Re: [PATCH v1 4/8] perf tools: Export dso__get_filename() with type argument

[sashiko-bot]

> The dso__get_filename() returns a new string of filepath for the given
> binary type.  It handles chroot and decompressed kernel modules so that
> callers can read the file contents.  Thus it'd be better to use it to
> check available binary types.
> 
> Add a type argument to handle a different path for symbol or debug info.
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928055055.95144-1-namhyung@kernel.org?part=4

Re: [PATCH v1 0/8] perf tools: Cleanup dso debug info access

[Arnaldo Carvalho de Melo]

On Sun, Sep 27, 2026 at 10:50:47PM -0700, Namhyung Kim wrote:
> Hello,
> 
> This is a preparation for DSO file access cleanup work.  IIUC DSO
> access has three types - symbol table, binary data and debug info.
> Those information can be placed separately like stripped binaries and
> split debug files.  I've added dbginfo_type to DSO to track location
> of debug info like others.
> 
> I'm sending this out before it grows too much.  This patchset focuses
> on debuginfo open path.  Once it looks good to others, I'll update
> other places accessing DWARF info to use this info and cleanup other
> file type accesses too.

Sashiko found an issue, apart from that:

Reviewed-by: Arnaldo Carvalho de Melo <acme@redhat.com>

- Arnaldo

Re: [PATCH v1 3/8] perf tools: Check system path when check debuginfo

[Namhyung Kim]

On Mon, Sep 28, 2026 at 05:59:56AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] perf tools: Loss of DWARF support for .gnu_debugdata and .zdebug_info
> --
> 
> commit 945ce7f07042383e50c7db270f3808ce02dd95b3
> Author: Namhyung Kim <namhyung@kernel.org>
> 
> perf tools: Check system path when check debuginfo
> 
> This commit removes the fallback to open the given binary path in
> debuginfo__new() and instead relies on checking the SYSTEM_PATH_DSO
> type within dso__find_dbginfo_type().
> 
> > diff --git a/tools/perf/util/debuginfo.c b/tools/perf/util/debuginfo.c
> > index d6bd0fcc22648..2669d5f5ee612 100644
> > --- a/tools/perf/util/debuginfo.c
> > +++ b/tools/perf/util/debuginfo.c
> [ ... ]
> > @@ -114,18 +114,12 @@ struct debuginfo *debuginfo__new(const char *path)
> >  	dso__find_dbginfo_type(dso);
> >  	if (dso__read_binary_type_filename(dso, dso__dbginfo_type(dso), &nil,
> >  					   buf, PATH_MAX) < 0)
> > -		goto out;
> > +		return NULL;
> >  
> >  	dinfo = __debuginfo__new(buf);
> >  	dso__put(dso);
> >  
> > -out:
> > -	if (dinfo)
> > -		return dinfo;
> > -
> > -	/* if failed to open all distro debuginfo, open given binary */
> > -	symbol__join_symfs(buf, path);
> > -	return __debuginfo__new(buf);
> 
> [Severity: High]
> Does removing this fallback break DWARF support for files using alternative
> debug sections like .gnu_debugdata (mini-debuginfo) or .zdebug_info?

IIUC ".gnu_debugdata" on Fedora only carries symbol tables so it doesn't
check DSO_BINARY_TYPE__GNU_DEBUGDATA and no need to check the section.
But I think I need to add ".zdebug_info".

> 
> The new logic entirely relies on dso__find_dbginfo_type() successfully
> returning a binary type. However, dso__find_dbginfo_type() strictly
> validates the file by exact string match:
> 
> tools/perf/util/dso.c:dso__find_dbginfo_type() {
> 	...
> 		if (filename__has_section(buf, ".debug_info"))
> 			break;
> 	...
> }
> 
> Because files relying on .gnu_debugdata or .zdebug_info do not have a section
> named exactly ".debug_info", this check fails. The type then defaults to
> DSO_BINARY_TYPE__NOT_FOUND, and debuginfo__new() returns NULL.
> 
> Could this cause features like inline callchain resolution to silently fail on
> these binaries since we completely bypass libdw's native support for parsing
> these alternative formats?

Something like this?

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 10c4cd35b6d9464b..0b152988d18b60b0 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -2130,7 +2130,8 @@ void dso__find_dbginfo_type(struct dso *dso)
                                                   buf, PATH_MAX) < 0)
                        continue;

-               if (filename__has_section(buf, ".debug_info"))
+               if (filename__has_section(buf, ".debug_info") ||
+                   filename__has_section(buf, ".zdebug_info"))
                        break;
        }

Thanks,
Namhyung


> 
> > +	return dinfo;
> >  }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260928055055.95144-1-namhyung@kernel.org?part=3

Re: [PATCH v1 0/8] perf tools: Cleanup dso debug info access

[Namhyung Kim]

On Mon, Sep 28, 2026 at 06:45:32PM +0200, Arnaldo Carvalho de Melo wrote:
> On Sun, Sep 27, 2026 at 10:50:47PM -0700, Namhyung Kim wrote:
> > Hello,
> > 
> > This is a preparation for DSO file access cleanup work.  IIUC DSO
> > access has three types - symbol table, binary data and debug info.
> > Those information can be placed separately like stripped binaries and
> > split debug files.  I've added dbginfo_type to DSO to track location
> > of debug info like others.
> > 
> > I'm sending this out before it grows too much.  This patchset focuses
> > on debuginfo open path.  Once it looks good to others, I'll update
> > other places accessing DWARF info to use this info and cleanup other
> > file type accesses too.
> 
> Sashiko found an issue, apart from that:
> 
> Reviewed-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Thanks for the review!
Namhyung