From 52274fadd6f2d013e662fa8965092f98c0d3d566 Mon Sep 17 00:00:00 2001 From: Soren Sandmann Date: Sun, 8 Oct 2006 04:03:02 +0000 Subject: [PATCH] Deal with address offsets. Address lookup is now: 2006-10-07 Soren Sandmann Deal with address offsets. Address lookup is now: - First convert the address to an offset into the file - Then convert to an offset into the text segment - Then add the load address of the text segment (found in the debug binary) - Then finally lookup the result in the symbol table. * elfparser.c (elf_parser_get_text_offset): New function * elfparser.c (elf_parser_lookup_symbol): Treat addresses as offsets into the text segment. * binfile.c (bin_file_new): Store the offset of the text section of the actual binary (not the debug one) (bin_file_lookup_symbol): Subtract text_offset before passing address to elf parser. * module/sysprof-module.c: Remove include of linux/config.h --- ChangeLog | 23 +++++++++++++++ TODO | 64 ++++++++++++++++++++++++++++++++++++++++- binfile.c | 15 ++++++++++ elfparser.c | 55 +++++++++++++++++++++++++++++++---- elfparser.h | 2 ++ module/sysprof-module.c | 1 - process.c | 13 +++++++-- 7 files changed, 162 insertions(+), 11 deletions(-) diff --git a/ChangeLog b/ChangeLog index e598d416..bec1a166 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,26 @@ +2006-10-07 Soren Sandmann + + Deal with address offsets. Address lookup is now: + + - First convert the address to an offset into the file + - Then convert to an offset into the text segment + - Then add the load address of the text segment (found in the + debug binary) + - Then finally lookup the result in the symbol table. + + * elfparser.c (elf_parser_get_text_offset): New function + + * elfparser.c (elf_parser_lookup_symbol): Treat addresses as + offsets into the text segment. + + * binfile.c (bin_file_new): Store the offset of the text section + of the actual binary (not the debug one) + + (bin_file_lookup_symbol): Subtract text_offset before passing + address to elf parser. + + * module/sysprof-module.c: Remove include of linux/config.h + 2006-08-27 Soren Sandmann * binparser.c: Remove old commented out code diff --git a/TODO b/TODO index 1acb347a..6e8aa47d 100644 --- a/TODO +++ b/TODO @@ -32,6 +32,10 @@ Before 1.0.4: Before 1.2: +* Try reproducing crash when profiling xrender demo + +* Fix (potential) performance issues in symbol lookup. + * Elf bugs: - when an elf file is read, it should be checked that the various @@ -41,6 +45,8 @@ Before 1.2: - Also error handling for bin_parser is necessary. - Can .gnu_debuglink recurse? + yes, it can, and we should probably not crash if there are + cycles in the graph. * Strategies for taking reliable stacktraces. @@ -76,7 +82,6 @@ Before 1.2: - do ebp based stackwalk in userland - do heuristic stackwalk in kernel - do heuristic stackwalk in userland - - * "Expand all" is horrendously slow because update screenshot gets called for every "expanded" signal. @@ -651,6 +656,63 @@ Later: -=-=-=-=-=-=-=-=-=-=-=-=-=-=- ALREADY DONE -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- +* Find out why we are getting bogus symbols reported for /usr/bin/Xorg + Like this: + + Everything 0.00 100.00 + [/usr/bin/Xorg] 0.00 94.79 + GetScratchPixmapHeader 0.00 94.79 + __libc_start_main 0.00 94.79 + FindAllClientResources 0.00 94.79 + FreeFontPath 0.00 94.79 + SProcRenderCreateConicalGradient 0.00 94.56 + ProcRenderTrapezoids 0.00 94.56 + AllocatePicture 0.00 94.56 + __glXDispatch 0.00 0.16 + __glXVendorPrivate 0.00 0.08 + __glXRender 0.00 0.08 + SmartScheduleStartTimer 0.00 0.08 + [./sysprof] 0.00 2.76 + [sshd: ssp@pts/0] 0.00 2.37 + [compiz] 0.00 0.08 + + What's going on here is that the computed load address for the X server + binary is different for the debug binary. The lowest allocated address + is 0x08047134 for the normal, and 0x8048134 for the debug. But it looks + like the addresses are still the same for the symbols. + The root of this problem may be that we should ignore the load + address of the debug binary, and just lookup the address computed. + The *text* segments have the same address though. Everything from + "gnu version" on has the same address. + + So: + - find out where in memory the text segment is + - take an address and compute its offset to the text segment + - in elf parser, find address of text segment + - add offset + - lookup resulting address + +So basically, "load address" should really be text address. Except of course +that load_address is not used in process.c - instead the 'load address' of the +first part of the file is computed and assumed to be equivalent to the +load address. So to lookup something you probably actually need +to know the load/offset at the elf parser level: + + lookup_symbol (elf, map, offset, address) + +then, + + real load address of text (lta) = map - offset + text_offset + + offset of called func (ocf): addr - lta + + thing to lookup in table: ocf + text_addr.loadaddr in debug + + addr - map - offset + text_offset + + + hmmm ... + * plug all the leaks - don't leak the presentation strings/objects - maybe add stack_stash_set_free_func() or something diff --git a/binfile.c b/binfile.c index 494c9c97..4e7b36cd 100644 --- a/binfile.c +++ b/binfile.c @@ -46,6 +46,8 @@ struct BinFile ino_t inode; char * undefined_name; + + gulong text_offset; }; /* FIXME: error handling */ @@ -154,6 +156,13 @@ bin_file_new (const char *filename) { bf = g_new0 (BinFile, 1); bf->elf = elf_parser_new (filename, NULL); + + /* We need the text offset of the actual binary, not the + * (potential) debug binary + */ + if (bf->elf) + bf->text_offset = elf_parser_get_text_offset (bf->elf); + bf->elf = find_separate_debug_file (bf->elf, filename); bf->inode = read_inode (filename); bf->filename = g_strdup (filename); @@ -188,10 +197,16 @@ bin_file_lookup_symbol (BinFile *bin_file, { if (bin_file->elf) { + address -= bin_file->text_offset; + const ElfSym *sym = elf_parser_lookup_symbol (bin_file->elf, address); + if (sym) + { + g_print ("found %lx => %s\n", address, bin_symbol_get_name (bin_file, sym)); return (const BinSymbol *)sym; + } } return (const BinSymbol *)bin_file->undefined_name; diff --git a/elfparser.c b/elfparser.c index cd9161ce..c8ed2b32 100644 --- a/elfparser.c +++ b/elfparser.c @@ -322,6 +322,21 @@ compare_sym (const void *a, const void *b) return 1; } +#if 0 +static void +dump_symbols (ElfParser *parser, ElfSym *syms, guint n_syms) +{ + int i; + + for (i = 0; i < n_syms; ++i) + { + ElfSym *s = &(syms[i]); + + g_print (" %s: %lx\n", elf_parser_get_sym_name (parser, s), s->address); + } +} +#endif + static void read_table (ElfParser *parser, const Section *sym_table, @@ -381,8 +396,12 @@ read_table (ElfParser *parser, parser->sym_strings = str_table->offset; parser->n_symbols = n_functions; parser->symbols = g_renew (ElfSym, parser->symbols, parser->n_symbols); - + qsort (parser->symbols, parser->n_symbols, sizeof (ElfSym), compare_sym); + +#if 0 + dump_symbols (parser, parser->symbols, parser->n_symbols); +#endif } static void @@ -432,7 +451,7 @@ elf_parser_get_load_address (ElfParser *parser) load_address = MIN (load_address, addr); } } - + #if 0 g_print ("load address: %8p\n", (void *)load_address); #endif @@ -473,10 +492,12 @@ do_lookup (ElfSym *symbols, } } +/* Address should be given in 'offset into text segment' */ const ElfSym * elf_parser_lookup_symbol (ElfParser *parser, gulong address) { + Section *text; const ElfSym *result; gsize size; @@ -485,15 +506,22 @@ elf_parser_lookup_symbol (ElfParser *parser, if (parser->n_symbols == 0) return NULL; - - address += elf_parser_get_load_address (parser); -#if 0 + text = find_section (parser, ".text"); + if (!text) + return NULL; + + address += text->load_address; + g_print ("the address we are looking up is %p\n", address); -#endif result = do_lookup (parser->symbols, address, 0, parser->n_symbols - 1); + if (result) + { + g_print ("found %s at %lx\n", elf_parser_get_sym_name (parser, result), result->address); + } + if (result) { BinRecord *symbol; @@ -513,6 +541,21 @@ elf_parser_lookup_symbol (ElfParser *parser, return result; } +gulong +elf_parser_get_text_offset (ElfParser *parser) +{ + const Section *text; + + g_return_val_if_fail (parser != NULL, (gulong)-1); + + text = find_section (parser, ".text"); + + if (!text) + return (gulong)-1; + + return text->offset; +} + const char * elf_parser_get_debug_link (ElfParser *parser, guint32 *crc32) { diff --git a/elfparser.h b/elfparser.h index 4c8003cf..714a7c1b 100644 --- a/elfparser.h +++ b/elfparser.h @@ -9,6 +9,8 @@ void elf_parser_free (ElfParser *parser); const char * elf_parser_get_debug_link (ElfParser *parser, guint32 *crc32); const guchar *elf_parser_get_eh_frame (ElfParser *parser); +gulong elf_parser_get_text_offset (ElfParser *parser); + /* Lookup a symbol in the file. * diff --git a/module/sysprof-module.c b/module/sysprof-module.c index cf03d37f..edb1a0cd 100644 --- a/module/sysprof-module.c +++ b/module/sysprof-module.c @@ -19,7 +19,6 @@ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. */ -#include #ifdef CONFIG_SMP # define __SMP__ #endif diff --git a/process.c b/process.c index f7824806..195799ed 100644 --- a/process.c +++ b/process.c @@ -555,7 +555,7 @@ process_lookup_symbol (Process *process, gulong address) Map *map = process_locate_map (process, address); /* g_print ("addr: %x\n", address); */ - + if (address == 0x1) { get_kernel_symbols (); @@ -580,15 +580,22 @@ process_lookup_symbol (Process *process, gulong address) g_print ("map address: %lx\n", map->start); g_print ("map offset: %lx\n", map->offset); - g_print ("address before: %lx\n", address); +#endif + g_print ("address before: %lx (%s)\n", address, map->filename); +#if 0 } #endif #if 0 - g_print ("address before: \n"); + g_print ("address before: \n"); #endif address -= map->start; address += map->offset; + +#if 0 + address -= map->start; + address += map->offset; +#endif #if 0 if (strcmp (map->filename, "/home/ssp/sysprof/sysprof") == 0)