From 54b815ddb428944a70694e3767a0fadbdd9ca9ea Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Sat, 24 Dec 2022 08:40:48 -0700 Subject: [PATCH] Refactor complaint thread-safety approach 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 302f107..eb648c6 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 #include @@ -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 e9e8466..1626f20 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 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 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 m_saved_warning_hook; + + /* The saved value of g_complaint_interceptor. */ + scoped_restore_tmpl m_saved_complaint_interceptor; /* A helper function that is used by the 'complaint' implementation to issue a complaint. */ @@ -87,7 +103,12 @@ private: 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 e20143b..2f771d8 100644 --- a/gdb/defs.h +++ b/gdb/defs.h @@ -562,7 +562,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 a0fddbe..f1434bc 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -4930,9 +4930,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) @@ -4942,18 +4939,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::vector>; + /* 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, + complaint_collection, + std::vector>; std::vector 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 errors; cooked_index_storage thread_storage; for (; iter != end; ++iter) @@ -4969,15 +4971,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 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 d211f1b..009bf2b 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.7.4