Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] gdb: Don't let SIGWINCH interrupt waiting for remote target
@ 2020-07-09 15:02 Andrew Burgess
  2020-07-09 16:00 ` Simon Marchi
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Burgess @ 2020-07-09 15:02 UTC (permalink / raw)
  To: gdb-patches

When connecting to a remote target a SIGWINCH will cause GDB to stop
waiting an complain about an interrupted system call, so:

  (gdb) target remote :1234
  # User resizes termainal...
  :1234: Interrupted system call.
  (gdb)

While investigating I also discovered that hitting Ctrl-C looks like
this:

  (gdb) target remote :1234
  # User hits Ctrl-C
  ^C:1234: Interrupted system call.
  (gdb) Quit
  (gdb)

Which doesn't seem great.

In this commit I wrap the code that waits for a connection in a loop
that will swallow any EINTR errors.  As a result SIGWINCH no longer
causes the connection loop to break, but nor does Ctrl-C.  So I add a
check of the QUIT flag, and now we get:

  (gdb) target remote :1234
  # User resizes termainal, and GDB continues to wait.

And also:

  (gdb) target remote :1234
  # User hits Ctrl-C
  ^CQuit
  (gdb)

Which seems better.

There is a test for this, which I've tried to make resilient against
timing issues, but I don't know how good it will actually turn out to
be.

gdb/ChangeLog:

	PR remote/26221
	* ser-tcp.c (wait_for_connection): Update comment.  Ignore EINTR
	errors, and handle QUIT from user.

gdb/testsuite/ChangeLog:

	PR remote/26221
	* gdb.server/connect-resize-quit.exp: New file.
---
 gdb/ChangeLog                                 |  6 ++
 gdb/ser-tcp.c                                 | 57 ++++++------
 gdb/testsuite/ChangeLog                       |  5 ++
 .../gdb.server/connect-resize-quit.exp        | 88 +++++++++++++++++++
 4 files changed, 131 insertions(+), 25 deletions(-)
 create mode 100644 gdb/testsuite/gdb.server/connect-resize-quit.exp

diff --git a/gdb/ser-tcp.c b/gdb/ser-tcp.c
index 7dd903dfaad..8fc19b959fc 100644
--- a/gdb/ser-tcp.c
+++ b/gdb/ser-tcp.c
@@ -82,10 +82,12 @@ static unsigned int tcp_retry_limit = 15;
 
 #define POLL_INTERVAL 5
 
-/* Helper function to wait a while.  If SOCK is not -1, wait on its
-   file descriptor.  Otherwise just wait on a timeout, updating
-   *POLLS.  Returns -1 on timeout or interrupt, otherwise the value of
-   select.  */
+/* Helper function to wait a while.  If SOCK is not -1, wait on its file
+   descriptor.  Otherwise just wait on a timeout, updating *POLLS.
+
+   Returns -1 on timeout, otherwise the value of select.  Interrupts
+   (EINTR) are swallowed within this function, however, this function will
+   throw an error if the user interrupts the wait.  */
 
 static int
 wait_for_connect (int sock, unsigned int *polls)
@@ -122,29 +124,34 @@ wait_for_connect (int sock, unsigned int *polls)
       t.tv_usec = 0;
     }
 
-  if (sock >= 0)
+  do
     {
-      fd_set rset, wset, eset;
-
-      FD_ZERO (&rset);
-      FD_SET (sock, &rset);
-      wset = rset;
-      eset = rset;
-
-      /* POSIX systems return connection success or failure by signalling
-	 wset.  Windows systems return success in wset and failure in
-	 eset.
-
-	 We must call select here, rather than gdb_select, because
-	 the serial structure has not yet been initialized - the
-	 MinGW select wrapper will not know that this FD refers
-	 to a socket.  */
-      n = select (sock + 1, &rset, &wset, &eset, &t);
+      QUIT;
+      if (sock >= 0)
+	{
+	  fd_set rset, wset, eset;
+
+	  FD_ZERO (&rset);
+	  FD_SET (sock, &rset);
+	  wset = rset;
+	  eset = rset;
+
+	  /* POSIX systems return connection success or failure by signalling
+	     wset.  Windows systems return success in wset and failure in
+	     eset.
+
+	     We must call select here, rather than gdb_select, because
+	     the serial structure has not yet been initialized - the
+	     MinGW select wrapper will not know that this FD refers
+	     to a socket.  */
+	  n = select (sock + 1, &rset, &wset, &eset, &t);
+	}
+      else
+	/* Use gdb_select here, since we have no file descriptors, and on
+	   Windows, plain select doesn't work in that case.  */
+	n = gdb_select (0, NULL, NULL, NULL, &t);
     }
-  else
-    /* Use gdb_select here, since we have no file descriptors, and on
-       Windows, plain select doesn't work in that case.  */
-    n = gdb_select (0, NULL, NULL, NULL, &t);
+  while (n == -1 && errno == EINTR);
 
   /* If we didn't time out, only count it as one poll.  */
   if (n > 0 || *polls < POLL_INTERVAL)
diff --git a/gdb/testsuite/gdb.server/connect-resize-quit.exp b/gdb/testsuite/gdb.server/connect-resize-quit.exp
new file mode 100644
index 00000000000..cd5db4a3675
--- /dev/null
+++ b/gdb/testsuite/gdb.server/connect-resize-quit.exp
@@ -0,0 +1,88 @@
+# Copyright 2020 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/>.  */
+
+# Check that a terminal resize while waiting to connect to a target
+# doesn't cause the wait to be abandoned.  Then check that the wait
+# can be interrupted with SIGINT.
+
+proc test_signal { sig } {
+    global gdb_prompt
+
+    with_test_prefix "SIG${sig}" {
+
+	set testname "waiting on target remote"
+
+	gdb_start
+
+	# Open a connection to a remote target, but use a port number that is
+	# unlikely to actually be in use.  We want this connection to block
+	# waiting for a remote so we can test GDB's behaviour in this blocked
+	# state.
+	gdb_test_no_output "set tcp connect-timeout 2"
+
+	set timeout_count 0
+	while { $timeout_count < 5} {
+	    # Try connecting.  This should block waiting for a remote to appear.
+	    send_gdb "target remote :0\n"
+
+	    # Now send a signal to GDB and follow up by sending some other command.
+	    set gdb_pid [exp_pid -i [board_info host fileid]]
+	    remote_exec host "kill -${sig} $gdb_pid"
+	    send_gdb "echo xxyyzz\\n\n"
+
+	    # Now try to figure out what GDB did.
+	    set got_timeout false
+	    gdb_test_multiple "" "$testname" {
+		-re "^target remote :0\r\n:0: Connection timed out\\..*$gdb_prompt $" {
+		    # It's possible that we didn't send the signal quickly enough.
+		    # Maybe the testing machine is heavily loaded?
+		    set got_timeout true
+		}
+		-re "^target remote :0\r\n:0: Interrupted system call\\.\r\n$gdb_prompt.*xxyyzz.*$gdb_prompt $" {
+		    fail "$gdb_test_name (interrupted by $sig)"
+		    return
+		}
+		-re "^target remote :0\r\necho xxyyzz\\\\n\r.:0: Connection timed out.\r\n$gdb_prompt.*xxyyzz.*$gdb_prompt $" {
+		    if { $sig == "WINCH" } {
+			pass $gdb_test_name
+		    } else {
+			fail "$gdb_test_name (unexpected timeout)"
+		    }
+		    return
+		}
+		-re "^target remote :0\r\nQuit\r\n$gdb_prompt.*xxyyzz.*$gdb_prompt $" {
+		    if { $sig == "INT" } {
+			pass $gdb_test_name
+		    } else {
+			fail "$gdb_test_name (unexpected QUIT)"
+		    }
+		    return
+		}
+	    }
+
+	    if { ! $got_timeout } {
+		# Some other unknown error.
+		return
+	    }
+
+	    incr timeout_count
+	}
+
+	unresolved "$testname (too many timeouts)"
+    }
+}
+
+test_signal WINCH
+test_signal INT
-- 
2.25.4



^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-07-10 13:22 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-09 15:02 [PATCH] gdb: Don't let SIGWINCH interrupt waiting for remote target Andrew Burgess
2020-07-09 16:00 ` Simon Marchi
2020-07-10 13:22   ` Andrew Burgess

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox