sysprof r397 - trunk

[ssp svn gnome org]

Author: ssp
Date: Sun Feb 17 23:31:19 2008
New Revision: 397
URL: http://svn.gnome.org/viewvc/sysprof?rev=397&view=rev

Log:
2008-02-17  Soren Sandmann  <sandmann redhat com>

	* 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




Modified:
   trunk/ChangeLog
   trunk/TODO
   trunk/binfile.c
   trunk/binfile.h
   trunk/collector.c
   trunk/elfparser.c
   trunk/process.c
   trunk/process.h

Modified: trunk/TODO
==============================================================================
--- trunk/TODO	(original)
+++ trunk/TODO	Sun Feb 17 23:31:19 2008
@@ -44,7 +44,7 @@
 		  - 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 @@
 		  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:
 

Modified: trunk/binfile.c
==============================================================================
--- trunk/binfile.c	(original)
+++ trunk/binfile.c	Sun Feb 17 23:31:19 2008
@@ -407,3 +407,13 @@
     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);
+}

Modified: trunk/binfile.h
==============================================================================
--- trunk/binfile.h	(original)
+++ trunk/binfile.h	Sun Feb 17 23:31:19 2008
@@ -40,5 +40,7 @@
 					 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

Modified: trunk/collector.c
==============================================================================
--- trunk/collector.c	(original)
+++ trunk/collector.c	Sun Feb 17 23:31:19 2008
@@ -383,7 +383,7 @@
 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 @@
 
 	/* 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 @@
     }
     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 @@
     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 @@
 	    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);

Modified: trunk/elfparser.c
==============================================================================
--- trunk/elfparser.c	(original)
+++ trunk/elfparser.c	Sun Feb 17 23:31:19 2008
@@ -57,6 +57,8 @@
     gsize		sym_strings;
     
     GMappedFile *	file;
+
+    char *		filename;
     
     const Section *	text_section;
 };
@@ -213,6 +215,8 @@
     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 @@
 	g_mapped_file_free (file);
 	return NULL;
     }
+
+    parser->filename = g_strdup (filename);
     
     parser->file = file;
     
@@ -356,6 +362,9 @@
     g_free (parser->symbols);
     
     bin_parser_free (parser->parser);
+
+    if (parser->filename)
+	g_free (parser->filename);
     
     g_free (parser);
 }
@@ -461,18 +470,16 @@
 	    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 @@
     
     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 @@
 	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 @@
 	    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 @@
 elf_parser_get_sym_address (ElfParser *parser,
 			    const ElfSym *sym)
 {
-    return sym->address;
+    return sym->address - parser->text_section->load_address;
 }
 
 /*

Modified: trunk/process.c
==============================================================================
--- trunk/process.c	(original)
+++ trunk/process.c	Sun Feb 17 23:31:19 2008
@@ -673,7 +673,7 @@
 }
 
 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 @@
 #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);
 }

Modified: trunk/process.h
==============================================================================
--- trunk/process.h	(original)
+++ trunk/process.h	Sun Feb 17 23:31:19 2008
@@ -52,7 +52,8 @@
 						   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);