Re: [PATCH v6 7/8] perf: disasm: prefer debugging files in build-id cache

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

On 25/07/24 05:15, Changbin Du wrote:
> The build-id cache might have both debugging and non-debugging files. Here
> we prefer the debugging version for annotation.

As I pointed out before, disassembling a different file
from the one that actually executed can have pitfalls.

If you want this, it needs to be optional, not the default.

But if you take the approach to remove vdso from the buildid
cache, and add back the debug version, then this patch would
not be needed for vdso.

> 
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Signed-off-by: Changbin Du <changbin.du@huawei.com>
> ---
>  tools/perf/util/disasm.c | 29 ++++++++++++++++++-----------
>  1 file changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
> index 6af9fbec3a95..5f66b3632770 100644
> --- a/tools/perf/util/disasm.c
> +++ b/tools/perf/util/disasm.c
> @@ -1162,18 +1162,25 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
>  	    !dso__is_kcore(dso))
>  		return SYMBOL_ANNOTATE_ERRNO__NO_VMLINUX;
>  
> -	build_id_filename = dso__build_id_filename(dso, NULL, 0, false);
> -	if (build_id_filename) {
> -		__symbol__join_symfs(filename, filename_size, build_id_filename);
> -		free(build_id_filename);
> -	} else {
> -		if (dso__has_build_id(dso))
> -			return ENOMEM;
> -		return fallback_filename(dso, filename, filename_size);
> -	}
> +	/* Prefer debugging file if exists, otherwise non-debugging one is used. */
> +	for (int i = 0; i < 2; i++) {
> +		build_id_filename = dso__build_id_filename(dso, NULL, 0, !i);
> +		if (build_id_filename) {
> +			__symbol__join_symfs(filename, filename_size, build_id_filename);
> +			free(build_id_filename);
> +		} else {
> +			if (dso__has_build_id(dso))
> +				return ENOMEM;
> +			return fallback_filename(dso, filename, filename_size);
> +		}
>  
> -	if (access(filename, R_OK))
> -		return fallback_filename(dso, filename, filename_size);
> +		if (!access(filename, R_OK))
> +			break;
> +		else if (i != 0) {
> +			/* nor debugging or non-debugging is found */
> +			return fallback_filename(dso, filename, filename_size);
> +		}
> +	}
>  
>  	if (dso__is_kcore(dso) || dso__is_vdso(dso))
>  		goto fallback;