From patchwork Tue Oct 17 10:36:40 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 23635 Received: (qmail 1894 invoked by alias); 17 Oct 2017 10:36:47 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 1873 invoked by uid 89); 17 Oct 2017 10:36:46 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=2*, quitting, secs, H*M:4c84 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 17 Oct 2017 10:36:43 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9784081DF4; Tue, 17 Oct 2017 10:36:42 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 9784081DF4 Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=palves@redhat.com Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id A882564441; Tue, 17 Oct 2017 10:36:41 +0000 (UTC) From: Pedro Alves Subject: Re: [PATCH] Add several "quit with live inferior" tests To: Simon Marchi References: <1507805601-22576-1-git-send-email-palves@redhat.com> <5ecf8183d97c76fc4ca009d6f3e10432@polymtl.ca> Cc: gdb-patches@sourceware.org Message-ID: <1f96677c-4c84-7e06-c98f-87fc88251f87@redhat.com> Date: Tue, 17 Oct 2017 11:36:40 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <5ecf8183d97c76fc4ca009d6f3e10432@polymtl.ca> On 10/12/2017 11:11 PM, Simon Marchi wrote: > On 2017-10-12 06:53, Pedro Alves wrote: >> In my multi-target branch, I had managed to break GDB exiting >> successfuly in response to "quit" or SIGHUP/SIGTERM when: >> >> - you're debugging with "target extended-remote", >> - have more than one inferior loaded in gdb, some running, and at >> least one not running, and, >> - quit gdb with the inferior that is not running yet selected. >> >> The testsuite still passed cleanly anyway. I only noticed because I >> was left with a bunch of core dumps in the gdb/testsuite/ directory -- >> the testsuite infrastructure closes GDB's pty after running each >> testcase, which results in GDB getting a SIGHUP and should make GDB >> exit gracefully. If GDB crashes at that point though, there's no >> indication about it in gdb.sum/gdb.log. >> >> This commit adds a multitude of tests exercising quitting GDB with >> live inferiors, some of which would have caught the problem. > > I think you accidentally a file (quit.c). Should it be named the same > as the exp file (quit-live.c)? Whoops. I originally wrote this as an addition to the existing gdb.base/quit.exp, which doesn't currently have a test program, and that's why I named the file quit.c. When I moved to a separate file, I had forgotten that I had also added quit.c, and assumed that that file exists in master... :-P I've renamed it to quit-live.c now, and added it to the patch. >> +# Test quitting GDB with live inferiors. >> +# >> +# Exercises combinations of: >> +# >> +# - quitting with "quit"command, or with SIGTERM/SIGHUP signals. > > missing space Fixed. > >> +# >> +# - quitting with live inferior selected, or file_stratum inferior >> +# selected. >> +# >> +# - quitting after "run", or after "attach". >> +# >> +# - quitting with local executable, or executable loaded from target >> +# directly (via default "target:/" sysroot), or with no executable >> +# loaded. >> + >> +# Note: sending an asynchronous SIGHUP with kill is not the exact same >> +# as closing GDB's input, and that resulting in SIGHUP. However, it's >> +# still a good approximation, and it has the advantage that because >> +# GDB still has a terminal, internal errors (if any) are visible in >> +# gdb.sum/gdb.log. >> + >> +standard_testfile quit.c >> + >> +if {[build_executable "failed to build" $testfile $srcfile debug]} { >> + return >> +} >> + >> +# Send signal SIG to GDB, and expect GDB to exit. >> + >> +proc test_quit_with_sig {sig} { >> + set gdb_pid [exp_pid -i [board_info host fileid]] >> + remote_exec host "kill -$sig ${gdb_pid}" >> + >> + set test "quit with SIG$sig" >> + # If GDB mishandles the signal and doesn't exit, this >> + # should FAIL with timeout. We don't expect a GDB prompt, >> + # so we see one, we'll FAIL too. > > "so if we see one" ? > > In this case, does the test fail if there's any output (no necessarily a > gdb_prompt)? It does, but via timeout. The prompt matching is referring to gdb_test_multiple's builtin match on $gdb_prompt $. (This is copied from some other test.) >> +with_test_prefix "quit with live inferior" { > > I think this prefix is not very useful, since it contains all the tests, > although I'm not against it either. Yeah, it was useful when the tests lived in quit.exp along the other, preexisting tests. I've removed it. Updated patch below. WDYT? From 5cafd2e6ebd94e038b97f78cb9ae77629cf05f00 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Tue, 17 Oct 2017 11:33:35 +0100 Subject: [PATCH] Add several "quit with live inferior" tests In my multi-target branch, I had managed to break GDB exiting successfuly in response to "quit" or SIGHUP/SIGTERM when: - you're debugging with "target extended-remote", - have more than one inferior loaded in gdb, some running, and at least one not running, and, - quit gdb with the inferior that is not running yet selected. The testsuite still passed cleanly anyway. I only noticed because I was left with a bunch of core dumps in the gdb/testsuite/ directory -- the testsuite infrastructure closes GDB's pty after running each testcase, which results in GDB getting a SIGHUP and should make GDB exit gracefully. If GDB crashes at that point though, there's no indication about it in gdb.sum/gdb.log. This commit adds a multitude of tests exercising quitting GDB with live inferiors, some of which would have caught the problem. gdb/testsuite/ChangeLog: yyyy-mm-dd Pedro Alves * gdb.base/quit-live.c: New file. * gdb.base/quit-live.exp: New file. --- gdb/testsuite/gdb.base/quit-live.c | 27 ++++++ gdb/testsuite/gdb.base/quit-live.exp | 178 +++++++++++++++++++++++++++++++++++ 2 files changed, 205 insertions(+) create mode 100644 gdb/testsuite/gdb.base/quit-live.c create mode 100644 gdb/testsuite/gdb.base/quit-live.exp diff --git a/gdb/testsuite/gdb.base/quit-live.c b/gdb/testsuite/gdb.base/quit-live.c new file mode 100644 index 0000000..d29fd25 --- /dev/null +++ b/gdb/testsuite/gdb.base/quit-live.c @@ -0,0 +1,27 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2017 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 + +int +main () +{ + int secs = 30; + + while (secs--) + sleep (1); +} diff --git a/gdb/testsuite/gdb.base/quit-live.exp b/gdb/testsuite/gdb.base/quit-live.exp new file mode 100644 index 0000000..0ea0080 --- /dev/null +++ b/gdb/testsuite/gdb.base/quit-live.exp @@ -0,0 +1,178 @@ +# Copyright (C) 2017 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 . + +# Test quitting GDB with live inferiors. +# +# Exercises combinations of: +# +# - quitting with "quit" command, or with SIGTERM/SIGHUP signals. +# +# - quitting with live inferior selected, or file_stratum inferior +# selected. +# +# - quitting after "run", or after "attach". +# +# - quitting with local executable, or executable loaded from target +# directly (via default "target:/" sysroot), or with no executable +# loaded. + +# Note: sending an asynchronous SIGHUP with kill is not the exact same +# as closing GDB's input, and that resulting in SIGHUP. However, it's +# still a good approximation, and it has the advantage that because +# GDB still has a terminal, internal errors (if any) are visible in +# gdb.sum/gdb.log. + +standard_testfile + +if {[build_executable "failed to build" $testfile $srcfile debug]} { + return +} + +# Send signal SIG to GDB, and expect GDB to exit. + +proc test_quit_with_sig {sig} { + set gdb_pid [exp_pid -i [board_info host fileid]] + remote_exec host "kill -$sig ${gdb_pid}" + + set test "quit with SIG$sig" + # If GDB mishandles the signal and doesn't exit, this should FAIL + # with timeout. We don't expect a GDB prompt, so if we see one, + # we'll FAIL too (without having to wait for timeout). + gdb_test_multiple "" $test { + eof { + pass $test + } + } +} + +# Call the "quit" command with an inferior live. +# +# APPEAR_HOW specifies how the running inferior appears in GDB. Can +# be either: +# +# - "run" +# +# Appear via the "run" command. +# +# - "attach" +# +# Appear via the "attach" command. +# +# - "attach-nofile" +# +# Appear via the "attach" command, but with no program preloaded in +# GDB so that GDB reads the program directly from the target when +# remote debugging (i.e., from the target:/ sysroot). This makes +# sure that GDB doesn't misbehave if it decides to close the +# 'target:/.../program' exec_file after closing the remote +# connection. +# +# EXTRA_INFERIOR is a boolean that specifies whether we try to quit +# GDB with an extra executable-only (before "run") inferior selected +# or whether we try to quit GDB when the live inferior is selected, +# with no extra inferior. +# +# QUIT_HOW specifies how to tell GDB to quit. It can be either "quit" +# (for "quit" command), "sighup" or "sigterm" (for quitting with +# SIGHUP and SIGTERM signals, respectively). + +proc quit_with_live_inferior {appear_how extra_inferior quit_how} { + global srcfile testfile binfile + global gdb_spawn_id gdb_prompt + + set test_spawn_id "" + + if {$appear_how != "attach-nofile"} { + clean_restart $binfile + } else { + clean_restart + } + + if {$appear_how == "run"} { + if ![runto_main] then { + fail "can't run to main" + return + } + } elseif {$appear_how == "attach" || $appear_how == "attach-nofile"} { + set test_spawn_id [spawn_wait_for_attach $binfile] + set testpid [spawn_id_get_pid $test_spawn_id] + + if {[gdb_test "attach $testpid" \ + "Attaching to .*process $testpid.*Reading symbols from.*" \ + "attach"] != 0} { + kill_wait_spawned_process $test_spawn_id + return + } + } else { + error "unhandled '\$appear_how': $appear_how" + } + + if {$extra_inferior} { + gdb_test "add-inferior" "Added inferior 2*" \ + "add empty inferior 2" + gdb_test "inferior 2" "Switching to inferior 2.*" \ + "switch to inferior 2" + } + + if {$quit_how == "quit"} { + # Make regexp that matches the "quit" command's output. + proc make_re {how} { + multi_line \ + "A debugging session is active.\[ \t\r\n\]*Inferior 1\[^\r\n\]* will be $how\." \ + "" \ + "Quit anyway\\? \\(y or n\\) $" + } + + if {$appear_how == "run"} { + set quit_anyway_re [make_re "killed"] + } else { + set quit_anyway_re [make_re "detached"] + } + + set test "quit with \"quit\"" + gdb_test_multiple "quit" $test { + -re $quit_anyway_re { + send_gdb "y\n" + gdb_test_multiple "" $test { + eof { + pass $test + } + } + } + } + } elseif {$quit_how == "sighup"} { + test_quit_with_sig HUP + } elseif {$quit_how == "sigterm"} { + test_quit_with_sig TERM + } else { + error "unhandled '\$quit_how': $quit_how" + } + + if {$test_spawn_id != ""} { + kill_wait_spawned_process $test_spawn_id + } +} + +foreach_with_prefix appear_how {"run" "attach" "attach-nofile"} { + if {$appear_how != "run" && ![can_spawn_for_attach]} { + continue + } + + foreach_with_prefix extra_inferior {0 1} { + foreach_with_prefix quit_how {"quit" "sigterm" "sighup"} { + quit_with_live_inferior $appear_how $extra_inferior $quit_how + } + } +}