From patchwork Fri Dec 6 10:46:15 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Simon Marchi (Code Review)" X-Patchwork-Id: 36555 Received: (qmail 126326 invoked by alias); 6 Dec 2019 10:46:21 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 126310 invoked by uid 89); 6 Dec 2019 10:46:21 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT autolearn=ham version=3.3.1 spammy=sourcing, sk:server, sk:server-, Wrap X-HELO: mx1.osci.io Received: from polly.osci.io (HELO mx1.osci.io) (8.43.85.229) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 06 Dec 2019 10:46:19 +0000 Received: by mx1.osci.io (Postfix, from userid 994) id BEDF9203C1; Fri, 6 Dec 2019 05:46:17 -0500 (EST) Received: from gnutoolchain-gerrit.osci.io (gnutoolchain-gerrit.osci.io [IPv6:2620:52:3:1:5054:ff:fe06:16ca]) by mx1.osci.io (Postfix) with ESMTP id 36469202E5 for ; Fri, 6 Dec 2019 05:46:16 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by gnutoolchain-gerrit.osci.io (Postfix) with ESMTP id 1920128174 for ; Fri, 6 Dec 2019 05:46:16 -0500 (EST) X-Gerrit-PatchSet: 2 Date: Fri, 6 Dec 2019 05:46:15 -0500 From: "Andrew Burgess (Code Review)" To: gdb-patches@sourceware.org Auto-Submitted: auto-generated X-Gerrit-MessageType: newpatchset Subject: [review v2] gdb: Enable stdin on exception in execute_gdb_command X-Gerrit-Change-Id: I1cfc36ee9f8484cc1ed82be9be338353db6bc080 X-Gerrit-Change-Number: 701 X-Gerrit-ChangeURL: X-Gerrit-Commit: 91a92842b8789b6dde27fccc8c868529e2ba21d5 In-Reply-To: References: Reply-To: andrew.burgess@embecosm.com, gdb-patches@sourceware.org MIME-Version: 1.0 Content-Disposition: inline User-Agent: Gerrit/3.0.3-79-g83ff7f88f1 Message-Id: <20191206104616.1920128174@gnutoolchain-gerrit.osci.io> 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(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 3b4afc8..e68591b 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,9 @@ +2019-12-06 Graham Markall + + PR gdb/23718 + * gdb/python/python.c (execute_gdb_command): Call + async_enable_stdin in catch block. + 2019-12-06 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 fad54e9..65ccc40 100644 --- a/gdb/python/python.c +++ b/gdb/python/python.c @@ -620,6 +620,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); } diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index d3666d9..5b19ddd 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,5 +1,10 @@ 2019-12-06 Andrew Burgess + PR gdb/23718 + * gdb.server/server-kill-python.exp: New file. + +2019-12-06 Andrew Burgess + * gdb.server/multi-ui-errors.c: New file. * gdb.server/multi-ui-errors.exp: 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 0000000..0130459 --- /dev/null +++ b/gdb/testsuite/gdb.server/server-kill-python.exp @@ -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 . + +# 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"