Re: [PATCH v3 09/21] perf annotate-arm64: Implement extract_op_location() callback

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Post-index memory references with register offsets are incorrectly classified as SIGNED_OFFSET.
--

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

perf annotate-arm64: Implement extract_op_location() callback

Implement the extract_op_location() callback for the arm64 architecture
to handle its specific assembly syntax and addressing modes, enabling data
type profiling for this architecture.

> diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
> index 451500def8c12..77b6f4841c849 100644
> --- a/tools/perf/util/annotate-arch/annotate-arm64.c
> +++ b/tools/perf/util/annotate-arch/annotate-arm64.c
[ ... ]
> +static int extract_op_location_arm64(const struct arch *arch,
> +				     struct disasm_line *dl __maybe_unused,
> +				     const char *op_str, int op_idx __maybe_unused,
> +				     struct annotated_op_loc *op_loc)
> +{
> +	const char *s = op_str;
> +	char *p = NULL;
> +
> +	if (op_str == NULL)
> +		return 0;
[ ... ]
> +	/*
> +	 * Handle memory references (e.g., [x0, #8]), identify
> +	 * arm64 specific addressing modes
> +	 */
> +	if (*s == arch->objdump.memory_ref_char) {
> +		op_loc->mem_ref = true;
> +
> +		p = strchr(s, ']');
> +		if (p == NULL)
> +			return -1;
> +
> +		/* Pre-index: [base, #imm]! */
> +		if (p[1] == '!')
> +			op_loc->addr_mode = PERF_ADDR_MODE_PRE_INDEX;
> +		/* Post-index: [base], #imm */
> +		else if (p[1] == ',' && strchr(p + 1, arch->objdump.imm_char))
> +			op_loc->addr_mode = PERF_ADDR_MODE_POST_INDEX;

[Severity: Medium]
Does this incorrectly restrict post-index addressing classification to only
immediate offsets?

In ARM64, a comma immediately following the closing bracket (p[1] == ',') is
sufficient to identify a post-index addressing mode, regardless of whether
the offset is an immediate or a register (for example, Advanced SIMD
instructions like ld1 {v0.16b}, [x2], x5).

If the condition fails for register offsets, it appears it will incorrectly
fall through to PERF_ADDR_MODE_SIGNED_OFFSET. Could this cause the base
register write-back to be silently ignored in adjust_reg_index_state(),
leading to stale and corrupted type state tracking for the base register?

> +		/* Signed offset: [base{, #imm}] */
> +		else
> +			op_loc->addr_mode = PERF_ADDR_MODE_SIGNED_OFFSET;
> +
> +		s++;
> +	}

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