From patchwork Mon Dec 23 01:45:39 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 37072 Received: (qmail 14200 invoked by alias); 23 Dec 2019 01:45:50 -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 14148 invoked by uid 89); 23 Dec 2019 01:45:49 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=fd, landed, gdb.execute, gdbexecute X-HELO: mail-wm1-f43.google.com Received: from mail-wm1-f43.google.com (HELO mail-wm1-f43.google.com) (209.85.128.43) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 23 Dec 2019 01:45:47 +0000 Received: by mail-wm1-f43.google.com with SMTP id a5so14422723wmb.0 for ; Sun, 22 Dec 2019 17:45:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :in-reply-to:references; bh=wBCcTWls3HjteWQDArxI4OKt7WO5jfeAdPnvl4p3w/g=; b=FhpbkFCcOqd4phpXJl7FrQZJljYh0jpIkDgINucK++fjtqQt5Y2Ssum2cdzzLAcIGC hqt52bu/5huF7D/9BacFYODk6kBs1HEaNGABw8SLFsYcWjEIX7Qq3OH2JzvntZw+2DaQ 1fsbmF//qJb0ehrFxq/sUxcTrKELh2N7te1MAb1y62t0bxVV1jN0Atf2FQ5KYotdhcsR PEgX/2Br8MwU8j3zxzoAYmtjnmsEtTk+9f10OVj0mC4taWN5/0aYCJKQ4fP8Ogdqw2jG yaKCppmVaoJgJKl98G/VWGcP7Vzo4WqBmiyHLAYDmolFCz3UoQ7hv24DLw+0pnikA6a+ mBpg== Return-Path: Received: from localhost (host86-186-80-236.range86-186.btcentralplus.com. [86.186.80.236]) by smtp.gmail.com with ESMTPSA id x11sm19778643wre.68.2019.12.22.17.45.44 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sun, 22 Dec 2019 17:45:44 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH 2/2] gdb: Enable stdin on exception in execute_gdb_command Date: Mon, 23 Dec 2019 01:45:39 +0000 Message-Id: <4c15d65a2baa7402b04f1449ccbfe44779c5d711.1577065294.git.andrew.burgess@embecosm.com> In-Reply-To: References: In-Reply-To: References: X-IsSubscribed: yes 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 | 6 ++ gdb/python/python.c | 6 ++ gdb/testsuite/ChangeLog | 5 ++ gdb/testsuite/gdb.server/server-kill-python.exp | 81 +++++++++++++++++++++++++ 4 files changed, 98 insertions(+) create mode 100644 gdb/testsuite/gdb.server/server-kill-python.exp diff --git a/gdb/python/python.c b/gdb/python/python.c index fad54e9cdb0..65ccc404b15 100644 --- a/gdb/python/python.c +++ b/gdb/python/python.c @@ -620,6 +620,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/gdb.server/server-kill-python.exp b/gdb/testsuite/gdb.server/server-kill-python.exp new file mode 100644 index 00000000000..0130459fce4 --- /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"