[review] gdb: Enable stdin on exception in execute_gdb_command
Commit Message
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/701
......................................................................
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
---
M gdb/ChangeLog
M gdb/python/python.c
M gdb/testsuite/ChangeLog
A gdb/testsuite/gdb.server/server-kill-python.exp
4 files changed, 98 insertions(+), 0 deletions(-)
Comments
Andrew Burgess has abandoned this change. ( https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/701 )
Change subject: gdb: Enable stdin on exception in execute_gdb_command
......................................................................
Abandoned
I've now posted this to the mailing list.
@@ -1,3 +1,9 @@
+2019-11-22 Graham Markall <graham.markall@embecosm.com>
+
+ PR gdb/23718
+ * gdb/python/python.c (execute_gdb_command): Call
+ async_enable_stdin in catch block.
+
2019-11-22 Andrew Burgess <andrew.burgess@embecosm.com>
* event-loop.c (start_event_loop): Wrap async_enable_stdin with
@@ -621,6 +621,12 @@
}
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);
}
@@ -1,5 +1,10 @@
2019-11-22 Andrew Burgess <andrew.burgess@embecosm.com>
+ PR gdb/23718
+ * gdb.server/server-kill-python.exp: New file.
+
+2019-11-22 Andrew Burgess <andrew.burgess@embecosm.com>
+
* gdb.server/multi-ui-errors.c: New file.
* gdb.server/multi-ui-errors.exp: New file.
new file mode 100644
@@ -0,0 +1,81 @@
+# 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/>.
+
+# 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]
+
+# 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
+
+# Figure out the pid for the gdbserver, and arrange to kill it in a
+# short while.
+set gdbserver_pid [exp_pid -i $server_spawn_id]
+after 1000 { remote_exec target "kill -9 $gdbserver_pid" }
+
+# 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
+}
+
+# Expect that we get back to a GDB prompt. We can't use
+# gdb_test_multiple here as we don't need to send a command to GDB;
+# the script we source at startup issues a command for us. Here we
+# really are waiting for GDB to give us back a prompt.
+set testname "landed at prompt after gdbserver dies"
+gdb_expect 10 {
+ -re "$gdb_prompt $" {
+ pass $testname
+ }
+ timeout {
+ fail "$testname (timeout)"
+ }
+}
+
+# Run a simple command to ensure we can interact with GDB.
+gdb_test "echo hello\\n" "hello" "can we interact with gdb"