From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x442.google.com (mail-wr1-x442.google.com [IPv6:2a00:1450:4864:20::442]) by sourceware.org (Postfix) with ESMTPS id 5904A384B821 for ; Thu, 9 Jul 2020 15:02:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 5904A384B821 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wr1-x442.google.com with SMTP id r12so2714422wrj.13 for ; Thu, 09 Jul 2020 08:02:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=Aya7O6sW2KknWvOJn5hTtHR8UbenW2wH1C7AiaGlwe8=; b=Dq27ONZeL7r/9S7/Qa3LHMAH/HK0V4BE7CPBxWqS+I0EkvVmFZ1+lccJ3uItWE9Ztf CFrp+/U4br/em6ijHPRoRjD7vdK9kDJc4XjSMlBYwKsjp+bDyxblcYN8/AWJiqKZE9xI OPYlgUSR3PONfkz6KgPHHJLitQNraAb/jpI9voHI/X4uEF4VEj4CFW7APrxadoaBU3ko pxPCEiAo03Hma8n3PCHukRgtHIp7Z3edPdLHyD2xnpd5amk2Gb31l63rKuy5yt48cnrk IY7USsB17tug5kIb0s0pQp4x7ouCszmn16Ql4r1I0+176LSga1yOVGJLUib/2IXlpd8z DJ/Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=Aya7O6sW2KknWvOJn5hTtHR8UbenW2wH1C7AiaGlwe8=; b=C9VQYyzosRi2if2WWacXQtY/V9A8FknXH11n+O8MqEve7U+rz+TOAfdloil6BKZkCQ j/EcLI7NSI5CkZqTfUfo8TaKSwton598ROLTubaN8Vz/t9wRa22TRMXSRJawayATUoi8 qgeq3rKZzTTLzcjynyiHukemVS9wHvjde3Du/PZ9IZc7smJueqHFLMq101OtEQJ/UV4b ApmvCnS5WImMNtms4ZxWoQ19Chey0t8w0IKrPYi15ue82jwTYhNJBxtQId4RrJs1HYlt qOmJnocYk4kKONbaAckf/02L9Jhuxba6gRgoCcVsOQxD/Taw9hd5HvKmQeFNUlxew2pP xSLw== X-Gm-Message-State: AOAM533x4u9THknM/SeRAPcemhBYPlTo1eExJV7p5FJQiIvr8XO2PqaD lD6Q2T7IfsUiSOp83qOIMvbx48wyZ/Y= X-Google-Smtp-Source: ABdhPJwzlmcS7Hh9Jpu8eitpkVgyXyans9BNcqlHCluirfrxggfjVghvccZgia1l8nJiGtT/8PrFPg== X-Received: by 2002:a5d:6a46:: with SMTP id t6mr66018232wrw.374.1594306952992; Thu, 09 Jul 2020 08:02:32 -0700 (PDT) Received: from localhost (host109-154-20-168.range109-154.btcentralplus.com. [109.154.20.168]) by smtp.gmail.com with ESMTPSA id q188sm5083579wma.46.2020.07.09.08.02.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 09 Jul 2020 08:02:32 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Subject: [PATCH] gdb: Don't let SIGWINCH interrupt waiting for remote target Date: Thu, 9 Jul 2020 16:02:28 +0100 Message-Id: <20200709150228.79385-1-andrew.burgess@embecosm.com> X-Mailer: git-send-email 2.25.4 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Thu, 09 Jul 2020 15:02:40 -0000 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 . */ + +# 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