Add thread-safety to gdb's BFD wrappers

This changes gdb to ensure that gdb's BFD cache is guarded by a lock.
This avoids any races when multiple threads might open a BFD (and thus
use the BFD cache) at the same time.

Currently, this change is not needed because the the main thread waits
for some DWARF scanning to be completed before returning.  The only
locking that's required is when opening DWO files, and there's a local
lock to this end in dwarf2/read.c.

However, in the coming patches, the DWARF reader will begin its work
earlier, in the background.  This means there is the potential for the
DWARF reader and other code on the main thread to both attempt to open
BFDs at the same time.

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 45f79deb31776ec5ed01376e0eaa2d1259d08fc2..a0fddbe623a4345938725bb7b6ed1de113d7e0f2 100644 (file)
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -3920,8 +3920,7 @@ static struct dwo_unit *
 lookup_dwo_unit (dwarf2_cu *cu, die_info *comp_unit_die, const char *dwo_name)
 {
 #if CXX_STD_THREAD
-  /* We need a lock here both to handle the DWO hash table, and BFD,
-     which is not thread-safe.  */
+  /* We need a lock here both to handle the DWO hash table.  */
   static std::mutex dwo_lock;
 
   std::lock_guard<std::mutex> guard (dwo_lock);

diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 1c496c498ac8917849ad485054a1988314c396b3..9f7d0f626c03a0526027a2859a26720f6ca9e59b 100644 (file)
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -35,6 +35,36 @@
 #include "cli/cli-style.h"
 #include <unordered_map>
 
+#if CXX_STD_THREAD
+
+#include <mutex>
+
+/* Lock held when doing BFD operations.  A recursive mutex is used
+   because we use this mutex internally and also for BFD, just to make
+   life a bit simpler, and we may sometimes hold it while calling into
+   BFD.  */
+static std::recursive_mutex gdb_bfd_mutex;
+
+/* BFD locking function.  */
+
+static bool
+gdb_bfd_lock (void *ignore)
+{
+  gdb_bfd_mutex.lock ();
+  return true;
+}
+
+/* BFD unlocking function.  */
+
+static bool
+gdb_bfd_unlock (void *ignore)
+{
+  gdb_bfd_mutex.unlock ();
+  return true;
+}
+
+#endif /* CXX_STD_THREAD */
+
 /* An object of this type is stored in the section's user data when
    mapping a section.  */
 
@@ -498,6 +528,10 @@ gdb_bfd_open (const char *name, const char *target, int fd,
       name += strlen (TARGET_SYSROOT_PREFIX);
     }
 
+#if CXX_STD_THREAD
+  std::lock_guard<std::recursive_mutex> guard (gdb_bfd_mutex);
+#endif
+
   if (gdb_bfd_cache == NULL)
     gdb_bfd_cache = htab_create_alloc (1, hash_bfd, eq_bfd, NULL,
                                       xcalloc, xfree);
@@ -625,6 +659,10 @@ gdb_bfd_ref (struct bfd *abfd)
   if (abfd == NULL)
     return;
 
+#if CXX_STD_THREAD
+  std::lock_guard<std::recursive_mutex> guard (gdb_bfd_mutex);
+#endif
+
   gdata = (struct gdb_bfd_data *) bfd_usrdata (abfd);
 
   bfd_cache_debug_printf ("Increase reference count on bfd %s (%s)",
@@ -654,6 +692,10 @@ gdb_bfd_unref (struct bfd *abfd)
   if (abfd == NULL)
     return;
 
+#if CXX_STD_THREAD
+  std::lock_guard<std::recursive_mutex> guard (gdb_bfd_mutex);
+#endif
+
   gdata = (struct gdb_bfd_data *) bfd_usrdata (abfd);
   gdb_assert (gdata->refc >= 1);
 
@@ -1171,6 +1213,22 @@ gdb_bfd_error_handler (const char *fmt, va_list ap)
   (*default_bfd_error_handler) (fmt, ap);
 }
 
+/* See gdb_bfd.h.  */
+
+void
+gdb_bfd_init ()
+{
+  if (bfd_init () == BFD_INIT_MAGIC)
+    {
+#if CXX_STD_THREAD
+      if (bfd_thread_init (gdb_bfd_lock, gdb_bfd_unlock, nullptr))
+#endif
+       return;
+    }
+
+  error (_("fatal error: libbfd ABI mismatch"));
+}
+
 void _initialize_gdb_bfd ();
 void
 _initialize_gdb_bfd ()

diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
index eeb782edcfccbac14f5aba05b6274aa5173163b6..3de9cd9e904c5d0006b14c40f35506f4a14ece8c 100644 (file)
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -258,4 +258,9 @@ gdb_bfd_sections (const gdb_bfd_ref_ptr &abfd)
 
 extern std::string gdb_bfd_errmsg (bfd_error_type error_tag, char **matching);
 
+/* A wrapper for bfd_init that also handles setting up for
+   multi-threading.  */
+
+extern void gdb_bfd_init ();
+
 #endif /* GDB_BFD_H */

diff --git a/gdb/main.c b/gdb/main.c
index 688db7655a9b4c41ce960f996132c4ba68419a8a..67c7a52f54f5c0d734e4bba198cab0521397da1a 100644 (file)
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -699,8 +699,7 @@ captured_main_1 (struct captured_main_args *context)
   auto temp_uiout = std::make_unique<cli_ui_out> (gdb_stdout);
   current_uiout = temp_uiout.get ();
 
-  if (bfd_init () != BFD_INIT_MAGIC)
-    error (_("fatal error: libbfd ABI mismatch"));
+  gdb_bfd_init ();
 
 #ifdef __MINGW32__
   /* On Windows, argv[0] is not necessarily set to absolute form when