From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id gb4xF4OXymn4ij8AWB0awg (envelope-from ) for ; Mon, 30 Mar 2026 11:32:19 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=YNaycvsa; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 5AD2E1E04F; Mon, 30 Mar 2026 11:32:19 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [38.145.34.32]) (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 ESMTPS id 7BA291E04F for ; Mon, 30 Mar 2026 11:32:18 -0400 (EDT) Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id 154334BB58E6 for ; Mon, 30 Mar 2026 15:32:17 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 154334BB58E6 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=YNaycvsa Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id 48DF04BA2E21 for ; Mon, 30 Mar 2026 15:31:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 48DF04BA2E21 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 48DF04BA2E21 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1774884665; cv=none; b=EzD6iZxg9ZdV2DbsOWDcxH/wTZf3zkrgi/kS0ZPfeuFnqtEqwAmZ+xCda+Z7alKyxNEEdGxX6sDxTiCZGCjtxMopH+4eqpBbLPq6EQqC3kq4Oa4dmqFD4Q5bG7eJgTWZHw6Y4H8Grte1iHDLolFfOBpPPkdVo6TJYs5d5c1EmPQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1774884665; c=relaxed/simple; bh=/Mge+0gpYuIrKMJdfWT4sUqn6h3tS2J/iPQeoaGtSvM=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=yDlTkNBq/LjPNxpPe1JUtuAeDfobN7Xne0lA+0yFXnCSa9EFkRXTDP1KPCO1YU/Px7HIlQdlIeH+9KRfdHdNHMuuurGI/tVWotrhLEXTBHPimFf4PW349MsCbDGA1YdPosmAWHALjiXnW/cNS3oAMwNhe/BGIcl88KD9Ocj6WnM= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 48DF04BA2E21 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1774884664; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=0olojZZkAntzSnzjebRdE6yviZ08NzXesC/I0sn176E=; b=YNaycvsaa+2qlYQITB5tugIdxFftVIpP2lcCpYB/FBI1tQ16pO+vQLV7iBebRLRIFTCTZ2 Lenn/Crc9acAfenJYaKhDpKe0bdxeCpoDMgyyLuR4lV65xhNxE4eJQU52eZqzYOLKZa5HH Y5SOwu95I8bbILp7DU+g0XUqOjqZb60= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-479-1thYuogcOMWnsSNhllEDCg-1; Mon, 30 Mar 2026 11:31:03 -0400 X-MC-Unique: 1thYuogcOMWnsSNhllEDCg-1 X-Mimecast-MFC-AGG-ID: 1thYuogcOMWnsSNhllEDCg_1774884662 Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-4837bfcfe0dso58571875e9.1 for ; Mon, 30 Mar 2026 08:31:03 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1774884662; x=1775489462; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=0olojZZkAntzSnzjebRdE6yviZ08NzXesC/I0sn176E=; b=QV30q3AfZM6lpWFELYuAdSUsqPMdPoNmdv82ocnG0ehcLej+xrKJDI+4dFX9w8NL8K /yUebzCkWhFw6CmZnAFz9KlrkPYtAZm+ct5+6TFIh154CWQkHlWy2W02F/6oLkz8vv1W eLNP1JWSZs3zOGDm7svQOPMaC84zsx9wHUVVG3nO1cVHjxITmrVlMVFq6rX+BWzRWaa/ fgTc7jy+jBAYxy12hPXC/7ygdCFWOWfBwK7UUshkg+z1aIe84Yk96F0AHWDIwAn6/4lp Ia1byP0LuO/4cgF/lrGEPw5jf2tbMXY5IriZbL2tC9OhRjie/zP6VgRRYS0kdR3YBxgh lO/w== X-Gm-Message-State: AOJu0YzAEbeQDslNKA6Z1MVbwz9zZbzhpOiQgBAhxxD9m/1r1WVfv22C zBn0vEMf3+73w+weRqNF+Kxk6uuUxeWdVGmRcwlKeOv1dovNs/3qUxaXA4P8yw/NrZMAjh9a6PV PW2gI7uqqYLvR0omhHWnynYy/9Obyv9pl21NkUD3PZjZulUiYX04ww6FRpmMIrsGj6IQ8bau3ND vYm7uX6xoLIu93dD+yXg/VMb/IuZHoncf2RsXXy5B0z/caQgE= X-Gm-Gg: ATEYQzz65Ir5gs8Edcy7l58zif9qEF/1WhSx4l/ZhdBHa5TACeWk3216wCD9GptX4I3 XARYBWUO6GTSOO4k2RzzWYZmHtV9KVJ1C4tYztKoV5wLgXZfq6Du5zwP3ZYb6eRqifATCR5sLfk ozgk+qgLjnFHqJGydsC0HaNVCHVdZHpqmxcAF+gG6q2f2PRh+GZrzQLtNOGz/L4IG/SzDMR+rQd 9WjoRA3CztGAgm4j1fOacGl63POK+VwZTJO2Hkxq49IMzmynAiT5vGrD1ZBchDbAXBa/KQstQyg 3f14ZyKtJ27VC9Rt8x6VgdoVkqq9FNnDMfvYUNfB/G+1Mll4LfSYeihC+B320UWajpD+Ew7F7bP dweyHn+pLISO+BMwI X-Received: by 2002:a05:600c:524f:b0:485:4535:73d with SMTP id 5b1f17b1804b1-48727d67a38mr223564065e9.2.1774884661683; Mon, 30 Mar 2026 08:31:01 -0700 (PDT) X-Received: by 2002:a05:600c:524f:b0:485:4535:73d with SMTP id 5b1f17b1804b1-48727d67a38mr223563085e9.2.1774884661032; Mon, 30 Mar 2026 08:31:01 -0700 (PDT) Received: from localhost ([31.111.84.232]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-43cf21e3602sm20302194f8f.4.2026.03.30.08.31.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 30 Mar 2026 08:31:00 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH 2/3] gdb: refactor core_target ::close and ::detach functions Date: Mon, 30 Mar 2026 16:30:52 +0100 Message-Id: X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: JEQJZguMgcFakhpn4ocsxgko2PHLq-xJCZAHK6fmU6g_1774884662 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit content-type: text/plain; charset="US-ASCII"; x-default=true 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~public-inbox=simark.ca@sourceware.org This patch refactors the core_target ::close, ::detach, and ::clear_core functions so that the m_core_bfd is not cleared before the core_target is deleted. My motivation for this change is the get_inferior_core_bfd function. This function checks to see if an inferior has a core_target in its target stack, if it does then there is an assert that the core_target's m_core_bfd will not be NULL. Currently, this assert is mostly correct, but during a call to target_detach, the assert stops being true. Calling target_detach will call core_target::detach, which calls core_target::clear_core, which sets m_core_bfd to NULL. The core_target is not unpushed from the inferior's target stack until GDB returns from ::clear_core back to ::detach. This means that, for a short period of time, from the moment m_core_bfd is set to NULL in ::clear_core, until the unpush back in ::detach, the assertion in get_inferior_core_bfd is no longer valid. Within this window we trigger the core_file_changed observer. If any of the observers call get_inferior_core_bfd then the assertion will trigger. Currently, no observer calls get_inferior_core_bfd, the observer just clears some caches. However, in the next commit I'd like to add a new Python event API for when the core file is changed. User code attached to this event can call Inferior.corefile, which is implemented by a call to get_inferior_core_bfd, and in this case the assert will trigger. I could just delete the assertion, but I'd prefer to not do that. I think by restructuring the code we can leave the assertion in place. The first thing to understand is that a core_target is not shareable, see process_stratum_target::is_shareable. This means that a core_target will only appear within a single inferior's target stack. Next there are two ways that a core_target can be removed from an inferior's target stack. First is via target_detach, this is triggered either by the 'detach' command, or by the 'core-file' command without a core filename. In both these cases target_detach is called, which calls core_target::detach, this function unpushes the core_target from the inferior's target stack. As the core_target is not shareable the reference count will return to zero, at which point the core_target will be closed and deleted. The second way that a core_target can be removed from an inferior's target_stack is by direct replacement. If a user loads a different process_stratum_target, e.g. 'target remote ....' then this replaces the core_target in the target_stack. Doing this reduces the core_target's reference count to zero, which causes the target to be closed and deleted. These two approaches differ in that the first calls core_target::detach and then core_target::close, while the second avoids calling ::detach, and immediately calls ::close. My proposal is that we can defer calling the core_file_changed observer until core_target::close. By this point the core_target will have been removed from the inferior's target_stack, and so the assert in get_inferior_core_bfd will still hold. We already call the observer at this point for the process_stratum_target replacement case (e.g. when a user does 'target remote ...' to replace a core file target), this proposal would just mean that we always call the observer at this point, rather than potentially calling it earlier in the detach case. This commit does this change by making a number of changes: * The code to reset m_core_bfd to NULL, and to trigger the core_file_changed observer, is removed from core_target::clear, this only leaves the code relating to exiting and cleaning up after the inferior that was created for inspecting the core file. * To reflect this change of focus, core_target::clear_core is renamed to core_target::exit_core_file_inferior. * In core_target::detach, nothing really needs to change other than calling ::exit_core_file_inferior. I have added an assert that reflects the fact that ::detach cannot be called twice on the same core_target (after the first call the core_target will always be closed and deleted). * In core_target::close the call to ::exit_core_file_inferior needs to be conditional. As discussed above, in the replacement case, ::close can be called without first calling ::detach. But in the target_detach case, ::detach will have already been called, and as a result ::exit_core_file_inferior will have already been called. * Also in core_target::close, we now unconditionally trigger the core_target_changed observer. This commit is a refactor, and there should be no user observable changes. --- gdb/corelow.c | 75 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 51 insertions(+), 24 deletions(-) diff --git a/gdb/corelow.c b/gdb/corelow.c index 216b4e70066..b4438990a7a 100644 --- a/gdb/corelow.c +++ b/gdb/corelow.c @@ -286,7 +286,7 @@ class core_target final : public process_stratum_target private: /* per-core data */ /* Get rid of the core inferior. */ - void clear_core (); + void exit_core_file_inferior (); /* The core's section table. Note that these target sections are *not* mapped in the current address spaces' set of target @@ -614,24 +614,23 @@ core_target::build_file_mappings () /* An arbitrary identifier for the core inferior. */ #define CORELOW_PID 1 +/* See class declaration above. */ + void -core_target::clear_core () +core_target::exit_core_file_inferior () { - if (this->core_bfd () != nullptr) - { - switch_to_no_thread (); /* Avoid confusion from thread - stuff. */ - exit_inferior (current_inferior ()); + /* Opening a core file ensures that some thread, even if it's just a + "fake" thread, will have been selected. */ + gdb_assert (inferior_ptid != null_ptid); - /* Clear out solib state while the bfd is still open. See - comments in clear_solib in solib.c. */ - clear_solib (current_program_space); + /* Avoid confusion from thread stuff. */ + switch_to_no_thread (); - m_core_bfd.reset (nullptr); + exit_inferior (current_inferior ()); - /* Notify that the core file has changed. */ - gdb::observers::core_file_changed.notify (current_inferior ()); - } + /* Clear out solib state while the bfd is still open. See + comments in clear_solib in solib.c. */ + clear_solib (current_program_space); } /* Close the core target. */ @@ -639,11 +638,38 @@ core_target::clear_core () void core_target::close () { - clear_core (); + /* The core BFD is set when the core_target is created and attached to + the inferior. It is never explicitly cleared, instead m_core_bfd will + have its reference count reduced when the core_target is deleted. */ + gdb_assert (this->core_bfd () != nullptr); + + /* If we called ::detach before calling ::close then the inferior will + have already been exited. This will happen if the user clears the + core file with the 'core-file' or 'detach' commands. + + However, if the user just causes the core_target to be unpushed, by + pushing an alternative target, e.g. 'target remote ....', then we will + not call ::detach before calling ::close. + + In the former case we don't want to exit the inferior twice; this is + mostly harmless except it causes two 'exited' events to be emitted in + the Python API, which isn't ideal. + + As opening a core_target always ensures that some thread is selected, + then we can tell if exit_core_file_inferior has already been called by + checking if no thread is now selected. */ + if (inferior_ptid != null_ptid) + exit_core_file_inferior (); /* Core targets are heap-allocated (see core_target_open), so here we delete ourselves. */ delete this; + + /* Notify that the core file has changed. This is intentionally done + after the core_target is deleted as nothing in here depends on the + core_target itself, the core_target has already been removed from the + inferior's target stack by this point. */ + gdb::observers::core_file_changed.notify (current_inferior ()); } /* Look for sections whose names start with `.reg/' so that we can @@ -1240,17 +1266,18 @@ void core_target::detach (inferior *inf, int from_tty) { /* The core BFD is set when the core_target is created and attached to - the inferior. It is only cleared during detach or close. After - detaching the core target will be closed and deleted, so detach can - never be called twice. What this means is that detach will never be - called without the core BFD being set. */ + the inferior. It is never explicitly cleared, instead m_core_bfd will + have its reference count reduced when the core_target is deleted. */ gdb_assert (this->core_bfd () != nullptr); - /* Get rid of the core. Don't rely on core_target::close doing it, - because target_detach may be called with core_target's refcount > 1, - meaning core_target::close may not be called yet by the - unpush_target call below. */ - clear_core (); + /* Similarly, the inferior and thread are created when the core_target is + opened, and are only exited when this function, or ::close are called. + As calling ::close deletes the core_target, then when this function is + called, the inferior will still be live. */ + gdb_assert (inferior_ptid != null_ptid); + + /* Get rid of the core inferior. */ + exit_core_file_inferior (); /* Note that 'this' may be dangling after this call. unpush_target closes the target if the refcount reaches 0, and our close -- 2.25.4