Lock bfd_stat and bfd_get_mtime

PR gdb/31713 points out some races when using the background DWARF
reader.

This particular patch fixes some of these, namely the ones when using
the sim.  In this case, the 'load' command calls reopen_exec_file,
which calls bfd_stat, which introduces a race.

BFD only locks globals -- concurrent use of a single BFD must be
handled by the application.  To this end, this patch adds locked
wrappers for bfd_stat and bfd_get_mtime.

I couldn't reproduce these data races but the original reporter tested
the patch and confirms that it helps.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31713
Approved-By: Andrew Burgess <aburgess@redhat.com>

diff --git a/gdb/corefile.c b/gdb/corefile.c
index f6ec3cd5ca1176c447cd12d56e4a19b60abe1948..be0f1075488df538e9e821e0008f7482550da051 100644 (file)
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -51,7 +51,7 @@ reopen_exec_file (void)
 
   /* If the timestamp of the exec file has changed, reopen it.  */
   struct stat st;
-  int res = bfd_stat (exec_bfd, &st);
+  int res = gdb_bfd_stat (exec_bfd, &st);
 
   if (res == 0
       && current_program_space->ebfd_mtime != 0
@@ -70,8 +70,8 @@ validate_files (void)
       if (!core_file_matches_executable_p (current_program_space->core_bfd (),
                                           current_program_space->exec_bfd ()))
        warning (_("core file may not match specified executable file."));
-      else if (bfd_get_mtime (current_program_space->exec_bfd ())
-              > bfd_get_mtime (current_program_space->core_bfd ()))
+      else if (gdb_bfd_get_mtime (current_program_space->exec_bfd ())
+              > gdb_bfd_get_mtime (current_program_space->core_bfd ()))
        warning (_("exec file is newer than core file."));
     }
 }

diff --git a/gdb/exec.c b/gdb/exec.c
index 82d9266b7e3669dabfe29c5362498d60be9fa6c5..73280ad59e7c26452fd88080bbbc15e6435ca27b 100644 (file)
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -483,7 +483,7 @@ exec_file_attach (const char *filename, int from_tty)
        = build_section_table (current_program_space->exec_bfd ());
 
       current_program_space->ebfd_mtime
-       = bfd_get_mtime (current_program_space->exec_bfd ());
+       = gdb_bfd_get_mtime (current_program_space->exec_bfd ());
 
       validate_files ();
 

diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 3683eeba52bf0b0d7e87a0bd150e8155dcef8a9d..c233551b4e8b51ce577ab8fd7e1f8bebdc52ca6c 100644 (file)
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -1121,6 +1121,32 @@ gdb_bfd_get_full_section_contents (bfd *abfd, asection *section,
                                   section_size);
 }
 
+/* See gdb_bfd.h.  */
+
+int
+gdb_bfd_stat (bfd *abfd, struct stat *sbuf)
+{
+#if CXX_STD_THREAD
+  gdb_bfd_data *gdata = (gdb_bfd_data *) bfd_usrdata (abfd);
+  std::lock_guard<std::mutex> guard (gdata->per_bfd_mutex);
+#endif
+
+  return bfd_stat (abfd, sbuf);
+}
+
+/* See gdb_bfd.h.  */
+
+long
+gdb_bfd_get_mtime (bfd *abfd)
+{
+#if CXX_STD_THREAD
+  gdb_bfd_data *gdata = (gdb_bfd_data *) bfd_usrdata (abfd);
+  std::lock_guard<std::mutex> guard (gdata->per_bfd_mutex);
+#endif
+
+  return bfd_get_mtime (abfd);
+}
+
 #define AMBIGUOUS_MESS1        ".\nMatching formats:"
 #define AMBIGUOUS_MESS2        \
   ".\nUse \"set gnutarget format-name\" to specify the format."

diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
index 331570d769c818355d764ad39bbc12c317730a01..a93d9aa92e8dd7c1206cc5bc0024acb3e288be9a 100644 (file)
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -251,6 +251,17 @@ gdb_bfd_sections (const gdb_bfd_ref_ptr &abfd)
   return gdb_bfd_section_range (abfd->sections);
 };
 
+/* A wrapper for bfd_stat that acquires the per-BFD lock on ABFD.  */
+
+extern int gdb_bfd_stat (bfd *abfd, struct stat *sbuf)
+  ATTRIBUTE_WARN_UNUSED_RESULT;
+
+/* A wrapper for bfd_get_mtime that acquires the per-BFD lock on
+   ABFD.  */
+
+extern long gdb_bfd_get_mtime (bfd *abfd)
+  ATTRIBUTE_WARN_UNUSED_RESULT;
+
 /* A wrapper for bfd_errmsg to produce a more helpful error message
    in the case of bfd_error_file_ambiguously recognized.
    MATCHING, if non-NULL, is the corresponding argument to

diff --git a/gdb/machoread.c b/gdb/machoread.c
index ef6cf6606299c1f77538169f121688ee9f151f15..ac764c09496000cba43b1c85a9b5c1a0ba35e074 100644 (file)
--- a/gdb/machoread.c
+++ b/gdb/machoread.c
@@ -434,7 +434,8 @@ macho_add_oso_symfile (oso_el *oso, const gdb_bfd_ref_ptr &abfd,
       return;
     }
 
-  if (abfd->my_archive == NULL && oso->mtime != bfd_get_mtime (abfd.get ()))
+  if (abfd->my_archive == nullptr
+      && oso->mtime != gdb_bfd_get_mtime (abfd.get ()))
     {
       warning (_("`%s': file time stamp mismatch."), oso->name);
       return;

diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index 76d17f548a58101d42c33c46cf534fbfb593a91e..84866e1e30aa91b42d1a68e2dae88e5259319ee9 100644 (file)
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -281,7 +281,7 @@ objfile::objfile (gdb_bfd_ref_ptr bfd_, program_space *pspace,
 
   if (obfd != nullptr)
     {
-      mtime = bfd_get_mtime (obfd.get ());
+      mtime = gdb_bfd_get_mtime (obfd.get ());
 
       /* Build section table.  */
       build_objfile_section_table (this);

diff --git a/gdb/symfile.c b/gdb/symfile.c
index fd21d2d2c238d8e437c807f20348704e5edf8a86..4d5f3d2626278eec835ae968fe55d3b61be9fed3 100644 (file)
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1275,9 +1275,9 @@ separate_debug_file_exists (const std::string &name, unsigned long crc,
      numbers will never set st_ino to zero, this is merely an
      optimization, so we do not need to worry about false negatives.  */
 
-  if (bfd_stat (abfd.get (), &abfd_stat) == 0
+  if (gdb_bfd_stat (abfd.get (), &abfd_stat) == 0
       && abfd_stat.st_ino != 0
-      && bfd_stat (parent_objfile->obfd.get (), &parent_stat) == 0)
+      && gdb_bfd_stat (parent_objfile->obfd.get (), &parent_stat) == 0)
     {
       if (abfd_stat.st_dev == parent_stat.st_dev
          && abfd_stat.st_ino == parent_stat.st_ino)
@@ -2496,7 +2496,7 @@ reread_symbols (int from_tty)
        continue;
 
       struct stat new_statbuf;
-      int res = bfd_stat (objfile->obfd.get (), &new_statbuf);
+      int res = gdb_bfd_stat (objfile->obfd.get (), &new_statbuf);
       if (res != 0)
        {
          /* If this object is from an archive (what you usually create