[PATCH 3/3] Further simplify gdb BFD caching

[Tom Tromey]

This patch wasn't necessary to fix any bug, but while working on the
previous patches I noticed that gdb was calling bfd_stat one extra
time, and that it would be simple to rearrange the code to avoid this
extra call.  This also changes gdb so that it no longer caches a BFD
if the stat fails, which seems preferable.

gdb/ChangeLog
2020-01-14  Tom Tromey  <tromey@adacore.com>

	* gdb_bfd.c (gdb_bfd_data): Remove "mt" parameter, add "st"
	parameter.  Don't call bfd_stat.
	(gdb_bfd_init_data): Replace "mt" parameter with "st".
	(gdb_bfd_open): Rearrange and update.
	(gdb_bfd_ref): Call bfd_stat.

Change-Id: I48f13f09f59bde49191cb40ee88ed3e18fa7c7d9
---
 gdb/ChangeLog |  8 ++++++
 gdb/gdb_bfd.c | 69 ++++++++++++++++++++++-----------------------------
 2 files changed, 37 insertions(+), 40 deletions(-)

diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 26968396a15..d64630b2362 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -59,26 +59,16 @@ static htab_t all_bfds;
 
 struct gdb_bfd_data
 {
-  gdb_bfd_data (bfd *abfd, time_t mt)
-    : mtime (mt),
-      size (bfd_get_size (abfd)),
+  /* Note that if ST is nullptr, then we simply fill in zeroes.  */
+  gdb_bfd_data (bfd *abfd, struct stat *st)
+    : mtime (st == nullptr ? 0 : st->st_mtime),
+      size (st == nullptr ? 0 : st->st_size),
+      inode (st == nullptr ? 0 : st->st_ino),
+      device_id (st == nullptr ? 0 : st->st_dev),
       relocation_computed (0),
       needs_relocations (0),
       crc_computed (0)
   {
-    struct stat buf;
-
-    if (bfd_stat (abfd, &buf) == 0)
-      {
-	inode = buf.st_ino;
-	device_id = buf.st_dev;
-      }
-    else
-      {
-	/* The stat failed.  */
-	inode = 0;
-	device_id = 0;
-      }
   }
 
   ~gdb_bfd_data ()
@@ -380,7 +370,7 @@ gdb_bfd_iovec_fileio_fstat (struct bfd *abfd, void *stream,
    BFD.  */
 
 static void
-gdb_bfd_init_data (struct bfd *abfd, time_t mtime)
+gdb_bfd_init_data (struct bfd *abfd, struct stat *st)
 {
   struct gdb_bfd_data *gdata;
   void **slot;
@@ -390,7 +380,7 @@ gdb_bfd_init_data (struct bfd *abfd, time_t mtime)
   /* Ask BFD to decompress sections in bfd_get_full_section_contents.  */
   abfd->flags |= BFD_DECOMPRESS;
 
-  gdata = new gdb_bfd_data (abfd, mtime);
+  gdata = new gdb_bfd_data (abfd, st);
   bfd_set_usrdata (abfd, gdata);
   bfd_alloc_data (abfd);
 
@@ -405,10 +395,7 @@ gdb_bfd_init_data (struct bfd *abfd, time_t mtime)
 gdb_bfd_ref_ptr
 gdb_bfd_open (const char *name, const char *target, int fd)
 {
-  hashval_t hash;
-  void **slot;
   bfd *abfd;
-  struct gdb_bfd_cache_search search;
   struct stat st;
 
   if (is_target_filename (name))
@@ -428,10 +415,6 @@ gdb_bfd_open (const char *name, const char *target, int fd)
       name += strlen (TARGET_SYSROOT_PREFIX);
     }
 
-  if (gdb_bfd_cache == NULL)
-    gdb_bfd_cache = htab_create_alloc (1, hash_bfd, eq_bfd, NULL,
-				       xcalloc, xfree);
-
   if (fd == -1)
     {
       fd = gdb_open_cloexec (name, O_RDONLY | O_BINARY, 0);
@@ -449,29 +432,31 @@ gdb_bfd_open (const char *name, const char *target, int fd)
   if (abfd == NULL)
     return NULL;
 
-  search.filename = name;
+  struct stat *st_ptr = &st;
   if (bfd_stat (abfd, &st) < 0)
     {
-      /* Weird situation here.  */
-      search.mtime = 0;
-      search.size = 0;
-      search.inode = 0;
-      search.device_id = 0;
+      /* Weird situation here -- don't cache if we can't stat.  */
+      st_ptr = nullptr;
     }
-  else
+
+  if (bfd_sharing && st_ptr != nullptr)
     {
+      if (gdb_bfd_cache == NULL)
+	gdb_bfd_cache = htab_create_alloc (1, hash_bfd, eq_bfd, NULL,
+					   xcalloc, xfree);
+
+      struct gdb_bfd_cache_search search;
+      search.filename = name;
       search.mtime = st.st_mtime;
       search.size = st.st_size;
       search.inode = st.st_ino;
       search.device_id = st.st_dev;
-    }
 
-  /* Note that this must compute the same result as hash_bfd.  */
-  hash = htab_hash_string (name);
+      /* Note that this must compute the same result as hash_bfd.  */
+      hashval_t hash = htab_hash_string (name);
 
-  if (bfd_sharing)
-    {
-      slot = htab_find_slot_with_hash (gdb_bfd_cache, &search, hash, INSERT);
+      void **slot = htab_find_slot_with_hash (gdb_bfd_cache, &search, hash,
+					      INSERT);
       if (*slot != nullptr)
 	{
 	  bfd_close (abfd);
@@ -500,7 +485,7 @@ gdb_bfd_open (const char *name, const char *target, int fd)
      and since we already entered it into the hash table using this
      mtime, if the file changed at the wrong moment, the race would
      lead to a hash table corruption.  */
-  gdb_bfd_init_data (abfd, search.mtime);
+  gdb_bfd_init_data (abfd, st_ptr);
   return gdb_bfd_ref_ptr (abfd);
 }
 
@@ -572,7 +557,11 @@ gdb_bfd_ref (struct bfd *abfd)
       return;
     }
 
-  gdb_bfd_init_data (abfd, bfd_get_mtime (abfd));
+  struct stat st, *st_ptr = &st;
+  if (bfd_stat (abfd, &st) < 0)
+    st_ptr = nullptr;
+
+  gdb_bfd_init_data (abfd, st_ptr);
 }
 
 /* See gdb_bfd.h.  */
-- 
2.21.1