From: Tom Tromey <tom@tromey.com>
To: gdb-patches@sourceware.org
Subject: [PATCH v4 05/19] Refactor complaint thread-safety approach
Date: Sun, 10 Dec 2023 14:41:13 -0700 [thread overview]
Message-ID: <20231210-t-bg-dwarf-reading-v4-5-b978c32fd12f@tromey.com> (raw)
In-Reply-To: <20231210-t-bg-dwarf-reading-v4-0-b978c32fd12f@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 bcce4f4c3e4..e73a82402c9 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -539,7 +539,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 78acbd39ef1..bd45c2c9170 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::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)
@@ -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<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 d211f1b08be..009bf2b0c1c 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.43.0
next prev parent reply other threads:[~2023-12-10 21:41 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-10 21:41 [PATCH v4 00/19] Index DWARF in the background Tom Tromey
2023-12-10 21:41 ` [PATCH v4 01/19] Don't use objfile::intern in DWO code Tom Tromey
2024-01-09 18:44 ` Simon Marchi
2024-01-09 19:12 ` Tom Tromey
2024-01-09 19:51 ` Tom Tromey
2023-12-10 21:41 ` [PATCH v4 02/19] Pre-read DWZ section data Tom Tromey
2023-12-10 21:41 ` [PATCH v4 03/19] Add a couple of bfd_cache_close calls Tom Tromey
2023-12-10 21:41 ` [PATCH v4 04/19] Add thread-safety to gdb's BFD wrappers Tom Tromey
2023-12-10 21:41 ` Tom Tromey [this message]
2023-12-10 21:41 ` [PATCH v4 06/19] Add deferred_warnings parameter to read_addrmap_from_aranges Tom Tromey
2023-12-10 21:41 ` [PATCH v4 07/19] Add quick_symbol_functions::compute_main_name Tom Tromey
2023-12-10 21:41 ` [PATCH v4 08/19] Add gdb::task_group Tom Tromey
2023-12-18 14:55 ` Tom Tromey
2023-12-10 21:41 ` [PATCH v4 09/19] Move cooked_index_storage to cooked-index.h Tom Tromey
2023-12-10 21:41 ` [PATCH v4 10/19] Add "maint set dwarf synchronous" Tom Tromey
2023-12-10 21:41 ` [PATCH v4 11/19] Change how cooked index waits for threads Tom Tromey
2023-12-10 21:41 ` [PATCH v4 12/19] Do more DWARF reading in the background Tom Tromey
2023-12-10 21:41 ` [PATCH v4 13/19] Simplify the public DWARF API Tom Tromey
2023-12-10 21:41 ` [PATCH v4 14/19] Remove two quick_symbol_functions methods Tom Tromey
2023-12-10 21:41 ` [PATCH v4 15/19] Change current_language to be a macro Tom Tromey
2023-12-10 21:41 ` [PATCH v4 16/19] Lazy language setting Tom Tromey
2023-12-10 21:41 ` [PATCH v4 17/19] Optimize lookup_minimal_symbol_text Tom Tromey
2023-12-10 21:41 ` [PATCH v4 18/19] Avoid language-based lookups in startup path Tom Tromey
2023-12-10 21:41 ` [PATCH v4 19/19] Back out some parallel_for_each features Tom Tromey
2024-01-09 1:08 ` [PATCH v4 00/19] Index DWARF in the background Tom Tromey
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231210-t-bg-dwarf-reading-v4-5-b978c32fd12f@tromey.com \
--to=tom@tromey.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).