New function.

2006-10-09  Soren Sandmann  <ssp@localhost.localdomain>

	* process.c (process_get_vdso_bytes): New function.

	* elfparser.c (parser_new_from_data): export this function as
	elf_parser_new_from_data().

	* binfile.c (read_inode): Don't stat if filename is
	'[vdso]'. Instead just return -1;
	(bin_file_new): Use elf_parser_new_from_data() when the file is
	[vdso].

	* process.c (read_maps): Change the offset of the vdso map to 0
	and the inode to -1.

	* elfparser.c (elf_parser_lookup_symbol): Remove unused 'size'
	variable.

	* binfile.c (find_separate_debug_file): Deal with cycles in the
	debuglink graph.

	* configure.ac: Set version to 1.1.0. Print warning about HEAD.

8 files changed, 190 insertions, 55 deletions

diff --git a/ChangeLog b/ChangeLog
index 8c5f920..f6fd32d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,4 +1,27 @@
-2006-10-08   Soren Sandmann <sandmann@daimi.au.dk>
+2006-10-09  Soren Sandmann  <ssp@localhost.localdomain>
+
+	* process.c (process_get_vdso_bytes): New function.
+
+	* elfparser.c (parser_new_from_data): export this function as
+	elf_parser_new_from_data(). 
+
+	* binfile.c (read_inode): Don't stat if filename is
+	'[vdso]'. Instead just return -1;
+	(bin_file_new): Use elf_parser_new_from_data() when the file is
+	[vdso]. 
+
+	* process.c (read_maps): Change the offset of the vdso map to 0
+	and the inode to -1.
+
+	* elfparser.c (elf_parser_lookup_symbol): Remove unused 'size'
+	variable.
+
+	* binfile.c (find_separate_debug_file): Deal with cycles in the
+	debuglink graph.
+
+	* configure.ac: Set version to 1.1.0. Print warning about HEAD.
+
+2006-10-08  Soren Sandmann  <ssp@localhost.localdomain>
 
 	* binfile.c: Don't include bfd.h
 
diff --git a/TODO b/TODO
index b91e3c5..2e89626 100644
--- a/TODO
+++ b/TODO
@@ -37,11 +37,16 @@ Before 1.2:
 * Elf bugs:
 	- error handling for bin_parser is necessary.
 
-	- don't loop infinitely if there are cycles in the debuglink graph.
-
 	* Find out why all apps have an "In file /usr/bin/<app binary>" below
 	  _libc_main. If possible, maybe make up a name for it.
 
+* vdso stuff:
+	- the "[vdso]" string should be #defined somewhere
+	- Does get_vdso_bytes() belong in process.c?
+	- Is basing on "[vdso]" always correct?
+
+* Convert things like [heap] and [stack] to more understandable labels.
+
 * Strategies for taking reliable stacktraces.
 
 	Three different kinds of files
@@ -64,6 +69,7 @@ Before 1.2:
 	- vdso
 		- assume its the same across processes, just look at
 		  sysprof's own copy.
+			Done: vdso is done now
 		- send copy of it to userspace once, or for every
 		  sample. 
 
@@ -154,8 +160,6 @@ Before 1.2:
 
 * Make it compile and work on x86-64
 
-* Add "sysprof --version"
-
 * With kernel module not installed, select Profiler->Start, then dismiss
   the alert. This causes the start button to appear prelighted. Probably
   just another gtk+ bug.
@@ -652,6 +656,10 @@ Later:
 
 -=-=-=-=-=-=-=-=-=-=-=-=-=-=- ALREADY DONE -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
 
+* don't loop infinitely if there are cycles in the debuglink graph.
+
+* Add "sysprof --version"
+
 * Fix (potential) performance issues in symbol lookup.
 
 - when an elf file is read, it should be checked that the various
diff --git a/binfile.c b/binfile.c
index 1560e18..91eadc8 100644
--- a/binfile.c
+++ b/binfile.c
@@ -23,9 +23,9 @@
 /* Most interesting code in this file is lifted from bfdutils.c
  * and process.c from Memprof,
  */
+#include "config.h"
 
 #include <glib.h>
-#include "binfile.h"
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
@@ -33,7 +33,10 @@
 #include <stdio.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+
+#include "binfile.h"
 #include "elfparser.h"
+#include "process.h"
 
 struct BinFile
 {
@@ -54,9 +57,11 @@ static ino_t
 read_inode (const char *filename)
 {
     struct stat statbuf;
-    
+
+    if (strcmp (filename, "[vdso]") == 0)
+	return (ino_t)-1;
+
     stat (filename, &statbuf);
-    
     return statbuf.st_ino;
 }
 
@@ -84,7 +89,7 @@ separate_debug_file_exists (const char *name, guint32 crc)
 }
 
 /* FIXME - not10: this should probably be detected by config.h -- find out what gdb does*/
-static const char *const debug_file_directory = "/usr/lib/debug";
+static const char *const debug_file_directory = DEBUGDIR;
 
 static ElfParser *
 get_debug_file (ElfParser *elf,
@@ -131,6 +136,21 @@ get_debug_file (ElfParser *elf,
     return result;
 }
 
+static gboolean
+list_contains_name (GList *names, const char *name)
+{
+    GList *list;
+    for (list = names; list != NULL; list = list->next)
+    {
+	const char *n = list->data;
+
+	if (strcmp (n, name) == 0)
+	    return TRUE;
+    }
+
+    return FALSE;
+}
+
 static ElfParser *
 find_separate_debug_file (ElfParser *elf,
 			  const char *filename)
@@ -138,29 +158,35 @@ find_separate_debug_file (ElfParser *elf,
     ElfParser *debug;
     char *debug_name = NULL;
     char *fname;
+    GList *seen_names = NULL;
 
     fname = g_strdup (filename);
 
-    /* FIXME: there is an infinite loop if there are cycles in
-     * the debug_link graph
-     */
     do
     {
+	if (list_contains_name (seen_names, fname))
+	{
+	    /* cycle detected, just return the original elf file itself */
+	    break;
+	}
+	    
 	debug = get_debug_file (elf, fname, &debug_name);
 
 	if (debug)
 	{
 	    elf_parser_free (elf);
 	    elf = debug;
-
-	    g_free (fname);
+	    
+	    seen_names = g_list_prepend (seen_names, fname);
 	    fname = debug_name;
 	}
     }
     while (debug);
 
     g_free (fname);
-
+    g_list_foreach (seen_names, (GFunc)g_free, NULL);
+    g_list_free (seen_names);
+    
     return elf;
 }
 
@@ -184,15 +210,38 @@ bin_file_new (const char *filename)
     else
     {
 	bf = g_new0 (BinFile, 1);
-	bf->elf = elf_parser_new (filename, NULL);
 
+	if (strcmp (filename, "[vdso]") == 0)
+	{
+	    gsize length;
+	    const guint8 *vdso_bytes;
+
+	    vdso_bytes = process_get_vdso_bytes (&length);
+	    if (vdso_bytes)
+	    {
+		bf->elf = elf_parser_new_from_data (vdso_bytes, length);
+#if 0
+		g_print ("got vdso elf: %p (%d)\n", bf->elf, length);
+#endif
+	    }
+	    else
+		bf->elf = NULL;
+	}
+	else
+	{
+	    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);
-	
+#if 0
+	    g_print ("text offset: %d\n", bf->text_offset);
+#endif
+	    
 	    bf->elf = find_separate_debug_file (bf->elf, filename);
 	}
 	
@@ -200,7 +249,7 @@ bin_file_new (const char *filename)
 	bf->filename = g_strdup (filename);
 	bf->undefined_name = g_strdup_printf ("In file %s", filename);
 	bf->ref_count = 1;
-
+	
 	g_hash_table_insert (bin_files, bf->filename, bf);
     }
     
@@ -229,12 +278,15 @@ bin_file_lookup_symbol (BinFile    *bin_file,
 {
     if (bin_file->elf)
     {
+#if 0
+	g_print ("bin file lookup lookup %d\n", address);
+#endif
 	address -= bin_file->text_offset;
 	
 	const ElfSym *sym = elf_parser_lookup_symbol (bin_file->elf, address);
 
 #if 0
-	g_print ("lookup in %s\n", bin_file->filename);
+	g_print ("lookup %d in %s\n", address, bin_file->filename);
 #endif
 	
 	if (sym)
diff --git a/configure.ac b/configure.ac
index a981f1b..398ecd9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,6 +1,6 @@
 AC_PREREQ(2.54)
 
-AC_INIT([sysprof], [1.1.0.snap071806])
+AC_INIT([sysprof], [1.1.0])
 AC_CONFIG_SRCDIR(sysprof.glade)
 
 AM_INIT_AUTOMAKE(no-define)
@@ -105,7 +105,7 @@ Makefile
 
 AC_OUTPUT
 
-if false; then
+if true; then
    echo
    echo "%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%"
    echo "@"  
@@ -114,7 +114,7 @@ if false; then
    echo "%      There are currently no known bugs in the kernel"
    echo "@      module in this version, but there could easily be"
    echo "%      unknown ones. Please report any crashes or lockups"
-   echo "@      that you experience."
+   echo "@      that you experience to sandmann@daimi.au.dk."
    echo "%"
    echo "@"
    echo "%      If you need a stable version of sysprof, either"
diff --git a/elfparser.c b/elfparser.c
index a7d013a..7d9447b 100644
--- a/elfparser.c
+++ b/elfparser.c
@@ -126,8 +126,9 @@ find_section (ElfParser *parser,
     return NULL;
 }
 
-static ElfParser *
-parser_new_from_data (const guchar *data, gsize length)
+ElfParser *
+elf_parser_new_from_data (const guchar *data,
+			  gsize length)
 {
     ElfParser *parser;
     gboolean is_64, is_big_endian;
@@ -214,7 +215,7 @@ elf_parser_new (const char *filename,
     g_print ("data %p: for %s\n", data, filename);
 #endif
 
-    parser = parser_new_from_data (data, length);
+    parser = elf_parser_new_from_data (data, length);
 
     if (!parser)
     {
@@ -550,7 +551,6 @@ elf_parser_lookup_symbol (ElfParser *parser,
 			  gulong     address)
 {
     const ElfSym *result;
-    gsize size;
     
     if (!parser->symbols)
 	read_symbols (parser);
diff --git a/elfparser.h b/elfparser.h
index 714a7c1..891abcc 100644
--- a/elfparser.h
+++ b/elfparser.h
@@ -3,6 +3,8 @@
 typedef struct ElfSym ElfSym;
 typedef struct ElfParser ElfParser;
 
+ElfParser *   elf_parser_new_from_data  (const guchar *data,
+					 gsize	       length);
 ElfParser *   elf_parser_new            (const char  *filename,
 					 GError     **err);
 void          elf_parser_free           (ElfParser   *parser);
diff --git a/process.c b/process.c
index e757a77..95a84f7 100644
--- a/process.c
+++ b/process.c
@@ -25,6 +25,7 @@
 #include <sys/stat.h>
 #include <errno.h>
 #include <unistd.h>
+#include <string.h>
 
 #include "process.h"
 #include "binfile.h"
@@ -115,10 +116,24 @@ read_maps (int pid)
 	    map->start = start;
 	    map->end = end;
 	    
-	    map->offset = offset;
+	    if (strcmp (map->filename, "[vdso]") == 0)
+	    {
+		/* The kernel reports offset the same as the
+		 * mapping address, which doesn't make much sense
+		 * to me. So just zero it out here
+		 */
+#if 0
+		g_print ("fixing up\n");
+#endif
+		map->offset = 0;
+		map->inode = -1;
+	    }
+	    else
+	    {
+		map->offset = offset;
+		map->inode = inode;
+	    }
 
-	    map->inode = inode;
-	    
 	    map->bin_file = NULL;
 	    
 	    result = g_list_prepend (result, map);
@@ -136,6 +151,61 @@ read_maps (int pid)
     return result;
 }
 
+static void
+free_maps (GList *maps)
+{
+    GList *list;
+    
+    for (list = maps; list != NULL; list = list->next)
+    {
+	Map *map = list->data;
+	
+	if (map->filename)
+	    g_free (map->filename);
+	
+	if (map->bin_file)
+	    bin_file_free (map->bin_file);
+	
+	g_free (map);
+    }
+    
+    g_list_free (maps);
+}
+
+const guint8 *
+process_get_vdso_bytes (gsize *length)
+{
+    static gboolean has_data;
+    static const guint8 *bytes = NULL;
+    static gsize n_bytes = 0;
+
+    if (!has_data)
+    {
+	GList *maps = read_maps (getpid());
+	GList *list;
+
+	for (list = maps; list != NULL; list = list->next)
+	{
+	    Map *map = list->data;
+
+	    if (strcmp (map->filename, "[vdso]") == 0)
+	    {
+		bytes = (guint8 *)map->start;
+		n_bytes = map->end - map->start;
+	    }
+	}
+
+	has_data = TRUE;
+	free_maps (maps);
+    }
+
+    if (length)
+	*length = n_bytes;
+
+    g_print ("the vdso is %p\n", bytes);
+    
+    return bytes;
+}
 
 static Process *
 create_process (const char *cmdline, int pid)
@@ -183,28 +253,6 @@ process_locate_map (Process *process, gulong addr)
 }
 
 static void
-process_free_maps (Process *process)
-{
-    GList *list;
-    
-    for (list = process->maps; list != NULL; list = list->next)
-    {
-	Map *map = list->data;
-	
-	if (map->filename)
-	    g_free (map->filename);
-	
-	if (map->bin_file)
-	    bin_file_free (map->bin_file);
-	
-	g_free (map);
-    }
-    
-    g_list_free (process->maps);
-}
-
-
-static void
 free_process (gpointer key, gpointer value, gpointer data)
 {
     char *cmdline = key;
@@ -218,7 +266,7 @@ free_process (gpointer key, gpointer value, gpointer data)
 #if 0
     process->cmdline = "You are using free()'d memory";
 #endif
-    process_free_maps (process);
+    free_maps (process->maps);
     g_list_free (process->bad_pages);
     g_free (cmdline);
     
@@ -265,7 +313,7 @@ process_ensure_map (Process *process, int pid, gulong addr)
     
     /* a map containing addr was not found */
     if (process->maps)
-	process_free_maps (process);
+	free_maps (process->maps);
     
     process->maps = read_maps (pid);
     
@@ -600,7 +648,7 @@ process_lookup_symbol (Process *process, gulong address)
 #endif
 
 #if 0
-    if (strcmp (map->filename, "/home/ssp/sysprof/sysprof") == 0)
+    if (strcmp (map->filename, "[vdso]") == 0)
     {
 	g_print ("address after: %lx\n", address);
     }
@@ -618,7 +666,8 @@ process_lookup_symbol (Process *process, gulong address)
 	 * file has changed since the process started. Just return
 	 * the undefined symbol in that case.
 	 */
-	
+	g_print ("warning: %s has wrong inode (%d, should be %d)\n",
+		 map->filename, map->inode, bin_file_get_inode (map->bin_file));
 	address = 0x0;
     }
 
diff --git a/process.h b/process.h
index 3edcdd7..ceb34af 100644
--- a/process.h
+++ b/process.h
@@ -55,5 +55,6 @@ const char *  process_lookup_symbol		  (Process    *process,
 						   gulong      address);
 const char *  process_get_cmdline                 (Process    *process);
 void	      process_flush_caches                (void);
+const guint8 *process_get_vdso_bytes		  (gsize      *length);
 
 #endif