Re: [PATCH v3 02/21] perf capstone: Fix arm64 jump/adrp disassembly mismatch with objdump

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Symbol resolution fails for relocated kernels (KASLR) and user-space shared libraries due to mismatched address spaces.
- [Low] Build failure due to undeclared `machine` variable and undefined `machine__normalized_is()` function in `capstone_init()`.
- [Low] Missing `<stdlib.h>` inclusion causes musl libc build failure.
- [Low] Usage of `ARM64_GRP_BRANCH_RELATIVE` causes build failures with Capstone versions prior to 5.0.
--

commit 1f9744f04e456b1209677658edb7f30a5b26e56d
Author: Tengda Wu <wutengda@huaweicloud.com>

perf capstone: Fix arm64 jump/adrp disassembly mismatch with objdump

The patch implements extended formatting for arm64 jump and adrp instructions
during symbol__disassemble_capstone() to match objdump's expected output style.
It adds instruction detail resolution in capstone_init() and formats the
output with correct symbol and offset mappings.

> diff --git a/tools/perf/util/capstone.c b/tools/perf/util/capstone.c
> index 9bba78ee0c5a2..a9c0a1d2eb036 100644
> --- a/tools/perf/util/capstone.c
> +++ b/tools/perf/util/capstone.c

[ ... ]

> @@ -218,6 +218,12 @@ static int capstone_init(uint16_t e_machine, csh *cs_handle, bool is64, bool is_
>  		 * on x86 by investigating instruction details.
>  		 */
>  		perf_cs_option(*cs_handle, CS_OPT_DETAIL, CS_OPT_ON);
> +	} else if (machine__normalized_is(machine, "arm64")) {

[Severity: Low]
Does this code cause a build failure? The variable machine is not declared
in capstone_init(), and the function machine__normalized_is() does not appear
to be defined in the codebase.

> +		/*
> +		 * Same as x86: arm64 needs instruction details to resolve
> +		 * symbolic addresses
> +		 */
> +		perf_cs_option(*cs_handle, CS_OPT_DETAIL, CS_OPT_ON);
>  	}
>  
>  	return 0;

[ ... ]

> +static bool is_pc_relative_insn(struct cs_insn *insn)
> +{
> +	int i;
> +
> +	if (insn->id == ARM64_INS_ADR || insn->id == ARM64_INS_ADRP)
> +		return true;
> +
> +	if (insn->detail == NULL)
> +		return false;
> +
> +	for (i = 0; i < insn->detail->groups_count; i++) {
> +		if (insn->detail->groups[i] == ARM64_GRP_JUMP ||
> +		    insn->detail->groups[i] == ARM64_GRP_CALL ||
> +		    insn->detail->groups[i] == ARM64_GRP_BRANCH_RELATIVE)

[Severity: Low]
Will this break compilation against Capstone versions prior to 5.0?
ARM64_GRP_BRANCH_RELATIVE was introduced in Capstone 5.0. Since the perf
tool's build feature detection doesn't mandate a specific Capstone version,
relying on this enum might cause build failures on older setups.

> +			return true;
> +	}
> +
> +	return false;
> +}
> +
> +static void format_capstone_insn_arm64(struct cs_insn *insn, char *buf,
> +				       size_t len, struct annotate_args *args,
> +				       u64 addr)
> +{

[ ... ]

> +	if (is_pc_relative_insn(insn)) {
> +		/* Extract last immediate value as address */
> +		last_imm = strrchr(buf, '#');
> +		if (!last_imm)
> +			return;
> +
> +		orig_addr = strtoull(last_imm + 1, &endptr, 16);

[Severity: Low]
Can this cause a build failure on musl libc? The POSIX standard function
strtoull() is used here, but stdlib.h is not explicitly included in the
file. Relying on implicit inclusions often works on glibc but breaks on
musl libc.

> +		if (endptr == last_imm + 1)
> +			return;
> +
> +		/* Relocate map that contains the address */
> +		if (dso__kernel(map__dso(map))) {
> +			found_map = maps__find(map__kmaps(map), orig_addr);
> +			if (found_map == NULL)
> +				return;
> +			map = found_map;
> +		}
> +
> +		/* Convert it to map-relative address for search */
> +		addr = map__map_ip(map, orig_addr);

[Severity: High]
Does this code fail to resolve symbols for relocated kernels (KASLR) or
user-space shared libraries (PIE)?

The extracted orig_addr is in the objdump address space. However,
maps__find() and map__map_ip() expect absolute memory addresses. By omitting
the translation step via map__objdump_2mem(map, orig_addr) (which the x86
implementation handles correctly), KASLR relocated kernels will fail in
maps__find(), and user-space shared libraries will underflow in
map__map_ip().

> +
> +		sym = map__find_symbol(map, addr);
> +		if (sym == NULL) {
> +			map__put(found_map);
> +			return;
> +		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701035355.752944-1-wutengda@huaweicloud.com?part=2