[PATCH v1 0/6] perf tools: Some improvements on data type profiler

[PATCH v1 0/6] perf tools: Some improvements on data type profiler

[Zecheng Li]

Hi all,

I've identified several missing data type annotations within the perf
tools when annotating the Linux kernel. This patch series improves the
coverage and correctness of data type annotations.

Here's a breakdown of the changes:

Patches 1 and 2 fixes direct type matching from DWARF. They fix cases
with intrusive linked lists (e.g, sched_balance_update_blocked_averages)
where type information was previously missed.

Patch 3 utilizes this to better determine the types of stack
variables for instruction tracking.

Patch 4 skips annotations for lea instructions, as these do not involve
memory access.

Patches 5 and 6 implement a basic idea for register offset tracking
based on arithmetic operations. While this feature has known limitations
and may regress in rare cases compared to the original, it generally
improves offset tracking in most scenarios.

Note: I will be communicating from a new email address, zli94@ncsu.edu,
as I will soon lose access to my current email account.

Thanks,
Zecheng

Zecheng Li (6):
  perf dwarf-aux: Use signed comparison in match_var_offset
  perf dwarf-aux: More accurate variable type match for breg
  perf dwarf-aux: Better type matching for stack variables
  perf annotate: Skip annotating data types to lea instructions
  perf dwarf-aux: Find pointer type to a type
  perf annotate: Track arithmetic instructions on pointers

 tools/perf/arch/x86/annotate/instructions.c | 109 ++++++++++++++++-
 tools/perf/util/annotate-data.c             |  14 ++-
 tools/perf/util/annotate-data.h             |   6 +
 tools/perf/util/annotate.c                  |   5 +
 tools/perf/util/dwarf-aux.c                 | 125 ++++++++++++++++++--
 tools/perf/util/dwarf-aux.h                 |   4 +
 6 files changed, 247 insertions(+), 16 deletions(-)

-- 
2.50.1.470.g6ba607880d-goog

[PATCH v1 1/6] perf dwarf-aux: Use signed comparison in match_var_offset

[Zecheng Li]

match_var_offset compares address offsets to determine if an access
falls within a variable's bounds. The offsets involved for those
relative to base registers from DW_OP_breg can be negative.

The current implementation uses unsigned types (u64) for these offsets,
which rejects almost all negative values.

This commit changes the local variables within match_var_offset to
signed types (s64) before performing comparisons. This ensures correct
behavior when addr_offset_ or addr_type_ are negative.

Signed-off-by: Zecheng Li <zecheng@google.com>
---
 tools/perf/util/dwarf-aux.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 559c953ca172..bf906dff9ef0 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -1388,10 +1388,12 @@ struct find_var_data {
 #define DWARF_OP_DIRECT_REGS  32
 
 static bool match_var_offset(Dwarf_Die *die_mem, struct find_var_data *data,
-			     u64 addr_offset, u64 addr_type, bool is_pointer)
+			     u64 addr_offset_, u64 addr_type_, bool is_pointer)
 {
 	Dwarf_Die type_die;
 	Dwarf_Word size;
+	s64 addr_offset = (s64)addr_offset_;
+	s64 addr_type = (s64)addr_type_;
 
 	if (addr_offset == addr_type) {
 		/* Update offset relative to the start of the variable */
@@ -1414,7 +1416,7 @@ static bool match_var_offset(Dwarf_Die *die_mem, struct find_var_data *data,
 	if (dwarf_aggregate_size(&type_die, &size) < 0)
 		return false;
 
-	if (addr_offset >= addr_type + size)
+	if (addr_offset_ - addr_type_ >= size)
 		return false;
 
 	/* Update offset relative to the start of the variable */
-- 
2.50.1.470.g6ba607880d-goog

[PATCH v1 2/6] perf dwarf-aux: More accurate variable type match for breg

[Zecheng Li]

Introduces the function is_breg_access_indirect to determine whether a
memory access involving a DW_OP_breg* operation refers to the variable's
value directly or requires dereferencing the variable's type as a
pointer based on the DWARF expression. Previously, all breg based
accesses were assumed to directly access the variable's value
(is_pointer = false).

The is_breg_access_indirect function handles three main cases:

1. Base register + offset only: (e.g., DW_OP_breg7 RSP+88) The
   calculated address is the location of the variable. The access is
   direct, so no type dereference is needed. Returns false.

2. Base register + offset, followed by other operations ending in
   DW_OP_stack_value, including DW_OP_deref: (e.g., DW_OP_breg7 RSP+96,
   DW_OP_deref, DW_OP_plus_uconst 0x64, DW_OP_stack_value) The DWARF
   expression computes the variable's value, but that value requires a
   dereference. The memory access is fetching that value, so no type
   dereference is needed. Returns false.

3. Base register + offset, followed only by DW_OP_stack_value: (e.g.,
   DW_OP_breg7 RSP+176, DW_OP_stack_value) This indicates the value at
   the base + offset is the variable's value. Since this value is being
   used as an address in the memory access, the variable's type is
   treated as a pointer and requires a type dereference. Returns true.

The is_pointer argument passed to match_var_offset is now set by
is_breg_access_indirect for breg accesses.

Signed-off-by: Zecheng Li <zecheng@google.com>
---
 tools/perf/util/dwarf-aux.c | 42 ++++++++++++++++++++++++++++++++-----
 1 file changed, 37 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index bf906dff9ef0..814c96ea509f 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -1424,6 +1424,38 @@ static bool match_var_offset(Dwarf_Die *die_mem, struct find_var_data *data,
 	return true;
 }
 
+/**
+ * is_breg_access_indirect - Check if breg based access implies type dereference
+ * @ops: DWARF operations array
+ * @nops: Number of operations in @ops
+ *
+ * Returns true if the DWARF expression indicates the variable's value is
+ * a pointer that the memory access dereferences.
+ * Returns false if the expression evaluates to the variable's value directly.
+ * This is called after check_allowed_ops.
+ */
+static bool is_breg_access_indirect(Dwarf_Op *ops, size_t nops)
+{
+	ops++;
+	nops--;
+
+	/* only the base register */
+	if (nops == 0)
+		return false;
+
+	switch (ops->atom) {
+	case DW_OP_stack_value:
+		return true;
+	case DW_OP_deref_size:
+	case DW_OP_deref:
+	case DW_OP_piece:
+		return false;
+	default:
+		/* unreachable, OP not supported */
+		return false;
+	}
+}
+
 /* Only checks direct child DIEs in the given scope. */
 static int __die_find_var_reg_cb(Dwarf_Die *die_mem, void *arg)
 {
@@ -1452,7 +1484,7 @@ static int __die_find_var_reg_cb(Dwarf_Die *die_mem, void *arg)
 		if (data->is_fbreg && ops->atom == DW_OP_fbreg &&
 		    check_allowed_ops(ops, nops) &&
 		    match_var_offset(die_mem, data, data->offset, ops->number,
-				     /*is_pointer=*/false))
+				     /*is_pointer=*/is_breg_access_indirect(ops, nops)))
 			return DIE_FIND_CB_END;
 
 		/* Only match with a simple case */
@@ -1464,11 +1496,11 @@ static int __die_find_var_reg_cb(Dwarf_Die *die_mem, void *arg)
 					     /*is_pointer=*/true))
 				return DIE_FIND_CB_END;
 
-			/* Local variables accessed by a register + offset */
+			/* variables accessed by a register + offset */
 			if (ops->atom == (DW_OP_breg0 + data->reg) &&
 			    check_allowed_ops(ops, nops) &&
 			    match_var_offset(die_mem, data, data->offset, ops->number,
-					     /*is_pointer=*/false))
+					     /*is_pointer=*/is_breg_access_indirect(ops, nops)))
 				return DIE_FIND_CB_END;
 		} else {
 			/* pointer variables saved in a register 32 or above */
@@ -1478,11 +1510,11 @@ static int __die_find_var_reg_cb(Dwarf_Die *die_mem, void *arg)
 					     /*is_pointer=*/true))
 				return DIE_FIND_CB_END;
 
-			/* Local variables accessed by a register + offset */
+			/* variables accessed by a register + offset */
 			if (ops->atom == DW_OP_bregx && data->reg == ops->number &&
 			    check_allowed_ops(ops, nops) &&
 			    match_var_offset(die_mem, data, data->offset, ops->number2,
-					     /*is_poitner=*/false))
+					     /*is_pointer=*/is_breg_access_indirect(ops, nops)))
 				return DIE_FIND_CB_END;
 		}
 	}
-- 
2.50.1.470.g6ba607880d-goog

[PATCH v1 3/6] perf dwarf-aux: Better type matching for stack variables

[Zecheng Li]

Utilizes the previous is_breg_access_indirect function to determine if
the stack location stores the variable itself or the struct it points
to.

If the DWARF expression shows DW_OP_stack_value without DW_OP_deref, it
indicates the variable value is the reg + offset itself, and the stack
location it points to is the dereferenced type.

Signed-off-by: Zecheng Li <zecheng@google.com>
---
 tools/perf/util/dwarf-aux.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 814c96ea509f..4039dbd2b8c0 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -1635,6 +1635,14 @@ static int __die_collect_vars_cb(Dwarf_Die *die_mem, void *arg)
 	if (die_get_real_type(die_mem, &type_die) == NULL)
 		return DIE_FIND_CB_SIBLING;
 
+	if ((ops->atom == DW_OP_fbreg || ops->atom == DW_OP_breg7) &&
+	    dwarf_tag(&type_die) == DW_TAG_pointer_type &&
+	    is_breg_access_indirect(ops, nops)) {
+		/* Get the target type of the pointer */
+		if (die_get_real_type(&type_die, &type_die) == NULL)
+			return DIE_FIND_CB_SIBLING;
+	}
+
 	vt = malloc(sizeof(*vt));
 	if (vt == NULL)
 		return DIE_FIND_CB_END;
-- 
2.50.1.470.g6ba607880d-goog

[PATCH v1 4/6] perf annotate: Skip annotating data types to lea instructions

[Zecheng Li]

Remove type annotation on lea instructions since they are not accessing
memory. It should be counted as `no_mem_ops`.

Signed-off-by: Zecheng Li <zecheng@google.com>
---
 tools/perf/util/annotate.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 0dd475a744b6..0d6f85ab9170 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -2729,6 +2729,11 @@ __hist_entry__get_data_type(struct hist_entry *he, struct arch *arch,
 		return &stackop_type;
 	}
 
+	if (!strncmp(dl->ins.name, "lea", 3)) {
+		istat->bad++;
+		return NULL;
+	}
+
 	for_each_insn_op_loc(&loc, i, op_loc) {
 		struct data_loc_info dloc = {
 			.arch = arch,
-- 
2.50.1.470.g6ba607880d-goog

[PATCH v1 5/6] perf dwarf-aux: Find pointer type to a type

[Zecheng Li]

Implement die_find_pointer_to_type that searches for the pointer type to
a given type in the CU. There's no guarantee that a pointer DIE exists
for every possible base type. Compilers only generate DIEs for types
actually used or defined.

The performance could be improved by adding a cache for the pointer
types. Currently its impact on the annotation time for vmlinux is low.

Signed-off-by: Zecheng Li <zecheng@google.com>
---
 tools/perf/util/dwarf-aux.c | 69 +++++++++++++++++++++++++++++++++++++
 tools/perf/util/dwarf-aux.h |  4 +++
 2 files changed, 73 insertions(+)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 4039dbd2b8c0..49d509839a85 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -2121,3 +2121,72 @@ Dwarf_Die *die_deref_ptr_type(Dwarf_Die *ptr_die, int offset,
 
 	return die_get_member_type(&type_die, offset, die_mem);
 }
+
+struct find_pointer_type_data {
+	/* DIE offset of the type we want to point to */
+	Dwarf_Off target_type_offset;
+	Dwarf_Die *found_die;
+};
+
+static int __die_find_pointer_to_type_cb(Dwarf_Die *die_mem, void *arg)
+{
+	struct find_pointer_type_data *data = arg;
+	Dwarf_Attribute type_attr;
+	Dwarf_Die type_die;
+	Dwarf_Off ref_type_offset;
+
+	if (dwarf_tag(die_mem) != DW_TAG_pointer_type)
+		return DIE_FIND_CB_CONTINUE;
+
+	if (!dwarf_attr(die_mem, DW_AT_type, &type_attr))
+		return DIE_FIND_CB_SIBLING;
+
+	/* Get the DIE this pointer points to */
+	if (!dwarf_formref_die(&type_attr, &type_die))
+		return DIE_FIND_CB_SIBLING;
+
+	ref_type_offset = dwarf_dieoffset(&type_die);
+
+	if (ref_type_offset != 0 && ref_type_offset == data->target_type_offset) {
+		/* This die_mem is a pointer to the target type */
+		if (data->found_die)
+			*data->found_die = *die_mem;
+		return DIE_FIND_CB_END;
+	}
+
+	return DIE_FIND_CB_SIBLING;
+}
+
+/**
+ * die_find_pointer_to_type - Find a DIE for a pointer to a given type
+ * @cu_die: The compilation unit to search within.
+ * @target_type: The DIE of the type you want to find a pointer to.
+ * @result_die: Buffer to store the found DW_TAG_pointer_type DIE.
+ *
+ * Scans the children of the @cu_die for a DW_TAG_pointer_type DIE
+ * whose DW_AT_type attribute references the @target_type.
+ *
+ * Return: @result_die if found, NULL otherwise.
+ */
+Dwarf_Die *die_find_pointer_to_type(Dwarf_Die *cu_die, Dwarf_Die *target_type,
+				   Dwarf_Die *result_die)
+{
+	struct find_pointer_type_data data;
+	Dwarf_Die search_mem;
+
+	if (!cu_die || !target_type || !result_die)
+		return NULL;
+
+	data.target_type_offset = dwarf_dieoffset(target_type);
+	if (data.target_type_offset == 0) {
+		pr_debug("Target type DIE has no offset\n");
+		return NULL;
+	}
+	data.found_die = result_die;
+
+	if (die_find_child(cu_die, __die_find_pointer_to_type_cb, &data, &search_mem))
+		return result_die;
+
+	return NULL;
+}
+
diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
index 892c8c5c23fc..7e1336ff276e 100644
--- a/tools/perf/util/dwarf-aux.h
+++ b/tools/perf/util/dwarf-aux.h
@@ -156,6 +156,10 @@ Dwarf_Die *die_get_member_type(Dwarf_Die *type_die, int offset, Dwarf_Die *die_m
 /* Return type info where the pointer and offset point to */
 Dwarf_Die *die_deref_ptr_type(Dwarf_Die *ptr_die, int offset, Dwarf_Die *die_mem);
 
+/* Find a DIE for a pointer to a given type */
+Dwarf_Die *die_find_pointer_to_type(Dwarf_Die *cu_die, Dwarf_Die *target_type,
+				   Dwarf_Die *result_die);
+
 /* Get byte offset range of given variable DIE */
 int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf);
 
-- 
2.50.1.470.g6ba607880d-goog

[PATCH v1 6/6] perf annotate: Track arithmetic instructions on pointers

[Zecheng Li]

Track the arithmetic operations on registers with pointer types. We
handle only add and lea instructions. The original pointer information
needs to be preserved for getting outermost struct types. For example,
reg0 points to a struct cfs_rq, when we add 0x10 to reg0, it should
preserve the information of struct cfs_rq + 0x10 in the register instead
of a pointer type to the child field at 0x10.

Details:

1.  struct type_state_reg now includes an offset, indicating if the
    register points to the start or an internal part of its associated
    type. This offset is used in mem to reg and reg to stack mem
    transfers, and also applied to the final type offset.

2.  lea offset(%sp/%fp), reg is now treated as taking the address of a
    stack variable. It worked fine in most cases, but an issue with this
    approach is the pointer type may not exist.

3.  lea offset(%base), reg is handled by moving the type from %base and
    adding an offset, similar to an add operation followed by a mov reg
    to reg.

4.  Non-stack variables from DWARF with non-zero offsets in their
    location expressions are now accepted with register offset tracking.

Limitation: Offset tracking for register moved to the stack is not yet
implemented. Currently, moving an register with offset to the stack
resolves to the member type, which worked in some of the cases. Strictly
it should be a pointer to the immediate child member. Multi-register
addressing modes in LEA are not supported.

Signed-off-by: Zecheng Li <zecheng@google.com>
---
 tools/perf/arch/x86/annotate/instructions.c | 109 +++++++++++++++++++-
 tools/perf/util/annotate-data.c             |  14 ++-
 tools/perf/util/annotate-data.h             |   6 ++
 3 files changed, 120 insertions(+), 9 deletions(-)

diff --git a/tools/perf/arch/x86/annotate/instructions.c b/tools/perf/arch/x86/annotate/instructions.c
index c6d403eae744..82fc0106a4de 100644
--- a/tools/perf/arch/x86/annotate/instructions.c
+++ b/tools/perf/arch/x86/annotate/instructions.c
@@ -248,6 +248,7 @@ static void update_insn_state_x86(struct type_state *state,
 			tsr = &state->regs[state->ret_reg];
 			tsr->type = type_die;
 			tsr->kind = TSR_KIND_TYPE;
+			tsr->offset = 0;
 			tsr->ok = true;
 
 			pr_debug_dtp("call [%x] return -> reg%d",
@@ -284,6 +285,7 @@ static void update_insn_state_x86(struct type_state *state,
 			    !strcmp(var_name, "this_cpu_off") &&
 			    tsr->kind == TSR_KIND_CONST) {
 				tsr->kind = TSR_KIND_PERCPU_BASE;
+				tsr->offset = 0;
 				tsr->ok = true;
 				imm_value = tsr->imm_value;
 			}
@@ -291,6 +293,15 @@ static void update_insn_state_x86(struct type_state *state,
 		else
 			return;
 
+		/* Ignore add to non-pointer types like int */
+		if (dwarf_tag(&tsr->type) == DW_TAG_pointer_type &&
+		    src->reg1 != DWARF_REG_PC && tsr->kind == TSR_KIND_TYPE && !dst->mem_ref) {
+			tsr->offset += imm_value;
+			pr_debug_dtp("add [%x] offset %#"PRIx64" to reg%d",
+				     insn_offset, imm_value, dst->reg1);
+			pr_debug_type_name(&tsr->type, tsr->kind);
+		}
+
 		if (tsr->kind != TSR_KIND_PERCPU_BASE)
 			return;
 
@@ -302,6 +313,7 @@ static void update_insn_state_x86(struct type_state *state,
 			 */
 			tsr->type = type_die;
 			tsr->kind = TSR_KIND_POINTER;
+			tsr->offset = 0;
 			tsr->ok = true;
 
 			pr_debug_dtp("add [%x] percpu %#"PRIx64" -> reg%d",
@@ -311,6 +323,68 @@ static void update_insn_state_x86(struct type_state *state,
 		return;
 	}
 
+	if (!strncmp(dl->ins.name, "lea", 3)) {
+		struct type_state_reg *src_tsr;
+		int sreg = src->reg1;
+
+		if (!has_reg_type(state, sreg) ||
+		    !has_reg_type(state, dst->reg1) ||
+		    !src->mem_ref)
+			return;
+
+		src_tsr = &state->regs[sreg];
+		tsr = &state->regs[dst->reg1];
+
+		tsr->copied_from = -1;
+		tsr->ok = false;
+
+		/* Case 1: Based on stack pointer or frame pointer */
+		if (sreg == fbreg || sreg == state->stack_reg) {
+			struct type_state_stack *stack;
+			int offset = src->offset - fboff;
+
+			stack = find_stack_state(state, offset);
+			if (!stack)
+				return;
+
+			/* TODO: the pointer type may not exist */
+			/* Now the register becomes a pointer to the stack variable */
+			if (!die_find_pointer_to_type(cu_die, &stack->type, &type_die))
+				return;
+
+			tsr->type = type_die;
+			tsr->kind = TSR_KIND_TYPE;
+			tsr->offset = 0;
+			tsr->ok = true;
+
+			pr_debug_dtp("lea [%x] -> reg%d points to %s + %#x on stack\n",
+					insn_offset, dst->reg1,
+					(sreg == fbreg) ? "fbreg" : "sp", offset);
+			pr_debug_type_name(&tsr->type, tsr->kind);
+		}
+		/* Case 2: Based on a register holding a typed pointer */
+		else if (has_reg_type(state, sreg) &&
+			 state->regs[sreg].ok &&
+			 state->regs[sreg].kind == TSR_KIND_TYPE) {
+
+			/* Check if the target type has a member at the new offset */
+			if (__die_get_real_type(&state->regs[sreg].type, &type_die) == NULL ||
+			    !die_get_member_type(&type_die,
+						 src->offset + src_tsr->offset, &type_die))
+				return;
+
+			tsr->type = src_tsr->type;
+			tsr->kind = TSR_KIND_TYPE;
+			tsr->offset = src->offset + src_tsr->offset;
+			tsr->ok = true;
+
+			pr_debug_dtp("lea [%x] -> reg%d points to reg%d + %#x\n",
+					insn_offset, dst->reg1, sreg, src->offset);
+			pr_debug_type_name(&tsr->type, tsr->kind);
+		}
+		return;
+	}
+
 	if (strncmp(dl->ins.name, "mov", 3))
 		return;
 
@@ -345,6 +419,7 @@ static void update_insn_state_x86(struct type_state *state,
 
 			if (var_addr == 40) {
 				tsr->kind = TSR_KIND_CANARY;
+				tsr->offset = 0;
 				tsr->ok = true;
 
 				pr_debug_dtp("mov [%x] stack canary -> reg%d\n",
@@ -361,6 +436,7 @@ static void update_insn_state_x86(struct type_state *state,
 
 			tsr->type = type_die;
 			tsr->kind = TSR_KIND_TYPE;
+			tsr->offset = 0;
 			tsr->ok = true;
 
 			pr_debug_dtp("mov [%x] this-cpu addr=%#"PRIx64" -> reg%d",
@@ -372,6 +448,7 @@ static void update_insn_state_x86(struct type_state *state,
 		if (src->imm) {
 			tsr->kind = TSR_KIND_CONST;
 			tsr->imm_value = src->offset;
+			tsr->offset = 0;
 			tsr->ok = true;
 
 			pr_debug_dtp("mov [%x] imm=%#x -> reg%d\n",
@@ -388,6 +465,7 @@ static void update_insn_state_x86(struct type_state *state,
 		tsr->type = state->regs[src->reg1].type;
 		tsr->kind = state->regs[src->reg1].kind;
 		tsr->imm_value = state->regs[src->reg1].imm_value;
+		tsr->offset = 0;
 		tsr->ok = true;
 
 		/* To copy back the variable type later (hopefully) */
@@ -421,12 +499,14 @@ static void update_insn_state_x86(struct type_state *state,
 			} else if (!stack->compound) {
 				tsr->type = stack->type;
 				tsr->kind = stack->kind;
+				tsr->offset = 0;
 				tsr->ok = true;
 			} else if (die_get_member_type(&stack->type,
 						       offset - stack->offset,
 						       &type_die)) {
 				tsr->type = type_die;
 				tsr->kind = TSR_KIND_TYPE;
+				tsr->offset = 0;
 				tsr->ok = true;
 			} else {
 				tsr->ok = false;
@@ -446,9 +526,10 @@ static void update_insn_state_x86(struct type_state *state,
 		else if (has_reg_type(state, sreg) && state->regs[sreg].ok &&
 			 state->regs[sreg].kind == TSR_KIND_TYPE &&
 			 die_deref_ptr_type(&state->regs[sreg].type,
-					    src->offset, &type_die)) {
+					    src->offset + state->regs[sreg].offset, &type_die)) {
 			tsr->type = type_die;
 			tsr->kind = TSR_KIND_TYPE;
+			tsr->offset = 0;
 			tsr->ok = true;
 
 			pr_debug_dtp("mov [%x] %#x(reg%d) -> reg%d",
@@ -473,6 +554,7 @@ static void update_insn_state_x86(struct type_state *state,
 
 			tsr->type = type_die;
 			tsr->kind = TSR_KIND_TYPE;
+			tsr->offset = 0;
 			tsr->ok = true;
 
 			pr_debug_dtp("mov [%x] global addr=%"PRIx64" -> reg%d",
@@ -504,6 +586,7 @@ static void update_insn_state_x86(struct type_state *state,
 			    die_get_member_type(&type_die, offset, &type_die)) {
 				tsr->type = type_die;
 				tsr->kind = TSR_KIND_TYPE;
+				tsr->offset = 0;
 				tsr->ok = true;
 
 				if (src->multi_regs) {
@@ -526,6 +609,7 @@ static void update_insn_state_x86(struct type_state *state,
 					     src->offset, &type_die)) {
 			tsr->type = type_die;
 			tsr->kind = TSR_KIND_TYPE;
+			tsr->offset = 0;
 			tsr->ok = true;
 
 			pr_debug_dtp("mov [%x] pointer %#x(reg%d) -> reg%d",
@@ -548,6 +632,7 @@ static void update_insn_state_x86(struct type_state *state,
 							&var_name, &offset) &&
 				    !strcmp(var_name, "__per_cpu_offset")) {
 					tsr->kind = TSR_KIND_PERCPU_BASE;
+					tsr->offset = 0;
 					tsr->ok = true;
 
 					pr_debug_dtp("mov [%x] percpu base reg%d\n",
@@ -571,6 +656,22 @@ static void update_insn_state_x86(struct type_state *state,
 			int offset = dst->offset - fboff;
 
 			tsr = &state->regs[src->reg1];
+			type_die = tsr->type;
+
+			/* The register is derived from a pointer to the type */
+			if (tsr->offset != 0) {
+				/*
+				 * deref gets the innermost field, but we actually want direct
+				 * child field and take a pointer to it.
+				 * However array type like unsigned long[] is already a pointer
+				 * and is the most common case for this kind of stack variable.
+				 */
+				if (die_deref_ptr_type(&tsr->type, tsr->offset, &type_die)) {
+					pr_debug_dtp("find member: tsr->offset: %d", tsr->offset);
+					pr_debug_type_name(&type_die, tsr->kind);
+				} else
+					return;
+			}
 
 			stack = find_stack_state(state, offset);
 			if (stack) {
@@ -583,10 +684,10 @@ static void update_insn_state_x86(struct type_state *state,
 				 */
 				if (!stack->compound)
 					set_stack_state(stack, offset, tsr->kind,
-							&tsr->type);
+							&type_die);
 			} else {
 				findnew_stack_state(state, offset, tsr->kind,
-						    &tsr->type);
+						    &type_die);
 			}
 
 			if (dst->reg1 == fbreg) {
@@ -596,7 +697,7 @@ static void update_insn_state_x86(struct type_state *state,
 				pr_debug_dtp("mov [%x] reg%d -> %#x(reg%d)",
 					     insn_offset, src->reg1, offset, dst->reg1);
 			}
-			pr_debug_type_name(&tsr->type, tsr->kind);
+			pr_debug_type_name(&type_die, tsr->kind);
 		}
 		/*
 		 * Ignore other transfers since it'd set a value in a struct
diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index 1ef2edbc71d9..9cb215e499ef 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -887,7 +887,7 @@ static void update_var_state(struct type_state *state, struct data_loc_info *dlo
 					     insn_offset, -offset);
 			}
 			pr_debug_type_name(&mem_die, TSR_KIND_TYPE);
-		} else if (has_reg_type(state, var->reg) && var->offset == 0) {
+		} else if (has_reg_type(state, var->reg)) {
 			struct type_state_reg *reg;
 			Dwarf_Die orig_type;
 
@@ -898,13 +898,17 @@ static void update_var_state(struct type_state *state, struct data_loc_info *dlo
 				continue;
 
 			orig_type = reg->type;
-
+			/*
+			 * var->offset + reg value is the beginning of the struct
+			 * reg->offset is the offset the reg points
+			 */
+			reg->offset = -var->offset;
 			reg->type = mem_die;
 			reg->kind = TSR_KIND_TYPE;
 			reg->ok = true;
 
-			pr_debug_dtp("var [%"PRIx64"] reg%d",
-				     insn_offset, var->reg);
+			pr_debug_dtp("var [%"PRIx64"] reg%d at offset %d",
+				     insn_offset, var->reg, var->offset);
 			pr_debug_type_name(&mem_die, TSR_KIND_TYPE);
 
 			/*
@@ -1092,7 +1096,7 @@ static enum type_match_result check_matching_type(struct type_state *state,
 		if (__die_get_real_type(&state->regs[reg].type, type_die) == NULL)
 			return PERF_TMR_NO_POINTER;
 
-		dloc->type_offset = dloc->op->offset;
+		dloc->type_offset = dloc->op->offset + state->regs[reg].offset;
 
 		if (dwarf_tag(type_die) == DW_TAG_typedef)
 			die_get_real_type(type_die, &sized_type);
diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h
index 541fee1a5f0a..a6df0491eebf 100644
--- a/tools/perf/util/annotate-data.h
+++ b/tools/perf/util/annotate-data.h
@@ -173,6 +173,12 @@ extern struct annotated_data_stat ann_data_stat;
 struct type_state_reg {
 	Dwarf_Die type;
 	u32 imm_value;
+        /*
+         * The offset within the struct that the register points to.
+         * A value of 0 means the register points to the beginning.
+	 * type_offset = op->offset + reg->offset
+         */
+	s32 offset;
 	bool ok;
 	bool caller_saved;
 	u8 kind;
-- 
2.50.1.470.g6ba607880d-goog

Re: [PATCH v1 1/6] perf dwarf-aux: Use signed comparison in match_var_offset

[Ian Rogers]

On Fri, Jul 25, 2025 at 1:28 PM Zecheng Li <zecheng@google.com> wrote:
>
> match_var_offset compares address offsets to determine if an access
> falls within a variable's bounds. The offsets involved for those
> relative to base registers from DW_OP_breg can be negative.
>
> The current implementation uses unsigned types (u64) for these offsets,
> which rejects almost all negative values.
>
> This commit changes the local variables within match_var_offset to
> signed types (s64) before performing comparisons. This ensures correct
> behavior when addr_offset_ or addr_type_ are negative.
>
> Signed-off-by: Zecheng Li <zecheng@google.com>
> ---
>  tools/perf/util/dwarf-aux.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index 559c953ca172..bf906dff9ef0 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -1388,10 +1388,12 @@ struct find_var_data {
>  #define DWARF_OP_DIRECT_REGS  32
>
>  static bool match_var_offset(Dwarf_Die *die_mem, struct find_var_data *data,
> -                            u64 addr_offset, u64 addr_type, bool is_pointer)
> +                            u64 addr_offset_, u64 addr_type_, bool is_pointer)
>  {
>         Dwarf_Die type_die;
>         Dwarf_Word size;
> +       s64 addr_offset = (s64)addr_offset_;
> +       s64 addr_type = (s64)addr_type_;

Would it be better to make the function take signed types? I'm
thinking if a 32-bit int is passed, with the signature as-is it is
unclear if sign-extension will happen.

Thanks,
Ian

>
>         if (addr_offset == addr_type) {
>                 /* Update offset relative to the start of the variable */
> @@ -1414,7 +1416,7 @@ static bool match_var_offset(Dwarf_Die *die_mem, struct find_var_data *data,
>         if (dwarf_aggregate_size(&type_die, &size) < 0)
>                 return false;
>
> -       if (addr_offset >= addr_type + size)
> +       if (addr_offset_ - addr_type_ >= size)
>                 return false;
>
>         /* Update offset relative to the start of the variable */
> --
> 2.50.1.470.g6ba607880d-goog
>

Re: [PATCH v1 3/6] perf dwarf-aux: Better type matching for stack variables

[Ian Rogers]

On Fri, Jul 25, 2025 at 1:28 PM Zecheng Li <zecheng@google.com> wrote:
>
> Utilizes the previous is_breg_access_indirect function to determine if
> the stack location stores the variable itself or the struct it points
> to.
>
> If the DWARF expression shows DW_OP_stack_value without DW_OP_deref, it
> indicates the variable value is the reg + offset itself, and the stack
> location it points to is the dereferenced type.
>
> Signed-off-by: Zecheng Li <zecheng@google.com>
> ---
>  tools/perf/util/dwarf-aux.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index 814c96ea509f..4039dbd2b8c0 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -1635,6 +1635,14 @@ static int __die_collect_vars_cb(Dwarf_Die *die_mem, void *arg)
>         if (die_get_real_type(die_mem, &type_die) == NULL)
>                 return DIE_FIND_CB_SIBLING;
>
> +       if ((ops->atom == DW_OP_fbreg || ops->atom == DW_OP_breg7) &&

A comment saying the significance of DW_OP_fbreg and DW_OP_breg7 would
be useful, for example, why not DW_OP_breg6? Isn't breg7 going to be
x86 specific?

Thanks,
Ian


> +           dwarf_tag(&type_die) == DW_TAG_pointer_type &&
> +           is_breg_access_indirect(ops, nops)) {
> +               /* Get the target type of the pointer */
> +               if (die_get_real_type(&type_die, &type_die) == NULL)
> +                       return DIE_FIND_CB_SIBLING;
> +       }
> +
>         vt = malloc(sizeof(*vt));
>         if (vt == NULL)
>                 return DIE_FIND_CB_END;
> --
> 2.50.1.470.g6ba607880d-goog
>

Re: [PATCH v1 4/6] perf annotate: Skip annotating data types to lea instructions

[Ian Rogers]

On Fri, Jul 25, 2025 at 1:28 PM Zecheng Li <zecheng@google.com> wrote:
>
> Remove type annotation on lea instructions since they are not accessing
> memory. It should be counted as `no_mem_ops`.
>
> Signed-off-by: Zecheng Li <zecheng@google.com>
> ---
>  tools/perf/util/annotate.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 0dd475a744b6..0d6f85ab9170 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -2729,6 +2729,11 @@ __hist_entry__get_data_type(struct hist_entry *he, struct arch *arch,
>                 return &stackop_type;
>         }
>
> +       if (!strncmp(dl->ins.name, "lea", 3)) {
> +               istat->bad++;
> +               return NULL;
> +       }

Should this be conditional on the arch being x86?

Thanks,
Ian

> +
>         for_each_insn_op_loc(&loc, i, op_loc) {
>                 struct data_loc_info dloc = {
>                         .arch = arch,
> --
> 2.50.1.470.g6ba607880d-goog
>

Re: [PATCH v1 1/6] perf dwarf-aux: Use signed comparison in match_var_offset

[Namhyung Kim]

On Fri, Jul 25, 2025 at 05:58:05PM -0700, Ian Rogers wrote:
> On Fri, Jul 25, 2025 at 1:28 PM Zecheng Li <zecheng@google.com> wrote:
> >
> > match_var_offset compares address offsets to determine if an access
> > falls within a variable's bounds. The offsets involved for those
> > relative to base registers from DW_OP_breg can be negative.
> >
> > The current implementation uses unsigned types (u64) for these offsets,
> > which rejects almost all negative values.
> >
> > This commit changes the local variables within match_var_offset to
> > signed types (s64) before performing comparisons. This ensures correct
> > behavior when addr_offset_ or addr_type_ are negative.
> >
> > Signed-off-by: Zecheng Li <zecheng@google.com>
> > ---
> >  tools/perf/util/dwarf-aux.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> > index 559c953ca172..bf906dff9ef0 100644
> > --- a/tools/perf/util/dwarf-aux.c
> > +++ b/tools/perf/util/dwarf-aux.c
> > @@ -1388,10 +1388,12 @@ struct find_var_data {
> >  #define DWARF_OP_DIRECT_REGS  32
> >
> >  static bool match_var_offset(Dwarf_Die *die_mem, struct find_var_data *data,
> > -                            u64 addr_offset, u64 addr_type, bool is_pointer)
> > +                            u64 addr_offset_, u64 addr_type_, bool is_pointer)
> >  {
> >         Dwarf_Die type_die;
> >         Dwarf_Word size;
> > +       s64 addr_offset = (s64)addr_offset_;
> > +       s64 addr_type = (s64)addr_type_;
> 
> Would it be better to make the function take signed types? I'm
> thinking if a 32-bit int is passed, with the signature as-is it is
> unclear if sign-extension will happen.

Hmm.. right.  The addr_offset often from 'int' type so negative value
can have the sign-extension problem.

Zecheng, can you please update the function signature to s64 and check
if the final offset is negative or bigger than the size?

Thanks,
Namhyung

> >
> >         if (addr_offset == addr_type) {
> >                 /* Update offset relative to the start of the variable */
> > @@ -1414,7 +1416,7 @@ static bool match_var_offset(Dwarf_Die *die_mem, struct find_var_data *data,
> >         if (dwarf_aggregate_size(&type_die, &size) < 0)
> >                 return false;
> >
> > -       if (addr_offset >= addr_type + size)
> > +       if (addr_offset_ - addr_type_ >= size)
> >                 return false;
> >
> >         /* Update offset relative to the start of the variable */
> > --
> > 2.50.1.470.g6ba607880d-goog
> >

Re: [PATCH v1 2/6] perf dwarf-aux: More accurate variable type match for breg

[Namhyung Kim]

On Fri, Jul 25, 2025 at 08:28:05PM +0000, Zecheng Li wrote:
> Introduces the function is_breg_access_indirect to determine whether a
> memory access involving a DW_OP_breg* operation refers to the variable's
> value directly or requires dereferencing the variable's type as a
> pointer based on the DWARF expression. Previously, all breg based
> accesses were assumed to directly access the variable's value
> (is_pointer = false).
> 
> The is_breg_access_indirect function handles three main cases:
> 
> 1. Base register + offset only: (e.g., DW_OP_breg7 RSP+88) The
>    calculated address is the location of the variable. The access is
>    direct, so no type dereference is needed. Returns false.

This is the common case.  Either basic type is store in the stack or
pointer type is spilled into the stack.

> 
> 2. Base register + offset, followed by other operations ending in
>    DW_OP_stack_value, including DW_OP_deref: (e.g., DW_OP_breg7 RSP+96,
>    DW_OP_deref, DW_OP_plus_uconst 0x64, DW_OP_stack_value) The DWARF
>    expression computes the variable's value, but that value requires a
>    dereference. The memory access is fetching that value, so no type
>    dereference is needed. Returns false.

This is a complex case: the variable needs a pointer calculation.  We
don't support those (complex) expressions for now.

> 
> 3. Base register + offset, followed only by DW_OP_stack_value: (e.g.,
>    DW_OP_breg7 RSP+176, DW_OP_stack_value) This indicates the value at
>    the base + offset is the variable's value. Since this value is being
>    used as an address in the memory access, the variable's type is
>    treated as a pointer and requires a type dereference. Returns true.

The value has a value.  But I guess the type dereference is only
required if the base register points to the stack.

> 
> The is_pointer argument passed to match_var_offset is now set by
> is_breg_access_indirect for breg accesses.
> 
> Signed-off-by: Zecheng Li <zecheng@google.com>
> ---
>  tools/perf/util/dwarf-aux.c | 42 ++++++++++++++++++++++++++++++++-----
>  1 file changed, 37 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index bf906dff9ef0..814c96ea509f 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -1424,6 +1424,38 @@ static bool match_var_offset(Dwarf_Die *die_mem, struct find_var_data *data,
>  	return true;
>  }
>  
> +/**
> + * is_breg_access_indirect - Check if breg based access implies type dereference
> + * @ops: DWARF operations array
> + * @nops: Number of operations in @ops
> + *
> + * Returns true if the DWARF expression indicates the variable's value is
> + * a pointer that the memory access dereferences.
> + * Returns false if the expression evaluates to the variable's value directly.
> + * This is called after check_allowed_ops.
> + */
> +static bool is_breg_access_indirect(Dwarf_Op *ops, size_t nops)
> +{
> +	ops++;
> +	nops--;
> +
> +	/* only the base register */
> +	if (nops == 0)
> +		return false;
> +
> +	switch (ops->atom) {
> +	case DW_OP_stack_value:
> +		return true;

As I said, I think it also need to check if the base is the stack.


> +	case DW_OP_deref_size:
> +	case DW_OP_deref:
> +	case DW_OP_piece:
> +		return false;

I'm not sure if it's always false.  I sometimes see this pattern

  DW_OP_bregN, DW_OP_deref*, DW_OP_stack_value

which I believe it's almost same as just

  DW_OP_bregN

No?

> +	default:
> +		/* unreachable, OP not supported */
> +		return false;
> +	}
> +}
> +
>  /* Only checks direct child DIEs in the given scope. */
>  static int __die_find_var_reg_cb(Dwarf_Die *die_mem, void *arg)
>  {
> @@ -1452,7 +1484,7 @@ static int __die_find_var_reg_cb(Dwarf_Die *die_mem, void *arg)
>  		if (data->is_fbreg && ops->atom == DW_OP_fbreg &&
>  		    check_allowed_ops(ops, nops) &&
>  		    match_var_offset(die_mem, data, data->offset, ops->number,
> -				     /*is_pointer=*/false))
> +				     /*is_pointer=*/is_breg_access_indirect(ops, nops)))

The annotate like /*is_pointer=*/ is used for constant arguments.
You can delete here and below.

Thanks,
Namhyung

>  			return DIE_FIND_CB_END;
>  
>  		/* Only match with a simple case */
> @@ -1464,11 +1496,11 @@ static int __die_find_var_reg_cb(Dwarf_Die *die_mem, void *arg)
>  					     /*is_pointer=*/true))
>  				return DIE_FIND_CB_END;
>  
> -			/* Local variables accessed by a register + offset */
> +			/* variables accessed by a register + offset */
>  			if (ops->atom == (DW_OP_breg0 + data->reg) &&
>  			    check_allowed_ops(ops, nops) &&
>  			    match_var_offset(die_mem, data, data->offset, ops->number,
> -					     /*is_pointer=*/false))
> +					     /*is_pointer=*/is_breg_access_indirect(ops, nops)))
>  				return DIE_FIND_CB_END;
>  		} else {
>  			/* pointer variables saved in a register 32 or above */
> @@ -1478,11 +1510,11 @@ static int __die_find_var_reg_cb(Dwarf_Die *die_mem, void *arg)
>  					     /*is_pointer=*/true))
>  				return DIE_FIND_CB_END;
>  
> -			/* Local variables accessed by a register + offset */
> +			/* variables accessed by a register + offset */
>  			if (ops->atom == DW_OP_bregx && data->reg == ops->number &&
>  			    check_allowed_ops(ops, nops) &&
>  			    match_var_offset(die_mem, data, data->offset, ops->number2,
> -					     /*is_poitner=*/false))
> +					     /*is_pointer=*/is_breg_access_indirect(ops, nops)))
>  				return DIE_FIND_CB_END;
>  		}
>  	}
> -- 
> 2.50.1.470.g6ba607880d-goog
> 

Re: [PATCH v1 3/6] perf dwarf-aux: Better type matching for stack variables

[Namhyung Kim]

On Fri, Jul 25, 2025 at 06:17:33PM -0700, Ian Rogers wrote:
> On Fri, Jul 25, 2025 at 1:28 PM Zecheng Li <zecheng@google.com> wrote:
> >
> > Utilizes the previous is_breg_access_indirect function to determine if
> > the stack location stores the variable itself or the struct it points
> > to.
> >
> > If the DWARF expression shows DW_OP_stack_value without DW_OP_deref, it
> > indicates the variable value is the reg + offset itself, and the stack
> > location it points to is the dereferenced type.
> >
> > Signed-off-by: Zecheng Li <zecheng@google.com>
> > ---
> >  tools/perf/util/dwarf-aux.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> > index 814c96ea509f..4039dbd2b8c0 100644
> > --- a/tools/perf/util/dwarf-aux.c
> > +++ b/tools/perf/util/dwarf-aux.c
> > @@ -1635,6 +1635,14 @@ static int __die_collect_vars_cb(Dwarf_Die *die_mem, void *arg)
> >         if (die_get_real_type(die_mem, &type_die) == NULL)
> >                 return DIE_FIND_CB_SIBLING;
> >
> > +       if ((ops->atom == DW_OP_fbreg || ops->atom == DW_OP_breg7) &&
> 
> A comment saying the significance of DW_OP_fbreg and DW_OP_breg7 would
> be useful, for example, why not DW_OP_breg6? Isn't breg7 going to be
> x86 specific?

Good point!  Right, it's x86 specific and we cannot hard-code.  It needs
a way to indicate this register is a stack pointer on the target arch.
Please see the instruction tracking code which checks state->stack_reg.
 
On x86_64, DWARF reg 7 is RSP.  If RBP is used as a frame pointer, it's
probably using DW_OP_fbreg expression.

> 
> > +           dwarf_tag(&type_die) == DW_TAG_pointer_type &&
> > +           is_breg_access_indirect(ops, nops)) {

Maybe it needs to check the register inside is_breg_access_indirect().

Thanks,
Namhyung


> > +               /* Get the target type of the pointer */
> > +               if (die_get_real_type(&type_die, &type_die) == NULL)
> > +                       return DIE_FIND_CB_SIBLING;
> > +       }
> > +
> >         vt = malloc(sizeof(*vt));
> >         if (vt == NULL)
> >                 return DIE_FIND_CB_END;
> > --
> > 2.50.1.470.g6ba607880d-goog
> >

Re: [PATCH v1 5/6] perf dwarf-aux: Find pointer type to a type

[Namhyung Kim]

On Fri, Jul 25, 2025 at 08:28:08PM +0000, Zecheng Li wrote:
> Implement die_find_pointer_to_type that searches for the pointer type to
> a given type in the CU. There's no guarantee that a pointer DIE exists
> for every possible base type. Compilers only generate DIEs for types
> actually used or defined.

Please describe why it's necessary first.  What happens if it fails to
fine one?

Thanks,
Namhyung

> 
> The performance could be improved by adding a cache for the pointer
> types. Currently its impact on the annotation time for vmlinux is low.
> 
> Signed-off-by: Zecheng Li <zecheng@google.com>
> ---
>  tools/perf/util/dwarf-aux.c | 69 +++++++++++++++++++++++++++++++++++++
>  tools/perf/util/dwarf-aux.h |  4 +++
>  2 files changed, 73 insertions(+)
> 
> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index 4039dbd2b8c0..49d509839a85 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -2121,3 +2121,72 @@ Dwarf_Die *die_deref_ptr_type(Dwarf_Die *ptr_die, int offset,
>  
>  	return die_get_member_type(&type_die, offset, die_mem);
>  }
> +
> +struct find_pointer_type_data {
> +	/* DIE offset of the type we want to point to */
> +	Dwarf_Off target_type_offset;
> +	Dwarf_Die *found_die;
> +};
> +
> +static int __die_find_pointer_to_type_cb(Dwarf_Die *die_mem, void *arg)
> +{
> +	struct find_pointer_type_data *data = arg;
> +	Dwarf_Attribute type_attr;
> +	Dwarf_Die type_die;
> +	Dwarf_Off ref_type_offset;
> +
> +	if (dwarf_tag(die_mem) != DW_TAG_pointer_type)
> +		return DIE_FIND_CB_CONTINUE;
> +
> +	if (!dwarf_attr(die_mem, DW_AT_type, &type_attr))
> +		return DIE_FIND_CB_SIBLING;
> +
> +	/* Get the DIE this pointer points to */
> +	if (!dwarf_formref_die(&type_attr, &type_die))
> +		return DIE_FIND_CB_SIBLING;
> +
> +	ref_type_offset = dwarf_dieoffset(&type_die);
> +
> +	if (ref_type_offset != 0 && ref_type_offset == data->target_type_offset) {
> +		/* This die_mem is a pointer to the target type */
> +		if (data->found_die)
> +			*data->found_die = *die_mem;
> +		return DIE_FIND_CB_END;
> +	}
> +
> +	return DIE_FIND_CB_SIBLING;
> +}
> +
> +/**
> + * die_find_pointer_to_type - Find a DIE for a pointer to a given type
> + * @cu_die: The compilation unit to search within.
> + * @target_type: The DIE of the type you want to find a pointer to.
> + * @result_die: Buffer to store the found DW_TAG_pointer_type DIE.
> + *
> + * Scans the children of the @cu_die for a DW_TAG_pointer_type DIE
> + * whose DW_AT_type attribute references the @target_type.
> + *
> + * Return: @result_die if found, NULL otherwise.
> + */
> +Dwarf_Die *die_find_pointer_to_type(Dwarf_Die *cu_die, Dwarf_Die *target_type,
> +				   Dwarf_Die *result_die)
> +{
> +	struct find_pointer_type_data data;
> +	Dwarf_Die search_mem;
> +
> +	if (!cu_die || !target_type || !result_die)
> +		return NULL;
> +
> +	data.target_type_offset = dwarf_dieoffset(target_type);
> +	if (data.target_type_offset == 0) {
> +		pr_debug("Target type DIE has no offset\n");
> +		return NULL;
> +	}
> +	data.found_die = result_die;
> +
> +	if (die_find_child(cu_die, __die_find_pointer_to_type_cb, &data, &search_mem))
> +		return result_die;
> +
> +	return NULL;
> +}
> +
> diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
> index 892c8c5c23fc..7e1336ff276e 100644
> --- a/tools/perf/util/dwarf-aux.h
> +++ b/tools/perf/util/dwarf-aux.h
> @@ -156,6 +156,10 @@ Dwarf_Die *die_get_member_type(Dwarf_Die *type_die, int offset, Dwarf_Die *die_m
>  /* Return type info where the pointer and offset point to */
>  Dwarf_Die *die_deref_ptr_type(Dwarf_Die *ptr_die, int offset, Dwarf_Die *die_mem);
>  
> +/* Find a DIE for a pointer to a given type */
> +Dwarf_Die *die_find_pointer_to_type(Dwarf_Die *cu_die, Dwarf_Die *target_type,
> +				   Dwarf_Die *result_die);
> +
>  /* Get byte offset range of given variable DIE */
>  int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf);
>  
> -- 
> 2.50.1.470.g6ba607880d-goog
> 

Re: [PATCH v1 6/6] perf annotate: Track arithmetic instructions on pointers

[Namhyung Kim]

On Fri, Jul 25, 2025 at 08:28:09PM +0000, Zecheng Li wrote:
> Track the arithmetic operations on registers with pointer types. We
> handle only add and lea instructions. The original pointer information

I'm afraid of possible inaccuracy due to incompleteness.  While I think
'add' is the common case, but it may have 'sub', 'mul', 'div' or other
arith operations too.  In general, I think it's better not to report
rather then incorrect result.

Maybe we can invalidate the state once it's used by any unsupported
arith instruction?

Thanks,
Namhyung


> needs to be preserved for getting outermost struct types. For example,
> reg0 points to a struct cfs_rq, when we add 0x10 to reg0, it should
> preserve the information of struct cfs_rq + 0x10 in the register instead
> of a pointer type to the child field at 0x10.
> 
> Details:
> 
> 1.  struct type_state_reg now includes an offset, indicating if the
>     register points to the start or an internal part of its associated
>     type. This offset is used in mem to reg and reg to stack mem
>     transfers, and also applied to the final type offset.
> 
> 2.  lea offset(%sp/%fp), reg is now treated as taking the address of a
>     stack variable. It worked fine in most cases, but an issue with this
>     approach is the pointer type may not exist.
> 
> 3.  lea offset(%base), reg is handled by moving the type from %base and
>     adding an offset, similar to an add operation followed by a mov reg
>     to reg.
> 
> 4.  Non-stack variables from DWARF with non-zero offsets in their
>     location expressions are now accepted with register offset tracking.
> 
> Limitation: Offset tracking for register moved to the stack is not yet
> implemented. Currently, moving an register with offset to the stack
> resolves to the member type, which worked in some of the cases. Strictly
> it should be a pointer to the immediate child member. Multi-register
> addressing modes in LEA are not supported.
> 
> Signed-off-by: Zecheng Li <zecheng@google.com>
> ---
>  tools/perf/arch/x86/annotate/instructions.c | 109 +++++++++++++++++++-
>  tools/perf/util/annotate-data.c             |  14 ++-
>  tools/perf/util/annotate-data.h             |   6 ++
>  3 files changed, 120 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/perf/arch/x86/annotate/instructions.c b/tools/perf/arch/x86/annotate/instructions.c
> index c6d403eae744..82fc0106a4de 100644
> --- a/tools/perf/arch/x86/annotate/instructions.c
> +++ b/tools/perf/arch/x86/annotate/instructions.c
> @@ -248,6 +248,7 @@ static void update_insn_state_x86(struct type_state *state,
>  			tsr = &state->regs[state->ret_reg];
>  			tsr->type = type_die;
>  			tsr->kind = TSR_KIND_TYPE;
> +			tsr->offset = 0;
>  			tsr->ok = true;
>  
>  			pr_debug_dtp("call [%x] return -> reg%d",
> @@ -284,6 +285,7 @@ static void update_insn_state_x86(struct type_state *state,
>  			    !strcmp(var_name, "this_cpu_off") &&
>  			    tsr->kind == TSR_KIND_CONST) {
>  				tsr->kind = TSR_KIND_PERCPU_BASE;
> +				tsr->offset = 0;
>  				tsr->ok = true;
>  				imm_value = tsr->imm_value;
>  			}
> @@ -291,6 +293,15 @@ static void update_insn_state_x86(struct type_state *state,
>  		else
>  			return;
>  
> +		/* Ignore add to non-pointer types like int */
> +		if (dwarf_tag(&tsr->type) == DW_TAG_pointer_type &&
> +		    src->reg1 != DWARF_REG_PC && tsr->kind == TSR_KIND_TYPE && !dst->mem_ref) {
> +			tsr->offset += imm_value;
> +			pr_debug_dtp("add [%x] offset %#"PRIx64" to reg%d",
> +				     insn_offset, imm_value, dst->reg1);
> +			pr_debug_type_name(&tsr->type, tsr->kind);
> +		}
> +
>  		if (tsr->kind != TSR_KIND_PERCPU_BASE)
>  			return;
>  
> @@ -302,6 +313,7 @@ static void update_insn_state_x86(struct type_state *state,
>  			 */
>  			tsr->type = type_die;
>  			tsr->kind = TSR_KIND_POINTER;
> +			tsr->offset = 0;
>  			tsr->ok = true;
>  
>  			pr_debug_dtp("add [%x] percpu %#"PRIx64" -> reg%d",
> @@ -311,6 +323,68 @@ static void update_insn_state_x86(struct type_state *state,
>  		return;
>  	}
>  
> +	if (!strncmp(dl->ins.name, "lea", 3)) {
> +		struct type_state_reg *src_tsr;
> +		int sreg = src->reg1;
> +
> +		if (!has_reg_type(state, sreg) ||
> +		    !has_reg_type(state, dst->reg1) ||
> +		    !src->mem_ref)
> +			return;
> +
> +		src_tsr = &state->regs[sreg];
> +		tsr = &state->regs[dst->reg1];
> +
> +		tsr->copied_from = -1;
> +		tsr->ok = false;
> +
> +		/* Case 1: Based on stack pointer or frame pointer */
> +		if (sreg == fbreg || sreg == state->stack_reg) {
> +			struct type_state_stack *stack;
> +			int offset = src->offset - fboff;
> +
> +			stack = find_stack_state(state, offset);
> +			if (!stack)
> +				return;
> +
> +			/* TODO: the pointer type may not exist */
> +			/* Now the register becomes a pointer to the stack variable */
> +			if (!die_find_pointer_to_type(cu_die, &stack->type, &type_die))
> +				return;
> +
> +			tsr->type = type_die;
> +			tsr->kind = TSR_KIND_TYPE;
> +			tsr->offset = 0;
> +			tsr->ok = true;
> +
> +			pr_debug_dtp("lea [%x] -> reg%d points to %s + %#x on stack\n",
> +					insn_offset, dst->reg1,
> +					(sreg == fbreg) ? "fbreg" : "sp", offset);
> +			pr_debug_type_name(&tsr->type, tsr->kind);
> +		}
> +		/* Case 2: Based on a register holding a typed pointer */
> +		else if (has_reg_type(state, sreg) &&
> +			 state->regs[sreg].ok &&
> +			 state->regs[sreg].kind == TSR_KIND_TYPE) {
> +
> +			/* Check if the target type has a member at the new offset */
> +			if (__die_get_real_type(&state->regs[sreg].type, &type_die) == NULL ||
> +			    !die_get_member_type(&type_die,
> +						 src->offset + src_tsr->offset, &type_die))
> +				return;
> +
> +			tsr->type = src_tsr->type;
> +			tsr->kind = TSR_KIND_TYPE;
> +			tsr->offset = src->offset + src_tsr->offset;
> +			tsr->ok = true;
> +
> +			pr_debug_dtp("lea [%x] -> reg%d points to reg%d + %#x\n",
> +					insn_offset, dst->reg1, sreg, src->offset);
> +			pr_debug_type_name(&tsr->type, tsr->kind);
> +		}
> +		return;
> +	}
> +
>  	if (strncmp(dl->ins.name, "mov", 3))
>  		return;
>  
> @@ -345,6 +419,7 @@ static void update_insn_state_x86(struct type_state *state,
>  
>  			if (var_addr == 40) {
>  				tsr->kind = TSR_KIND_CANARY;
> +				tsr->offset = 0;
>  				tsr->ok = true;
>  
>  				pr_debug_dtp("mov [%x] stack canary -> reg%d\n",
> @@ -361,6 +436,7 @@ static void update_insn_state_x86(struct type_state *state,
>  
>  			tsr->type = type_die;
>  			tsr->kind = TSR_KIND_TYPE;
> +			tsr->offset = 0;
>  			tsr->ok = true;
>  
>  			pr_debug_dtp("mov [%x] this-cpu addr=%#"PRIx64" -> reg%d",
> @@ -372,6 +448,7 @@ static void update_insn_state_x86(struct type_state *state,
>  		if (src->imm) {
>  			tsr->kind = TSR_KIND_CONST;
>  			tsr->imm_value = src->offset;
> +			tsr->offset = 0;
>  			tsr->ok = true;
>  
>  			pr_debug_dtp("mov [%x] imm=%#x -> reg%d\n",
> @@ -388,6 +465,7 @@ static void update_insn_state_x86(struct type_state *state,
>  		tsr->type = state->regs[src->reg1].type;
>  		tsr->kind = state->regs[src->reg1].kind;
>  		tsr->imm_value = state->regs[src->reg1].imm_value;
> +		tsr->offset = 0;
>  		tsr->ok = true;
>  
>  		/* To copy back the variable type later (hopefully) */
> @@ -421,12 +499,14 @@ static void update_insn_state_x86(struct type_state *state,
>  			} else if (!stack->compound) {
>  				tsr->type = stack->type;
>  				tsr->kind = stack->kind;
> +				tsr->offset = 0;
>  				tsr->ok = true;
>  			} else if (die_get_member_type(&stack->type,
>  						       offset - stack->offset,
>  						       &type_die)) {
>  				tsr->type = type_die;
>  				tsr->kind = TSR_KIND_TYPE;
> +				tsr->offset = 0;
>  				tsr->ok = true;
>  			} else {
>  				tsr->ok = false;
> @@ -446,9 +526,10 @@ static void update_insn_state_x86(struct type_state *state,
>  		else if (has_reg_type(state, sreg) && state->regs[sreg].ok &&
>  			 state->regs[sreg].kind == TSR_KIND_TYPE &&
>  			 die_deref_ptr_type(&state->regs[sreg].type,
> -					    src->offset, &type_die)) {
> +					    src->offset + state->regs[sreg].offset, &type_die)) {
>  			tsr->type = type_die;
>  			tsr->kind = TSR_KIND_TYPE;
> +			tsr->offset = 0;
>  			tsr->ok = true;
>  
>  			pr_debug_dtp("mov [%x] %#x(reg%d) -> reg%d",
> @@ -473,6 +554,7 @@ static void update_insn_state_x86(struct type_state *state,
>  
>  			tsr->type = type_die;
>  			tsr->kind = TSR_KIND_TYPE;
> +			tsr->offset = 0;
>  			tsr->ok = true;
>  
>  			pr_debug_dtp("mov [%x] global addr=%"PRIx64" -> reg%d",
> @@ -504,6 +586,7 @@ static void update_insn_state_x86(struct type_state *state,
>  			    die_get_member_type(&type_die, offset, &type_die)) {
>  				tsr->type = type_die;
>  				tsr->kind = TSR_KIND_TYPE;
> +				tsr->offset = 0;
>  				tsr->ok = true;
>  
>  				if (src->multi_regs) {
> @@ -526,6 +609,7 @@ static void update_insn_state_x86(struct type_state *state,
>  					     src->offset, &type_die)) {
>  			tsr->type = type_die;
>  			tsr->kind = TSR_KIND_TYPE;
> +			tsr->offset = 0;
>  			tsr->ok = true;
>  
>  			pr_debug_dtp("mov [%x] pointer %#x(reg%d) -> reg%d",
> @@ -548,6 +632,7 @@ static void update_insn_state_x86(struct type_state *state,
>  							&var_name, &offset) &&
>  				    !strcmp(var_name, "__per_cpu_offset")) {
>  					tsr->kind = TSR_KIND_PERCPU_BASE;
> +					tsr->offset = 0;
>  					tsr->ok = true;
>  
>  					pr_debug_dtp("mov [%x] percpu base reg%d\n",
> @@ -571,6 +656,22 @@ static void update_insn_state_x86(struct type_state *state,
>  			int offset = dst->offset - fboff;
>  
>  			tsr = &state->regs[src->reg1];
> +			type_die = tsr->type;
> +
> +			/* The register is derived from a pointer to the type */
> +			if (tsr->offset != 0) {
> +				/*
> +				 * deref gets the innermost field, but we actually want direct
> +				 * child field and take a pointer to it.
> +				 * However array type like unsigned long[] is already a pointer
> +				 * and is the most common case for this kind of stack variable.
> +				 */
> +				if (die_deref_ptr_type(&tsr->type, tsr->offset, &type_die)) {
> +					pr_debug_dtp("find member: tsr->offset: %d", tsr->offset);
> +					pr_debug_type_name(&type_die, tsr->kind);
> +				} else
> +					return;
> +			}
>  
>  			stack = find_stack_state(state, offset);
>  			if (stack) {
> @@ -583,10 +684,10 @@ static void update_insn_state_x86(struct type_state *state,
>  				 */
>  				if (!stack->compound)
>  					set_stack_state(stack, offset, tsr->kind,
> -							&tsr->type);
> +							&type_die);
>  			} else {
>  				findnew_stack_state(state, offset, tsr->kind,
> -						    &tsr->type);
> +						    &type_die);
>  			}
>  
>  			if (dst->reg1 == fbreg) {
> @@ -596,7 +697,7 @@ static void update_insn_state_x86(struct type_state *state,
>  				pr_debug_dtp("mov [%x] reg%d -> %#x(reg%d)",
>  					     insn_offset, src->reg1, offset, dst->reg1);
>  			}
> -			pr_debug_type_name(&tsr->type, tsr->kind);
> +			pr_debug_type_name(&type_die, tsr->kind);
>  		}
>  		/*
>  		 * Ignore other transfers since it'd set a value in a struct
> diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> index 1ef2edbc71d9..9cb215e499ef 100644
> --- a/tools/perf/util/annotate-data.c
> +++ b/tools/perf/util/annotate-data.c
> @@ -887,7 +887,7 @@ static void update_var_state(struct type_state *state, struct data_loc_info *dlo
>  					     insn_offset, -offset);
>  			}
>  			pr_debug_type_name(&mem_die, TSR_KIND_TYPE);
> -		} else if (has_reg_type(state, var->reg) && var->offset == 0) {
> +		} else if (has_reg_type(state, var->reg)) {
>  			struct type_state_reg *reg;
>  			Dwarf_Die orig_type;
>  
> @@ -898,13 +898,17 @@ static void update_var_state(struct type_state *state, struct data_loc_info *dlo
>  				continue;
>  
>  			orig_type = reg->type;
> -
> +			/*
> +			 * var->offset + reg value is the beginning of the struct
> +			 * reg->offset is the offset the reg points
> +			 */
> +			reg->offset = -var->offset;
>  			reg->type = mem_die;
>  			reg->kind = TSR_KIND_TYPE;
>  			reg->ok = true;
>  
> -			pr_debug_dtp("var [%"PRIx64"] reg%d",
> -				     insn_offset, var->reg);
> +			pr_debug_dtp("var [%"PRIx64"] reg%d at offset %d",
> +				     insn_offset, var->reg, var->offset);
>  			pr_debug_type_name(&mem_die, TSR_KIND_TYPE);
>  
>  			/*
> @@ -1092,7 +1096,7 @@ static enum type_match_result check_matching_type(struct type_state *state,
>  		if (__die_get_real_type(&state->regs[reg].type, type_die) == NULL)
>  			return PERF_TMR_NO_POINTER;
>  
> -		dloc->type_offset = dloc->op->offset;
> +		dloc->type_offset = dloc->op->offset + state->regs[reg].offset;
>  
>  		if (dwarf_tag(type_die) == DW_TAG_typedef)
>  			die_get_real_type(type_die, &sized_type);
> diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h
> index 541fee1a5f0a..a6df0491eebf 100644
> --- a/tools/perf/util/annotate-data.h
> +++ b/tools/perf/util/annotate-data.h
> @@ -173,6 +173,12 @@ extern struct annotated_data_stat ann_data_stat;
>  struct type_state_reg {
>  	Dwarf_Die type;
>  	u32 imm_value;
> +        /*
> +         * The offset within the struct that the register points to.
> +         * A value of 0 means the register points to the beginning.
> +	 * type_offset = op->offset + reg->offset
> +         */
> +	s32 offset;
>  	bool ok;
>  	bool caller_saved;
>  	u8 kind;
> -- 
> 2.50.1.470.g6ba607880d-goog
>