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

[sashiko-bot@kernel.org]

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 <wutengda@huaweicloud.com>

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.

> 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

[ ... ]

> @@ -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;

[ ... ]

> +	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.

> +
> +	if (tsr->kind == TSR_KIND_TYPE || tsr->kind == TSR_KIND_POINTER)
> +		tsr->copied_from = sreg;
> +
> +	pr_debug_dtp("mov [%x] reg%d -> reg%d", insn_offset, sreg, dreg);
> +	pr_debug_type_name(&tsr->type, tsr->kind);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1788872630.git.wutengda@huaweicloud.com?part=22