From: Namhyung Kim <namhyung@kernel.org>
To: Zecheng Li <zecheng@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
Adrian Hunter <adrian.hunter@intel.com>,
"Liang, Kan" <kan.liang@linux.intel.com>,
Masami Hiramatsu <mhiramat@kernel.org>,
Xu Liu <xliuprof@google.com>,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 05/10] perf annotate: Save pointer offset in stack state
Date: Sat, 4 Oct 2025 16:59:24 +0900 [thread overview]
Message-ID: <aODT3BM5055oxTio@google.com> (raw)
In-Reply-To: <20250917195808.2514277-6-zecheng@google.com>
On Wed, Sep 17, 2025 at 07:58:03PM +0000, Zecheng Li wrote:
> The tracked pointer offset was not being preserved in the stack state,
> which could lead to incorrect type analysis. This change adds a
> ptr_offset field to the type_state_stack struct and passes it to
> set_stack_state and findnew_stack_state to ensure the offset is
> preserved after the pointer is loaded from a stack location. It improves
> the type annotation coverage and quality.
>
> Signed-off-by: Zecheng Li <zecheng@google.com>
Reviewed-by: Namhyung Kim <namhyung@kernel.org>
Thanks,
Namhyung
> ---
> tools/perf/arch/x86/annotate/instructions.c | 8 ++++----
> tools/perf/util/annotate-data.c | 12 +++++++-----
> tools/perf/util/annotate-data.h | 7 +++++--
> 3 files changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/tools/perf/arch/x86/annotate/instructions.c b/tools/perf/arch/x86/annotate/instructions.c
> index cfb07cff8fc8..709c6f7efe82 100644
> --- a/tools/perf/arch/x86/annotate/instructions.c
> +++ b/tools/perf/arch/x86/annotate/instructions.c
> @@ -525,12 +525,12 @@ static void update_insn_state_x86(struct type_state *state,
> } else if (stack->kind == TSR_KIND_POINTER) {
> tsr->type = stack->type;
> tsr->kind = stack->kind;
> - tsr->offset = 0;
> + tsr->offset = stack->ptr_offset;
> tsr->ok = true;
> } else if (!stack->compound) {
> tsr->type = stack->type;
> tsr->kind = stack->kind;
> - tsr->offset = 0;
> + tsr->offset = stack->ptr_offset;
> tsr->ok = true;
> } else if (die_get_member_type(&stack->type,
> offset - stack->offset,
> @@ -713,10 +713,10 @@ static void update_insn_state_x86(struct type_state *state,
> */
> if (!stack->compound)
> set_stack_state(stack, offset, tsr->kind,
> - &tsr->type);
> + &tsr->type, tsr->offset);
> } else {
> findnew_stack_state(state, offset, tsr->kind,
> - &tsr->type);
> + &tsr->type, tsr->offset);
> }
>
> if (dst->reg1 == fbreg) {
> diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> index 6ca5489f3c4c..68c69d343bff 100644
> --- a/tools/perf/util/annotate-data.c
> +++ b/tools/perf/util/annotate-data.c
> @@ -577,7 +577,7 @@ struct type_state_stack *find_stack_state(struct type_state *state,
> }
>
> void set_stack_state(struct type_state_stack *stack, int offset, u8 kind,
> - Dwarf_Die *type_die)
> + Dwarf_Die *type_die, int ptr_offset)
> {
> int tag;
> Dwarf_Word size;
> @@ -592,6 +592,7 @@ void set_stack_state(struct type_state_stack *stack, int offset, u8 kind,
> stack->type = *type_die;
> stack->size = size;
> stack->offset = offset;
> + stack->ptr_offset = ptr_offset;
> stack->kind = kind;
>
> switch (tag) {
> @@ -607,18 +608,19 @@ void set_stack_state(struct type_state_stack *stack, int offset, u8 kind,
>
> struct type_state_stack *findnew_stack_state(struct type_state *state,
> int offset, u8 kind,
> - Dwarf_Die *type_die)
> + Dwarf_Die *type_die,
> + int ptr_offset)
> {
> struct type_state_stack *stack = find_stack_state(state, offset);
>
> if (stack) {
> - set_stack_state(stack, offset, kind, type_die);
> + set_stack_state(stack, offset, kind, type_die, ptr_offset);
> return stack;
> }
>
> stack = malloc(sizeof(*stack));
> if (stack) {
> - set_stack_state(stack, offset, kind, type_die);
> + set_stack_state(stack, offset, kind, type_die, ptr_offset);
> list_add(&stack->list, &state->stack_vars);
> }
> return stack;
> @@ -888,7 +890,7 @@ static void update_var_state(struct type_state *state, struct data_loc_info *dlo
> continue;
>
> findnew_stack_state(state, offset, TSR_KIND_TYPE,
> - &mem_die);
> + &mem_die, /*ptr_offset=*/0);
>
> if (var->reg == state->stack_reg) {
> pr_debug_dtp("var [%"PRIx64"] %#x(reg%d)",
> diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h
> index 20237e7e4e2f..e1e9c5f6915a 100644
> --- a/tools/perf/util/annotate-data.h
> +++ b/tools/perf/util/annotate-data.h
> @@ -191,6 +191,8 @@ struct type_state_stack {
> struct list_head list;
> Dwarf_Die type;
> int offset;
> + /* pointer offset, saves tsr->offset on the stack state */
> + int ptr_offset;
> int size;
> bool compound;
> u8 kind;
> @@ -244,9 +246,10 @@ int annotated_data_type__get_member_name(struct annotated_data_type *adt,
> bool has_reg_type(struct type_state *state, int reg);
> struct type_state_stack *findnew_stack_state(struct type_state *state,
> int offset, u8 kind,
> - Dwarf_Die *type_die);
> + Dwarf_Die *type_die,
> + int ptr_offset);
> void set_stack_state(struct type_state_stack *stack, int offset, u8 kind,
> - Dwarf_Die *type_die);
> + Dwarf_Die *type_die, int ptr_offset);
> struct type_state_stack *find_stack_state(struct type_state *state,
> int offset);
> bool get_global_var_type(Dwarf_Die *cu_die, struct data_loc_info *dloc,
> --
> 2.51.0.384.g4c02a37b29-goog
>
next prev parent reply other threads:[~2025-10-04 7:59 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-17 19:57 [PATCH v3 00/10] perf tools: Some improvements on data type profiler Zecheng Li
2025-09-17 19:57 ` [PATCH v3 01/10] perf annotate: Skip annotating data types to lea instructions Zecheng Li
2025-10-03 5:36 ` Namhyung Kim
2025-09-17 19:58 ` [PATCH v3 02/10] perf annotate: Rename TSR_KIND_POINTER to TSR_KIND_PERCPU_POINTER Zecheng Li
2025-10-03 5:37 ` Namhyung Kim
2025-10-03 19:10 ` Arnaldo Carvalho de Melo
2025-09-17 19:58 ` [PATCH v3 03/10] perf annotate: Track address registers via TSR_KIND_POINTER Zecheng Li
2025-10-03 5:52 ` Namhyung Kim
2025-09-17 19:58 ` [PATCH v3 04/10] perf annotate: Track arithmetic instructions on pointers Zecheng Li
2025-10-04 7:57 ` Namhyung Kim
2025-09-17 19:58 ` [PATCH v3 05/10] perf annotate: Save pointer offset in stack state Zecheng Li
2025-10-04 7:59 ` Namhyung Kim [this message]
2025-09-17 19:58 ` [PATCH v3 06/10] perf annotate: Invalidate register states for untracked instructions Zecheng Li
2025-10-04 8:04 ` Namhyung Kim
2025-09-17 19:58 ` [PATCH v3 07/10] perf dwarf-aux: Skip check_variable for die_find_variable_by_reg Zecheng Li
2025-09-17 19:58 ` [PATCH v3 08/10] perf dwarf-aux: Preserve typedefs in match_var_offset Zecheng Li
2025-09-17 19:58 ` [PATCH v3 09/10] perf annotate: Improve type comparison from different scopes Zecheng Li
2025-09-17 19:58 ` [PATCH v3 10/10] perf dwarf-aux: Support DW_OP_piece expressions Zecheng Li
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aODT3BM5055oxTio@google.com \
--to=namhyung@kernel.org \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=irogers@google.com \
--cc=jolsa@kernel.org \
--cc=kan.liang@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mhiramat@kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=xliuprof@google.com \
--cc=zecheng@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.