Introduce thread-safe handling for complaints
authorTom Tromey <tom@tromey.com>
Mon, 6 Sep 2021 16:20:02 +0000 (10:20 -0600)
committerTom Tromey <tom@tromey.com>
Tue, 12 Apr 2022 15:31:16 +0000 (09:31 -0600)
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'.

gdb/complaints.c
gdb/complaints.h

index 122f2c2810d4404ff64635abb994c76151050d7b..7e39f241938496cfbd9901930a87a1c27807d454 100644 (file)
@@ -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)
index e3d5d4474e720ab38c6c37651bb5124128ae9857..09d8cd39b276dad190d98a9d8f01f45c08fdffee 100644 (file)
@@ -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) */