From: Tom Tromey <tromey@adacore.com>
To: gdb-patches@sourceware.org
Cc: Tom Tromey <tromey@adacore.com>
Subject: [PATCH v2] Capture warnings when writing to the index cache
Date: Tue, 12 Mar 2024 08:40:33 -0600 [thread overview]
Message-ID: <20240312144033.77496-1-tromey@adacore.com> (raw)
PR symtab/30837 points out a race that can occur when writing to the
index cache: a call to ada_encode can cause a warning, which is
forbidden on a worker thread.
This patch fixes the problem by arranging to capture any such
warnings.
This is v2 of the patch. It is rebased on top of some other changes
in the same area. v1 was here:
https://sourceware.org/pipermail/gdb-patches/2024-February/206595.html
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30837
---
gdb/dwarf2/cooked-index.c | 18 +++++++++++++-----
gdb/dwarf2/cooked-index.h | 9 ++++++---
gdb/dwarf2/read-debug-names.c | 2 +-
gdb/dwarf2/read.c | 2 +-
gdb/utils.h | 14 +++++++++++++-
5 files changed, 34 insertions(+), 11 deletions(-)
diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index 2a1ca6fd225..f78b00df446 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -581,10 +581,18 @@ cooked_index_worker::set (cooked_state desired_state)
/* See cooked-index.h. */
void
-cooked_index_worker::write_to_cache (const cooked_index *idx) const
+cooked_index_worker::write_to_cache (const cooked_index *idx,
+ deferred_warnings *warn) const
{
if (idx != nullptr)
- m_cache_store.store ();
+ {
+ /* Writing to the index cache may cause a warning to be emitted.
+ See PR symtab/30837. This arranges to capture all such
+ warnings. This is safe because we know the deferred_warnings
+ object isn't in use by any other thread at this point. */
+ scoped_restore_warning_hook defer (*warn);
+ m_cache_store.store ();
+ }
}
cooked_index::cooked_index (dwarf2_per_objfile *per_objfile,
@@ -623,7 +631,7 @@ cooked_index::wait (cooked_state desired_state, bool allow_quit)
}
void
-cooked_index::set_contents (vec_type &&vec)
+cooked_index::set_contents (vec_type &&vec, deferred_warnings *warn)
{
gdb_assert (m_vector.empty ());
m_vector = std::move (vec);
@@ -635,10 +643,10 @@ cooked_index::set_contents (vec_type &&vec)
finalization. However, that would take a slot in the global
thread pool, and if enough such tasks were submitted at once, it
would cause a livelock. */
- gdb::task_group finalizers ([this] ()
+ gdb::task_group finalizers ([=] ()
{
m_state->set (cooked_state::FINALIZED);
- m_state->write_to_cache (index_for_writing ());
+ m_state->write_to_cache (index_for_writing (), warn);
m_state->set (cooked_state::CACHE_DONE);
});
diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index 91419b79edc..19720c256db 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -494,7 +494,8 @@ class cooked_index_worker
void set (cooked_state desired_state);
/* Write to the index cache. */
- void write_to_cache (const cooked_index *idx) const;
+ void write_to_cache (const cooked_index *idx,
+ deferred_warnings *warn) const;
/* Helper function that does the work of reading. This must be able
to be run in a worker thread without problems. */
@@ -615,8 +616,10 @@ class cooked_index : public dwarf_scanner_base
void start_reading ();
/* Called by cooked_index_worker to set the contents of this index
- and transition to the MAIN_AVAILABLE state. */
- void set_contents (vec_type &&vec);
+ and transition to the MAIN_AVAILABLE state. WARN is used to
+ collect any warnings that may arise when writing to the
+ cache. */
+ void set_contents (vec_type &&vec, deferred_warnings *warn);
/* A range over a vector of subranges. */
using range = range_chain<cooked_index_shard::range>;
diff --git a/gdb/dwarf2/read-debug-names.c b/gdb/dwarf2/read-debug-names.c
index 0add8040894..0d60b01f408 100644
--- a/gdb/dwarf2/read-debug-names.c
+++ b/gdb/dwarf2/read-debug-names.c
@@ -352,7 +352,7 @@ cooked_index_debug_names::do_reading ()
cooked_index *table
= (gdb::checked_static_cast<cooked_index *>
(per_bfd->index_table.get ()));
- table->set_contents (std::move (indexes));
+ table->set_contents (std::move (indexes), &m_warnings);
bfd_thread_cleanup ();
}
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 4afb026b8ce..390224dfe1c 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -4923,7 +4923,7 @@ cooked_index_debug_info::done_reading ()
cooked_index *table
= (gdb::checked_static_cast<cooked_index *>
(per_bfd->index_table.get ()));
- table->set_contents (std::move (indexes));
+ table->set_contents (std::move (indexes), &m_warnings);
}
void
diff --git a/gdb/utils.h b/gdb/utils.h
index 2acd1fc4624..d7db1d84e2f 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -21,6 +21,7 @@
#include "exceptions.h"
#include "gdbsupport/array-view.h"
+#include "gdbsupport/function-view.h"
#include "gdbsupport/scoped_restore.h"
#include <chrono>
@@ -374,7 +375,7 @@ assign_return_if_changed (T &lval, const T &val)
}
/* A function that can be used to intercept warnings. */
-typedef void (*warning_hook_handler) (const char *, va_list);
+typedef gdb::function_view<void (const char *, va_list)> warning_hook_handler;
/* Set the thread-local warning hook, and restore the old value when
finished. */
@@ -439,6 +440,17 @@ struct deferred_warnings
m_warnings.emplace_back (std::move (msg));
}
+ /* A variant of 'warn' so that this object can be used as a warning
+ hook; see scoped_restore_warning_hook. Note that no locking is
+ done, so users have to be careful to only install this into a
+ single thread at a time. */
+ void operator() (const char *format, va_list args) ATTRIBUTE_PRINTF (2, 0)
+ {
+ string_file msg (m_can_style);
+ msg.vprintf (format, args);
+ m_warnings.emplace_back (std::move (msg));
+ }
+
/* Emit all warnings. */
void emit () const
{
--
2.43.0
next reply other threads:[~2024-03-12 14:41 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-12 14:40 Tom Tromey [this message]
2024-03-26 15:49 ` Tom Tromey
2024-03-27 3:08 ` Simon Marchi
2024-03-27 13:25 ` Tom Tromey
2024-03-27 14:39 ` Simon Marchi
2024-04-08 4:23 ` Simon Marchi
2024-04-08 12:59 ` Tom Tromey
2024-04-09 1:33 ` Simon Marchi
2024-04-09 15:20 ` 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=20240312144033.77496-1-tromey@adacore.com \
--to=tromey@adacore.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