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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).