[1/2] gdb: Re-enable stdin for all UIs from start_event_loop
Commit Message
If we catch an exception in start_event_loop's call to
gdb_do_one_event, then it is possible that the current_ui has changed
since we called async_disable_stdin. If that's the case then calling
async_enable_stdin will be called on the wrong UI.
To solve this problem we wrap the call to async_enable_stdin with
SWITCH_THRU_ALL_UIS, this causes us to try and re-enable stdin for all
UIs, which will catch any for which we called async_disable_stdin.
gdb/ChangeLog:
* event-loop.c (start_event_loop): Wrap async_enable_stdin with
SWITCH_THRU_ALL_UIS.
gdb/testsuite/ChangeLog:
* gdb.server/multi-ui-errors.c: New file.
* gdb.server/multi-ui-errors.exp: New file.
Change-Id: I1e18deff2e6f4e17f7a13adce3553eb001cad93b
---
gdb/ChangeLog | 5 ++
gdb/event-loop.c | 5 +-
gdb/testsuite/ChangeLog | 5 ++
gdb/testsuite/gdb.server/multi-ui-errors.c | 30 ++++++++
gdb/testsuite/gdb.server/multi-ui-errors.exp | 107 +++++++++++++++++++++++++++
5 files changed, 151 insertions(+), 1 deletion(-)
create mode 100644 gdb/testsuite/gdb.server/multi-ui-errors.c
create mode 100644 gdb/testsuite/gdb.server/multi-ui-errors.exp
Comments
This looks good to me, pending a few comments below.
On 12/23/19 1:45 AM, Andrew Burgess wrote:
> If we catch an exception in start_event_loop's call to
> gdb_do_one_event, then it is possible that the current_ui has changed
> since we called async_disable_stdin. If that's the case then calling
> async_enable_stdin will be called on the wrong UI.
>
> To solve this problem we wrap the call to async_enable_stdin with
> SWITCH_THRU_ALL_UIS, this causes us to try and re-enable stdin for all
> UIs, which will catch any for which we called async_disable_stdin.
>
> gdb/ChangeLog:
>
> * event-loop.c (start_event_loop): Wrap async_enable_stdin with
> SWITCH_THRU_ALL_UIS.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.server/multi-ui-errors.c: New file.
> * gdb.server/multi-ui-errors.exp: New file.
>
The test fails when run against --target_board=native-extended-gdbserver:
target extended-remote localhost:2348
Already connected to a remote target. Disconnect? (y or n) [answered Y; input not from terminal]
n
Remote debugging using localhost:2348
Reading symbols from /lib64/ld-linux-x86-64.so.2...
Reading symbols from /usr/lib/debug/usr/lib64/ld-2.26.so.debug...
0x00007ffff7dd8ed0 in _start () from /lib64/ld-linux-x86-64.so.2
(gdb) FAIL: gdb.server/multi-ui-errors.exp: target extended-remote localhost:2348 (got interactive prompt)
Looks like it's just missing the standard "disconnect" most
gdb.server/ tests do.
> Change-Id: I1e18deff2e6f4e17f7a13adce3553eb001cad93b
> ---
> gdb/ChangeLog | 5 ++
> gdb/event-loop.c | 5 +-
> gdb/testsuite/ChangeLog | 5 ++
> gdb/testsuite/gdb.server/multi-ui-errors.c | 30 ++++++++
> gdb/testsuite/gdb.server/multi-ui-errors.exp | 107 +++++++++++++++++++++++++++
> 5 files changed, 151 insertions(+), 1 deletion(-)
> create mode 100644 gdb/testsuite/gdb.server/multi-ui-errors.c
> create mode 100644 gdb/testsuite/gdb.server/multi-ui-errors.exp
>
> diff --git a/gdb/event-loop.c b/gdb/event-loop.c
> index affa00b4aa9..b9444153ff8 100644
> --- a/gdb/event-loop.c
> +++ b/gdb/event-loop.c
> @@ -376,7 +376,10 @@ start_event_loop (void)
> /* If any exception escaped to here, we better enable
> stdin. Otherwise, any command that calls async_disable_stdin,
> and then throws, will leave stdin inoperable. */
> - async_enable_stdin ();
> + SWITCH_THRU_ALL_UIS ()
> + {
> + async_enable_stdin ();
> + }
> /* If we long-jumped out of do_one_event, we probably didn't
> get around to resetting the prompt, which leaves readline
> in a messed-up state. Reset it here. */
> diff --git a/gdb/testsuite/gdb.server/multi-ui-errors.c b/gdb/testsuite/gdb.server/multi-ui-errors.c
> new file mode 100644
> index 00000000000..0e6c0ea0fde
> --- /dev/null
> +++ b/gdb/testsuite/gdb.server/multi-ui-errors.c
> @@ -0,0 +1,30 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> + Copyright 2019 Free Software Foundation, Inc.
Remember to update this to "2019-2020".
> +
> + 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 <stdio.h>
> +#include <unistd.h>
> +
> +int
> +main (void)
> +{
> + printf ("@@XX@@ Inferior Starting @@XX@@\n");
> +
> + while (1)
> + sleep (1);
Avoid infinite loops. For example, replace the "while" with
a "for" loop that counts 60 seconds.
> +
> + return 0;
> +}
> diff --git a/gdb/testsuite/gdb.server/multi-ui-errors.exp b/gdb/testsuite/gdb.server/multi-ui-errors.exp
> new file mode 100644
> index 00000000000..5162a1a6f9c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.server/multi-ui-errors.exp
> @@ -0,0 +1,107 @@
> +# This testcase is part of GDB, the GNU debugger.
> +#
> +# Copyright 2019 Free Software Foundation, Inc.
Ditto.
> +#
> +# 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 what happens if we have multiple UIs in use, and an error
> +# occurs while running a GDB command. Specifically, do both UIs
> +# return to an interactive state, or does one (or both) of them get
> +# stuck in a non-interactive state.
> +
> +load_lib gdbserver-support.exp
> +
> +standard_testfile
> +
> +if {[skip_gdbserver_tests]} {
> + return 0
> +}
> +
> +if {[build_executable "failed to prepare" ${testfile} \
> + ${srcfile}] == -1} {
> + return -1
> +}
> +
> +clean_restart $testfile
Use prepare_for_testing ?
> +
> +# Start gdbserver.
> +set res [gdbserver_spawn "${binfile}"]
> +set gdbserver_protocol [lindex $res 0]
> +set gdbserver_gdbport [lindex $res 1]
> +set gdbserver_pid [exp_pid -i $server_spawn_id]
> +
> +# Save the main UI's spawn ID.
> +set gdb_main_spawn_id $gdb_spawn_id
> +
> +# Create the new PTY for the secondary console UI, issue the 'new-ui'
> +# command, and wait for a prompt on the second UI.
> +spawn -pty
> +set extra_spawn_id $spawn_id
> +set extra_tty_name $spawn_out(slave,name)
> +gdb_test_multiple "new-ui console $extra_tty_name" "new-ui" {
> + -re "New UI allocated\r\n$gdb_prompt $" {
> + pass $gdb_test_name
> + }
> +}
> +with_spawn_id $extra_spawn_id {
> + gdb_test_multiple "" "initial prompt on extra console" {
> + -re "$gdb_prompt $" {
> + pass $gdb_test_name
> + }
> + }
> +}
> +
> +# Connect to the remote and continue its execution from the other UI.
> +with_spawn_id $extra_spawn_id {
> + gdb_test "target $gdbserver_protocol $gdbserver_gdbport" ".*"
> + send_gdb "continue\n"
> +}
> +
> +# We're going to kill the gdbserver, but before we do, lets make sure
> +# that the inferior has started executing.
> +with_spawn_id $server_spawn_id {
> + gdb_expect 3 {
> + -re "@@XX@@ Inferior Starting @@XX@@" {
> + verbose -log "APB: Spotted running pattern"
> + }
> + timeout {
> + verbose -log "APB: Didn't find running message"
> + }
> + }
> +}
> +
> +# Interact with the main UI.
> +with_spawn_id $gdb_main_spawn_id {
> + gdb_test "echo hello\\n" "hello" "interact with GDB's main UI"
> +}
> +
> +# Now kill the gdbserver.
> +remote_exec target "kill -9 $gdbserver_pid"
> +
> +# We expect to land back at a GDB prompt in both UIs. Right now there
> +# seems to be some "issues" with the prompt automatically being
> +# displayed... Or maybe this is intentional and I'm missunderstanding
> +# the expected behaviour.
Please avoid "I" in comments. Can you clarify what you mean by "issues"?
> +# Either way, getting a prompt isn't the
> +# point of this test. The point is that we should not be able to
> +# interact with GDB from either interpreter now.
I guess you meant "should" instead of "should not"?
> +
> +with_spawn_id $gdb_main_spawn_id {
> + gdb_test "echo" "" \
> + "main UI, prompt after gdbserver dies"
> +}
> +
> +with_spawn_id $extra_spawn_id {
> + gdb_test "echo" "" \
> + "extra UI, prompt after gdbserver dies"
> +}
>
Thanks,
Pedro Alves
@@ -376,7 +376,10 @@ start_event_loop (void)
/* If any exception escaped to here, we better enable
stdin. Otherwise, any command that calls async_disable_stdin,
and then throws, will leave stdin inoperable. */
- async_enable_stdin ();
+ SWITCH_THRU_ALL_UIS ()
+ {
+ async_enable_stdin ();
+ }
/* If we long-jumped out of do_one_event, we probably didn't
get around to resetting the prompt, which leaves readline
in a messed-up state. Reset it here. */
new file mode 100644
@@ -0,0 +1,30 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2019 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 <stdio.h>
+#include <unistd.h>
+
+int
+main (void)
+{
+ printf ("@@XX@@ Inferior Starting @@XX@@\n");
+
+ while (1)
+ sleep (1);
+
+ return 0;
+}
new file mode 100644
@@ -0,0 +1,107 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2019 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 what happens if we have multiple UIs in use, and an error
+# occurs while running a GDB command. Specifically, do both UIs
+# return to an interactive state, or does one (or both) of them get
+# stuck in a non-interactive state.
+
+load_lib gdbserver-support.exp
+
+standard_testfile
+
+if {[skip_gdbserver_tests]} {
+ return 0
+}
+
+if {[build_executable "failed to prepare" ${testfile} \
+ ${srcfile}] == -1} {
+ return -1
+}
+
+clean_restart $testfile
+
+# Start gdbserver.
+set res [gdbserver_spawn "${binfile}"]
+set gdbserver_protocol [lindex $res 0]
+set gdbserver_gdbport [lindex $res 1]
+set gdbserver_pid [exp_pid -i $server_spawn_id]
+
+# Save the main UI's spawn ID.
+set gdb_main_spawn_id $gdb_spawn_id
+
+# Create the new PTY for the secondary console UI, issue the 'new-ui'
+# command, and wait for a prompt on the second UI.
+spawn -pty
+set extra_spawn_id $spawn_id
+set extra_tty_name $spawn_out(slave,name)
+gdb_test_multiple "new-ui console $extra_tty_name" "new-ui" {
+ -re "New UI allocated\r\n$gdb_prompt $" {
+ pass $gdb_test_name
+ }
+}
+with_spawn_id $extra_spawn_id {
+ gdb_test_multiple "" "initial prompt on extra console" {
+ -re "$gdb_prompt $" {
+ pass $gdb_test_name
+ }
+ }
+}
+
+# Connect to the remote and continue its execution from the other UI.
+with_spawn_id $extra_spawn_id {
+ gdb_test "target $gdbserver_protocol $gdbserver_gdbport" ".*"
+ send_gdb "continue\n"
+}
+
+# We're going to kill the gdbserver, but before we do, lets make sure
+# that the inferior has started executing.
+with_spawn_id $server_spawn_id {
+ gdb_expect 3 {
+ -re "@@XX@@ Inferior Starting @@XX@@" {
+ verbose -log "APB: Spotted running pattern"
+ }
+ timeout {
+ verbose -log "APB: Didn't find running message"
+ }
+ }
+}
+
+# Interact with the main UI.
+with_spawn_id $gdb_main_spawn_id {
+ gdb_test "echo hello\\n" "hello" "interact with GDB's main UI"
+}
+
+# Now kill the gdbserver.
+remote_exec target "kill -9 $gdbserver_pid"
+
+# We expect to land back at a GDB prompt in both UIs. Right now there
+# seems to be some "issues" with the prompt automatically being
+# displayed... Or maybe this is intentional and I'm missunderstanding
+# the expected behaviour. Either way, getting a prompt isn't the
+# point of this test. The point is that we should not be able to
+# interact with GDB from either interpreter now.
+
+with_spawn_id $gdb_main_spawn_id {
+ gdb_test "echo" "" \
+ "main UI, prompt after gdbserver dies"
+}
+
+with_spawn_id $extra_spawn_id {
+ gdb_test "echo" "" \
+ "extra UI, prompt after gdbserver dies"
+}