- fix off-by-one errors during symbol insertion and lookup - don''t store the symbol type, as it wasn''t needed at all so far and is only needed now at parsing time - don''t insert certain kinds of symbols Signed-off-by: Jan Beulich <jbeulich@novell.com> --- a/tools/xentrace/xenctx.c +++ b/tools/xentrace/xenctx.c @@ -19,6 +19,7 @@ #include <unistd.h> #include <errno.h> #include <signal.h> +#include <ctype.h> #include <string.h> #include <inttypes.h> #include <getopt.h> @@ -58,7 +59,6 @@ int disp_tlb; struct symbol { guest_word_t address; - char type; char *name; struct symbol *next; } *symbol_table = NULL; @@ -112,12 +112,12 @@ static void insert_symbol(struct symbol /* The System.map is usually already sorted... */ if (prev - && prev->address < symbol->address + && prev->address <= symbol->address && (!prev->next || prev->next->address > symbol->address)) { s = prev; } else { /* ... otherwise do crappy/slow search for the correct place */ - while(s && s->next && s->next->address < symbol->address) + while (s->next && s->next->address <= symbol->address) s = s->next; } @@ -130,13 +130,13 @@ static struct symbol *lookup_symbol(gues { struct symbol *s = symbol_table; - while(s && s->next && s->next->address < address) - s = s->next; + if (!s) + return NULL; - if (s && s->address < address) - return s; + while (s->next && s->next->address < address) + s = s->next; - return NULL; + return s->next && s->next->address <= address ? s->next : s; } static void print_symbol(guest_word_t addr) @@ -159,7 +159,7 @@ static void print_symbol(guest_word_t ad static void read_symbol_table(const char *symtab) { - char line[256]; + char type, line[256]; char *p; struct symbol *symbol; FILE *f; @@ -178,9 +178,13 @@ static void read_symbol_table(const char /* need more checks for syntax here... */ symbol->address = strtoull(line, &p, 16); - p++; - symbol->type = *p++; - p++; + if (!isspace(*p++)) + continue; + type = *p++; + if (!isalpha(type) && type != ''?'') + continue; + if (!isspace(*p++)) + continue; /* in the future we should handle the module name * being appended here, this would allow us to use @@ -190,7 +194,18 @@ static void read_symbol_table(const char p[strlen(p)-1] = ''\0''; symbol->name = strdup(p); - insert_symbol(symbol); + switch (type) { + case ''A'': /* global absolute */ + case ''a'': /* local absolute */ + break; + case ''U'': /* undefined */ + case ''v'': /* undefined weak object */ + case ''w'': /* undefined weak function */ + continue; + default: + insert_symbol(symbol); + break; + } if (strcmp(symbol->name, "_stext") == 0) kernel_stext = symbol->address; _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich writes ("[Xen-devel] [PATCH] xenctx: misc adjustments"):> - fix off-by-one errors during symbol insertion and lookup > - don''t store the symbol type, as it wasn''t needed at all so far and > is only needed now at parsing time > - don''t insert certain kinds of symbols...> - insert_symbol(symbol); > + switch (type) { > + case ''A'': /* global absolute */ > + case ''a'': /* local absolute */ > + break;Is there some reason why these shouldn''t included ? Perhaps I''m misunderstanding what their purpose is. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>>> On 11.01.11 at 20:27, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote: > Jan Beulich writes ("[Xen-devel] [PATCH] xenctx: misc adjustments"): >> - fix off-by-one errors during symbol insertion and lookup >> - don''t store the symbol type, as it wasn''t needed at all so far and >> is only needed now at parsing time >> - don''t insert certain kinds of symbols > ... >> - insert_symbol(symbol); >> + switch (type) { >> + case ''A'': /* global absolute */ >> + case ''a'': /* local absolute */ >> + break; > > Is there some reason why these shouldn''t included ? Perhaps I''m > misunderstanding what their purpose is.Absolute symbols (with very few exceptions) don''t represent addresses (take the symbol CRC values in older Linux as an example; current Linux has vDSO relative(!) addresses among them). Since some versions have absolute _text etc, they''re being allowed to fall into the code path following the switch statement (other than undefined symbols), but for the purpose of annotating stack traces they''re useless (and in all reality, for relocatable kernels, there ought to be a mechanism to add a fixed offset to all addresses, which immediately disqualifies all absolute ones except those few special purpose ones). Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich writes ("Re: [Xen-devel] [PATCH] xenctx: misc adjustments"):> On 11.01.11 at 20:27, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote: > > Is there some reason why these shouldn''t included ? Perhaps I''m > > misunderstanding what their purpose is. > > Absolute symbols (with very few exceptions) don''t represent > addresses (take the symbol CRC values in older Linux as an > example; current Linux has vDSO relative(!) addresses among > them).Thanks for the explanation.> Since some versions have absolute _text etc, they''re being > allowed to fall into the code path following the switch > statement (other than undefined symbols), but for the purpose > of annotating stack traces they''re useless (and in all reality, for > relocatable kernels, there ought to be a mechanism to add a > fixed offset to all addresses, which immediately disqualifies > all absolute ones except those few special purpose ones).Right. Well I think this is a bugfix so can go in during the feature freeze. But ATM we''re hoping for a test pass so we can 4.1.0 rc1 so I''m going to hold off. Thanks, Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel