* [PATCH v3 00/10] perf tools: Some improvements on data type profiler
@ 2025-09-17 19:57 Zecheng Li
2025-09-17 19:57 ` [PATCH v3 01/10] perf annotate: Skip annotating data types to lea instructions Zecheng Li
` (9 more replies)
0 siblings, 10 replies; 18+ messages in thread
From: Zecheng Li @ 2025-09-17 19:57 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Liang, Kan, Masami Hiramatsu
Cc: Xu Liu, linux-perf-users, linux-kernel, 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.
Some patches from the previous version of this series were
cherry-picked. This revision adds new improvements based on feedback and
further development.
Here's a breakdown of the changes in this revision:
Patch 1 skips annotations for LEA instructions in x86, as these do not
involve memory access. It now returns NO_TYPE.
Patches 2-3 implement the TSR_KIND_POINTER to represent registers
holding memory addresses of the type. Two points here may need more
discussion:
- When moving a TSR_KIND_POINTER to the stack in set_stack_state, how
should we obtain the pointer size for the target arch?
- If the target instruction is a stack memory access and the stack state
is TSR_KIND_POINTER, we should find or compose a pointer type to
state->type. How should this be implemented?
Patches 4-6 implement a basic approach for register offset tracking that
supports add, sub, and lea operations. The register state is invalidated
when an unsupported arithmetic instruction is encountered. This revision
uses TSR_KIND_POINTER to avoid finding the pointer type in DWARF and
preserves the pointer offset information in the stack state.
One question is what type should we use for the pointer + offset case.
For example, if an offset of 128 is added to a struct rq * pointer, it
then points to &rq->cfs_rq. Should we try to find the pointer type for
cfs_rq or just use the original pointer type (rq *)? It is currently
implemented to use the original pointer type.
Patches 7-9 split patch 8 from v2 with some minor improvements. It skips
check_variable when the type is found directly by register, since
sufficient checking is already performed in match_var_offset.
check_variable lacks some DWARF information to correctly determine if a
variable is valid. I also found it is able to find members for
typedef'd types so I preserve them in match_var_offset.
Patch 10 implements support for DW_OP_piece. Currently, this is allowed
in check_allowed_ops but is handled like other single location
expressions. This patch splits any expression containing DW_OP_piece
into multiple parts and handle them separately.
I have tested each patch on a vmlinux and manually checked the results.
After applying all patches, there are less missing or incorrect
annotations. No obvious regressions were observed.
v3:
Already cherry-picked patches in v2:
perf dwarf-aux: Use signed variable types in match_var_offset
perf dwarf-aux: More accurate variable type match for breg
perf dwarf-aux: Better variable collection for insn tracking
perf dwarf-aux: Skip check_variable for die_find_variable_by_reg
v2:
https://lore.kernel.org/all/20250825195412.223077-1-zecheng@google.com/
1. update the match_var_offset function signature to s64
2. correct the comment for is_breg_access_indirect. Use simpler logic to
match the expressions we support.
3. add is_reg_var_addr to indicate whether a register holds an address
of the variable. This defers the type dereference logic to
update_var_state.
4. invalidate register state for unsupported instructions.
5. include two new patches related to improving data type profiler.
v1:
https://lore.kernel.org/linux-perf-users/20250725202809.1230085-1-zecheng@google.com/
Zecheng Li (10):
perf annotate: Skip annotating data types to lea instructions
perf annotate: Rename TSR_KIND_POINTER to TSR_KIND_PERCPU_POINTER
perf annotate: Track address registers via TSR_KIND_POINTER
perf annotate: Track arithmetic instructions on pointers
perf annotate: Save pointer offset in stack state
perf annotate: Invalidate register states for untracked instructions
perf dwarf-aux: Skip check_variable for die_find_variable_by_reg
perf dwarf-aux: Preserve typedefs in match_var_offset
perf annotate: Improve type comparison from different scopes
perf dwarf-aux: Support DW_OP_piece expressions
tools/perf/arch/x86/annotate/instructions.c | 175 ++++++++++++-
tools/perf/util/annotate-data.c | 93 +++++--
tools/perf/util/annotate-data.h | 14 +-
tools/perf/util/annotate.c | 19 ++
tools/perf/util/dwarf-aux.c | 266 +++++++++++++++-----
tools/perf/util/dwarf-aux.h | 2 +-
6 files changed, 474 insertions(+), 95 deletions(-)
--
2.51.0.384.g4c02a37b29-goog
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 01/10] perf annotate: Skip annotating data types to lea instructions
2025-09-17 19:57 [PATCH v3 00/10] perf tools: Some improvements on data type profiler Zecheng Li
@ 2025-09-17 19:57 ` 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
` (8 subsequent siblings)
9 siblings, 1 reply; 18+ messages in thread
From: Zecheng Li @ 2025-09-17 19:57 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Liang, Kan, Masami Hiramatsu
Cc: Xu Liu, linux-perf-users, linux-kernel, Zecheng Li
Introduce a helper function is_address_gen_insn() to check
arch-dependent address generation instructions like lea in x86. Remove
type annotation on these 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 | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index c9b220d9f924..e2370b7fd599 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -2699,6 +2699,19 @@ static bool is_stack_canary(struct arch *arch, struct annotated_op_loc *loc)
return false;
}
+/**
+ * Returns true if the instruction has a memory operand without
+ * performing a load/store
+ */
+static bool is_address_gen_insn(struct arch *arch, struct disasm_line *dl)
+{
+ if (arch__is(arch, "x86"))
+ if (!strncmp(dl->ins.name, "lea", 3))
+ return true;
+
+ return false;
+}
+
static struct disasm_line *
annotation__prev_asm_line(struct annotation *notes, struct disasm_line *curr)
{
@@ -2807,6 +2820,12 @@ __hist_entry__get_data_type(struct hist_entry *he, struct arch *arch,
return &stackop_type;
}
+ if (is_address_gen_insn(arch, dl)) {
+ istat->bad++;
+ ann_data_stat.no_mem_ops++;
+ return NO_TYPE;
+ }
+
for_each_insn_op_loc(&loc, i, op_loc) {
struct data_loc_info dloc = {
.arch = arch,
--
2.51.0.384.g4c02a37b29-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 02/10] perf annotate: Rename TSR_KIND_POINTER to TSR_KIND_PERCPU_POINTER
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-09-17 19:58 ` Zecheng Li
2025-10-03 5:37 ` Namhyung Kim
2025-09-17 19:58 ` [PATCH v3 03/10] perf annotate: Track address registers via TSR_KIND_POINTER Zecheng Li
` (7 subsequent siblings)
9 siblings, 1 reply; 18+ messages in thread
From: Zecheng Li @ 2025-09-17 19:58 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Liang, Kan, Masami Hiramatsu
Cc: Xu Liu, linux-perf-users, linux-kernel, Zecheng Li
TSR_KIND_POINTER only represents percpu pointers currently. Rename it to
TSR_KIND_PERCPU_POINTER so we can use the TSR_KIND_POINTER to represent
pointer to a type.
Signed-off-by: Zecheng Li <zecheng@google.com>
---
tools/perf/arch/x86/annotate/instructions.c | 4 ++--
tools/perf/util/annotate-data.c | 6 +++---
tools/perf/util/annotate-data.h | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/perf/arch/x86/annotate/instructions.c b/tools/perf/arch/x86/annotate/instructions.c
index c6d403eae744..da98a4e3c52c 100644
--- a/tools/perf/arch/x86/annotate/instructions.c
+++ b/tools/perf/arch/x86/annotate/instructions.c
@@ -301,7 +301,7 @@ static void update_insn_state_x86(struct type_state *state,
* as a pointer.
*/
tsr->type = type_die;
- tsr->kind = TSR_KIND_POINTER;
+ tsr->kind = TSR_KIND_PERCPU_POINTER;
tsr->ok = true;
pr_debug_dtp("add [%x] percpu %#"PRIx64" -> reg%d",
@@ -521,7 +521,7 @@ static void update_insn_state_x86(struct type_state *state,
}
/* And then dereference the calculated pointer if it has one */
else if (has_reg_type(state, sreg) && state->regs[sreg].ok &&
- state->regs[sreg].kind == TSR_KIND_POINTER &&
+ state->regs[sreg].kind == TSR_KIND_PERCPU_POINTER &&
die_get_member_type(&state->regs[sreg].type,
src->offset, &type_die)) {
tsr->type = type_die;
diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index 258157cc43c2..903027a6fb7d 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -58,7 +58,7 @@ void pr_debug_type_name(Dwarf_Die *die, enum type_state_kind kind)
case TSR_KIND_CONST:
pr_info(" constant\n");
return;
- case TSR_KIND_POINTER:
+ case TSR_KIND_PERCPU_POINTER:
pr_info(" pointer");
/* it also prints the type info */
break;
@@ -591,7 +591,7 @@ void set_stack_state(struct type_state_stack *stack, int offset, u8 kind,
switch (tag) {
case DW_TAG_structure_type:
case DW_TAG_union_type:
- stack->compound = (kind != TSR_KIND_POINTER);
+ stack->compound = (kind != TSR_KIND_PERCPU_POINTER);
break;
default:
stack->compound = false;
@@ -1116,7 +1116,7 @@ static enum type_match_result check_matching_type(struct type_state *state,
return PERF_TMR_OK;
}
- if (state->regs[reg].kind == TSR_KIND_POINTER) {
+ 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 541fee1a5f0a..dd3807b55208 100644
--- a/tools/perf/util/annotate-data.h
+++ b/tools/perf/util/annotate-data.h
@@ -34,7 +34,7 @@ enum type_state_kind {
TSR_KIND_TYPE,
TSR_KIND_PERCPU_BASE,
TSR_KIND_CONST,
- TSR_KIND_POINTER,
+ TSR_KIND_PERCPU_POINTER,
TSR_KIND_CANARY,
};
--
2.51.0.384.g4c02a37b29-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 03/10] perf annotate: Track address registers via TSR_KIND_POINTER
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-09-17 19:58 ` [PATCH v3 02/10] perf annotate: Rename TSR_KIND_POINTER to TSR_KIND_PERCPU_POINTER Zecheng Li
@ 2025-09-17 19:58 ` 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
` (6 subsequent siblings)
9 siblings, 1 reply; 18+ messages in thread
From: Zecheng Li @ 2025-09-17 19:58 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Liang, Kan, Masami Hiramatsu
Cc: Xu Liu, linux-perf-users, linux-kernel, Zecheng Li
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@google.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) {
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;
+ 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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 04/10] perf annotate: Track arithmetic instructions on pointers
2025-09-17 19:57 [PATCH v3 00/10] perf tools: Some improvements on data type profiler Zecheng Li
` (2 preceding siblings ...)
2025-09-17 19:58 ` [PATCH v3 03/10] perf annotate: Track address registers via TSR_KIND_POINTER Zecheng Li
@ 2025-09-17 19:58 ` 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
` (5 subsequent siblings)
9 siblings, 1 reply; 18+ messages in thread
From: Zecheng Li @ 2025-09-17 19:58 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Liang, Kan, Masami Hiramatsu
Cc: Xu Liu, linux-perf-users, linux-kernel, Zecheng Li
Track the arithmetic operations on registers with pointer types. We
handle only add, sub 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.
Multi-register addressing modes in LEA are not supported.
Signed-off-by: Zecheng Li <zecheng@google.com>
---
tools/perf/arch/x86/annotate/instructions.c | 121 +++++++++++++++++++-
tools/perf/util/annotate-data.c | 17 ++-
tools/perf/util/annotate-data.h | 6 +
3 files changed, 136 insertions(+), 8 deletions(-)
diff --git a/tools/perf/arch/x86/annotate/instructions.c b/tools/perf/arch/x86/annotate/instructions.c
index 698cbb299c6d..cfb07cff8fc8 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,16 @@ static void update_insn_state_x86(struct type_state *state,
else
return;
+ /* Ignore add to non-pointer types like int */
+ if (tsr->kind == TSR_KIND_POINTER ||
+ (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 +314,7 @@ static void update_insn_state_x86(struct type_state *state,
*/
tsr->type = type_die;
tsr->kind = TSR_KIND_PERCPU_POINTER;
+ tsr->offset = 0;
tsr->ok = true;
pr_debug_dtp("add [%x] percpu %#"PRIx64" -> reg%d",
@@ -311,6 +324,93 @@ static void update_insn_state_x86(struct type_state *state,
return;
}
+ if (!strncmp(dl->ins.name, "sub", 3)) {
+ u64 imm_value = -1ULL;
+
+ if (!has_reg_type(state, dst->reg1))
+ return;
+
+ tsr = &state->regs[dst->reg1];
+ tsr->copied_from = -1;
+
+ if (src->imm)
+ imm_value = src->offset;
+ else if (has_reg_type(state, src->reg1) &&
+ state->regs[src->reg1].kind == TSR_KIND_CONST)
+ imm_value = state->regs[src->reg1].imm_value;
+
+ if (tsr->kind == TSR_KIND_POINTER ||
+ (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("sub [%x] offset %#"PRIx64" to reg%d",
+ insn_offset, imm_value, dst->reg1);
+ pr_debug_type_name(&tsr->type, tsr->kind);
+ }
+ }
+
+ if (!strncmp(dl->ins.name, "lea", 3)) {
+ int sreg = src->reg1;
+ struct type_state_reg src_tsr;
+
+ 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;
+
+ tsr->type = stack->type;
+ tsr->kind = TSR_KIND_POINTER;
+ tsr->offset = offset - stack->offset;
+ tsr->ok = true;
+
+ if (sreg == fbreg) {
+ pr_debug_dtp("lea [%x] address of -%#x(stack) -> reg%d",
+ insn_offset, -src->offset, dst->reg1);
+ } else {
+ pr_debug_dtp("lea [%x] address of %#x(reg%d) -> reg%d",
+ insn_offset, src->offset, sreg, dst->reg1);
+ }
+
+ pr_debug_type_name(&tsr->type, tsr->kind);
+ }
+ /* Case 2: Based on a register holding a typed pointer */
+ else if (src_tsr.ok && src_tsr.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) == NULL)
+ 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] address of %s%#x(reg%d) -> reg%d",
+ insn_offset, src->offset < 0 ? "-" : "",
+ abs(src->offset), sreg, dst->reg1);
+
+ pr_debug_type_name(&tsr->type, tsr->kind);
+ }
+ return;
+ }
+
if (strncmp(dl->ins.name, "mov", 3))
return;
@@ -345,6 +445,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 +462,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 +474,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 +491,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 = state->regs[src->reg1].offset;
tsr->ok = true;
/* To copy back the variable type later (hopefully) */
@@ -421,16 +525,19 @@ 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->ok = true;
} 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;
@@ -450,9 +557,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",
@@ -463,9 +571,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_POINTER &&
die_get_member_type(&state->regs[sreg].type,
- src->offset, &type_die)) {
+ src->offset + state->regs[sreg].offset, &type_die)) {
tsr->type = state->regs[sreg].type;
tsr->kind = TSR_KIND_TYPE;
+ tsr->offset = src->offset + state->regs[sreg].offset;
tsr->ok = true;
pr_debug_dtp("mov [%x] addr %#x(reg%d) -> reg%d",
@@ -490,6 +599,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",
@@ -521,6 +631,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) {
@@ -543,6 +654,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",
@@ -565,6 +677,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",
@@ -613,6 +726,10 @@ 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);
}
+ if (tsr->offset != 0)
+ pr_debug_dtp(" reg%d offset %#x ->",
+ src->reg1, tsr->offset);
+
pr_debug_type_name(&tsr->type, tsr->kind);
}
/*
diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index 31b5896276f1..6ca5489f3c4c 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -898,7 +898,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;
@@ -914,6 +914,7 @@ static void update_var_state(struct type_state *state, struct data_loc_info *dlo
!is_better_type(®->type, &mem_die))
continue;
+ reg->offset = -var->offset;
reg->type = mem_die;
reg->kind = TSR_KIND_POINTER;
reg->ok = true;
@@ -925,13 +926,17 @@ static void update_var_state(struct type_state *state, struct data_loc_info *dlo
}
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 offset %x",
+ insn_offset, var->reg, var->offset);
pr_debug_type_name(&mem_die, TSR_KIND_TYPE);
/*
@@ -1119,7 +1124,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);
@@ -1148,7 +1153,7 @@ static enum type_match_result check_matching_type(struct type_state *state,
*/
*type_die = state->regs[reg].type;
- dloc->type_offset = dloc->op->offset;
+ dloc->type_offset = dloc->op->offset + state->regs[reg].offset;
/* Get the size of the actual type */
if (dwarf_aggregate_size(type_die, &size) < 0 ||
diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h
index fd0d1084bc4e..20237e7e4e2f 100644
--- a/tools/perf/util/annotate-data.h
+++ b/tools/perf/util/annotate-data.h
@@ -174,6 +174,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.51.0.384.g4c02a37b29-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 05/10] perf annotate: Save pointer offset in stack state
2025-09-17 19:57 [PATCH v3 00/10] perf tools: Some improvements on data type profiler Zecheng Li
` (3 preceding siblings ...)
2025-09-17 19:58 ` [PATCH v3 04/10] perf annotate: Track arithmetic instructions on pointers Zecheng Li
@ 2025-09-17 19:58 ` Zecheng Li
2025-10-04 7:59 ` Namhyung Kim
2025-09-17 19:58 ` [PATCH v3 06/10] perf annotate: Invalidate register states for untracked instructions Zecheng Li
` (4 subsequent siblings)
9 siblings, 1 reply; 18+ messages in thread
From: Zecheng Li @ 2025-09-17 19:58 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Liang, Kan, Masami Hiramatsu
Cc: Xu Liu, linux-perf-users, linux-kernel, Zecheng Li
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 06/10] perf annotate: Invalidate register states for untracked instructions
2025-09-17 19:57 [PATCH v3 00/10] perf tools: Some improvements on data type profiler Zecheng Li
` (4 preceding siblings ...)
2025-09-17 19:58 ` [PATCH v3 05/10] perf annotate: Save pointer offset in stack state Zecheng Li
@ 2025-09-17 19:58 ` 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
` (3 subsequent siblings)
9 siblings, 1 reply; 18+ messages in thread
From: Zecheng Li @ 2025-09-17 19:58 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Liang, Kan, Masami Hiramatsu
Cc: Xu Liu, linux-perf-users, linux-kernel, Zecheng Li
When tracking variable types, instructions that modify a pointer value
in an untracked way can lead to incorrect type propagation. To prevent
this, invalidate the register state when encountering such instructions.
This change invalidates pointer types for various arithmetic and bitwise
operations that current pointer offset tracking doesn't support, like
imul, shl, and, inc, etc.
A special case is added for 'xor reg, reg', which is a common idiom for
zeroing a register. For this, the register state is updated to be a
constant with a value of 0.
This could introduce slight regressions if a variable is zeroed and then
reused. This can be addressed in the future by using all DWARF locations
for instruction tracking instead of only the first one.
Signed-off-by: Zecheng Li <zecheng@google.com>
---
tools/perf/arch/x86/annotate/instructions.c | 29 +++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/tools/perf/arch/x86/annotate/instructions.c b/tools/perf/arch/x86/annotate/instructions.c
index 709c6f7efe82..3c98f72c423f 100644
--- a/tools/perf/arch/x86/annotate/instructions.c
+++ b/tools/perf/arch/x86/annotate/instructions.c
@@ -411,6 +411,35 @@ static void update_insn_state_x86(struct type_state *state,
return;
}
+ /* Invalidate register states for other ops which may change pointers */
+ if (has_reg_type(state, dst->reg1) && !dst->mem_ref &&
+ dwarf_tag(&state->regs[dst->reg1].type) == DW_TAG_pointer_type) {
+ if (!strncmp(dl->ins.name, "imul", 4) || !strncmp(dl->ins.name, "mul", 3) ||
+ !strncmp(dl->ins.name, "idiv", 4) || !strncmp(dl->ins.name, "div", 3) ||
+ !strncmp(dl->ins.name, "shl", 3) || !strncmp(dl->ins.name, "shr", 3) ||
+ !strncmp(dl->ins.name, "sar", 3) || !strncmp(dl->ins.name, "and", 3) ||
+ !strncmp(dl->ins.name, "or", 2) || !strncmp(dl->ins.name, "neg", 3) ||
+ !strncmp(dl->ins.name, "inc", 3) || !strncmp(dl->ins.name, "dec", 3)) {
+ pr_debug_dtp("%s [%x] invalidate reg%d\n",
+ dl->ins.name, insn_offset, dst->reg1);
+ state->regs[dst->reg1].ok = false;
+ state->regs[dst->reg1].copied_from = -1;
+ return;
+ }
+
+ if (!strncmp(dl->ins.name, "xor", 3) && dst->reg1 == src->reg1) {
+ /* xor reg, reg clears the register */
+ pr_debug_dtp("xor [%x] clear reg%d\n",
+ insn_offset, dst->reg1);
+
+ state->regs[dst->reg1].kind = TSR_KIND_CONST;
+ state->regs[dst->reg1].imm_value = 0;
+ state->regs[dst->reg1].ok = false;
+ state->regs[dst->reg1].copied_from = -1;
+ return;
+ }
+ }
+
if (strncmp(dl->ins.name, "mov", 3))
return;
--
2.51.0.384.g4c02a37b29-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 07/10] perf dwarf-aux: Skip check_variable for die_find_variable_by_reg
2025-09-17 19:57 [PATCH v3 00/10] perf tools: Some improvements on data type profiler Zecheng Li
` (5 preceding siblings ...)
2025-09-17 19:58 ` [PATCH v3 06/10] perf annotate: Invalidate register states for untracked instructions Zecheng Li
@ 2025-09-17 19:58 ` Zecheng Li
2025-09-17 19:58 ` [PATCH v3 08/10] perf dwarf-aux: Preserve typedefs in match_var_offset Zecheng Li
` (2 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Zecheng Li @ 2025-09-17 19:58 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Liang, Kan, Masami Hiramatsu
Cc: Xu Liu, linux-perf-users, linux-kernel, Zecheng Li
In die_find_variable_by_reg, match_var_offset already performs
sufficient checking and type matching. The additional check_variable
call is redundant, and its need_pointer logic is only a heuristic. Since
DWARF encodes accurate type information, which match_var_offset
verifies, skipping check_variable improves both coverage and accuracy.
Return type from die_find_variable_by_reg via a new `type` field in
find_var_data.
Signed-off-by: Zecheng Li <zecheng@google.com>
---
tools/perf/util/annotate-data.c | 8 +++++---
tools/perf/util/dwarf-aux.c | 18 +++++++++++-------
tools/perf/util/dwarf-aux.h | 2 +-
3 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index 68c69d343bff..7e4c045d0f4d 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -1596,19 +1596,21 @@ static int find_data_type_die(struct data_loc_info *dloc, Dwarf_Die *type_die)
if (!die_find_variable_by_addr(&scopes[i], dloc->var_addr,
&var_die, &type_offset))
continue;
+ /* Found a variable, see if it's correct */
+ result = check_variable(dloc, &var_die, &mem_die, reg,
+ type_offset, is_fbreg);
} else {
/* Look up variables/parameters in this scope */
if (!die_find_variable_by_reg(&scopes[i], pc, reg,
- &type_offset, is_fbreg, &var_die))
+ &mem_die, &type_offset, is_fbreg, &var_die))
continue;
+ result = PERF_TMR_OK;
}
pr_debug_dtp("found \"%s\" (die: %#lx) in scope=%d/%d (die: %#lx) ",
dwarf_diename(&var_die), (long)dwarf_dieoffset(&var_die),
i+1, nr_scopes, (long)dwarf_dieoffset(&scopes[i]));
- /* Found a variable, see if it's correct */
- result = check_variable(dloc, &var_die, &mem_die, reg, type_offset, is_fbreg);
if (result == PERF_TMR_OK) {
if (reg == DWARF_REG_PC) {
pr_debug_dtp("addr=%#"PRIx64" type_offset=%#x\n",
diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 9267af204c7d..b57cdc8860f0 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -1378,6 +1378,8 @@ struct find_var_data {
Dwarf_Addr addr;
/* Target register */
unsigned reg;
+ /* Access data type */
+ Dwarf_Die type;
/* Access offset, set for global data */
int offset;
/* True if the current register is the frame base */
@@ -1390,7 +1392,6 @@ struct find_var_data {
static bool match_var_offset(Dwarf_Die *die_mem, struct find_var_data *data,
s64 addr_offset, s64 addr_type, bool is_pointer)
{
- Dwarf_Die type_die;
Dwarf_Word size;
s64 offset = addr_offset - addr_type;
@@ -1403,16 +1404,16 @@ static bool match_var_offset(Dwarf_Die *die_mem, struct find_var_data *data,
if (offset < 0)
return false;
- if (die_get_real_type(die_mem, &type_die) == NULL)
+ if (die_get_real_type(die_mem, &data->type) == NULL)
return false;
- if (is_pointer && dwarf_tag(&type_die) == DW_TAG_pointer_type) {
+ if (is_pointer && dwarf_tag(&data->type) == DW_TAG_pointer_type) {
/* Get the target type of the pointer */
- if (die_get_real_type(&type_die, &type_die) == NULL)
+ if (die_get_real_type(&data->type, &data->type) == NULL)
return false;
}
- if (dwarf_aggregate_size(&type_die, &size) < 0)
+ if (dwarf_aggregate_size(&data->type, &size) < 0)
return false;
if ((u64)offset >= size)
@@ -1529,7 +1530,7 @@ static int __die_find_var_reg_cb(Dwarf_Die *die_mem, void *arg)
* when the variable is in the stack.
*/
Dwarf_Die *die_find_variable_by_reg(Dwarf_Die *sc_die, Dwarf_Addr pc, int reg,
- int *poffset, bool is_fbreg,
+ Dwarf_Die *type_die, int *poffset, bool is_fbreg,
Dwarf_Die *die_mem)
{
struct find_var_data data = {
@@ -1541,8 +1542,11 @@ Dwarf_Die *die_find_variable_by_reg(Dwarf_Die *sc_die, Dwarf_Addr pc, int reg,
Dwarf_Die *result;
result = die_find_child(sc_die, __die_find_var_reg_cb, &data, die_mem);
- if (result)
+ if (result) {
*poffset = data.offset;
+ *type_die = data.type;
+ }
+
return result;
}
diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
index cd481ec9c5a1..b3ee5df0b6be 100644
--- a/tools/perf/util/dwarf-aux.h
+++ b/tools/perf/util/dwarf-aux.h
@@ -163,7 +163,7 @@ int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf);
/* Find a variable saved in the 'reg' at given address */
Dwarf_Die *die_find_variable_by_reg(Dwarf_Die *sc_die, Dwarf_Addr pc, int reg,
- int *poffset, bool is_fbreg,
+ Dwarf_Die *type_die, int *poffset, bool is_fbreg,
Dwarf_Die *die_mem);
/* Find a (global) variable located in the 'addr' */
--
2.51.0.384.g4c02a37b29-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 08/10] perf dwarf-aux: Preserve typedefs in match_var_offset
2025-09-17 19:57 [PATCH v3 00/10] perf tools: Some improvements on data type profiler Zecheng Li
` (6 preceding siblings ...)
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 ` 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
9 siblings, 0 replies; 18+ messages in thread
From: Zecheng Li @ 2025-09-17 19:58 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Liang, Kan, Masami Hiramatsu
Cc: Xu Liu, linux-perf-users, linux-kernel, Zecheng Li
Since we are skipping the check_variable, we need to preserve typedefs
in match_var_offset to match the results by __die_get_real_type. Also
move the (offset == 0) branch after the is_pointer check to ensure the
correct type is used, fixing cases where an incorrect pointer type was
chosen when the access offset was 0.
Signed-off-by: Zecheng Li <zecheng@google.com>
---
tools/perf/util/dwarf-aux.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index b57cdc8860f0..b2189de07daf 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -1395,24 +1395,24 @@ static bool match_var_offset(Dwarf_Die *die_mem, struct find_var_data *data,
Dwarf_Word size;
s64 offset = addr_offset - addr_type;
- if (offset == 0) {
- /* Update offset relative to the start of the variable */
- data->offset = 0;
- return true;
- }
-
if (offset < 0)
return false;
- if (die_get_real_type(die_mem, &data->type) == NULL)
+ if (__die_get_real_type(die_mem, &data->type) == NULL)
return false;
if (is_pointer && dwarf_tag(&data->type) == DW_TAG_pointer_type) {
/* Get the target type of the pointer */
- if (die_get_real_type(&data->type, &data->type) == NULL)
+ if (__die_get_real_type(&data->type, &data->type) == NULL)
return false;
}
+ if (offset == 0) {
+ /* Update offset relative to the start of the variable */
+ data->offset = 0;
+ return true;
+ }
+
if (dwarf_aggregate_size(&data->type, &size) < 0)
return false;
--
2.51.0.384.g4c02a37b29-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 09/10] perf annotate: Improve type comparison from different scopes
2025-09-17 19:57 [PATCH v3 00/10] perf tools: Some improvements on data type profiler Zecheng Li
` (7 preceding siblings ...)
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 ` Zecheng Li
2025-09-17 19:58 ` [PATCH v3 10/10] perf dwarf-aux: Support DW_OP_piece expressions Zecheng Li
9 siblings, 0 replies; 18+ messages in thread
From: Zecheng Li @ 2025-09-17 19:58 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Liang, Kan, Masami Hiramatsu
Cc: Xu Liu, linux-perf-users, linux-kernel, Zecheng Li
When comparing types from different scopes, first compare their type
offsets. A larger offset means the field belongs to an outer
(enclosing) struct. This helps resolve cases where a pointer is found
in an inner scope, but a struct containing that pointer exists in an
outer scope. Previously, is_better_type would prefer the pointer type,
but the struct type is actually more complete and should be chosen.
Prefer types from outer scopes when is_better_type cannot determine
a better type. This sometimes helps pick a more complete type.
Signed-off-by: Zecheng Li <zecheng@google.com>
---
tools/perf/util/annotate-data.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index 7e4c045d0f4d..51765bd36c47 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -1622,7 +1622,9 @@ static int find_data_type_die(struct data_loc_info *dloc, Dwarf_Die *type_die)
pr_debug_dtp("type_offset=%#x\n", type_offset);
}
- if (!found || is_better_type(type_die, &mem_die)) {
+ if (!found || dloc->type_offset < type_offset ||
+ (dloc->type_offset == type_offset &&
+ !is_better_type(&mem_die, type_die))) {
*type_die = mem_die;
dloc->type_offset = type_offset;
found = true;
--
2.51.0.384.g4c02a37b29-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 10/10] perf dwarf-aux: Support DW_OP_piece expressions
2025-09-17 19:57 [PATCH v3 00/10] perf tools: Some improvements on data type profiler Zecheng Li
` (8 preceding siblings ...)
2025-09-17 19:58 ` [PATCH v3 09/10] perf annotate: Improve type comparison from different scopes Zecheng Li
@ 2025-09-17 19:58 ` Zecheng Li
9 siblings, 0 replies; 18+ messages in thread
From: Zecheng Li @ 2025-09-17 19:58 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Liang, Kan, Masami Hiramatsu
Cc: Xu Liu, linux-perf-users, linux-kernel, Zecheng Li
Support variables split across multiple registers or stack locations by
handling DW_OP_piece in DWARF expressions. This enables correct matching
of such variables by iterating over all pieces in the expression.
There are two cases for matching memory access on the target register:
1. Accessing a struct member:
- The type is the original variable's type.
- The offset is the sum of the piece's offset and the operand's
offset.
2. Dereferencing a member:
- The type is the member of the original variable (the member must be
a pointer).
- The size must match the piece size.
- The access offset is the operand's offset.
This change improves support for piece-wise variable locations in DWARF
expressions.
Signed-off-by: Zecheng Li <zecheng@google.com>
---
tools/perf/util/dwarf-aux.c | 244 +++++++++++++++++++++++++++---------
1 file changed, 187 insertions(+), 57 deletions(-)
diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index b2189de07daf..aae15a2cb7d0 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -1390,21 +1390,44 @@ struct find_var_data {
#define DWARF_OP_DIRECT_REGS 32
static bool match_var_offset(Dwarf_Die *die_mem, struct find_var_data *data,
- s64 addr_offset, s64 addr_type, bool is_pointer)
+ s64 addr_offset, s64 addr_type, s64 piece_offset,
+ s64 piece_size, bool is_pointer)
{
- Dwarf_Word size;
+ Dwarf_Word size = 0;
s64 offset = addr_offset - addr_type;
- if (offset < 0)
+ if (piece_size == 0 || offset < 0)
return false;
+ if (piece_size > 0 && !is_pointer) {
+ offset += piece_offset;
+ size = piece_offset + piece_size;
+ }
+
if (__die_get_real_type(die_mem, &data->type) == NULL)
return false;
- if (is_pointer && dwarf_tag(&data->type) == DW_TAG_pointer_type) {
- /* Get the target type of the pointer */
- if (__die_get_real_type(&data->type, &data->type) == NULL)
- return false;
+ if (is_pointer) {
+ if (piece_size < 0 && dwarf_tag(&data->type) == DW_TAG_pointer_type) {
+ /* Get the target type of the pointer */
+ if (__die_get_real_type(&data->type, &data->type) == NULL)
+ return false;
+ }
+
+ if (piece_size > 0) {
+ Dwarf_Die member_die;
+
+ if (die_get_member_type(&data->type, piece_offset, &member_die) == NULL)
+ return false;
+
+ if (dwarf_aggregate_size(&member_die, &size) < 0)
+ return false;
+
+ if (size == (u64)piece_size &&
+ dwarf_tag(&data->type) == DW_TAG_pointer_type)
+ if (__die_get_real_type(&member_die, &data->type) == NULL)
+ return false;
+ }
}
if (offset == 0) {
@@ -1413,7 +1436,7 @@ static bool match_var_offset(Dwarf_Die *die_mem, struct find_var_data *data,
return true;
}
- if (dwarf_aggregate_size(&data->type, &size) < 0)
+ if (size == 0 && dwarf_aggregate_size(&data->type, &size) < 0)
return false;
if ((u64)offset >= size)
@@ -1452,6 +1475,67 @@ static bool is_breg_access_indirect(Dwarf_Op *ops, size_t nops)
return false;
}
+struct op_piece_iter {
+ /* Pointer to the beginning of the op array */
+ Dwarf_Op *ops;
+ size_t nops;
+ /* The index where the next search will begin */
+ size_t current_idx;
+ size_t next_offset;
+
+ /* Pointer to the start of the current piece's ops */
+ Dwarf_Op *piece_ops;
+ /* The number of ops in the current piece */
+ size_t num_piece_ops;
+ size_t piece_offset;
+};
+
+static void op_piece_iter_init(struct op_piece_iter *it, Dwarf_Op *ops, size_t nops)
+{
+ it->ops = ops;
+ it->nops = nops;
+ it->current_idx = 0;
+ it->next_offset = 0;
+ it->piece_ops = NULL;
+ it->num_piece_ops = 0;
+ it->piece_offset = 0;
+}
+
+/* Finds the next non-empty piece segment. */
+static bool op_piece_iter_next(struct op_piece_iter *it)
+{
+ /* Loop until a non-empty piece is found */
+ while (it->current_idx < it->nops) {
+ size_t start;
+ size_t end;
+
+ start = it->current_idx;
+ end = start;
+
+ while (end < it->nops && it->ops[end].atom != DW_OP_piece)
+ end++;
+
+ /* The number of ops in this segment, including DW_OP_piece */
+ it->num_piece_ops = min(end - start + 1, it->nops - start);
+ it->piece_ops = &it->ops[start];
+ it->piece_offset = it->next_offset;
+
+ it->current_idx = end;
+ if (it->current_idx < it->nops) {
+ const Dwarf_Op *piece_op = &it->ops[it->current_idx];
+ size_t piece_size = (size_t)piece_op->number;
+
+ it->next_offset += piece_size;
+ it->current_idx++;
+ }
+
+ if (end > start)
+ return true;
+ }
+
+ return false;
+}
+
/* Only checks direct child DIEs in the given scope. */
static int __die_find_var_reg_cb(Dwarf_Die *die_mem, void *arg)
{
@@ -1470,48 +1554,65 @@ static int __die_find_var_reg_cb(Dwarf_Die *die_mem, void *arg)
return DIE_FIND_CB_SIBLING;
while ((off = dwarf_getlocations(&attr, off, &base, &start, &end, &ops, &nops)) > 0) {
+ struct op_piece_iter piece_iter;
/* Assuming the location list is sorted by address */
if (end <= data->pc)
continue;
if (start > data->pc)
break;
- /* Local variables accessed using frame base register */
- 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_breg_access_indirect(ops, nops)))
- return DIE_FIND_CB_END;
+ op_piece_iter_init(&piece_iter, ops, nops);
+ while (op_piece_iter_next(&piece_iter)) {
+ Dwarf_Op *pops = piece_iter.piece_ops;
+ size_t pnops = piece_iter.num_piece_ops;
+ size_t piece_offset = piece_iter.piece_offset;
+ int piece_size = -1;
+ bool is_pointer = true;
+ int access_offset = data->offset;
- /* Only match with a simple case */
- if (data->reg < DWARF_OP_DIRECT_REGS) {
- /* pointer variables saved in a register 0 to 31 */
- if (ops->atom == (DW_OP_reg0 + data->reg) &&
- check_allowed_ops(ops, nops) &&
- match_var_offset(die_mem, data, data->offset, 0,
- /*is_pointer=*/true))
- return DIE_FIND_CB_END;
+ if (pops[pnops - 1].atom == DW_OP_piece)
+ piece_size = (int)pops[pnops - 1].number;
- /* 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_breg_access_indirect(ops, nops)))
- return DIE_FIND_CB_END;
- } else {
- /* pointer variables saved in a register 32 or above */
- if (ops->atom == DW_OP_regx && ops->number == data->reg &&
- check_allowed_ops(ops, nops) &&
- match_var_offset(die_mem, data, data->offset, 0,
- /*is_pointer=*/true))
- return DIE_FIND_CB_END;
+ if (!check_allowed_ops(pops, pnops))
+ continue;
- /* 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_breg_access_indirect(ops, nops)))
+ if ((data->is_fbreg && pops->atom == DW_OP_fbreg) ||
+ (pops->atom == DW_OP_breg0 + data->reg) ||
+ (pops->atom == DW_OP_bregx && data->reg == pops->number))
+ is_pointer = is_breg_access_indirect(pops, pnops);
+
+ /* Local variables accessed using frame base register */
+ if (data->is_fbreg && pops->atom == DW_OP_fbreg &&
+ match_var_offset(die_mem, data, access_offset,
+ pops->number, piece_offset, piece_size, is_pointer))
return DIE_FIND_CB_END;
+
+ /* Only match with a simple case */
+ if (data->reg < DWARF_OP_DIRECT_REGS) {
+ /* pointer variables saved in a register 0 to 31 */
+ if (pops->atom == (DW_OP_reg0 + data->reg) &&
+ match_var_offset(die_mem, data, access_offset,
+ 0, piece_offset, piece_size, is_pointer))
+ return DIE_FIND_CB_END;
+
+ /* variables accessed by a register + offset */
+ if (pops->atom == (DW_OP_breg0 + data->reg) &&
+ match_var_offset(die_mem, data, access_offset,
+ pops->number, piece_offset, piece_size, is_pointer))
+ return DIE_FIND_CB_END;
+ } else {
+ /* pointer variables saved in a register 32 or above */
+ if (pops->atom == DW_OP_regx && pops->number == data->reg &&
+ match_var_offset(die_mem, data, access_offset,
+ 0, piece_offset, piece_size, is_pointer))
+ return DIE_FIND_CB_END;
+
+ /* variables accessed by a register + offset */
+ if (pops->atom == DW_OP_bregx && data->reg == pops->number &&
+ match_var_offset(die_mem, data, access_offset,
+ pops->number2, piece_offset, piece_size, is_pointer))
+ return DIE_FIND_CB_END;
+ }
}
}
return DIE_FIND_CB_SIBLING;
@@ -1572,7 +1673,7 @@ static int __die_find_var_addr_cb(Dwarf_Die *die_mem, void *arg)
continue;
if (check_allowed_ops(ops, nops) &&
- match_var_offset(die_mem, data, data->addr, ops->number,
+ match_var_offset(die_mem, data, data->addr, ops->number, 0, -1,
/*is_pointer=*/false))
return DIE_FIND_CB_END;
}
@@ -1613,6 +1714,7 @@ static int __die_collect_vars_cb(Dwarf_Die *die_mem, void *arg)
Dwarf_Op *ops;
size_t nops;
struct die_var_type *vt;
+ struct op_piece_iter piece_iter;
if (tag != DW_TAG_variable && tag != DW_TAG_formal_parameter)
return DIE_FIND_CB_SIBLING;
@@ -1634,25 +1736,53 @@ 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;
- vt = malloc(sizeof(*vt));
- if (vt == NULL)
- return DIE_FIND_CB_END;
+ op_piece_iter_init(&piece_iter, ops, nops);
+ while (op_piece_iter_next(&piece_iter)) {
+ Dwarf_Op *pops = piece_iter.ops;
+ size_t pnops = piece_iter.num_piece_ops;
+ size_t piece_offset = piece_iter.piece_offset;
+ size_t offset = offset_from_dwarf_op(pops);
+ s64 piece_size = -1;
+ /* Usually a register holds the value of the variable */
+ bool is_reg_var_addr = false;
+
+ if (((pops->atom >= DW_OP_breg0 && pops->atom <= DW_OP_breg31) ||
+ pops->atom == DW_OP_bregx || pops->atom == DW_OP_fbreg) &&
+ !is_breg_access_indirect(pops, pnops))
+ /* The register holds the address of the variable. */
+ is_reg_var_addr = true;
+
+ if (pops[pnops - 1].atom == DW_OP_piece)
+ piece_size = (s64)pops[pnops - 1].number;
+
+ if (piece_size > 0) {
+ if (!is_reg_var_addr) {
+ size_t size;
+
+ if (die_get_member_type(&type_die, piece_offset, &type_die) == NULL)
+ continue;
- /* Usually a register holds the value of a variable */
- vt->is_reg_var_addr = false;
+ if (dwarf_aggregate_size(&type_die, &size) < 0)
+ continue;
- if (((ops->atom >= DW_OP_breg0 && ops->atom <= DW_OP_breg31) ||
- ops->atom == DW_OP_bregx || ops->atom == DW_OP_fbreg) &&
- !is_breg_access_indirect(ops, nops))
- /* The register contains an address of the variable. */
- vt->is_reg_var_addr = true;
+ if (size != (u64)piece_size)
+ continue;
+ } else
+ offset += piece_offset;
+ }
- vt->die_off = dwarf_dieoffset(&type_die);
- vt->addr = start;
- vt->reg = reg_from_dwarf_op(ops);
- vt->offset = offset_from_dwarf_op(ops);
- vt->next = *var_types;
- *var_types = vt;
+ vt = malloc(sizeof(*vt));
+ if (vt == NULL)
+ return DIE_FIND_CB_END;
+
+ vt->is_reg_var_addr = is_reg_var_addr;
+ vt->die_off = dwarf_dieoffset(&type_die);
+ vt->addr = start;
+ vt->reg = reg_from_dwarf_op(pops);
+ vt->offset = offset;
+ vt->next = *var_types;
+ *var_types = vt;
+ }
return DIE_FIND_CB_SIBLING;
}
--
2.51.0.384.g4c02a37b29-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3 01/10] perf annotate: Skip annotating data types to lea instructions
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
0 siblings, 0 replies; 18+ messages in thread
From: Namhyung Kim @ 2025-10-03 5:36 UTC (permalink / raw)
To: Zecheng Li
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
Adrian Hunter, Liang, Kan, Masami Hiramatsu, Xu Liu,
linux-perf-users, linux-kernel
Hello,
On Wed, Sep 17, 2025 at 07:57:59PM +0000, Zecheng Li wrote:
> Introduce a helper function is_address_gen_insn() to check
> arch-dependent address generation instructions like lea in x86. Remove
> type annotation on these 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 | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index c9b220d9f924..e2370b7fd599 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -2699,6 +2699,19 @@ static bool is_stack_canary(struct arch *arch, struct annotated_op_loc *loc)
> return false;
> }
>
> +/**
> + * Returns true if the instruction has a memory operand without
> + * performing a load/store
> + */
> +static bool is_address_gen_insn(struct arch *arch, struct disasm_line *dl)
> +{
> + if (arch__is(arch, "x86"))
Please put parentheses if the inner statement is placed on multiple
lines even if it's a single statement.
Otherwise, looks good to me.
Thanks,
Namhyung
> + if (!strncmp(dl->ins.name, "lea", 3))
> + return true;
> +
> + return false;
> +}
> +
> static struct disasm_line *
> annotation__prev_asm_line(struct annotation *notes, struct disasm_line *curr)
> {
> @@ -2807,6 +2820,12 @@ __hist_entry__get_data_type(struct hist_entry *he, struct arch *arch,
> return &stackop_type;
> }
>
> + if (is_address_gen_insn(arch, dl)) {
> + istat->bad++;
> + ann_data_stat.no_mem_ops++;
> + return NO_TYPE;
> + }
> +
> for_each_insn_op_loc(&loc, i, op_loc) {
> struct data_loc_info dloc = {
> .arch = arch,
> --
> 2.51.0.384.g4c02a37b29-goog
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 02/10] perf annotate: Rename TSR_KIND_POINTER to TSR_KIND_PERCPU_POINTER
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
0 siblings, 1 reply; 18+ messages in thread
From: Namhyung Kim @ 2025-10-03 5:37 UTC (permalink / raw)
To: Zecheng Li
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
Adrian Hunter, Liang, Kan, Masami Hiramatsu, Xu Liu,
linux-perf-users, linux-kernel
On Wed, Sep 17, 2025 at 07:58:00PM +0000, Zecheng Li wrote:
> TSR_KIND_POINTER only represents percpu pointers currently. Rename it to
> TSR_KIND_PERCPU_POINTER so we can use the TSR_KIND_POINTER to represent
> pointer to a type.
>
> Signed-off-by: Zecheng Li <zecheng@google.com>
Reviewed-by: Namhyung Kim <namhyung@kernel.org>
Thanks,
Namhyung
> ---
> tools/perf/arch/x86/annotate/instructions.c | 4 ++--
> tools/perf/util/annotate-data.c | 6 +++---
> tools/perf/util/annotate-data.h | 2 +-
> 3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/arch/x86/annotate/instructions.c b/tools/perf/arch/x86/annotate/instructions.c
> index c6d403eae744..da98a4e3c52c 100644
> --- a/tools/perf/arch/x86/annotate/instructions.c
> +++ b/tools/perf/arch/x86/annotate/instructions.c
> @@ -301,7 +301,7 @@ static void update_insn_state_x86(struct type_state *state,
> * as a pointer.
> */
> tsr->type = type_die;
> - tsr->kind = TSR_KIND_POINTER;
> + tsr->kind = TSR_KIND_PERCPU_POINTER;
> tsr->ok = true;
>
> pr_debug_dtp("add [%x] percpu %#"PRIx64" -> reg%d",
> @@ -521,7 +521,7 @@ static void update_insn_state_x86(struct type_state *state,
> }
> /* And then dereference the calculated pointer if it has one */
> else if (has_reg_type(state, sreg) && state->regs[sreg].ok &&
> - state->regs[sreg].kind == TSR_KIND_POINTER &&
> + state->regs[sreg].kind == TSR_KIND_PERCPU_POINTER &&
> die_get_member_type(&state->regs[sreg].type,
> src->offset, &type_die)) {
> tsr->type = type_die;
> diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> index 258157cc43c2..903027a6fb7d 100644
> --- a/tools/perf/util/annotate-data.c
> +++ b/tools/perf/util/annotate-data.c
> @@ -58,7 +58,7 @@ void pr_debug_type_name(Dwarf_Die *die, enum type_state_kind kind)
> case TSR_KIND_CONST:
> pr_info(" constant\n");
> return;
> - case TSR_KIND_POINTER:
> + case TSR_KIND_PERCPU_POINTER:
> pr_info(" pointer");
> /* it also prints the type info */
> break;
> @@ -591,7 +591,7 @@ void set_stack_state(struct type_state_stack *stack, int offset, u8 kind,
> switch (tag) {
> case DW_TAG_structure_type:
> case DW_TAG_union_type:
> - stack->compound = (kind != TSR_KIND_POINTER);
> + stack->compound = (kind != TSR_KIND_PERCPU_POINTER);
> break;
> default:
> stack->compound = false;
> @@ -1116,7 +1116,7 @@ static enum type_match_result check_matching_type(struct type_state *state,
> return PERF_TMR_OK;
> }
>
> - if (state->regs[reg].kind == TSR_KIND_POINTER) {
> + 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 541fee1a5f0a..dd3807b55208 100644
> --- a/tools/perf/util/annotate-data.h
> +++ b/tools/perf/util/annotate-data.h
> @@ -34,7 +34,7 @@ enum type_state_kind {
> TSR_KIND_TYPE,
> TSR_KIND_PERCPU_BASE,
> TSR_KIND_CONST,
> - TSR_KIND_POINTER,
> + TSR_KIND_PERCPU_POINTER,
> TSR_KIND_CANARY,
> };
>
> --
> 2.51.0.384.g4c02a37b29-goog
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 03/10] perf annotate: Track address registers via TSR_KIND_POINTER
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
0 siblings, 0 replies; 18+ messages in thread
From: Namhyung Kim @ 2025-10-03 5:52 UTC (permalink / raw)
To: Zecheng Li
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
Adrian Hunter, Liang, Kan, Masami Hiramatsu, Xu Liu,
linux-perf-users, linux-kernel
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@google.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
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 02/10] perf annotate: Rename TSR_KIND_POINTER to TSR_KIND_PERCPU_POINTER
2025-10-03 5:37 ` Namhyung Kim
@ 2025-10-03 19:10 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-10-03 19:10 UTC (permalink / raw)
To: Namhyung Kim
Cc: Zecheng Li, Peter Zijlstra, Ingo Molnar, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Liang, Kan, Masami Hiramatsu, Xu Liu, linux-perf-users,
linux-kernel
On Fri, Oct 03, 2025 at 02:37:11PM +0900, Namhyung Kim wrote:
> On Wed, Sep 17, 2025 at 07:58:00PM +0000, Zecheng Li wrote:
> > TSR_KIND_POINTER only represents percpu pointers currently. Rename it to
> > TSR_KIND_PERCPU_POINTER so we can use the TSR_KIND_POINTER to represent
> > pointer to a type.
> >
> > Signed-off-by: Zecheng Li <zecheng@google.com>
>
> Reviewed-by: Namhyung Kim <namhyung@kernel.org>
Cherry picked this one.
Thanks,
- Arnaldo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 04/10] perf annotate: Track arithmetic instructions on pointers
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
0 siblings, 0 replies; 18+ messages in thread
From: Namhyung Kim @ 2025-10-04 7:57 UTC (permalink / raw)
To: Zecheng Li
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
Adrian Hunter, Liang, Kan, Masami Hiramatsu, Xu Liu,
linux-perf-users, linux-kernel
On Wed, Sep 17, 2025 at 07:58:02PM +0000, Zecheng Li wrote:
> Track the arithmetic operations on registers with pointer types. We
> handle only add, sub 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.
>
> Multi-register addressing modes in LEA are not supported.
>
> Signed-off-by: Zecheng Li <zecheng@google.com>
> ---
> tools/perf/arch/x86/annotate/instructions.c | 121 +++++++++++++++++++-
> tools/perf/util/annotate-data.c | 17 ++-
> tools/perf/util/annotate-data.h | 6 +
> 3 files changed, 136 insertions(+), 8 deletions(-)
>
> diff --git a/tools/perf/arch/x86/annotate/instructions.c b/tools/perf/arch/x86/annotate/instructions.c
> index 698cbb299c6d..cfb07cff8fc8 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,16 @@ static void update_insn_state_x86(struct type_state *state,
> else
> return;
>
> + /* Ignore add to non-pointer types like int */
Probably we want to update TSR_KIND_CONST imm_value as well.
> + if (tsr->kind == TSR_KIND_POINTER ||
> + (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 +314,7 @@ static void update_insn_state_x86(struct type_state *state,
> */
> tsr->type = type_die;
> tsr->kind = TSR_KIND_PERCPU_POINTER;
> + tsr->offset = 0;
> tsr->ok = true;
>
> pr_debug_dtp("add [%x] percpu %#"PRIx64" -> reg%d",
> @@ -311,6 +324,93 @@ static void update_insn_state_x86(struct type_state *state,
> return;
> }
>
> + if (!strncmp(dl->ins.name, "sub", 3)) {
> + u64 imm_value = -1ULL;
> +
> + if (!has_reg_type(state, dst->reg1))
> + return;
> +
> + tsr = &state->regs[dst->reg1];
> + tsr->copied_from = -1;
> +
> + if (src->imm)
> + imm_value = src->offset;
> + else if (has_reg_type(state, src->reg1) &&
> + state->regs[src->reg1].kind == TSR_KIND_CONST)
> + imm_value = state->regs[src->reg1].imm_value;
> +
> + if (tsr->kind == TSR_KIND_POINTER ||
Ditto.
> + (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("sub [%x] offset %#"PRIx64" to reg%d",
> + insn_offset, imm_value, dst->reg1);
> + pr_debug_type_name(&tsr->type, tsr->kind);
> + }
> + }
> +
> + if (!strncmp(dl->ins.name, "lea", 3)) {
> + int sreg = src->reg1;
> + struct type_state_reg src_tsr;
> +
> + 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;
> +
> + tsr->type = stack->type;
> + tsr->kind = TSR_KIND_POINTER;
> + tsr->offset = offset - stack->offset;
> + tsr->ok = true;
> +
> + if (sreg == fbreg) {
> + pr_debug_dtp("lea [%x] address of -%#x(stack) -> reg%d",
> + insn_offset, -src->offset, dst->reg1);
> + } else {
> + pr_debug_dtp("lea [%x] address of %#x(reg%d) -> reg%d",
> + insn_offset, src->offset, sreg, dst->reg1);
> + }
> +
> + pr_debug_type_name(&tsr->type, tsr->kind);
> + }
> + /* Case 2: Based on a register holding a typed pointer */
> + else if (src_tsr.ok && src_tsr.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) == NULL)
> + return;
> +
> + tsr->type = src_tsr.type;
> + tsr->kind = TSR_KIND_TYPE;
Did you mean TSR_KIND_POINTER?
> + tsr->offset = src->offset + src_tsr.offset;
> + tsr->ok = true;
> +
> + pr_debug_dtp("lea [%x] address of %s%#x(reg%d) -> reg%d",
> + insn_offset, src->offset < 0 ? "-" : "",
> + abs(src->offset), sreg, dst->reg1);
> +
> + pr_debug_type_name(&tsr->type, tsr->kind);
> + }
> + return;
> + }
> +
> if (strncmp(dl->ins.name, "mov", 3))
> return;
>
> @@ -345,6 +445,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 +462,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 +474,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 +491,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 = state->regs[src->reg1].offset;
> tsr->ok = true;
>
> /* To copy back the variable type later (hopefully) */
> @@ -421,16 +525,19 @@ 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->ok = true;
> } 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;
> @@ -450,9 +557,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",
> @@ -463,9 +571,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_POINTER &&
> die_get_member_type(&state->regs[sreg].type,
> - src->offset, &type_die)) {
> + src->offset + state->regs[sreg].offset, &type_die)) {
> tsr->type = state->regs[sreg].type;
> tsr->kind = TSR_KIND_TYPE;
> + tsr->offset = src->offset + state->regs[sreg].offset;
> tsr->ok = true;
>
> pr_debug_dtp("mov [%x] addr %#x(reg%d) -> reg%d",
> @@ -490,6 +599,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",
> @@ -521,6 +631,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) {
> @@ -543,6 +654,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",
> @@ -565,6 +677,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",
> @@ -613,6 +726,10 @@ 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);
> }
> + if (tsr->offset != 0)
Parentheses please.
Thanks,
Namhyung
> + pr_debug_dtp(" reg%d offset %#x ->",
> + src->reg1, tsr->offset);
> +
> pr_debug_type_name(&tsr->type, tsr->kind);
> }
> /*
> diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> index 31b5896276f1..6ca5489f3c4c 100644
> --- a/tools/perf/util/annotate-data.c
> +++ b/tools/perf/util/annotate-data.c
> @@ -898,7 +898,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;
>
> @@ -914,6 +914,7 @@ static void update_var_state(struct type_state *state, struct data_loc_info *dlo
> !is_better_type(®->type, &mem_die))
> continue;
>
> + reg->offset = -var->offset;
> reg->type = mem_die;
> reg->kind = TSR_KIND_POINTER;
> reg->ok = true;
> @@ -925,13 +926,17 @@ static void update_var_state(struct type_state *state, struct data_loc_info *dlo
> }
>
> 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 offset %x",
> + insn_offset, var->reg, var->offset);
> pr_debug_type_name(&mem_die, TSR_KIND_TYPE);
>
> /*
> @@ -1119,7 +1124,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);
> @@ -1148,7 +1153,7 @@ static enum type_match_result check_matching_type(struct type_state *state,
> */
> *type_die = state->regs[reg].type;
>
> - dloc->type_offset = dloc->op->offset;
> + dloc->type_offset = dloc->op->offset + state->regs[reg].offset;
>
> /* Get the size of the actual type */
> if (dwarf_aggregate_size(type_die, &size) < 0 ||
> diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h
> index fd0d1084bc4e..20237e7e4e2f 100644
> --- a/tools/perf/util/annotate-data.h
> +++ b/tools/perf/util/annotate-data.h
> @@ -174,6 +174,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.51.0.384.g4c02a37b29-goog
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 05/10] perf annotate: Save pointer offset in stack state
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
0 siblings, 0 replies; 18+ messages in thread
From: Namhyung Kim @ 2025-10-04 7:59 UTC (permalink / raw)
To: Zecheng Li
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
Adrian Hunter, Liang, Kan, Masami Hiramatsu, Xu Liu,
linux-perf-users, linux-kernel
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
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 06/10] perf annotate: Invalidate register states for untracked instructions
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
0 siblings, 0 replies; 18+ messages in thread
From: Namhyung Kim @ 2025-10-04 8:04 UTC (permalink / raw)
To: Zecheng Li
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
Adrian Hunter, Liang, Kan, Masami Hiramatsu, Xu Liu,
linux-perf-users, linux-kernel
On Wed, Sep 17, 2025 at 07:58:04PM +0000, Zecheng Li wrote:
> When tracking variable types, instructions that modify a pointer value
> in an untracked way can lead to incorrect type propagation. To prevent
> this, invalidate the register state when encountering such instructions.
>
> This change invalidates pointer types for various arithmetic and bitwise
> operations that current pointer offset tracking doesn't support, like
> imul, shl, and, inc, etc.
>
> A special case is added for 'xor reg, reg', which is a common idiom for
> zeroing a register. For this, the register state is updated to be a
> constant with a value of 0.
>
> This could introduce slight regressions if a variable is zeroed and then
> reused. This can be addressed in the future by using all DWARF locations
> for instruction tracking instead of only the first one.
>
> Signed-off-by: Zecheng Li <zecheng@google.com>
> ---
> tools/perf/arch/x86/annotate/instructions.c | 29 +++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/tools/perf/arch/x86/annotate/instructions.c b/tools/perf/arch/x86/annotate/instructions.c
> index 709c6f7efe82..3c98f72c423f 100644
> --- a/tools/perf/arch/x86/annotate/instructions.c
> +++ b/tools/perf/arch/x86/annotate/instructions.c
> @@ -411,6 +411,35 @@ static void update_insn_state_x86(struct type_state *state,
> return;
> }
>
> + /* Invalidate register states for other ops which may change pointers */
> + if (has_reg_type(state, dst->reg1) && !dst->mem_ref &&
> + dwarf_tag(&state->regs[dst->reg1].type) == DW_TAG_pointer_type) {
> + if (!strncmp(dl->ins.name, "imul", 4) || !strncmp(dl->ins.name, "mul", 3) ||
> + !strncmp(dl->ins.name, "idiv", 4) || !strncmp(dl->ins.name, "div", 3) ||
> + !strncmp(dl->ins.name, "shl", 3) || !strncmp(dl->ins.name, "shr", 3) ||
> + !strncmp(dl->ins.name, "sar", 3) || !strncmp(dl->ins.name, "and", 3) ||
> + !strncmp(dl->ins.name, "or", 2) || !strncmp(dl->ins.name, "neg", 3) ||
> + !strncmp(dl->ins.name, "inc", 3) || !strncmp(dl->ins.name, "dec", 3)) {
> + pr_debug_dtp("%s [%x] invalidate reg%d\n",
> + dl->ins.name, insn_offset, dst->reg1);
> + state->regs[dst->reg1].ok = false;
> + state->regs[dst->reg1].copied_from = -1;
> + return;
> + }
> +
> + if (!strncmp(dl->ins.name, "xor", 3) && dst->reg1 == src->reg1) {
> + /* xor reg, reg clears the register */
> + pr_debug_dtp("xor [%x] clear reg%d\n",
> + insn_offset, dst->reg1);
> +
> + state->regs[dst->reg1].kind = TSR_KIND_CONST;
> + state->regs[dst->reg1].imm_value = 0;
> + state->regs[dst->reg1].ok = false;
I think .ok should be 'true'.
Thanks,
Namhyung
> + state->regs[dst->reg1].copied_from = -1;
> + return;
> + }
> + }
> +
> if (strncmp(dl->ins.name, "mov", 3))
> return;
>
> --
> 2.51.0.384.g4c02a37b29-goog
>
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2025-10-04 8:04 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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
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).