Re: perf script intel_pt decoding reading instructions from wrong shared lib

[Adrian Hunter <adrian.hunter@intel.com>]

On 15/04/2025 00:59, Todd Lipcon wrote:
> Hi folks,
> 
> It seems there may be a bug in perf-script when processing intel_pt.
> Specifically, in order to reconstruct the instructions, it maps
> instruction pointers back to DSO-relative addresses, and then reads
> from the associated ELF files to find the actual instructions that
> were traced.
> 
> In the case that the ELF file has an associated separate-debug ELF
> file linked via `.gnu-debuglink` (common in some Linux distros), it
> appears to be reading the debuginfo file instead of the shared library
> that actually contains the code.
> 
> I've posted a simple repro here:
> https://github.com/toddlipcon/perf-debuglink-bug/blob/main/repro.sh
> 
> I haven't dug into the code yet enough to see where it's going wrong,
> but figured I would report to the users list first.

Seems to be caused by commit 5363c306787c8 - see further below.
Reverting that makes the issue go away.

With that commit, dso__load() sets the binary_type to the first symbol
source file type, which is DSO_BINARY_TYPE__DEBUGLINK in this case.
That causes dso__get_filename() to get the file name of the debug file,
not the file actually executed.

Leaving the binary_type as DSO_BINARY_TYPE__NOT_FOUND will result in
try_to_open_dso() using either DSO_BINARY_TYPE__BUILD_ID_CACHE or
DSO_BINARY_TYPE__SYSTEM_PATH_DSO.

A quick workaround hack:

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index dd30f17cb1ca..09fd79ca1d48 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1825,7 +1825,10 @@ int dso__load(struct dso *dso, struct map *map)
 		if (next_slot) {
 			ss_pos++;
 
-			if (dso__binary_type(dso) == DSO_BINARY_TYPE__NOT_FOUND)
+			if (dso__binary_type(dso) == DSO_BINARY_TYPE__NOT_FOUND ||
+			    symtab_type == DSO_BINARY_TYPE__BUILD_ID_CACHE ||
+			    (symtab_type == DSO_BINARY_TYPE__SYSTEM_PATH_DSO &&
+			     dso__binary_type(dso) != DSO_BINARY_TYPE__BUILD_ID_CACHE))
 				dso__set_binary_type(dso, symtab_type);
 
 			if (syms_ss && runtime_ss)



commit 5363c306787c88d41a41493f81b4308643696f6e
Author: Namhyung Kim <namhyung@kernel.org>
Date:   Fri Apr 26 14:51:38 2024 -0700

    perf symbol: Set binary_type of dso when loading
    
    For the kernel dso, it sets the binary type of dso when loading the
    symbol table.  But it seems not to do that for user DSOs.  Actually
    it sets the symtab type only.  It's not clear why we want to maintain
    the two separately but it uses the binary type info before getting
    the disassembly.
    
    Let's use the symtab type as binary type too if it's not set.  I think
    it's ok to set the binary type when it founds a symsrc whether or not
    it has actual symbols.
    
    Signed-off-by: Namhyung Kim <namhyung@kernel.org>
    Tested-by: Alexander Monakov <amonakov@ispras.ru>
    Link: https://lore.kernel.org/r/20240426215139.1271039-1-namhyung@kernel.org
    Cc: Ian Rogers <irogers@google.com>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Cc: Adrian Hunter <adrian.hunter@intel.com>
    Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
    Cc: Jiri Olsa <jolsa@kernel.org>
    Cc: Ingo Molnar <mingo@kernel.org>
    Cc: Kan Liang <kan.liang@linux.intel.com>
    Cc: LKML <linux-kernel@vger.kernel.org>
    Cc:  <linux-perf-users@vger.kernel.org>
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index a18927d792af..3bbf173ad822 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1931,6 +1931,9 @@ int dso__load(struct dso *dso, struct map *map)
 		if (next_slot) {
 			ss_pos++;
 
+			if (dso__binary_type(dso) == DSO_BINARY_TYPE__NOT_FOUND)
+				dso__set_binary_type(dso, symtab_type);
+
 			if (syms_ss && runtime_ss)
 				break;
 		} else {