From: Tom Tromey <tom@tromey.com>
Date: Fri, 24 Mar 2023 21:53:22 +0000 (-0600)
Subject: Fix race in background index-cache writing
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=6f214d0f399847b13f979651c3b46befcbb42140;p=binutils-gdb.git

Fix race in background index-cache writing

Tom de Vries pointed out a bug in the index-cache background writer --
sometimes it will fail.  He also noted that it fails when the number
of worker threads is set to zero.  These turn out to be the same
problem -- the cache can't be written to until the per-BFD's
"index_table" member is set.

This patch avoids the race by rearranging the code slightly, to ensure
the cache cannot possibly be written before the member is set.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30261
---

diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index 900f13c7de9..1b1a16b1ae2 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -443,26 +443,32 @@ cooked_index_shard::wait (bool allow_quit) const
     m_future.wait ();
 }
 
-cooked_index::cooked_index (vec_type &&vec, dwarf2_per_bfd *per_bfd)
+cooked_index::cooked_index (vec_type &&vec)
   : m_vector (std::move (vec))
 {
   for (auto &idx : m_vector)
     idx->finalize ();
 
-  /* This must be set after all the finalization tasks have been
-     started, because it may call 'wait'.  */
-  m_write_future
-    = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd] ()
-      {
-	maybe_write_index (per_bfd);
-      });
-
   /* ACTIVE_VECTORS is not locked, and this assert ensures that this
      will be caught if ever moved to the background.  */
   gdb_assert (is_main_thread ());
   active_vectors.insert (this);
 }
 
+/* See cooked-index.h.  */
+
+void
+cooked_index::start_writing_index (dwarf2_per_bfd *per_bfd)
+{
+  /* This must be set after all the finalization tasks have been
+     started, because it may call 'wait'.  */
+  m_write_future
+    = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd] ()
+	{
+	  maybe_write_index (per_bfd);
+	});
+}
+
 cooked_index::~cooked_index ()
 {
   /* The 'finalize' method may be run in a different thread.  If
diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index b16b87bbb0e..f2664929e52 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -366,7 +366,7 @@ public:
      object.  */
   using vec_type = std::vector<std::unique_ptr<cooked_index_shard>>;
 
-  cooked_index (vec_type &&vec, dwarf2_per_bfd *per_bfd);
+  explicit cooked_index (vec_type &&vec);
   ~cooked_index () override;
   DISABLE_COPY_AND_ASSIGN (cooked_index);
 
@@ -429,6 +429,9 @@ public:
     m_write_future.wait ();
   }
 
+  /* Start writing to the index cache, if the user asked for this.  */
+  void start_writing_index (dwarf2_per_bfd *per_bfd);
+
 private:
 
   /* Maybe write the index to the index cache.  */
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index b362dfddb0c..8f35b973f3e 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -5147,9 +5147,13 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile)
   indexes.push_back (index_storage.release ());
   indexes.shrink_to_fit ();
 
-  cooked_index *vec = new cooked_index (std::move (indexes), per_bfd);
+  cooked_index *vec = new cooked_index (std::move (indexes));
   per_bfd->index_table.reset (vec);
 
+  /* Cannot start writing the index entry until after the
+     'index_table' member has been set.  */
+  vec->start_writing_index (per_bfd);
+
   const cooked_index_entry *main_entry = vec->get_main ();
   if (main_entry != nullptr)
     {