[PATCH 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:
- using the global BFD lock in gdb_bfd_check_format, and
- removing the local lock.

Likewise, use the global BFD lock in gdb_bfd_check_format_matches.

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     |  8 ++++++--
 gdb/gdb_bfd.h     |  4 ++--
 3 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 8067af5f52e..742be7656b1 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -6744,21 +6744,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 (!gdb_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 7a845d6e04b..749d19789cf 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -1026,9 +1026,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));
 }
 
@@ -1205,6 +1205,8 @@ gdb_bfd_canonicalize_symtab (bfd *abfd, bool should_throw)
 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);
 }
 
@@ -1213,6 +1215,8 @@ gdb_bfd_check_format (bfd *abfd, bfd_format format)
 bool
 gdb_bfd_check_format_matches (bfd *abfd, bfd_format format, char ***matching)
 {
+  gdb::lock_guard<gdb::recursive_mutex> guard (gdb_bfd_mutex);
+
   return bfd_check_format_matches (abfd, format, matching);
 }
 
diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
index ff9f2337e6e..09b544b3b26 100644
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -289,11 +289,11 @@ 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.  */
+/* A wrapper for bfd_check_format that acquires the BFD lock.  */
 
 extern bool gdb_bfd_check_format (bfd *abfd, bfd_format format);
 
-/* A wrapper for bfd_check_format_matches.  */
+/* A wrapper for bfd_check_format_matches that acquires the BFD lock.  */
 
 extern bool gdb_bfd_check_format_matches (bfd *abfd, bfd_format format,
 					  char ***matching);
-- 
2.51.0