public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Tom Tromey <tom@tromey.com>
To: gdb-patches@sourceware.org
Cc: Tom Tromey <tom@tromey.com>
Subject: [PATCH 01/15] Add thread-safety to gdb's BFD wrappers
Date: Sun, 29 Oct 2023 11:35:20 -0600	[thread overview]
Message-ID: <20231029173839.471514-2-tom@tromey.com> (raw)
In-Reply-To: <20231029173839.471514-1-tom@tromey.com>

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.
---
 gdb/dwarf2/read.c |  3 +--
 gdb/gdb_bfd.c     | 53 +++++++++++++++++++++++++++++++++++++++++++++++
 gdb/gdb_bfd.h     |  5 +++++
 gdb/main.c        |  3 +--
 4 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 3b3daf28965..088f01d807d 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -3936,8 +3936,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 56a4c5ecc91..5f100a50fa5 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -35,6 +35,34 @@
 #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 void
+gdb_bfd_lock ()
+{
+  gdb_bfd_mutex.lock ();
+}
+
+/* BFD unlocking function.  */
+
+static void
+gdb_bfd_unlock ()
+{
+  gdb_bfd_mutex.unlock ();
+}
+
+#endif /* CXX_STD_THREAD */
+
 /* An object of this type is stored in the section's user data when
    mapping a section.  */
 
@@ -498,6 +526,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 +657,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 +690,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 +1211,19 @@ 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)
+    error (_("fatal error: libbfd ABI mismatch"));
+
+#if CXX_STD_THREAD
+  bfd_thread_init (gdb_bfd_lock, gdb_bfd_unlock);
+#endif
+}
+
 void _initialize_gdb_bfd ();
 void
 _initialize_gdb_bfd ()
diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
index 604365b61b1..669c3a06c98 100644
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -250,4 +250,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 2da39f89a90..39eacd2208b 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -688,8 +688,7 @@ captured_main_1 (struct captured_main_args *context)
   gdb_stdtargerr = gdb_stderr;
   gdb_stdtargin = gdb_stdin;
 
-  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
-- 
2.41.0


  reply	other threads:[~2023-10-29 17:38 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-29 17:35 [PATCH 00/15] Index DWARF in the background Tom Tromey
2023-10-29 17:35 ` Tom Tromey [this message]
2023-10-29 17:35 ` [PATCH 02/15] Refactor complaint thread-safety approach Tom Tromey
2023-10-29 17:35 ` [PATCH 03/15] Add quick_symbol_functions::compute_main_name Tom Tromey
2023-10-29 17:35 ` [PATCH 04/15] Add gdb::task_group Tom Tromey
2023-10-29 17:35 ` [PATCH 05/15] Move cooked_index_storage to cooked-index.h Tom Tromey
2023-10-29 17:35 ` [PATCH 06/15] Add "maint set dwarf synchronous" Tom Tromey
2023-10-29 17:50   ` Eli Zaretskii
     [not found]     ` <87v89tfz40.fsf@tromey.com>
2023-11-23  6:08       ` Eli Zaretskii
2023-10-29 17:35 ` [PATCH 07/15] Change how cooked index waits for threads Tom Tromey
2023-10-29 17:35 ` [PATCH 08/15] Do more DWARF reading in the background Tom Tromey
2023-10-29 17:35 ` [PATCH 09/15] Simplify the public DWARF API Tom Tromey
2023-10-29 17:35 ` [PATCH 10/15] Remove two quick_symbol_functions methods Tom Tromey
2023-10-29 17:35 ` [PATCH 11/15] Change current_language to be a macro Tom Tromey
2023-10-29 17:35 ` [PATCH 12/15] Lazy language setting Tom Tromey
2023-10-29 17:35 ` [PATCH 13/15] Optimize lookup_minimal_symbol_text Tom Tromey
2023-10-29 17:35 ` [PATCH 14/15] Avoid language-based lookups in startup path Tom Tromey
2023-10-29 17:35 ` [PATCH 15/15] Back out some parallel_for_each features Tom Tromey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231029173839.471514-2-tom@tromey.com \
    --to=tom@tromey.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).