Re: [PATCH v5 21/26] perf annotate-arm64: Support stack variable tracking

On 2026/9/8 21:36, sashiko-bot@kernel.org wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Invalid register comparison causes unparsed operands to be falsely treated as stack variables.
--

commit 7ef0e34126bb69c05756eba7378c5775c6ea2fd3
Author: Tengda Wu [off-list ref]

perf annotate-arm64: Support stack variable tracking

This commit extends update_insn_state_arm64 to track data types stored on the
stack. It handles stack loads and stores, introducing
propagate_store_reg_state to follow data types between registers and memory.

quoted

diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
index 9d43e2c2041cd..d520975c59363 100644
--- a/tools/perf/util/annotate-arch/annotate-arm64.c
+++ b/tools/perf/util/annotate-arch/annotate-arm64.c

@@ -542,12 +544,57 @@ static int propagate_load_reg_state(struct type_state *state,

[ ... ]

quoted

+	if (dloc->fb_cfa) {
+		u64 ip = dloc->ms->sym->start + dl->al.offset;
+		u64 pc = map__rip_2objdump(dloc->ms->map, ip);
+
+		if (die_get_cfa(dloc->di->dbg, pc, &fbreg, &fboff) < 0)
+			fbreg = -1;
+	}
+
 retry:
 	if (arch_get_reg_offset(dloc->arch, src, sreg, state, false, &reg_offset))
 		return -1;
 
 	reg_offset += mem_offset;
 
+	/* Check stack variables with offset */
+	if (sreg == fbreg || sreg == state->stack_reg) {

[Severity: Medium]
When an unparsed or unsupported base register leaves sreg as -1, and fbreg is
also -1, does this condition evaluate to true? Could this cause unparsed
source operands to be incorrectly treated as stack loads?

Agreed.

[ ... ]

quoted

+static void propagate_store_reg_state(struct type_state *state,
+				      struct data_loc_info *dloc,
+				      struct disasm_line *dl, int sreg,
+				      struct annotated_op_loc *dst,
+				      int mem_offset)
+{

[ ... ]

quoted

+	if (dloc->fb_cfa) {
+		u64 ip = dloc->ms->sym->start + dl->al.offset;
+		u64 pc = map__rip_2objdump(dloc->ms->map, ip);
+
+		if (die_get_cfa(dloc->di->dbg, pc, &fbreg, &fboff) < 0)
+			fbreg = -1;
+	}
+
+	/* Check stack variables with offset */
+	if (dreg == fbreg || dreg == state->stack_reg) {

[Severity: Medium]
Similarly to the load case, if the base register extraction leaves dreg as -1
and fbreg is also -1, does this check evaluate to true? Will this erroneously
treat unparsed target operands as stack stores, potentially corrupting the
stack type state tracking?

Agreed. It would be best to add a guard for when dreg is not -1.

Tengda