Consolidate calls to bfd_set_cacheable

I noticed that some spots in gdb call bfd_set_cacheable after opening
a BFD.

The BFD file cache is a bit odd.  BFDs that are opened locally are
unconditionally registered with the cache, and their underlying file
descriptor will always be closed when bfd_cache_close_all is called.
However, only "cacheable" BFDs will be eligible for reopening when
needed -- and by default BFD decides that if a file descriptor is
passed in, then it should not be cacheable.  If a non-cacheable BFD's
file descriptor is closed, there is no offical way to reopen it.

gdb needs to call bfd_cache_close_all, because some systems cannot
start an executable when it has an open file descriptor referencing
it.

However, gdb also will sometimes passes an open file descriptor to the
various BFD open functions.  And, due to lazy DWARF reading, gdb may
also need to reopen these BFDs.

Rather than having all the callers figure out when exactly to set the
cacheable flag, I think it makes sense to consolidate this logic into
the gdb_bfd.c wrapper functions.  It is ok to do this because gdb
always passes a filename to these open functions, so reopening should
work ok.

Regression tested on x86-64 Fedora 38.

Reviewed-by: John Baldwin <jhb@FreeBSD.org>
This commit is contained in:
Tom Tromey 2023-08-04 05:58:35 -06:00
parent c8f6fc9200
commit 0ecf440316
5 changed files with 5 additions and 9 deletions

View file

@ -9184,7 +9184,6 @@ try_open_dwop_file (dwarf2_per_objfile *per_objfile,
gnutarget, desc)); gnutarget, desc));
if (sym_bfd == NULL) if (sym_bfd == NULL)
return NULL; return NULL;
bfd_set_cacheable (sym_bfd.get (), 1);
if (!bfd_check_format (sym_bfd.get (), bfd_object)) if (!bfd_check_format (sym_bfd.get (), bfd_object))
return NULL; return NULL;

View file

@ -564,6 +564,8 @@ gdb_bfd_open (const char *name, const char *target, int fd,
if (abfd == NULL) if (abfd == NULL)
return NULL; return NULL;
bfd_set_cacheable (abfd, 1);
bfd_cache_debug_printf ("Creating new bfd %s for %s", bfd_cache_debug_printf ("Creating new bfd %s for %s",
host_address_to_string (abfd), host_address_to_string (abfd),
bfd_get_filename (abfd)); bfd_get_filename (abfd));
@ -877,6 +879,9 @@ gdb_bfd_fopen (const char *filename, const char *target, const char *mode,
{ {
bfd *result = bfd_fopen (filename, target, mode, fd); bfd *result = bfd_fopen (filename, target, mode, fd);
if (result != nullptr)
bfd_set_cacheable (result, 1);
return gdb_bfd_ref_ptr::new_reference (result); return gdb_bfd_ref_ptr::new_reference (result);
} }

View file

@ -447,8 +447,6 @@ macho_add_oso_symfile (oso_el *oso, const gdb_bfd_ref_ptr &abfd,
return; return;
} }
bfd_set_cacheable (abfd.get (), 1);
/* Read symbols table. */ /* Read symbols table. */
storage = bfd_get_symtab_upper_bound (abfd.get ()); storage = bfd_get_symtab_upper_bound (abfd.get ());
symbol_table = (asymbol **) xmalloc (storage); symbol_table = (asymbol **) xmalloc (storage);

View file

@ -430,9 +430,6 @@ solib_bfd_fopen (const char *pathname, int fd)
{ {
gdb_bfd_ref_ptr abfd (gdb_bfd_open (pathname, gnutarget, fd)); gdb_bfd_ref_ptr abfd (gdb_bfd_open (pathname, gnutarget, fd));
if (abfd != NULL && !gdb_bfd_has_target_filename (abfd.get ()))
bfd_set_cacheable (abfd.get (), 1);
if (abfd == NULL) if (abfd == NULL)
{ {
/* Arrange to free PATHNAME when the error is thrown. */ /* Arrange to free PATHNAME when the error is thrown. */

View file

@ -1762,9 +1762,6 @@ symfile_bfd_open (const char *name)
error (_("`%s': can't open to read symbols: %s."), name, error (_("`%s': can't open to read symbols: %s."), name,
bfd_errmsg (bfd_get_error ())); bfd_errmsg (bfd_get_error ()));
if (!gdb_bfd_has_target_filename (sym_bfd.get ()))
bfd_set_cacheable (sym_bfd.get (), 1);
if (!bfd_check_format (sym_bfd.get (), bfd_object)) if (!bfd_check_format (sym_bfd.get (), bfd_object))
error (_("`%s': can't read symbols: %s."), name, error (_("`%s': can't read symbols: %s."), name,
bfd_errmsg (bfd_get_error ())); bfd_errmsg (bfd_get_error ()));