perf dwarf-aux: Skip check_variable for variable lookup

Both die_find_variable_by_reg and die_find_variable_by_addr call
match_var_offset which already performs sufficient checking and type
matching. The additional check_variable call is redundant, and its
need_pointer logic is only a heuristic. Since DWARF encodes accurate
type information, which match_var_offset verifies, skipping
check_variable improves both coverage and accuracy.

Return the matched type from die_find_variable_by_reg and
die_find_variable_by_addr via the existing `type` field in
find_var_data, removing the need for check_variable in
find_data_type_die.

Signed-off-by: Zecheng Li <zecheng@google.com>
Signed-off-by: Zecheng Li <zli94@ncsu.edu>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>

diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index cda020ea18d526337a927e9c51c08210586001bb..23a09bf58f864fefcf51adc2fceae6ec1368341b 100644 (file)
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -814,9 +814,8 @@ 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(dloc, &var_die, type_die, DWARF_REG_PC, offset,
-                          /*is_fbreg=*/false) == PERF_TMR_OK) {
+       if (die_find_variable_by_addr(cu_die, var_addr, &var_die, type_die,
+                                     &offset)) {
                var_name = dwarf_diename(&var_die);
                *var_offset = offset;
                goto ok;
@@ -1606,12 +1605,13 @@ retry:
 
                if (reg == DWARF_REG_PC) {
                        if (!die_find_variable_by_addr(&scopes[i], dloc->var_addr,
-                                                      &var_die, &type_offset))
+                                                      &var_die, &mem_die,
+                                                      &type_offset))
                                continue;
                } else {
                        /* Look up variables/parameters in this scope */
                        if (!die_find_variable_by_reg(&scopes[i], pc, reg,
-                                                     &type_offset, is_fbreg, &var_die))
+                                                     &mem_die, &type_offset, is_fbreg, &var_die))
                                continue;
                }
 
@@ -1619,26 +1619,20 @@ retry:
                             dwarf_diename(&var_die), (long)dwarf_dieoffset(&var_die),
                             i+1, nr_scopes, (long)dwarf_dieoffset(&scopes[i]));
 
-               /* Found a variable, see if it's correct */
-               result = check_variable(dloc, &var_die, &mem_die, reg, type_offset, is_fbreg);
-               if (result == PERF_TMR_OK) {
-                       if (reg == DWARF_REG_PC) {
-                               pr_debug_dtp("addr=%#"PRIx64" type_offset=%#x\n",
-                                            dloc->var_addr, type_offset);
-                       } else if (reg == DWARF_REG_FB || is_fbreg) {
-                               pr_debug_dtp("stack_offset=%#x type_offset=%#x\n",
-                                            fb_offset, type_offset);
-                       } else {
-                               pr_debug_dtp("type_offset=%#x\n", type_offset);
-                       }
-
-                       if (!found || is_better_type(type_die, &mem_die)) {
-                               *type_die = mem_die;
-                               dloc->type_offset = type_offset;
-                               found = true;
-                       }
+               if (reg == DWARF_REG_PC) {
+                       pr_debug_dtp("addr=%#"PRIx64" type_offset=%#x\n",
+                                    dloc->var_addr, type_offset);
+               } else if (reg == DWARF_REG_FB || is_fbreg) {
+                       pr_debug_dtp("stack_offset=%#x type_offset=%#x\n",
+                                    fb_offset, type_offset);
                } else {
-                       pr_debug_dtp("failed: %s\n", match_result_str(result));
+                       pr_debug_dtp("type_offset=%#x\n", type_offset);
+               }
+
+               if (!found || is_better_type(type_die, &mem_die)) {
+                       *type_die = mem_die;
+                       dloc->type_offset = type_offset;
+                       found = true;
                }
 
                pr_debug_location(&var_die, pc, reg);

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 3b0fc9038f1995a68386e1ebb2db56b7b0b22821..1484aa7568262dfea667193a8da1d833d422e629 100644 (file)
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -1560,7 +1560,7 @@ static int __die_find_var_reg_cb(Dwarf_Die *die_mem, void *arg)
  * when the variable is in the stack.
  */
 Dwarf_Die *die_find_variable_by_reg(Dwarf_Die *sc_die, Dwarf_Addr pc, int reg,
-                                   int *poffset, bool is_fbreg,
+                                   Dwarf_Die *type_die, int *poffset, bool is_fbreg,
                                    Dwarf_Die *die_mem)
 {
        struct find_var_data data = {
@@ -1572,8 +1572,10 @@ Dwarf_Die *die_find_variable_by_reg(Dwarf_Die *sc_die, Dwarf_Addr pc, int reg,
        Dwarf_Die *result;
 
        result = die_find_child(sc_die, __die_find_var_reg_cb, &data, die_mem);
-       if (result)
+       if (result) {
                *poffset = data.offset;
+               *type_die = data.type;
+       }
        return result;
 }
 
@@ -1617,7 +1619,8 @@ static int __die_find_var_addr_cb(Dwarf_Die *die_mem, void *arg)
  * This is usually for global variables.
  */
 Dwarf_Die *die_find_variable_by_addr(Dwarf_Die *sc_die, Dwarf_Addr addr,
-                                    Dwarf_Die *die_mem, int *offset)
+                                    Dwarf_Die *die_mem, Dwarf_Die *type_die,
+                                    int *offset)
 {
        struct find_var_data data = {
                .addr = addr,
@@ -1625,8 +1628,10 @@ Dwarf_Die *die_find_variable_by_addr(Dwarf_Die *sc_die, Dwarf_Addr addr,
        Dwarf_Die *result;
 
        result = die_find_child(sc_die, __die_find_var_addr_cb, &data, die_mem);
-       if (result)
+       if (result) {
                *offset = data.offset;
+               *type_die = data.type;
+       }
        return result;
 }
 

diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
index 99d2735122d5fe09a7054a07a0c827120a513197..939a59c91796ad135ce6da0b6ca349a7d5f12df3 100644 (file)
--- a/tools/perf/util/dwarf-aux.h
+++ b/tools/perf/util/dwarf-aux.h
@@ -165,12 +165,13 @@ int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf);
 
 /* Find a variable saved in the 'reg' at given address */
 Dwarf_Die *die_find_variable_by_reg(Dwarf_Die *sc_die, Dwarf_Addr pc, int reg,
-                                   int *poffset, bool is_fbreg,
+                                   Dwarf_Die *type_die, int *poffset, bool is_fbreg,
                                    Dwarf_Die *die_mem);
 
 /* Find a (global) variable located in the 'addr' */
 Dwarf_Die *die_find_variable_by_addr(Dwarf_Die *sc_die, Dwarf_Addr addr,
-                                    Dwarf_Die *die_mem, int *offset);
+                                    Dwarf_Die *die_mem, Dwarf_Die *type_die,
+                                    int *offset);
 
 /* Save all variables and parameters in this scope */
 void die_collect_vars(Dwarf_Die *sc_die, struct die_var_type **var_types);