From patchwork Sun Oct 29 17:35:20 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 78680 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 CC0733864821 for ; Sun, 29 Oct 2023 17:39:09 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from omta40.uswest2.a.cloudfilter.net (omta40.uswest2.a.cloudfilter.net [35.89.44.39]) by sourceware.org (Postfix) with ESMTPS id E8C9E3861856 for ; Sun, 29 Oct 2023 17:38:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E8C9E3861856 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 E8C9E3861856 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=35.89.44.39 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698601112; cv=none; b=EndI1eZEMXgMUI5gkFGGjQ9epYQ2i6v5KzupjBxLhZ6Yds2D3YPQFoTjCn+0saGiiLCqqMsDJRPzHxBbEOwDkUHcKlnliGGUqTfC4Mrpjg/zMhO4653opaPkVhS8JVL0Cllk3GiwCZBTqfn+eX+bvhIUSfR9Embg+FC9EcHcpEE= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698601112; c=relaxed/simple; bh=nqNy+K9nVf2BC16pbne6uQyVbaJmU1s3c8E83+fyoXU=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=P0ZrfdUrMypkORBKbgZxaWtjlPWHTcFME+5jsrl/L3v8LxsOBln91NKO62JFyBa0DE63LQRAmWbJSU2cZcvBNSM3yvvy8XXLMxHFpmSymi7SW8QH1+ToFxf7PUQQnxhlAhdtrUQYRfmb4+g5qrFe4KszPEJekQidMxlEZeBWrIE= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from eig-obgw-5009a.ext.cloudfilter.net ([10.0.29.176]) by cmsmtp with ESMTPS id wrLfqrGGLL9Agx9k1qcQfr; Sun, 29 Oct 2023 17:38:25 +0000 Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with ESMTPS id x9k0qDJuWI9gux9k0qss0z; Sun, 29 Oct 2023 17:38:25 +0000 X-Authority-Analysis: v=2.4 cv=Ds1FRUz+ c=1 sm=1 tr=0 ts=653e9891 a=ApxJNpeYhEAb1aAlGBBbmA==:117 a=ApxJNpeYhEAb1aAlGBBbmA==:17 a=OWjo9vPv0XrRhIrVQ50Ab3nP57M=:19 a=dLZJa+xiwSxG16/P+YVxDGlgEgI=:19 a=bhdUkHdE2iEA:10 a=Qbun_eYptAEA:10 a=29SN2e_Ic2r5L5NX2uIA:9 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tromey.com; s=default; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: 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:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=TFX1E47Le8Gw09yc1kN+VFRAnvb844T/PBAKfUESCuw=; b=l8f5m1eqQyOd0iIRld6JWiHXxH aGYjiuV1Ztah1Em/0As6f8oFAsLF3EGQQdYYagBOKteQxblZgRwUL/tviO8rwg0+hXWCRvZzl0s72 UGVn2C4GWnwlS86fzWD+7O4Dz; Received: from 97-122-77-73.hlrn.qwest.net ([97.122.77.73]:47464 helo=localhost.localdomain) by box5379.bluehost.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96.2) (envelope-from ) id 1qx9k0-000nk4-0k; Sun, 29 Oct 2023 11:38:24 -0600 From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [PATCH 01/15] Add thread-safety to gdb's BFD wrappers Date: Sun, 29 Oct 2023 11:35:20 -0600 Message-ID: <20231029173839.471514-2-tom@tromey.com> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20231029173839.471514-1-tom@tromey.com> References: <20231029173839.471514-1-tom@tromey.com> 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: 97.122.77.73 X-Source-L: No X-Exim-ID: 1qx9k0-000nk4-0k X-Source: X-Source-Args: X-Source-Dir: X-Source-Sender: 97-122-77-73.hlrn.qwest.net (localhost.localdomain) [97.122.77.73]:47464 X-Source-Auth: tom+tromey.com X-Email-Count: 2 X-Org: HG=bhshared;ORG=bluehost; X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTM3OS5ibHVlaG9zdC5jb20= X-Local-Domain: yes X-CMAE-Envelope: MS4xfA4HEBi4i8R6CEoGfjETqY5/40ZY78Lja8Y4MPSht/FZveFPB5Eaf0Nj+XQL1ZKPD5H/Ompbcarxsz6Cqfxd6sGxtE/NaWo7WDp5DVto0snBjB3D0uDJ ffHoitQVJ7KoBRjyfgX3XnA3QyecubicD0NGSV7UfzE+QIkeAjzlFTFd4Q3N4U64/YcuBooWvyEq6GI4KUy4uJiHh7G/Jb6jG+M= X-Spam-Status: No, score=-3024.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, JMQ_SPF_NEUTRAL, RCVD_IN_DNSWL_LOW, 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.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 This changes gdb to ensure that gdb's BFD cache is guarded by a lock. This avoids any races when multiple threads might open a BFD (and thus use the BFD cache) at the same time. Currently, this change is not needed because the the main thread waits for some DWARF scanning to be completed before returning. The only locking that's required is when opening DWO files, and there's a local lock to this end in dwarf2/read.c. However, in the coming patches, the DWARF reader will begin its work earlier, in the background. This means there is the potential for the DWARF reader and other code on the main thread to both attempt to open BFDs at the same time. --- gdb/dwarf2/read.c | 3 +-- gdb/gdb_bfd.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++ gdb/gdb_bfd.h | 5 +++++ gdb/main.c | 3 +-- 4 files changed, 60 insertions(+), 4 deletions(-) diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 3b3daf28965..088f01d807d 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -3936,8 +3936,7 @@ static struct dwo_unit * lookup_dwo_unit (dwarf2_cu *cu, die_info *comp_unit_die, const char *dwo_name) { #if CXX_STD_THREAD - /* We need a lock here both to handle the DWO hash table, and BFD, - which is not thread-safe. */ + /* We need a lock here both to handle the DWO hash table. */ static std::mutex dwo_lock; std::lock_guard guard (dwo_lock); diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c index 56a4c5ecc91..5f100a50fa5 100644 --- a/gdb/gdb_bfd.c +++ b/gdb/gdb_bfd.c @@ -35,6 +35,34 @@ #include "cli/cli-style.h" #include +#if CXX_STD_THREAD + +#include + +/* Lock held when doing BFD operations. A recursive mutex is used + because we use this mutex internally and also for BFD, just to make + life a bit simpler, and we may sometimes hold it while calling into + BFD. */ +static std::recursive_mutex gdb_bfd_mutex; + +/* BFD locking function. */ + +static void +gdb_bfd_lock () +{ + gdb_bfd_mutex.lock (); +} + +/* BFD unlocking function. */ + +static void +gdb_bfd_unlock () +{ + gdb_bfd_mutex.unlock (); +} + +#endif /* CXX_STD_THREAD */ + /* An object of this type is stored in the section's user data when mapping a section. */ @@ -498,6 +526,10 @@ gdb_bfd_open (const char *name, const char *target, int fd, name += strlen (TARGET_SYSROOT_PREFIX); } +#if CXX_STD_THREAD + std::lock_guard guard (gdb_bfd_mutex); +#endif + if (gdb_bfd_cache == NULL) gdb_bfd_cache = htab_create_alloc (1, hash_bfd, eq_bfd, NULL, xcalloc, xfree); @@ -625,6 +657,10 @@ gdb_bfd_ref (struct bfd *abfd) if (abfd == NULL) return; +#if CXX_STD_THREAD + std::lock_guard guard (gdb_bfd_mutex); +#endif + gdata = (struct gdb_bfd_data *) bfd_usrdata (abfd); bfd_cache_debug_printf ("Increase reference count on bfd %s (%s)", @@ -654,6 +690,10 @@ gdb_bfd_unref (struct bfd *abfd) if (abfd == NULL) return; +#if CXX_STD_THREAD + std::lock_guard guard (gdb_bfd_mutex); +#endif + gdata = (struct gdb_bfd_data *) bfd_usrdata (abfd); gdb_assert (gdata->refc >= 1); @@ -1171,6 +1211,19 @@ gdb_bfd_error_handler (const char *fmt, va_list ap) (*default_bfd_error_handler) (fmt, ap); } +/* See gdb_bfd.h. */ + +void +gdb_bfd_init () +{ + if (bfd_init () != BFD_INIT_MAGIC) + error (_("fatal error: libbfd ABI mismatch")); + +#if CXX_STD_THREAD + bfd_thread_init (gdb_bfd_lock, gdb_bfd_unlock); +#endif +} + void _initialize_gdb_bfd (); void _initialize_gdb_bfd () diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h index 604365b61b1..669c3a06c98 100644 --- a/gdb/gdb_bfd.h +++ b/gdb/gdb_bfd.h @@ -250,4 +250,9 @@ gdb_bfd_sections (const gdb_bfd_ref_ptr &abfd) extern std::string gdb_bfd_errmsg (bfd_error_type error_tag, char **matching); +/* A wrapper for bfd_init that also handles setting up for + multi-threading. */ + +extern void gdb_bfd_init (); + #endif /* GDB_BFD_H */ diff --git a/gdb/main.c b/gdb/main.c index 2da39f89a90..39eacd2208b 100644 --- a/gdb/main.c +++ b/gdb/main.c @@ -688,8 +688,7 @@ captured_main_1 (struct captured_main_args *context) gdb_stdtargerr = gdb_stderr; gdb_stdtargin = gdb_stdin; - if (bfd_init () != BFD_INIT_MAGIC) - error (_("fatal error: libbfd ABI mismatch")); + gdb_bfd_init (); #ifdef __MINGW32__ /* On Windows, argv[0] is not necessarily set to absolute form when