This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Add several "quit with live inferior" tests


On 2017-10-17 06:36, Pedro Alves wrote:
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 <palves@redhat.com>
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  <palves@redhat.com>

	* 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 <http://www.gnu.org/licenses/>. */
+
+#include <unistd.h>
+
+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 <http://www.gnu.org/licenses/>.
+
+# 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
+	}
+    }
+}

Looks good to me.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]