From ebb772876861246dfe3fd05c3078f6884a38fa27 Mon Sep 17 00:00:00 2001 From: Soeren Sandmann Date: Sat, 4 Mar 2006 03:28:26 +0000 Subject: [PATCH] Check that the inodes match. Fri Mar 3 22:28:03 2006 Soeren Sandmann * 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 | 9 +++++++++ NEWS | 5 ++++- TODO | 16 ++++++++++++---- binfile.c | 36 ++++++++++++++++++++++++++++++++++++ binfile.h | 2 ++ process.c | 26 ++++++++++++++++++++------ sysprof.c | 4 ++-- 7 files changed, 85 insertions(+), 13 deletions(-) diff --git a/ChangeLog b/ChangeLog index 6af1a4d9..0ab99ecb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,12 @@ +Fri Mar 3 22:28:03 2006 Soeren Sandmann + + * 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 * treeviewutils.c (tree_view_foreach_visible): Handle NULL models. diff --git a/NEWS b/NEWS index a97e151c..83ee20cb 100644 --- a/NEWS +++ b/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 diff --git a/TODO b/TODO index 7f5210f5..b5b91ac6 100644 --- a/TODO +++ b/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). diff --git a/binfile.c b/binfile.c index 4c7c2fb9..5e3ecf27 100644 --- a/binfile.c +++ b/binfile.c @@ -32,6 +32,8 @@ #include #include #include +#include +#include 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; diff --git a/binfile.h b/binfile.h index f26b15bb..d3f643fb 100644 --- a/binfile.h +++ b/binfile.h @@ -25,6 +25,7 @@ #define BIN_FILE_H #include +#include 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 diff --git a/process.c b/process.c index 80e1bc82..c99f8728 100644 --- a/process.c +++ b/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", - &start, &end, &offset, file); - if (count == 4) + buffer, "%lx-%lx %*15s %lx %*x:%*x %lu %255s", + &start, &end, &offset, &inode, file); + 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 diff --git a/sysprof.c b/sysprof.c index a6e3bda9..7dcb9b37 100644 --- a/sysprof.c +++ b/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); }