From patchwork Sun Feb 18 01:10:06 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 85932 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 41A903860C2B for ; Sun, 18 Feb 2024 01:14:23 +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 020CE385C6E8 for ; Sun, 18 Feb 2024 01:10:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 020CE385C6E8 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 020CE385C6E8 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=1708218618; cv=none; b=Api8RzIeqZkfiAxCL+e41Sn+39GBOhEwr0BoAbgLdgGYIo0oOnRe3FFTlGSxHVNyst96Qhbxoa0AlFhaxvQLBGbTmeX/zrBzlqmNjTqWukG6b3+kfkvagjIcCpi0EkgoDFA4cwoC/sZt/qaeZg97PReXD4z7N/eHbtcbQdXZhdc= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708218618; c=relaxed/simple; bh=4oONb1xk8uXItiSLdqU51sYfHXaLFmws+RXpAAmMtDo=; h=DKIM-Signature:From:Date:Subject:MIME-Version:Message-Id:To; b=n7UzwcKWOpweuKoDPO+CmLY+TrunyKJwEd4OKCXg1R4WeMpGNdGbeh2B+57wbpfX7jOkgroh22PLQUxggZrOr2WcxnIDtDQRcQpy8JKthQC+aMgOgnzyM0jDAmlsV/rsfcjOC73fmLZes8X4AYt1sBrRRC5ZMSBCTLaMJbQ0Nl0= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from eig-obgw-6010a.ext.cloudfilter.net ([10.0.30.248]) by cmsmtp with ESMTPS id bRcBrFDFy80oibVh1rX0kD; Sun, 18 Feb 2024 01:10:07 +0000 Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with ESMTPS id bVgzr6ShCrLNabVgzrj1Gg; Sun, 18 Feb 2024 01:10:06 +0000 X-Authority-Analysis: v=2.4 cv=MNnDm9Zl c=1 sm=1 tr=0 ts=65d158ee a=ApxJNpeYhEAb1aAlGBBbmA==:117 a=ApxJNpeYhEAb1aAlGBBbmA==:17 a=IkcTkHD0fZMA:10 a=k7vzHIieQBIA:10 a=Qbun_eYptAEA:10 a=CCpqsmhAAAAA:8 a=qFjlajEAVd0_sq2azYkA: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=Ct4IUt32FMIQHJwLuUuc9/lJNUKB/vRgfvzKP341Pn8=; b=kulzEzdae+ToBOrN8QXgRLZBTI cFBvAAuvFvyA0CzUGm/MO03MHJijsKGHc7nMgKDMKbIgEcQMHlmqqWVZjcnjTPZtaCyT3xHKIalKI IFOd8G9rV0S0XwIlZJpx6VfZz; Received: from 71-211-170-195.hlrn.qwest.net ([71.211.170.195]:49770 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 1rbVgz-003IPy-0w for gdb-patches@sourceware.org; Sat, 17 Feb 2024 18:10:05 -0700 From: Tom Tromey Date: Sat, 17 Feb 2024 18:10:06 -0700 Subject: [PATCH 5/7] Fix race in background DWARF indexer MIME-Version: 1.0 Message-Id: <20240217-dwarf-race-relocate-v1-5-d3d2d908c1e8@tromey.com> References: <20240217-dwarf-race-relocate-v1-0-d3d2d908c1e8@tromey.com> In-Reply-To: <20240217-dwarf-race-relocate-v1-0-d3d2d908c1e8@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: 71.211.170.195 X-Source-L: No X-Exim-ID: 1rbVgz-003IPy-0w X-Source: X-Source-Args: X-Source-Dir: X-Source-Sender: 71-211-170-195.hlrn.qwest.net ([192.168.0.21]) [71.211.170.195]:49770 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: MS4xfACFUsbkwWRIYLPZ59BYbJhBAYqNKS7kXNTmn4Qf64z9WQ7BNZ8CU7zBwm2QOIkwH7FxSnw3Y9k/fs2T5Y4QHFzC7y5XWrhSXiD4a8dajFaaiStA9sKU znMzKnC+WsK5eOJl8PawQ5FsF8j2oxychAnB4hDaZnWtfk4tTap5Jy/T6CDdPCV87uiT+IWOURX6c4ei5U1qjT3dFQ4mbpb8pfY= X-Spam-Status: No, score=-3022.5 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 PR gdb/31261 points out a race in the background DWARF indexer. Looking into it, the problem is in dwarf2_per_objfile::adjust, which does: CORE_ADDR baseaddr = objfile->text_section_offset (); CORE_ADDR tem = (CORE_ADDR) addr + baseaddr; tem = gdbarch_adjust_dwarf2_addr (objfile->arch (), tem); return (unrelocated_addr) (tem - baseaddr); This code looks innocuous (or at least, it did to me), but if indexing is still happening when the objfile is relocated, this causes a data race when accessing the section offsets. I considered a couple of fixes for this. A simple one is to have relocation wait until indexing is done. However, it is better to avoid any blocking like this; and I figured there is no reason for the DWARF reader to require this information... famous last words. Adjustment is needed to work around a sort of deficiency in the MIPS target, where whether a function uses the MIPS16 ABI can apparently only be determined by examining the minimal symbols. To handle this, the patch uses the new lookup_minimal_symbol_by_pc_section overload that works solely off the per-BFD object -- which only holds unrelocated data and which is effectively read-only at the time of DWARF indexing. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31261 --- gdb/arch-utils.c | 5 +++-- gdb/arch-utils.h | 3 ++- gdb/dwarf2/frame.c | 13 ++++++++++--- gdb/dwarf2/read.c | 12 ++++++------ gdb/gdbarch-gen.h | 4 ++-- gdb/gdbarch.c | 6 +++--- gdb/gdbarch_components.py | 4 ++-- gdb/mips-tdep.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++ gdb/mips-tdep.h | 2 ++ 9 files changed, 79 insertions(+), 19 deletions(-) diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c index 1faa013c16f..a5cbd3f06d2 100644 --- a/gdb/arch-utils.c +++ b/gdb/arch-utils.c @@ -222,8 +222,9 @@ default_make_symbol_special (struct symbol *sym, struct objfile *objfile) /* See arch-utils.h. */ -CORE_ADDR -default_adjust_dwarf2_addr (CORE_ADDR pc) +unrelocated_addr +default_adjust_dwarf2_addr (objfile_per_bfd_storage *per_bfd, + unrelocated_addr pc) { return pc; } diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h index 0f37aaf20f8..21cdade77b9 100644 --- a/gdb/arch-utils.h +++ b/gdb/arch-utils.h @@ -105,7 +105,8 @@ void default_make_symbol_special (struct symbol *sym, struct objfile *objfile); /* Do nothing default implementation of gdbarch_adjust_dwarf2_addr. */ -CORE_ADDR default_adjust_dwarf2_addr (CORE_ADDR pc); +unrelocated_addr default_adjust_dwarf2_addr (objfile_per_bfd_storage *per_bfd, + unrelocated_addr pc); /* Do nothing default implementation of gdbarch_adjust_dwarf2_line. */ diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c index fc6704f434e..738906a77b9 100644 --- a/gdb/dwarf2/frame.c +++ b/gdb/dwarf2/frame.c @@ -145,13 +145,17 @@ typedef std::vector dwarf2_fde_table; struct comp_unit { comp_unit (struct objfile *objf) - : abfd (objf->obfd.get ()) + : abfd (objf->obfd.get ()), + per_bfd (objf->per_bfd) { } /* Keep the bfd convenient. */ bfd *abfd; + /* Also the per-bfd. */ + objfile_per_bfd_storage *per_bfd; + /* Pointer to the .debug_frame section loaded into memory. */ const gdb_byte *dwarf_frame_buffer = nullptr; @@ -1965,14 +1969,17 @@ decode_frame_entry_1 (struct gdbarch *gdbarch, = read_encoded_value (unit, fde->cie->encoding, fde->cie->ptr_size, buf, &bytes_read, (unrelocated_addr) 0); fde->initial_location - = (unrelocated_addr) gdbarch_adjust_dwarf2_addr (gdbarch, init_addr); + = gdbarch_adjust_dwarf2_addr (gdbarch, unit->per_bfd, + (unrelocated_addr) init_addr); buf += bytes_read; ULONGEST range = read_encoded_value (unit, fde->cie->encoding & 0x0f, fde->cie->ptr_size, buf, &bytes_read, (unrelocated_addr) 0); - ULONGEST addr = gdbarch_adjust_dwarf2_addr (gdbarch, init_addr + range); + ULONGEST addr + = (ULONGEST) gdbarch_adjust_dwarf2_addr (gdbarch, unit->per_bfd, + (unrelocated_addr) (init_addr + range)); fde->address_range = addr - (ULONGEST) fde->initial_location; buf += bytes_read; diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 486be7e4921..27feae20508 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -1211,10 +1211,8 @@ dwarf2_invalid_attrib_class_complaint (const char *arg1, const char *arg2) unrelocated_addr dwarf2_per_objfile::adjust (unrelocated_addr addr) { - CORE_ADDR baseaddr = objfile->text_section_offset (); - CORE_ADDR tem = (CORE_ADDR) addr + baseaddr; - tem = gdbarch_adjust_dwarf2_addr (objfile->arch (), tem); - return (unrelocated_addr) (tem - baseaddr); + return gdbarch_adjust_dwarf2_addr (objfile->arch (), objfile->per_bfd, + addr); } /* See read.h. */ @@ -1223,8 +1221,10 @@ CORE_ADDR dwarf2_per_objfile::relocate (unrelocated_addr addr) { CORE_ADDR baseaddr = objfile->text_section_offset (); - CORE_ADDR tem = (CORE_ADDR) addr + baseaddr; - return gdbarch_adjust_dwarf2_addr (objfile->arch (), tem); + unrelocated_addr adj = gdbarch_adjust_dwarf2_addr (objfile->arch (), + objfile->per_bfd, + addr); + return (CORE_ADDR) adj + baseaddr; } /* Hash function for line_header_hash. */ diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h index 7a57bdcebe2..911547e8ef7 100644 --- a/gdb/gdbarch-gen.h +++ b/gdb/gdbarch-gen.h @@ -862,8 +862,8 @@ extern void set_gdbarch_make_symbol_special (struct gdbarch *gdbarch, gdbarch_ma code have the ISA bit set, matching line information and the symbol table. */ -typedef CORE_ADDR (gdbarch_adjust_dwarf2_addr_ftype) (CORE_ADDR pc); -extern CORE_ADDR gdbarch_adjust_dwarf2_addr (struct gdbarch *gdbarch, CORE_ADDR pc); +typedef unrelocated_addr (gdbarch_adjust_dwarf2_addr_ftype) (objfile_per_bfd_storage *per_bfd, unrelocated_addr pc); +extern unrelocated_addr gdbarch_adjust_dwarf2_addr (struct gdbarch *gdbarch, objfile_per_bfd_storage *per_bfd, unrelocated_addr pc); extern void set_gdbarch_adjust_dwarf2_addr (struct gdbarch *gdbarch, gdbarch_adjust_dwarf2_addr_ftype *adjust_dwarf2_addr); /* Adjust the address updated by a line entry in a backend-specific way. diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c index b9be3948d1e..34cd64edae1 100644 --- a/gdb/gdbarch.c +++ b/gdb/gdbarch.c @@ -3541,14 +3541,14 @@ set_gdbarch_make_symbol_special (struct gdbarch *gdbarch, gdbarch->make_symbol_special = make_symbol_special; } -CORE_ADDR -gdbarch_adjust_dwarf2_addr (struct gdbarch *gdbarch, CORE_ADDR pc) +unrelocated_addr +gdbarch_adjust_dwarf2_addr (struct gdbarch *gdbarch, objfile_per_bfd_storage *per_bfd, unrelocated_addr pc) { gdb_assert (gdbarch != NULL); gdb_assert (gdbarch->adjust_dwarf2_addr != NULL); if (gdbarch_debug >= 2) gdb_printf (gdb_stdlog, "gdbarch_adjust_dwarf2_addr called\n"); - return gdbarch->adjust_dwarf2_addr (pc); + return gdbarch->adjust_dwarf2_addr (per_bfd, pc); } void diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py index d76b820c1b5..2a5d1e544e4 100644 --- a/gdb/gdbarch_components.py +++ b/gdb/gdbarch_components.py @@ -1497,9 +1497,9 @@ sure addresses in FDE, range records, etc. referring to compressed code have the ISA bit set, matching line information and the symbol table. """, - type="CORE_ADDR", + type="unrelocated_addr", name="adjust_dwarf2_addr", - params=[("CORE_ADDR", "pc")], + params=[("objfile_per_bfd_storage *", "per_bfd"), ("unrelocated_addr", "pc")], predefault="default_adjust_dwarf2_addr", invalid=False, ) diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c index bf0b66c4b00..8364cb0d009 100644 --- a/gdb/mips-tdep.c +++ b/gdb/mips-tdep.c @@ -356,6 +356,12 @@ is_compact_addr (CORE_ADDR addr) return ((addr) & 1); } +static int +is_compact_addr (unrelocated_addr addr) +{ + return ((CORE_ADDR) addr & 1); +} + /* Return one iff ADDR denotes standard ISA code. */ static int @@ -364,6 +370,12 @@ is_mips_addr (CORE_ADDR addr) return !is_compact_addr (addr); } +static int +is_mips_addr (unrelocated_addr addr) +{ + return !is_compact_addr (addr); +} + /* Return one iff ADDR denotes MIPS16 code. */ static int @@ -388,6 +400,12 @@ unmake_compact_addr (CORE_ADDR addr) return ((addr) & ~(CORE_ADDR) 1); } +static unrelocated_addr +unmake_compact_addr (unrelocated_addr addr) +{ + return (unrelocated_addr) (to_underlying (addr) & ~(CORE_ADDR) 1); +} + /* Add the ISA (compression) bit to ADDR. */ static CORE_ADDR @@ -396,6 +414,14 @@ make_compact_addr (CORE_ADDR addr) return ((addr) | (CORE_ADDR) 1); } +/* Add the ISA (compression) bit to ADDR. */ + +static unrelocated_addr +make_compact_addr (unrelocated_addr addr) +{ + return (unrelocated_addr) (to_underlying (addr) | (CORE_ADDR) 1); +} + /* Extern version of unmake_compact_addr; we use a separate function so that unmake_compact_addr can be inlined throughout this file. */ @@ -1225,6 +1251,21 @@ mips_pc_is_mips (CORE_ADDR memaddr) return is_mips_addr (memaddr); } +int +mips_pc_is_mips (objfile_per_bfd_storage *per_bfd, unrelocated_addr memaddr) +{ + /* Flags indicating that this is a MIPS16 or microMIPS function is + stored by elfread.c in the high bit of the info field. Use this + to decide if the function is standard MIPS. Otherwise if bit 0 + of the address is clear, then this is a standard MIPS function. */ + minimal_symbol *sym + = lookup_minimal_symbol_by_pc (per_bfd, make_compact_addr (memaddr)); + if (sym != nullptr) + return msymbol_is_mips (sym); + else + return is_mips_addr (memaddr); +} + /* Tell if the program counter value in MEMADDR is in a MIPS16 function. */ int @@ -1309,6 +1350,14 @@ mips_adjust_dwarf2_addr (CORE_ADDR pc) return mips_pc_is_mips (pc) ? pc : make_compact_addr (pc); } +static unrelocated_addr +mips_adjust_dwarf2_addr (objfile_per_bfd_storage *per_bfd, + unrelocated_addr pc) +{ + pc = unmake_compact_addr (pc); + return mips_pc_is_mips (per_bfd, pc) ? pc : make_compact_addr (pc); +} + /* Recalculate the line record requested so that the resulting PC has the ISA bit set correctly, used by DWARF-2 machinery. The need for this adjustment comes from some records associated with compressed diff --git a/gdb/mips-tdep.h b/gdb/mips-tdep.h index b64f37cebbb..9dd9bf16e7c 100644 --- a/gdb/mips-tdep.h +++ b/gdb/mips-tdep.h @@ -173,6 +173,8 @@ extern CORE_ADDR mips_unmake_compact_addr (CORE_ADDR addr); /* Tell if the program counter value in MEMADDR is in a standard MIPS function. */ extern int mips_pc_is_mips (CORE_ADDR memaddr); +extern int mips_pc_is_mips (objfile_per_bfd_storage *per_bfd, + unrelocated_addr memaddr); /* Tell if the program counter value in MEMADDR is in a MIPS16 function. */