[PATCH v2] gdb, python: selectively omit enabling stdin in gdb.execute exception
Aktemur, Tankut Baris
tankut.baris.aktemur@intel.com
Fri Mar 31 08:07:58 GMT 2023
On Wednesday, March 29, 2023 4:03 PM, Bruno Larsen wrote:
> On 27/02/2023 10:56, Tankut Baris Aktemur via Gdb-patches wrote:
> > 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 already disabled stdin, because it was running a
> > synchronous execution command such as "continue" or "run". 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 the prompt was already blocked. If so,
> > we leave enabling stdin to GDB.
> >
> > 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/python.c | 26 ++++++++++-
> > gdb/testsuite/gdb.python/py-cmd-exception.exp | 43 +++++++++++++++++++
> > gdb/testsuite/gdb.python/py-cmd-exception.py | 33 ++++++++++++++
> > 3 files changed, 100 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
> >
> > diff --git a/gdb/python/python.c b/gdb/python/python.c
> > index 1ed13f2789b..9a0cc1e0d4e 100644
> > --- a/gdb/python/python.c
> > +++ b/gdb/python/python.c
> > @@ -653,6 +653,11 @@ execute_gdb_command (PyObject *self, PyObject *args, PyObject *kw)
> >
> > scoped_restore preventer = prevent_dont_repeat ();
> >
> > + /* If the executed command raises an exception, we may have to
> > + enable stdin and recover the GDB prompt. Check the current
> > + state. */
> > + bool prompt_was_blocked = (current_ui->prompt_state == PROMPT_BLOCKED);
> > +
> > try
> > {
> > gdbpy_allow_threads allow_threads;
> > @@ -700,8 +705,25 @@ 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 the prompt was already blocked before
> > + we started executing the command. This could be the case, for
>
> I feel like this explanation is a little backwards. "we will do A before
> B, unless C" is a bit confusing in my opinion. I think the comment could
> be reworded to
>
> "two usual places in which stdin would be re-enabled. So we check here
> if stdin should be re-enabled, and do it if it is the case. An example
> of when stdin should not be re-enabled is if we are currently (...)"
I have updated the comment as follows:
+ two usual places in which stdin would be re-enabled. So, we check
+ here if stdin should be re-enabled, and do so if it is the case.
+ Stdin should not be re-enabled if it is already blocked because,
+ for example, we are running a command in the context of a
+ synchronous execution command ("run", "continue", etc.). Like
+ this:
...
> > + instance, if we are currently handling emitted Python events inside
> > + a synchronous execution command ("run", "continue", etc.).
> > + Like this:
> > +
> > + User runs "continue"
> > + --> command blocks the prompt
> > + --> Python API is invoked, e.g. via events
> > + --> gdb.execute invoked inside Python
> > + --> command raises an exception
> > + --> this location
> > +
> > + In this case case, GDB would go back to the top "continue" command
> > + and move on with its normal course of execution. That is, it
> > + would enable stdin in the way it normally does. */
> > + if (!prompt_was_blocked)
> > + 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..6ab1970b26b
> > --- /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 would
> > +# cause the GDB prompt to be displayed prematurely.
> > +
> > +load_lib gdb-python.exp
> > +
> > +require !use_gdb_stub allow_python_tests
> > +
> > +standard_testfile py-cmd.c
>
> I think we like to avoid re-using test files, because it could lead to
> issues with hard-to-notice dependencies and such. I think making a
> minimal py-cmd-exception.c with a 2-line main function would probably be
> enough, right?
I've added a separate test file.
> With these changes, you can add my rb tag:
> Reviewed-By: Bruno Larsen <blarsen@redhat.com>
>
> --
> Cheers,
> Bruno
Thanks for the review.
-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
More information about the Gdb-patches
mailing list