[2/2] gdb, python: selectively omit enabling stdin in gdb.execute exception

Message ID 20230224140410.2647437-2-tankut.baris.aktemur@intel.com
State New
Headers
Series [1/2] gdb, python: do minor modernization in execute_gdb_command |

Commit Message

Aktemur, Tankut Baris Feb. 24, 2023, 2:04 p.m. UTC
  From the Python API, we can execute GDB commands via gdb.execute.  If
the command gives an exception, however, we need to recover the GDB
prompt and enable stdin, because the exception does not reach
top-level GDB or normal_stop.  This was done in commit

  commit 1ba1ac88011703abcd0271e4f5d00927dc69a09a
  Author: Andrew Burgess <andrew.burgess@embecosm.com>
  Date:   Tue Nov 19 11:17:20 2019 +0000

    gdb: Enable stdin on exception in execute_gdb_command

However, we face a glitch if the Python side executes the command in a
context where GDB had emitted a Python event.  As an example, suppose
we have the following objfile event listener, specified in a file
named file.py:

~~~
import gdb

class MyListener:
    def __init__(self):
        gdb.events.new_objfile.connect(self.handle_new_objfile_event)
        self.processed_objfile = False

    def handle_new_objfile_event(self, event):
        if self.processed_objfile:
            return

        print("loading " + event.new_objfile.filename)
        self.processed_objfile = True
        gdb.execute("print a")

the_listener = MyListener()
~~~

The executed command "print a", gives an error because "a" is not
defined.  We use the listener as follows:

  $ gdb -q -ex "source file.py" -ex "run" --args a.out
  Reading symbols from /tmp/a.out...
  Starting program: /tmp/a.out
  loading /lib64/ld-linux-x86-64.so.2
  Python Exception <class 'gdb.error'>: No symbol "a" in current context.
  (gdb) [Inferior 1 (process 3980401) exited normally]

Note how the GDB prompt comes inbetween the exception message and the
inferior's exit message.  We have this obscure behavior, because GDB
continues to execute its flow after emitting the Python event.  In
this case, GDB would enable stdin in the normal way.  Hence, we do not
need to explicitly enable stdin in execute_gdb_command when an
exception occurs.

As a solution, we track whether we are inside emitting a Python event.

With this patch, we see

  $ gdb -q -ex "source file.py" -ex "run" --args a.out
  Reading symbols from /tmp/a.out...
  Starting program: /tmp/a.out
  loading /lib64/ld-linux-x86-64.so.2
  Python Exception <class 'gdb.error'>: No symbol "a" in current context.
  [Inferior 1 (process 3984511) exited normally]
  (gdb)

Regression-tested on X86_64 Linux using the default board file (i.e.  unix).

Co-Authored-By: Oguzhan Karakaya <oguzhan.karakaya@intel.com>
---
 gdb/python/py-event.c                         |  6 +++
 gdb/python/py-event.h                         |  5 +++
 gdb/python/python.c                           |  9 +++-
 gdb/testsuite/gdb.python/py-cmd-exception.exp | 43 +++++++++++++++++++
 gdb/testsuite/gdb.python/py-cmd-exception.py  | 32 ++++++++++++++
 5 files changed, 93 insertions(+), 2 deletions(-)
 create mode 100644 gdb/testsuite/gdb.python/py-cmd-exception.exp
 create mode 100644 gdb/testsuite/gdb.python/py-cmd-exception.py
  

Comments

Tom Tromey Feb. 24, 2023, 7:43 p.m. UTC | #1
>>>>> "Tankut" == Tankut Baris Aktemur via Gdb-patches <gdb-patches@sourceware.org> writes:

Tankut> +/* See py-event.h.  */
Tankut> +
Tankut> +bool in_evpy_emit_event = false;
 
It seems like there has to be a better way than introducing this global.

For one thing, are we sure this is the only possible context where this
bug can occur?

I wonder if execute_gdb_command could instead determine if stdin is
enabled before the 'try', and only re-enabled it if it was.

Tom
  
Terekhov, Mikhail via Gdb-patches Feb. 27, 2023, 9:53 a.m. UTC | #2
On Friday, February 24, 2023 8:43 PM, Tom Tromey wrote:
> >>>>> "Tankut" == Tankut Baris Aktemur via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Tankut> +/* See py-event.h.  */
> Tankut> +
> Tankut> +bool in_evpy_emit_event = false;
> 
> It seems like there has to be a better way than introducing this global.
> 
> For one thing, are we sure this is the only possible context where this
> bug can occur?
> 
> I wonder if execute_gdb_command could instead determine if stdin is
> enabled before the 'try', and only re-enabled it if it was.

This would work, I think.  We have the following state:

  User runs a synchronous execution command such as "continue", "run", etc.
  --> command blocks the prompt
  --> Python API is invoked, e.g.  via events
  --> gdb.execute invoked inside Python
  --> command raises an exception
  --> we're inside the 'catch' block.

Before running the command that was passed to us with gdb.execute,
current_ui's prompt_state is already PROMPT_BLOCKED.  It seems we can
use this to omit enabling stdin.

I'll send the change as v2.

Thank you.
-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
  

Patch

diff --git a/gdb/python/py-event.c b/gdb/python/py-event.c
index 3ff31acf85b..6daf504398b 100644
--- a/gdb/python/py-event.c
+++ b/gdb/python/py-event.c
@@ -75,6 +75,9 @@  gdbpy_initialize_event_generic (PyTypeObject *type,
   return gdb_pymodule_addobject (gdb_module, name, (PyObject *) type);
 }
 
+/* See py-event.h.  */
+
+bool in_evpy_emit_event = false;
 
 /* Notify the list of listens that the given EVENT has occurred.
    returns 0 if emit is successful -1 otherwise.  */
@@ -85,6 +88,9 @@  evpy_emit_event (PyObject *event,
 {
   Py_ssize_t i;
 
+  scoped_restore save_flag
+    = make_scoped_restore (&in_evpy_emit_event, true);
+
   /* Create a copy of call back list and use that for
      notifying listeners to avoid skipping callbacks
      in the case of a callback being disconnected during
diff --git a/gdb/python/py-event.h b/gdb/python/py-event.h
index 0a7d31d02e9..8e42bcd00dd 100644
--- a/gdb/python/py-event.h
+++ b/gdb/python/py-event.h
@@ -58,6 +58,11 @@  extern int emit_inferior_call_event (inferior_call_kind kind,
 extern int emit_register_changed_event (frame_info_ptr frame,
 					int regnum);
 extern int emit_memory_changed_event (CORE_ADDR addr, ssize_t len);
+
+/* Flag to indicate whether we are currently handling emitted Python
+   events.  */
+extern bool in_evpy_emit_event;
+
 extern int evpy_emit_event (PyObject *event,
 			    eventregistry_object *registry);
 
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 5719f351528..074b92c4d02 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -700,8 +700,13 @@  execute_gdb_command (PyObject *self, PyObject *args, PyObject *kw)
 	 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 ();
+	 re-enable stdin here, unless we are currently handling emitted
+	 Python events.  In that case, GDB would continue its normal course
+	 of execution and enable stdin itself in the way it normally
+	 does.  */
+      if (!in_evpy_emit_event)
+	async_enable_stdin ();
+
       GDB_PY_HANDLE_EXCEPTION (except);
     }
 
diff --git a/gdb/testsuite/gdb.python/py-cmd-exception.exp b/gdb/testsuite/gdb.python/py-cmd-exception.exp
new file mode 100644
index 00000000000..9cb4fd6b233
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-cmd-exception.exp
@@ -0,0 +1,43 @@ 
+# Copyright (C) 2023 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 file is part of the GDB testsuite.  It tests a corner case where
+# the executed GDB command gives an exception and enabling the stdin causes
+# the GDB prompt to be displayed prematurely.
+
+load_lib gdb-python.exp
+
+require !use_gdb_stub allow_python_tests
+
+standard_testfile py-cmd.c
+
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
+    return -1
+}
+
+set pyfile [gdb_remote_download host ${srcdir}/${subdir}/${testfile}.py]
+gdb_test_no_output "source $pyfile" "source the script"
+
+gdb_start_cmd
+
+gdb_test_multiple "" "check the prompt" {
+    -re "breakpoint $decimal, main .*\r\n$gdb_prompt $" {
+	# The prompt is positioned correctly.
+	pass $gdb_test_name
+    }
+    -re "No symbol \"a\" in current context.\r\n$gdb_prompt " {
+	fail $gdb_test_name
+    }
+}
diff --git a/gdb/testsuite/gdb.python/py-cmd-exception.py b/gdb/testsuite/gdb.python/py-cmd-exception.py
new file mode 100644
index 00000000000..846c41e07de
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-cmd-exception.py
@@ -0,0 +1,32 @@ 
+# Copyright (C) 2023 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/>.
+
+import gdb
+
+class MyListener:
+    def __init__(self):
+        gdb.events.new_objfile.connect(self.handle_new_objfile_event)
+        self.processed_objfile = False
+
+    def handle_new_objfile_event(self, event):
+        if self.processed_objfile:
+            return
+
+        print('loading ' + event.new_objfile.filename)
+        self.processed_objfile = True
+
+        gdb.execute('print a')
+
+the_listener = MyListener()