Capture warnings when writing to the index cache

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
This commit is contained in:
Tom Tromey
2024-02-13 13:55:34 -07:00
parent bb9a951fab
commit 818ef5f413
5 changed files with 34 additions and 11 deletions

View File

@@ -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);
});

View File

@@ -494,7 +494,8 @@ protected:
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 @@ public:
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>;

View File

@@ -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 ();
}

View File

@@ -4924,7 +4924,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

View File

@@ -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
{