Consolidate calls to bfd_set_cacheable
authorTom Tromey <tom@tromey.com>
Fri, 4 Aug 2023 11:58:35 +0000 (05:58 -0600)
committerTom Tromey <tom@tromey.com>
Fri, 4 Aug 2023 18:05:53 +0000 (12:05 -0600)
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>
gdb/dwarf2/read.c
gdb/gdb_bfd.c
gdb/machoread.c
gdb/solib.c
gdb/symfile.c

index 61730f6481c219ee0baf3049f402cf970483f3d0..d5d405f780d72fa2ff24014dfcc6a70d748a0e7a 100644 (file)
@@ -9184,7 +9184,6 @@ try_open_dwop_file (dwarf2_per_objfile *per_objfile,
                                         gnutarget, desc));
   if (sym_bfd == NULL)
     return NULL;
-  bfd_set_cacheable (sym_bfd.get (), 1);
 
   if (!bfd_check_format (sym_bfd.get (), bfd_object))
     return NULL;
index 17e454eb9fd11fd14e29441404a6f7e88029798e..9227a6ce01e7eb2afd5f2b30b9c4a51fcdda07d6 100644 (file)
@@ -564,6 +564,8 @@ gdb_bfd_open (const char *name, const char *target, int fd,
   if (abfd == NULL)
     return NULL;
 
+  bfd_set_cacheable (abfd, 1);
+
   bfd_cache_debug_printf ("Creating new bfd %s for %s",
                          host_address_to_string (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);
 
+  if (result != nullptr)
+    bfd_set_cacheable (result, 1);
+
   return gdb_bfd_ref_ptr::new_reference (result);
 }
 
index daf62563754e4cd84d99abf3d0db72b5ae48f400..5154d1a31a38410a9f663b88de6c4a31afb465ba 100644 (file)
@@ -447,8 +447,6 @@ macho_add_oso_symfile (oso_el *oso, const gdb_bfd_ref_ptr &abfd,
       return;
     }
 
-  bfd_set_cacheable (abfd.get (), 1);
-
   /* Read symbols table.  */
   storage = bfd_get_symtab_upper_bound (abfd.get ());
   symbol_table = (asymbol **) xmalloc (storage);
index 701efa85d1a60611bc86a7f5c5203d4b6d5c7edd..4f980e9365c4ad4236bfed2fbf60f2740f8091b3 100644 (file)
@@ -430,9 +430,6 @@ solib_bfd_fopen (const char *pathname, int 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)
     {
       /* Arrange to free PATHNAME when the error is thrown.  */
index d28404070cda2340f1f7f8961699ee60fd5fe6d8..1b46ec45f2e751d66cc185d70ba0b32aac6f3973 100644 (file)
@@ -1762,9 +1762,6 @@ symfile_bfd_open (const char *name)
     error (_("`%s': can't open to read symbols: %s."), name,
           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))
     error (_("`%s': can't read symbols: %s."), name,
           bfd_errmsg (bfd_get_error ()));