From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f52.google.com (mail-wm1-f52.google.com [209.85.128.52]) by sourceware.org (Postfix) with ESMTPS id 100EB3857BA8 for ; Wed, 22 Jun 2022 19:20:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 100EB3857BA8 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wm1-f52.google.com with SMTP id l126-20020a1c2584000000b0039c1a10507fso215859wml.1 for ; Wed, 22 Jun 2022 12:20:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:subject:date:message-id:mime-version :content-transfer-encoding; bh=wazFSAhycVqHg8GNLhi45xHdjXSrGAnPeqE1a2LnIhY=; b=wURaXeVg+qGlRm4FYOe88MM3unkZgtX6SRF+eAsUkpDQrPYN/vDA+w/zRTylnrPxHB 1rivkgXXoovCSY+HTMSVMEJWzJRUSTenGespZ/xqEuyW14jVb/tOSs8O2AcnOkDKQUmS 8e3UcIVCYWbK6g1VptBgHj1TOaredkzvdfYyvYWTWswFfNtSRy7p23k6ugnKsOzNt3Df vFDkv++2BeNv6Q8H1sxh0dkWuJm+/dyPE2R9/MxZYG8qfYG7/aPjHzVhPZGF3qw+xTzm q/xyLqVF+3ZVemIJklhrEXTtAgnKDI1PECiR68/JUqS9PsFYcXT4kPcPGL+zNe3QMqs8 Lc3w== X-Gm-Message-State: AOAM533zufM+MMEeQR//W8Cl1FznSqEfzoQhzeC1UKKj0oc3txiYp/Qu 3Fo5OE5svUcQEmbnznX514eWImaYYw0= X-Google-Smtp-Source: ABdhPJwYHHffL6gNJ4QoL2/GRFKxaw2UEbm89iY8cRpPDQlj+uDAOmPmHWabQYbeL0teU+rGv16zPg== X-Received: by 2002:a1c:5444:0:b0:39c:3761:ac37 with SMTP id p4-20020a1c5444000000b0039c3761ac37mr49211389wmi.144.1655925642136; Wed, 22 Jun 2022 12:20:42 -0700 (PDT) Received: from localhost ([2001:8a0:f924:2600:209d:85e2:409e:8726]) by smtp.gmail.com with ESMTPSA id s9-20020a5d6a89000000b0021b98d73a4esm5353632wru.114.2022.06.22.12.20.40 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 22 Jun 2022 12:20:40 -0700 (PDT) From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH] [master & gdb 12] Fix core-file -> detach -> crash (corefiles/29275) Date: Wed, 22 Jun 2022 20:20:39 +0100 Message-Id: <20220622192039.1627458-1-pedro@palves.net> X-Mailer: git-send-email 2.36.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-9.8 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, 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.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 22 Jun 2022 19:20:46 -0000 After loading a core file, you're supposed to be able to use "detach" to unload the core file. That unfortunately regressed starting with GDB 11, with these commits: 1192f124a308 - gdb: generalize commit_resume, avoid commit-resuming when threads have pending statuses 408f66864a1a - detach in all-stop with threads running resulting in a GDB crash: ... Thread 1 "gdb" received signal SIGSEGV, Segmentation fault. 0x0000555555e842bf in maybe_set_commit_resumed_all_targets () at ../../src/gdb/infrun.c:2899 2899 if (proc_target->commit_resumed_state) (top-gdb) bt #0 0x0000555555e842bf in maybe_set_commit_resumed_all_targets () at ../../src/gdb/infrun.c:2899 #1 0x0000555555e848bf in scoped_disable_commit_resumed::reset (this=0x7fffffffd440) at ../../src/gdb/infrun.c:3023 #2 0x0000555555e84a0c in scoped_disable_commit_resumed::reset_and_commit (this=0x7fffffffd440) at ../../src/gdb/infrun.c:3049 #3 0x0000555555e739cd in detach_command (args=0x0, from_tty=1) at ../../src/gdb/infcmd.c:2791 #4 0x0000555555c0ba46 in do_simple_func (args=0x0, from_tty=1, c=0x55555662a600) at ../../src/gdb/cli/cli-decode.c:95 #5 0x0000555555c112b0 in cmd_func (cmd=0x55555662a600, args=0x0, from_tty=1) at ../../src/gdb/cli/cli-decode.c:2514 #6 0x0000555556173b1f in execute_command (p=0x5555565c5916 "", from_tty=1) at ../../src/gdb/top.c:699 The code that crashes looks like: static void maybe_set_commit_resumed_all_targets () { scoped_restore_current_thread restore_thread; for (inferior *inf : all_non_exited_inferiors ()) { process_stratum_target *proc_target = inf->process_target (); if (proc_target->commit_resumed_state) ^^^^^^^^^^^ With 'proc_target' above being null. all_non_exited_inferiors filters out inferiors that have pid==0. We get here at the end of detach_command, after core_target::detach has already run, at which point the inferior _should_ have pid==0 and no process target. It is clear it no longer has a process target, but, it still has a pid!=0 somehow. The reason the inferior still has pid!=0, is that core_target::detach just unpushes, and relies on core_target::close to actually do the getting rid of the core and exiting the inferior. The problem with that is that detach_command grabs an extra strong reference to the process stratum target, so the unpush_target inside core_target::detach doesn't actually result in a call to core_target::close. Fix this my moving the cleaning up the core inferior to a shared routine called by both core_target::close and core_target::detach. We still need to cleanup the inferior from within core_file::close because there are paths to it that want to get rid of the core without going through detach. E.g., "core-file" -> "run". This commit includes a new test added to gdb.base/corefile.exp to cover the "core-file core" -> "detach" scenario. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29275 Change-Id: Ic42bdd03182166b19f598428b0dbc2ce6f67c893 --- gdb/corelow.c | 27 +++++++++++++++++++++------ gdb/testsuite/gdb.base/corefile.exp | 12 ++++++++++++ 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/gdb/corelow.c b/gdb/corelow.c index 8c33fb7ebb2..2037ef3dc89 100644 --- a/gdb/corelow.c +++ b/gdb/corelow.c @@ -122,6 +122,9 @@ class core_target final : public process_stratum_target private: /* per-core data */ + /* Get rid of the core inferior. */ + void clear_core (); + /* The core's section table. Note that these target sections are *not* mapped in the current address spaces' set of target sections --- those should come only from pure executable or @@ -308,10 +311,8 @@ core_target::build_file_mappings () /* An arbitrary identifier for the core inferior. */ #define CORELOW_PID 1 -/* Close the core target. */ - void -core_target::close () +core_target::clear_core () { if (core_bfd) { @@ -325,6 +326,14 @@ core_target::close () current_program_space->cbfd.reset (nullptr); } +} + +/* Close the core target. */ + +void +core_target::close () +{ + clear_core (); /* Core targets are heap-allocated (see core_target_open), so here we delete ourselves. */ @@ -631,9 +640,15 @@ core_target_open (const char *arg, int from_tty) void core_target::detach (inferior *inf, int from_tty) { - /* Note that 'this' is dangling after this call. unpush_target - closes the target, and our close implementation deletes - 'this'. */ + /* 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 (); + + /* Note that 'this' may be dangling after this call. unpush_target + closes the target if the refcount reaches 0, and our close + implementation deletes 'this'. */ inf->unpush_target (this); /* Clear the register cache and the frame cache. */ diff --git a/gdb/testsuite/gdb.base/corefile.exp b/gdb/testsuite/gdb.base/corefile.exp index 4ed92a02955..7f3d2efe3a2 100644 --- a/gdb/testsuite/gdb.base/corefile.exp +++ b/gdb/testsuite/gdb.base/corefile.exp @@ -207,6 +207,16 @@ gdb_test "up" "#\[0-9\]* *\[0-9xa-fH'\]* in .* \\(.*\\).*" "up in corefile.exp ( gdb_test "core" "No core file now." +# Test that we can unload the core with the "detach" command. + +proc_with_prefix corefile_detach {} { + clean_restart $::binfile + + gdb_test "core-file $::corefile" "Core was generated by .*" "load core" + gdb_test "detach" "No core file now\\." "detach core" +} + +corefile_detach # Test a run (start) command will clear any loaded core file. @@ -222,6 +232,8 @@ proc corefile_test_run {} { return } + clean_restart $::binfile + gdb_test "core-file $corefile" "Core was generated by .*" "run: load core again" gdb_test "info files" "\r\nLocal core dump file:\r\n.*" "run: sanity check we see the core file" base-commit: 90b7a5df152a64d2bea20beb438e8b81049a5c30 -- 2.36.0