From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2126) id E4D3B3858416; Tue, 26 Mar 2024 15:55:44 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E4D3B3858416 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1711468544; bh=xD1792K6vPbr1YdFAR3gzNf8MHFCtCtjhtKEBkvmhb4=; h=From:To:Subject:Date:From; b=NQRKj6cSQk6/w1u2m0t7KCJ+FZfpSREI7St8J4S6PfOWu3QXuQmx1rFbjO083lYuK cMV7ARC6aKma52vX4FQuJbGfZ67SQLplcyzZvNECYfm4ChKR3+j2DptVWoe6CJpK4d 1Xiknzy3F32GIGHh01z0NdjUOiuGaCkJoWUxi4S4= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Tom Tromey To: gdb-cvs@sourceware.org Subject: [binutils-gdb] Capture warnings when writing to the index cache X-Act-Checkin: binutils-gdb X-Git-Author: Tom Tromey X-Git-Refname: refs/heads/master X-Git-Oldrev: bb9a951fab7a30cc1209c6b8b1716c13456e8b1a X-Git-Newrev: 818ef5f4137aaff3afdb52f8bbd3a4c3a9ffa28b Message-Id: <20240326155544.E4D3B3858416@sourceware.org> Date: Tue, 26 Mar 2024 15:55:44 +0000 (GMT) List-Id: https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3D818ef5f4137a= aff3afdb52f8bbd3a4c3a9ffa28b commit 818ef5f4137aaff3afdb52f8bbd3a4c3a9ffa28b Author: Tom Tromey Date: Tue Feb 13 13:55:34 2024 -0700 Capture warnings when writing to the index cache =20 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. =20 This patch fixes the problem by arranging to capture any such warnings. =20 This is v2 of the patch. It is rebased on top of some other changes in the same area. v1 was here: =20 https://sourceware.org/pipermail/gdb-patches/2024-February/206595.h= tml =20 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=3D30837 Diff: --- 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. */ =20 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 !=3D 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 (); + } } =20 cooked_index::cooked_index (dwarf2_per_objfile *per_objfile, @@ -623,7 +631,7 @@ cooked_index::wait (cooked_state desired_state, bool al= low_quit) } =20 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 =3D 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 ([=3D] () { 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); }); =20 diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h index e73bd7c73c3..7ff609b9d73 100644 --- a/gdb/dwarf2/cooked-index.h +++ b/gdb/dwarf2/cooked-index.h @@ -494,7 +494,8 @@ protected: void set (cooked_state desired_state); =20 /* 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; =20 /* 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 (); =20 /* 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); =20 /* A range over a vector of subranges. */ using range =3D range_chain; 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 =3D (gdb::checked_static_cast (per_bfd->index_table.get ())); - table->set_contents (std::move (indexes)); + table->set_contents (std::move (indexes), &m_warnings); =20 bfd_thread_cleanup (); } diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 7442094874c..a747922a4ed 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -4924,7 +4924,7 @@ cooked_index_debug_info::done_reading () cooked_index *table =3D (gdb::checked_static_cast (per_bfd->index_table.get ())); - table->set_contents (std::move (indexes)); + table->set_contents (std::move (indexes), &m_warnings); } =20 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 @@ =20 #include "exceptions.h" #include "gdbsupport/array-view.h" +#include "gdbsupport/function-view.h" #include "gdbsupport/scoped_restore.h" #include =20 @@ -374,7 +375,7 @@ assign_return_if_changed (T &lval, const T &val) } =20 /* A function that can be used to intercept warnings. */ -typedef void (*warning_hook_handler) (const char *, va_list); +typedef gdb::function_view warning_hook_hand= ler; =20 /* 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)); } =20 + /* 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 {