From 0ecf4403168bb53c6ce729b1f042610f82043d45 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Fri, 4 Aug 2023 05:58:35 -0600 Subject: [PATCH] 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 --- gdb/dwarf2/read.c | 1 - gdb/gdb_bfd.c | 5 +++++ gdb/machoread.c | 2 -- gdb/solib.c | 3 --- gdb/symfile.c | 3 --- 5 files changed, 5 insertions(+), 9 deletions(-) diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 61730f6..d5d405f 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -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; diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c index 17e454e..9227a6c 100644 --- a/gdb/gdb_bfd.c +++ b/gdb/gdb_bfd.c @@ -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); } diff --git a/gdb/machoread.c b/gdb/machoread.c index daf6256..5154d1a 100644 --- a/gdb/machoread.c +++ b/gdb/machoread.c @@ -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); diff --git a/gdb/solib.c b/gdb/solib.c index 701efa8..4f980e9 100644 --- a/gdb/solib.c +++ b/gdb/solib.c @@ -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. */ diff --git a/gdb/symfile.c b/gdb/symfile.c index d284040..1b46ec4 100644 --- a/gdb/symfile.c +++ b/gdb/symfile.c @@ -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 ())); -- 2.7.4