[Bug symtab/30392] [gdb/symtab] thread sanitizer data race in gdb.base/index-cache.exp

["vries at gcc dot gnu.org" <sourceware-bugzilla@sourceware.org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=30392

--- Comment #4 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Simon Marchi from comment #3)
> (In reply to Tom Tromey from comment #2)
> > The reader probably should capture the necessarily globals
> > on the main thread and stash them until the index has been
> > written.
> 
> I tried to look into it, but then gave up.
> 
> The first issue we hit (the failure Tom de Vries shows) is an unprotected
> access to global_index_cache::m_enabled.  That is easily fixed by making
> m_enabled an std::atomic<bool> (don't know if that's the right way, but it
> makes tsan happy).

This tries the idea of caputuring the value of global_index_cache::m_enabled on
cooked_index construction:
...
diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index 25635d9b72e..d331abe08fd 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -446,6 +446,8 @@ cooked_index_shard::wait (bool allow_quit) const
 cooked_index::cooked_index (vec_type &&vec)
   : m_vector (std::move (vec))
 {
+  m_global_index_cache_enabled = global_index_cache.enabled ();
+
   for (auto &idx : m_vector)
     idx->finalize ();

@@ -635,7 +637,8 @@ cooked_index::maybe_write_index (dwarf2_per_bfd *per_bfd)

   wait ();

   /* (maybe) store an index in the cache.  */
-  global_index_cache.store (per_bfd);
+  if (m_global_index_cache_enabled)
+    global_index_cache.store (per_bfd);
 }

 /* Wait for all the index cache entries to be written before gdb
diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index 0d6f3e5aa0e..e1454c97ba4 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -443,6 +443,8 @@ class cooked_index : public dwarf_scanner_base

   /* A future that tracks when the 'index_write' method is done.  */
   gdb::future<void> m_write_future;
+
+  bool m_global_index_cache_enabled;
 };

 #endif /* GDB_DWARF2_COOKED_INDEX_H */
diff --git a/gdb/dwarf2/index-cache.c b/gdb/dwarf2/index-cache.c
index 79ab706ee9d..790aa52ea02 100644
--- a/gdb/dwarf2/index-cache.c
+++ b/gdb/dwarf2/index-cache.c
@@ -91,9 +91,6 @@ index_cache::disable ()
 voide--
 index_cache::store (dwarf2_per_bfd *per_bfd)
 {
-  if (!enabled ())
-    return;
-
   /* Get build id of objfile.  */
   const bfd_build_id *build_id = build_id_bfd_get (per_bfd->obfd);
   if (build_id == nullptr)
...
and that fixes that particular data race.

-- 
You are receiving this mail because:
You are on the CC list for the bug.