Re: [PATCH v4 20/23] perf annotate-arm64: Support 'add' instruction tracking

On 8/8/26 8:23 PM, Tengda Wu wrote:
> Extend update_insn_state() for arm64 to track 'add' instructions for
> structure member address calculation, which commonly appear as:
> 
>    add  dreg, base, #offset
>    add  dreg, base, reg2     (reg2 holds a constant)
> 
> Unlike x86, the arm64 'add' instruction has an extra base register among
> its source operands. Therefore, in terms of propagating the data type,
> it is essentially performing a 'mov', except that before the 'mov', it
> first needs to be updated by adding the offset or reg2.
> 
> A real-world example is shown below:
> 
>    ffff80008001c9a8 <flush_ptrace_hw_breakpoint>:
>    ffff80008001c9c4:  add  x19, x0, #0xeb8   // x0 (task_struct*) + 0xeb8 -> x19
> * ffff80008001c9d0:  ldr  x0, [x19]
> 
> Before this commit, the type flow broke at the 'add' instruction,
> leaving the subsequent load with no type information:
> 
>    chk [28] reg19 offset=0 ok=0 kind=0 cfa : no type information
>    final result: no type information
> 
> After this commit, the tracker correctly follows the member address
> calculation:
> 
>    var [0] reg0 offset 0 type='struct task_struct*'
>    add [1c] address of 0xeb8(reg0) -> reg19 type='struct task_struct*'
>    chk [28] reg19 offset=0 ok=1 kind=1 (struct task_struct*) : Good!
>    found by insn track: 0(reg19) type-offset=0xeb8
>    final result: type='struct task_struct'
> 
> Signed-off-by: Tengda Wu <[email protected]>
> ---
>   .../perf/util/annotate-arch/annotate-arm64.c  | 87 ++++++++++++++++++-
>   1 file changed, 85 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
> index 7b780bad8c07..eaeb4433fc3a 100644
> --- a/tools/perf/util/annotate-arch/annotate-arm64.c
> +++ b/tools/perf/util/annotate-arch/annotate-arm64.c
> @@ -688,6 +688,87 @@ static void update_mov_insn_state(struct type_state *state,
>   	pr_debug_type_name(&tsr->type, tsr->kind);
>   }
>   
> +static void update_add_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;
> +	struct type_state_reg src_tsr;
> +	u32 insn_offset = dl->al.offset;
> +	int sreg = src->reg1;
> +	int dreg = dst->reg1;
> +	u64 imm_value;
> +
> +	if (!has_reg_type(state, dreg))
> +		return;
> +
> +	tsr = &state->regs[dreg];
> +	tsr->copied_from = -1;
> +
> +retry:
> +	if (!has_reg_type(state, sreg) || !state->regs[sreg].ok) {
> +		invalidate_reg_state(tsr);
> +		return;
> +	}
> +
> +	src_tsr = state->regs[sreg];
> +
> +	/*
> +	 * Handle 'add' instructions of the form:
> +	 *   add  dreg, base, #offset     (immediate offset)
> +	 *   add  dreg, base, reg2        (reg2 holds a constant)
> +	 *
> +	 * For case 2, retrieve the constant value from reg2
> +	 * and use it as the offset.
> +	 */
> +	imm_value = src->offset;
> +	if (src->multi_regs) {
> +		int reg2 = (sreg == src->reg1) ? src->reg2 : src->reg1;
> +
> +		if (!has_reg_type(state, reg2) || !state->regs[reg2].ok) {
> +			/* Unable to resolve type for dst, bail out */
> +			invalidate_reg_state(tsr);
> +			return;
> +		}
> +		if (state->regs[reg2].kind == TSR_KIND_CONST)
> +			imm_value = state->regs[reg2].imm_value;

First, when reg2 is valid but not TSR_KIND_CONST - say a pointer
freshly loaded from memory - the addend is unknown at analysis time,
yet imm_value silently stays 0 and propagation continues, tracking
the destination as base + 0. Note the asymmetry: no state at all
invalidates, but state that doesn't yield a value pretends the
addend is zero. Unknown addend should invalidate as well:

         if (state->regs[reg2].kind != TSR_KIND_CONST) {
             invalidate_reg_state(tsr);
             return;
         }

Second, the shifted/extended forms still break even with a constant
reg2: extract_op_location_arm64() drops the "lsl #3" (or uxtw/sxtw)
modifier, so add x0, x1, x2, lsl #3 contributes the unshifted value
and maps later accesses to the wrong field. Maybe flag such operands
during extraction (or fail the reg2 extraction) so the handler can
skip them.

> +	}
> +
> +	if (src_tsr.kind == TSR_KIND_CONST) {
> +		tsr->kind = src_tsr.kind;
> +		tsr->imm_value = src_tsr.imm_value + imm_value;
> +		tsr->offset = 0;
> +		tsr->ok = src_tsr.ok;
> +
> +		pr_debug_dtp("add [%x] imm %#"PRIx64"(reg%d) -> reg%d\n",
> +			     insn_offset, imm_value, sreg, dreg);

And since add is commutative, this early return drops the pointer
side for add rd, const_reg, ptr_reg: sreg starts as reg1, hits the
CONST case and returns before the goto retry at the bottom ever gets
a chance to evaluate the pointer register. The result should be the
pointer with the constant applied as an offset, not a plain constant.
Maybe check the other register for a pointer type before falling into
the CONST case (or move the CONST handling after the retry).

Thanks,
Shuai