[PATCH v3 1/4] perf symbol: Fix symbol loading from libbfd

[PATCH v3 1/4] perf symbol: Fix symbol loading from libbfd

[Namhyung Kim]

The dso__load() should return a positive number (number of symbols
loaded) when it succeded to load symbols.  But dso__load_bfd_symbols()
only returns 0 and it's hard to check if it actually loaded symbols or
not.  Let's return the value of dso__has_symbol() and check it in
dso__load() so that it can return 1 for success.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/r/sashiko-outbox-162540@kernel.org
Fixes: eac9a4342e5447cad ("perf symbols: Try reading the symbol table with libbfd")
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/tests/pe-file-parsing.c | 2 +-
 tools/perf/util/libbfd.c           | 2 +-
 tools/perf/util/symbol.c           | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/tests/pe-file-parsing.c b/tools/perf/tests/pe-file-parsing.c
index 30c7da79e109b480..3979f38bf127c218 100644
--- a/tools/perf/tests/pe-file-parsing.c
+++ b/tools/perf/tests/pe-file-parsing.c
@@ -59,7 +59,7 @@ static int run_dir(const char *d)
 	TEST_ASSERT_VAL("Failed to get dso", dso);
 
 	ret = dso__load_bfd_symbols(dso, debugfile);
-	TEST_ASSERT_VAL("Failed to load symbols", ret == 0);
+	TEST_ASSERT_VAL("Failed to load symbols", ret > 0);
 
 	dso__sort_by_name(dso);
 	sym = dso__find_symbol_by_name(dso, "main", &idx);
diff --git a/tools/perf/util/libbfd.c b/tools/perf/util/libbfd.c
index a907d7c1ac948808..a4dcb406c79e9cee 100644
--- a/tools/perf/util/libbfd.c
+++ b/tools/perf/util/libbfd.c
@@ -463,7 +463,7 @@ int dso__load_bfd_symbols(struct dso *dso, const char *debugfile)
 	symbols__fixup_duplicate(dso__symbols(dso));
 	dso__set_adjust_symbols(dso, true);
 
-	err = 0;
+	err = dso__has_symbols(dso);
 out_free:
 	free(symbols);
 out_close:
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 116870595f75c7dd..2b1c1b8d9652e720 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1921,8 +1921,8 @@ int dso__load(struct dso *dso, struct map *map)
 		if (nsexit)
 			nsinfo__mountns_enter(dso__nsinfo(dso), &nsc);
 
-		if (bfdrc == 0) {
-			ret = 0;
+		if (bfdrc > 0) {
+			ret = 1;
 			break;
 		}
 
-- 
2.56.0.385.gd3acb90ef8-goog

[PATCH v3 2/4] iperf symbol: Add missing symsrc__destroy() for libbfd

[Namhyung Kim]

When libbfd loaded symbols and there's a pending syms_ss, it'd be lost
or overwrite existing symbols.  Let's destroy it before breaking the
loop so that it can safely return from the function.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/r/sashiko-outbox-162540@kernel.org
Fixes: eac9a4342e5447cad ("perf symbols: Try reading the symbol table with libbfd")
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/symbol.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 2b1c1b8d9652e720..02845b69ae6b5fcf 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1923,7 +1923,7 @@ int dso__load(struct dso *dso, struct map *map)
 
 		if (bfdrc > 0) {
 			ret = 1;
-			break;
+			goto out_destroy;
 		}
 
 		if (!is_reg || sirc < 0)
@@ -1975,6 +1975,7 @@ int dso__load(struct dso *dso, struct map *map)
 			ret += nr_plt;
 	}
 
+out_destroy:
 	for (; ss_pos > 0; ss_pos--)
 		symsrc__destroy(&ss_[ss_pos - 1]);
 out_free:
-- 
2.56.0.385.gd3acb90ef8-goog

[PATCH v3 3/4] perf symbol: Set dso symtab type for libbfd

[Namhyung Kim]

It missed to set the dso symtab type when the symbols are loaded by
libbfd.  When it happens both syms_ss and runtime_ss may not be set
and won't call dso__load_sym().

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

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 02845b69ae6b5fcf..b61a574ce9a30721 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1922,6 +1922,7 @@ int dso__load(struct dso *dso, struct map *map)
 			nsinfo__mountns_enter(dso__nsinfo(dso), &nsc);
 
 		if (bfdrc > 0) {
+			dso__set_symtab_type(dso, symtab_type);
 			ret = 1;
 			goto out_destroy;
 		}
-- 
2.56.0.385.gd3acb90ef8-goog

[PATCH v3 4/4] perf test: Update PE file test for other locations

[Namhyung Kim]

The test__pe_file_parsing() expects the test is in the tools/perf
directory, but it could be root or a separate build directory.  Let's
update the logic to try other paths like in the shell_tests__dir_fd().

Also add some debug messages to inform users why it's skipped.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/tests/pe-file-parsing.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/tools/perf/tests/pe-file-parsing.c b/tools/perf/tests/pe-file-parsing.c
index 3979f38bf127c218..190520ffc3ad6d0c 100644
--- a/tools/perf/tests/pe-file-parsing.c
+++ b/tools/perf/tests/pe-file-parsing.c
@@ -74,17 +74,20 @@ static int test__pe_file_parsing(struct test_suite *test __maybe_unused,
 {
 	struct stat st;
 	char path_dir[PATH_MAX];
-
-	/* First try development tree tests. */
-	if (!lstat("./tests", &st))
-		return run_dir("./tests");
-
-	/* Then installed path. */
+	const char * const tests_dirs[] = {
+		"./tools/perf/tests",
+		"./tests",
+		"./source/tests",
+		path_dir,
+	};
 	snprintf(path_dir, PATH_MAX, "%s/tests", get_argv_exec_path());
 
-	if (!lstat(path_dir, &st))
-		return run_dir(path_dir);
+	for (size_t i = 0; i < ARRAY_SIZE(tests_dirs); ++i) {
+		if (!lstat(tests_dirs[i], &st))
+			return run_dir(tests_dirs[i]);
+	}
 
+	pr_debug("cannot find the tests directory\n");
 	return TEST_SKIP;
 }
 
@@ -93,6 +96,7 @@ static int test__pe_file_parsing(struct test_suite *test __maybe_unused,
 static int test__pe_file_parsing(struct test_suite *test __maybe_unused,
 			  int subtest __maybe_unused)
 {
+	pr_debug("libbfd is not available\n");
 	return TEST_SKIP;
 }
 
-- 
2.56.0.385.gd3acb90ef8-goog

Re: [PATCH v3 1/4] perf symbol: Fix symbol loading from libbfd

[Arnaldo Carvalho de Melo]

On Mon, Oct 12, 2026 at 04:59:34PM -0700, Namhyung Kim wrote:
> The dso__load() should return a positive number (number of symbols
> loaded) when it succeded to load symbols.  But dso__load_bfd_symbols()
> only returns 0 and it's hard to check if it actually loaded symbols or
> not.  Let's return the value of dso__has_symbol() and check it in
> dso__load() so that it can return 1 for success.
> 
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/r/sashiko-outbox-162540@kernel.org
> Fixes: eac9a4342e5447cad ("perf symbols: Try reading the symbol table with libbfd")
> Reviewed-by: Ian Rogers <irogers@google.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Thanks, applied to perf-tools-next, for v7.4.

- Arnaldo

Re: [PATCH v3 1/4] perf symbol: Fix symbol loading from libbfd

[Namhyung Kim]

On Tue, Oct 13, 2026 at 03:35:24PM -0300, Arnaldo Carvalho de Melo wrote:
> On Mon, Oct 12, 2026 at 04:59:34PM -0700, Namhyung Kim wrote:
> > The dso__load() should return a positive number (number of symbols
> > loaded) when it succeded to load symbols.  But dso__load_bfd_symbols()
> > only returns 0 and it's hard to check if it actually loaded symbols or
> > not.  Let's return the value of dso__has_symbol() and check it in
> > dso__load() so that it can return 1 for success.
> > 
> > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > Closes: https://lore.kernel.org/r/sashiko-outbox-162540@kernel.org
> > Fixes: eac9a4342e5447cad ("perf symbols: Try reading the symbol table with libbfd")
> > Reviewed-by: Ian Rogers <irogers@google.com>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> 
> Thanks, applied to perf-tools-next, for v7.4.

Thanks Arnaldo!

As you may know, this is from my other series on cleaning up symtab and
binary types.  I'd like to land it as well so that I can continue
working on the related issues.  There are some Sashiko feedback left,
but I think they are either pre-existing bugs or known issues.

Thanks,
Namhyung

Re: [PATCH v3 1/4] perf symbol: Fix symbol loading from libbfd

[Arnaldo Carvalho de Melo]

On Tue, Oct 13, 2026 at 02:02:15PM -0700, Namhyung Kim wrote:
> On Tue, Oct 13, 2026 at 03:35:24PM -0300, Arnaldo Carvalho de Melo wrote:
> > On Mon, Oct 12, 2026 at 04:59:34PM -0700, Namhyung Kim wrote:
> > > The dso__load() should return a positive number (number of symbols
> > > loaded) when it succeded to load symbols.  But dso__load_bfd_symbols()
> > > only returns 0 and it's hard to check if it actually loaded symbols or
> > > not.  Let's return the value of dso__has_symbol() and check it in
> > > dso__load() so that it can return 1 for success.
> > > 
> > > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > > Closes: https://lore.kernel.org/r/sashiko-outbox-162540@kernel.org
> > > Fixes: eac9a4342e5447cad ("perf symbols: Try reading the symbol table with libbfd")
> > > Reviewed-by: Ian Rogers <irogers@google.com>
> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > 
> > Thanks, applied to perf-tools-next, for v7.4.
> 
> Thanks Arnaldo!
> 
> As you may know, this is from my other series on cleaning up symtab and
> binary types.  I'd like to land it as well so that I can continue
> working on the related issues.  There are some Sashiko feedback left,
> but I think they are either pre-existing bugs or known issues.

Right, its now at tmp.perf-tools-next, while I test build in my
container set, will push it to perf-tools-next next.

- Arnaldo

Re: [PATCH v3 1/4] perf symbol: Fix symbol loading from libbfd

[Arnaldo Carvalho de Melo]

On Wed, Oct 14, 2026 at 07:17:39PM -0300, Arnaldo Carvalho de Melo wrote:
> On Tue, Oct 13, 2026 at 02:02:15PM -0700, Namhyung Kim wrote:
> > On Tue, Oct 13, 2026 at 03:35:24PM -0300, Arnaldo Carvalho de Melo wrote:
> > > On Mon, Oct 12, 2026 at 04:59:34PM -0700, Namhyung Kim wrote:
> > > > The dso__load() should return a positive number (number of symbols
> > > > loaded) when it succeded to load symbols.  But dso__load_bfd_symbols()
> > > > only returns 0 and it's hard to check if it actually loaded symbols or
> > > > not.  Let's return the value of dso__has_symbol() and check it in
> > > > dso__load() so that it can return 1 for success.
> > > > 
> > > > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > > > Closes: https://lore.kernel.org/r/sashiko-outbox-162540@kernel.org
> > > > Fixes: eac9a4342e5447cad ("perf symbols: Try reading the symbol table with libbfd")
> > > > Reviewed-by: Ian Rogers <irogers@google.com>
> > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > 
> > > Thanks, applied to perf-tools-next, for v7.4.
> > 
> > Thanks Arnaldo!
> > 
> > As you may know, this is from my other series on cleaning up symtab and
> > binary types.  I'd like to land it as well so that I can continue
> > working on the related issues.  There are some Sashiko feedback left,
> > but I think they are either pre-existing bugs or known issues.
> 
> Right, its now at tmp.perf-tools-next, while I test build in my
> container set, will push it to perf-tools-next next.

Building it with CC=clang I get:

  LD      /tmp/build/perf-tools-next/tests/perf-test-in.o
util/dso.c:942:7: error: mutex '_dso__data_open_lock' is not held on every path through here [-Werror,-Wthread-safety-analysis]
  942 |                 if (dso__data_open_cnt) {
      |                     ^
util/dso.c:940:4: note: mutex acquired here
  940 |                         mutex_lock(dso__data_open_lock());
      |                         ^
util/dso.c:942:7: error: reading variable 'dso__data_open_cnt' requires holding mutex '_dso__data_open_lock'   CC      /tmp/build/perf-tools-next/util/maps.o
[-Werror,-Wthread-safety-analysis]
  942 |                 if (dso__data_open_cnt) {
      |                     ^
util/dso.c:943:4: error: calling function 'close_first_dso' requires holding mutex '_dso__data_open_lock' exclusively [-Werror,-Wthread-safety-analysis]
  943 |                         close_first_dso();
      |                         ^
util/dso.c:949:3: error: releasing mutex '_dso__data_open_lock' that was not held [-Werror,-Wthread-safety-analysis]
  949 |                 mutex_unlock(dso__data_open_lock());
      |                 ^
4 errors generated.
make[4]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:95: /tmp/build/perf-tools-next/util/dso.o] Error 1
  CC      /tmp/build/perf-tools-next/util/thread.o

Bisected to:

⬢ [acme@tb-phoenix2-perf perf-tools-next]$ git bisect good
3759ca226d57d06c4c52a1e64890faf58a29ebf1 is the first 'bad' commit
commit 3759ca226d57d06c4c52a1e64890faf58a29ebf1
Author: Namhyung Kim <namhyung@kernel.org>
Date:   Tue Oct 13 10:21:51 2026 -0700

    perf tools: Factor out dso__find_binary_type()

    Separate out the logic to detect binary type of a DSO.  It checks if
    the candidate file contains ".text" section.  There could be data-only
    DSOs but as our use cases focus on the code (perf annotate or intel
    PT), we can ignore them for now.

    But it also can fail due to open file limit.  Handle the case by
    retrying with _dso__data_open_lock.

    Introduce bintype_searched flag and it accessors to prevent repeated
    (failed) search.

    Reviewed-by: Ian Rogers <irogers@google.com>
    Signed-off-by: Namhyung Kim <namhyung@kernel.org>
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

 tools/perf/util/dso.c | 138 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------
 tools/perf/util/dso.h |  13 +++++++++++++
 2 files changed, 139 insertions(+), 12 deletions(-)
⬢ [acme@tb-phoenix2-perf perf-tools-next]$