From patchwork Fri Mar 24 21:55:28 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 66872 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 987553858426 for ; Fri, 24 Mar 2023 21:55:53 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from gproxy2-pub.mail.unifiedlayer.com (gproxy2-pub.mail.unifiedlayer.com [69.89.18.3]) by sourceware.org (Postfix) with ESMTPS id 61F483858D28 for ; Fri, 24 Mar 2023 21:55:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 61F483858D28 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=tromey.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tromey.com Received: from cmgw12.mail.unifiedlayer.com (unknown [10.0.90.127]) by progateway4.mail.pro1.eigbox.com (Postfix) with ESMTP id A369610043B48 for ; Fri, 24 Mar 2023 21:55:37 +0000 (UTC) Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with ESMTP id fpNppQjkmBdWBfpNppPAg6; Fri, 24 Mar 2023 21:55:37 +0000 X-Authority-Reason: nr=8 X-Authority-Analysis: v=2.4 cv=Yfh4Wydf c=1 sm=1 tr=0 ts=641e1c59 a=ApxJNpeYhEAb1aAlGBBbmA==:117 a=ApxJNpeYhEAb1aAlGBBbmA==:17 a=dLZJa+xiwSxG16/P+YVxDGlgEgI=:19 a=k__wU0fu6RkA:10:nop_rcvd_month_year a=Qbun_eYptAEA:10:endurance_base64_authed_username_1 a=CCpqsmhAAAAA:8 a=NQkP0NwDUGFq1Em9Jq8A:9 a=ul9cdbp4aOFLsgKbc677:22 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tromey.com; s=default; h=Content-Transfer-Encoding:MIME-Version:Message-Id:Date:Subject: Cc:To:From:Sender:Reply-To:Content-Type:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: In-Reply-To:References:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=j+aJh4FBXtS421SPo/HuMeEJEilLywJpgqFY+anuxC8=; b=nfEmUjpqD+V4Dnv6PWn+TW2N03 rhfsYgrGbntSyZx1uFzgu05lKxKA+mTpjEKcoef6V8d3uORKbbwA7H413DKdoEd2zv390MWixhMyg eTq/JY9BqkVLFUzjHMy88dmnR; Received: from 71-211-185-113.hlrn.qwest.net ([71.211.185.113]:35632 helo=localhost.localdomain) by box5379.bluehost.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1pfpNp-003SC7-Dy; Fri, 24 Mar 2023 15:55:37 -0600 From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [PATCH] Fix race in background index-cache writing Date: Fri, 24 Mar 2023 15:55:28 -0600 Message-Id: <20230324215528.891455-1-tom@tromey.com> X-Mailer: git-send-email 2.39.1 MIME-Version: 1.0 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: 71.211.185.113 X-Source-L: No X-Exim-ID: 1pfpNp-003SC7-Dy X-Source: X-Source-Args: X-Source-Dir: X-Source-Sender: 71-211-185-113.hlrn.qwest.net (localhost.localdomain) [71.211.185.113]:35632 X-Source-Auth: tom+tromey.com X-Email-Count: 2 X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTM3OS5ibHVlaG9zdC5jb20= X-Local-Domain: yes X-Spam-Status: No, score=-3025.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, JMQ_SPF_NEUTRAL, RCVD_IN_ABUSEAT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP 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.29 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 Sender: "Gdb-patches" Tom de Vries pointed out a bug in the index-cache background writer -- sometimes it will fail. He also noted that it fails when the number of worker threads is set to zero. These turn out to be the same problem -- the cache can't be written to until the per-BFD's "index_table" member is set. This patch avoids the race by rearranging the code slightly, to ensure the cache cannot possibly be written before the member is set. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30261 --- gdb/dwarf2/cooked-index.c | 24 +++++++++++++++--------- gdb/dwarf2/cooked-index.h | 5 ++++- gdb/dwarf2/read.c | 6 +++++- 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c index 3a907690906..167beb2bf0e 100644 --- a/gdb/dwarf2/cooked-index.c +++ b/gdb/dwarf2/cooked-index.c @@ -442,26 +442,32 @@ cooked_index_shard::wait (bool allow_quit) const m_future.wait (); } -cooked_index::cooked_index (vec_type &&vec, dwarf2_per_bfd *per_bfd) +cooked_index::cooked_index (vec_type &&vec) : m_vector (std::move (vec)) { for (auto &idx : m_vector) idx->finalize (); - /* This must be set after all the finalization tasks have been - started, because it may call 'wait'. */ - m_write_future - = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd] () - { - maybe_write_index (per_bfd); - }); - /* ACTIVE_VECTORS is not locked, and this assert ensures that this will be caught if ever moved to the background. */ gdb_assert (is_main_thread ()); active_vectors.insert (this); } +/* See cooked-index.h. */ + +void +cooked_index::start_writing_index (dwarf2_per_bfd *per_bfd) +{ + /* This must be set after all the finalization tasks have been + started, because it may call 'wait'. */ + m_write_future + = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd] () + { + maybe_write_index (per_bfd); + }); +} + cooked_index::~cooked_index () { /* The 'finalize' method may be run in a different thread. If diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h index b16b87bbb0e..f2664929e52 100644 --- a/gdb/dwarf2/cooked-index.h +++ b/gdb/dwarf2/cooked-index.h @@ -366,7 +366,7 @@ class cooked_index : public dwarf_scanner_base object. */ using vec_type = std::vector>; - cooked_index (vec_type &&vec, dwarf2_per_bfd *per_bfd); + explicit cooked_index (vec_type &&vec); ~cooked_index () override; DISABLE_COPY_AND_ASSIGN (cooked_index); @@ -429,6 +429,9 @@ class cooked_index : public dwarf_scanner_base m_write_future.wait (); } + /* Start writing to the index cache, if the user asked for this. */ + void start_writing_index (dwarf2_per_bfd *per_bfd); + private: /* Maybe write the index to the index cache. */ diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index c910be875a3..dbf323d0da0 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -5147,9 +5147,13 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile) indexes.push_back (index_storage.release ()); indexes.shrink_to_fit (); - cooked_index *vec = new cooked_index (std::move (indexes), per_bfd); + cooked_index *vec = new cooked_index (std::move (indexes)); per_bfd->index_table.reset (vec); + /* Cannot start writing the index entry until after the + 'index_table' member has been set. */ + vec->start_writing_index (per_bfd); + const cooked_index_entry *main_entry = vec->get_main (); if (main_entry != nullptr) {