RE: [PATCH v3] gdb, python: selectively omit enabling stdin in gdb.execute exception

["Aktemur, Tankut Baris" <tankut.baris.aktemur@intel.com>]

Kindly pinging.

Regards
-Baris

On Friday, March 31, 2023 10:11 AM, Aktemur, Tankut Baris 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>
> Reviewed-By: Bruno Larsen <blarsen@redhat.com>
> ---
>  gdb/python/python.c                           | 29 +++++++++++--
>  gdb/testsuite/gdb.python/py-cmd-exception.c   | 22 ++++++++++
>  gdb/testsuite/gdb.python/py-cmd-exception.exp | 43 +++++++++++++++++++
>  gdb/testsuite/gdb.python/py-cmd-exception.py  | 33 ++++++++++++++
>  4 files changed, 123 insertions(+), 4 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.python/py-cmd-exception.c
>  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 b295ff88743..5b34d142d1d 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;
> @@ -698,10 +703,26 @@ execute_gdb_command (PyObject *self, PyObject *args, PyObject *kw)
>      {
>        /* 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 ();
> +	 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:
> +
> +	 User runs "continue"
> +	 --> command blocks the prompt
> +	 --> Python API is invoked, e.g.  via events
> +	 --> gdb.execute(C) invoked inside Python
> +	 --> command C 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.c b/gdb/testsuite/gdb.python/py-cmd-
> exception.c
> new file mode 100644
> index 00000000000..f151960ce3a
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-cmd-exception.c
> @@ -0,0 +1,22 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 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/>.  */
> +
> +int
> +main ()
> +{
> +  return 0;
> +}
> 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..3b81131ee08
> --- /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
> +
> +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..51199bd3fe3
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-cmd-exception.py
> @@ -0,0 +1,33 @@
> +# 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
> +
> +        # There is no variable 'a'.  The command raises an exception.
> +        gdb.execute('print a')
> +
> +the_listener = MyListener()
> --
> 2.25.1

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