Re: [PATCH v5 13/26] perf annotate-arm64: Enable instruction tracking support

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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incomplete initialization of ARM64 caller-saved registers in init_type_state() misses x30 (Link Register) and x18. According to the ARM64 AAPCS64 ABI, x30 is intrinsically caller-saved (it is clobbered by function calls to store the return address), and x18 is also a temporary/caller-saved register (unless strictly reserved by a platform ABI). Because these registers are omitted during initialization, their `caller_saved` flags remain `false`. Consequently, their data type states are not invalidated across function calls, allowing stale type info to erroneously propagate.
--

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

perf annotate-arm64: Enable instruction tracking support

Enable basic instruction tracking for arm64 by implementing three
essential functions in the find_data_type_block() call path. This
allows arm64 to support basic variable type inference during
instruction tracking.

quoted

diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
index d8a5904359b1a..8fda4d4d68887 100644
--- a/tools/perf/util/annotate-arch/annotate-arm64.c
+++ b/tools/perf/util/annotate-arch/annotate-arm64.c

[ ... ]

quoted

@@ -442,6 +446,77 @@ static void extract_op_location_arm64(const struct arch *arch,

[ ... ]

quoted

+	if (ins__is_call(&dl->ins)) {
+		struct symbol *func = dl->ops.target.sym;
+		const char *call_name;
+		u64 call_addr;
+
+		call_name = func ? func->name : dl->ops.target.name;
+		pr_debug_dtp("call [%x] %s\n", insn_offset, call_name ?: "<unknown>");
+
+		/* Invalidate caller-saved registers after call */
+		call_addr = map__rip_2objdump(dloc->ms->map,
+					      dloc->ms->sym->start + dl->al.offset);
+		for (unsigned int i = 0; i < ARRAY_SIZE(state->regs); i++) {
+			struct type_state_reg *reg = &state->regs[i];
+
+			if (!reg->caller_saved)
+				continue;

[Severity: Medium]
Because x30 and x18 are omitted from being marked as caller-saved during type
state initialization, their caller_saved flags will be false.

Are these registers unintentionally skipped here, preventing them from being
invalidated across function calls?

quoted

+			/* Keep register valid within DWARF location lifetime */
+			if (reg->lifetime_active && call_addr < reg->lifetime_end)
+				continue;
+			invalidate_reg_state(reg);
+		}
+		return;
+	}

[ ... ]

quoted

diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index 104b80d471f1b..c51b20a7af9b6 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c

[ ... ]

quoted

@@ -178,6 +179,13 @@ static void init_type_state(struct type_state *state, const struct arch *arch)
 		state->regs[11].caller_saved = true;
 		state->ret_reg = 0;
 		state->stack_reg = X86_REG_SP;
+	} else if (arch__is_arm64(arch)) {
+		int i;
+
+		for (i = 0; i < 18; i++)
+			state->regs[i].caller_saved = true;

[Severity: Medium]
According to the ARM64 AAPCS64 ABI, x30 (Link Register) is intrinsically
caller-saved since it is clobbered by function calls to store the return
address.

x18 is also typically a temporary/caller-saved register unless strictly reserved
by a platform ABI.

This loop halts at 17, failing to set the caller_saved flag for x18 and x30.

Could this omission allow stale data type information for these registers to
erroneously propagate across function calls?

x18 does need to be set as caller-saved, but x30 is not really necessary, because
it generally serves as LR and is not used to carry data type information.
So I'm going to add only x18.

Thanks,
Tengda