Re: [PATCH v7 13/15] perf annotate-arm64: Support 'adrp' instruction to track global variables

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf: Stale DWARF lifetime metadata preserved in update_adrp_insn_state
--

commit e2010b4bef31520f5bbfd7a86530f7efd90eacaa
Author: Tengda Wu <wutengda@huaweicloud.com>

perf annotate-arm64: Support 'adrp' instruction to track global variables

Extend update_insn_state_arm64() to track global variable types calculated
via page-relative addressing by introducing TSR_KIND_GLOBAL_ADDR.

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

[ ... ]

> @@ -945,6 +974,39 @@ static void update_add_insn_state(struct type_state *state,
>  	invalidate_reg_state(tsr);
>  }
>  
> +static void update_adrp_insn_state(struct type_state *state,
> +				   struct disasm_line *dl,
> +				   struct annotated_op_loc *dst)
> +{
> +	struct type_state_reg *tsr;
> +	u32 insn_offset = dl->al.offset;
> +	int dreg = dst->reg1;
> +
> +	if (!has_reg_type(state, dreg))
> +		return;
> +
> +	tsr = &state->regs[dreg];
> +	tsr->copied_from = -1;
> +
> +	if (!dl->ops.source.addr) {
> +		invalidate_reg_state(tsr);
> +		return;
> +	}
> +
> +	tsr->kind = TSR_KIND_GLOBAL_ADDR;
> +	/*
> +	 * Stores a partial page-relative address. The full absolute address
> +	 * of the global variable will be resolved when a subsequent 'add' or
> +	 * 'ldr' instruction consumes this register.
> +	 */
> +	tsr->imm_value = dl->ops.source.addr;
> +	tsr->offset = 0;
> +	tsr->ok = true;

[Severity: Medium]
When assigning the new state to the target register, does this code leak
stale DWARF lifetime metadata?

If the register was previously tracking a local variable with an active DWARF
lifetime, the new global address state inherits this stale metadata because
lifetime_active and lifetime_end are not cleared.

When a subsequent function call instruction is encountered, caller-saved
registers are incorrectly preserved if the stale metadata evaluates to true for
reg->lifetime_active and call_addr < reg->lifetime_end. This can poison type
propagation if the register is accidentally reused in subsequent instructions.

Should invalidate_reg_state() be called before assigning the new state, or
should the lifetime fields be manually cleared?

> +
> +	pr_debug_dtp("adrp [%x] global addr=%#"PRIx64" -> reg%d\n",
> +		     insn_offset, tsr->imm_value, dreg);
> +}
> +

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