[PATCH v2 05/18] Refactor complaint thread-safety approach

[Tom Tromey <tom@tromey.com>]

This patch changes the way complaint works in a background thread.
The new approach requires installing a complaint interceptor in each
worker, and then the resulting complaints are treated as one of the
results of the computation.  This change is needed for a subsequent
patch, where installing a complaint interceptor around a parallel-for
is no longer a viable approach.
---
 gdb/complaints.c  | 24 +++++++++++-------------
 gdb/complaints.h  | 37 +++++++++++++++++++++++++++++--------
 gdb/defs.h        |  2 +-
 gdb/dwarf2/read.c | 31 ++++++++++++++++++-------------
 gdb/top.c         |  2 +-
 5 files changed, 60 insertions(+), 36 deletions(-)

diff --git a/gdb/complaints.c b/gdb/complaints.c
index 302f107b519..eb648c655ed 100644
--- a/gdb/complaints.c
+++ b/gdb/complaints.c
@@ -21,6 +21,7 @@
 #include "complaints.h"
 #include "command.h"
 #include "gdbcmd.h"
+#include "run-on-main-thread.h"
 #include "gdbsupport/selftest.h"
 #include <unordered_map>
 #include <mutex>
@@ -78,17 +79,14 @@ clear_complaints ()
 
 /* See complaints.h.  */
 
-complaint_interceptor *complaint_interceptor::g_complaint_interceptor;
+thread_local complaint_interceptor *complaint_interceptor::g_complaint_interceptor;
 
 /* See complaints.h.  */
 
 complaint_interceptor::complaint_interceptor ()
-  : m_saved_warning_hook (deprecated_warning_hook)
+  : m_saved_warning_hook (&deprecated_warning_hook, issue_complaint),
+    m_saved_complaint_interceptor (&g_complaint_interceptor, this)
 {
-  /* These cannot be stacked.  */
-  gdb_assert (g_complaint_interceptor == nullptr);
-  g_complaint_interceptor = this;
-  deprecated_warning_hook = issue_complaint;
 }
 
 /* A helper that wraps a warning hook.  */
@@ -104,19 +102,19 @@ wrap_warning_hook (void (*hook) (const char *, va_list), ...)
 
 /* See complaints.h.  */
 
-complaint_interceptor::~complaint_interceptor ()
+void
+re_emit_complaints (const complaint_collection &complaints)
 {
-  for (const std::string &str : m_complaints)
+  gdb_assert (is_main_thread ());
+
+  for (const std::string &str : complaints)
     {
-      if (m_saved_warning_hook)
-	wrap_warning_hook (m_saved_warning_hook, str.c_str ());
+      if (deprecated_warning_hook)
+	wrap_warning_hook (deprecated_warning_hook, str.c_str ());
       else
 	gdb_printf (gdb_stderr, _("During symbol reading: %s\n"),
 		    str.c_str ());
     }
-
-  g_complaint_interceptor = nullptr;
-  deprecated_warning_hook = m_saved_warning_hook;
 }
 
 /* See complaints.h.  */
diff --git a/gdb/complaints.h b/gdb/complaints.h
index e9e846684ac..1626f200685 100644
--- a/gdb/complaints.h
+++ b/gdb/complaints.h
@@ -57,29 +57,45 @@ have_complaint ()
 
 extern void clear_complaints ();
 
+/* Type of collected complaints.  */
+
+typedef std::unordered_set<std::string> complaint_collection;
+
 /* A class that can handle calls to complaint from multiple threads.
    When this is instantiated, it hooks into the complaint mechanism,
-   so the 'complaint' macro can continue to be used.  When it is
-   destroyed, it issues all the complaints that have been stored.  It
-   should only be instantiated in the main thread.  */
+   so the 'complaint' macro can continue to be used.  It collects all
+   the complaints (and warnings) emitted while it is installed, and
+   these can be retrieved before the object is destroyed.  This is
+   intended for use from worker threads (though it will also work on
+   the main thread).  Messages can be re-emitted on the main thread
+   using re_emit_complaints, see below.  */
 
 class complaint_interceptor
 {
 public:
 
   complaint_interceptor ();
-  ~complaint_interceptor ();
+  ~complaint_interceptor () = default;
 
   DISABLE_COPY_AND_ASSIGN (complaint_interceptor);
 
+  complaint_collection &&release ()
+  {
+    return std::move (m_complaints);
+  }
+
 private:
 
   /* The issued complaints.  */
-  std::unordered_set<std::string> m_complaints;
+  complaint_collection m_complaints;
+
+  typedef void (*saved_warning_hook_ftype) (const char *, va_list);
 
   /* The saved value of deprecated_warning_hook.  */
-  void (*m_saved_warning_hook) (const char *, va_list)
-    ATTRIBUTE_FPTR_PRINTF (1,0);
+  scoped_restore_tmpl<saved_warning_hook_ftype> m_saved_warning_hook;
+
+  /* The saved value of g_complaint_interceptor.  */
+  scoped_restore_tmpl<complaint_interceptor *> m_saved_complaint_interceptor;
 
   /* A helper function that is used by the 'complaint' implementation
      to issue a complaint.  */
@@ -87,7 +103,12 @@ class complaint_interceptor
     ATTRIBUTE_PRINTF (1, 0);
 
   /* This object.  Used by the static callback function.  */
-  static complaint_interceptor *g_complaint_interceptor;
+  static thread_local complaint_interceptor *g_complaint_interceptor;
 };
 
+/* Re-emit complaints that were collected by complaint_interceptor.
+   This can only be called on the main thread.  */
+
+extern void re_emit_complaints (const complaint_collection &);
+
 #endif /* !defined (COMPLAINTS_H) */
diff --git a/gdb/defs.h b/gdb/defs.h
index b8612e1ac6d..a539faa00f0 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -545,7 +545,7 @@ extern void (*deprecated_print_frame_info_listing_hook) (struct symtab * s,
 							 int noerror);
 extern int (*deprecated_query_hook) (const char *, va_list)
      ATTRIBUTE_FPTR_PRINTF(1,0);
-extern void (*deprecated_warning_hook) (const char *, va_list)
+extern thread_local void (*deprecated_warning_hook) (const char *, va_list)
      ATTRIBUTE_FPTR_PRINTF(1,0);
 extern void (*deprecated_readline_begin_hook) (const char *, ...)
      ATTRIBUTE_FPTR_PRINTF_1;
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 86fd42c3579..e1f1db4dcb3 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -4946,9 +4946,6 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile)
 			       index_storage.get_addrmap ());
 
   {
-    /* Ensure that complaints are handled correctly.  */
-    complaint_interceptor complaint_handler;
-
     using iter_type = decltype (per_bfd->all_units.begin ());
 
     auto task_size_ = [] (iter_type iter)
@@ -4958,18 +4955,23 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile)
       };
     auto task_size = gdb::make_function_view (task_size_);
 
-    /* Each thread returns a pair holding a cooked index, and a vector
-       of errors that should be printed.  The latter is done because
-       GDB's I/O system is not thread-safe.  run_on_main_thread could be
-       used, but that would mean the messages are printed after the
-       prompt, which looks weird.  */
-    using result_type = std::pair<std::unique_ptr<cooked_index_shard>,
-				  std::vector<gdb_exception>>;
+    /* Each thread returns a tuple holding a cooked index, any
+       collected complaints, and a vector of errors that should be
+       printed.  The latter is done because GDB's I/O system is not
+       thread-safe.  run_on_main_thread could be used, but that would
+       mean the messages are printed after the prompt, which looks
+       weird.  */
+    using result_type = std::tuple<std::unique_ptr<cooked_index_shard>,
+				   complaint_collection,
+				   std::vector<gdb_exception>>;
     std::vector<result_type> results
       = gdb::parallel_for_each (1, per_bfd->all_units.begin (),
 				per_bfd->all_units.end (),
 				[=] (iter_type iter, iter_type end)
       {
+	/* Ensure that complaints are handled correctly.  */
+	complaint_interceptor complaint_handler;
+
 	std::vector<gdb_exception> errors;
 	cooked_index_storage thread_storage;
 	for (; iter != end; ++iter)
@@ -4985,15 +4987,18 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile)
 		errors.push_back (std::move (except));
 	      }
 	  }
-	return result_type (thread_storage.release (), std::move (errors));
+	return result_type (thread_storage.release (),
+			    complaint_handler.release (),
+			    std::move (errors));
       }, task_size);
 
     /* Only show a given exception a single time.  */
     std::unordered_set<gdb_exception> seen_exceptions;
     for (auto &one_result : results)
       {
-	indexes.push_back (std::move (one_result.first));
-	for (auto &one_exc : one_result.second)
+	indexes.push_back (std::move (std::get<0> (one_result)));
+	re_emit_complaints (std::get<1> (one_result));
+	for (auto &one_exc : std::get<2> (one_result))
 	  if (seen_exceptions.insert (one_exc).second)
 	    exception_print (gdb_stderr, one_exc);
       }
diff --git a/gdb/top.c b/gdb/top.c
index 5028440b671..93c30c06ef3 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -221,7 +221,7 @@ int (*deprecated_query_hook) (const char *, va_list);
 
 /* Replaces most of warning.  */
 
-void (*deprecated_warning_hook) (const char *, va_list);
+thread_local void (*deprecated_warning_hook) (const char *, va_list);
 
 /* These three functions support getting lines of text from the user.
    They are used in sequence.  First deprecated_readline_begin_hook is

-- 
2.41.0