linux-kernel - Re: [PATCH 2/2] perf: disasm: prefer symsrc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <395cfff7-9692-4123-96b6-353752007f46@intel.com>
Date: Thu, 13 Jun 2024 11:15:28 +0300
From: Adrian Hunter <adrian.hunter@...el.com>
To: Changbin Du <changbin.du@...wei.com>,
 Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
 Arnaldo Carvalho de Melo <acme@...nel.org>,
 Namhyung Kim <namhyung@...nel.org>, Nathan Chancellor <nathan@...nel.org>
Cc: Mark Rutland <mark.rutland@....com>,
 Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
 Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
 "Liang, Kan" <kan.liang@...ux.intel.com>,
 Nick Desaulniers <ndesaulniers@...gle.com>, Bill Wendling
 <morbo@...gle.com>, Justin Stitt <justinstitt@...gle.com>,
 linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
 llvm@...ts.linux.dev
Subject: Re: [PATCH 2/2] perf: disasm: prefer symsrc_filename for filename

On 13/06/24 09:35, Changbin Du wrote:
> If we already found a debugging version when loading symbols for that dso,
> then use the same file for disasm instead of looking up in buildid-cache.

In the past, there have been cases where the debugging version has not
worked for reading object code.  I don't remember the details, but the
symbols and debugging information was OK while the object code was not.

In general, using anything other than the file that was actually executed
for reading object code seems like a bad idea.

> 
> Signed-off-by: Changbin Du <changbin.du@...wei.com>
> ---
>  tools/perf/util/disasm.c |  5 +++++
>  tools/perf/util/symbol.c | 12 ++++++++----
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
> index 72aec8f61b94..75cfc6fbd11d 100644
> --- a/tools/perf/util/disasm.c
> +++ b/tools/perf/util/disasm.c
> @@ -1103,6 +1103,11 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
>  	    !dso__is_kcore(dso))
>  		return SYMBOL_ANNOTATE_ERRNO__NO_VMLINUX;
>  
> +	if (dso__symsrc_filename(dso)) {
> +		strlcpy(filename, dso__symsrc_filename(dso), filename_size);
> +		return 0;
> +	}
> +
>  	build_id_filename = dso__build_id_filename(dso, NULL, 0, false);
>  	if (build_id_filename) {
>  		__symbol__join_symfs(filename, filename_size, build_id_filename);
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 8d040039a7ce..a90c647d37e1 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -2025,12 +2025,14 @@ int dso__load_vmlinux(struct dso *dso, struct map *map,
>  		dso__set_binary_type(dso, DSO_BINARY_TYPE__VMLINUX);
>  
>  	err = dso__load_sym(dso, map, &ss, &ss, 0);
> -	symsrc__destroy(&ss);
> -
>  	if (err > 0) {
>  		dso__set_loaded(dso);
>  		pr_debug("Using %s for symbols\n", symfs_vmlinux);
> +
> +		if (symsrc__has_symtab(&ss) && !dso__symsrc_filename(dso))
> +			dso__set_symsrc_filename(dso, strdup(symfs_vmlinux));
>  	}
> +	symsrc__destroy(&ss);
>  
>  	return err;
>  }
> @@ -2395,12 +2397,14 @@ static int dso__load_vdso(struct dso *dso, struct map *map,
>  	dso__set_binary_type(dso, DSO_BINARY_TYPE__SYSTEM_PATH_DSO);
>  
>  	err = dso__load_sym(dso, map, &ss, &ss, 0);
> -	symsrc__destroy(&ss);
> -
>  	if (err > 0) {
>  		dso__set_loaded(dso);
>  		pr_debug("Using %s for %s symbols\n", symfs_vdso, dso__short_name(dso));
> +
> +		if (symsrc__has_symtab(&ss) && !dso__symsrc_filename(dso))
> +			dso__set_symsrc_filename(dso, strdup(symfs_vdso));
>  	}
> +	symsrc__destroy(&ss);
>  
>  	return err;
>  }