- 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