From patchwork Fri Oct 20 03:31:19 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 78192 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 34EE33857703 for ; Fri, 20 Oct 2023 03:32:55 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id EA5FF3858D33 for ; Fri, 20 Oct 2023 03:32:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org EA5FF3858D33 Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=polymtl.ca Authentication-Results: sourceware.org; spf=none smtp.mailfrom=polymtl.ca ARC-Filter: OpenARC Filter v1.0.0 sourceware.org EA5FF3858D33 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=132.207.4.11 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1697772761; cv=none; b=D8XCravxPdrz9paQGf+szvYUTcgWiPQcx1zAL40XIi4UMfihl7tfdWiBZjoPKMjYYO9pv00pHtxPDU4iDgAvUr0PYccO6PmctlH1+KKE6aEtwhs6KfI9zlDRYEawE38++z3sP6cxifySGYrUdor+E68VDQQV8WCyhCdygYx03Ms= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1697772761; c=relaxed/simple; bh=tAnF3LlaUtkKvarXxX67tzNRET2OilVGtTlkdDcjCIw=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=T5aHdJWcdkKRo5mE38gpwNXXpSJWIL55Mk6BMYMeoyz6tbBDW8Vl5WYVtlgUDql13NbLMn+hwPUqAigUTFayP3EAdVj2StVtjg3WLYq9BBQpIPaJEgmZVD25Ah9XwONtM+GnoRJuc7v4GPdKod+ahlQAp/TN6vp58W93NkRIqjc= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 39K3WXvT016737 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 19 Oct 2023 23:32:38 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 39K3WXvT016737 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=polymtl.ca; s=default; t=1697772758; bh=fOmYNV7CB822Usnrmn+XmUunNqC718N2RLTb0ulNk/Y=; h=From:To:Cc:Subject:Date:From; b=YhhVawesjRG2LGxz4PMFbWNZUCkqB9uySRuNAu3S16YlcLT4lp/aZGA8mMkGcNGgB oI0l06jCBLxZirzUw1u702pYy+W1T1sx1nxveUgKGjVXkFSXZ16NoIkRxSaFYl8hdN 29RxCFT9UzEo1kyDoFBdqrnxb6OPd57dhF90/J6Y= Received: from simark.localdomain (modemcable238.237-201-24.mc.videotron.ca [24.201.237.238]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 624AA1E091; Thu, 19 Oct 2023 23:32:33 -0400 (EDT) From: Simon Marchi To: gdb-patches@sourceware.org Cc: Simon Marchi Subject: [PATCH v2] gdb: fix owner passed to remove_target_sections in clear_solib Date: Thu, 19 Oct 2023 23:31:19 -0400 Message-ID: <20231020033231.1261222-1-simon.marchi@polymtl.ca> X-Mailer: git-send-email 2.42.0 MIME-Version: 1.0 X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Fri, 20 Oct 2023 03:32:33 +0000 X-Spam-Status: No, score=-3188.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, SPF_NONE, 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 From: Simon Marchi Commit 8971d2788e7 ("gdb: link so_list using intrusive_list") introduced a bug in clear_solib. Instead of passing an `so_list *` to remove_target_sections, it passed an `so_list **`. This was not caught by the compiler, because remove_target_sections takes a `void *` as the "owner", so you can pass it any pointer and it won't complain. This happened because I previously had a patch to change the type of the disposer parameter to be a reference rather than a pointer, so had to change `so` to `&so`. When dropping that patch, I forgot to revert this bit and / or it got re-introduced when handling subsequent merge conflicts. And I didn't properly retest. Fix that, but try to make things less error prone. Make the current add_target_sections and remove_target_sections methods of program_space private, and add wrappers that are typed, with the types we expect (bfd *, objfile *, so_list *). Perhaps there is a more elegant way to do this, but I think that this is already better / safer than what we have, as it would have caught my mistake. Change-Id: I600cab5ea0408ccc5638467b760768161ca3036c --- New in v2: - Adjust call in program_space::exec_close - Rename private methods (suffix with `_1`) to avoid using them by mistake --- gdb/exec.c | 10 +++++----- gdb/progspace.c | 3 ++- gdb/progspace.h | 29 ++++++++++++++++++++++++++--- gdb/solib.c | 2 +- 4 files changed, 34 insertions(+), 10 deletions(-) base-commit: 2d1777b530d7832db5d8d7017378354c28816554 diff --git a/gdb/exec.c b/gdb/exec.c index 0f9f9d076c68..f07d9d3ef338 100644 --- a/gdb/exec.c +++ b/gdb/exec.c @@ -492,8 +492,8 @@ exec_file_attach (const char *filename, int from_tty) /* Add the executable's sections to the current address spaces' list of sections. This possibly pushes the exec_ops target. */ - current_program_space->add_target_sections (¤t_program_space->ebfd, - sections); + current_program_space->add_target_sections + (current_program_space->ebfd.get (), sections); /* Tell display code (if any) about the changed file name. */ if (deprecated_exec_file_display_hook) @@ -599,8 +599,8 @@ build_section_table (struct bfd *some_bfd) current set of target sections. */ void -program_space::add_target_sections (const void *owner, - const std::vector §ions) +program_space::add_target_sections_1 + (const void *owner, const std::vector §ions) { if (!sections.empty ()) { @@ -651,7 +651,7 @@ program_space::add_target_sections (struct objfile *objfile) OWNER must be the same value passed to add_target_sections. */ void -program_space::remove_target_sections (const void *owner) +program_space::remove_target_sections_1 (const void *owner) { gdb_assert (owner != NULL); diff --git a/gdb/progspace.c b/gdb/progspace.c index cca651fdb6f8..839707e9d71f 100644 --- a/gdb/progspace.c +++ b/gdb/progspace.c @@ -207,10 +207,11 @@ program_space::exec_close () { /* Removing target sections may close the exec_ops target. Clear ebfd before doing so to prevent recursion. */ + bfd *saved_ebfd = ebfd.get (); ebfd.reset (nullptr); ebfd_mtime = 0; - remove_target_sections (&ebfd); + remove_target_sections (saved_ebfd); exec_filename.reset (nullptr); } diff --git a/gdb/progspace.h b/gdb/progspace.h index a480f560a77a..4db544b7d2e6 100644 --- a/gdb/progspace.h +++ b/gdb/progspace.h @@ -284,12 +284,28 @@ struct program_space bool empty (); /* Remove all target sections owned by OWNER. */ - void remove_target_sections (const void *owner); + void remove_target_sections (const objfile *owner) + { remove_target_sections_1 ((const void *) owner); } + + void remove_target_sections (const shobj *owner) + { remove_target_sections_1 ((const void *) owner); } + + void remove_target_sections (const bfd *owner) + { remove_target_sections_1 ((const void *) owner); } /* Add the sections array defined by SECTIONS to the current set of target sections. */ - void add_target_sections (const void *owner, - const std::vector §ions); + void add_target_sections (const objfile *owner, + const std::vector §ions) + { add_target_sections_1 ((const void *) owner, sections); } + + void add_target_sections (const shobj *owner, + const std::vector §ions) + { add_target_sections_1 ((const void *) owner, sections); } + + void add_target_sections (const bfd *owner, + const std::vector §ions) + { add_target_sections_1 ((const void *) owner, sections); } /* Add the sections of OBJFILE to the current set of target sections. They are given OBJFILE as the "owner". */ @@ -376,6 +392,13 @@ struct program_space registry registry_fields; private: + /* Helper for the public remove_target_sections methods. */ + void remove_target_sections_1 (const void *owner); + + /* Helper for the public add_target_sections methods. */ + void add_target_sections_1 (const void *owner, + const std::vector §ions); + /* The set of target sections matching the sections mapped into this program space. Managed by both exec_ops and solib.c. */ std::vector m_target_sections; diff --git a/gdb/solib.c b/gdb/solib.c index 71970636dc58..b9fb911a8101 100644 --- a/gdb/solib.c +++ b/gdb/solib.c @@ -1195,7 +1195,7 @@ clear_solib (void) current_program_space->so_list.clear_and_dispose ([] (shobj *so) { notify_solib_unloaded (current_program_space, *so); - current_program_space->remove_target_sections (&so); + current_program_space->remove_target_sections (so); delete so; });