[PATCH 2/3] Consistently use BFD's time

Tom Tromey tromey@adacore.com
Tue Jan 14 21:10:00 GMT 2020


gdb uses the gnulib stat, while BFD does not.  This can lead to
inconsistencies between the two, because the gnulib stat adjusts for
timezones.

This patch changes gdb to use bfd_stat when examining a BFD's time.
This avoids the problem; and also opens the door to caching target
BFDs as well.

One possible downside here is that gdb must now create a BFD before
checking the cache.

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

	* gdb_bfd.c (gdb_bfd_open): Use bfd_stat.
	* symfile.c (reread_symbols): Use bfd_stat.

Change-Id: Ie937630a221cbae15809c99327b47c1cbce141c0
---
 gdb/ChangeLog |  5 +++++
 gdb/gdb_bfd.c | 49 +++++++++++++++++++++++++------------------------
 gdb/symfile.c |  5 +----
 3 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index a1ee7814f32..26968396a15 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -442,8 +442,15 @@ gdb_bfd_open (const char *name, const char *target, int fd)
 	}
     }
 
+  /* We open the BFD here so that we can use BFD to get the mtime.
+     This is important because gdb uses the gnulib stat, but BFD does
+     not, and this can lead to differences on some systems.  */
+  abfd = bfd_fopen (name, target, FOPEN_RB, fd);
+  if (abfd == NULL)
+    return NULL;
+
   search.filename = name;
-  if (fstat (fd, &st) < 0)
+  if (bfd_stat (abfd, &st) < 0)
     {
       /* Weird situation here.  */
       search.mtime = 0;
@@ -461,38 +468,32 @@ gdb_bfd_open (const char *name, const char *target, int fd)
 
   /* Note that this must compute the same result as hash_bfd.  */
   hash = htab_hash_string (name);
-  /* Note that we cannot use htab_find_slot_with_hash here, because
-     opening the BFD may fail; and this would violate hashtab
-     invariants.  */
-  abfd = (struct bfd *) htab_find_with_hash (gdb_bfd_cache, &search, hash);
-  if (bfd_sharing && abfd != NULL)
+
+  if (bfd_sharing)
     {
-      if (debug_bfd_cache)
-	fprintf_unfiltered (gdb_stdlog,
-			    "Reusing cached bfd %s for %s\n",
-			    host_address_to_string (abfd),
-			    bfd_get_filename (abfd));
-      close (fd);
-      return gdb_bfd_ref_ptr::new_reference (abfd);
+      slot = htab_find_slot_with_hash (gdb_bfd_cache, &search, hash, INSERT);
+      if (*slot != nullptr)
+	{
+	  bfd_close (abfd);
+	  abfd = (bfd *) *slot;
+	  if (debug_bfd_cache)
+	    fprintf_unfiltered (gdb_stdlog,
+				"Reusing cached bfd %s for %s\n",
+				host_address_to_string (abfd),
+				bfd_get_filename (abfd));
+	  close (fd);
+	  return gdb_bfd_ref_ptr::new_reference (abfd);
+	}
+      else
+	*slot = abfd;
     }
 
-  abfd = bfd_fopen (name, target, FOPEN_RB, fd);
-  if (abfd == NULL)
-    return NULL;
-
   if (debug_bfd_cache)
     fprintf_unfiltered (gdb_stdlog,
 			"Creating new bfd %s for %s\n",
 			host_address_to_string (abfd),
 			bfd_get_filename (abfd));
 
-  if (bfd_sharing)
-    {
-      slot = htab_find_slot_with_hash (gdb_bfd_cache, &search, hash, INSERT);
-      gdb_assert (!*slot);
-      *slot = abfd;
-    }
-
   /* It's important to pass the already-computed mtime here, rather
      than, say, calling gdb_bfd_ref_ptr::new_reference here.  BFD by
      default will "stat" the file each time bfd_get_mtime is called --
diff --git a/gdb/symfile.c b/gdb/symfile.c
index f7bada75f35..18995351ed3 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -2456,10 +2456,7 @@ reread_symbols (void)
 	 `ar', often called a `static library' on most systems, though
 	 a `shared library' on AIX is also an archive), then you should
 	 stat on the archive name, not member name.  */
-      if (objfile->obfd->my_archive)
-	res = stat (objfile->obfd->my_archive->filename, &new_statbuf);
-      else
-	res = stat (objfile_name (objfile), &new_statbuf);
+      res = bfd_stat (objfile->obfd, &new_statbuf);
       if (res != 0)
 	{
 	  /* FIXME, should use print_sys_errmsg but it's not filtered.  */
-- 
2.21.1



More information about the Gdb-patches mailing list