From patchwork Sun Jan 28 16:28:09 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 84847 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 62D71385842B for ; Sun, 28 Jan 2024 16:29:55 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from omta36.uswest2.a.cloudfilter.net (omta36.uswest2.a.cloudfilter.net [35.89.44.35]) by sourceware.org (Postfix) with ESMTPS id 5688A3858413 for ; Sun, 28 Jan 2024 16:28:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5688A3858413 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=tromey.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tromey.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 5688A3858413 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=35.89.44.35 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706459295; cv=none; b=iBwQGPuRuNaAia1mRq1zm5Qbs7kBtlwgK2sHoydbf4S5TC3gLwdTApRuJolFfALva7CpRxElsKWTI1vTxluXT8Knr1WkGUIbPjCMDwIDwSiB4/kQCqDSkYENXlsYPnEFzDuc2rfxdCHyBFOVu8YN3zglSV8GBoK1Ejs+Rte93tg= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706459295; c=relaxed/simple; bh=mIpto61yYSDvthiqYtFU8gp/HcySpR0Pr5vJo+hruJM=; h=DKIM-Signature:From:Date:Subject:MIME-Version:Message-Id:To; b=qYJWypxqfiVRkR+d3qHDO4vcN8LtXl1vLuZFC38GM8ZC+yxq2JBgb0ohc+TRWNn6At1PwJdt8LbX9rYLH5tjyd7rzMpj7w/o3lXPvYuBCUTsHEJpPU6q/XofjF2Prdr27GfrOACG/pAp5n15hJTUb3Q+r2E+RU3cASm8ptsCAWw= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from eig-obgw-6004a.ext.cloudfilter.net ([10.0.30.197]) by cmsmtp with ESMTPS id U3PAr2iScCF6GU80xrGEGB; Sun, 28 Jan 2024 16:28:11 +0000 Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with ESMTPS id U80wrC5Yj55BJU80wrzB0Z; Sun, 28 Jan 2024 16:28:10 +0000 X-Authority-Analysis: v=2.4 cv=QcR1A+Xv c=1 sm=1 tr=0 ts=65b6809a a=ApxJNpeYhEAb1aAlGBBbmA==:117 a=ApxJNpeYhEAb1aAlGBBbmA==:17 a=IkcTkHD0fZMA:10 a=dEuoMetlWLkA:10 a=Qbun_eYptAEA:10 a=CCpqsmhAAAAA:8 a=vRvTRJHNINYyb3rVQ-4A:9 a=QEXdDO2ut3YA:10 a=ul9cdbp4aOFLsgKbc677:22 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tromey.com; s=default; h=To:In-Reply-To:References:Message-Id:Content-Transfer-Encoding: Content-Type:MIME-Version:Subject:Date:From:Sender:Reply-To:Cc:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=bmVJFnxgJUYEq9H9+nLJgzMOs6W7yvgourUhFeKxxFU=; b=N+ATVMeoj/bkMD8y5xeMBNzWW1 B4zA5E2hJASvXx9AwmhXrMBU0M4NtCVHdyhBjVGbYn6v2X+C9XH+ERHTNAjojhUac5BVlGcfpa3Nc 70r6fqxm52NDziToj8XSQHuEZ; Received: from 97-122-68-157.hlrn.qwest.net ([97.122.68.157]:36512 helo=[192.168.0.21]) by box5379.bluehost.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96.2) (envelope-from ) id 1rU80v-003tVN-33 for gdb-patches@sourceware.org; Sun, 28 Jan 2024 09:28:09 -0700 From: Tom Tromey Date: Sun, 28 Jan 2024 09:28:09 -0700 Subject: [PATCH 5/5] Avoid race when writing to index cache MIME-Version: 1.0 Message-Id: <20240128-pr-31262-index-cache-race-v1-5-4fe53c5265e3@tromey.com> References: <20240128-pr-31262-index-cache-race-v1-0-4fe53c5265e3@tromey.com> In-Reply-To: <20240128-pr-31262-index-cache-race-v1-0-4fe53c5265e3@tromey.com> To: gdb-patches@sourceware.org X-Mailer: b4 0.12.4 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - box5379.bluehost.com X-AntiAbuse: Original Domain - sourceware.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - tromey.com X-BWhitelist: no X-Source-IP: 97.122.68.157 X-Source-L: No X-Exim-ID: 1rU80v-003tVN-33 X-Source: X-Source-Args: X-Source-Dir: X-Source-Sender: 97-122-68-157.hlrn.qwest.net ([192.168.0.21]) [97.122.68.157]:36512 X-Source-Auth: tom+tromey.com X-Email-Count: 6 X-Org: HG=bhshared;ORG=bluehost; X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTM3OS5ibHVlaG9zdC5jb20= X-Local-Domain: yes X-CMAE-Envelope: MS4xfE0zeKue1J1rRqibzfHQ7U93CNQBT3gAD1DIyUVyC4ME4ZNSCWv03Oni6WE15FMoscw7glgWSS4y65pEvkKVP3XJbjhTy7C2u7bq2yFdGt3/Y8bfRbye zak5HcWTsVXhAHw2fGb9Z4FjD2XewgfVFQCYduP8H9hYjttaHDYwaZavTwpf4HQJQic/O/Myy1fw5hOq7nRIHiRNACuIvZD5rjs= X-Spam-Status: No, score=-3022.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, JMQ_SPF_NEUTRAL, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org 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. 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. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31262 --- 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 ad9fe871552..25f3f132989 100644 --- a/gdb/dwarf2/cooked-index.c +++ b/gdb/dwarf2/cooked-index.c @@ -566,6 +566,15 @@ cooked_index_worker::set (cooked_state desired_state) #endif /* CXX_STD_THREAD */ } +/* See cooked-index.h. */ + +void +cooked_index_worker::write_to_cache (const cooked_index *idx) const +{ + if (idx != nullptr) + m_cache_store.store (); +} + cooked_index::cooked_index (dwarf2_per_objfile *per_objfile, std::unique_ptr &&worker) : m_state (std::move (worker)), @@ -609,17 +618,16 @@ cooked_index::set_contents (vec_type &&vec) m_state->set (cooked_state::MAIN_AVAILABLE); - 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 = 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); }); for (auto &idx : m_vector) @@ -824,17 +832,6 @@ cooked_index::dump (gdbarch *arch) } } -void -cooked_index::maybe_write_index (const index_cache_store_context &ctx) -{ - if (index_for_writing () != 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 fd7686205d5..a5fe479df14 100644 --- a/gdb/dwarf2/cooked-index.h +++ b/gdb/dwarf2/cooked-index.h @@ -511,7 +511,8 @@ class cooked_index_worker public: 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 () { } @@ -530,10 +531,15 @@ class cooked_index_worker protected: - /* 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); + /* 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 () = 0; @@ -578,6 +584,8 @@ class cooked_index_worker 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; }; /* The main index of DIEs. @@ -715,9 +723,6 @@ class cooked_index : public dwarf_scanner_base private: - /* 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" /* When set to true, show debug messages about the index cache. */ static bool debug_index_cache = false; @@ -94,6 +95,9 @@ index_cache_store_context::index_cache_store_context (const 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;