[PATCH v2] perf symbols: Don't apply the symfs layout to synthesised paths

[PATCH v2] perf symbols: Don't apply the symfs layout to synthesised paths

[Zhan Xusheng]

From: Zhan Xusheng <zhanxusheng1024@gmail.com>

From: Zhan Xusheng <zhanxusheng@xiaomi.com>

The flat symfs layout was implemented inside __symbol__join_symfs() alone,
which went from

        return path__join(bf, size, symbol_conf.symfs, path);

to taking perf_basename(path) first.  No caller was changed, so all of them
got it.  Most pass dso__long_name(), which is what the option is about, but
some pass a path perf built itself:

  dso.c        "/usr/lib/debug"             -> "debug"
  dso.c        "/usr/lib/debug/.build-id/"  -> ""
  build-id.c   "/usr/lib/debug/.build-id/"  -> ""
  disasm.c     <file under buildid_dir>     -> last component

perf_basename() yields "" for a path ending in '/'.

Tracing the lookups against an empty symfs,

  perf record -o pd.data -- sleep 0.3
  strace -f -e trace=openat,newfstatat \
      perf report -i pd.data --symfs <symfs>,flat --stdio

15 paths get tried.  Four of them differ, in every case only in the
prefix, which the patch restores:

  FEDORA, UBUNTU, MIXEDUP_UBUNTU
    <symfs>/debug/  ->  <symfs>//usr/lib/debug/
  BUILDID_DEBUGINFO
    <symfs>/        ->  <symfs>//usr/lib/debug/.build-id/

The tail is dso__long_name() for the first three and the build-id file for
the fourth, the same either way, so the build-id file was being looked for
in the symfs root and the distro debuginfo under <symfs>/debug/.  Those
prefixes follow neither layout: perf's own prefix is flattened while
dso__long_name() is still appended whole.

Among the 11 paths that do not change are the basename lookups the option
is for, <symfs>/sleep and <symfs>/vmlinux.  The same trace with
,hierarchy is identical between the two builds.

Use path__join() at those sites, which is what the helper did for them
before.  A hierarchy layout is unaffected, being that same call.  The
OPENEMBEDDED site passes "", where perf_basename() is already a no-op;
it is converted for uniformity.  Every remaining __symbol__join_symfs()
caller passes a path from the profiled system.

Fixes: f182573e06ab ("perf tools: Add layout support for --symfs option")
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
v1->v2:
- Added the traced before/after to the changelog, as asked by Ian Rogers.
  No code change.

v1: https://lore.kernel.org/r/20260811050046.4019578-1-zhanxusheng@xiaomi.com

 tools/perf/util/build-id.c |  2 +-
 tools/perf/util/disasm.c   |  4 +++-
 tools/perf/util/dso.c      | 11 ++++++-----
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index eb95ab90f974..0c89fe75a650 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -599,7 +599,7 @@ static char *build_id_cache__find_debug(const char *sbuild_id,
 		dirname = dirbuf;
 	}
 
-	len = __symbol__join_symfs(debugfile, PATH_MAX, dirname);
+	len = path__join(debugfile, PATH_MAX, symbol_conf.symfs, dirname);
 	snprintf(debugfile + len, PATH_MAX - len, "%.2s/%s.debug", sbuild_id,
 		 sbuild_id + 2);
 
diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
index 0a1a7e9cf3ef..decf9348d735 100644
--- a/tools/perf/util/disasm.c
+++ b/tools/perf/util/disasm.c
@@ -31,6 +31,7 @@
 #include "map.h"
 #include "maps.h"
 #include "namespaces.h"
+#include "path.h"
 #include "srcline.h"
 #include "symbol.h"
 #include "thread.h"
@@ -1173,7 +1174,8 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
 
 	build_id_filename = dso__build_id_filename(dso, NULL, 0, false);
 	if (build_id_filename) {
-		__symbol__join_symfs(filename, filename_size, build_id_filename);
+		path__join(filename, filename_size, symbol_conf.symfs,
+			   build_id_filename);
 		free(build_id_filename);
 	} else {
 		if (dso__has_build_id(dso))
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 2309196d8df3..314c9a0edf28 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -167,12 +167,12 @@ int dso__read_binary_type_filename(const struct dso *dso,
 		break;
 
 	case DSO_BINARY_TYPE__FEDORA_DEBUGINFO:
-		len = __symbol__join_symfs(filename, size, "/usr/lib/debug");
+		len = path__join(filename, size, symbol_conf.symfs, "/usr/lib/debug");
 		snprintf(filename + len, size - len, "%s.debug", dso__long_name(dso));
 		break;
 
 	case DSO_BINARY_TYPE__UBUNTU_DEBUGINFO:
-		len = __symbol__join_symfs(filename, size, "/usr/lib/debug");
+		len = path__join(filename, size, symbol_conf.symfs, "/usr/lib/debug");
 		snprintf(filename + len, size - len, "%s", dso__long_name(dso));
 		break;
 
@@ -187,7 +187,7 @@ int dso__read_binary_type_filename(const struct dso *dso,
 			ret = -1;
 			break;
 		}
-		len = __symbol__join_symfs(filename, size, "/usr/lib/debug");
+		len = path__join(filename, size, symbol_conf.symfs, "/usr/lib/debug");
 		snprintf(filename + len, size - len, "%s", dso__long_name(dso) + 4);
 		break;
 
@@ -200,7 +200,7 @@ int dso__read_binary_type_filename(const struct dso *dso,
 		while (last_slash != dso__long_name(dso) && *last_slash != '/')
 			last_slash--;
 
-		len = __symbol__join_symfs(filename, size, "");
+		len = path__join(filename, size, symbol_conf.symfs, "");
 		dir_size = last_slash - dso__long_name(dso) + 2;
 		if (dir_size > (size - len)) {
 			ret = -1;
@@ -219,7 +219,8 @@ int dso__read_binary_type_filename(const struct dso *dso,
 		}
 
 		build_id__snprintf(dso__bid(dso), build_id_hex, sizeof(build_id_hex));
-		len = __symbol__join_symfs(filename, size, "/usr/lib/debug/.build-id/");
+		len = path__join(filename, size, symbol_conf.symfs,
+				 "/usr/lib/debug/.build-id/");
 		snprintf(filename + len, size - len, "%.2s/%s.debug",
 			 build_id_hex, build_id_hex + 2);
 		break;
-- 
2.43.0

Re: [PATCH v2] perf symbols: Don't apply the symfs layout to synthesised paths

[sashiko-bot]

> From: Zhan Xusheng <zhanxusheng1024@gmail.com>
> 
> From: Zhan Xusheng <zhanxusheng@xiaomi.com>
> 
> The flat symfs layout was implemented inside __symbol__join_symfs() alone,
> which went from
> 
>         return path__join(bf, size, symbol_conf.symfs, path);
> 
> to taking perf_basename(path) first.  No caller was changed, so all of them
> got it.  Most pass dso__long_name(), which is what the option is about, but
> some pass a path perf built itself:
> 
>   dso.c        "/usr/lib/debug"             -> "debug"
>   dso.c        "/usr/lib/debug/.build-id/"  -> ""
> [ ... ]
> it is converted for uniformity.  Every remaining __symbol__join_symfs()
> caller passes a path from the profiled system.
> 
> Fixes: f182573e06ab ("perf tools: Add layout support for --symfs option")
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819094621.844115-1-zhanxusheng@xiaomi.com?part=1