[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aN9kjm4cLV6mp8Ep@google.com>
Date: Fri, 3 Oct 2025 14:52:14 +0900
From: Namhyung Kim <namhyung@...nel.org>
To: Zecheng Li <zecheng@...gle.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
Adrian Hunter <adrian.hunter@...el.com>,
"Liang, Kan" <kan.liang@...ux.intel.com>,
Masami Hiramatsu <mhiramat@...nel.org>,
Xu Liu <xliuprof@...gle.com>, linux-perf-users@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 03/10] perf annotate: Track address registers via
TSR_KIND_POINTER
On Wed, Sep 17, 2025 at 07:58:01PM +0000, Zecheng Li wrote:
> Introduce TSR_KIND_POINTER to improve the data type profiler's ability
> to track pointer-based memory accesses and address register variables.
>
> TSR_KIND_POINTER represents a register that holds the address of the
> type in the `type_state_reg`. The semantics match the `breg` registers
> that describe a memory location.
>
> This change implements handling for this new kind in mov instructions
> and in the check_matching_type() function. When a TSR_KIND_POINTER is
> moved to the stack, the stack state size is set to the architecture's
> pointer size.
>
> Signed-off-by: Zecheng Li <zecheng@...gle.com>
> ---
> tools/perf/arch/x86/annotate/instructions.c | 19 +++++++-
> tools/perf/util/annotate-data.c | 52 +++++++++++++++++++--
> tools/perf/util/annotate-data.h | 1 +
> 3 files changed, 66 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/arch/x86/annotate/instructions.c b/tools/perf/arch/x86/annotate/instructions.c
> index da98a4e3c52c..698cbb299c6d 100644
> --- a/tools/perf/arch/x86/annotate/instructions.c
> +++ b/tools/perf/arch/x86/annotate/instructions.c
> @@ -391,7 +391,7 @@ static void update_insn_state_x86(struct type_state *state,
> tsr->ok = true;
>
> /* To copy back the variable type later (hopefully) */
> - if (tsr->kind == TSR_KIND_TYPE)
> + if (tsr->kind == TSR_KIND_TYPE || tsr->kind == TSR_KIND_POINTER)
> tsr->copied_from = src->reg1;
>
> pr_debug_dtp("mov [%x] reg%d -> reg%d",
> @@ -418,6 +418,10 @@ static void update_insn_state_x86(struct type_state *state,
> if (stack == NULL) {
> tsr->ok = false;
> return;
> + } else if (stack->kind == TSR_KIND_POINTER) {
> + tsr->type = stack->type;
> + tsr->kind = stack->kind;
> + tsr->ok = true;
> } else if (!stack->compound) {
Looks like you can reues the !stack->compound block below. But you need
to update set_stack_state() not to set it for POINTER types.
> tsr->type = stack->type;
> tsr->kind = stack->kind;
> @@ -455,6 +459,19 @@ static void update_insn_state_x86(struct type_state *state,
> insn_offset, src->offset, sreg, dst->reg1);
> pr_debug_type_name(&tsr->type, tsr->kind);
> }
> + /* Handle dereference of TSR_KIND_POINTER registers */
> + else if (has_reg_type(state, sreg) && state->regs[sreg].ok &&
> + state->regs[sreg].kind == TSR_KIND_POINTER &&
> + die_get_member_type(&state->regs[sreg].type,
> + src->offset, &type_die)) {
> + tsr->type = state->regs[sreg].type;
> + tsr->kind = TSR_KIND_TYPE;
> + tsr->ok = true;
> +
> + pr_debug_dtp("mov [%x] addr %#x(reg%d) -> reg%d",
> + insn_offset, src->offset, sreg, dst->reg1);
> + pr_debug_type_name(&tsr->type, tsr->kind);
> + }
> /* Or check if it's a global variable */
> else if (sreg == DWARF_REG_PC) {
> struct map_symbol *ms = dloc->ms;
> diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> index 903027a6fb7d..31b5896276f1 100644
> --- a/tools/perf/util/annotate-data.c
> +++ b/tools/perf/util/annotate-data.c
> @@ -59,6 +59,10 @@ void pr_debug_type_name(Dwarf_Die *die, enum type_state_kind kind)
> pr_info(" constant\n");
> return;
> case TSR_KIND_PERCPU_POINTER:
> + pr_info(" percpu pointer");
> + /* it also prints the type info */
> + break;
> + case TSR_KIND_POINTER:
> pr_info(" pointer");
> /* it also prints the type info */
> break;
> @@ -578,7 +582,9 @@ void set_stack_state(struct type_state_stack *stack, int offset, u8 kind,
> int tag;
> Dwarf_Word size;
>
> - if (dwarf_aggregate_size(type_die, &size) < 0)
> + if (kind == TSR_KIND_POINTER)
> + size = 8;
Maybe better to use 'sizeof(void *)'. Later, we may support different
architectures with different pointer size, but that would need many more
work, I guess. :)
Thanks,
Namhyung
> + else if (dwarf_aggregate_size(type_die, &size) < 0)
> size = 0;
>
> tag = dwarf_tag(type_die);
> @@ -898,13 +904,25 @@ static void update_var_state(struct type_state *state, struct data_loc_info *dlo
>
> reg = &state->regs[var->reg];
>
> - /* For gp registers, skip the address registers for now */
> - if (var->is_reg_var_addr)
> + if (reg->ok && reg->kind == TSR_KIND_TYPE &&
> + (!is_better_type(®->type, &mem_die) || var->is_reg_var_addr))
> continue;
>
> - if (reg->ok && reg->kind == TSR_KIND_TYPE &&
> - !is_better_type(®->type, &mem_die))
> + /* Handle address registers with TSR_KIND_POINTER */
> + if (var->is_reg_var_addr) {
> + if (reg->ok && reg->kind == TSR_KIND_POINTER &&
> + !is_better_type(®->type, &mem_die))
> + continue;
> +
> + reg->type = mem_die;
> + reg->kind = TSR_KIND_POINTER;
> + reg->ok = true;
> +
> + pr_debug_dtp("var [%"PRIx64"] reg%d addr offset %x",
> + insn_offset, var->reg, var->offset);
> + pr_debug_type_name(&mem_die, TSR_KIND_POINTER);
> continue;
> + }
>
> orig_type = reg->type;
>
> @@ -1116,6 +1134,30 @@ static enum type_match_result check_matching_type(struct type_state *state,
> return PERF_TMR_OK;
> }
>
> + if (state->regs[reg].kind == TSR_KIND_POINTER) {
> + struct strbuf sb;
> +
> + strbuf_init(&sb, 32);
> + die_get_typename_from_type(&state->regs[reg].type, &sb);
> + pr_debug_dtp("(ptr->%s)", sb.buf);
> + strbuf_release(&sb);
> +
> + /*
> + * Register holds a pointer (address) to the target variable.
> + * The type is the type of the variable it points to.
> + */
> + *type_die = state->regs[reg].type;
> +
> + dloc->type_offset = dloc->op->offset;
> +
> + /* Get the size of the actual type */
> + if (dwarf_aggregate_size(type_die, &size) < 0 ||
> + (unsigned)dloc->type_offset >= size)
> + return PERF_TMR_BAD_OFFSET;
> +
> + return PERF_TMR_OK;
> + }
> +
> if (state->regs[reg].kind == TSR_KIND_PERCPU_POINTER) {
> pr_debug_dtp("percpu ptr");
>
> diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h
> index dd3807b55208..fd0d1084bc4e 100644
> --- a/tools/perf/util/annotate-data.h
> +++ b/tools/perf/util/annotate-data.h
> @@ -35,6 +35,7 @@ enum type_state_kind {
> TSR_KIND_PERCPU_BASE,
> TSR_KIND_CONST,
> TSR_KIND_PERCPU_POINTER,
> + TSR_KIND_POINTER,
> TSR_KIND_CANARY,
> };
>
> --
> 2.51.0.384.g4c02a37b29-goog
>
Powered by blists - more mailing lists