From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 40C+EtACUmWSYQUAWB0awg (envelope-from ) for ; Mon, 13 Nov 2023 06:04:48 -0500 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=bOCoFVvt; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 434661E0C1; Mon, 13 Nov 2023 06:04:48 -0500 (EST) Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (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 297531E091 for ; Mon, 13 Nov 2023 06:04:46 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 641AB383A623 for ; Mon, 13 Nov 2023 11:04:45 +0000 (GMT) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 172683858D32 for ; Mon, 13 Nov 2023 11:04:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 172683858D32 Authentication-Results: sourceware.org; dmarc=pass (p=none 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 172683858D32 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=1699873475; cv=none; b=ngIxWgk7o7zGH/Bb5nOpiZwidSJJokrrvkVAK+61V/50F3Ahbl9VUeVD2ZtYcW6CmhYeJqOEUYzZM0QySL+v8KlM+W/xD0jVATgoToAqK30MhMw7hnksMNCC9Z+ysbrXxkwGQ6P5z4E0sHUSjIfyhTpKr8cJysOVW7SuTbUDtUg= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699873475; c=relaxed/simple; bh=5+aC0zW57b8l7dsJvW8MUxdD1BGSMvuHAgywlygG5eA=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=tRQU5KH0L49Wdy0Mwq2IcZrtcyU+xJRaqXwCOnhF8Nz9+HRDd4UdsFY4f9YQfmkZeSIV1d9TMuQ7ajg1jbBdO20buhC7P700xSltTFbi+grY2xdqWYJrCQ4uGZzPaTjjKsloJeJWSIN2SAG5Nmb26yjNjOe+DHdAqXyfH2M7a/g= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1699873472; 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: in-reply-to:in-reply-to:references:references; bh=nsaPRgmli4Pp2bZmWylELk66OOdu2lApkBMNMVAjtDA=; b=bOCoFVvtHyBgNTL9nPYfN3EtglSY7c3XnmuOm2gknwk2V2IADy9y9+aoJGyTy3QqruOAq4 2ySB5acpVDM3XJ8sVXq65bm2o90vFYGkoSe3DFxFxghaye5v5q9B9Mo95CK1gwR3REbNf1 0YqMsywWJd4bEL/RpzsWd/KehRVc/Pw= Received: from mail-lj1-f198.google.com (mail-lj1-f198.google.com [209.85.208.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-77-PvWiW7WjOc2PdJRx3QlLqw-1; Mon, 13 Nov 2023 06:04:31 -0500 X-MC-Unique: PvWiW7WjOc2PdJRx3QlLqw-1 Received: by mail-lj1-f198.google.com with SMTP id 38308e7fff4ca-2c50d4a1a33so35370221fa.0 for ; Mon, 13 Nov 2023 03:04:31 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699873469; x=1700478269; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=nsaPRgmli4Pp2bZmWylELk66OOdu2lApkBMNMVAjtDA=; b=XKyD+RTszdAGN1KGxaRWwfsKPahNOFkKVaniwWI0LHJf/81HIo6A49bkjVCShPrRp+ J0JSsbQCNTrZGbblniMDyLZhSSES4344nbHbgh6zc4Jh2ZTub3cZABydOb3LSvu8EWnq KxZ7c5d83jUwjKLB6YkcfIY/GjcWo+E+5tb9zoNocQds9+o26UP7iak0rBKrx0tuzV5i iqST0LzCFsFmputpAsToK83HWELUFGKXTuwn3n1LEl5X7Zu8eNP9oL0Ewk3zGos1EJg2 /pcxMIuwJs0Lg6eVKxZkVRDNj1GvfysTtZ6CWbhoeNAThfwP9VxBrLATvYRDMjAWHmEo Fmmw== X-Gm-Message-State: AOJu0YwrIPOy0wcU6aO8uL5ygmYPNiBcSGgJN8XBeotp8hHHn9XNDGmW FcqIVL7XUWRaaToYzMH+4txTTa8ePg++otpqOfXMKHru2ouwQM3LGIwIraC8UdS/bhr4nnAuLZw xTUj3vi4NT1+3HRRYlYpKdeC5ORLibg== X-Received: by 2002:a2e:9b42:0:b0:2c6:ede0:9371 with SMTP id o2-20020a2e9b42000000b002c6ede09371mr4896910ljj.19.1699873469350; Mon, 13 Nov 2023 03:04:29 -0800 (PST) X-Google-Smtp-Source: AGHT+IGar8oebSsm3aA7pfI1tKEcciZw1CumN1L4SDWEFPCEGWXpmFiPS+Ddv+wMfCFEXIzHRO63Yw== X-Received: by 2002:a2e:9b42:0:b0:2c6:ede0:9371 with SMTP id o2-20020a2e9b42000000b002c6ede09371mr4896890ljj.19.1699873468900; Mon, 13 Nov 2023 03:04:28 -0800 (PST) Received: from localhost (105.226.159.143.dyn.plus.net. [143.159.226.105]) by smtp.gmail.com with ESMTPSA id e6-20020a05600c218600b00405c7591b09sm7597540wme.35.2023.11.13.03.04.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 13 Nov 2023 03:04:28 -0800 (PST) From: Andrew Burgess To: Kevin Buettner , gdb-patches@sourceware.org Cc: Kevin Buettner Subject: Re: [PATCH 2/2] New test: gdb.base/process-dies-while-detaching.exp In-Reply-To: <20231111223046.109727-3-kevinb@redhat.com> References: <20231111223046.109727-1-kevinb@redhat.com> <20231111223046.109727-3-kevinb@redhat.com> Date: Mon, 13 Nov 2023 11:04:27 +0000 Message-ID: <87fs19symc.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, 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+public-inbox=simark.ca@sourceware.org Kevin Buettner writes: > Andrew Burgess is the primary author of this test case. Its design > is similar to that of gdb.threads/main-thread-exit-during-detach.exp, > which was also written by Andrew. > > This test checks that GDB correctly handles several cases that can > occur when GDB attempts to detach an inferior process. The process > can exit or be terminated (e.g. via SIGKILL) prior to GDB's event > loop getting a chance to remove it from GDB's internal data > structures. To complicate things even more, detach works differently > when a checkpoint (created via GDB's "checkpoint" command) exists for > the inferior. This test checks all four possibilities: process exit > with no checkpoint, process termination with no checkpoint, process > exit with a checkpoint, and process termination with a checkpoint. > > Co-Authored-By: Andrew Burgess > --- > gdb/testsuite/gdb.base/kill-during-detach.c | 32 +++++ > gdb/testsuite/gdb.base/kill-during-detach.exp | 132 ++++++++++++++++++ I really dislike having tests added in a separate commit to the change they relate too. Please would you consider combining these two commits. I often end up looking back at historic commits: I want to modify some corner of GDB so I use 'git blame' to find a commit to look at. In an ideal world, the commit includes tests, then, when I modify the code in question, I know that this particular test is a good candidate to use to ensure I've not broken anything. But in addition, I often find I can understand some code better if I can see it in action. Having an included test means I immediately have a good test which makes use of the code I'm interested in. Neither of the above are impossible to figure out when the tests are separated out like this .... but, in this case I see no benefit to splitting out the tests, only downsides. Otherwise, this looks good to me. Approved-By: Andrew Burgess Thanks, Andrew > 2 files changed, 164 insertions(+) > create mode 100644 gdb/testsuite/gdb.base/kill-during-detach.c > create mode 100644 gdb/testsuite/gdb.base/kill-during-detach.exp > > diff --git a/gdb/testsuite/gdb.base/kill-during-detach.c b/gdb/testsuite/gdb.base/kill-during-detach.c > new file mode 100644 > index 00000000000..2d9cca91e2f > --- /dev/null > +++ b/gdb/testsuite/gdb.base/kill-during-detach.c > @@ -0,0 +1,32 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2023 Free Software Foundation, Inc. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see . */ > + > +#include > + > +volatile int dont_exit_just_yet = 1; > + > +int > +main () > +{ > + alarm (300); > + > + /* Spin until GDB releases us. */ > + while (dont_exit_just_yet) > + usleep (100000); > + > + _exit (0); > +} > diff --git a/gdb/testsuite/gdb.base/kill-during-detach.exp b/gdb/testsuite/gdb.base/kill-during-detach.exp > new file mode 100644 > index 00000000000..26028d5fc34 > --- /dev/null > +++ b/gdb/testsuite/gdb.base/kill-during-detach.exp > @@ -0,0 +1,132 @@ > +# Copyright 2023 Free Software Foundation, Inc. > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see . > + > +# This test checks that GDB correctly handles several cases that can > +# occur when GDB attempts to detach an inferior process. The process > +# can exit or be terminated (e.g. via SIGKILL) prior to GDB's event > +# loop getting a chance to remove it from GDB's internal data > +# structures. To complicate things even more, detach works differently > +# when a checkpoint (created via GDB's "checkpoint" command) exists for > +# the inferior. This test checks all four possibilities: process exit > +# with no checkpoint, process termination with no checkpoint, process > +# exit with a checkpoint, and process termination with a checkpoint. > + > +standard_testfile > + > +# This test requires python. > +require allow_python_tests > + > +# This test attempts to kill a process on the host running GDB, so > +# disallow remote targets. (Setting --target_board to > +# native-gdbserver or native-extended-gdbserver should still work.) > +require {!is_remote target} > + > +# Checkpoint support only works on native Linux: > +if { [istarget "*-*-linux*"] && [target_info gdb_protocol] == ""} { > + set has_checkpoint true > +} else { > + set has_checkpoint false > +} > + > +if {[build_executable "failed to prepare" $testfile $srcfile] == -1} { > + return -1 > +} > + > +# Start an inferior, which blocks in a spin loop. Setup a Python > +# function that performs an action based on EXIT_P that will cause the > +# inferior to exit, and then, within the same Python function, ask GDB > +# to detach from the inferior. Use 'continue&' to run the inferior in > +# the background, and then invoke the Python function. Note, too, that > +# non-stop mode is enabled during the restart; if this is not done, > +# remote_target::putpkt_binary in remote.c will disallow some of the > +# operations necessary for this test. > +# > +# The idea is that GDB's event loop will not get a chance to handle > +# the inferior exiting, so it will only be at the point that we try to > +# detach that we notice that the inferior has exited. > +# > +# When EXIT_P is true the action we perform to terminate the inferior > +# is to set a flag in the inferior, which allows the inferior to break > +# out of its spin loop. > +# > +# When EXIT_P is false the action we perform is to send SIGKILL to the > +# inferior. > +# > +# When CHECKPOINT_P is true, before issuing 'continue&' we use the > +# 'checkpoint' command to create a checkpoint of GDB. > +# > +# When CHECKPOINT_P is false we don't use the 'checkpoint' command. > +proc run_test { exit_p checkpoint_p } { > + save_vars { ::GDBFLAGS } { > + append ::GDBFLAGS " -ex \"set non-stop on\"" > + clean_restart $::binfile > + } > + > + if {![runto_main]} { > + return -1 > + } > + > + if { $checkpoint_p } { > + gdb_test "checkpoint" \ > + "checkpoint 1: fork returned pid $::decimal\\." > + } > + > + # Must get the PID before we resume the inferior. > + set inf_pid [get_inferior_pid] > + > + # Put the PID in a python variable so that a numerical PID won't > + # appear in the PASS/FAIL output. > + gdb_test_no_output "python inf_pid=$inf_pid" "assign inf_pid" > + > + gdb_test "continue &" > + > + if { $exit_p } { > + set action_line "gdb.execute(\"set variable dont_exit_just_yet=0\")" > + } else { > + set action_line "os.kill(inf_pid, signal.SIGKILL)" > + } > + > + gdb_test_multiline "Create worker function" \ > + "python" "" \ > + "import time" "" \ > + "import os" "" \ > + "import signal" "" \ > + "def kill_and_detach():" "" \ > + " $action_line" "" \ > + " time.sleep(1)" "" \ > + " gdb.execute(\"detach\")" "" \ > + "end" "" > + > + if { $checkpoint_p } { > + # NOTE: The 'checkpoint' system in GDB appears to be a little > + # iffy. This detach does seem to restore the checkpoint, but > + # it leaves the inferior stuck in a running state. > + gdb_test_no_output "python kill_and_detach()" > + } else { > + gdb_test "python kill_and_detach()" \ > + "\\\[Inferior $::decimal \[^\r\n\]+ detached\\\]" > + } > +} > + > +if { $has_checkpoint } { > + set checkpoint_iters { true false } > +} else { > + set checkpoint_iters { false } > +} > + > +foreach_with_prefix exit_p { true false } { > + foreach_with_prefix checkpoint_p $checkpoint_iters { > + run_test $exit_p $checkpoint_p > + } > +} > -- > 2.41.0