From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2126) id CF04C385E036; Sat, 9 Mar 2024 00:27:39 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org CF04C385E036 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1709944059; bh=FKvHwLNazeRA0GFOWsRc9MLzakkO0p2dwYfTVrr6nkY=; h=From:To:Subject:Date:From; b=kcZwYbKooBjCX72LzCt1mLKDAs31pYpB5mn7GAkpYorDsZZJI/Cr4+xbspD4XrDuI igUgRA7byKnGbbe3l7yLeEi6oUFDFVEsQXYEss1CRqmPJEs3XOl4t1imcDEb5kJUaZ sBIc2aIakCQhjq55loBS1qLM0s2g+yhp5YvfQl4Y= 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] Avoid race when writing to index cache X-Act-Checkin: binutils-gdb X-Git-Author: Tom Tromey X-Git-Refname: refs/heads/master X-Git-Oldrev: ba9583c7d598df78c45b86073cdbf1a9d7c40377 X-Git-Newrev: ed29a346be439466ff2a5ce33e715e02c49fbdac Message-Id: <20240309002739.CF04C385E036@sourceware.org> Date: Sat, 9 Mar 2024 00:27:39 +0000 (GMT) List-Id: https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3Ded29a346be43= 9466ff2a5ce33e715e02c49fbdac commit ed29a346be439466ff2a5ce33e715e02c49fbdac Author: Tom Tromey Date: Sun Jan 28 09:14:04 2024 -0700 Avoid race when writing to index cache =20 The background DWARF reader changes introduced a race when writing to the index cache. The problem here is that constructing the index_cache_store_context object should only happen on the main thread, to ensure that the various value captures do not race. =20 This patch adds an assert to the construct to that effect, and then arranges for this object to be constructed by the cooked_index_worker constructor -- which is only invoked on the main thread. =20 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=3D31262 Diff: --- gdb/dwarf2/cooked-index.c | 27 ++++++++++++--------------- gdb/dwarf2/cooked-index.h | 15 ++++++++++----- gdb/dwarf2/index-cache.c | 4 ++++ 3 files changed, 26 insertions(+), 20 deletions(-) diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c index 5cbee83228b..2a1ca6fd225 100644 --- a/gdb/dwarf2/cooked-index.c +++ b/gdb/dwarf2/cooked-index.c @@ -578,6 +578,15 @@ cooked_index_worker::set (cooked_state desired_state) #endif /* CXX_STD_THREAD */ } =20 +/* See cooked-index.h. */ + +void +cooked_index_worker::write_to_cache (const cooked_index *idx) const +{ + if (idx !=3D nullptr) + m_cache_store.store (); +} + cooked_index::cooked_index (dwarf2_per_objfile *per_objfile, std::unique_ptr &&worker) : m_state (std::move (worker)), @@ -621,17 +630,16 @@ cooked_index::set_contents (vec_type &&vec) =20 m_state->set (cooked_state::MAIN_AVAILABLE); =20 - index_cache_store_context ctx (global_index_cache, m_per_bfd); - /* This is run after finalization is done -- but not before. If this task were submitted earlier, it would have to wait for 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, ctx =3D std::move (ctx)] () + gdb::task_group finalizers ([this] () { m_state->set (cooked_state::FINALIZED); - maybe_write_index (ctx); + m_state->write_to_cache (index_for_writing ()); + m_state->set (cooked_state::CACHE_DONE); }); =20 for (auto &idx : m_vector) @@ -836,17 +844,6 @@ cooked_index::dump (gdbarch *arch) } } =20 -void -cooked_index::maybe_write_index (const index_cache_store_context &ctx) -{ - if (index_for_writing () !=3D nullptr) - { - /* (maybe) store an index in the cache. */ - ctx.store (); - } - m_state->set (cooked_state::CACHE_DONE); -} - /* Wait for all the index cache entries to be written before gdb exits. */ static void diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h index 17659df0531..91419b79edc 100644 --- a/gdb/dwarf2/cooked-index.h +++ b/gdb/dwarf2/cooked-index.h @@ -467,7 +467,8 @@ class cooked_index_worker public: =20 explicit cooked_index_worker (dwarf2_per_objfile *per_objfile) - : m_per_objfile (per_objfile) + : m_per_objfile (per_objfile), + m_cache_store (global_index_cache, per_objfile->per_bfd) { } virtual ~cooked_index_worker () { } @@ -486,10 +487,15 @@ public: =20 protected: =20 - /* Let cooked_index call the 'set' method. */ + /* Let cooked_index call the 'set' and 'write_to_cache' methods. */ friend class cooked_index; + + /* Set the current state. */ void set (cooked_state desired_state); =20 + /* Write to the index cache. */ + void write_to_cache (const cooked_index *idx) const; + /* Helper function that does the work of reading. This must be able to be run in a worker thread without problems. */ virtual void do_reading () =3D 0; @@ -534,6 +540,8 @@ protected: scanning is stopped and this exception will later be reported by the 'wait' method. */ std::optional m_failed; + /* An object used to write to the index cache. */ + index_cache_store_context m_cache_store; }; =20 /* The main index of DIEs. @@ -671,9 +679,6 @@ public: =20 private: =20 - /* Maybe write the index to the index cache. */ - void maybe_write_index (const index_cache_store_context &); - /* The vector of cooked_index objects. This is stored because the entries are stored on the obstacks in those objects. */ vec_type m_vector; diff --git a/gdb/dwarf2/index-cache.c b/gdb/dwarf2/index-cache.c index 780d2c4f200..d9047ef46bb 100644 --- a/gdb/dwarf2/index-cache.c +++ b/gdb/dwarf2/index-cache.c @@ -33,6 +33,7 @@ #include "gdbsupport/selftest.h" #include #include +#include "run-on-main-thread.h" =20 /* When set to true, show debug messages about the index cache. */ static bool debug_index_cache =3D false; @@ -94,6 +95,9 @@ index_cache_store_context::index_cache_store_context (con= st index_cache &ic, m_dir (ic.m_dir), m_per_bfd (per_bfd) { + /* Capturing globals may only be done on the main thread. */ + gdb_assert (is_main_thread ()); + if (!m_enabled) return;