perf symbol fixes
cover perf symbol fixes
The following patches fix some bugs in our symbol code and adds heuristics to select which duplicate symbol to use.
In the process this fixes the issue on ppc where we need to handle both the old style and new style 64bit ABI.
Anton
PATCH 1/4 perf symbols: Fix ppc64 SEGV in dso__load_sym with debuginfo files
64bit PowerPC debuginfo files have an empty function descriptor section. I hit a SEGV when perf tried to use this section for symbol resolution.
To fix this we need to check the section is valid and we can do this by checking for type SHT_PROGBITS.
---
tools/perf/util/symbol.c +2 -0
@@ -1113,6 +1113,8 @@ static int dso__load_sym(struct dso *dso } opdsec =elf_section_by_name (elf, &ehdr, &opdshdr,".opd" , &opdidx); +if (opdshdr.sh_type !=SHT_PROGBITS ) + opdsec =NULL ;if (opdsec) opddata =elf_rawdata (opdsec,NULL );
PATCH 2/4 perf symbols: /proc/kallsyms does not sort module symbols
In reply to Anton Blanchardkallsyms__parse assumes that /proc/kallsyms is sorted and sets the end of the previous symbol to the start of the current one.
Unfortunately module symbols are not sorted, eg:
ffffffffa0081f30 t e1000_clean_rx_irq [e1000e]ffffffffa00817a0 t e1000_alloc_rx_buffers [e1000e]
Some symbols end up with a negative length and others have a length larger than they should. This results in confusing perf output.
We already have a function to fixup the end of zero length symbols so use that instead.
---
tools/perf/util/symbol.c +11 -22
@@ -440,18 +440,11 @@ int kallsyms__parse(const char *filenamechar *line =NULL ;size_t n;int err =-1 ; -u64 prev_start =0 ; -char prev_symbol_type =0 ; -char *prev_symbol_name;FILE *file =fopen (filename,"r" );if (file ==NULL ) goto out_failure; - prev_symbol_name =malloc (KSYM_NAME_LEN ); -if (prev_symbol_name ==NULL ) - goto out_close; - err =0 ;while (!feof (file)) {
@@ -482,24 +475,18 @@ int kallsyms__parse(const char *filenamebreak ; } -if (prev_symbol_type) { -u64 end = start; -if (end != prev_start) - --end; - err =process_symbol (arg, prev_symbol_name, - prev_symbol_type, prev_start, end); -if (err) -break ; - } - -memcpy (prev_symbol_name, symbol_name, len +1 ); - prev_symbol_type = symbol_type; - prev_start = start; +/* +* module symbols are not sorted so we add all +* symbols with zero length and rely on +* symbols__fixup_end() to fix it up. +*/ + err =process_symbol (arg, symbol_name, + symbol_type, start, start); +if (err) +break ; } -free (prev_symbol_name);free (line); -out_close:fclose (file);return err;
@@ -705,6 +692,8 @@ int dso__load_kallsyms(struct dso *dso,if (dso__load_all_kallsyms (dso, filename, map) <0 )return -1 ; +symbols__fixup_end (&dso->symbols[map->type]); +if (dso->kernel ==DSO_TYPE_GUEST_KERNEL ) dso->symtab_type =SYMTAB__GUEST_KALLSYMS ;else
PATCH 3/4 perf symbols: Preserve symbol scope when parsing /proc/kallsyms
In reply to Anton Blanchardkallsyms__parse capitalises the symbol type, so every symbol is marked global. Remove this and fix symbol_type__is_a to handle both local and global symbols.
---
tools/perf/util/symbol.c +4 -2
@@ -76,11 +76,13 @@ static void dso__set_sorted_by_name(strubool symbol_type__is_a (char symbol_type,enum map_type map_type) { + symbol_type =toupper (symbol_type); +switch (map_type) {case MAP__FUNCTION :return symbol_type =='T' || symbol_type =='W' ;case MAP__VARIABLE : -return symbol_type =='D' || symbol_type =='d' ; +return symbol_type =='D' ;default :return false; }
@@ -465,7 +467,7 @@ int kallsyms__parse(const char *filenameif (len +2 >= line_len)continue ; - symbol_type =toupper (line[len]); + symbol_type = line[len]; len +=2 ; symbol_name = line + len; len = line_len - len;
PATCH 4/4 perf symbols: Add some heuristics for choosing the best duplicate symbol
In reply to Anton BlanchardTry and pick the best symbol based on a few heuristics:
- Prefer a non weak symbol over a weak one - Prefer a global symbol over a non global one - Prefer a symbol with less underscores (idea taken from kallsyms.c) - If all else fails, choose the symbol with the longest name
---
tools/perf/util/symbol.c +88 -0
@@ -88,6 +88,92 @@ bool symbol_type__is_a(char symbol_type, } } +static int prefix_underscores_count (const char *str) +{ +const char *tail = str; + +while (*tail =='_' ) + tail++; + +return tail - str; +} + +#define SYMBOL_A 0 +#define SYMBOL_B 1 + +static int choose_best_symbol (struct symbol *syma,struct symbol *symb) +{ +s64 a; +s64 b; + +/* Prefer a symbol with non zero length */ + a = syma->end - syma->start; + b = symb->end - symb->start; +if ((b ==0 ) && (a >0 )) +return SYMBOL_A ; +else if ((a ==0 ) && (b >0 )) +return SYMBOL_B ; + +/* Prefer a non weak symbol over a weak one */ + a = syma->binding ==STB_WEAK ; + b = symb->binding ==STB_WEAK ; +if (b && !a) +return SYMBOL_A ; +if (a && !b) +return SYMBOL_B ; + +/* Prefer a global symbol over a non global one */ + a = syma->binding ==STB_GLOBAL ; + b = symb->binding ==STB_GLOBAL ; +if (a && !b) +return SYMBOL_A ; +if (b && !a) +return SYMBOL_B ; + +/* Prefer a symbol with less underscores */ + a =prefix_underscores_count (syma->name); + b =prefix_underscores_count (symb->name); +if (b > a) +return SYMBOL_A ; +else if (a > b) +return SYMBOL_B ; + +/* If all else fails, choose the symbol with the longest name */ +if (strlen (syma->name) >=strlen (symb->name)) +return SYMBOL_A ; +else +return SYMBOL_B ; +} + +static void symbols__fixup_duplicate (struct rb_root *symbols) +{ +struct rb_node *nd; +struct symbol *curr, *next; + + nd =rb_first (symbols); + +while (nd) { + curr = rb_entry(nd, struct symbol, rb_node); +again: + nd =rb_next (&curr->rb_node); + next = rb_entry(nd, struct symbol, rb_node); + +if (!nd) +break ; + +if (curr->start != next->start) +continue ; + +if (choose_best_symbol (curr, next) ==SYMBOL_A ) { +rb_erase (&next->rb_node, symbols); + goto again; + }else { + nd =rb_next (&curr->rb_node); +rb_erase (&curr->rb_node, symbols); + } + } +} +static void symbols__fixup_end (struct rb_root *symbols) {struct rb_node *nd, *prevnd =rb_first (symbols);
@@ -694,6 +780,7 @@ int dso__load_kallsyms(struct dso *dso,if (dso__load_all_kallsyms (dso, filename, map) <0 )return -1 ; +symbols__fixup_duplicate (&dso->symbols[map->type]);symbols__fixup_end (&dso->symbols[map->type]);if (dso->kernel ==DSO_TYPE_GUEST_KERNEL )
@@ -1271,6 +1358,7 @@ new_symbol:* For misannotated, zeroed, ASM function sizes. */ if (nr >0 ) { +symbols__fixup_duplicate (&dso->symbols[map->type]);symbols__fixup_end (&dso->symbols[map->type]);if (kmap) {/*