Check that the inodes match.

Fri Mar  3 22:28:03 2006  Soeren Sandmann  <sandmann@redhat.com>

        * process.c (process_lookup_symbol): Check that the inodes match.

        * binfile.c (read_symbols): Read the inode of the file

        * binfile.c (read_symbols): Close the bfd if the symbol table
        could not be read.

ChangeLog

@@ -1,3 +1,12 @@
+Fri Mar  3 22:28:03 2006  Soeren Sandmann  <sandmann@redhat.com>
+
+	* process.c (process_lookup_symbol): Check that the inodes match.
+
+	* binfile.c (read_symbols): Read the inode of the file
+
+	* binfile.c (read_symbols): Close the bfd if the symbol table
+	could not be read.
+
 Thu Mar  2 22:54:37 2006  Soeren Sandmann  <sandmann@redhat.com>
 
 	* treeviewutils.c (tree_view_foreach_visible): Handle NULL models.

NEWS

@@ -1,3 +1,6 @@
 - New 'everything' object
-- New commandline version 
+- New commandline version [Lorenzo Colitti]
 - Assign time spent in kernel to the user process responsible
+- New screenshot window
+- Device based on udev [Kristian Hoegsberg]
+- Performance improvements

TODO

@@ -31,6 +31,7 @@ Before 1.2:
 	- 'No symbols in binary file'
 	- 'Address has no corrresponding symbol in file'
 	- etc.
+  done: HEAD will not load files with the wrong inode now.
 
 * Consider deleting cmdline hack in process.c and replace with something at 
   the symbol resolution level. Will require more memory though.
@@ -142,7 +143,10 @@ Before 1.2:
   we'd lose the ability to do non-racy file naming. We could pass a list
   of the process mappings with each stack though. Doing this would also solve
   the problem of not being able to get maps of processes running as root.
-	Might be too expensive though.
+	Might be too expensive though. User stacks seem to be on the order
+  of 100K usually, which for 200 times a second means a bandwidth of
+  20MB/s, which is probably too much. One question is how much of it
+  usually changes. 
 
 * If interrupt happens in kernel mode, send both
   kernel stack and user space stack, have userspace stitch them
@@ -171,6 +175,9 @@ Before 1.2:
 
   Maybe get_user_pages() is the way forward at least for some stuff.
 
+  note btw. that the kernel pages are only one or two pages, so we 
+  could easily just dump them to userspace.
+
 - See if there is a way to make it distcheck
 
 - grep "FIXME - not10"
@@ -196,6 +203,7 @@ Before 1.2:
 		  it when you use the wm button or the menu item? It actually
 		  seems that it only forgets the position when you click the
 		  button with the mouse. But not if you use the keyboard ...
+			This is a gtk+ bug.
 
 - Find out how gdb does backtraces; they may have a better way. Also
   find out what dwarf2 is and how to use it. Look into libunwind.
@@ -217,9 +225,6 @@ http://www.linuxbase.org/spec/booksets/LSB-Embedded/LSB-Embedded/ehframe.html
 		so (can we know the size in advance?))
 	- instead of what we do now: set the busy cursor unconditionally
 	
-- Charge 'self' properly to processes that don't get any stack trace at all 
-	(probably we get that for free with stackstash reorganisation)
-
 - Consider adding ability to show more than one function at a time. Algorithm:
 	Find all relevant nodes;
 	For each relevant node
@@ -466,6 +471,9 @@ Later:
 
 DONE:
 
+- Charge 'self' properly to processes that don't get any stack trace at all 
+	(probably we get that for free with stackstash reorganisation)
+
 - CVS head now has two radio buttons in the right pane, and
   caller pane is gone. (This turned out to be a bad idea, because it
   is often useful to click on ancestors to move up the tree).

binfile.c

@@ -32,6 +32,8 @@
 #include <unistd.h>
 #include <fcntl.h>
 #include <stdio.h>
+#include <sys/types.h>
+#include <sys/stat.h>
 
 static void bfd_nonfatal (const char *string);
 static void bfd_fatal (const char *string);
@@ -44,6 +46,7 @@ struct BinFile
     Symbol     *symbols;
     Symbol	undefined;
     int         ref_count;
+    ino_t	inode;
 };
 
 static bfd *
@@ -295,6 +298,21 @@ compare_address (const void *a, const void *b)
 	return 1;
 }
 
+static gboolean
+read_inode (const char *filename,
+	    ino_t      *inode)
+{
+    struct stat statbuf;
+
+    if (stat (filename, &statbuf) == 0)
+    {
+	*inode = statbuf.st_ino;
+	return TRUE;
+    }
+
+    return FALSE;
+}
+
 static void
 read_symbols (BinFile *bf)
 {
@@ -315,6 +333,12 @@ read_symbols (BinFile *bf)
     if (!bfd)
 	return;
 
+    if (!read_inode (bf->filename, &bf->inode))
+    {
+	bfd_close (bfd);
+	return;
+    }
+
     separate_debug_file = find_separate_debug_file (bfd);
     if (separate_debug_file)
     {
@@ -330,7 +354,10 @@ read_symbols (BinFile *bf)
     bfd_symbols = slurp_symtab (bfd, &n_symbols);
     
     if (!bfd_symbols)
+    {
+	bfd_close (bfd);
 	return;
+    }
     
     load_address = 0xffffffff;
     for (sec = bfd->sections; sec != NULL; sec = sec->next)
@@ -416,6 +443,12 @@ bin_file_new (const char *filename)
     return bf;
 }
 
+ino_t
+bin_file_get_inode     (BinFile    *bin_file)
+{
+    return bin_file->inode;
+}
+
 void
 bin_file_free (BinFile *bf)
 {
@@ -455,6 +488,9 @@ bin_file_lookup_symbol (BinFile    *bf,
     
     if (!bf->symbols || (bf->n_symbols == 0))
 	return &(bf->undefined);
+
+    if (address == 0x0)
+	return &(bf->undefined);
     
     data = bf->symbols;
 

binfile.h

@@ -25,6 +25,7 @@
 #define BIN_FILE_H
 
 #include <glib.h>
+#include <sys/types.h>
 
 typedef struct BinFile BinFile;
 typedef struct Symbol Symbol;
@@ -35,6 +36,7 @@ BinFile *     bin_file_new           (const char *filename);
 void          bin_file_free          (BinFile    *bin_file);
 const Symbol *bin_file_lookup_symbol (BinFile    *bin_file,
 				      gulong      address);
+ino_t	      bin_file_get_inode     (BinFile    *bin_file);
 
 /* Symbol */
 struct Symbol

process.c

@@ -41,6 +41,7 @@ struct Map
     gulong	start;
     gulong	end;
     gulong      offset;
+    gulong	inode;
 #if 0
     gboolean	do_offset;
 #endif
@@ -95,15 +96,16 @@ read_maps (int pid)
 	gulong start;
 	gulong end;
 	gulong offset;
+	gulong inode;
 	
 #if 0
 	g_print ("buffer: %s\n", buffer);
 #endif
 	
 	count = sscanf (
-	    buffer, "%lx-%lx %*15s %lx %*x:%*x %*u %255s", 
+	    buffer, "%lx-%lx %*15s %lx %*x:%*x %lu %255s", 
-	    &start, &end, &offset, file);
+	    &start, &end, &offset, &inode, file);
-	if (count == 4)
+	if (count == 5)
 	{
 	    Map *map;
 	    
@@ -114,6 +116,8 @@ read_maps (int pid)
 	    map->end = end;
 	    
 	    map->offset = offset;
+
+	    map->inode = inode;
 	    
 	    map->bin_file = NULL;
 	    
@@ -567,16 +571,26 @@ process_lookup_symbol (Process *process, gulong address)
 	
 	return &process->undefined;
     }
-    
+
     address -= map->start;
     address += map->offset;
     
     if (!map->bin_file)
 	map->bin_file = bin_file_new (map->filename);
-    
+
 /*     g_print ("%s: start: %p, load: %p\n", */
 /* 	     map->filename, map->start, bin_file_get_load_address (map->bin_file)); */
-    
+
+    if (map->inode != bin_file_get_inode (map->bin_file))
+    {
+	/* If the inodes don't match, it's probably because the
+	 * file has changed since the process started. Just return
+	 * the undefined symbol in that case.
+	 */
+	
+	address = 0x0;
+    }
+
     result = bin_file_lookup_symbol (map->bin_file, address);
     
 #if 0

sysprof.c

@@ -1,6 +1,6 @@
 /* Sysprof -- Sampling, systemwide CPU profiler
  * Copyright 2004, Red Hat, Inc.
- * Copyright 2004, 2005, Soeren Sandmann
+ * Copyright 2004, 2005, 2006, Soeren Sandmann
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -675,7 +675,7 @@ on_about_activated (GtkWidget *widget, gpointer data)
     gtk_show_about_dialog (GTK_WINDOW (app->main_window),
 			   "logo", app->icon,
 			   "name", APPLICATION_NAME,
-			   "copyright", "Copyright 2004-2005, S"OSLASH"ren Sandmann",
+			   "copyright", "Copyright 2004-2006, S"OSLASH"ren Sandmann",
 			   "version", PACKAGE_VERSION,
 			   NULL);
 }