Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH 0/2] [gdb] TSAN fixes
@ 2026-05-27 19:02 Tom de Vries
  2026-05-27 19:02 ` [PATCH 1/2] [gdb] Add gdb_bfd_check_format and gdb_bfd_check_format_matches Tom de Vries
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Tom de Vries @ 2026-05-27 19:02 UTC (permalink / raw)
  To: gdb-patches

I build gdb with TSAN at -O0, ran the testsuite and found a few problems.

This series fixes those.

The first patch is a refactoring patch.

The second patch removes a local lock in try_open_dwop_file.

Changes since v1:
- commited an approved patch [1]
- combined two patches related to the local lock in try_open_dwop_file
- replaced last patch by new patch changing the locking scheme

Changes since v2:
- committed another approved patch [2]
- use gdb_bfd_check_format consistently instead of just once
- add gdb_bfd_check_format_matches, addressing the same issue as for
  gdb_bfd_check_format
- factor out refactoring patch, to make it easier to review

[1] https://sourceware.org/pipermail/gdb-patches/2026-January/224422.html
[2] https://sourceware.org/pipermail/gdb-patches/2026-February/225028.html

Versions:
- v1 https://sourceware.org/pipermail/gdb-patches/2026-January/224424.html
- v2 https://sourceware.org/pipermail/gdb-patches/2026-February/225027.html

Tom de Vries (2):
  [gdb] Add gdb_bfd_check_format and gdb_bfd_check_format_matches
  [gdb/symtab] Fix data race in try_open_dwop_file

 gdb/build-id.c                    |  4 ++--
 gdb/cli/cli-dump.c                |  2 +-
 gdb/compile/compile-object-load.c |  2 +-
 gdb/corelow.c                     |  8 ++++----
 gdb/dwarf2/dwz.c                  |  2 +-
 gdb/dwarf2/read.c                 | 15 ++++-----------
 gdb/exec.c                        |  2 +-
 gdb/gdb_bfd.c                     | 24 ++++++++++++++++++++++--
 gdb/gdb_bfd.h                     | 11 ++++++++++-
 gdb/i386-darwin-tdep.c            |  2 +-
 gdb/jit.c                         |  2 +-
 gdb/machoread.c                   |  6 +++---
 gdb/minidebug.c                   |  2 +-
 gdb/solib-aix.c                   |  6 +++---
 gdb/solib-rocm.c                  |  2 +-
 gdb/solib.c                       |  4 ++--
 gdb/symfile-mem.c                 |  2 +-
 gdb/symfile.c                     |  6 +++---
 gdb/windows-nat.c                 |  2 +-
 19 files changed, 63 insertions(+), 41 deletions(-)


base-commit: d623dadc2c4ecd509359015ae5a1ac856ba89b05
-- 
2.51.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] [gdb] Add gdb_bfd_check_format and gdb_bfd_check_format_matches
  2026-05-27 19:02 [PATCH 0/2] [gdb] TSAN fixes Tom de Vries
@ 2026-05-27 19:02 ` Tom de Vries
  2026-05-27 19:02 ` [PATCH 2/2] [gdb/symtab] Fix data race in try_open_dwop_file Tom de Vries
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Tom de Vries @ 2026-05-27 19:02 UTC (permalink / raw)
  To: gdb-patches

Add new bfd wrappers:
- gdb_bfd_check_format for bfd_check_format, and
- gdb_bfd_check_format_matches for bfd_check_format_matches.
and run this command to use them:
...
$ sed -i 's/bfd_check_format/gdb_bfd_check_format/' \
  $(find gdb* -type f | egrep -v "/testsuite/|ChangeLog")
...

No functional changes.
---
 gdb/build-id.c                    |  4 ++--
 gdb/cli/cli-dump.c                |  2 +-
 gdb/compile/compile-object-load.c |  2 +-
 gdb/corelow.c                     |  8 ++++----
 gdb/dwarf2/dwz.c                  |  2 +-
 gdb/dwarf2/read.c                 |  2 +-
 gdb/exec.c                        |  2 +-
 gdb/gdb_bfd.c                     | 16 ++++++++++++++++
 gdb/gdb_bfd.h                     | 11 ++++++++++-
 gdb/i386-darwin-tdep.c            |  2 +-
 gdb/jit.c                         |  2 +-
 gdb/machoread.c                   |  6 +++---
 gdb/minidebug.c                   |  2 +-
 gdb/solib-aix.c                   |  6 +++---
 gdb/solib-rocm.c                  |  2 +-
 gdb/solib.c                       |  4 ++--
 gdb/symfile-mem.c                 |  2 +-
 gdb/symfile.c                     |  6 +++---
 gdb/windows-nat.c                 |  2 +-
 19 files changed, 54 insertions(+), 29 deletions(-)

diff --git a/gdb/build-id.c b/gdb/build-id.c
index b49216d4cad..123d87e519c 100644
--- a/gdb/build-id.c
+++ b/gdb/build-id.c
@@ -42,8 +42,8 @@ build_id_bfd_get (bfd *abfd)
   if (abfd == nullptr)
     return nullptr;
 
-  if (!bfd_check_format (abfd, bfd_object)
-      && !bfd_check_format (abfd, bfd_core))
+  if (!gdb_bfd_check_format (abfd, bfd_object)
+      && !gdb_bfd_check_format (abfd, bfd_core))
     return NULL;
 
   if (abfd->build_id != NULL)
diff --git a/gdb/cli/cli-dump.c b/gdb/cli/cli-dump.c
index 66ab5922693..6c6abaa9de3 100644
--- a/gdb/cli/cli-dump.c
+++ b/gdb/cli/cli-dump.c
@@ -89,7 +89,7 @@ bfd_openr_or_error (const char *filename, const char *target)
 	   styled_string (file_name_style.style (), filename),
 	   bfd_errmsg (bfd_get_error ()));
 
-  if (!bfd_check_format (ibfd.get (), bfd_object))
+  if (!gdb_bfd_check_format (ibfd.get (), bfd_object))
     error (_("'%ps' is not a recognized file format."),
 	   styled_string (file_name_style.style (), filename));
 
diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c
index 4ea924582fb..20e4c3e51ce 100644
--- a/gdb/compile/compile-object-load.c
+++ b/gdb/compile/compile-object-load.c
@@ -621,7 +621,7 @@ compile_object_load (const compile_file_names &file_names,
 	   styled_string (file_name_style.style (), filename.get ()),
 	   bfd_errmsg (bfd_get_error ()));
 
-  if (!bfd_check_format_matches (abfd.get (), bfd_object, &matching))
+  if (!gdb_bfd_check_format_matches (abfd.get (), bfd_object, &matching))
     error (_("\"%ps\": not in loadable format: %s"),
 	   styled_string (file_name_style.style (), filename.get ()),
 	   gdb_bfd_errmsg (bfd_get_error (), matching).c_str ());
diff --git a/gdb/corelow.c b/gdb/corelow.c
index 008da68155f..819e7cae6f9 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -450,7 +450,7 @@ core_target::build_file_mappings ()
 	    = gdb_bfd_open (expanded_fname.get (), gnutarget);
 
 	  if (tmp_bfd != nullptr
-	      && bfd_check_format (tmp_bfd.get (), bfd_object)
+	      && gdb_bfd_check_format (tmp_bfd.get (), bfd_object)
 	      && build_id_bfd_get (tmp_bfd.get ()) != nullptr)
 	    {
 	      /* The newly opened TMP_BFD has a build-id, and this mapped
@@ -469,7 +469,7 @@ core_target::build_file_mappings ()
 
       if ((expanded_fname == nullptr
 	   || abfd == nullptr
-	   || !bfd_check_format (abfd.get (), bfd_object))
+	   || !gdb_bfd_check_format (abfd.get (), bfd_object))
 	  && file_data.build_id != nullptr)
 	{
 	  abfd = find_objfile_by_build_id (current_program_space,
@@ -492,7 +492,7 @@ core_target::build_file_mappings ()
 
       if (expanded_fname == nullptr
 	  || abfd == nullptr
-	  || !bfd_check_format (abfd.get (), bfd_object))
+	  || !gdb_bfd_check_format (abfd.get (), bfd_object))
 	{
 	  /* If ABFD was opened, but the wrong format, close it now.  */
 	  abfd = nullptr;
@@ -1071,7 +1071,7 @@ core_target_open (const char *arg, int from_tty)
   if (temp_bfd == NULL)
     perror_with_name (filename.c_str ());
 
-  if (!bfd_check_format (temp_bfd.get (), bfd_core))
+  if (!gdb_bfd_check_format (temp_bfd.get (), bfd_core))
     {
       /* Do it after the err msg */
       /* FIXME: should be checking for errors from bfd_close (for one
diff --git a/gdb/dwarf2/dwz.c b/gdb/dwarf2/dwz.c
index 8f3d52567ec..e61e7fd7605 100644
--- a/gdb/dwarf2/dwz.c
+++ b/gdb/dwarf2/dwz.c
@@ -171,7 +171,7 @@ get_debug_sup_info (bfd *abfd,
 static bool
 verify_id (bfd *abfd, size_t len, const bfd_byte *buildid, bool dwarf5)
 {
-  if (!bfd_check_format (abfd, bfd_object))
+  if (!gdb_bfd_check_format (abfd, bfd_object))
     return false;
 
   if (dwarf5)
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index c761b732819..8067af5f52e 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -6750,7 +6750,7 @@ try_open_dwop_file (dwarf2_per_bfd *per_bfd, const char *file_name, int is_dwp,
     static gdb::mutex mutex;
     gdb::lock_guard<gdb::mutex> lock (mutex);
 
-    if (!bfd_check_format (sym_bfd.get (), bfd_object))
+    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.
diff --git a/gdb/exec.c b/gdb/exec.c
index 873686e6286..36ec45c14bc 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -479,7 +479,7 @@ exec_file_attach (const char *filename, int from_tty)
 	  (make_unique_xstrdup (gdb_realpath_keepfile
 				  (scratch_pathname).c_str ()));
 
-      if (!bfd_check_format_matches (current_program_space->exec_bfd (),
+      if (!gdb_bfd_check_format_matches (current_program_space->exec_bfd (),
 				     bfd_object, &matching))
 	{
 	  /* Make sure to close exec_bfd, or else "run" might try to use
diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 952875955ad..7a845d6e04b 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -1200,6 +1200,22 @@ 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)
+{
+  return bfd_check_format (abfd, format);
+}
+
+/* See gdb_bfd.h.  */
+
+bool
+gdb_bfd_check_format_matches (bfd *abfd, bfd_format format, char ***matching)
+{
+  return bfd_check_format_matches (abfd, format, matching);
+}
+
 /* Implement the 'maint info bfd' command.  */
 
 static void
diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
index 69bfa3011aa..ff9f2337e6e 100644
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -268,7 +268,7 @@ extern long gdb_bfd_get_mtime (bfd *abfd)
 /* 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
-   bfd_check_format_matches, and will be freed.  */
+   gdb_bfd_check_format_matches, and will be freed.  */
 
 extern std::string gdb_bfd_errmsg (bfd_error_type error_tag, char **matching);
 
@@ -289,4 +289,13 @@ 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.  */
+
+extern bool gdb_bfd_check_format (bfd *abfd, bfd_format format);
+
+/* A wrapper for bfd_check_format_matches.  */
+
+extern bool gdb_bfd_check_format_matches (bfd *abfd, bfd_format format,
+					  char ***matching);
+
 #endif /* GDB_GDB_BFD_H */
diff --git a/gdb/i386-darwin-tdep.c b/gdb/i386-darwin-tdep.c
index b9ea7cb3f36..f7512eaf9a3 100644
--- a/gdb/i386-darwin-tdep.c
+++ b/gdb/i386-darwin-tdep.c
@@ -277,7 +277,7 @@ i386_darwin_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 static enum gdb_osabi
 i386_mach_o_osabi_sniffer (bfd *abfd)
 {
-  if (!bfd_check_format (abfd, bfd_object))
+  if (!gdb_bfd_check_format (abfd, bfd_object))
     return GDB_OSABI_UNKNOWN;
 
   if (bfd_get_arch (abfd) == bfd_arch_i386)
diff --git a/gdb/jit.c b/gdb/jit.c
index 5fa3869316c..251e8331c4a 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -782,7 +782,7 @@ jit_bfd_try_read_symtab (struct jit_code_entry *code_entry,
 
   /* Check the format.  NOTE: This initializes important data that GDB uses!
      We would segfault later without this line.  */
-  if (!bfd_check_format (nbfd.get (), bfd_object))
+  if (!gdb_bfd_check_format (nbfd.get (), bfd_object))
     {
       gdb_printf (gdb_stderr, _("\
 JITed symbol file is not an object file, ignoring it.\n"));
diff --git a/gdb/machoread.c b/gdb/machoread.c
index 6bd063a89e9..d273ec2ef90 100644
--- a/gdb/machoread.c
+++ b/gdb/machoread.c
@@ -422,7 +422,7 @@ macho_add_oso_symfile (oso_el *oso, const gdb_bfd_ref_ptr &abfd,
 
   macho_debug (0, _("Loading debugging symbols from oso: %s\n"), oso->name);
 
-  if (!bfd_check_format (abfd.get (), bfd_object))
+  if (!gdb_bfd_check_format (abfd.get (), bfd_object))
     {
       warning (_("`%s': can't read symbols: %s."), oso->name,
 	       bfd_errmsg (bfd_get_error ()));
@@ -634,7 +634,7 @@ macho_symfile_read_all_oso (std::vector<oso_el> *oso_vector_ptr,
 	      ix = last_ix;
 	      continue;
 	    }
-	  if (!bfd_check_format (archive_bfd.get (), bfd_archive))
+	  if (!gdb_bfd_check_format (archive_bfd.get (), bfd_archive))
 	    {
 	      warning (_("OSO archive file \"%s\" not an archive."),
 		       archive_name.c_str ());
@@ -748,7 +748,7 @@ macho_check_dsym (struct objfile *objfile, std::string *filenamep)
       return NULL;
     }
 
-  if (!bfd_check_format (dsym_bfd.get (), bfd_object))
+  if (!gdb_bfd_check_format (dsym_bfd.get (), bfd_object))
     {
       warning (_("bad dsym file format: %s"), bfd_errmsg (bfd_get_error ()));
       return NULL;
diff --git a/gdb/minidebug.c b/gdb/minidebug.c
index 10222ec233d..8c84b7c9664 100644
--- a/gdb/minidebug.c
+++ b/gdb/minidebug.c
@@ -251,7 +251,7 @@ find_separate_debug_file_in_section (struct objfile *objfile)
   if (abfd == NULL)
     return NULL;
 
-  if (!bfd_check_format (abfd.get (), bfd_object))
+  if (!gdb_bfd_check_format (abfd.get (), bfd_object))
     {
       warning (_("Cannot parse .gnu_debugdata section; not a BFD object"));
       return NULL;
diff --git a/gdb/solib-aix.c b/gdb/solib-aix.c
index 4f029529106..70576065121 100644
--- a/gdb/solib-aix.c
+++ b/gdb/solib-aix.c
@@ -547,10 +547,10 @@ aix_solib_ops::bfd_open (const char *pathname) const
       return NULL;
     }
 
-  if (bfd_check_format (archive_bfd.get (), bfd_object))
+  if (gdb_bfd_check_format (archive_bfd.get (), bfd_object))
     return archive_bfd;
 
-  if (! bfd_check_format (archive_bfd.get (), bfd_archive))
+  if (! gdb_bfd_check_format (archive_bfd.get (), bfd_archive))
     {
       warning (_("\"%ps\": not in executable format: %s."),
 	       styled_string (file_name_style.style (), filename.c_str ()),
@@ -590,7 +590,7 @@ aix_solib_ops::bfd_open (const char *pathname) const
       return NULL;
     }
 
-  if (! bfd_check_format (object_bfd.get (), bfd_object))
+  if (! gdb_bfd_check_format (object_bfd.get (), bfd_object))
     {
       warning (_("%ps(%s): not in object format: %s."),
 	       styled_string (file_name_style.style (), filename.c_str ()),
diff --git a/gdb/solib-rocm.c b/gdb/solib-rocm.c
index 38904b9633c..34cb2bb9e57 100644
--- a/gdb/solib-rocm.c
+++ b/gdb/solib-rocm.c
@@ -677,7 +677,7 @@ rocm_solib_ops::bfd_open (const char *pathname) const
 	   bfd_errmsg (bfd_get_error ()));
 
   /* Check bfd format.  */
-  if (!bfd_check_format (abfd.get (), bfd_object))
+  if (!gdb_bfd_check_format (abfd.get (), bfd_object))
     error (_("`%s': not in executable format: %s"),
 	   bfd_get_filename (abfd.get ()), bfd_errmsg (bfd_get_error ()));
 
diff --git a/gdb/solib.c b/gdb/solib.c
index 158c58d9689..d92d0dd9a3d 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -419,7 +419,7 @@ solib_bfd_open (const char *pathname)
   gdb_bfd_ref_ptr abfd (solib_bfd_fopen (found_pathname.get (), found_file));
 
   /* Check bfd format.  */
-  if (!bfd_check_format (abfd.get (), bfd_object))
+  if (!gdb_bfd_check_format (abfd.get (), bfd_object))
     error (_("`%ps': not in executable format: %s"),
 	   styled_string (file_name_style.style (),
 			  bfd_get_filename (abfd.get ())),
@@ -1683,7 +1683,7 @@ gdb_bfd_read_elf_soname (const char *filename)
     return {};
 
   /* Check that ABFD is an ET_DYN ELF file.  */
-  if (!bfd_check_format (abfd.get (), bfd_object)
+  if (!gdb_bfd_check_format (abfd.get (), bfd_object)
       || !(bfd_get_file_flags (abfd.get ()) & DYNAMIC))
     return {};
 
diff --git a/gdb/symfile-mem.c b/gdb/symfile-mem.c
index 941292d3128..7d86fd88228 100644
--- a/gdb/symfile-mem.c
+++ b/gdb/symfile-mem.c
@@ -108,7 +108,7 @@ symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr,
     name = "shared object read from target memory";
   bfd_set_filename (nbfd, name);
 
-  if (!bfd_check_format (nbfd, bfd_object))
+  if (!gdb_bfd_check_format (nbfd, bfd_object))
     error (_("Got object file from memory but can't read symbols: %s."),
 	   bfd_errmsg (bfd_get_error ()));
 
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 0413f7f7ce1..5f046a03567 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1706,7 +1706,7 @@ symfile_bfd_open (const char *name)
 	   styled_string (file_name_style.style (), name),
 	   bfd_errmsg (bfd_get_error ()));
 
-  if (!bfd_check_format (sym_bfd.get (), bfd_object))
+  if (!gdb_bfd_check_format (sym_bfd.get (), bfd_object))
     error (_("`%ps': can't read symbols: %s."),
 	   styled_string (file_name_style.style (), name),
 	   bfd_errmsg (bfd_get_error ()));
@@ -2018,7 +2018,7 @@ generic_load (const char *args, int from_tty)
   if (loadfile_bfd == NULL)
     perror_with_name (filename.get ());
 
-  if (!bfd_check_format (loadfile_bfd.get (), bfd_object))
+  if (!gdb_bfd_check_format (loadfile_bfd.get (), bfd_object))
     error (_("\"%ps\" is not an object file: %s"),
 	   styled_string (file_name_style.style (), filename.get ()),
 	   bfd_errmsg (bfd_get_error ()));
@@ -2564,7 +2564,7 @@ reread_symbols (int from_tty)
 	  std::string original_name = objfile.original_name;
 
 	  /* bfd_openr sets cacheable to true, which is what we want.  */
-	  if (!bfd_check_format (objfile.obfd.get (), bfd_object))
+	  if (!gdb_bfd_check_format (objfile.obfd.get (), bfd_object))
 	    error (_("Can't read symbols from %ps: %s."),
 		   styled_string (file_name_style.style (),
 				  objfile_name (&objfile)),
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index a284438bd36..d9e924f16c6 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -699,7 +699,7 @@ windows_make_so (const char *name, LPVOID load_addr)
       if (abfd == NULL)
 	return so;
 
-      if (bfd_check_format (abfd.get (), bfd_object))
+      if (gdb_bfd_check_format (abfd.get (), bfd_object))
 	text = bfd_get_section_by_name (abfd.get (), ".text");
 
       if (!text)
-- 
2.51.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/2] [gdb/symtab] Fix data race in try_open_dwop_file
  2026-05-27 19:02 [PATCH 0/2] [gdb] TSAN fixes Tom de Vries
  2026-05-27 19:02 ` [PATCH 1/2] [gdb] Add gdb_bfd_check_format and gdb_bfd_check_format_matches Tom de Vries
@ 2026-05-27 19:02 ` Tom de Vries
  2026-05-27 19:03 ` [PATCH 0/2] [gdb] TSAN fixes Tom de Vries
  2026-05-28 16:37 ` Tom Tromey
  3 siblings, 0 replies; 5+ messages in thread
From: Tom de Vries @ 2026-05-27 19:02 UTC (permalink / raw)
  To: gdb-patches

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] [gdb] TSAN fixes
  2026-05-27 19:02 [PATCH 0/2] [gdb] TSAN fixes Tom de Vries
  2026-05-27 19:02 ` [PATCH 1/2] [gdb] Add gdb_bfd_check_format and gdb_bfd_check_format_matches Tom de Vries
  2026-05-27 19:02 ` [PATCH 2/2] [gdb/symtab] Fix data race in try_open_dwop_file Tom de Vries
@ 2026-05-27 19:03 ` Tom de Vries
  2026-05-28 16:37 ` Tom Tromey
  3 siblings, 0 replies; 5+ messages in thread
From: Tom de Vries @ 2026-05-27 19:03 UTC (permalink / raw)
  To: gdb-patches

On 5/27/26 9:02 PM, Tom de Vries wrote:
> I build gdb with TSAN at -O0, ran the testsuite and found a few problems.
> 
> This series fixes those.
> 
> The first patch is a refactoring patch.
> 
> The second patch removes a local lock in try_open_dwop_file.
>

I forgot to mention: this is v3.

Thanks,
- Tom


> Changes since v1:
> - commited an approved patch [1]
> - combined two patches related to the local lock in try_open_dwop_file
> - replaced last patch by new patch changing the locking scheme
> 
> Changes since v2:
> - committed another approved patch [2]
> - use gdb_bfd_check_format consistently instead of just once
> - add gdb_bfd_check_format_matches, addressing the same issue as for
>    gdb_bfd_check_format
> - factor out refactoring patch, to make it easier to review
> 
> [1] https://sourceware.org/pipermail/gdb-patches/2026-January/224422.html
> [2] https://sourceware.org/pipermail/gdb-patches/2026-February/225028.html
> 
> Versions:
> - v1 https://sourceware.org/pipermail/gdb-patches/2026-January/224424.html
> - v2 https://sourceware.org/pipermail/gdb-patches/2026-February/225027.html
> 
> Tom de Vries (2):
>    [gdb] Add gdb_bfd_check_format and gdb_bfd_check_format_matches
>    [gdb/symtab] Fix data race in try_open_dwop_file
> 
>   gdb/build-id.c                    |  4 ++--
>   gdb/cli/cli-dump.c                |  2 +-
>   gdb/compile/compile-object-load.c |  2 +-
>   gdb/corelow.c                     |  8 ++++----
>   gdb/dwarf2/dwz.c                  |  2 +-
>   gdb/dwarf2/read.c                 | 15 ++++-----------
>   gdb/exec.c                        |  2 +-
>   gdb/gdb_bfd.c                     | 24 ++++++++++++++++++++++--
>   gdb/gdb_bfd.h                     | 11 ++++++++++-
>   gdb/i386-darwin-tdep.c            |  2 +-
>   gdb/jit.c                         |  2 +-
>   gdb/machoread.c                   |  6 +++---
>   gdb/minidebug.c                   |  2 +-
>   gdb/solib-aix.c                   |  6 +++---
>   gdb/solib-rocm.c                  |  2 +-
>   gdb/solib.c                       |  4 ++--
>   gdb/symfile-mem.c                 |  2 +-
>   gdb/symfile.c                     |  6 +++---
>   gdb/windows-nat.c                 |  2 +-
>   19 files changed, 63 insertions(+), 41 deletions(-)
> 
> 
> base-commit: d623dadc2c4ecd509359015ae5a1ac856ba89b05


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] [gdb] TSAN fixes
  2026-05-27 19:02 [PATCH 0/2] [gdb] TSAN fixes Tom de Vries
                   ` (2 preceding siblings ...)
  2026-05-27 19:03 ` [PATCH 0/2] [gdb] TSAN fixes Tom de Vries
@ 2026-05-28 16:37 ` Tom Tromey
  3 siblings, 0 replies; 5+ messages in thread
From: Tom Tromey @ 2026-05-28 16:37 UTC (permalink / raw)
  To: Tom de Vries; +Cc: gdb-patches

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> I build gdb with TSAN at -O0, ran the testsuite and found a few problems.
Tom> This series fixes those.

Tom> The first patch is a refactoring patch.

Tom> The second patch removes a local lock in try_open_dwop_file.

Thanks.  I think this is ok.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-05-28 16:38 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-05-27 19:02 [PATCH 0/2] [gdb] TSAN fixes Tom de Vries
2026-05-27 19:02 ` [PATCH 1/2] [gdb] Add gdb_bfd_check_format and gdb_bfd_check_format_matches Tom de Vries
2026-05-27 19:02 ` [PATCH 2/2] [gdb/symtab] Fix data race in try_open_dwop_file Tom de Vries
2026-05-27 19:03 ` [PATCH 0/2] [gdb] TSAN fixes Tom de Vries
2026-05-28 16:37 ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox