diff mbox series

Capture warnings when writing to the index cache

Message ID 20240214123220.2301600-1-tromey@adacore.com
State New
Headers
Series Capture warnings when writing to the index cache |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Tom Tromey Feb. 14, 2024, 12:32 p.m. UTC 
  PR symtab/30837 points out a race that can occur when writing to the
index cache: a call to ada_encode can cause a warning, which is
forbidden on a worker thread.

This patch fixes the problem by arranging to capture any such
warnings.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30837
---
 gdb/dwarf2/cooked-index.c     | 15 +++++++++++----
 gdb/dwarf2/cooked-index.h     |  9 ++++++---
 gdb/dwarf2/read-debug-names.c |  2 +-
 gdb/dwarf2/read.c             |  2 +-
 gdb/utils.h                   | 14 +++++++++++++-
 5 files changed, 32 insertions(+), 10 deletions(-)
diff mbox series

Patch

diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index dc8b6f08ff4..d42f7672260 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -614,7 +614,7 @@  cooked_index::wait (cooked_state desired_state, bool allow_quit)
 }
 
 void
-cooked_index::set_contents (vec_type &&vec)
+cooked_index::set_contents (vec_type &&vec, deferred_warnings *warn)
 {
   gdb_assert (m_vector.empty ());
   m_vector = std::move (vec);
@@ -628,10 +628,10 @@  cooked_index::set_contents (vec_type &&vec)
      finalization.  However, that would take a slot in the global
      thread pool, and if enough such tasks were submitted at once, it
      would cause a livelock.  */
-  gdb::task_group finalizers ([this, ctx = std::move (ctx)] ()
+  gdb::task_group finalizers ([this, warn, ctx = std::move (ctx)] ()
   {
     m_state->set (cooked_state::FINALIZED);
-    maybe_write_index (m_per_bfd, ctx);
+    maybe_write_index (m_per_bfd, ctx, warn);
   });
 
   for (auto &idx : m_vector)
@@ -838,10 +838,17 @@  cooked_index::dump (gdbarch *arch)
 
 void
 cooked_index::maybe_write_index (dwarf2_per_bfd *per_bfd,
-				 const index_cache_store_context &ctx)
+				 const index_cache_store_context &ctx,
+				 deferred_warnings *warn)
 {
   if (index_for_writing () != nullptr)
     {
+      /* Writing to the index cache may cause a warning to be
+	 emitted.  See PR symtab/30837.  This arranges to capture all
+	 such warnings.  This is safe because we know the
+	 deferred_warnings object isn't in use by any other thread at
+	 this point.  */
+      scoped_restore_warning_hook defer (*warn);
       /* (maybe) store an index in the cache.  */
       global_index_cache.store (m_per_bfd, ctx);
     }
diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index 629a5b6b9ee..4ed52d8871c 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -607,8 +607,10 @@  class cooked_index : public dwarf_scanner_base
   void start_reading ();
 
   /* Called by cooked_index_worker to set the contents of this index
-     and transition to the MAIN_AVAILABLE state.  */
-  void set_contents (vec_type &&vec);
+     and transition to the MAIN_AVAILABLE state.  WARN is used to
+     collect any warnings that may arise when writing to the
+     cache.  */
+  void set_contents (vec_type &&vec, deferred_warnings *warn);
 
   /* A range over a vector of subranges.  */
   using range = range_chain<cooked_index_shard::range>;
@@ -673,7 +675,8 @@  class cooked_index : public dwarf_scanner_base
 
   /* Maybe write the index to the index cache.  */
   void maybe_write_index (dwarf2_per_bfd *per_bfd,
-			  const index_cache_store_context &);
+			  const index_cache_store_context &ctx,
+			  deferred_warnings *warn);
 
   /* The vector of cooked_index objects.  This is stored because the
      entries are stored on the obstacks in those objects.  */
diff --git a/gdb/dwarf2/read-debug-names.c b/gdb/dwarf2/read-debug-names.c
index e2563e84fcf..e325711af3a 100644
--- a/gdb/dwarf2/read-debug-names.c
+++ b/gdb/dwarf2/read-debug-names.c
@@ -348,7 +348,7 @@  cooked_index_debug_names::do_reading ()
   cooked_index *table
     = (gdb::checked_static_cast<cooked_index *>
        (per_bfd->index_table.get ()));
-  table->set_contents (std::move (indexes));
+  table->set_contents (std::move (indexes), &m_warnings);
 
   bfd_thread_cleanup ();
 }
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 486be7e4921..4c2ea39565a 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -4916,7 +4916,7 @@  cooked_index_debug_info::done_reading ()
   cooked_index *table
     = (gdb::checked_static_cast<cooked_index *>
        (per_bfd->index_table.get ()));
-  table->set_contents (std::move (indexes));
+  table->set_contents (std::move (indexes), &m_warnings);
 }
 
 void
diff --git a/gdb/utils.h b/gdb/utils.h
index 2acd1fc4624..d7db1d84e2f 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -21,6 +21,7 @@ 
 
 #include "exceptions.h"
 #include "gdbsupport/array-view.h"
+#include "gdbsupport/function-view.h"
 #include "gdbsupport/scoped_restore.h"
 #include <chrono>
 
@@ -374,7 +375,7 @@  assign_return_if_changed (T &lval, const T &val)
 }
 
 /* A function that can be used to intercept warnings.  */
-typedef void (*warning_hook_handler) (const char *, va_list);
+typedef gdb::function_view<void (const char *, va_list)> warning_hook_handler;
 
 /* Set the thread-local warning hook, and restore the old value when
    finished.  */
@@ -439,6 +440,17 @@  struct deferred_warnings
     m_warnings.emplace_back (std::move (msg));
   }
 
+  /* A variant of 'warn' so that this object can be used as a warning
+     hook; see scoped_restore_warning_hook.  Note that no locking is
+     done, so users have to be careful to only install this into a
+     single thread at a time.  */
+  void operator() (const char *format, va_list args) ATTRIBUTE_PRINTF (2, 0)
+  {
+    string_file msg (m_can_style);
+    msg.vprintf (format, args);
+    m_warnings.emplace_back (std::move (msg));
+  }
+
   /* Emit all warnings.  */
   void emit () const
   {