From: Andrew Burgess Date: Tue, 19 Nov 2019 11:17:20 +0000 (+0000) Subject: gdb: Enable stdin on exception in execute_gdb_command X-Git-Url: http://drtracing.org/?a=commitdiff_plain;h=1ba1ac88011703abcd0271e4f5d00927dc69a09a;p=deliverable%2Fbinutils-gdb.git gdb: Enable stdin on exception in execute_gdb_command This is an update of this patch: https://sourceware.org/ml/gdb-patches/2018-09/msg00884.html This patch attempts to address PR gdb/23718 by re-enabling stdin whenever an exception is caught during gdb.execute(). When Python gdb.execute() is called, an exception could occur (e.g. the target disappearing), which is then converted into a Python exception. If stdin was disabled before the exception is caught, it is not re-enabled, because the exception doesn't propagate to the top level of the event loop, whose catch block would otherwise enable it. The result is that when execution of a Python script completes, GDB does not prompt or accept input, and is effectively hung. This change rectifies the issue by re-enabling stdin in the catch block of execute_gdb_command, prior to converting the exception to a Python exception. Since this patch was originally posted I've added a test, and also I converted the code to re-enable stdin from this: SWITCH_THRU_ALL_UIS () { async_enable_stdin (); } to simply this: async_enable_stdin (); My reasoning is that we only need the SWITCH_THRU_ALL_UIS if, at the time the exception is caught, the current_ui might be different than at the time we called async_disable_stdin. Within python's execute_gdb_command I think it should be impossible to switch current_ui, so the SWITCH_THRU_ALL_UIS isn't needed. gdb/ChangeLog: PR gdb/23718 * gdb/python/python.c (execute_gdb_command): Call async_enable_stdin in catch block. gdb/testsuite/ChangeLog: PR gdb/23718 * gdb.server/server-kill-python.exp: New file. Change-Id: I1cfc36ee9f8484cc1ed82be9be338353db6bc080 --- diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 20304ced1f..ca2e49eb50 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,9 @@ +2020-01-24 Graham Markall + + PR gdb/23718 + * gdb/python/python.c (execute_gdb_command): Call + async_enable_stdin in catch block. + 2020-01-24 Andrew Burgess * event-loop.c (start_event_loop): Wrap async_enable_stdin with diff --git a/gdb/python/python.c b/gdb/python/python.c index f75c7b1706..27d6042c61 100644 --- a/gdb/python/python.c +++ b/gdb/python/python.c @@ -624,6 +624,12 @@ execute_gdb_command (PyObject *self, PyObject *args, PyObject *kw) } catch (const gdb_exception &except) { + /* If an exception occurred then we won't hit normal_stop (), or have + an exception reach the top level of the event loop, which are the + two usual places in which stdin would be re-enabled. So, before we + convert the exception and continue back in Python, we should + re-enable stdin here. */ + async_enable_stdin (); GDB_PY_HANDLE_EXCEPTION (except); } diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index facd32fe17..3a97848060 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2020-01-24 Andrew Burgess + + PR gdb/23718 + * gdb.server/server-kill-python.exp: New file. + 2020-01-24 Andrew Burgess * gdb.server/multi-ui-errors.c: New file. diff --git a/gdb/testsuite/gdb.server/server-kill-python.exp b/gdb/testsuite/gdb.server/server-kill-python.exp new file mode 100644 index 0000000000..28064ca0a4 --- /dev/null +++ b/gdb/testsuite/gdb.server/server-kill-python.exp @@ -0,0 +1,88 @@ +# This testcase is part of GDB, the GNU debugger. +# +# Copyright 2019-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 . + +# This test script exposes a bug where, if gdbserver dies while GDB is +# sourcing a python command like 'gdb.execute ("continue")', then GDB +# will deadlock. + +load_lib gdbserver-support.exp + +standard_testfile multi-ui-errors.c + +if {[skip_gdbserver_tests]} { + return 0 +} + +if {[build_executable "failed to prepare" ${testfile} \ + ${srcfile}] == -1} { + return -1 +} + +# 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] + +# Generate a python script we will later source. +set file1 [standard_output_file file1.py] +set fd [open "$file1" w] +puts $fd \ +"import gdb + +def do_gdb_stuff (): + gdb.execute ('target $gdbserver_protocol $gdbserver_gdbport') + gdb.execute ('continue') + +do_gdb_stuff()" +close $fd + +# Now start GDB, sourcing the python command file we generated above. +# Set the height and width so we don't end up at a paging prompt. +if {[gdb_spawn_with_cmdline_opts \ + "-quiet -iex \"set height 0\" -iex \"set width 0\" -ex \"source $file1\""] != 0} { + fail "spawn" + return +} + +# Wait for the inferior to start up. +with_spawn_id $server_spawn_id { + gdb_test_multiple "" "ensure inferior is running" { + -re "@@XX@@ Inferior Starting @@XX@@" { + pass $gdb_test_name + } + timeout { + fail $gdb_tst_name + } + } +} + +# Now kill the gdbserver. +remote_exec target "kill -9 $gdbserver_pid" + +# Wait for GDB to return to a prompt. +gdb_test_multiple "" "landed at prompt after gdbserver dies" { + -re "$gdb_prompt $" { + pass $gdb_test_name + } + timeout { + fail "$gdb_test_name (timeout)" + } +} + +# Run a simple command to ensure we can interact with GDB. +gdb_test "echo hello\\n" "hello" "can we interact with gdb"