Capture warnings when writing to the index cache

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.

This is v2 of the patch.  It is rebased on top of some other changes
in the same area.  v1 was here:

    https://sourceware.org/pipermail/gdb-patches/2024-February/206595.html

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

diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index 2a1ca6fd225d42fb29c36f1ee869ea4a6793e351..f78b00df446e9e8b1f0df65343b2a72b287a0ebd 100644 (file)
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -581,10 +581,18 @@ cooked_index_worker::set (cooked_state desired_state)
 /* See cooked-index.h.  */
 
 void
-cooked_index_worker::write_to_cache (const cooked_index *idx) const
+cooked_index_worker::write_to_cache (const cooked_index *idx,
+                                    deferred_warnings *warn) const
 {
   if (idx != nullptr)
-    m_cache_store.store ();
+    {
+      /* 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);
+      m_cache_store.store ();
+    }
 }
 
 cooked_index::cooked_index (dwarf2_per_objfile *per_objfile,
@@ -623,7 +631,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);
@@ -635,10 +643,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] ()
+  gdb::task_group finalizers ([=] ()
   {
     m_state->set (cooked_state::FINALIZED);
-    m_state->write_to_cache (index_for_writing ());
+    m_state->write_to_cache (index_for_writing (), warn);
     m_state->set (cooked_state::CACHE_DONE);
   });
 

diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index e73bd7c73c36b99132465c492b0687f6bc6d19a7..7ff609b9d735d76cebae35071354c6401934d678 100644 (file)
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -494,7 +494,8 @@ protected:
   void set (cooked_state desired_state);
 
   /* Write to the index cache.  */
-  void write_to_cache (const cooked_index *idx) const;
+  void write_to_cache (const cooked_index *idx,
+                      deferred_warnings *warn) const;
 
   /* Helper function that does the work of reading.  This must be able
      to be run in a worker thread without problems.  */
@@ -615,8 +616,10 @@ public:
   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>;

diff --git a/gdb/dwarf2/read-debug-names.c b/gdb/dwarf2/read-debug-names.c
index 0add8040894c20781f6e297936d038013e5d712b..0d60b01f408cc427c0f654c0e3f89216ce8a54f2 100644 (file)
--- a/gdb/dwarf2/read-debug-names.c
+++ b/gdb/dwarf2/read-debug-names.c
@@ -352,7 +352,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 7442094874c0cb1554f04548934f05c8d4ee1b05..a747922a4ed4ad4c14aed8a00e8f1946216b0c54 100644 (file)
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -4924,7 +4924,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 2acd1fc4624dfc966919eb6fffa50a3245611d23..d7db1d84e2f159a71bf1a44781956d6718361ce7 100644 (file)
--- 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
   {