[binutils-gdb] Introduce thread-safe handling for complaints

[binutils-gdb] Introduce thread-safe handling for complaints

[Tom Tromey]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=da6322977928bc23b0d6a3a88af8c935fb334f3e

commit da6322977928bc23b0d6a3a88af8c935fb334f3e
Author: Tom Tromey <tom@tromey.com>
Date:   Mon Sep 6 10:20:02 2021 -0600

    Introduce thread-safe handling for complaints
    
    This introduces a new class that can be used to make the "complaint"
    code thread-safe.  Instantiating the class installs a new handler that
    collects complaints, and then prints them all when the object is
    destroyed.
    
    This approach requires some locks.  I couldn't think of a better way
    to handle this, though, because the I/O system is not thread-safe.
    
    It seemed to me that only GDB developers are likely to enable
    complaints, and because the complaint macro handle this case already
    (before any locks are required), I reasoned that any performance
    degradation that would result here would be fine.
    
    As an aside about complaints -- are they useful at all?  I just ignore
    them, myself, since mostly they seem to indicate compiler problems
    that can't be solved in the GDB world anyway.  I'd personally prefer
    them to be in a separate tool, like a hypothetical 'dwarflint'.

Diff:
---
 gdb/complaints.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 gdb/complaints.h | 33 +++++++++++++++++++++++++++
 2 files changed, 99 insertions(+), 2 deletions(-)

diff --git a/gdb/complaints.c b/gdb/complaints.c
index 122f2c2810d..7e39f241938 100644
--- a/gdb/complaints.c
+++ b/gdb/complaints.c
@@ -23,6 +23,7 @@
 #include "gdbcmd.h"
 #include "gdbsupport/selftest.h"
 #include <unordered_map>
+#include <mutex>
 
 /* Map format strings to counters.  */
 
@@ -34,6 +35,10 @@ static std::unordered_map<const char *, int> counters;
 
 int stop_whining = 0;
 
+#if CXX_STD_THREAD
+static std::mutex complaint_mutex;
+#endif /* CXX_STD_THREAD */
+
 /* See complaints.h.  */
 
 void
@@ -41,8 +46,13 @@ complaint_internal (const char *fmt, ...)
 {
   va_list args;
 
-  if (++counters[fmt] > stop_whining)
-    return;
+  {
+#if CXX_STD_THREAD
+    std::lock_guard<std::mutex> guard (complaint_mutex);
+#endif
+    if (++counters[fmt] > stop_whining)
+      return;
+  }
 
   va_start (args, fmt);
 
@@ -66,6 +76,60 @@ clear_complaints ()
   counters.clear ();
 }
 
+/* See complaints.h.  */
+
+complaint_interceptor *complaint_interceptor::g_complaint_interceptor;
+
+/* See complaints.h.  */
+
+complaint_interceptor::complaint_interceptor ()
+  : m_saved_warning_hook (deprecated_warning_hook)
+{
+  /* 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.  */
+
+static void
+wrap_warning_hook (void (*hook) (const char *, va_list), ...)
+{
+  va_list args;
+  va_start (args, hook);
+  hook ("%s", args);
+  va_end (args);
+}
+
+/* See complaints.h.  */
+
+complaint_interceptor::~complaint_interceptor ()
+{
+  for (const std::string &str : m_complaints)
+    {
+      if (m_saved_warning_hook)
+	wrap_warning_hook (m_saved_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.  */
+
+void
+complaint_interceptor::issue_complaint (const char *fmt, va_list args)
+{
+#if CXX_STD_THREAD
+  std::lock_guard<std::mutex> guard (complaint_mutex);
+#endif
+  g_complaint_interceptor->m_complaints.insert (string_vprintf (fmt, args));
+}
+
 static void
 complaints_show_value (struct ui_file *file, int from_tty,
 		       struct cmd_list_element *cmd, const char *value)
diff --git a/gdb/complaints.h b/gdb/complaints.h
index e3d5d4474e7..09d8cd39b27 100644
--- a/gdb/complaints.h
+++ b/gdb/complaints.h
@@ -21,6 +21,8 @@
 #if !defined (COMPLAINTS_H)
 #define COMPLAINTS_H
 
+#include <unordered_set>
+
 /* Helper for complaint.  */
 extern void complaint_internal (const char *fmt, ...)
   ATTRIBUTE_PRINTF (1, 2);
@@ -46,5 +48,36 @@ extern int stop_whining;
 
 extern void clear_complaints ();
 
+/* 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.  */
+
+class complaint_interceptor
+{
+public:
+
+  complaint_interceptor ();
+  ~complaint_interceptor ();
+
+  DISABLE_COPY_AND_ASSIGN (complaint_interceptor);
+
+private:
+
+  /* The issued complaints.  */
+  std::unordered_set<std::string> m_complaints;
+
+  /* The saved value of deprecated_warning_hook.  */
+  void (*m_saved_warning_hook) (const char *, va_list)
+    ATTRIBUTE_FPTR_PRINTF(1,0);
+
+  /* A helper function that is used by the 'complaint' implementation
+     to issue a complaint.  */
+  static void issue_complaint (const char *, va_list);
+
+  /* This object.  Used by the static callback function.  */
+  static complaint_interceptor *g_complaint_interceptor;
+};
 
 #endif /* !defined (COMPLAINTS_H) */