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  <[EMAIL PROTECTED]>

        * 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);
_______________________________________________
SVN-commits-list mailing list (read only)
http://mail.gnome.org/mailman/listinfo/svn-commits-list

Want to limit the commits to a few modules? Go to above URL, log in to edit 
your options and select the modules ('topics') you want.
Module maintainer? It is possible to set the reply-to to your development 
mailing list. Email [EMAIL PROTECTED] if interested.

Reply via email to