This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] Replace reread_symbols by load+free calls
On Thu, 25 Jun 2009 21:58:04 +0200, Pedro Alves wrote:
> On Thursday 25 June 2009 20:21:20, Jan Kratochvil wrote:
> > + Â Â Âfor (so = master_so_list (); so != NULL; so = so->next)
> > +ÂÂÂÂÂÂÂif (so->objfile == objfile)
> > +ÂÂÂÂÂÂÂ Âbreak;
> > + Â Â Âif (so != NULL)
> > +ÂÂÂÂÂÂÂcontinue;
>
> Would this work instead?
>
> if ((objfile->flags & (OBJF_SHARED | OBJF_USERLOADED) == OBJF_SHARED))
> continue;
It got me some time but yes, in practice it should work, thanks.
> On Thursday 25 June 2009 20:21:20, Jan Kratochvil wrote:
> > + /* 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. */
>
> Can you elaborate on this FIXME note? exec_bfd may or not have symbols in it.
> It may or not be the same bfd as symfile_objfile. Note that there are
> "exec-file" and "symbol-file" separate commands. In fact, I believe,
> historicaly, the "file" command that does both things at once was only
> added later. Do you plan on somehow eliminating exec_bfd?
Currently for "file /bin/sleep" the binary itself has three separate open bfds:
(gdb) p object_files->obfd
$1 = (bfd *) 0x1c71770
(gdb) p object_files->obfd->filename
$2 = 0x1c82750 "/usr/lib/debug/bin/sleep.debug"
(gdb) p object_files->next->obfd
$3 = (bfd *) 0x1c708d0
(gdb) p object_files->next->obfd->filename
$4 = 0x1c6f6d0 "/bin/sleep"
(gdb) p exec_bfd
$5 = (bfd *) 0x1c6f740
(gdb) p exec_bfd->filename
$6 = 0x1a29600 "/bin/sleep"
(gdb) p symfile_objfile == object_files->next
$7 = 1
I would like to have only two separate open bfds in this case.
Currently object_files are iterated as symbol sources. Such unification would
mean that in this case FILEBIN would _also_ get searched for symbols. Is it
a problem?
(gdb) exec-file FILEBIN
(gdb) symbol-file FILESYM
It would provide a common list + structure (objfile) for handling all open
inferior files.
> I can't quite parse what this comment is really saying. Who calls
> breakpoint_re_set? Who is complying?
OK, updated:
/* SYMFILE_DEFER_BP_RESET being used below requires one to call
breakpoint_re_set before returning the control to user.
clear_symtab_users already calls breakpoint_re_set. BACK_TO is
set to call clear_symtab_users. clear_symtab_users should be also
called whenever any symbols go away by free_objfile below. */
Put there in also back an equivalent for former:
- /* We need to do this whenever any symbols go away. */
- make_cleanup (clear_symtab_users_cleanup, 0 /*ignore*/);
Regression tested on {x86_64,i686}-fedora-linux-gnu.
Thanks,
Jan
2009-06-26 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..c02b856 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,59 @@ 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;
+ struct cleanup *back_to = NULL;
/* With the addition of shared libraries, this should be modified,
the load time should be saved in the partial symbol tables, since
@@ -2249,282 +2290,96 @@ 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. */
+ 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 prepare section_addr_info ourselves.
+
+ The condition here is in this case equivalent to the presence in
+ so_list_head. OBJF_USERLOADED libraries need to be still handled here
+ as they are not listed in so_list_head. Fortunately the only such
+ libraries having (OBJF_SHARED | OBJF_USERLOADED) do not need
+ section_addr_info. */
+
+ if ((objfile->flags & (OBJF_SHARED | OBJF_USERLOADED)) == OBJF_SHARED)
+ continue;
+
+ if (!objfile_has_changed (objfile))
+ continue;
+
+ /* 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. */
- /* We're done reading the symbol file; finish off complaints. */
- clear_complaints (&symfile_complaints, 0, 1);
+ if (exec_bfd != NULL && strcmp (bfd_get_filename (objfile->obfd),
+ bfd_get_filename (exec_bfd)) == 0)
+ {
+ /* Reload EXEC_BFD without asking anything. */
- /* Getting new symbols may change our opinion about what is
- frameless. */
+ exec_file_attach (bfd_get_filename (objfile->obfd), 0);
+ }
- reinit_frame_cache ();
+ if (!reread_one)
+ {
+ /* SYMFILE_DEFER_BP_RESET being used below requires one to call
+ breakpoint_re_set before returning the control to user.
+ clear_symtab_users already calls breakpoint_re_set. BACK_TO is
+ set to call clear_symtab_users. clear_symtab_users should be also
+ called whenever any symbols go away by free_objfile below. */
- /* Discard cleanups as symbol reading was successful. */
- discard_cleanups (old_cleanups);
+ back_to = make_cleanup (clear_symtab_users_cleanup, NULL /*ignore*/);
- /* 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);
- }
+ reread_one = 1;
}
- }
- if (reread_one)
- {
- 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 ();
- }
-
-}
+ 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;
+ }
-/* 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;
+ symbol_file_add (bfd_get_filename (objfile->obfd), add_flags, NULL,
+ objfile->flags);
- /* Does the updated objfile's debug info live in a
- separate file? */
- debug_file = find_separate_debug_file (objfile);
+ /* 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;
- 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);
+ free_objfile (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)
+ if (reread_one)
{
- /* 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);
-}
+ /* Call clear_symtab_users before the notification below. */
+ do_cleanups (back_to);
+ /* 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. */
+ solib_add (NULL, 0, NULL, auto_solib_add);
+}
typedef struct
{
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 *);