Re: [PATCH v4 15/23] perf annotate-arm64: Support store instruction tracking

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] For post-index addressing modes using a register offset (e.g., `ldr x0, [x1], x2` or `str x1, [x0], x2`), the base register's type state is incorrectly preserved instead of being invalidated.
--

--- Patch [15]: [PATCH v4 15/23] perf annotate-arm64: Support store instruction tracking ---
commit 5069bd6d8cc957f98ba504ae076e8ee35699d61a
Author: Tengda Wu <wutengda@huaweicloud.com>

perf annotate-arm64: Support store instruction tracking

This commit extends the arm64 update_insn_state() function to handle
store (STR) instructions. It ignores the transfer itself since it won't
change the type, but calls adjust_reg_index_state() to handle addressing
modes for the destination register.

> diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
> --- a/tools/perf/util/annotate-arch/annotate-arm64.c
> +++ b/tools/perf/util/annotate-arch/annotate-arm64.c

[ ... ]

> @@ -575,6 +575,17 @@ static void update_insn_state_arm64(struct type_state *state,
>  	/* Memory to register transfers */
>  	if (!strncmp(dl->ins.name, "ld", 2))
>  		update_load_insn_state(state, dl, src, dst);
> +	/* Register to memory transfers */
> +	else if (!strncmp(dl->ins.name, "st", 2)) {
> +		/*
> +		 * Ignore transfers since it'd set a value in a struct
> +		 * and won't change the type.
> +		 *
> +		 * Needs to update the pre-index and post-index addressing
> +		 * modes for the destination register.
> +		 */
> +		adjust_reg_index_state(state, dst, "str", insn_offset);

[Severity: Medium]
When using post-index addressing modes with a register offset (e.g., 
"str x1, [x0], x2"), does adjust_reg_index_state() correctly invalidate 
the base register's state?

Looking at adjust_reg_index_state() in the same file, when op_loc->multi_regs 
is true (indicating a dynamic register offset), op_loc->offset is 0:

tools/perf/util/annotate-arch/annotate-arm64.c:adjust_reg_index_state() {
    ...
	tsr->offset = op_loc->offset + tsr->offset;
    ...
}

Since op_loc->offset is 0 in this case, this seems to retain the old static
offset instead of clearing the state via invalidate_reg_state().

Can this cause incorrect struct field attribution in the perf annotate 
--data-type output for subsequent accesses using the modified base register?

> +	}
>  }
>  #endif

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