Deal with address offsets. Address lookup is now:

2006-10-07  Soren Sandmann  <sandmann@daimi.au.dk>

	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

@@ -1,3 +1,26 @@
+2006-10-07  Soren Sandmann  <sandmann@daimi.au.dk>
+
+	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 <sandmann@redhat.com>
 
 	* binparser.c: Remove old commented out code

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

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;

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,
@@ -383,6 +398,10 @@ read_table (ElfParser *parser,
     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
@@ -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;
     
@@ -486,14 +507,21 @@ elf_parser_lookup_symbol (ElfParser *parser,
     if (parser->n_symbols == 0)
 	return NULL;
 
-    address += elf_parser_get_load_address (parser);
+    text = find_section (parser, ".text");
+    if (!text)
+	return NULL;
+    
+    address += text->load_address;
 
-#if 0
     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)
 {

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.
  *

module/sysprof-module.c

@@ -19,7 +19,6 @@
  * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
  */
 
-#include <linux/config.h>
 #ifdef CONFIG_SMP
 # define __SMP__
 #endif

process.c

@@ -580,16 +580,23 @@ 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)
     {