Re: [PATCH v5 22/26] perf annotate-arm64: Support 'mov' instruction tracking

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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Destination register's DWARF lifetime state is improperly preserved during register-to-register 'mov' instruction tracking.
--

commit e7b1d4e72b07c94aacdc6e7221ef41f56b7b017c
Author: Tengda Wu [off-list ref]

perf annotate-arm64: Support 'mov' instruction tracking

Extend update_insn_state_arm64() to support register-to-register and
immediate-to-register 'mov' instructions. For register-to-register 'mov',
it propagates data type information from the source to the destination.

quoted

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

[ ... ]

quoted

@@ -810,6 +812,58 @@ static void update_store_insn_state(struct type_state *state,
 	adjust_reg_index_state(state, dloc, dl, dst);
 }
 
+static void update_mov_insn_state(struct type_state *state,
+				  struct disasm_line *dl,
+				  struct annotated_op_loc *src,
+				  struct annotated_op_loc *dst)
+{
+	struct type_state_reg *tsr;
+	u32 insn_offset = dl->al.offset;
+	int sreg = src->reg1;
+	int dreg = dst->reg1;
+
+	if (!has_reg_type(state, dreg))
+		return;
+
+	tsr = &state->regs[dreg];
+	tsr->copied_from = -1;

[ ... ]

quoted

+	tsr->type = state->regs[sreg].type;
+	tsr->kind = state->regs[sreg].kind;
+	tsr->imm_value = state->regs[sreg].imm_value;
+	tsr->offset = state->regs[sreg].offset;
+	tsr->ok = state->regs[sreg].ok;
+	/*
+	 * lifetime_active and lifetime_end represent DWARF variable lifetime
+	 * ranges tied to the source register. Copying them risks incorrectly
+	 * invalidating the destination register after a call instruction,
+	 * so intentionally omit them.
+	 */

[Severity: Medium]
While avoiding copying the source's lifetime, does this leave the
destination register's existing lifetime values stale?

If the destination register (dreg) previously had an active DWARF lifetime
tracking state (lifetime_active = true), those fields are not cleared
here since invalidate_reg_state() isn't called for this valid register
transfer path.

Should lifetime_active be set to false and lifetime_end set to 0?

Otherwise, the destination register might falsely survive caller-saved
register invalidations during subsequent call instructions and corrupt
downstream type inference.

It seems that the scenario I was originally worried about in my comment, where
dreg is incorrectly invalidated, does not exist.

Consider the following three consecutive instructions:

  mov dreg, x0
  call xxxx             // x0 lifetime end
  ldr x1, [dreg, #16]

1. If dreg is a callee-saved register, such as x20, then it is not affected
   by the call instruction, and setting its lifetime to any value will not matter.
2. If dreg is a caller-saved register, such as x1, then it is affected by
   the call instruction, and its lifetime needs to be cleared.

In practice, case 2 is very rare and looks more like undefined behavior, so
only case 1 needs to be considered. In that case, staying consistent with x86
may be the better choice:

  tsr->lifetime_active = state->regs[src->reg1].lifetime_active;
  tsr->lifetime_end = state->regs[src->reg1].lifetime_end;

Given the above, I think we can safely copy the lifetime fields and drop the
original concern.

Thanks,
Tengda