Re: [PATCH v1 5/5] perf symbol: Ensure dso__set_is_64_bit() for libbfd

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf symbol: bfd_get_arch_size() always returns -1 in dso__load_bfd_symbols()
--

commit 6d3220ac244766831d73c524c0b8f5184af28a7b
Author: Namhyung Kim <namhyung@kernel.org>

perf symbol: Ensure dso__set_is_64_bit() for libbfd

This commit calls dso__set_is_64_bit() using bfd_get_arch_size() when
loading symbols via libbfd in dso__load_bfd_symbols().

> diff --git a/tools/perf/util/libbfd.c b/tools/perf/util/libbfd.c
> index a4dcb406c79e9..9cc6216ce3adb 100644
> --- a/tools/perf/util/libbfd.c
> +++ b/tools/perf/util/libbfd.c
> @@ -462,6 +462,7 @@ int dso__load_bfd_symbols(struct dso *dso, const char *debugfile)
>  	symbols__fixup_end(dso__symbols(dso), false);
>  	symbols__fixup_duplicate(dso__symbols(dso));
>  	dso__set_adjust_symbols(dso, true);
> +	dso__set_is_64_bit(dso, bfd_get_arch_size(abfd) == 64);

[Severity: Medium]
Will bfd_get_arch_size(abfd) always return -1 here?

Earlier in tools/perf/util/libbfd.c:dso__load_bfd_symbols(), ELF binaries
are rejected before reaching this point:

	if (bfd_get_flavour(abfd) == bfd_target_elf_flavour)
		goto out_close;

In GNU binutils libbfd, bfd_get_arch_size() only checks ELF targets and
returns -1 for any other BFD flavour:

	if (abfd->xvec->flavour == bfd_target_elf_flavour)
		return get_elf_backend_data (abfd)->s->arch_size;
	return -1;

Because abfd is guaranteed to be non-ELF here (such as a 64-bit PE/COFF
binary), bfd_get_arch_size(abfd) == 64 always evaluates to false and
unconditionally sets dso->is_64_bit to false.

Should this use bfd_arch_bits_per_address(abfd) == 64 instead so that the
address size is queried across all BFD flavours?

>  
>  	err = dso__has_symbols(dso);
>  out_free:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261018163741.850434-1-namhyung@kernel.org?part=5