public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Tom Tromey <tom@tromey.com>
To: gdb-patches@sourceware.org
Cc: Tom Tromey <tom@tromey.com>
Subject: [PATCH] Consolidate calls to bfd_set_cacheable
Date: Fri,  4 Aug 2023 10:19:00 -0600	[thread overview]
Message-ID: <20230804161900.384191-1-tom@tromey.com> (raw)

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.

Regression tested on x86-64 Fedora 38.
---
 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 61730f6481c..d5d405f780d 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 17e454eb9fd..9227a6ce01e 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 daf62563754..5154d1a31a3 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 701efa85d1a..4f980e9365c 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 d28404070cd..1b46ec45f2e 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.41.0


             reply	other threads:[~2023-08-04 16:19 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-04 16:19 Tom Tromey [this message]
2023-08-04 16:31 ` John Baldwin
2023-08-04 18:04   ` Tom Tromey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230804161900.384191-1-tom@tromey.com \
    --to=tom@tromey.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).