[PATCH v3 05/10] perf annotate: Save pointer offset in stack state

Zecheng Li posted 10 patches 4 months, 3 weeks ago
There is a newer version of this series
[PATCH v3 05/10] perf annotate: Save pointer offset in stack state
Posted by Zecheng Li 4 months, 3 weeks ago
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>
---
 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
Re: [PATCH v3 05/10] perf annotate: Save pointer offset in stack state
Posted by Namhyung Kim 4 months, 1 week ago
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
>