From b508965d35321534e84daf8946b2cc5f64517db9 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 18 Mar 2024 22:50:53 -0700 Subject: perf dwarf-aux: Remove unused pc argument It's not used, let's get rid of it. Signed-off-by: Namhyung Kim Reviewed-by: Arnaldo Carvalho de Melo Cc: Adrian Hunter Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Linus Torvalds Cc: Masami Hiramatsu Cc: Peter Zijlstra Cc: Stephane Eranian Link: https://lore.kernel.org/r/20240319055115.4063940-2-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index 30c4d19fcf11..59ce5f4f4a40 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -263,7 +263,7 @@ static int find_data_type_die(struct debuginfo *di, u64 pc, u64 addr, offset = loc->offset; if (reg == DWARF_REG_PC) { - if (die_find_variable_by_addr(&cu_die, pc, addr, &var_die, &offset)) { + if (die_find_variable_by_addr(&cu_die, addr, &var_die, &offset)) { ret = check_variable(&var_die, type_die, offset, /*is_pointer=*/false); loc->offset = offset; @@ -312,7 +312,7 @@ retry: /* Search from the inner-most scope to the outer */ for (i = nr_scopes - 1; i >= 0; i--) { if (reg == DWARF_REG_PC) { - if (!die_find_variable_by_addr(&scopes[i], pc, addr, + if (!die_find_variable_by_addr(&scopes[i], addr, &var_die, &offset)) continue; } else { -- cgit From a3f4d5b57dd8fd2a749be261d7253616f15671b5 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 18 Mar 2024 22:50:58 -0700 Subject: perf annotate-data: Introduce 'struct data_loc_info' The find_data_type() needs many information to describe the location of the data. Add the new 'struct data_loc_info' to pass those information at once. No functional changes intended. Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Linus Torvalds Cc: Masami Hiramatsu Cc: Peter Zijlstra Cc: Stephane Eranian Link: https://lore.kernel.org/r/20240319055115.4063940-7-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 83 ++++++++++++++++++++++------------------- 1 file changed, 44 insertions(+), 39 deletions(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index 59ce5f4f4a40..ff81d164aa57 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -239,21 +239,28 @@ static int check_variable(Dwarf_Die *var_die, Dwarf_Die *type_die, int offset, } /* The result will be saved in @type_die */ -static int find_data_type_die(struct debuginfo *di, u64 pc, u64 addr, - const char *var_name, struct annotated_op_loc *loc, - Dwarf_Die *type_die) +static int find_data_type_die(struct data_loc_info *dloc, Dwarf_Die *type_die) { + struct annotated_op_loc *loc = dloc->op; Dwarf_Die cu_die, var_die; Dwarf_Die *scopes = NULL; int reg, offset; int ret = -1; int i, nr_scopes; int fbreg = -1; - bool is_fbreg = false; int fb_offset = 0; + bool is_fbreg = false; + u64 pc; + + /* + * IP is a relative instruction address from the start of the map, as + * it can be randomized/relocated, it needs to translate to PC which is + * a file address for DWARF processing. + */ + pc = map__rip_2objdump(dloc->ms->map, dloc->ip); /* Get a compile_unit for this address */ - if (!find_cu_die(di, pc, &cu_die)) { + if (!find_cu_die(dloc->di, pc, &cu_die)) { pr_debug("cannot find CU for address %" PRIx64 "\n", pc); ann_data_stat.no_cuinfo++; return -1; @@ -263,18 +270,19 @@ static int find_data_type_die(struct debuginfo *di, u64 pc, u64 addr, offset = loc->offset; if (reg == DWARF_REG_PC) { - if (die_find_variable_by_addr(&cu_die, addr, &var_die, &offset)) { + if (die_find_variable_by_addr(&cu_die, dloc->var_addr, &var_die, + &offset)) { ret = check_variable(&var_die, type_die, offset, /*is_pointer=*/false); - loc->offset = offset; + dloc->type_offset = offset; goto out; } - if (var_name && die_find_variable_at(&cu_die, var_name, pc, - &var_die)) { - ret = check_variable(&var_die, type_die, 0, + if (dloc->var_name && + die_find_variable_at(&cu_die, dloc->var_name, pc, &var_die)) { + ret = check_variable(&var_die, type_die, dloc->type_offset, /*is_pointer=*/false); - /* loc->offset will be updated by the caller */ + /* dloc->type_offset was updated by the caller */ goto out; } } @@ -291,10 +299,11 @@ static int find_data_type_die(struct debuginfo *di, u64 pc, u64 addr, dwarf_formblock(&attr, &block) == 0 && block.length == 1) { switch (*block.data) { case DW_OP_reg0 ... DW_OP_reg31: - fbreg = *block.data - DW_OP_reg0; + fbreg = dloc->fbreg = *block.data - DW_OP_reg0; break; case DW_OP_call_frame_cfa: - if (die_get_cfa(di->dbg, pc, &fbreg, + dloc->fb_cfa = true; + if (die_get_cfa(dloc->di->dbg, pc, &fbreg, &fb_offset) < 0) fbreg = -1; break; @@ -312,7 +321,7 @@ retry: /* Search from the inner-most scope to the outer */ for (i = nr_scopes - 1; i >= 0; i--) { if (reg == DWARF_REG_PC) { - if (!die_find_variable_by_addr(&scopes[i], addr, + if (!die_find_variable_by_addr(&scopes[i], dloc->var_addr, &var_die, &offset)) continue; } else { @@ -325,7 +334,7 @@ retry: /* Found a variable, see if it's correct */ ret = check_variable(&var_die, type_die, offset, reg != DWARF_REG_PC && !is_fbreg); - loc->offset = offset; + dloc->type_offset = offset; goto out; } @@ -344,50 +353,46 @@ out: /** * find_data_type - Return a data type at the location - * @ms: map and symbol at the location - * @ip: instruction address of the memory access - * @loc: instruction operand location - * @addr: data address of the memory access - * @var_name: global variable name + * @dloc: data location * * This functions searches the debug information of the binary to get the data - * type it accesses. The exact location is expressed by (@ip, reg, offset) - * for pointer variables or (@ip, @addr) for global variables. Note that global - * variables might update the @loc->offset after finding the start of the variable. - * If it cannot find a global variable by address, it tried to fine a declaration - * of the variable using @var_name. In that case, @loc->offset won't be updated. + * type it accesses. The exact location is expressed by (ip, reg, offset) + * for pointer variables or (ip, addr) for global variables. Note that global + * variables might update the @dloc->type_offset after finding the start of the + * variable. If it cannot find a global variable by address, it tried to find + * a declaration of the variable using var_name. In that case, @dloc->offset + * won't be updated. * * It return %NULL if not found. */ -struct annotated_data_type *find_data_type(struct map_symbol *ms, u64 ip, - struct annotated_op_loc *loc, u64 addr, - const char *var_name) +struct annotated_data_type *find_data_type(struct data_loc_info *dloc) { struct annotated_data_type *result = NULL; - struct dso *dso = map__dso(ms->map); - struct debuginfo *di; + struct dso *dso = map__dso(dloc->ms->map); Dwarf_Die type_die; - u64 pc; - di = debuginfo__new(dso->long_name); - if (di == NULL) { + dloc->di = debuginfo__new(dso->long_name); + if (dloc->di == NULL) { pr_debug("cannot get the debug info\n"); return NULL; } /* - * IP is a relative instruction address from the start of the map, as - * it can be randomized/relocated, it needs to translate to PC which is - * a file address for DWARF processing. + * The type offset is the same as instruction offset by default. + * But when finding a global variable, the offset won't be valid. */ - pc = map__rip_2objdump(ms->map, ip); - if (find_data_type_die(di, pc, addr, var_name, loc, &type_die) < 0) + if (dloc->var_name == NULL) + dloc->type_offset = dloc->op->offset; + + dloc->fbreg = -1; + + if (find_data_type_die(dloc, &type_die) < 0) goto out; result = dso__findnew_data_type(dso, &type_die); out: - debuginfo__delete(di); + debuginfo__delete(dloc->di); return result; } -- cgit From 90429524f3e7342e98fd3a14deaa46f1dd3f6ffe Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 18 Mar 2024 22:51:00 -0700 Subject: perf annotate-data: Add debug messages Add a new debug option "type-profile" to enable the detailed info during the type analysis especially for instruction tracking. You can use this before the command name like 'report' or 'annotate'. $ perf --debug type-profile annotate --data-type Committer testing: First get some memory events: $ perf mem record ls Then, without data-type profiling debug: $ perf annotate --data-type | head Annotate type: 'struct rtld_global' in /usr/lib64/ld-linux-x86-64.so.2 (1 samples): ============================================================================ samples offset size field 1 0 4336 struct rtld_global { 0 0 0 struct link_namespaces* _dl_ns; 0 2560 8 size_t _dl_nns; 0 2568 40 __rtld_lock_recursive_t _dl_load_lock { 0 2568 40 pthread_mutex_t mutex { 0 2568 40 struct __pthread_mutex_s __data { 0 2568 4 int __lock; $ And with only data-type profiling: $ perf --debug type-profile annotate --data-type | head ----------------------------------------------------------- find_data_type_die [1e67] for reg13873052 (PC) offset=0x150e2 in dl_main CU die offset: 0x29cd3 found PC-rel by addr=0x34020 offset=0x20 ----------------------------------------------------------- find_data_type_die [2e] for reg12 offset=0 in __GI___readdir64 CU die offset: 0x137a45 frame base: cfa=1 fbreg=-1 found "__futex" in scope=2/2 (die: 0x137ad5) 0(reg12) type=int (die:2a) ----------------------------------------------------------- find_data_type_die [52] for reg5 offset=0 in __memmove_avx_unaligned_erms CU die offset: 0x1124ed no variable found Annotate type: 'struct rtld_global' in /usr/lib64/ld-linux-x86-64.so.2 (1 samples): ============================================================================ samples offset size field 1 0 4336 struct rtld_global { 0 0 0 struct link_namespaces* _dl_ns; 0 2560 8 size_t _dl_nns; 0 2568 40 __rtld_lock_recursive_t _dl_load_lock { 0 2568 40 pthread_mutex_t mutex { 0 2568 40 struct __pthread_mutex_s __data { 0 2568 4 int __lock; $ Signed-off-by: Namhyung Kim Tested-by: Arnaldo Carvalho de Melo Cc: Adrian Hunter Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Linus Torvalds Cc: Masami Hiramatsu Cc: Peter Zijlstra Cc: Stephane Eranian Link: https://lore.kernel.org/r/20240319055115.4063940-9-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 74 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 67 insertions(+), 7 deletions(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index ff81d164aa57..f482ccfdaa91 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -23,6 +23,29 @@ #include "symbol.h" #include "symbol_conf.h" +#define pr_debug_dtp(fmt, ...) \ +do { \ + if (debug_type_profile) \ + pr_info(fmt, ##__VA_ARGS__); \ + else \ + pr_debug3(fmt, ##__VA_ARGS__); \ +} while (0) + +static void pr_debug_type_name(Dwarf_Die *die) +{ + struct strbuf sb; + char *str; + + if (!debug_type_profile && verbose < 3) + return; + + strbuf_init(&sb, 32); + die_get_typename_from_type(die, &sb); + str = strbuf_detach(&sb, NULL); + pr_info(" type=%s (die:%lx)\n", str, (long)dwarf_dieoffset(die)); + free(str); +} + /* * Compare type name and size to maintain them in a tree. * I'm not sure if DWARF would have information of a single type in many @@ -201,7 +224,7 @@ static int check_variable(Dwarf_Die *var_die, Dwarf_Die *type_die, int offset, /* Get the type of the variable */ if (die_get_real_type(var_die, type_die) == NULL) { - pr_debug("variable has no type\n"); + pr_debug_dtp("variable has no type\n"); ann_data_stat.no_typeinfo++; return -1; } @@ -215,7 +238,7 @@ static int check_variable(Dwarf_Die *var_die, Dwarf_Die *type_die, int offset, if ((dwarf_tag(type_die) != DW_TAG_pointer_type && dwarf_tag(type_die) != DW_TAG_array_type) || die_get_real_type(type_die, type_die) == NULL) { - pr_debug("no pointer or no type\n"); + pr_debug_dtp("no pointer or no type\n"); ann_data_stat.no_typeinfo++; return -1; } @@ -223,14 +246,15 @@ static int check_variable(Dwarf_Die *var_die, Dwarf_Die *type_die, int offset, /* Get the size of the actual type */ if (dwarf_aggregate_size(type_die, &size) < 0) { - pr_debug("type size is unknown\n"); + pr_debug_dtp("type size is unknown\n"); ann_data_stat.invalid_size++; return -1; } /* Minimal sanity check */ if ((unsigned)offset >= size) { - pr_debug("offset: %d is bigger than size: %" PRIu64 "\n", offset, size); + pr_debug_dtp("offset: %d is bigger than size: %"PRIu64"\n", + offset, size); ann_data_stat.bad_offset++; return -1; } @@ -251,6 +275,19 @@ static int find_data_type_die(struct data_loc_info *dloc, Dwarf_Die *type_die) int fb_offset = 0; bool is_fbreg = false; u64 pc; + char buf[64]; + + if (dloc->op->multi_regs) + snprintf(buf, sizeof(buf), " or reg%d", dloc->op->reg2); + else if (dloc->op->reg1 == DWARF_REG_PC) + snprintf(buf, sizeof(buf), " (PC)"); + else + buf[0] = '\0'; + + pr_debug_dtp("-----------------------------------------------------------\n"); + pr_debug_dtp("%s [%"PRIx64"] for reg%d%s offset=%#x in %s\n", + __func__, dloc->ip - dloc->ms->sym->start, + dloc->op->reg1, buf, dloc->op->offset, dloc->ms->sym->name); /* * IP is a relative instruction address from the start of the map, as @@ -261,7 +298,7 @@ static int find_data_type_die(struct data_loc_info *dloc, Dwarf_Die *type_die) /* Get a compile_unit for this address */ if (!find_cu_die(dloc->di, pc, &cu_die)) { - pr_debug("cannot find CU for address %" PRIx64 "\n", pc); + pr_debug_dtp("cannot find CU for address %"PRIx64"\n", pc); ann_data_stat.no_cuinfo++; return -1; } @@ -269,12 +306,17 @@ static int find_data_type_die(struct data_loc_info *dloc, Dwarf_Die *type_die) reg = loc->reg1; offset = loc->offset; + pr_debug_dtp("CU die offset: %#lx\n", (long)dwarf_dieoffset(&cu_die)); + if (reg == DWARF_REG_PC) { if (die_find_variable_by_addr(&cu_die, dloc->var_addr, &var_die, &offset)) { ret = check_variable(&var_die, type_die, offset, /*is_pointer=*/false); dloc->type_offset = offset; + + pr_debug_dtp("found PC-rel by addr=%#"PRIx64" offset=%#x\n", + dloc->var_addr, offset); goto out; } @@ -310,6 +352,9 @@ static int find_data_type_die(struct data_loc_info *dloc, Dwarf_Die *type_die) default: break; } + + pr_debug_dtp("frame base: cfa=%d fbreg=%d\n", + dloc->fb_cfa, fbreg); } } @@ -334,6 +379,19 @@ retry: /* Found a variable, see if it's correct */ ret = check_variable(&var_die, type_die, offset, reg != DWARF_REG_PC && !is_fbreg); + if (ret == 0) { + pr_debug_dtp("found \"%s\" in scope=%d/%d (die: %#lx) ", + dwarf_diename(&var_die), i+1, nr_scopes, + (long)dwarf_dieoffset(&scopes[i])); + if (reg == DWARF_REG_PC) + pr_debug_dtp("%#x(PC) offset=%#x", loc->offset, offset); + else if (reg == DWARF_REG_FB || is_fbreg) + pr_debug_dtp("%#x(reg%d) stack fb_offset=%#x offset=%#x", + loc->offset, reg, fb_offset, offset); + else + pr_debug_dtp("%#x(reg%d)", loc->offset, reg); + pr_debug_type_name(type_die); + } dloc->type_offset = offset; goto out; } @@ -343,8 +401,10 @@ retry: goto retry; } - if (ret < 0) + if (ret < 0) { + pr_debug_dtp("no variable found\n"); ann_data_stat.no_var++; + } out: free(scopes); @@ -373,7 +433,7 @@ struct annotated_data_type *find_data_type(struct data_loc_info *dloc) dloc->di = debuginfo__new(dso->long_name); if (dloc->di == NULL) { - pr_debug("cannot get the debug info\n"); + pr_debug_dtp("cannot get the debug info\n"); return NULL; } -- cgit From 06b2ce75386df04b7aea53818ed3c42ee50ec426 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 18 Mar 2024 22:51:01 -0700 Subject: perf annotate-data: Maintain variable type info As it collected basic block and variable information in each scope, it now can build a state table to find matching variable at the location. The struct type_state is to keep the type info saved in each register and stack slot. The update_var_state() updates the table when it finds variables in the current address. It expects die_collect_vars() filled a list of variables with type info and starting address. Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Linus Torvalds Cc: Masami Hiramatsu Cc: Peter Zijlstra Cc: Stephane Eranian Link: https://lore.kernel.org/r/20240319055115.4063940-10-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 173 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 173 insertions(+) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index f482ccfdaa91..8eaa06f1cee5 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -46,6 +46,62 @@ static void pr_debug_type_name(Dwarf_Die *die) free(str); } +/* Type information in a register, valid when ok is true */ +struct type_state_reg { + Dwarf_Die type; + bool ok; +}; + +/* Type information in a stack location, dynamically allocated */ +struct type_state_stack { + struct list_head list; + Dwarf_Die type; + int offset; + int size; + bool compound; +}; + +/* FIXME: This should be arch-dependent */ +#define TYPE_STATE_MAX_REGS 16 + +/* + * State table to maintain type info in each register and stack location. + * It'll be updated when new variable is allocated or type info is moved + * to a new location (register or stack). As it'd be used with the + * shortest path of basic blocks, it only maintains a single table. + */ +struct type_state { + struct type_state_reg regs[TYPE_STATE_MAX_REGS]; + struct list_head stack_vars; +}; + +static bool has_reg_type(struct type_state *state, int reg) +{ + return (unsigned)reg < ARRAY_SIZE(state->regs); +} + +/* These declarations will be remove once they are changed to static */ +void init_type_state(struct type_state *state, struct arch *arch __maybe_unused); +void exit_type_state(struct type_state *state); +void update_var_state(struct type_state *state, struct data_loc_info *dloc, + u64 addr, u64 insn_offset, struct die_var_type *var_types); + +void init_type_state(struct type_state *state, struct arch *arch __maybe_unused) +{ + memset(state, 0, sizeof(*state)); + INIT_LIST_HEAD(&state->stack_vars); +} + +void exit_type_state(struct type_state *state) +{ + struct type_state_stack *stack, *tmp; + + list_for_each_entry_safe(stack, tmp, &state->stack_vars, list) { + list_del(&stack->list); + free(stack); + } +} + /* * Compare type name and size to maintain them in a tree. * I'm not sure if DWARF would have information of a single type in many @@ -262,6 +318,123 @@ static int check_variable(Dwarf_Die *var_die, Dwarf_Die *type_die, int offset, return 0; } +static struct type_state_stack *find_stack_state(struct type_state *state, + int offset) +{ + struct type_state_stack *stack; + + list_for_each_entry(stack, &state->stack_vars, list) { + if (offset == stack->offset) + return stack; + + if (stack->compound && stack->offset < offset && + offset < stack->offset + stack->size) + return stack; + } + return NULL; +} + +static void set_stack_state(struct type_state_stack *stack, int offset, + Dwarf_Die *type_die) +{ + int tag; + Dwarf_Word size; + + if (dwarf_aggregate_size(type_die, &size) < 0) + size = 0; + + tag = dwarf_tag(type_die); + + stack->type = *type_die; + stack->size = size; + stack->offset = offset; + + switch (tag) { + case DW_TAG_structure_type: + case DW_TAG_union_type: + stack->compound = true; + break; + default: + stack->compound = false; + break; + } +} + +static struct type_state_stack *findnew_stack_state(struct type_state *state, + int offset, Dwarf_Die *type_die) +{ + struct type_state_stack *stack = find_stack_state(state, offset); + + if (stack) { + set_stack_state(stack, offset, type_die); + return stack; + } + + stack = malloc(sizeof(*stack)); + if (stack) { + set_stack_state(stack, offset, type_die); + list_add(&stack->list, &state->stack_vars); + } + return stack; +} + +/** + * update_var_state - Update type state using given variables + * @state: type state table + * @dloc: data location info + * @addr: instruction address to match with variable + * @insn_offset: instruction offset (for debug) + * @var_types: list of variables with type info + * + * This function fills the @state table using @var_types info. Each variable + * is used only at the given location and updates an entry in the table. + */ +void update_var_state(struct type_state *state, struct data_loc_info *dloc, + u64 addr, u64 insn_offset, struct die_var_type *var_types) +{ + Dwarf_Die mem_die; + struct die_var_type *var; + int fbreg = dloc->fbreg; + int fb_offset = 0; + + if (dloc->fb_cfa) { + if (die_get_cfa(dloc->di->dbg, addr, &fbreg, &fb_offset) < 0) + fbreg = -1; + } + + for (var = var_types; var != NULL; var = var->next) { + if (var->addr != addr) + continue; + /* Get the type DIE using the offset */ + if (!dwarf_offdie(dloc->di->dbg, var->die_off, &mem_die)) + continue; + + if (var->reg == DWARF_REG_FB) { + findnew_stack_state(state, var->offset, &mem_die); + + pr_debug_dtp("var [%"PRIx64"] -%#x(stack) type=", + insn_offset, -var->offset); + pr_debug_type_name(&mem_die); + } else if (var->reg == fbreg) { + findnew_stack_state(state, var->offset - fb_offset, &mem_die); + + pr_debug_dtp("var [%"PRIx64"] -%#x(stack) type=", + insn_offset, -var->offset + fb_offset); + pr_debug_type_name(&mem_die); + } else if (has_reg_type(state, var->reg) && var->offset == 0) { + struct type_state_reg *reg; + + reg = &state->regs[var->reg]; + reg->type = mem_die; + reg->ok = true; + + pr_debug_dtp("var [%"PRIx64"] reg%d type=", + insn_offset, var->reg); + pr_debug_type_name(&mem_die); + } + } +} + /* The result will be saved in @type_die */ static int find_data_type_die(struct data_loc_info *dloc, Dwarf_Die *type_die) { -- cgit From 4f903455befa257c50422a4570c4dca0020a1fc8 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 18 Mar 2024 22:51:02 -0700 Subject: perf annotate-data: Add update_insn_state() The update_insn_state() function is to update the type state table after processing each instruction. For now, it handles MOV (on x86) insn to transfer type info from the source location to the target. The location can be a register or a stack slot. Check carefully when memory reference happens and fetch the type correctly. It basically ignores write to a memory since it doesn't change the type info. One exception is writes to (new) stack slots for register spilling. Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Linus Torvalds Cc: Masami Hiramatsu Cc: Peter Zijlstra Cc: Stephane Eranian Link: https://lore.kernel.org/r/20240319055115.4063940-11-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 161 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 158 insertions(+), 3 deletions(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index 8eaa06f1cee5..592437b6c097 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -71,7 +71,9 @@ struct type_state_stack { * shortest path of basic blocks, it only maintains a single table. */ struct type_state { + /* state of general purpose registers */ struct type_state_reg regs[TYPE_STATE_MAX_REGS]; + /* state of stack location */ struct list_head stack_vars; }; @@ -85,6 +87,8 @@ void init_type_state(struct type_state *state, struct arch *arch __maybe_unused) void exit_type_state(struct type_state *state); void update_var_state(struct type_state *state, struct data_loc_info *dloc, u64 addr, u64 insn_offset, struct die_var_type *var_types); +void update_insn_state(struct type_state *state, struct data_loc_info *dloc, + struct disasm_line *dl); void init_type_state(struct type_state *state, struct arch *arch __maybe_unused) { @@ -412,13 +416,13 @@ void update_var_state(struct type_state *state, struct data_loc_info *dloc, if (var->reg == DWARF_REG_FB) { findnew_stack_state(state, var->offset, &mem_die); - pr_debug_dtp("var [%"PRIx64"] -%#x(stack) type=", + pr_debug_dtp("var [%"PRIx64"] -%#x(stack)", insn_offset, -var->offset); pr_debug_type_name(&mem_die); } else if (var->reg == fbreg) { findnew_stack_state(state, var->offset - fb_offset, &mem_die); - pr_debug_dtp("var [%"PRIx64"] -%#x(stack) type=", + pr_debug_dtp("var [%"PRIx64"] -%#x(stack)", insn_offset, -var->offset + fb_offset); pr_debug_type_name(&mem_die); } else if (has_reg_type(state, var->reg) && var->offset == 0) { @@ -428,13 +432,164 @@ void update_var_state(struct type_state *state, struct data_loc_info *dloc, reg->type = mem_die; reg->ok = true; - pr_debug_dtp("var [%"PRIx64"] reg%d type=", + pr_debug_dtp("var [%"PRIx64"] reg%d", insn_offset, var->reg); pr_debug_type_name(&mem_die); } } } +static void update_insn_state_x86(struct type_state *state, + struct data_loc_info *dloc, + struct disasm_line *dl) +{ + struct annotated_insn_loc loc; + struct annotated_op_loc *src = &loc.ops[INSN_OP_SOURCE]; + struct annotated_op_loc *dst = &loc.ops[INSN_OP_TARGET]; + struct type_state_reg *tsr; + Dwarf_Die type_die; + u32 insn_offset = dl->al.offset; + int fbreg = dloc->fbreg; + int fboff = 0; + + if (annotate_get_insn_location(dloc->arch, dl, &loc) < 0) + return; + + if (strncmp(dl->ins.name, "mov", 3)) + return; + + if (dloc->fb_cfa) { + u64 ip = dloc->ms->sym->start + dl->al.offset; + u64 pc = map__rip_2objdump(dloc->ms->map, ip); + + if (die_get_cfa(dloc->di->dbg, pc, &fbreg, &fboff) < 0) + fbreg = -1; + } + + /* Case 1. register to register transfers */ + if (!src->mem_ref && !dst->mem_ref) { + if (!has_reg_type(state, dst->reg1)) + return; + + tsr = &state->regs[dst->reg1]; + if (!has_reg_type(state, src->reg1) || + !state->regs[src->reg1].ok) { + tsr->ok = false; + return; + } + + tsr->type = state->regs[src->reg1].type; + tsr->ok = true; + + pr_debug_dtp("mov [%x] reg%d -> reg%d", + insn_offset, src->reg1, dst->reg1); + pr_debug_type_name(&tsr->type); + } + /* Case 2. memory to register transers */ + if (src->mem_ref && !dst->mem_ref) { + int sreg = src->reg1; + + if (!has_reg_type(state, dst->reg1)) + return; + + tsr = &state->regs[dst->reg1]; + +retry: + /* Check stack variables with offset */ + if (sreg == fbreg) { + struct type_state_stack *stack; + int offset = src->offset - fboff; + + stack = find_stack_state(state, offset); + if (stack == NULL) { + tsr->ok = false; + return; + } else if (!stack->compound) { + tsr->type = stack->type; + tsr->ok = true; + } else if (die_get_member_type(&stack->type, + offset - stack->offset, + &type_die)) { + tsr->type = type_die; + tsr->ok = true; + } else { + tsr->ok = false; + return; + } + + pr_debug_dtp("mov [%x] -%#x(stack) -> reg%d", + insn_offset, -offset, dst->reg1); + pr_debug_type_name(&tsr->type); + } + /* And then dereference the pointer if it has one */ + else if (has_reg_type(state, sreg) && state->regs[sreg].ok && + die_deref_ptr_type(&state->regs[sreg].type, + src->offset, &type_die)) { + tsr->type = type_die; + tsr->ok = true; + + pr_debug_dtp("mov [%x] %#x(reg%d) -> reg%d", + insn_offset, src->offset, sreg, dst->reg1); + pr_debug_type_name(&tsr->type); + } + /* Or try another register if any */ + else if (src->multi_regs && sreg == src->reg1 && + src->reg1 != src->reg2) { + sreg = src->reg2; + goto retry; + } + /* It failed to get a type info, mark it as invalid */ + else { + tsr->ok = false; + } + } + /* Case 3. register to memory transfers */ + if (!src->mem_ref && dst->mem_ref) { + if (!has_reg_type(state, src->reg1) || + !state->regs[src->reg1].ok) + return; + + /* Check stack variables with offset */ + if (dst->reg1 == fbreg) { + struct type_state_stack *stack; + int offset = dst->offset - fboff; + + stack = find_stack_state(state, offset); + if (stack) { + /* + * The source register is likely to hold a type + * of member if it's a compound type. Do not + * update the stack variable type since we can + * get the member type later by using the + * die_get_member_type(). + */ + if (!stack->compound) + set_stack_state(stack, offset, + &state->regs[src->reg1].type); + } else { + findnew_stack_state(state, offset, + &state->regs[src->reg1].type); + } + + pr_debug_dtp("mov [%x] reg%d -> -%#x(stack)", + insn_offset, src->reg1, -offset); + pr_debug_type_name(&state->regs[src->reg1].type); + } + /* + * Ignore other transfers since it'd set a value in a struct + * and won't change the type. + */ + } + /* Case 4. memory to memory transfers (not handled for now) */ +} + +void update_insn_state(struct type_state *state, struct data_loc_info *dloc, + struct disasm_line *dl) +{ + if (arch__is(dloc->arch, "x86")) + update_insn_state_x86(state, dloc, dl); +} + /* The result will be saved in @type_die */ static int find_data_type_die(struct data_loc_info *dloc, Dwarf_Die *type_die) { -- cgit From 1ebb5e17ef21b492ee60654d4e22cbfb3763661f Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 18 Mar 2024 22:51:03 -0700 Subject: perf annotate-data: Add get_global_var_type() Accessing global variable is common when it tracks execution later. Factor out the common code into a function for later use. It adds thread and cpumode to struct data_loc_info to find (global) symbols if needed. Also remove var_name as it's retrieved in the helper function. Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Linus Torvalds Cc: Masami Hiramatsu Cc: Peter Zijlstra Cc: Stephane Eranian Link: https://lore.kernel.org/r/20240319055115.4063940-12-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 62 +++++++++++++++++++++++++++++++---------- 1 file changed, 48 insertions(+), 14 deletions(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index 592437b6c097..3b661e693410 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -22,6 +22,7 @@ #include "strbuf.h" #include "symbol.h" #include "symbol_conf.h" +#include "thread.h" #define pr_debug_dtp(fmt, ...) \ do { \ @@ -382,6 +383,50 @@ static struct type_state_stack *findnew_stack_state(struct type_state *state, return stack; } +static bool get_global_var_type(Dwarf_Die *cu_die, struct data_loc_info *dloc, + u64 ip, u64 var_addr, int *var_offset, + Dwarf_Die *type_die) +{ + u64 pc, mem_addr; + int offset; + bool is_pointer = false; + const char *var_name = NULL; + Dwarf_Die var_die; + struct addr_location al; + struct symbol *sym; + + /* Try to get the variable by address first */ + if (die_find_variable_by_addr(cu_die, var_addr, &var_die, &offset) && + check_variable(&var_die, type_die, offset, is_pointer) == 0) { + *var_offset = offset; + return true; + } + + /* Kernel symbols might be relocated */ + mem_addr = var_addr + map__reloc(dloc->ms->map); + + addr_location__init(&al); + sym = thread__find_symbol_fb(dloc->thread, dloc->cpumode, + mem_addr, &al); + if (sym) { + var_name = sym->name; + /* Calculate type offset from the start of variable */ + *var_offset = mem_addr - map__unmap_ip(al.map, sym->start); + } + addr_location__exit(&al); + if (var_name == NULL) + return false; + + pc = map__rip_2objdump(dloc->ms->map, ip); + + /* Try to get the name of global variable */ + if (die_find_variable_at(cu_die, var_name, pc, &var_die) && + check_variable(&var_die, type_die, *var_offset, is_pointer) == 0) + return true; + + return false; +} + /** * update_var_state - Update type state using given variables * @state: type state table @@ -637,24 +682,14 @@ static int find_data_type_die(struct data_loc_info *dloc, Dwarf_Die *type_die) pr_debug_dtp("CU die offset: %#lx\n", (long)dwarf_dieoffset(&cu_die)); if (reg == DWARF_REG_PC) { - if (die_find_variable_by_addr(&cu_die, dloc->var_addr, &var_die, - &offset)) { - ret = check_variable(&var_die, type_die, offset, - /*is_pointer=*/false); + if (get_global_var_type(&cu_die, dloc, dloc->ip, dloc->var_addr, + &offset, type_die)) { dloc->type_offset = offset; pr_debug_dtp("found PC-rel by addr=%#"PRIx64" offset=%#x\n", dloc->var_addr, offset); goto out; } - - if (dloc->var_name && - die_find_variable_at(&cu_die, dloc->var_name, pc, &var_die)) { - ret = check_variable(&var_die, type_die, dloc->type_offset, - /*is_pointer=*/false); - /* dloc->type_offset was updated by the caller */ - goto out; - } } /* Get a list of nested scopes - i.e. (inlined) functions and blocks. */ @@ -769,8 +804,7 @@ struct annotated_data_type *find_data_type(struct data_loc_info *dloc) * The type offset is the same as instruction offset by default. * But when finding a global variable, the offset won't be valid. */ - if (dloc->var_name == NULL) - dloc->type_offset = dloc->op->offset; + dloc->type_offset = dloc->op->offset; dloc->fbreg = -1; -- cgit From 0a41e5d6849b4f705c377d34799485b546764970 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 18 Mar 2024 22:51:04 -0700 Subject: perf annotate-data: Handle global variable access When updating the instruction states, it also needs to handle global variable accesses. Same as it does for PC-relative addressing, it can look up the type by address (if it's defined in the same file), or by name after finding the symbol by address (for declarations). Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Linus Torvalds Cc: Masami Hiramatsu Cc: Peter Zijlstra Cc: Stephane Eranian Link: https://lore.kernel.org/r/20240319055115.4063940-13-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 46 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 4 deletions(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index 3b661e693410..2cc9f56e3eea 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -89,7 +89,7 @@ void exit_type_state(struct type_state *state); void update_var_state(struct type_state *state, struct data_loc_info *dloc, u64 addr, u64 insn_offset, struct die_var_type *var_types); void update_insn_state(struct type_state *state, struct data_loc_info *dloc, - struct disasm_line *dl); + Dwarf_Die *cu_die, struct disasm_line *dl); void init_type_state(struct type_state *state, struct arch *arch __maybe_unused) { @@ -485,7 +485,7 @@ void update_var_state(struct type_state *state, struct data_loc_info *dloc, } static void update_insn_state_x86(struct type_state *state, - struct data_loc_info *dloc, + struct data_loc_info *dloc, Dwarf_Die *cu_die, struct disasm_line *dl) { struct annotated_insn_loc loc; @@ -577,6 +577,29 @@ retry: insn_offset, src->offset, sreg, dst->reg1); pr_debug_type_name(&tsr->type); } + /* Or check if it's a global variable */ + else if (sreg == DWARF_REG_PC) { + struct map_symbol *ms = dloc->ms; + u64 ip = ms->sym->start + dl->al.offset; + u64 addr; + int offset; + + addr = annotate_calc_pcrel(ms, ip, src->offset, dl); + + if (!get_global_var_type(cu_die, dloc, ip, addr, &offset, + &type_die) || + !die_get_member_type(&type_die, offset, &type_die)) { + tsr->ok = false; + return; + } + + tsr->type = type_die; + tsr->ok = true; + + pr_debug_dtp("mov [%x] global addr=%"PRIx64" -> reg%d", + insn_offset, addr, dst->reg1); + pr_debug_type_name(&type_die); + } /* Or try another register if any */ else if (src->multi_regs && sreg == src->reg1 && src->reg1 != src->reg2) { @@ -628,11 +651,26 @@ retry: /* Case 4. memory to memory transfers (not handled for now) */ } +/** + * update_insn_state - Update type state for an instruction + * @state: type state table + * @dloc: data location info + * @cu_die: compile unit debug entry + * @dl: disasm line for the instruction + * + * This function updates the @state table for the target operand of the + * instruction at @dl if it transfers the type like MOV on x86. Since it + * tracks the type, it won't care about the values like in arithmetic + * instructions like ADD/SUB/MUL/DIV and INC/DEC. + * + * Note that ops->reg2 is only available when both mem_ref and multi_regs + * are true. + */ void update_insn_state(struct type_state *state, struct data_loc_info *dloc, - struct disasm_line *dl) + Dwarf_Die *cu_die, struct disasm_line *dl) { if (arch__is(dloc->arch, "x86")) - update_insn_state_x86(state, dloc, dl); + update_insn_state_x86(state, dloc, cu_die, dl); } /* The result will be saved in @type_die */ -- cgit From cffb7910afbdf73ca5b2fdf8a617584a7dcecf14 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 18 Mar 2024 22:51:05 -0700 Subject: perf annotate-data: Handle call instructions When updating instruction states, the call instruction should play a role since it changes the register states. For simplicity, mark some registers as caller-saved registers (should be arch-dependent), and invalidate them all after a function call. If the function returns something, the designated register (ret_reg) will have the type info. Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Linus Torvalds Cc: Masami Hiramatsu Cc: Peter Zijlstra Cc: Stephane Eranian Link: https://lore.kernel.org/r/20240319055115.4063940-14-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 54 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 2 deletions(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index 2cc9f56e3eea..6bcf22e523cb 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -47,10 +47,14 @@ static void pr_debug_type_name(Dwarf_Die *die) free(str); } -/* Type information in a register, valid when ok is true */ +/* + * Type information in a register, valid when @ok is true. + * The @caller_saved registers are invalidated after a function call. + */ struct type_state_reg { Dwarf_Die type; bool ok; + bool caller_saved; }; /* Type information in a stack location, dynamically allocated */ @@ -76,6 +80,8 @@ struct type_state { struct type_state_reg regs[TYPE_STATE_MAX_REGS]; /* state of stack location */ struct list_head stack_vars; + /* return value register */ + int ret_reg; }; static bool has_reg_type(struct type_state *state, int reg) @@ -91,10 +97,23 @@ void update_var_state(struct type_state *state, struct data_loc_info *dloc, void update_insn_state(struct type_state *state, struct data_loc_info *dloc, Dwarf_Die *cu_die, struct disasm_line *dl); -void init_type_state(struct type_state *state, struct arch *arch __maybe_unused) +void init_type_state(struct type_state *state, struct arch *arch) { memset(state, 0, sizeof(*state)); INIT_LIST_HEAD(&state->stack_vars); + + if (arch__is(arch, "x86")) { + state->regs[0].caller_saved = true; + state->regs[1].caller_saved = true; + state->regs[2].caller_saved = true; + state->regs[4].caller_saved = true; + state->regs[5].caller_saved = true; + state->regs[8].caller_saved = true; + state->regs[9].caller_saved = true; + state->regs[10].caller_saved = true; + state->regs[11].caller_saved = true; + state->ret_reg = 0; + } } void exit_type_state(struct type_state *state) @@ -500,6 +519,37 @@ static void update_insn_state_x86(struct type_state *state, if (annotate_get_insn_location(dloc->arch, dl, &loc) < 0) return; + if (ins__is_call(&dl->ins)) { + struct symbol *func = dl->ops.target.sym; + + if (func == NULL) + return; + + /* __fentry__ will preserve all registers */ + if (!strcmp(func->name, "__fentry__")) + return; + + pr_debug_dtp("call [%x] %s\n", insn_offset, func->name); + + /* Otherwise invalidate caller-saved registers after call */ + for (unsigned i = 0; i < ARRAY_SIZE(state->regs); i++) { + if (state->regs[i].caller_saved) + state->regs[i].ok = false; + } + + /* Update register with the return type (if any) */ + if (die_find_func_rettype(cu_die, func->name, &type_die)) { + tsr = &state->regs[state->ret_reg]; + tsr->type = type_die; + tsr->ok = true; + + pr_debug_dtp("call [%x] return -> reg%d", + insn_offset, state->ret_reg); + pr_debug_type_name(&type_die); + } + return; + } + if (strncmp(dl->ins.name, "mov", 3)) return; -- cgit From eb8a55e01de9a671e130b4c0e5b483997f4f491f Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 18 Mar 2024 22:51:06 -0700 Subject: perf annotate-data: Implement instruction tracking If it failed to find a variable for the location directly, it might be due to a missing variable in the source code. For example, accessing pointer variables in a chain can result in the case like below: struct foo *foo = ...; int i = foo->bar->baz; The DWARF debug information is created for each variable so it'd have one for 'foo'. But there's no variable for 'foo->bar' and then it cannot know the type of 'bar' and 'baz'. The above source code can be compiled to the follow x86 instructions: mov 0x8(%rax), %rcx mov 0x4(%rcx), %rdx <=== PMU sample mov %rdx, -4(%rbp) Let's say 'foo' is located in the %rax and it has a pointer to struct foo. But perf sample is captured in the second instruction and there is no variable or type info for the %rcx. It'd be great if compiler could generate debug info for %rcx, but we should handle it on our side. So this patch implements the logic to iterate instructions and update the type table for each location. As it already collected a list of scopes including the target instruction, we can use it to construct the type table smartly. +---------------- scope[0] subprogram | | +-------------- scope[1] lexical_block | | | | +------------ scope[2] inlined_subroutine | | | | | | +---------- scope[3] inlined_subroutine | | | | | | | | +-------- scope[4] lexical_block | | | | | | | | | | *** target instruction ... Image the target instruction has 5 scopes, each scope will have its own variables and parameters. Then it can start with the innermost scope (4). So it'd search the shortest path from the start of scope[4] to the target address and build a list of basic blocks. Then it iterates the basic blocks with the variables in the scope and update the table. If it finds a type at the target instruction, then returns it. Otherwise, it moves to the upper scope[3]. Now it'd search the shortest path from the start of scope[3] to the start of scope[4]. Then connect it to the existing basic block list. Then it'd iterate the blocks with variables for both scopes. It can repeat this until it finds a type at the target instruction or reaches to the top scope[0]. As the basic blocks contain the shortest path, it won't worry about branches and can update the table simply. The final check will be done by find_matching_type() in the next patch. Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Linus Torvalds Cc: Masami Hiramatsu Cc: Peter Zijlstra Cc: Stephane Eranian Link: https://lore.kernel.org/r/20240319055115.4063940-15-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 223 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 209 insertions(+), 14 deletions(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index 6bcf22e523cb..13ba65693367 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -89,15 +89,7 @@ static bool has_reg_type(struct type_state *state, int reg) return (unsigned)reg < ARRAY_SIZE(state->regs); } -/* These declarations will be remove once they are changed to static */ -void init_type_state(struct type_state *state, struct arch *arch __maybe_unused); -void exit_type_state(struct type_state *state); -void update_var_state(struct type_state *state, struct data_loc_info *dloc, - u64 addr, u64 insn_offset, struct die_var_type *var_types); -void update_insn_state(struct type_state *state, struct data_loc_info *dloc, - Dwarf_Die *cu_die, struct disasm_line *dl); - -void init_type_state(struct type_state *state, struct arch *arch) +static void init_type_state(struct type_state *state, struct arch *arch) { memset(state, 0, sizeof(*state)); INIT_LIST_HEAD(&state->stack_vars); @@ -116,7 +108,7 @@ void init_type_state(struct type_state *state, struct arch *arch) } } -void exit_type_state(struct type_state *state) +static void exit_type_state(struct type_state *state) { struct type_state_stack *stack, *tmp; @@ -457,8 +449,8 @@ static bool get_global_var_type(Dwarf_Die *cu_die, struct data_loc_info *dloc, * This function fills the @state table using @var_types info. Each variable * is used only at the given location and updates an entry in the table. */ -void update_var_state(struct type_state *state, struct data_loc_info *dloc, - u64 addr, u64 insn_offset, struct die_var_type *var_types) +static void update_var_state(struct type_state *state, struct data_loc_info *dloc, + u64 addr, u64 insn_offset, struct die_var_type *var_types) { Dwarf_Die mem_die; struct die_var_type *var; @@ -716,13 +708,207 @@ retry: * Note that ops->reg2 is only available when both mem_ref and multi_regs * are true. */ -void update_insn_state(struct type_state *state, struct data_loc_info *dloc, - Dwarf_Die *cu_die, struct disasm_line *dl) +static void update_insn_state(struct type_state *state, struct data_loc_info *dloc, + Dwarf_Die *cu_die, struct disasm_line *dl) { if (arch__is(dloc->arch, "x86")) update_insn_state_x86(state, dloc, cu_die, dl); } +/* + * Prepend this_blocks (from the outer scope) to full_blocks, removing + * duplicate disasm line. + */ +static void prepend_basic_blocks(struct list_head *this_blocks, + struct list_head *full_blocks) +{ + struct annotated_basic_block *first_bb, *last_bb; + + last_bb = list_last_entry(this_blocks, typeof(*last_bb), list); + first_bb = list_first_entry(full_blocks, typeof(*first_bb), list); + + if (list_empty(full_blocks)) + goto out; + + /* Last insn in this_blocks should be same as first insn in full_blocks */ + if (last_bb->end != first_bb->begin) { + pr_debug("prepend basic blocks: mismatched disasm line %"PRIx64" -> %"PRIx64"\n", + last_bb->end->al.offset, first_bb->begin->al.offset); + goto out; + } + + /* Is the basic block have only one disasm_line? */ + if (last_bb->begin == last_bb->end) { + list_del(&last_bb->list); + free(last_bb); + goto out; + } + + /* Point to the insn before the last when adding this block to full_blocks */ + last_bb->end = list_prev_entry(last_bb->end, al.node); + +out: + list_splice(this_blocks, full_blocks); +} + +static void delete_basic_blocks(struct list_head *basic_blocks) +{ + struct annotated_basic_block *bb, *tmp; + + list_for_each_entry_safe(bb, tmp, basic_blocks, list) { + list_del(&bb->list); + free(bb); + } +} + +/* Make sure all variables have a valid start address */ +static void fixup_var_address(struct die_var_type *var_types, u64 addr) +{ + while (var_types) { + /* + * Some variables have no address range meaning it's always + * available in the whole scope. Let's adjust the start + * address to the start of the scope. + */ + if (var_types->addr == 0) + var_types->addr = addr; + + var_types = var_types->next; + } +} + +static void delete_var_types(struct die_var_type *var_types) +{ + while (var_types) { + struct die_var_type *next = var_types->next; + + free(var_types); + var_types = next; + } +} + +/* It's at the target address, check if it has a matching type */ +static bool find_matching_type(struct type_state *state __maybe_unused, + struct data_loc_info *dloc __maybe_unused, + int reg __maybe_unused, + Dwarf_Die *type_die __maybe_unused) +{ + /* TODO */ + return false; +} + +/* Iterate instructions in basic blocks and update type table */ +static bool find_data_type_insn(struct data_loc_info *dloc, int reg, + struct list_head *basic_blocks, + struct die_var_type *var_types, + Dwarf_Die *cu_die, Dwarf_Die *type_die) +{ + struct type_state state; + struct symbol *sym = dloc->ms->sym; + struct annotation *notes = symbol__annotation(sym); + struct annotated_basic_block *bb; + bool found = false; + + init_type_state(&state, dloc->arch); + + list_for_each_entry(bb, basic_blocks, list) { + struct disasm_line *dl = bb->begin; + + pr_debug_dtp("bb: [%"PRIx64" - %"PRIx64"]\n", + bb->begin->al.offset, bb->end->al.offset); + + list_for_each_entry_from(dl, ¬es->src->source, al.node) { + u64 this_ip = sym->start + dl->al.offset; + u64 addr = map__rip_2objdump(dloc->ms->map, this_ip); + + /* Update variable type at this address */ + update_var_state(&state, dloc, addr, dl->al.offset, var_types); + + if (this_ip == dloc->ip) { + found = find_matching_type(&state, dloc, reg, + type_die); + goto out; + } + + /* Update type table after processing the instruction */ + update_insn_state(&state, dloc, cu_die, dl); + if (dl == bb->end) + break; + } + } + +out: + exit_type_state(&state); + return found; +} + +/* + * Construct a list of basic blocks for each scope with variables and try to find + * the data type by updating a type state table through instructions. + */ +static int find_data_type_block(struct data_loc_info *dloc, int reg, + Dwarf_Die *cu_die, Dwarf_Die *scopes, + int nr_scopes, Dwarf_Die *type_die) +{ + LIST_HEAD(basic_blocks); + struct die_var_type *var_types = NULL; + u64 src_ip, dst_ip, prev_dst_ip; + int ret = -1; + + /* TODO: other architecture support */ + if (!arch__is(dloc->arch, "x86")) + return -1; + + prev_dst_ip = dst_ip = dloc->ip; + for (int i = nr_scopes - 1; i >= 0; i--) { + Dwarf_Addr base, start, end; + LIST_HEAD(this_blocks); + + if (dwarf_ranges(&scopes[i], 0, &base, &start, &end) < 0) + break; + + pr_debug_dtp("scope: [%d/%d] (die:%lx)\n", + i + 1, nr_scopes, (long)dwarf_dieoffset(&scopes[i])); + src_ip = map__objdump_2rip(dloc->ms->map, start); + +again: + /* Get basic blocks for this scope */ + if (annotate_get_basic_blocks(dloc->ms->sym, src_ip, dst_ip, + &this_blocks) < 0) { + /* Try previous block if they are not connected */ + if (prev_dst_ip != dst_ip) { + dst_ip = prev_dst_ip; + goto again; + } + + pr_debug_dtp("cannot find a basic block from %"PRIx64" to %"PRIx64"\n", + src_ip - dloc->ms->sym->start, + dst_ip - dloc->ms->sym->start); + continue; + } + prepend_basic_blocks(&this_blocks, &basic_blocks); + + /* Get variable info for this scope and add to var_types list */ + die_collect_vars(&scopes[i], &var_types); + fixup_var_address(var_types, start); + + /* Find from start of this scope to the target instruction */ + if (find_data_type_insn(dloc, reg, &basic_blocks, var_types, + cu_die, type_die)) { + ret = 0; + break; + } + + /* Go up to the next scope and find blocks to the start */ + prev_dst_ip = dst_ip; + dst_ip = src_ip; + } + + delete_basic_blocks(&basic_blocks); + delete_var_types(var_types); + return ret; +} + /* The result will be saved in @type_die */ static int find_data_type_die(struct data_loc_info *dloc, Dwarf_Die *type_die) { @@ -847,6 +1033,15 @@ retry: goto out; } + if (reg != DWARF_REG_PC) { + ret = find_data_type_block(dloc, reg, &cu_die, scopes, + nr_scopes, type_die); + if (ret == 0) { + ann_data_stat.insn_track++; + goto out; + } + } + if (loc->multi_regs && reg == loc->reg1 && loc->reg1 != loc->reg2) { reg = loc->reg2; goto retry; -- cgit From bdc80ace07106a62b51d1752869df29dbd65ad2c Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 18 Mar 2024 22:51:07 -0700 Subject: perf annotate-data: Check register state for type As instruction tracking updates the type state for each register, check the final type info for the target register at the given instruction. Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Linus Torvalds Cc: Masami Hiramatsu Cc: Peter Zijlstra Cc: Stephane Eranian Link: https://lore.kernel.org/r/20240319055115.4063940-16-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 88 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 81 insertions(+), 7 deletions(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index 13ba65693367..f5329a78a97d 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -788,12 +788,83 @@ static void delete_var_types(struct die_var_type *var_types) } /* It's at the target address, check if it has a matching type */ -static bool find_matching_type(struct type_state *state __maybe_unused, - struct data_loc_info *dloc __maybe_unused, - int reg __maybe_unused, - Dwarf_Die *type_die __maybe_unused) +static bool check_matching_type(struct type_state *state, + struct data_loc_info *dloc, int reg, + Dwarf_Die *type_die) { - /* TODO */ + Dwarf_Word size; + u32 insn_offset = dloc->ip - dloc->ms->sym->start; + + pr_debug_dtp("chk [%x] reg%d offset=%#x ok=%d", + insn_offset, reg, dloc->op->offset, state->regs[reg].ok); + + if (state->regs[reg].ok) { + int tag = dwarf_tag(&state->regs[reg].type); + + pr_debug_dtp("\n"); + + /* + * Normal registers should hold a pointer (or array) to + * dereference a memory location. + */ + if (tag != DW_TAG_pointer_type && tag != DW_TAG_array_type) + return false; + + /* Remove the pointer and get the target type */ + if (die_get_real_type(&state->regs[reg].type, type_die) == NULL) + return false; + + 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 false; + + return true; + } + + if (reg == dloc->fbreg) { + struct type_state_stack *stack; + + pr_debug_dtp(" fbreg\n"); + + stack = find_stack_state(state, dloc->type_offset); + if (stack == NULL) + return false; + + *type_die = stack->type; + /* Update the type offset from the start of slot */ + dloc->type_offset -= stack->offset; + + return true; + } + + if (dloc->fb_cfa) { + struct type_state_stack *stack; + u64 pc = map__rip_2objdump(dloc->ms->map, dloc->ip); + int fbreg, fboff; + + pr_debug_dtp(" cfa\n"); + + if (die_get_cfa(dloc->di->dbg, pc, &fbreg, &fboff) < 0) + fbreg = -1; + + if (reg != fbreg) + return false; + + stack = find_stack_state(state, dloc->type_offset - fboff); + if (stack == NULL) + return false; + + *type_die = stack->type; + /* Update the type offset from the start of slot */ + dloc->type_offset -= fboff + stack->offset; + + return true; + } + + pr_debug_dtp("\n"); return false; } @@ -825,8 +896,8 @@ static bool find_data_type_insn(struct data_loc_info *dloc, int reg, update_var_state(&state, dloc, addr, dl->al.offset, var_types); if (this_ip == dloc->ip) { - found = find_matching_type(&state, dloc, reg, - type_die); + found = check_matching_type(&state, dloc, reg, + type_die); goto out; } @@ -896,6 +967,9 @@ again: if (find_data_type_insn(dloc, reg, &basic_blocks, var_types, cu_die, type_die)) { ret = 0; + pr_debug_dtp("found by insn track: %#x(reg%d) type-offset=%#x", + dloc->op->offset, reg, dloc->type_offset); + pr_debug_type_name(type_die); break; } -- cgit From 02e17ca917423c622da10ac6bd0924c17462962e Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 18 Mar 2024 22:51:09 -0700 Subject: perf annotate-data: Handle this-cpu variables in kernel On x86, the kernel gets the current task using the current macro like below: #define current get_current() static __always_inline struct task_struct *get_current(void) { return this_cpu_read_stable(pcpu_hot.current_task); } So it returns the current_task field of struct pcpu_hot which is the first member. On my build, it's located at 0x32940. $ nm vmlinux | grep pcpu_hot 0000000000032940 D pcpu_hot And the current macro generates the instructions like below: mov %gs:0x32940, %rcx So the %gs segment register points to the beginning of the per-cpu region of this cpu and it points the variable with a constant. Let's update the instruction location info to have a segment register and handle %gs in kernel to look up a global variable. Pretend it as a global variable by changing the register number to DWARF_REG_PC. Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Linus Torvalds Cc: Masami Hiramatsu Cc: Peter Zijlstra Cc: Stephane Eranian Link: https://lore.kernel.org/r/20240319055115.4063940-18-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index f5329a78a97d..d57622ddd5d3 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -790,7 +790,7 @@ static void delete_var_types(struct die_var_type *var_types) /* It's at the target address, check if it has a matching type */ static bool check_matching_type(struct type_state *state, struct data_loc_info *dloc, int reg, - Dwarf_Die *type_die) + Dwarf_Die *cu_die, Dwarf_Die *type_die) { Dwarf_Word size; u32 insn_offset = dloc->ip - dloc->ms->sym->start; @@ -864,6 +864,23 @@ static bool check_matching_type(struct type_state *state, return true; } + if (map__dso(dloc->ms->map)->kernel && arch__is(dloc->arch, "x86")) { + u64 addr; + int offset; + + if (dloc->op->segment == INSN_SEG_X86_GS && dloc->op->imm) { + pr_debug_dtp(" this-cpu var\n"); + + addr = dloc->op->offset; + + if (get_global_var_type(cu_die, dloc, dloc->ip, addr, + &offset, type_die)) { + dloc->type_offset = offset; + return true; + } + } + } + pr_debug_dtp("\n"); return false; } @@ -897,7 +914,7 @@ static bool find_data_type_insn(struct data_loc_info *dloc, int reg, if (this_ip == dloc->ip) { found = check_matching_type(&state, dloc, reg, - type_die); + cu_die, type_die); goto out; } -- cgit From ad62edbfc55b23347539c8c6fff9a70de17c4b95 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 18 Mar 2024 22:51:10 -0700 Subject: perf annotate-data: Track instructions with a this-cpu variable Like global variables, this per-cpu variables should be tracked correctly. Factor our get_global_var_type() to handle both global and per-cpu (for this cpu) variables in the same manner. Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Linus Torvalds Cc: Masami Hiramatsu Cc: Peter Zijlstra Cc: Stephane Eranian Link: https://lore.kernel.org/r/20240319055115.4063940-19-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index d57622ddd5d3..48fea0c716ef 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -553,12 +553,41 @@ static void update_insn_state_x86(struct type_state *state, fbreg = -1; } - /* Case 1. register to register transfers */ + /* Case 1. register to register or segment:offset to register transfers */ if (!src->mem_ref && !dst->mem_ref) { if (!has_reg_type(state, dst->reg1)) return; tsr = &state->regs[dst->reg1]; + if (map__dso(dloc->ms->map)->kernel && + src->segment == INSN_SEG_X86_GS && src->imm) { + u64 ip = dloc->ms->sym->start + dl->al.offset; + u64 var_addr; + int offset; + + /* + * In kernel, %gs points to a per-cpu region for the + * current CPU. Access with a constant offset should + * be treated as a global variable access. + */ + var_addr = src->offset; + + if (!get_global_var_type(cu_die, dloc, ip, var_addr, + &offset, &type_die) || + !die_get_member_type(&type_die, offset, &type_die)) { + tsr->ok = false; + return; + } + + tsr->type = type_die; + tsr->ok = true; + + pr_debug_dtp("mov [%x] this-cpu addr=%#"PRIx64" -> reg%d", + insn_offset, var_addr, dst->reg1); + pr_debug_type_name(&tsr->type); + return; + } + if (!has_reg_type(state, src->reg1) || !state->regs[src->reg1].ok) { tsr->ok = false; -- cgit From f5b095924d0c1c2f0698df07c54887d92c39fd3a Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 18 Mar 2024 22:51:11 -0700 Subject: perf annotate-data: Support general per-cpu access This is to support per-cpu variable access often without a matching DWARF entry. For some reason, I cannot find debug info of per-cpu variables sometimes. They have more complex pattern to calculate the address of per-cpu variables like below. 2b7d: mov -0x1e0(%rbp),%rax ; rax = cpu 2b84: mov -0x7da0f7e0(,%rax,8),%rcx ; rcx = __per_cpu_offset[cpu] * 2b8c: mov 0x34870(%rcx),%rax ; *(__per_cpu_offset[cpu] + 0x34870) Let's assume the rax register has a number for a CPU at 2b7d. The next instruction is to get the per-cpu offset' for that cpu. The offset -0x7da0f7e0 is 0xffffffff825f0820 in u64 which is the address of the '__per_cpu_offset' array in my system. So it'd get the actual offset of that CPU's per-cpu region and save it to the rcx register. Then, at 2b8c, accesses using rcx can be handled same as the global variable access. To handle this case, it should check if the offset of the instruction matches to the address of '__per_cpu_offset'. Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Linus Torvalds Cc: Masami Hiramatsu Cc: Peter Zijlstra Cc: Stephane Eranian Link: https://lore.kernel.org/r/20240319055115.4063940-20-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 213 +++++++++++++++++++++++++++++++--------- 1 file changed, 169 insertions(+), 44 deletions(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index 48fea0c716ef..83b5aa00f01c 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -24,6 +24,12 @@ #include "symbol_conf.h" #include "thread.h" +enum type_state_kind { + TSR_KIND_INVALID = 0, + TSR_KIND_TYPE, + TSR_KIND_PERCPU_BASE, +}; + #define pr_debug_dtp(fmt, ...) \ do { \ if (debug_type_profile) \ @@ -32,7 +38,7 @@ do { \ pr_debug3(fmt, ##__VA_ARGS__); \ } while (0) -static void pr_debug_type_name(Dwarf_Die *die) +static void pr_debug_type_name(Dwarf_Die *die, enum type_state_kind kind) { struct strbuf sb; char *str; @@ -40,6 +46,18 @@ static void pr_debug_type_name(Dwarf_Die *die) if (!debug_type_profile && verbose < 3) return; + switch (kind) { + case TSR_KIND_INVALID: + pr_info("\n"); + return; + case TSR_KIND_PERCPU_BASE: + pr_info(" percpu base\n"); + return; + case TSR_KIND_TYPE: + default: + break; + } + strbuf_init(&sb, 32); die_get_typename_from_type(die, &sb); str = strbuf_detach(&sb, NULL); @@ -53,8 +71,10 @@ static void pr_debug_type_name(Dwarf_Die *die) */ struct type_state_reg { Dwarf_Die type; + u32 imm_value; bool ok; bool caller_saved; + u8 kind; }; /* Type information in a stack location, dynamically allocated */ @@ -64,6 +84,7 @@ struct type_state_stack { int offset; int size; bool compound; + u8 kind; }; /* FIXME: This should be arch-dependent */ @@ -82,6 +103,8 @@ struct type_state { struct list_head stack_vars; /* return value register */ int ret_reg; + /* stack pointer register */ + int stack_reg; }; static bool has_reg_type(struct type_state *state, int reg) @@ -105,6 +128,7 @@ static void init_type_state(struct type_state *state, struct arch *arch) state->regs[10].caller_saved = true; state->regs[11].caller_saved = true; state->ret_reg = 0; + state->stack_reg = 7; } } @@ -350,7 +374,7 @@ static struct type_state_stack *find_stack_state(struct type_state *state, return NULL; } -static void set_stack_state(struct type_state_stack *stack, int offset, +static void set_stack_state(struct type_state_stack *stack, int offset, u8 kind, Dwarf_Die *type_die) { int tag; @@ -364,6 +388,7 @@ static void set_stack_state(struct type_state_stack *stack, int offset, stack->type = *type_die; stack->size = size; stack->offset = offset; + stack->kind = kind; switch (tag) { case DW_TAG_structure_type: @@ -377,34 +402,60 @@ static void set_stack_state(struct type_state_stack *stack, int offset, } static struct type_state_stack *findnew_stack_state(struct type_state *state, - int offset, Dwarf_Die *type_die) + int offset, u8 kind, + Dwarf_Die *type_die) { struct type_state_stack *stack = find_stack_state(state, offset); if (stack) { - set_stack_state(stack, offset, type_die); + set_stack_state(stack, offset, kind, type_die); return stack; } stack = malloc(sizeof(*stack)); if (stack) { - set_stack_state(stack, offset, type_die); + set_stack_state(stack, offset, kind, type_die); list_add(&stack->list, &state->stack_vars); } return stack; } +static bool get_global_var_info(struct data_loc_info *dloc, u64 addr, + const char **var_name, int *var_offset) +{ + struct addr_location al; + struct symbol *sym; + u64 mem_addr; + + /* Kernel symbols might be relocated */ + mem_addr = addr + map__reloc(dloc->ms->map); + + addr_location__init(&al); + sym = thread__find_symbol_fb(dloc->thread, dloc->cpumode, + mem_addr, &al); + if (sym) { + *var_name = sym->name; + /* Calculate type offset from the start of variable */ + *var_offset = mem_addr - map__unmap_ip(al.map, sym->start); + } else { + *var_name = NULL; + } + addr_location__exit(&al); + if (*var_name == NULL) + return false; + + return true; +} + static bool get_global_var_type(Dwarf_Die *cu_die, struct data_loc_info *dloc, u64 ip, u64 var_addr, int *var_offset, Dwarf_Die *type_die) { - u64 pc, mem_addr; + u64 pc; int offset; bool is_pointer = false; - const char *var_name = NULL; + const char *var_name; Dwarf_Die var_die; - struct addr_location al; - struct symbol *sym; /* Try to get the variable by address first */ if (die_find_variable_by_addr(cu_die, var_addr, &var_die, &offset) && @@ -413,19 +464,7 @@ static bool get_global_var_type(Dwarf_Die *cu_die, struct data_loc_info *dloc, return true; } - /* Kernel symbols might be relocated */ - mem_addr = var_addr + map__reloc(dloc->ms->map); - - addr_location__init(&al); - sym = thread__find_symbol_fb(dloc->thread, dloc->cpumode, - mem_addr, &al); - if (sym) { - var_name = sym->name; - /* Calculate type offset from the start of variable */ - *var_offset = mem_addr - map__unmap_ip(al.map, sym->start); - } - addr_location__exit(&al); - if (var_name == NULL) + if (!get_global_var_info(dloc, var_addr, &var_name, var_offset)) return false; pc = map__rip_2objdump(dloc->ms->map, ip); @@ -470,27 +509,30 @@ static void update_var_state(struct type_state *state, struct data_loc_info *dlo continue; if (var->reg == DWARF_REG_FB) { - findnew_stack_state(state, var->offset, &mem_die); + findnew_stack_state(state, var->offset, TSR_KIND_TYPE, + &mem_die); pr_debug_dtp("var [%"PRIx64"] -%#x(stack)", insn_offset, -var->offset); - pr_debug_type_name(&mem_die); + pr_debug_type_name(&mem_die, TSR_KIND_TYPE); } else if (var->reg == fbreg) { - findnew_stack_state(state, var->offset - fb_offset, &mem_die); + findnew_stack_state(state, var->offset - fb_offset, + TSR_KIND_TYPE, &mem_die); pr_debug_dtp("var [%"PRIx64"] -%#x(stack)", insn_offset, -var->offset + fb_offset); - pr_debug_type_name(&mem_die); + pr_debug_type_name(&mem_die, TSR_KIND_TYPE); } else if (has_reg_type(state, var->reg) && var->offset == 0) { struct type_state_reg *reg; reg = &state->regs[var->reg]; 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_type_name(&mem_die); + pr_debug_type_name(&mem_die, TSR_KIND_TYPE); } } } @@ -533,11 +575,12 @@ static void update_insn_state_x86(struct type_state *state, if (die_find_func_rettype(cu_die, func->name, &type_die)) { tsr = &state->regs[state->ret_reg]; tsr->type = type_die; + tsr->kind = TSR_KIND_TYPE; tsr->ok = true; pr_debug_dtp("call [%x] return -> reg%d", insn_offset, state->ret_reg); - pr_debug_type_name(&type_die); + pr_debug_type_name(&type_die, tsr->kind); } return; } @@ -580,11 +623,12 @@ static void update_insn_state_x86(struct type_state *state, } tsr->type = type_die; + tsr->kind = TSR_KIND_TYPE; tsr->ok = true; pr_debug_dtp("mov [%x] this-cpu addr=%#"PRIx64" -> reg%d", insn_offset, var_addr, dst->reg1); - pr_debug_type_name(&tsr->type); + pr_debug_type_name(&tsr->type, tsr->kind); return; } @@ -595,11 +639,12 @@ 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->ok = true; pr_debug_dtp("mov [%x] reg%d -> reg%d", insn_offset, src->reg1, dst->reg1); - pr_debug_type_name(&tsr->type); + pr_debug_type_name(&tsr->type, tsr->kind); } /* Case 2. memory to register transers */ if (src->mem_ref && !dst->mem_ref) { @@ -622,11 +667,13 @@ retry: return; } else if (!stack->compound) { tsr->type = stack->type; + tsr->kind = stack->kind; 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->ok = true; } else { tsr->ok = false; @@ -635,18 +682,20 @@ retry: pr_debug_dtp("mov [%x] -%#x(stack) -> reg%d", insn_offset, -offset, dst->reg1); - pr_debug_type_name(&tsr->type); + pr_debug_type_name(&tsr->type, tsr->kind); } /* And then dereference the pointer if it has one */ 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)) { tsr->type = type_die; + tsr->kind = TSR_KIND_TYPE; tsr->ok = true; pr_debug_dtp("mov [%x] %#x(reg%d) -> reg%d", insn_offset, src->offset, sreg, dst->reg1); - pr_debug_type_name(&tsr->type); + pr_debug_type_name(&tsr->type, tsr->kind); } /* Or check if it's a global variable */ else if (sreg == DWARF_REG_PC) { @@ -665,11 +714,37 @@ retry: } tsr->type = type_die; + tsr->kind = TSR_KIND_TYPE; tsr->ok = true; pr_debug_dtp("mov [%x] global addr=%"PRIx64" -> reg%d", insn_offset, addr, dst->reg1); - pr_debug_type_name(&type_die); + pr_debug_type_name(&type_die, tsr->kind); + } + /* And check percpu access with base register */ + else if (has_reg_type(state, sreg) && + state->regs[sreg].kind == TSR_KIND_PERCPU_BASE) { + u64 ip = dloc->ms->sym->start + dl->al.offset; + int offset; + + /* + * In kernel, %gs points to a per-cpu region for the + * current CPU. Access with a constant offset should + * be treated as a global variable access. + */ + if (get_global_var_type(cu_die, dloc, ip, src->offset, + &offset, &type_die) && + die_get_member_type(&type_die, offset, &type_die)) { + tsr->type = type_die; + tsr->kind = TSR_KIND_TYPE; + tsr->ok = true; + + pr_debug_dtp("mov [%x] percpu %#x(reg%d) -> reg%d type=", + insn_offset, src->offset, sreg, dst->reg1); + pr_debug_type_name(&tsr->type, tsr->kind); + } else { + tsr->ok = false; + } } /* Or try another register if any */ else if (src->multi_regs && sreg == src->reg1 && @@ -677,8 +752,22 @@ retry: sreg = src->reg2; goto retry; } - /* It failed to get a type info, mark it as invalid */ else { + int offset; + const char *var_name = NULL; + + /* it might be per-cpu variable (in kernel) access */ + if (src->offset < 0) { + if (get_global_var_info(dloc, (s64)src->offset, + &var_name, &offset) && + !strcmp(var_name, "__per_cpu_offset")) { + tsr->kind = TSR_KIND_PERCPU_BASE; + + pr_debug_dtp("mov [%x] percpu base reg%d\n", + insn_offset, dst->reg1); + } + } + tsr->ok = false; } } @@ -693,6 +782,8 @@ retry: struct type_state_stack *stack; int offset = dst->offset - fboff; + tsr = &state->regs[src->reg1]; + stack = find_stack_state(state, offset); if (stack) { /* @@ -703,16 +794,16 @@ retry: * die_get_member_type(). */ if (!stack->compound) - set_stack_state(stack, offset, - &state->regs[src->reg1].type); + set_stack_state(stack, offset, tsr->kind, + &tsr->type); } else { - findnew_stack_state(state, offset, - &state->regs[src->reg1].type); + findnew_stack_state(state, offset, tsr->kind, + &tsr->type); } pr_debug_dtp("mov [%x] reg%d -> -%#x(stack)", insn_offset, src->reg1, -offset); - pr_debug_type_name(&state->regs[src->reg1].type); + pr_debug_type_name(&tsr->type, tsr->kind); } /* * Ignore other transfers since it'd set a value in a struct @@ -824,10 +915,11 @@ static bool check_matching_type(struct type_state *state, Dwarf_Word size; u32 insn_offset = dloc->ip - dloc->ms->sym->start; - pr_debug_dtp("chk [%x] reg%d offset=%#x ok=%d", - insn_offset, reg, dloc->op->offset, state->regs[reg].ok); + pr_debug_dtp("chk [%x] reg%d offset=%#x ok=%d kind=%d", + insn_offset, reg, dloc->op->offset, + state->regs[reg].ok, state->regs[reg].kind); - if (state->regs[reg].ok) { + if (state->regs[reg].ok && state->regs[reg].kind == TSR_KIND_TYPE) { int tag = dwarf_tag(&state->regs[reg].type); pr_debug_dtp("\n"); @@ -893,10 +985,25 @@ static bool check_matching_type(struct type_state *state, return true; } + if (state->regs[reg].kind == TSR_KIND_PERCPU_BASE) { + u64 var_addr = dloc->op->offset; + int var_offset; + + pr_debug_dtp(" percpu var\n"); + + if (get_global_var_type(cu_die, dloc, dloc->ip, var_addr, + &var_offset, type_die)) { + dloc->type_offset = var_offset; + return true; + } + return false; + } + if (map__dso(dloc->ms->map)->kernel && arch__is(dloc->arch, "x86")) { u64 addr; int offset; + /* Direct this-cpu access like "%gs:0x34740" */ if (dloc->op->segment == INSN_SEG_X86_GS && dloc->op->imm) { pr_debug_dtp(" this-cpu var\n"); @@ -907,6 +1014,24 @@ static bool check_matching_type(struct type_state *state, dloc->type_offset = offset; return true; } + return false; + } + + /* Access to per-cpu base like "-0x7dcf0500(,%rdx,8)" */ + if (dloc->op->offset < 0 && reg != state->stack_reg) { + const char *var_name = NULL; + + addr = (s64) dloc->op->offset; + + if (get_global_var_info(dloc, addr, &var_name, &offset) && + !strcmp(var_name, "__per_cpu_offset") && offset == 0 && + get_global_var_type(cu_die, dloc, dloc->ip, addr, + &offset, type_die)) { + pr_debug_dtp(" percpu base\n"); + + dloc->type_offset = offset; + return true; + } } } @@ -1015,7 +1140,7 @@ again: ret = 0; pr_debug_dtp("found by insn track: %#x(reg%d) type-offset=%#x", dloc->op->offset, reg, dloc->type_offset); - pr_debug_type_name(type_die); + pr_debug_type_name(type_die, TSR_KIND_TYPE); break; } @@ -1147,7 +1272,7 @@ retry: loc->offset, reg, fb_offset, offset); else pr_debug_dtp("%#x(reg%d)", loc->offset, reg); - pr_debug_type_name(type_die); + pr_debug_type_name(type_die, TSR_KIND_TYPE); } dloc->type_offset = offset; goto out; -- cgit From eb9190afaed6afd5ed54bb0a3269eec338663858 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 18 Mar 2024 22:51:12 -0700 Subject: perf annotate-data: Handle ADD instructions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are different patterns for percpu variable access using a constant value added to the base.  2aeb:  mov    -0x7da0f7e0(,%rax,8),%r14 # r14 = __per_cpu_offset[cpu]  2af3:  mov    $0x34740,%rax # rax = address of runqueues * 2afa:  add    %rax,%r14 # r14 = &per_cpu(runqueues, cpu)  2bfd:  cmpl   $0x0,0x10(%r14) # cpu_rq(cpu)->has_blocked_load  2b03:  je     0x2b36 At the first instruction, r14 has the __per_cpu_offset. And then rax has an immediate value and then added to r14 to calculate the address of a per-cpu variable. So it needs to track the immediate values and ADD instructions. Similar but a little different case is to use "this_cpu_off" instead of "__per_cpu_offset" for the current CPU. This time the variable address comes with PC-rel addressing. 89: mov $0x34740,%rax # rax = address of runqueues * 90: add %gs:0x7f015f60(%rip),%rax # 19a78 98: incl 0xd8c(%rax) # cpu_rq(cpu)->sched_count Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Linus Torvalds Cc: Masami Hiramatsu Cc: Peter Zijlstra Cc: Stephane Eranian Link: https://lore.kernel.org/r/20240319055115.4063940-21-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 107 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 105 insertions(+), 2 deletions(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index 83b5aa00f01c..bd10a576cfbf 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -28,6 +28,8 @@ enum type_state_kind { TSR_KIND_INVALID = 0, TSR_KIND_TYPE, TSR_KIND_PERCPU_BASE, + TSR_KIND_CONST, + TSR_KIND_POINTER, }; #define pr_debug_dtp(fmt, ...) \ @@ -53,6 +55,13 @@ static void pr_debug_type_name(Dwarf_Die *die, enum type_state_kind kind) case TSR_KIND_PERCPU_BASE: pr_info(" percpu base\n"); return; + case TSR_KIND_CONST: + pr_info(" constant\n"); + return; + case TSR_KIND_POINTER: + pr_info(" pointer"); + /* it also prints the type info */ + break; case TSR_KIND_TYPE: default: break; @@ -393,7 +402,7 @@ static 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 = true; + stack->compound = (kind != TSR_KIND_POINTER); break; default: stack->compound = false; @@ -585,6 +594,58 @@ static void update_insn_state_x86(struct type_state *state, return; } + if (!strncmp(dl->ins.name, "add", 3)) { + u64 imm_value = -1ULL; + int offset; + const char *var_name = NULL; + struct map_symbol *ms = dloc->ms; + u64 ip = ms->sym->start + dl->al.offset; + + if (!has_reg_type(state, dst->reg1)) + return; + + tsr = &state->regs[dst->reg1]; + + 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; + else if (src->reg1 == DWARF_REG_PC) { + u64 var_addr = annotate_calc_pcrel(dloc->ms, ip, + src->offset, dl); + + if (get_global_var_info(dloc, var_addr, + &var_name, &offset) && + !strcmp(var_name, "this_cpu_off") && + tsr->kind == TSR_KIND_CONST) { + tsr->kind = TSR_KIND_PERCPU_BASE; + imm_value = tsr->imm_value; + } + } + else + return; + + if (tsr->kind != TSR_KIND_PERCPU_BASE) + return; + + if (get_global_var_type(cu_die, dloc, ip, imm_value, &offset, + &type_die) && offset == 0) { + /* + * This is not a pointer type, but it should be treated + * as a pointer. + */ + tsr->type = type_die; + tsr->kind = TSR_KIND_POINTER; + tsr->ok = true; + + pr_debug_dtp("add [%x] percpu %#"PRIx64" -> reg%d", + insn_offset, imm_value, dst->reg1); + pr_debug_type_name(&tsr->type, tsr->kind); + } + return; + } + if (strncmp(dl->ins.name, "mov", 3)) return; @@ -632,6 +693,16 @@ static void update_insn_state_x86(struct type_state *state, return; } + if (src->imm) { + tsr->kind = TSR_KIND_CONST; + tsr->imm_value = src->offset; + tsr->ok = true; + + pr_debug_dtp("mov [%x] imm=%#x -> reg%d\n", + insn_offset, tsr->imm_value, dst->reg1); + return; + } + if (!has_reg_type(state, src->reg1) || !state->regs[src->reg1].ok) { tsr->ok = false; @@ -739,13 +810,26 @@ retry: tsr->kind = TSR_KIND_TYPE; tsr->ok = true; - pr_debug_dtp("mov [%x] percpu %#x(reg%d) -> reg%d type=", + pr_debug_dtp("mov [%x] percpu %#x(reg%d) -> reg%d", insn_offset, src->offset, sreg, dst->reg1); pr_debug_type_name(&tsr->type, tsr->kind); } else { tsr->ok = false; } } + /* 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 && + die_get_member_type(&state->regs[sreg].type, + src->offset, &type_die)) { + tsr->type = type_die; + tsr->kind = TSR_KIND_TYPE; + tsr->ok = true; + + pr_debug_dtp("mov [%x] pointer %#x(reg%d) -> reg%d", + insn_offset, src->offset, sreg, dst->reg1); + pr_debug_type_name(&tsr->type, tsr->kind); + } /* Or try another register if any */ else if (src->multi_regs && sreg == src->reg1 && src->reg1 != src->reg2) { @@ -999,6 +1083,25 @@ static bool check_matching_type(struct type_state *state, return false; } + if (state->regs[reg].ok && state->regs[reg].kind == TSR_KIND_POINTER) { + pr_debug_dtp(" percpu ptr\n"); + + /* + * It's actaully pointer but the address was calculated using + * some arithmetic. So it points to the actual type already. + */ + *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 false; + + return true; + } + if (map__dso(dloc->ms->map)->kernel && arch__is(dloc->arch, "x86")) { u64 addr; int offset; -- cgit From b3c95109c131fcc959d2473e7c384d8cc62d23d0 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 18 Mar 2024 22:51:13 -0700 Subject: perf annotate-data: Add stack canary type When the stack protector is enabled, compiler would generate code to check stack overflow with a special value called 'stack carary' at runtime. On x86_64, GCC hard-codes the stack canary as %gs:40. While there's a definition of fixed_percpu_data in asm/processor.h, it seems that the header is not included everywhere and many places it cannot find the type info. As it's in the well-known location (at %gs:40), let's add a pseudo stack canary type to handle it specially. Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Linus Torvalds Cc: Masami Hiramatsu Cc: Peter Zijlstra Cc: Stephane Eranian Link: https://lore.kernel.org/r/20240319055115.4063940-22-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 46 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index bd10a576cfbf..633fe125fcd8 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -30,6 +30,7 @@ enum type_state_kind { TSR_KIND_PERCPU_BASE, TSR_KIND_CONST, TSR_KIND_POINTER, + TSR_KIND_CANARY, }; #define pr_debug_dtp(fmt, ...) \ @@ -62,6 +63,9 @@ static void pr_debug_type_name(Dwarf_Die *die, enum type_state_kind kind) pr_info(" pointer"); /* it also prints the type info */ break; + case TSR_KIND_CANARY: + pr_info(" stack canary\n"); + return; case TSR_KIND_TYPE: default: break; @@ -676,6 +680,15 @@ static void update_insn_state_x86(struct type_state *state, */ var_addr = src->offset; + if (var_addr == 40) { + tsr->kind = TSR_KIND_CANARY; + tsr->ok = true; + + pr_debug_dtp("mov [%x] stack canary -> reg%d\n", + insn_offset, dst->reg1); + return; + } + if (!get_global_var_type(cu_die, dloc, ip, var_addr, &offset, &type_die) || !die_get_member_type(&type_die, offset, &type_die)) { @@ -991,6 +1004,16 @@ static void delete_var_types(struct die_var_type *var_types) } } +/* should match to is_stack_canary() in util/annotate.c */ +static void setup_stack_canary(struct data_loc_info *dloc) +{ + if (arch__is(dloc->arch, "x86")) { + dloc->op->segment = INSN_SEG_X86_GS; + dloc->op->imm = true; + dloc->op->offset = 40; + } +} + /* It's at the target address, check if it has a matching type */ static bool check_matching_type(struct type_state *state, struct data_loc_info *dloc, int reg, @@ -1038,6 +1061,11 @@ static bool check_matching_type(struct type_state *state, if (stack == NULL) return false; + if (stack->kind == TSR_KIND_CANARY) { + setup_stack_canary(dloc); + return false; + } + *type_die = stack->type; /* Update the type offset from the start of slot */ dloc->type_offset -= stack->offset; @@ -1062,6 +1090,11 @@ static bool check_matching_type(struct type_state *state, if (stack == NULL) return false; + if (stack->kind == TSR_KIND_CANARY) { + setup_stack_canary(dloc); + return false; + } + *type_die = stack->type; /* Update the type offset from the start of slot */ dloc->type_offset -= fboff + stack->offset; @@ -1102,6 +1135,19 @@ static bool check_matching_type(struct type_state *state, return true; } + if (state->regs[reg].ok && state->regs[reg].kind == TSR_KIND_CANARY) { + pr_debug_dtp(" stack canary\n"); + + /* + * This is a saved value of the stack canary which will be handled + * in the outer logic when it returns failure here. Pretend it's + * from the stack canary directly. + */ + setup_stack_canary(dloc); + + return false; + } + if (map__dso(dloc->ms->map)->kernel && arch__is(dloc->arch, "x86")) { u64 addr; int offset; -- cgit From 55ee3d005d62279d3951a26d5c211a4d9aebc222 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 18 Mar 2024 22:51:14 -0700 Subject: perf annotate-data: Add a cache for global variable types They are often searched by many different places. Let's add a cache for them to reduce the duplicate DWARF access. Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Linus Torvalds Cc: Masami Hiramatsu Cc: Peter Zijlstra Cc: Stephane Eranian Link: https://lore.kernel.org/r/20240319055115.4063940-23-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 107 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 104 insertions(+), 3 deletions(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index 633fe125fcd8..969e2f82079c 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -433,6 +433,91 @@ static struct type_state_stack *findnew_stack_state(struct type_state *state, return stack; } +/* Maintain a cache for quick global variable lookup */ +struct global_var_entry { + struct rb_node node; + char *name; + u64 start; + u64 end; + u64 die_offset; +}; + +static int global_var_cmp(const void *_key, const struct rb_node *node) +{ + const u64 addr = (uintptr_t)_key; + struct global_var_entry *gvar; + + gvar = rb_entry(node, struct global_var_entry, node); + + if (gvar->start <= addr && addr < gvar->end) + return 0; + return gvar->start > addr ? -1 : 1; +} + +static bool global_var_less(struct rb_node *node_a, const struct rb_node *node_b) +{ + struct global_var_entry *gvar_a, *gvar_b; + + gvar_a = rb_entry(node_a, struct global_var_entry, node); + gvar_b = rb_entry(node_b, struct global_var_entry, node); + + return gvar_a->start < gvar_b->start; +} + +static struct global_var_entry *global_var__find(struct data_loc_info *dloc, u64 addr) +{ + struct dso *dso = map__dso(dloc->ms->map); + struct rb_node *node; + + node = rb_find((void *)(uintptr_t)addr, &dso->global_vars, global_var_cmp); + if (node == NULL) + return NULL; + + return rb_entry(node, struct global_var_entry, node); +} + +static bool global_var__add(struct data_loc_info *dloc, u64 addr, + const char *name, Dwarf_Die *type_die) +{ + struct dso *dso = map__dso(dloc->ms->map); + struct global_var_entry *gvar; + Dwarf_Word size; + + if (dwarf_aggregate_size(type_die, &size) < 0) + return false; + + gvar = malloc(sizeof(*gvar)); + if (gvar == NULL) + return false; + + gvar->name = strdup(name); + if (gvar->name == NULL) { + free(gvar); + return false; + } + + gvar->start = addr; + gvar->end = addr + size; + gvar->die_offset = dwarf_dieoffset(type_die); + + rb_add(&gvar->node, &dso->global_vars, global_var_less); + return true; +} + +void global_var_type__tree_delete(struct rb_root *root) +{ + struct global_var_entry *gvar; + + while (!RB_EMPTY_ROOT(root)) { + struct rb_node *node = rb_first(root); + + rb_erase(node, root); + gvar = rb_entry(node, struct global_var_entry, node); + free(gvar->name); + free(gvar); + } +} + static bool get_global_var_info(struct data_loc_info *dloc, u64 addr, const char **var_name, int *var_offset) { @@ -467,14 +552,25 @@ static bool get_global_var_type(Dwarf_Die *cu_die, struct data_loc_info *dloc, u64 pc; int offset; bool is_pointer = false; - const char *var_name; + const char *var_name = NULL; + struct global_var_entry *gvar; Dwarf_Die var_die; + gvar = global_var__find(dloc, var_addr); + if (gvar) { + if (!dwarf_offdie(dloc->di->dbg, gvar->die_offset, type_die)) + return false; + + *var_offset = var_addr - gvar->start; + return true; + } + /* Try to get the variable by address first */ if (die_find_variable_by_addr(cu_die, var_addr, &var_die, &offset) && check_variable(&var_die, type_die, offset, is_pointer) == 0) { + var_name = dwarf_diename(&var_die); *var_offset = offset; - return true; + goto ok; } if (!get_global_var_info(dloc, var_addr, &var_name, var_offset)) @@ -485,9 +581,14 @@ static bool get_global_var_type(Dwarf_Die *cu_die, struct data_loc_info *dloc, /* Try to get the name of global variable */ if (die_find_variable_at(cu_die, var_name, pc, &var_die) && check_variable(&var_die, type_die, *var_offset, is_pointer) == 0) - return true; + goto ok; return false; + +ok: + /* The address should point to the start of the variable */ + global_var__add(dloc, var_addr - *var_offset, var_name, type_die); + return true; } /** -- cgit From bd62de08084c24a4ff1b1875d53cc4cc1ea2312d Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 18 Mar 2024 22:51:15 -0700 Subject: perf annotate-data: Do not retry for invalid types In some cases, it was able to find a type or location info (for per-cpu variable) but cannot match because of invalid offset or missing global information. In those cases, it's meaningless to go to the outer scope and retry because there will be no additional information. Let's change the return type of find_matching_type() and bail out if it returns -1 for the cases. Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Linus Torvalds Cc: Masami Hiramatsu Cc: Peter Zijlstra Cc: Stephane Eranian Link: https://lore.kernel.org/r/20240319055115.4063940-24-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 83 ++++++++++++++++++++++++----------------- 1 file changed, 48 insertions(+), 35 deletions(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index 969e2f82079c..043d80791bd0 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -1115,10 +1115,15 @@ static void setup_stack_canary(struct data_loc_info *dloc) } } -/* It's at the target address, check if it has a matching type */ -static bool check_matching_type(struct type_state *state, - struct data_loc_info *dloc, int reg, - Dwarf_Die *cu_die, Dwarf_Die *type_die) +/* + * It's at the target address, check if it has a matching type. + * It returns 1 if found, 0 if not or -1 if not found but no need to + * repeat the search. The last case is for per-cpu variables which + * are similar to global variables and no additional info is needed. + */ +static int check_matching_type(struct type_state *state, + struct data_loc_info *dloc, int reg, + Dwarf_Die *cu_die, Dwarf_Die *type_die) { Dwarf_Word size; u32 insn_offset = dloc->ip - dloc->ms->sym->start; @@ -1137,20 +1142,20 @@ static bool check_matching_type(struct type_state *state, * dereference a memory location. */ if (tag != DW_TAG_pointer_type && tag != DW_TAG_array_type) - return false; + return -1; /* Remove the pointer and get the target type */ if (die_get_real_type(&state->regs[reg].type, type_die) == NULL) - return false; + return -1; 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 false; + return -1; - return true; + return 1; } if (reg == dloc->fbreg) { @@ -1160,18 +1165,18 @@ static bool check_matching_type(struct type_state *state, stack = find_stack_state(state, dloc->type_offset); if (stack == NULL) - return false; + return 0; if (stack->kind == TSR_KIND_CANARY) { setup_stack_canary(dloc); - return false; + return -1; } *type_die = stack->type; /* Update the type offset from the start of slot */ dloc->type_offset -= stack->offset; - return true; + return 1; } if (dloc->fb_cfa) { @@ -1185,22 +1190,22 @@ static bool check_matching_type(struct type_state *state, fbreg = -1; if (reg != fbreg) - return false; + return 0; stack = find_stack_state(state, dloc->type_offset - fboff); if (stack == NULL) - return false; + return 0; if (stack->kind == TSR_KIND_CANARY) { setup_stack_canary(dloc); - return false; + return -1; } *type_die = stack->type; /* Update the type offset from the start of slot */ dloc->type_offset -= fboff + stack->offset; - return true; + return 1; } if (state->regs[reg].kind == TSR_KIND_PERCPU_BASE) { @@ -1212,9 +1217,10 @@ static bool check_matching_type(struct type_state *state, if (get_global_var_type(cu_die, dloc, dloc->ip, var_addr, &var_offset, type_die)) { dloc->type_offset = var_offset; - return true; + return 1; } - return false; + /* No need to retry per-cpu (global) variables */ + return -1; } if (state->regs[reg].ok && state->regs[reg].kind == TSR_KIND_POINTER) { @@ -1231,9 +1237,9 @@ static bool check_matching_type(struct type_state *state, /* Get the size of the actual type */ if (dwarf_aggregate_size(type_die, &size) < 0 || (unsigned)dloc->type_offset >= size) - return false; + return -1; - return true; + return 1; } if (state->regs[reg].ok && state->regs[reg].kind == TSR_KIND_CANARY) { @@ -1246,7 +1252,7 @@ static bool check_matching_type(struct type_state *state, */ setup_stack_canary(dloc); - return false; + return -1; } if (map__dso(dloc->ms->map)->kernel && arch__is(dloc->arch, "x86")) { @@ -1262,9 +1268,9 @@ static bool check_matching_type(struct type_state *state, if (get_global_var_type(cu_die, dloc, dloc->ip, addr, &offset, type_die)) { dloc->type_offset = offset; - return true; + return 1; } - return false; + return -1; } /* Access to per-cpu base like "-0x7dcf0500(,%rdx,8)" */ @@ -1280,26 +1286,28 @@ static bool check_matching_type(struct type_state *state, pr_debug_dtp(" percpu base\n"); dloc->type_offset = offset; - return true; + return 1; } + pr_debug_dtp(" negative offset\n"); + return -1; } } pr_debug_dtp("\n"); - return false; + return 0; } /* Iterate instructions in basic blocks and update type table */ -static bool find_data_type_insn(struct data_loc_info *dloc, int reg, - struct list_head *basic_blocks, - struct die_var_type *var_types, - Dwarf_Die *cu_die, Dwarf_Die *type_die) +static int find_data_type_insn(struct data_loc_info *dloc, int reg, + struct list_head *basic_blocks, + struct die_var_type *var_types, + Dwarf_Die *cu_die, Dwarf_Die *type_die) { struct type_state state; struct symbol *sym = dloc->ms->sym; struct annotation *notes = symbol__annotation(sym); struct annotated_basic_block *bb; - bool found = false; + int ret = 0; init_type_state(&state, dloc->arch); @@ -1317,8 +1325,8 @@ static bool find_data_type_insn(struct data_loc_info *dloc, int reg, update_var_state(&state, dloc, addr, dl->al.offset, var_types); if (this_ip == dloc->ip) { - found = check_matching_type(&state, dloc, reg, - cu_die, type_die); + ret = check_matching_type(&state, dloc, reg, + cu_die, type_die); goto out; } @@ -1331,7 +1339,7 @@ static bool find_data_type_insn(struct data_loc_info *dloc, int reg, out: exit_type_state(&state); - return found; + return ret; } /* @@ -1355,6 +1363,7 @@ static int find_data_type_block(struct data_loc_info *dloc, int reg, for (int i = nr_scopes - 1; i >= 0; i--) { Dwarf_Addr base, start, end; LIST_HEAD(this_blocks); + int found; if (dwarf_ranges(&scopes[i], 0, &base, &start, &end) < 0) break; @@ -1385,15 +1394,19 @@ again: fixup_var_address(var_types, start); /* Find from start of this scope to the target instruction */ - if (find_data_type_insn(dloc, reg, &basic_blocks, var_types, - cu_die, type_die)) { - ret = 0; + found = find_data_type_insn(dloc, reg, &basic_blocks, var_types, + cu_die, type_die); + if (found > 0) { pr_debug_dtp("found by insn track: %#x(reg%d) type-offset=%#x", dloc->op->offset, reg, dloc->type_offset); pr_debug_type_name(type_die, TSR_KIND_TYPE); + ret = 0; break; } + if (found < 0) + break; + /* Go up to the next scope and find blocks to the start */ prev_dst_ip = dst_ip; dst_ip = src_ip; -- cgit From 657852135d39b75b1b5139839b7388c1d47f3ecc Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Fri, 5 Apr 2024 14:17:58 -0700 Subject: perf annotate-data: Fix global variable lookup The recent change in the global variable handling added a bug to miss setting the return value even if it found a data type. Also add the type name in the debug message. Fixes: 1ebb5e17ef21b492 ("perf annotate-data: Add get_global_var_type()") Reviewed-by: Ian Rogers Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ingo Molnar Cc: Jiri Olsa Cc: Kan Liang Cc: Namhyung Kim Cc: Peter Zijlstra Link: https://lore.kernel.org/r/20240405211800.1412920-3-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index 043d80791bd0..1047ea9d578c 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -1468,8 +1468,10 @@ static int find_data_type_die(struct data_loc_info *dloc, Dwarf_Die *type_die) &offset, type_die)) { dloc->type_offset = offset; - pr_debug_dtp("found PC-rel by addr=%#"PRIx64" offset=%#x\n", + pr_debug_dtp("found PC-rel by addr=%#"PRIx64" offset=%#x", dloc->var_addr, offset); + pr_debug_type_name(type_die, TSR_KIND_TYPE); + ret = 0; goto out; } } -- cgit From 879ebf3c830dba437781d034c536af53b0bff0c0 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Fri, 5 Apr 2024 14:17:59 -0700 Subject: perf annotate-data: Do not delete non-asm lines For data type profiling, it removed non-instruction lines from the list of annotation lines. It was to simplify the implementation dealing with instructions like to calculate the PC-relative address and to search the shortest path to the target instruction or basic block. But it means that it removes all the comments and debug information in the annotate output like source file name and line numbers. To support both code annotation and data type annotation, it'd be better to keep the non-instruction lines as well. So this change is to skip those lines during the data type profiling and to display them in the normal perf annotate output. No function changes intended (other than having more lines). Reviewed-by: Ian Rogers Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ingo Molnar Cc: Jiri Olsa Cc: Kan Liang Cc: Peter Zijlstra Link: https://lore.kernel.org/r/20240405211800.1412920-4-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index 1047ea9d578c..b69a1cd1577a 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -1314,6 +1314,8 @@ static int find_data_type_insn(struct data_loc_info *dloc, int reg, list_for_each_entry(bb, basic_blocks, list) { struct disasm_line *dl = bb->begin; + BUG_ON(bb->begin->al.offset == -1 || bb->end->al.offset == -1); + pr_debug_dtp("bb: [%"PRIx64" - %"PRIx64"]\n", bb->begin->al.offset, bb->end->al.offset); @@ -1321,6 +1323,10 @@ static int find_data_type_insn(struct data_loc_info *dloc, int reg, u64 this_ip = sym->start + dl->al.offset; u64 addr = map__rip_2objdump(dloc->ms->map, this_ip); + /* Skip comment or debug info lines */ + if (dl->al.offset == -1) + continue; + /* Update variable type at this address */ update_var_state(&state, dloc, addr, dl->al.offset, var_types); -- cgit From 9b561be15febda6f9b314c9eab51e157f8f34dea Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Wed, 10 Apr 2024 20:32:52 -0700 Subject: perf annotate-data: Add hist_entry__annotate_data_tty() And move the related code into util/annotate-data.c file. Reviewed-by: Ian Rogers Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ingo Molnar Cc: Jiri Olsa Cc: Kan Liang Cc: Peter Zijlstra Link: https://lore.kernel.org/r/20240411033256.2099646-4-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 112 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index b69a1cd1577a..b150137a92dc 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -19,6 +19,7 @@ #include "evlist.h" #include "map.h" #include "map_symbol.h" +#include "sort.h" #include "strbuf.h" #include "symbol.h" #include "symbol_conf.h" @@ -1710,3 +1711,114 @@ int annotated_data_type__update_samples(struct annotated_data_type *adt, h->addr[offset].period += period; return 0; } + +static void print_annotated_data_header(struct hist_entry *he, struct evsel *evsel) +{ + struct dso *dso = map__dso(he->ms.map); + int nr_members = 1; + int nr_samples = he->stat.nr_events; + int width = 7; + const char *val_hdr = "Percent"; + + if (evsel__is_group_event(evsel)) { + struct hist_entry *pair; + + list_for_each_entry(pair, &he->pairs.head, pairs.node) + nr_samples += pair->stat.nr_events; + } + + printf("Annotate type: '%s' in %s (%d samples):\n", + he->mem_type->self.type_name, dso->name, nr_samples); + + if (evsel__is_group_event(evsel)) { + struct evsel *pos; + int i = 0; + + for_each_group_evsel(pos, evsel) + printf(" event[%d] = %s\n", i++, pos->name); + + nr_members = evsel->core.nr_members; + } + + if (symbol_conf.show_total_period) { + width = 11; + val_hdr = "Period"; + } else if (symbol_conf.show_nr_samples) { + width = 7; + val_hdr = "Samples"; + } + + printf("============================================================================\n"); + printf("%*s %10s %10s %s\n", (width + 1) * nr_members, val_hdr, + "offset", "size", "field"); +} + +static void print_annotated_data_value(struct type_hist *h, u64 period, int nr_samples) +{ + double percent = h->period ? (100.0 * period / h->period) : 0; + const char *color = get_percent_color(percent); + + if (symbol_conf.show_total_period) + color_fprintf(stdout, color, " %11" PRIu64, period); + else if (symbol_conf.show_nr_samples) + color_fprintf(stdout, color, " %7d", nr_samples); + else + color_fprintf(stdout, color, " %7.2f", percent); +} + +static void print_annotated_data_type(struct annotated_data_type *mem_type, + struct annotated_member *member, + struct evsel *evsel, int indent) +{ + struct annotated_member *child; + struct type_hist *h = mem_type->histograms[evsel->core.idx]; + int i, nr_events = 1, samples = 0; + u64 period = 0; + int width = symbol_conf.show_total_period ? 11 : 7; + + for (i = 0; i < member->size; i++) { + samples += h->addr[member->offset + i].nr_samples; + period += h->addr[member->offset + i].period; + } + print_annotated_data_value(h, period, samples); + + if (evsel__is_group_event(evsel)) { + struct evsel *pos; + + for_each_group_member(pos, evsel) { + h = mem_type->histograms[pos->core.idx]; + + samples = 0; + period = 0; + for (i = 0; i < member->size; i++) { + samples += h->addr[member->offset + i].nr_samples; + period += h->addr[member->offset + i].period; + } + print_annotated_data_value(h, period, samples); + } + nr_events = evsel->core.nr_members; + } + + printf(" %10d %10d %*s%s\t%s", + member->offset, member->size, indent, "", member->type_name, + member->var_name ?: ""); + + if (!list_empty(&member->children)) + printf(" {\n"); + + list_for_each_entry(child, &member->children, node) + print_annotated_data_type(mem_type, child, evsel, indent + 4); + + if (!list_empty(&member->children)) + printf("%*s}", (width + 1) * nr_events + 24 + indent, ""); + printf(";\n"); +} + +int hist_entry__annotate_data_tty(struct hist_entry *he, struct evsel *evsel) +{ + print_annotated_data_header(he, evsel); + print_annotated_data_type(he->mem_type, &he->mem_type->self, evsel, 0); + printf("\n"); + + return 0; +} -- cgit From d001c7a7f473674353311a79cf140d054b26afcd Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Wed, 10 Apr 2024 20:32:53 -0700 Subject: perf annotate-data: Add hist_entry__annotate_data_tui() Support data type profiling output on TUI. Testing from Arnaldo: First make sure that the debug information for your workload binaries in embedded in them by building it with '-g' or install the debuginfo packages, since our workload is 'find': root@number:~# type find find is hashed (/usr/bin/find) root@number:~# rpm -qf /usr/bin/find findutils-4.9.0-5.fc39.x86_64 root@number:~# dnf debuginfo-install findutils root@number:~# Then collect some data: root@number:~# echo 1 > /proc/sys/vm/drop_caches root@number:~# perf mem record find / > /dev/null [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.331 MB perf.data (3982 samples) ] root@number:~# Finally do data-type annotation with the following command, that will default, as 'perf report' to the --tui mode, with lines colored to highlight the hotspots, etc. root@number:~# perf annotate --data-type Annotate type: 'struct predicate' (58 samples) Percent Offset Size Field 100.00 0 312 struct predicate { 0.00 0 8 PRED_FUNC pred_func; 0.00 8 8 char* p_name; 0.00 16 4 enum predicate_type p_type; 0.00 20 4 enum predicate_precedence p_prec; 0.00 24 1 _Bool side_effects; 0.00 25 1 _Bool no_default_print; 0.00 26 1 _Bool need_stat; 0.00 27 1 _Bool need_type; 0.00 28 1 _Bool need_inum; 0.00 32 4 enum EvaluationCost p_cost; 0.00 36 4 float est_success_rate; 0.00 40 1 _Bool literal_control_chars; 0.00 41 1 _Bool artificial; 0.00 48 8 char* arg_text; Reviewed-by: Ian Rogers Signed-off-by: Namhyung Kim Tested-by: Arnaldo Carvalho de Melo Cc: Adrian Hunter Cc: Ingo Molnar Cc: Jiri Olsa Cc: Kan Liang Cc: Peter Zijlstra Link: https://lore.kernel.org/r/20240411033256.2099646-5-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index b150137a92dc..1cd857400038 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -1820,5 +1820,6 @@ int hist_entry__annotate_data_tty(struct hist_entry *he, struct evsel *evsel) print_annotated_data_type(he->mem_type, &he->mem_type->self, evsel, 0); printf("\n"); - return 0; + /* move to the next entry */ + return '>'; } -- cgit From 2bc3cf575a162a2ca9c98262a63e95cc2b619de7 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Fri, 12 Apr 2024 11:33:07 -0700 Subject: perf annotate-data: Improve debug message with location info To verify it found the correct variable, let's add the location expression to the debug message. $ perf --debug type-profile annotate --data-type ... ----------------------------------------------------------- find data type for 0xaf0(reg15) at schedule+0xeb CU for kernel/sched/core.c (die:0x1180523) frame base: cfa=0 fbreg=6 found "rq" in scope=3/4 (die: 0x11b6a00) type_offset=0xaf0 variable location: reg15 type='struct rq' size=0xfc0 (die:0x11892e2) ----------------------------------------------------------- find data type for 0x7bc(reg3) at tcp_get_info+0x62 CU for net/ipv4/tcp.c (die:0x7b5f516) frame base: cfa=0 fbreg=6 offset: 1980 is bigger than size: 760 check variable "sk" failed (die: 0x7b92b2c) variable location: reg3 type='struct sock' size=0x2f8 (die:0x7b63c3a) ----------------------------------------------------------- ... The first case is fine. It looked up a data type in r15 with offset of 0xaf0 at schedule+0xeb. It found the CU die and the frame base info and the variable "rq" was found in the scope 3/4. Its location is the r15 register and the type size is 0xfc0 which includes 0xaf0. But the second case is not good. It looked up a data type in rbx (reg3) with offset 0x7bc. It found a CU and the frame base which is good so far. And it also found a variable "sk" but the access offset is bigger than the type size (1980 vs. 760 or 0x7bc vs. 0x2f8). The variable has the right location (reg3) but I need to figure out why it accesses beyond what it's supposed to. Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Kan Liang Cc: Peter Zijlstra Link: https://lore.kernel.org/r/20240412183310.2518474-2-namhyung@kernel.org [ Fix the build on 32-bit by casting Dwarf_Word to (long) in pr_debug_location() ] Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 99 ++++++++++++++++++++++++++++++++++------- 1 file changed, 82 insertions(+), 17 deletions(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index 1cd857400038..e53d66c46c54 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -46,6 +46,7 @@ static void pr_debug_type_name(Dwarf_Die *die, enum type_state_kind kind) { struct strbuf sb; char *str; + Dwarf_Word size = 0; if (!debug_type_profile && verbose < 3) return; @@ -72,13 +73,67 @@ static void pr_debug_type_name(Dwarf_Die *die, enum type_state_kind kind) break; } + dwarf_aggregate_size(die, &size); + strbuf_init(&sb, 32); die_get_typename_from_type(die, &sb); str = strbuf_detach(&sb, NULL); - pr_info(" type=%s (die:%lx)\n", str, (long)dwarf_dieoffset(die)); + pr_info(" type='%s' size=%#lx (die:%#lx)\n", + str, (long)size, (long)dwarf_dieoffset(die)); free(str); } +static void pr_debug_location(Dwarf_Die *die, u64 pc, int reg) +{ + ptrdiff_t off = 0; + Dwarf_Attribute attr; + Dwarf_Addr base, start, end; + Dwarf_Op *ops; + size_t nops; + + if (!debug_type_profile && verbose < 3) + return; + + if (dwarf_attr(die, DW_AT_location, &attr) == NULL) + return; + + while ((off = dwarf_getlocations(&attr, off, &base, &start, &end, &ops, &nops)) > 0) { + if (reg != DWARF_REG_PC && end < pc) + continue; + if (reg != DWARF_REG_PC && start > pc) + break; + + pr_info(" variable location: "); + switch (ops->atom) { + case DW_OP_reg0 ...DW_OP_reg31: + pr_info("reg%d\n", ops->atom - DW_OP_reg0); + break; + case DW_OP_breg0 ...DW_OP_breg31: + pr_info("base=reg%d, offset=%#lx\n", + ops->atom - DW_OP_breg0, (long)ops->number); + break; + case DW_OP_regx: + pr_info("reg%ld\n", (long)ops->number); + break; + case DW_OP_bregx: + pr_info("base=reg%ld, offset=%#lx\n", + (long)ops->number, (long)ops->number2); + break; + case DW_OP_fbreg: + pr_info("use frame base, offset=%#lx\n", (long)ops->number); + break; + case DW_OP_addr: + pr_info("address=%#lx\n", (long)ops->number); + break; + default: + pr_info("unknown: code=%#x, number=%#lx\n", + ops->atom, (long)ops->number); + break; + } + break; + } +} + /* * Type information in a register, valid when @ok is true. * The @caller_saved registers are invalidated after a function call. @@ -1404,7 +1459,7 @@ again: found = find_data_type_insn(dloc, reg, &basic_blocks, var_types, cu_die, type_die); if (found > 0) { - pr_debug_dtp("found by insn track: %#x(reg%d) type-offset=%#x", + pr_debug_dtp("found by insn track: %#x(reg%d) type-offset=%#x\n", dloc->op->offset, reg, dloc->type_offset); pr_debug_type_name(type_die, TSR_KIND_TYPE); ret = 0; @@ -1440,16 +1495,16 @@ static int find_data_type_die(struct data_loc_info *dloc, Dwarf_Die *type_die) char buf[64]; if (dloc->op->multi_regs) - snprintf(buf, sizeof(buf), " or reg%d", dloc->op->reg2); + snprintf(buf, sizeof(buf), "reg%d, reg%d", dloc->op->reg1, dloc->op->reg2); else if (dloc->op->reg1 == DWARF_REG_PC) - snprintf(buf, sizeof(buf), " (PC)"); + snprintf(buf, sizeof(buf), "PC"); else - buf[0] = '\0'; + snprintf(buf, sizeof(buf), "reg%d", dloc->op->reg1); pr_debug_dtp("-----------------------------------------------------------\n"); - pr_debug_dtp("%s [%"PRIx64"] for reg%d%s offset=%#x in %s\n", - __func__, dloc->ip - dloc->ms->sym->start, - dloc->op->reg1, buf, dloc->op->offset, dloc->ms->sym->name); + pr_debug_dtp("find data type for %#x(%s) at %s+%#"PRIx64"\n", + dloc->op->offset, buf, dloc->ms->sym->name, + dloc->ip - dloc->ms->sym->start); /* * IP is a relative instruction address from the start of the map, as @@ -1468,14 +1523,15 @@ static int find_data_type_die(struct data_loc_info *dloc, Dwarf_Die *type_die) reg = loc->reg1; offset = loc->offset; - pr_debug_dtp("CU die offset: %#lx\n", (long)dwarf_dieoffset(&cu_die)); + pr_debug_dtp("CU for %s (die:%#lx)\n", + dwarf_diename(&cu_die), (long)dwarf_dieoffset(&cu_die)); if (reg == DWARF_REG_PC) { if (get_global_var_type(&cu_die, dloc, dloc->ip, dloc->var_addr, &offset, type_die)) { dloc->type_offset = offset; - pr_debug_dtp("found PC-rel by addr=%#"PRIx64" offset=%#x", + pr_debug_dtp("found by addr=%#"PRIx64" type_offset=%#x\n", dloc->var_addr, offset); pr_debug_type_name(type_die, TSR_KIND_TYPE); ret = 0; @@ -1537,13 +1593,22 @@ retry: pr_debug_dtp("found \"%s\" in scope=%d/%d (die: %#lx) ", dwarf_diename(&var_die), i+1, nr_scopes, (long)dwarf_dieoffset(&scopes[i])); - if (reg == DWARF_REG_PC) - pr_debug_dtp("%#x(PC) offset=%#x", loc->offset, offset); - else if (reg == DWARF_REG_FB || is_fbreg) - pr_debug_dtp("%#x(reg%d) stack fb_offset=%#x offset=%#x", - loc->offset, reg, fb_offset, offset); - else - pr_debug_dtp("%#x(reg%d)", loc->offset, reg); + if (reg == DWARF_REG_PC) { + pr_debug_dtp("addr=%#"PRIx64" type_offset=%#x\n", + dloc->var_addr, offset); + } else if (reg == DWARF_REG_FB || is_fbreg) { + pr_debug_dtp("stack_offset=%#x type_offset=%#x\n", + fb_offset, offset); + } else { + pr_debug_dtp("type_offset=%#x\n", offset); + } + pr_debug_location(&var_die, pc, reg); + pr_debug_type_name(type_die, TSR_KIND_TYPE); + } else { + pr_debug_dtp("check variable \"%s\" failed (die: %#lx)\n", + dwarf_diename(&var_die), + (long)dwarf_dieoffset(&var_die)); + pr_debug_location(&var_die, pc, reg); pr_debug_type_name(type_die, TSR_KIND_TYPE); } dloc->type_offset = offset; -- cgit From a5a00497b9dfefbf6872f387bc7692919e1785d3 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Fri, 12 Apr 2024 11:33:10 -0700 Subject: perf annotate-data: Handle RSP if it's not the FB register In some cases, the stack pointer on x86 (rsp = reg7) is used to point variables on stack but it's not the frame base register. Then it should handle the register like normal registers (IOW not to access the other stack variables using offset calculation) but it should not assume it would have a pointer. Before: ----------------------------------------------------------- find data type for 0x7c(reg7) at tcp_getsockopt+0xb62 CU for net/ipv4/tcp.c (die:0x7b5f516) frame base: cfa=0 fbreg=6 no pointer or no type check variable "zc" failed (die: 0x7b9580a) variable location: base=reg7, offset=0x40 type='struct tcp_zerocopy_receive' size=0x40 (die:0x7b947f4) After: ----------------------------------------------------------- find data type for 0x7c(reg7) at tcp_getsockopt+0xb62 CU for net/ipv4/tcp.c (die:0x7b5f516) frame base: cfa=0 fbreg=6 found "zc" in scope=3/3 (die: 0x7b957fc) type_offset=0x3c variable location: base=reg7, offset=0x40 type='struct tcp_zerocopy_receive' size=0x40 (die:0x7b947f4) Note that the type-offset was properly calculated to 0x3c as the variable starts at 0x40. Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Kan Liang Cc: Peter Zijlstra Link: https://lore.kernel.org/r/20240412183310.2518474-5-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index e53d66c46c54..12d5faff3b7a 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -25,6 +25,9 @@ #include "symbol_conf.h" #include "thread.h" +/* register number of the stack pointer */ +#define X86_REG_SP 7 + enum type_state_kind { TSR_KIND_INVALID = 0, TSR_KIND_TYPE, @@ -197,7 +200,7 @@ static void init_type_state(struct type_state *state, struct arch *arch) state->regs[10].caller_saved = true; state->regs[11].caller_saved = true; state->ret_reg = 0; - state->stack_reg = 7; + state->stack_reg = X86_REG_SP; } } @@ -382,10 +385,18 @@ static bool find_cu_die(struct debuginfo *di, u64 pc, Dwarf_Die *cu_die) } /* The type info will be saved in @type_die */ -static int check_variable(Dwarf_Die *var_die, Dwarf_Die *type_die, int offset, - bool is_pointer) +static int check_variable(struct data_loc_info *dloc, Dwarf_Die *var_die, + Dwarf_Die *type_die, int reg, int offset, bool is_fbreg) { Dwarf_Word size; + bool is_pointer = true; + + if (reg == DWARF_REG_PC) + is_pointer = false; + else if (reg == dloc->fbreg || is_fbreg) + is_pointer = false; + else if (arch__is(dloc->arch, "x86") && reg == X86_REG_SP) + is_pointer = false; /* Get the type of the variable */ if (die_get_real_type(var_die, type_die) == NULL) { @@ -607,7 +618,6 @@ static bool get_global_var_type(Dwarf_Die *cu_die, struct data_loc_info *dloc, { u64 pc; int offset; - bool is_pointer = false; const char *var_name = NULL; struct global_var_entry *gvar; Dwarf_Die var_die; @@ -623,7 +633,8 @@ static bool get_global_var_type(Dwarf_Die *cu_die, struct data_loc_info *dloc, /* Try to get the variable by address first */ if (die_find_variable_by_addr(cu_die, var_addr, &var_die, &offset) && - check_variable(&var_die, type_die, offset, is_pointer) == 0) { + check_variable(dloc, &var_die, type_die, DWARF_REG_PC, offset, + /*is_fbreg=*/false) == 0) { var_name = dwarf_diename(&var_die); *var_offset = offset; goto ok; @@ -636,7 +647,8 @@ static bool get_global_var_type(Dwarf_Die *cu_die, struct data_loc_info *dloc, /* Try to get the name of global variable */ if (die_find_variable_at(cu_die, var_name, pc, &var_die) && - check_variable(&var_die, type_die, *var_offset, is_pointer) == 0) + check_variable(dloc, &var_die, type_die, DWARF_REG_PC, *var_offset, + /*is_fbreg=*/false) == 0) goto ok; return false; @@ -1587,8 +1599,7 @@ retry: } /* Found a variable, see if it's correct */ - ret = check_variable(&var_die, type_die, offset, - reg != DWARF_REG_PC && !is_fbreg); + ret = check_variable(dloc, &var_die, type_die, reg, offset, is_fbreg); if (ret == 0) { pr_debug_dtp("found \"%s\" in scope=%d/%d (die: %#lx) ", dwarf_diename(&var_die), i+1, nr_scopes, -- cgit From c1da8411e4be2a96a448979baede9a0e86c5baf8 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Wed, 1 May 2024 23:00:07 -0700 Subject: perf annotate-data: Collect global variables in advance Currently it looks up global variables from the current CU using address and name. But it sometimes fails to find a variable as the variable can come from a different CU - but it's still strange it failed to find a declaration for some reason. Anyway, it can collect all global variables from all CU once and then lookup them later on. This slightly improves the success rate of my test data set. Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Kan Liang Cc: Peter Zijlstra Link: https://lore.kernel.org/r/20240502060011.1838090-3-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 57 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 55 insertions(+), 2 deletions(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index 12d5faff3b7a..4dd0911904f2 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -28,6 +28,8 @@ /* register number of the stack pointer */ #define X86_REG_SP 7 +static void delete_var_types(struct die_var_type *var_types); + enum type_state_kind { TSR_KIND_INVALID = 0, TSR_KIND_TYPE, @@ -557,8 +559,8 @@ static bool global_var__add(struct data_loc_info *dloc, u64 addr, if (gvar == NULL) return false; - gvar->name = strdup(name); - if (gvar->name == NULL) { + gvar->name = name ? strdup(name) : NULL; + if (name && gvar->name == NULL) { free(gvar); return false; } @@ -612,6 +614,53 @@ static bool get_global_var_info(struct data_loc_info *dloc, u64 addr, return true; } +static void global_var__collect(struct data_loc_info *dloc) +{ + Dwarf *dwarf = dloc->di->dbg; + Dwarf_Off off, next_off; + Dwarf_Die cu_die, type_die; + size_t header_size; + + /* Iterate all CU and collect global variables that have no location in a register. */ + off = 0; + while (dwarf_nextcu(dwarf, off, &next_off, &header_size, + NULL, NULL, NULL) == 0) { + struct die_var_type *var_types = NULL; + struct die_var_type *pos; + + if (dwarf_offdie(dwarf, off + header_size, &cu_die) == NULL) { + off = next_off; + continue; + } + + die_collect_global_vars(&cu_die, &var_types); + + for (pos = var_types; pos; pos = pos->next) { + const char *var_name = NULL; + int var_offset = 0; + + if (pos->reg != -1) + continue; + + if (!dwarf_offdie(dwarf, pos->die_off, &type_die)) + continue; + + if (!get_global_var_info(dloc, pos->addr, &var_name, + &var_offset)) + continue; + + if (var_offset != 0) + continue; + + global_var__add(dloc, pos->addr, var_name, &type_die); + } + + delete_var_types(var_types); + + off = next_off; + } +} + static bool get_global_var_type(Dwarf_Die *cu_die, struct data_loc_info *dloc, u64 ip, u64 var_addr, int *var_offset, Dwarf_Die *type_die) @@ -620,8 +669,12 @@ static bool get_global_var_type(Dwarf_Die *cu_die, struct data_loc_info *dloc, int offset; const char *var_name = NULL; struct global_var_entry *gvar; + struct dso *dso = map__dso(dloc->ms->map); Dwarf_Die var_die; + if (RB_EMPTY_ROOT(&dso->global_vars)) + global_var__collect(dloc); + gvar = global_var__find(dloc, var_addr); if (gvar) { if (!dwarf_offdie(dloc->di->dbg, gvar->die_offset, type_die)) -- cgit From 4449c9047dc6f9f68333a720958cd7b58225910d Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Wed, 1 May 2024 23:00:08 -0700 Subject: perf annotate-data: Handle direct global variable access Like per-cpu base offset array, sometimes it accesses the global variable directly using the offset. Allow this type of instructions as long as it finds a global variable for the address. movslq %edi, %rcx mov -0x7dc94ae0(,%rcx,8), %rcx <<<--- here As %rcx has a valid type (i.e. array index) from the first instruction, it will be checked by the first case in check_matching_type(). But as it's not a pointer type, the match will fail. But in this case, it should check if it accesses the kernel global array variable. Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Kan Liang Cc: Peter Zijlstra Link: https://lore.kernel.org/r/20240502060011.1838090-4-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index 4dd0911904f2..f1e52a531563 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -1256,14 +1256,19 @@ static int check_matching_type(struct type_state *state, if (state->regs[reg].ok && state->regs[reg].kind == TSR_KIND_TYPE) { int tag = dwarf_tag(&state->regs[reg].type); - pr_debug_dtp("\n"); - /* * Normal registers should hold a pointer (or array) to * dereference a memory location. */ - if (tag != DW_TAG_pointer_type && tag != DW_TAG_array_type) + if (tag != DW_TAG_pointer_type && tag != DW_TAG_array_type) { + if (dloc->op->offset < 0 && reg != state->stack_reg) + goto check_kernel; + + pr_debug_dtp("\n"); return -1; + } + + pr_debug_dtp("\n"); /* Remove the pointer and get the target type */ if (die_get_real_type(&state->regs[reg].type, type_die) == NULL) @@ -1376,12 +1381,14 @@ static int check_matching_type(struct type_state *state, return -1; } - if (map__dso(dloc->ms->map)->kernel && arch__is(dloc->arch, "x86")) { +check_kernel: + if (map__dso(dloc->ms->map)->kernel) { u64 addr; int offset; /* Direct this-cpu access like "%gs:0x34740" */ - if (dloc->op->segment == INSN_SEG_X86_GS && dloc->op->imm) { + if (dloc->op->segment == INSN_SEG_X86_GS && dloc->op->imm && + arch__is(dloc->arch, "x86")) { pr_debug_dtp(" this-cpu var\n"); addr = dloc->op->offset; @@ -1394,17 +1401,13 @@ static int check_matching_type(struct type_state *state, return -1; } - /* Access to per-cpu base like "-0x7dcf0500(,%rdx,8)" */ + /* Access to global variable like "-0x7dcf0500(,%rdx,8)" */ if (dloc->op->offset < 0 && reg != state->stack_reg) { - const char *var_name = NULL; - addr = (s64) dloc->op->offset; - if (get_global_var_info(dloc, addr, &var_name, &offset) && - !strcmp(var_name, "__per_cpu_offset") && offset == 0 && - get_global_var_type(cu_die, dloc, dloc->ip, addr, + if (get_global_var_type(cu_die, dloc, dloc->ip, addr, &offset, type_die)) { - pr_debug_dtp(" percpu base\n"); + pr_debug_dtp(" global var\n"); dloc->type_offset = offset; return 1; -- cgit From eba1f853edf794ec259ec7b5e5a6efee5ede989f Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Wed, 1 May 2024 23:00:09 -0700 Subject: perf annotate-data: Check memory access with two registers The following instruction pattern is used to access a global variable. mov $0x231c0, %rax movsql %edi, %rcx mov -0x7dc94ae0(,%rcx,8), %rcx cmpl $0x0, 0xa60(%rcx,%rax,1) <<<--- here The first instruction set the address of the per-cpu variable (here, it is 'runqueues' of type 'struct rq'). The second instruction seems like a cpu number of the per-cpu base. The third instruction get the base offset of per-cpu area for that cpu. The last instruction compares the value of the per-cpu variable at the offset of 0xa60. Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Kan Liang Cc: Peter Zijlstra Link: https://lore.kernel.org/r/20240502060011.1838090-5-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 44 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 39 insertions(+), 5 deletions(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index f1e52a531563..245e3ef3e2ff 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -1031,22 +1031,37 @@ retry: else if (has_reg_type(state, sreg) && state->regs[sreg].kind == TSR_KIND_PERCPU_BASE) { u64 ip = dloc->ms->sym->start + dl->al.offset; + u64 var_addr = src->offset; int offset; + if (src->multi_regs) { + int reg2 = (sreg == src->reg1) ? src->reg2 : src->reg1; + + if (has_reg_type(state, reg2) && state->regs[reg2].ok && + state->regs[reg2].kind == TSR_KIND_CONST) + var_addr += state->regs[reg2].imm_value; + } + /* * In kernel, %gs points to a per-cpu region for the * current CPU. Access with a constant offset should * be treated as a global variable access. */ - if (get_global_var_type(cu_die, dloc, ip, src->offset, + if (get_global_var_type(cu_die, dloc, ip, var_addr, &offset, &type_die) && die_get_member_type(&type_die, offset, &type_die)) { tsr->type = type_die; tsr->kind = TSR_KIND_TYPE; tsr->ok = true; - pr_debug_dtp("mov [%x] percpu %#x(reg%d) -> reg%d", - insn_offset, src->offset, sreg, dst->reg1); + if (src->multi_regs) { + pr_debug_dtp("mov [%x] percpu %#x(reg%d,reg%d) -> reg%d", + insn_offset, src->offset, src->reg1, + src->reg2, dst->reg1); + } else { + pr_debug_dtp("mov [%x] percpu %#x(reg%d) -> reg%d", + insn_offset, src->offset, sreg, dst->reg1); + } pr_debug_type_name(&tsr->type, tsr->kind); } else { tsr->ok = false; @@ -1340,6 +1355,17 @@ static int check_matching_type(struct type_state *state, pr_debug_dtp(" percpu var\n"); + if (dloc->op->multi_regs) { + int reg2 = dloc->op->reg2; + + if (dloc->op->reg2 == reg) + reg2 = dloc->op->reg1; + + if (has_reg_type(state, reg2) && state->regs[reg2].ok && + state->regs[reg2].kind == TSR_KIND_CONST) + var_addr += state->regs[reg2].imm_value; + } + if (get_global_var_type(cu_die, dloc, dloc->ip, var_addr, &var_offset, type_die)) { dloc->type_offset = var_offset; @@ -1527,8 +1553,16 @@ again: found = find_data_type_insn(dloc, reg, &basic_blocks, var_types, cu_die, type_die); if (found > 0) { - pr_debug_dtp("found by insn track: %#x(reg%d) type-offset=%#x\n", - dloc->op->offset, reg, dloc->type_offset); + char buf[64]; + + if (dloc->op->multi_regs) + snprintf(buf, sizeof(buf), "reg%d, reg%d", + dloc->op->reg1, dloc->op->reg2); + else + snprintf(buf, sizeof(buf), "reg%d", dloc->op->reg1); + + pr_debug_dtp("found by insn track: %#x(%s) type-offset=%#x\n", + dloc->op->offset, buf, dloc->type_offset); pr_debug_type_name(type_die, TSR_KIND_TYPE); ret = 0; break; -- cgit From af89e8f2bdb2ff9252317307a755f97dd02f6cd7 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Wed, 1 May 2024 23:00:10 -0700 Subject: perf annotate-data: Handle multi regs in find_data_type_block() The instruction tracking should be the same for the both registers. Just do it once and compare the result with multi regs as with the previous patches. Then we don't need to call find_data_type_block() separately for each reg. Let's remove the 'reg' argument from the relevant functions. Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Kan Liang Cc: Peter Zijlstra Link: https://lore.kernel.org/r/20240502060011.1838090-6-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index 245e3ef3e2ff..68fe7999f033 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -1258,11 +1258,12 @@ static void setup_stack_canary(struct data_loc_info *dloc) * are similar to global variables and no additional info is needed. */ static int check_matching_type(struct type_state *state, - struct data_loc_info *dloc, int reg, + struct data_loc_info *dloc, Dwarf_Die *cu_die, Dwarf_Die *type_die) { Dwarf_Word size; u32 insn_offset = dloc->ip - dloc->ms->sym->start; + int reg = dloc->op->reg1; pr_debug_dtp("chk [%x] reg%d offset=%#x ok=%d kind=%d", insn_offset, reg, dloc->op->offset, @@ -1448,7 +1449,7 @@ check_kernel: } /* Iterate instructions in basic blocks and update type table */ -static int find_data_type_insn(struct data_loc_info *dloc, int reg, +static int find_data_type_insn(struct data_loc_info *dloc, struct list_head *basic_blocks, struct die_var_type *var_types, Dwarf_Die *cu_die, Dwarf_Die *type_die) @@ -1481,7 +1482,7 @@ static int find_data_type_insn(struct data_loc_info *dloc, int reg, update_var_state(&state, dloc, addr, dl->al.offset, var_types); if (this_ip == dloc->ip) { - ret = check_matching_type(&state, dloc, reg, + ret = check_matching_type(&state, dloc, cu_die, type_die); goto out; } @@ -1502,7 +1503,7 @@ out: * Construct a list of basic blocks for each scope with variables and try to find * the data type by updating a type state table through instructions. */ -static int find_data_type_block(struct data_loc_info *dloc, int reg, +static int find_data_type_block(struct data_loc_info *dloc, Dwarf_Die *cu_die, Dwarf_Die *scopes, int nr_scopes, Dwarf_Die *type_die) { @@ -1550,7 +1551,7 @@ again: fixup_var_address(var_types, start); /* Find from start of this scope to the target instruction */ - found = find_data_type_insn(dloc, reg, &basic_blocks, var_types, + found = find_data_type_insn(dloc, &basic_blocks, var_types, cu_die, type_die); if (found > 0) { char buf[64]; @@ -1716,8 +1717,13 @@ retry: goto out; } + if (loc->multi_regs && reg == loc->reg1 && loc->reg1 != loc->reg2) { + reg = loc->reg2; + goto retry; + } + if (reg != DWARF_REG_PC) { - ret = find_data_type_block(dloc, reg, &cu_die, scopes, + ret = find_data_type_block(dloc, &cu_die, scopes, nr_scopes, type_die); if (ret == 0) { ann_data_stat.insn_track++; @@ -1725,11 +1731,6 @@ retry: } } - if (loc->multi_regs && reg == loc->reg1 && loc->reg1 != loc->reg2) { - reg = loc->reg2; - goto retry; - } - if (ret < 0) { pr_debug_dtp("no variable found\n"); ann_data_stat.no_var++; -- cgit From b7d4aacfc894ca2d86b11ef738f94e6c8cf2536b Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Wed, 1 May 2024 23:00:11 -0700 Subject: perf annotate-data: Check kind of stack variables I sometimes see ("unknown type") in the result and it was because it didn't check the type of stack variables properly during the instruction tracking. The stack can carry constant values (without type info) and if the target instruction is accessing the stack location, it resulted in the "unknown type". Maybe we could pick one of integer types for the constant, but it doesn't really mean anything useful. Let's just drop the stack slot if it doesn't have a valid type info. Here's an example how it got the unknown type. Note that 0xffffff48 = -0xb8. ----------------------------------------------------------- find data type for 0xffffff48(reg6) at ... CU for ... frame base: cfa=0 fbreg=6 scope: [2/2] (die:11cb97f) bb: [37 - 3a] var [37] reg15 type='int' size=0x4 (die:0x1180633) bb: [40 - 4b] mov [40] imm=0x1 -> reg13 var [45] reg8 type='sigset_t*' size=0x8 (die:0x11a39ee) mov [45] imm=0x1 -> reg2 <--- here reg2 has a constant bb: [215 - 237] mov [218] reg2 -> -0xb8(stack) constant <--- and save it to the stack mov [225] reg13 -> -0xc4(stack) constant call [22f] find_task_by_vgpid call [22f] return -> reg0 type='struct task_struct*' size=0x8 (die:0x11881e8) bb: [5c8 - 5cf] bb: [2fb - 302] mov [2fb] -0xc4(stack) -> reg13 constant bb: [13b - 14d] mov [143] 0xd50(reg3) -> reg5 type='struct task_struct*' size=0x8 (die:0xa31f3c) bb: [153 - 153] chk [153] reg6 offset=0xffffff48 ok=0 kind=0 fbreg <--- access here found by insn track: 0xffffff48(reg6) type-offset=0 type='G^KU' size=0 (die:0xffffffffffffffff) Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Kan Liang Cc: Peter Zijlstra Link: https://lore.kernel.org/r/20240502060011.1838090-7-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index 68fe7999f033..2c98813f95cd 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -1314,6 +1314,9 @@ static int check_matching_type(struct type_state *state, return -1; } + if (stack->kind != TSR_KIND_TYPE) + return 0; + *type_die = stack->type; /* Update the type offset from the start of slot */ dloc->type_offset -= stack->offset; @@ -1343,6 +1346,9 @@ static int check_matching_type(struct type_state *state, return -1; } + if (stack->kind != TSR_KIND_TYPE) + return 0; + *type_die = stack->type; /* Update the type offset from the start of slot */ dloc->type_offset -= fboff + stack->offset; -- cgit From ee756ef7491eafd70f390343a1d90930af125a51 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Sat, 4 May 2024 14:38:01 -0700 Subject: perf dso: Add reference count checking and accessor functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add reference count checking to struct dso, this can help with implementing correct reference counting discipline. To avoid RC_CHK_ACCESS everywhere, add accessor functions for the variables in struct dso. The majority of the change is mechanical in nature and not easy to split up. Committer testing: 'perf test' up to this patch shows no regressions. But: util/symbol.c: In function ‘dso__load_bfd_symbols’: util/symbol.c:1683:9: error: too few arguments to function ‘dso__set_adjust_symbols’ 1683 | dso__set_adjust_symbols(dso); | ^~~~~~~~~~~~~~~~~~~~~~~ In file included from util/symbol.c:21: util/dso.h:268:20: note: declared here 268 | static inline void dso__set_adjust_symbols(struct dso *dso, bool val) | ^~~~~~~~~~~~~~~~~~~~~~~ make[6]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:106: /tmp/tmp.ZWHbQftdN6/util/symbol.o] Error 1 MKDIR /tmp/tmp.ZWHbQftdN6/tests/workloads/ make[6]: *** Waiting for unfinished jobs.... This was updated: - symbols__fixup_end(&dso->symbols, false); - symbols__fixup_duplicate(&dso->symbols); - dso->adjust_symbols = 1; + symbols__fixup_end(dso__symbols(dso), false); + symbols__fixup_duplicate(dso__symbols(dso)); + dso__set_adjust_symbols(dso); But not build tested with BUILD_NONDISTRO and libbfd devel files installed (binutils-devel on fedora). Add the missing argument: symbols__fixup_end(dso__symbols(dso), false); symbols__fixup_duplicate(dso__symbols(dso)); - dso__set_adjust_symbols(dso); + dso__set_adjust_symbols(dso, true); Signed-off-by: Ian Rogers Tested-by: Arnaldo Carvalho de Melo Cc: Adrian Hunter Cc: Ahelenia Ziemiańska Cc: Alexander Shishkin Cc: Andi Kleen Cc: Athira Rajeev Cc: Ben Gainey Cc: Changbin Du Cc: Chengen Du Cc: Colin Ian King Cc: Dima Kogan Cc: Ilkka Koskinen Cc: Ingo Molnar Cc: James Clark Cc: Jiri Olsa Cc: K Prateek Nayak Cc: Kan Liang Cc: Leo Yan Cc: Li Dong Cc: Mark Rutland Cc: Masami Hiramatsu Cc: Namhyung Kim Cc: Paran Lee Cc: Peter Zijlstra Cc: Song Liu Cc: Sun Haiyong Cc: Thomas Richter Cc: Tiezhu Yang Cc: Yanteng Si Cc: zhaimingbing Link: https://lore.kernel.org/r/20240504213803.218974-6-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index 2c98813f95cd..faefa444af1e 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -336,7 +336,7 @@ static struct annotated_data_type *dso__findnew_data_type(struct dso *dso, /* Check existing nodes in dso->data_types tree */ key.self.type_name = type_name; key.self.size = size; - node = rb_find(&key, &dso->data_types, data_type_cmp); + node = rb_find(&key, dso__data_types(dso), data_type_cmp); if (node) { result = rb_entry(node, struct annotated_data_type, node); free(type_name); @@ -357,7 +357,7 @@ static struct annotated_data_type *dso__findnew_data_type(struct dso *dso, if (symbol_conf.annotate_data_member) add_member_types(result, type_die); - rb_add(&result->node, &dso->data_types, data_type_less); + rb_add(&result->node, dso__data_types(dso), data_type_less); return result; } @@ -538,7 +538,7 @@ static struct global_var_entry *global_var__find(struct data_loc_info *dloc, u64 struct dso *dso = map__dso(dloc->ms->map); struct rb_node *node; - node = rb_find((void *)(uintptr_t)addr, &dso->global_vars, global_var_cmp); + node = rb_find((void *)(uintptr_t)addr, dso__global_vars(dso), global_var_cmp); if (node == NULL) return NULL; @@ -569,7 +569,7 @@ static bool global_var__add(struct data_loc_info *dloc, u64 addr, gvar->end = addr + size; gvar->die_offset = dwarf_dieoffset(type_die); - rb_add(&gvar->node, &dso->global_vars, global_var_less); + rb_add(&gvar->node, dso__global_vars(dso), global_var_less); return true; } @@ -672,7 +672,7 @@ static bool get_global_var_type(Dwarf_Die *cu_die, struct data_loc_info *dloc, struct dso *dso = map__dso(dloc->ms->map); Dwarf_Die var_die; - if (RB_EMPTY_ROOT(&dso->global_vars)) + if (RB_EMPTY_ROOT(dso__global_vars(dso))) global_var__collect(dloc); gvar = global_var__find(dloc, var_addr); @@ -889,7 +889,7 @@ static void update_insn_state_x86(struct type_state *state, return; tsr = &state->regs[dst->reg1]; - if (map__dso(dloc->ms->map)->kernel && + if (dso__kernel(map__dso(dloc->ms->map)) && src->segment == INSN_SEG_X86_GS && src->imm) { u64 ip = dloc->ms->sym->start + dl->al.offset; u64 var_addr; @@ -1415,7 +1415,7 @@ static int check_matching_type(struct type_state *state, } check_kernel: - if (map__dso(dloc->ms->map)->kernel) { + if (dso__kernel(map__dso(dloc->ms->map))) { u64 addr; int offset; @@ -1767,7 +1767,7 @@ struct annotated_data_type *find_data_type(struct data_loc_info *dloc) struct dso *dso = map__dso(dloc->ms->map); Dwarf_Die type_die; - dloc->di = debuginfo__new(dso->long_name); + dloc->di = debuginfo__new(dso__long_name(dso)); if (dloc->di == NULL) { pr_debug_dtp("cannot get the debug info\n"); return NULL; @@ -1901,7 +1901,7 @@ static void print_annotated_data_header(struct hist_entry *he, struct evsel *evs } printf("Annotate type: '%s' in %s (%d samples):\n", - he->mem_type->self.type_name, dso->name, nr_samples); + he->mem_type->self.type_name, dso__name(dso), nr_samples); if (evsel__is_group_event(evsel)) { struct evsel *pos; -- cgit From 69fb6eab1969d09187feff14f370e01032054f1f Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Tue, 7 May 2024 00:04:06 -0300 Subject: perf annotate: Use zfree() to avoid possibly accessing dangling pointers When freeing a->b it is good practice to set a->b to NULL using zfree(&a->b) so that when we have a bug where a reference to a freed 'a' pointer is kept somewhere, we can more quickly cause a segfault if some code tries to use a->b. This is mostly done but some new cases were introduced recently, convert them to zfree(). Cc: Adrian Hunter Cc: Ian Rogers Cc: Jiri Olsa Cc: Kan Liang Cc: Namhyung Kim Link: https://lore.kernel.org/lkml/ZjmbHHrjIm5YRIBv@x1 Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index faefa444af1e..57e7d4b3550b 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -8,6 +8,7 @@ #include #include #include +#include #include "annotate.h" #include "annotate-data.h" @@ -311,8 +312,8 @@ static void delete_members(struct annotated_member *member) list_for_each_entry_safe(child, tmp, &member->children, node) { list_del(&child->node); delete_members(child); - free(child->type_name); - free(child->var_name); + zfree(&child->type_name); + zfree(&child->var_name); free(child); } } @@ -582,7 +583,7 @@ void global_var_type__tree_delete(struct rb_root *root) rb_erase(node, root); gvar = rb_entry(node, struct global_var_entry, node); - free(gvar->name); + zfree(&gvar->name); free(gvar); } } @@ -1817,16 +1818,16 @@ static int alloc_data_type_histograms(struct annotated_data_type *adt, int nr_en err: while (--i >= 0) - free(adt->histograms[i]); - free(adt->histograms); + zfree(&(adt->histograms[i])); + zfree(&adt->histograms); return -ENOMEM; } static void delete_data_type_histograms(struct annotated_data_type *adt) { for (int i = 0; i < adt->nr_histograms; i++) - free(adt->histograms[i]); - free(adt->histograms); + zfree(&(adt->histograms[i])); + zfree(&adt->histograms); } void annotated_data_type__tree_delete(struct rb_root *root) @@ -1840,7 +1841,7 @@ void annotated_data_type__tree_delete(struct rb_root *root) pos = rb_entry(node, struct annotated_data_type, node); delete_members(&pos->self); delete_data_type_histograms(pos); - free(pos->self.type_name); + zfree(&pos->self.type_name); free(pos); } } -- cgit From 2af1280b190c408bd590704806dd0d2d1cf52db5 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Fri, 10 May 2024 14:04:52 -0700 Subject: perf annotate-data: Ensure the number of type histograms Arnaldo reported that there is a case where nr_histograms and histograms don't agree each other. It ended up in a segfault trying to access a NULL histograms array. Let's make sure to update the nr_histograms when the histograms array is changed. Reported-by: Arnaldo Carvalho de Melo Reviewed-by: Ian Rogers Signed-off-by: Namhyung Kim Tested-by: Arnaldo Carvalho de Melo Cc: Adrian Hunter Cc: Ingo Molnar Cc: Jiri Olsa Cc: Kan Liang Cc: Peter Zijlstra Link: https://lore.kernel.org/r/20240510210452.2449944-2-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate-data.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'tools/perf/util/annotate-data.c') diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index 57e7d4b3550b..965da6c0b542 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -1800,7 +1800,6 @@ static int alloc_data_type_histograms(struct annotated_data_type *adt, int nr_en sz += sizeof(struct type_hist_entry) * adt->self.size; /* Allocate a table of pointers for each event */ - adt->nr_histograms = nr_entries; adt->histograms = calloc(nr_entries, sizeof(*adt->histograms)); if (adt->histograms == NULL) return -ENOMEM; @@ -1814,6 +1813,8 @@ static int alloc_data_type_histograms(struct annotated_data_type *adt, int nr_en if (adt->histograms[i] == NULL) goto err; } + + adt->nr_histograms = nr_entries; return 0; err: @@ -1827,7 +1828,9 @@ static void delete_data_type_histograms(struct annotated_data_type *adt) { for (int i = 0; i < adt->nr_histograms; i++) zfree(&(adt->histograms[i])); + zfree(&adt->histograms); + adt->nr_histograms = 0; } void annotated_data_type__tree_delete(struct rb_root *root) -- cgit