This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[patch] Replace reread_symbols by load+free calls
On Tue, 23 Jun 2009 22:00:25 +0200, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
>
> Jan> IMO the whole `struct objfile' should replaced by a new one
> Jan> (instead of its in-place patching). Which will probably have
> Jan> other dependencies...
>
> I'm interested to hear your idea and reasoning.
Sometimes there are differentiations between complete objects destructors
(objfile_free_data) and in-place object deletions (clear_objfile_data);
OK did not briefly found more such cases now. Expecting it was there for some
performance improvements which not worth it nowadays.
reread_symbols was also trying to be in-place variant but thus it had to
redefine functions loading+freeing objfiles.
Expecting this patch may affect the behavior a bit but it should only improve.
The exec_bfd handling is ugly but it is left as is, for some different patch.
No regressions on {x86_64,i686}-fedora-linux-gnu.
Thanks,
Jan
2009-06-25 Jan Kratochvil <jan.kratochvil@redhat.com>
Simplify reread_symbols.
* objfiles.h (struct objfile <mtime>): New FIXME comment.
* solib.c (update_solib_list): Reload the library if it has changed.
* symfile.c: Include solist.h.
(reread_symbols): Replace the in-place rereading by calls to
symbol_file_add and free_objfile. Call now also solib_add. Move the
variables new_modtime, new_statbuf and res with the OBJFILE change
checking code to ...
(objfile_has_changed): ... a new function.
(reread_separate_symbols): Remove the function.
* symfile.h (objfile_has_changed): New prototype.
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index 60d3143..e03861f 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -236,7 +236,7 @@ struct objfile
struct gdbarch *gdbarch;
/* The modification timestamp of the object file, as of the last time
- we read its symbols. */
+ we read its symbols. FIXME: Replace it by a bfd_get_mtime call. */
long mtime;
diff --git a/gdb/solib.c b/gdb/solib.c
index d194ac7..fae0f4a 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -615,7 +615,7 @@ update_solib_list (int from_tty, struct target_ops *target)
/* If the shared object appears on the inferior's list too, then
it's still loaded, so we don't need to do anything. Delete
it from the inferior's list, and leave it on GDB's list. */
- if (i)
+ if (i && (gdb->objfile == NULL || !objfile_has_changed (gdb->objfile)))
{
*i_link = i->next;
free_so (i);
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 25f3f26..f503756 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -55,6 +55,7 @@
#include "elf-bfd.h"
#include "solib.h"
#include "remote.h"
+#include "solist.h"
#include <sys/types.h>
#include <fcntl.h>
@@ -96,8 +97,6 @@ static void symbol_file_add_main_1 (char *args, int from_tty, int flags);
static void add_symbol_file_command (char *, int);
-static void reread_separate_symbols (struct objfile *objfile);
-
static void cashier_psymtab (struct partial_symtab *);
bfd *symfile_bfd_open (char *);
@@ -2231,17 +2230,58 @@ add_symbol_file_command (char *args, int from_tty)
reinit_frame_cache ();
do_cleanups (my_cleanups);
}
-
+
+/* Return non-zero if OBJFILE (or its SEPARATE_DEBUG_OBJFILE) has changed on
+ the disk since its load. You should not call this function for the separate
+ debug info file itself. */
+
+int
+objfile_has_changed (struct objfile *objfile)
+{
+ long new_modtime;
+ struct stat new_statbuf;
+ int res;
+
+ if (objfile->obfd == NULL)
+ return 0;
+
+#ifdef DEPRECATED_IBM6000_TARGET
+ /* If this object is from a shared library, then you should
+ stat on the library name, not member name. */
+
+ if (objfile->obfd->my_archive)
+ res = stat (objfile->obfd->my_archive->filename, &new_statbuf);
+ else
+#endif
+ res = stat (objfile->name, &new_statbuf);
+ if (res != 0)
+ {
+ /* FIXME, should use print_sys_errmsg but it's not filtered. */
+ printf_unfiltered (_("`%s' has disappeared; keeping its symbols.\n"),
+ objfile->name);
+ return 0;
+ }
+
+ new_modtime = new_statbuf.st_mtime;
+ if (new_modtime != objfile->mtime)
+ {
+ printf_unfiltered (_("`%s' has changed; re-reading symbols.\n"),
+ objfile->name);
+ return 1;
+ }
+
+ if (objfile->separate_debug_objfile)
+ return objfile_has_changed (objfile->separate_debug_objfile);
+
+ return 0;
+}
/* Re-read symbols if a symbol-file has changed. */
void
reread_symbols (void)
{
- struct objfile *objfile;
- long new_modtime;
+ struct objfile *objfile, *objfile_next;
int reread_one = 0;
- struct stat new_statbuf;
- int res;
/* With the addition of shared libraries, this should be modified,
the load time should be saved in the partial symbol tables, since
@@ -2249,283 +2289,85 @@ reread_symbols (void)
This routine should then walk down each partial symbol table
and see if the symbol table that it originates from has been changed */
- for (objfile = object_files; objfile; objfile = objfile->next)
+ for (objfile = object_files; objfile != NULL; objfile = objfile_next)
{
- if (objfile->obfd)
- {
-#ifdef DEPRECATED_IBM6000_TARGET
- /* If this object is from a shared library, then you should
- stat on the library name, not member name. */
+ struct so_list *so;
+ int add_flags;
- if (objfile->obfd->my_archive)
- res = stat (objfile->obfd->my_archive->filename, &new_statbuf);
- else
-#endif
- res = stat (objfile->name, &new_statbuf);
- if (res != 0)
- {
- /* FIXME, should use print_sys_errmsg but it's not filtered. */
- printf_unfiltered (_("`%s' has disappeared; keeping its symbols.\n"),
- objfile->name);
- continue;
- }
- new_modtime = new_statbuf.st_mtime;
- if (new_modtime != objfile->mtime)
- {
- struct cleanup *old_cleanups;
- struct section_offsets *offsets;
- int num_offsets;
- char *obfd_filename;
-
- printf_unfiltered (_("`%s' has changed; re-reading symbols.\n"),
- objfile->name);
-
- /* There are various functions like symbol_file_add,
- symfile_bfd_open, syms_from_objfile, etc., which might
- appear to do what we want. But they have various other
- effects which we *don't* want. So we just do stuff
- ourselves. We don't worry about mapped files (for one thing,
- any mapped file will be out of date). */
-
- /* If we get an error, blow away this objfile (not sure if
- that is the correct response for things like shared
- libraries). */
- old_cleanups = make_cleanup_free_objfile (objfile);
- /* We need to do this whenever any symbols go away. */
- make_cleanup (clear_symtab_users_cleanup, 0 /*ignore*/);
-
- if (exec_bfd != NULL && strcmp (bfd_get_filename (objfile->obfd),
- bfd_get_filename (exec_bfd)) == 0)
- {
- /* Reload EXEC_BFD without asking anything. */
+ objfile_next = objfile->next;
- exec_file_attach (bfd_get_filename (objfile->obfd), 0);
- }
+ if (objfile->obfd == NULL)
+ continue;
- /* Clean up any state BFD has sitting around. We don't need
- to close the descriptor but BFD lacks a way of closing the
- BFD without closing the descriptor. */
- obfd_filename = bfd_get_filename (objfile->obfd);
- if (!bfd_close (objfile->obfd))
- error (_("Can't close BFD for %s: %s"), objfile->name,
- bfd_errmsg (bfd_get_error ()));
- if (remote_filename_p (obfd_filename))
- objfile->obfd = remote_bfd_open (obfd_filename, gnutarget);
- else
- objfile->obfd = bfd_openr (obfd_filename, gnutarget);
- if (objfile->obfd == NULL)
- error (_("Can't open %s to read symbols."), objfile->name);
- /* bfd_openr sets cacheable to true, which is what we want. */
- if (!bfd_check_format (objfile->obfd, bfd_object))
- error (_("Can't read symbols from %s: %s."), objfile->name,
- bfd_errmsg (bfd_get_error ()));
-
- /* Save the offsets, we will nuke them with the rest of the
- objfile_obstack. */
- num_offsets = objfile->num_sections;
- offsets = ((struct section_offsets *)
- alloca (SIZEOF_N_SECTION_OFFSETS (num_offsets)));
- memcpy (offsets, objfile->section_offsets,
- SIZEOF_N_SECTION_OFFSETS (num_offsets));
-
- /* Remove any references to this objfile in the global
- value lists. */
- preserve_values (objfile);
-
- /* Nuke all the state that we will re-read. Much of the following
- code which sets things to NULL really is necessary to tell
- other parts of GDB that there is nothing currently there.
-
- Try to keep the freeing order compatible with free_objfile. */
-
- if (objfile->sf != NULL)
- {
- (*objfile->sf->sym_finish) (objfile);
- }
+ /* Separate debug info files are always checked/loaded/unloaded by the
+ called functions together with their main binary file. */
- clear_objfile_data (objfile);
-
- /* FIXME: Do we have to free a whole linked list, or is this
- enough? */
- if (objfile->global_psymbols.list)
- xfree (objfile->global_psymbols.list);
- memset (&objfile->global_psymbols, 0,
- sizeof (objfile->global_psymbols));
- if (objfile->static_psymbols.list)
- xfree (objfile->static_psymbols.list);
- memset (&objfile->static_psymbols, 0,
- sizeof (objfile->static_psymbols));
-
- /* Free the obstacks for non-reusable objfiles */
- bcache_xfree (objfile->psymbol_cache);
- objfile->psymbol_cache = bcache_xmalloc ();
- bcache_xfree (objfile->macro_cache);
- objfile->macro_cache = bcache_xmalloc ();
- if (objfile->demangled_names_hash != NULL)
- {
- htab_delete (objfile->demangled_names_hash);
- objfile->demangled_names_hash = NULL;
- }
- obstack_free (&objfile->objfile_obstack, 0);
- objfile->sections = NULL;
- objfile->symtabs = NULL;
- objfile->psymtabs = NULL;
- objfile->psymtabs_addrmap = NULL;
- objfile->free_psymtabs = NULL;
- objfile->cp_namespace_symtab = NULL;
- objfile->msymbols = NULL;
- objfile->deprecated_sym_private = NULL;
- objfile->minimal_symbol_count = 0;
- memset (&objfile->msymbol_hash, 0,
- sizeof (objfile->msymbol_hash));
- memset (&objfile->msymbol_demangled_hash, 0,
- sizeof (objfile->msymbol_demangled_hash));
-
- objfile->psymbol_cache = bcache_xmalloc ();
- objfile->macro_cache = bcache_xmalloc ();
- /* obstack_init also initializes the obstack so it is
- empty. We could use obstack_specify_allocation but
- gdb_obstack.h specifies the alloc/dealloc
- functions. */
- obstack_init (&objfile->objfile_obstack);
- if (build_objfile_section_table (objfile))
- {
- error (_("Can't find the file sections in `%s': %s"),
- objfile->name, bfd_errmsg (bfd_get_error ()));
- }
- terminate_minimal_symbol_table (objfile);
-
- /* We use the same section offsets as from last time. I'm not
- sure whether that is always correct for shared libraries. */
- objfile->section_offsets = (struct section_offsets *)
- obstack_alloc (&objfile->objfile_obstack,
- SIZEOF_N_SECTION_OFFSETS (num_offsets));
- memcpy (objfile->section_offsets, offsets,
- SIZEOF_N_SECTION_OFFSETS (num_offsets));
- objfile->num_sections = num_offsets;
-
- /* What the hell is sym_new_init for, anyway? The concept of
- distinguishing between the main file and additional files
- in this way seems rather dubious. */
- if (objfile == symfile_objfile)
- {
- (*objfile->sf->sym_new_init) (objfile);
- }
+ if (objfile->separate_debug_objfile_backlink != NULL)
+ continue;
- (*objfile->sf->sym_init) (objfile);
- clear_complaints (&symfile_complaints, 1, 1);
- /* The "mainline" parameter is a hideous hack; I think leaving it
- zero is OK since dbxread.c also does what it needs to do if
- objfile->global_psymbols.size is 0. */
- (*objfile->sf->sym_read) (objfile, 0);
- if (!have_partial_symbols () && !have_full_symbols ())
- {
- wrap_here ("");
- printf_unfiltered (_("(no debugging symbols found)\n"));
- wrap_here ("");
- }
+ /* OBJFILE listed as a shared library would get a stale pointer in
+ SO_LIST. The OBJFILE update will be taken care of by solib_add below.
+ Thus we also do not need to call build_section_table ourselves. */
- /* We're done reading the symbol file; finish off complaints. */
- clear_complaints (&symfile_complaints, 0, 1);
+ for (so = master_so_list (); so != NULL; so = so->next)
+ if (so->objfile == objfile)
+ break;
+ if (so != NULL)
+ continue;
- /* Getting new symbols may change our opinion about what is
- frameless. */
+ if (!objfile_has_changed (objfile))
+ continue;
- reinit_frame_cache ();
+ /* FIXME: EXEC_BFD should be always referenced only through
+ SYMFILE_OBJFILE. Currently exec_bfd exists on its own and it does not
+ have to be listed among OBJECT_FILES. exec_bfd_mtime should either be
+ used or rather dropped. */
- /* Discard cleanups as symbol reading was successful. */
- discard_cleanups (old_cleanups);
+ if (exec_bfd != NULL && strcmp (bfd_get_filename (objfile->obfd),
+ bfd_get_filename (exec_bfd)) == 0)
+ {
+ /* Reload EXEC_BFD without asking anything. */
- /* If the mtime has changed between the time we set new_modtime
- and now, we *want* this to be out of date, so don't call stat
- again now. */
- objfile->mtime = new_modtime;
- reread_one = 1;
- reread_separate_symbols (objfile);
- init_entry_point_info (objfile);
- }
+ exec_file_attach (bfd_get_filename (objfile->obfd), 0);
+ }
+
+ add_flags = SYMFILE_DEFER_BP_RESET;
+ if (objfile == symfile_objfile)
+ {
+ add_flags |= SYMFILE_MAINLINE;
+
+ /* SYMFILE_OBJFILE could get freed as it has no reference counter. */
+ symfile_objfile = NULL;
}
+
+ symbol_file_add (bfd_get_filename (objfile->obfd), add_flags, NULL,
+ objfile->flags);
+
+ /* free_objfile will free also SEPARATE_DEBUG_OBJFILE, if it exists. */
+ if (objfile_next && objfile_next == objfile->separate_debug_objfile)
+ objfile_next = objfile_next->next;
+
+ free_objfile (objfile);
+
+ reread_one = 1;
}
if (reread_one)
{
+ /* breakpoint_re_set gets called to comply with SYMFILE_DEFER_BP_RESET
+ above. */
clear_symtab_users ();
+
/* At least one objfile has changed, so we can consider that
the executable we're debugging has changed too. */
observer_notify_executable_changed ();
}
-
-}
+ /* Perform functionality of this function also for the shared libraries. */
-/* Handle separate debug info for OBJFILE, which has just been
- re-read:
- - If we had separate debug info before, but now we don't, get rid
- of the separated objfile.
- - If we didn't have separated debug info before, but now we do,
- read in the new separated debug info file.
- - If the debug link points to a different file, toss the old one
- and read the new one.
- This function does *not* handle the case where objfile is still
- using the same separate debug info file, but that file's timestamp
- has changed. That case should be handled by the loop in
- reread_symbols already. */
-static void
-reread_separate_symbols (struct objfile *objfile)
-{
- char *debug_file;
- unsigned long crc32;
-
- /* Does the updated objfile's debug info live in a
- separate file? */
- debug_file = find_separate_debug_file (objfile);
-
- if (objfile->separate_debug_objfile)
- {
- /* There are two cases where we need to get rid of
- the old separated debug info objfile:
- - if the new primary objfile doesn't have
- separated debug info, or
- - if the new primary objfile has separate debug
- info, but it's under a different filename.
-
- If the old and new objfiles both have separate
- debug info, under the same filename, then we're
- okay --- if the separated file's contents have
- changed, we will have caught that when we
- visited it in this function's outermost
- loop. */
- if (! debug_file
- || strcmp (debug_file, objfile->separate_debug_objfile->name) != 0)
- free_objfile (objfile->separate_debug_objfile);
- }
-
- /* If the new objfile has separate debug info, and we
- haven't loaded it already, do so now. */
- if (debug_file
- && ! objfile->separate_debug_objfile)
- {
- /* Use the same section offset table as objfile itself.
- Preserve the flags from objfile that make sense. */
- objfile->separate_debug_objfile
- = (symbol_file_add_with_addrs_or_offsets
- (symfile_bfd_open (debug_file),
- info_verbose ? SYMFILE_VERBOSE : 0,
- 0, /* No addr table. */
- objfile->section_offsets, objfile->num_sections,
- objfile->flags & (OBJF_REORDERED | OBJF_SHARED | OBJF_READNOW
- | OBJF_USERLOADED)));
- objfile->separate_debug_objfile->separate_debug_objfile_backlink
- = objfile;
- }
- if (debug_file)
- xfree (debug_file);
+ solib_add (NULL, 0, NULL, auto_solib_add);
}
-
-
-
-
typedef struct
{
char *ext;
diff --git a/gdb/symfile.h b/gdb/symfile.h
index bba242c..86e51f6 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -382,6 +382,8 @@ extern int symfile_map_offsets_to_segments (bfd *,
struct symfile_segment_data *get_symfile_segment_data (bfd *abfd);
void free_symfile_segment_data (struct symfile_segment_data *data);
+extern int objfile_has_changed (struct objfile *objfile);
+
/* From dwarf2read.c */
extern int dwarf2_has_info (struct objfile *);