gdb: Enable stdin on exception in execute_gdb_command
authorAndrew Burgess <andrew.burgess@embecosm.com>
Tue, 19 Nov 2019 11:17:20 +0000 (11:17 +0000)
committerAndrew Burgess <andrew.burgess@embecosm.com>
Fri, 24 Jan 2020 00:11:53 +0000 (00:11 +0000)
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

gdb/ChangeLog
gdb/python/python.c
gdb/testsuite/ChangeLog
gdb/testsuite/gdb.server/server-kill-python.exp [new file with mode: 0644]

index 20304ced1f257dd408360d2c9485df3d4c8836d5..ca2e49eb508a913b6ccc15cbce3ee2c2460fc579 100644 (file)
@@ -1,3 +1,9 @@
+2020-01-24  Graham Markall  <graham.markall@embecosm.com>
+
+       PR gdb/23718
+       * gdb/python/python.c (execute_gdb_command): Call
+       async_enable_stdin in catch block.
+
 2020-01-24  Andrew Burgess  <andrew.burgess@embecosm.com>
 
        * event-loop.c (start_event_loop): Wrap async_enable_stdin with
index f75c7b170630e58063f7956ad08bae584a0f039b..27d6042c618887cf1a31546d7737bf805196824e 100644 (file)
@@ -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);
     }
 
index facd32fe178e0216cd8793353acb6887782d51f0..3a97848060121a68c12498aa44e26de877319699 100644 (file)
@@ -1,3 +1,8 @@
+2020-01-24  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+       PR gdb/23718
+       * gdb.server/server-kill-python.exp: New file.
+
 2020-01-24  Andrew Burgess  <andrew.burgess@embecosm.com>
 
        * 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 (file)
index 0000000..28064ca
--- /dev/null
@@ -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 <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]
+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"