diff --git a/ChangeLog b/ChangeLog index 5386ea82..2f3337c0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,18 @@ +2008-02-17 Soren Sandmann + + * collector.c (lookup_symbol): Add commented out code to reject + callback. + + * elfparser.c (struct ElfParser): Store the filename if any + (elf_parser_get_sym_address): Subtract the load address, so the + result will be an offset into the text section. + + * process.[ch] (process_lookup_symbol): Add an offset out-argument + + * binfile.[ch] (bin_symbol_get_address): New function + + * TODO: updates + 2008-02-16 Soren Sandmann * module/sysprof-module.c (nt_memcpy): Add a memcpy() that uses diff --git a/TODO b/TODO index d745026a..20ff100d 100644 --- a/TODO +++ b/TODO @@ -44,7 +44,7 @@ Before 1.2: - GObject signal emission overhead accounts for 18% of the time. Consider adding a forked version of GtkTreeStore with - performance fixes. + performance and bug fixes. * Make sure that labels look decent in case of "No Map" etc. @@ -363,6 +363,17 @@ Before 1.2: one can be completed using the DWARF information for the shared part. +* Notes on heuristic stack walking + + - We can reject addresses that point exactly to the beginning of a + function since these are likely callbacks. Note though that the + first time a function in a shared library is called, it goes + through dynamic linker resolution which will cause the stack to + contain a callback of the function. This needs to be investigated + in more detail. + + - We are already rejecting addresses outside the text section + (addresses of global variables and the like). * How to get the user stack: diff --git a/binfile.c b/binfile.c index 0a059a85..9315e98d 100644 --- a/binfile.c +++ b/binfile.c @@ -407,3 +407,13 @@ bin_symbol_get_name (BinFile *file, else return elf_parser_get_sym_name (file->elf, (const ElfSym *)symbol); } + +gulong +bin_symbol_get_address (BinFile *file, + const BinSymbol *symbol) +{ + if (file->undefined_name == (char *)symbol) + return 0x0; + else + return file->text_offset + elf_parser_get_sym_address (file->elf, (const ElfSym *)symbol); +} diff --git a/binfile.h b/binfile.h index 713b2250..44dbbb68 100644 --- a/binfile.h +++ b/binfile.h @@ -40,5 +40,7 @@ gboolean bin_file_check_inode (BinFile *bin_file, ino_t inode); const char * bin_symbol_get_name (BinFile *bin_file, const BinSymbol *symbol); +gulong bin_symbol_get_address (BinFile *bin_file, + const BinSymbol *symbol); #endif diff --git a/collector.c b/collector.c index 95fae189..61924079 100644 --- a/collector.c +++ b/collector.c @@ -383,7 +383,7 @@ static char * lookup_symbol (Process *process, gpointer address, GHashTable *unique_symbols, gboolean kernel, - gboolean first_kernel_addr) + gboolean first_addr) { const char *sym; @@ -396,11 +396,11 @@ lookup_symbol (Process *process, gpointer address, /* If offset is 0, it is a callback, not a return address. * - * If "first_kernel_addr" is true, then the address is an + * If "first_addr" is true, then the address is an * instruction pointer, not a return address, so it may * legitimately be at offset 0. */ - if (offset == 0 && !first_kernel_addr) + if (offset == 0 && !first_addr) sym = NULL; /* If offset is greater than 4096, then what happened is most @@ -416,7 +416,19 @@ lookup_symbol (Process *process, gpointer address, } else { - sym = process_lookup_symbol (process, (gulong)address); + gulong offset; + + sym = process_lookup_symbol (process, (gulong)address, &offset); + + if (offset == 0 && !first_addr) + { +#if 0 + sym = g_strdup_printf ("%s [callback]", sym); + g_print ("rejecting %s since it looks like a callback\n", + sym); + sym = NULL; +#endif + } } if (sym) @@ -435,7 +447,7 @@ resolve_symbols (GList *trace, gint size, gpointer data) GPtrArray *resolved_trace = g_ptr_array_new (); char *cmdline; gboolean in_kernel = FALSE; - gboolean first_kernel_addr = TRUE; + gboolean first_addr = TRUE; for (list = trace; list && list->next; list = list->next) { @@ -452,8 +464,8 @@ resolve_symbols (GList *trace, gint size, gpointer data) in_kernel = FALSE; symbol = lookup_symbol (process, address, info->unique_symbols, - in_kernel, first_kernel_addr); - first_kernel_addr = FALSE; + in_kernel, first_addr); + first_addr = FALSE; if (symbol) g_ptr_array_add (resolved_trace, symbol); diff --git a/elfparser.c b/elfparser.c index 688c4fba..e014438f 100644 --- a/elfparser.c +++ b/elfparser.c @@ -57,6 +57,8 @@ struct ElfParser gsize sym_strings; GMappedFile * file; + + char * filename; const Section * text_section; }; @@ -213,6 +215,8 @@ elf_parser_new_from_data (const guchar *data, parser->text_section = find_section (parser, ".text", SHT_PROGBITS); if (!parser->text_section) parser->text_section = find_section (parser, ".text", SHT_NOBITS); + + parser->filename = NULL; return parser; } @@ -252,6 +256,8 @@ elf_parser_new (const char *filename, g_mapped_file_free (file); return NULL; } + + parser->filename = g_strdup (filename); parser->file = file; @@ -356,6 +362,9 @@ elf_parser_free (ElfParser *parser) g_free (parser->symbols); bin_parser_free (parser->parser); + + if (parser->filename) + g_free (parser->filename); g_free (parser); } @@ -461,18 +470,16 @@ read_table (ElfParser *parser, n_functions++; #if 0 - g_print ("symbol: %s: %d\n", get_string_indirect (parser->parser, + g_print (" symbol: %s: %lx\n", get_string_indirect (parser->parser, parser->sym_format, "st_name", - str_table->offset), addr); -#endif -#if 0 - g_print (" sym %d in %p (info: %d:%d) (func:global %d:%d)\n", addr, parser, info & 0xf, info >> 4, STT_FUNC, STB_GLOBAL); + str_table->offset), addr - parser->text_section->load_address); + g_print (" sym %d in %p (info: %d:%d) (func:global %d:%d)\n", addr, parser, info & 0xf, info >> 4, STT_FUNC, STB_GLOBAL); #endif } else if (addr != 0) { #if 0 - g_print (" rejecting %d in %p (info: %d:%d) (func:global %d:%d)\n", addr, parser, info & 0xf, info >> 4, STT_FUNC, STB_GLOBAL); + g_print (" rejecting %d in %p (info: %d:%d) (func:global %d:%d)\n", addr, parser, info & 0xf, info >> 4, STT_FUNC, STB_GLOBAL); #endif } @@ -496,10 +503,16 @@ read_symbols (ElfParser *parser) if (symtab && strtab) { +#if 0 + g_print ("reading symbol table of %s\n", parser->filename); +#endif read_table (parser, symtab, strtab); } else if (dynsym && dynstr) { +#if 0 + g_print ("reading dynamic symbol table of %s\n", parser->filename); +#endif read_table (parser, dynsym, dynstr); } else @@ -565,7 +578,7 @@ elf_parser_lookup_symbol (ElfParser *parser, return NULL; address += parser->text_section->load_address; - + #if 0 g_print ("elf: the address we are looking up is %p\n", address); #endif @@ -596,6 +609,13 @@ elf_parser_lookup_symbol (ElfParser *parser, result = NULL; } + if (result) + { + /* Reject the symbols if the address is outside the text section */ + if (address > parser->text_section->load_address + parser->text_section->size) + result = NULL; + } + return result; } @@ -662,7 +682,7 @@ gulong elf_parser_get_sym_address (ElfParser *parser, const ElfSym *sym) { - return sym->address; + return sym->address - parser->text_section->load_address; } /* diff --git a/process.c b/process.c index c4ad121e..bcba43f8 100644 --- a/process.c +++ b/process.c @@ -673,7 +673,7 @@ process_lookup_kernel_symbol (gulong address, } const char * -process_lookup_symbol (Process *process, gulong address) +process_lookup_symbol (Process *process, gulong address, gulong *offset) { static const char *const kernel = "kernel"; const BinSymbol *result; @@ -752,6 +752,17 @@ process_lookup_symbol (Process *process, gulong address) #endif /* g_print ("(%x) %x %x name; %s\n", address, map->start, map->offset, result->name); */ + +#if 0 + g_print ("name: %s (in %s)\n", bin_symbol_get_name (map->bin_file, result), map->filename); + g_print (" in addr: %lx\n", address); + g_print (" out addr: %lx\n", bin_symbol_get_address (map->bin_file, result)); + g_print (" map start: %lx\n", map->start); + g_print (" map offset: %lx\n", map->offset); +#endif + + if (offset) + *offset = address - bin_symbol_get_address (map->bin_file, result); return bin_symbol_get_name (map->bin_file, result); } diff --git a/process.h b/process.h index a7d7b6ee..a72714cd 100644 --- a/process.h +++ b/process.h @@ -52,7 +52,8 @@ void process_ensure_map (Process *process, int pid, gulong address); const char * process_lookup_symbol (Process *process, - gulong address); + gulong address, + gulong *offset); const char * process_get_cmdline (Process *process); void process_flush_caches (void); const guint8 *process_get_vdso_bytes (gsize *length);