Re: [PATCH 2/2] perf: disasm: prefer symsrc_filename for filename

[Adrian Hunter]

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@xxxxxxxxxx>
> ---
>  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;
>  }