[PATCH v2 2/2] [gdb/symtab] Fix data race in try_open_dwop_file

[Tom de Vries <tdevries@suse.de>]

The call to bfd_check_format in try_open_dwop_file:
...
    /* The operations below are not thread-safe, use a lock to synchronize
       concurrent accesses.  */
    static gdb::mutex mutex;
    gdb::lock_guard<gdb::mutex> lock (mutex);

    if (!bfd_check_format (sym_bfd.get (), bfd_object))
      return NULL;
...
accesses the sym_bfd.get () BFD, so it should be guarded by the global BFD
lock.

Fix this by:
- adding a gdb_bfd_check_format that uses the global BFD lock,
- using it in try_open_dwop_file instead of bfd_check_format, and
- removing the local lock.

The local lock also guarded a call to gdb_bfd_record_inclusion, which doesn't
do any locking, so likewise use the global BFD lock in
gdb_bfd_record_inclusion.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33809
---
 gdb/dwarf2/read.c | 15 ++++-----------
 gdb/gdb_bfd.c     | 14 ++++++++++++--
 gdb/gdb_bfd.h     |  4 ++++
 3 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index efe0b046f15..c4979b1e494 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -6998,21 +6998,14 @@ try_open_dwop_file (dwarf2_per_bfd *per_bfd, const char *file_name, int is_dwp,
   if (sym_bfd == NULL)
     return NULL;
 
-  {
-    /* The operations below are not thread-safe, use a lock to synchronize
-       concurrent accesses.  */
-    static gdb::mutex mutex;
-    gdb::lock_guard<gdb::mutex> lock (mutex);
-
-    if (!bfd_check_format (sym_bfd.get (), bfd_object))
-      return NULL;
+  if (!gdb_bfd_check_format (sym_bfd.get (), bfd_object))
+    return NULL;
 
-    /* Success.  Record the bfd as having been included by the objfile's bfd.
+  /* Success.  Record the bfd as having been included by the objfile's bfd.
      This is important because things like demangled_names_hash lives in the
      objfile's per_bfd space and may have references to things like symbol
      names that live in the DWO/DWP file's per_bfd space.  PR 16426.  */
-    gdb_bfd_record_inclusion (per_bfd->obfd, sym_bfd.get ());
-  }
+  gdb_bfd_record_inclusion (per_bfd->obfd, sym_bfd.get ());
 
   return sym_bfd;
 }
diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 712cea494fa..bf91b4de155 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -1022,9 +1022,9 @@ gdb_bfd_openr_next_archived_file (bfd *archive, bfd *previous)
 void
 gdb_bfd_record_inclusion (bfd *includer, bfd *includee)
 {
-  struct gdb_bfd_data *gdata;
+  gdb::lock_guard<gdb::recursive_mutex> guard (gdb_bfd_mutex);
 
-  gdata = (struct gdb_bfd_data *) bfd_usrdata (includer);
+  struct gdb_bfd_data *gdata = (struct gdb_bfd_data *) bfd_usrdata (includer);
   gdata->included_bfds.push_back (gdb_bfd_ref_ptr::new_reference (includee));
 }
 
@@ -1196,6 +1196,16 @@ gdb_bfd_canonicalize_symtab (bfd *abfd, bool should_throw)
 			       symbol_table.size () - 1);
 }
 
+/* See gdb_bfd.h.  */
+
+bool
+gdb_bfd_check_format (bfd *abfd, bfd_format format)
+{
+  gdb::lock_guard<gdb::recursive_mutex> guard (gdb_bfd_mutex);
+
+  return bfd_check_format (abfd, format);
+}
+
 /* Implement the 'maint info bfd' command.  */
 
 static void
diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
index 69bfa3011aa..2308e76fdd8 100644
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -289,4 +289,8 @@ extern void gdb_bfd_init ();
 extern gdb::array_view<asymbol *> gdb_bfd_canonicalize_symtab
      (bfd *abfd, bool should_throw = true);
 
+/* A wrapper for bfd_check_format that acquires the BFD lock.  */
+
+extern bool gdb_bfd_check_format (bfd *abfd, bfd_format format);
+
 #endif /* GDB_GDB_BFD_H */
-- 
2.51.0