[PATCH] gdb: Fix from_tty argument to gdb.execute in Python.
Andrew Burgess
andrew.burgess@embecosm.com
Tue Sep 22 13:51:57 GMT 2020
* Gareth Rees <grees@undo.io> [2020-09-22 11:23:43 +0100]:
> Prior to commit 56bcdbea2b, the from_tty keyword argument to the
> Python function gdb.execute controlled whether the command took input
> from the terminal. When from_tty=True, the "starti" command prompted
> the user:
>
> (gdb) python gdb.execute("starti", from_tty=True)
> The program being debugged has been started already.
> Start it from the beginning? (y or n) y
> Starting program: /bin/true
>
> Program stopped.
>
> When from_tty=False, it did not prompt the user, and "yes" was assumed:
>
> (gdb) python gdb.execute('starti', from_tty=False)
>
> Program stopped.
>
> However, after commit 56bcdbea2b, the from_tty keyword argument no
> longer had this effect. For example, as of commit 7ade7fba75:
>
> (gdb) python gdb.execute('starti', from_tty=True)
> The program being debugged has been started already.
> Start it from the beginning? (y or n) [answered Y; input not from terminal]
> Starting program: /bin/true
>
> Program stopped.
>
> Note the "[answered Y; input not from terminal]" in the output even
> though from_tty=True was requested.
>
> Looking at commit 56bcdbea2b, it seems that the behaviour of the
> from_tty argument was changed accidentally. The commit message said:
>
> Let gdb.execute handle multi-line commands
>
> This changes the Python API so that gdb.execute can now handle
> multi-line commands, like "commands" or "define".
>
> and there was no mention of changing the effect of the from_tty
> argument. It looks as though the code for setting the instream to 0
> was accidentally moved from execute_user_command() to
> execute_control_commands() along with the code for iterating over a
> series of command lines.
>
> Accordingly, the simplest way to fix this is to partially reverse
> commit 56bcdbea2b by moving the code for setting the instream to 0
> back to execute_user_command() where it was to begin with.
>
> Additionally, add a test case to reduce the risk of similar breakage
> in future.
>
> gdb/ChangeLog:
>
> * cli/cli-script.c (execute_control_commands): don't set
> instream to 0 here as this breaks the from_tty argument to
> gdb.execute in Python.
> (execute_user_command): set instream to 0 here instead.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.python/python.exp: add test cases for the from_tty
> argument to gdb.execute.
> ---
> gdb/cli/cli-script.c | 16 ++++++++--------
> gdb/testsuite/gdb.python/python.exp | 13 +++++++++++++
> 2 files changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
> index da4a41023a..4adcda85e6 100644
> --- a/gdb/cli/cli-script.c
> +++ b/gdb/cli/cli-script.c
> @@ -392,14 +392,6 @@ execute_cmd_post_hook (struct cmd_list_element *c)
> void
> execute_control_commands (struct command_line *cmdlines, int from_tty)
> {
> - /* Set the instream to 0, indicating execution of a
> - user-defined function. */
> - scoped_restore restore_instream
> - = make_scoped_restore (¤t_ui->instream, nullptr);
> - scoped_restore save_async = make_scoped_restore (¤t_ui->async, 0);
> - scoped_restore save_nesting
> - = make_scoped_restore (&command_nest_depth, command_nest_depth + 1);
> -
> while (cmdlines)
> {
> enum command_control_type ret = execute_control_command (cmdlines,
> @@ -464,6 +456,14 @@ execute_user_command (struct cmd_list_element *c, const char *args)
> if (user_args_stack.size () > max_user_call_depth)
> error (_("Max user call depth exceeded -- command aborted."));
>
> + /* Set the instream to 0, indicating execution of a
> + user-defined function. */
> + scoped_restore restore_instream
> + = make_scoped_restore (¤t_ui->instream, nullptr);
> + scoped_restore save_async = make_scoped_restore (¤t_ui->async, 0);
> + scoped_restore save_nesting
> + = make_scoped_restore (&command_nest_depth, command_nest_depth + 1);
I'm happy with the change to instream being moved, however, I'm not
sure about the other two lines. command_nest_depth especially I think
should be left where it is now, with current HEAD my gdb session looks
like this:
....
(gdb) set trace-commands on
(gdb) python gdb.execute ("starti", False)
+python gdb.execute ("starti", False)
++starti
Program stopped.
And with your patch:
...
(gdb) set trace-commands on
(gdb) python gdb.execute ("starti", False)
+python gdb.execute ("starti", False)
+starti
Program stopped.
I think the version in current HEAD is better.
I'm tempted to think the async change should also remain in its
current location. I believe this means that if GDB is in async mode,
a Python gdb.execute of something like next/continue will operate in
synchronous mode before returning. Though I don't know if that would
mean there's no way to initiate async control from Python....
... I think you might want to see what others have to say on this.
Thanks,
Andrew
> +
> execute_control_commands (cmdlines, 0);
> }
>
> diff --git a/gdb/testsuite/gdb.python/python.exp b/gdb/testsuite/gdb.python/python.exp
> index a031ea5a18..017f33afe5 100644
> --- a/gdb/testsuite/gdb.python/python.exp
> +++ b/gdb/testsuite/gdb.python/python.exp
> @@ -526,3 +526,16 @@ gdb_test "print \$cvar3" "= void" \
> # Test PR 23669, the following would invoke the "commands" command instead of
> # "show commands".
> gdb_test "python gdb.execute(\"show commands\")" "$decimal print \\\$cvar3.*"
> +
> +# Test that the from_tty argument to gdb.execute is effective. If
> +# False, the user is not prompted for decisions such as restarting the
> +# program, and "yes" is assumed. If True, the user is prompted.
> +gdb_test "python gdb.execute('starti', from_tty=False)" \
> + "Program stopped.*" \
> + "starti via gdb.execute, not from tty"
> +gdb_test_multiple "python gdb.execute('starti', from_tty=True)" \
> + "starti via gdb.execute, from tty" {
> + -re {The program being debugged has been started already\.\r\nStart it from the beginning\? \(y or n\) $} {
> + gdb_test "y" "Starting program:.*" "starti via interactive input"
> + }
> +}
> --
> 2.26.0
>
More information about the Gdb-patches
mailing list