Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Gareth Rees <grees@undo.io>
To: Andrew Burgess <andrew.burgess@embecosm.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH] gdb: Fix from_tty argument to gdb.execute in Python.
Date: Thu, 24 Sep 2020 12:16:32 +0100	[thread overview]
Message-ID: <CAA8DSpFzDGNhyZESWFO6fw++65r_tbz7YXv-_Wn+V2btL88Y0A@mail.gmail.com> (raw)
In-Reply-To: <20200924110912.GK1540618@embecosm.com>

[-- Attachment #1: Type: text/plain, Size: 90 bytes --]

Here's a patch with a comment and commit message updated to use "nullptr"
instead of "0".

[-- Attachment #2: 0001-gdb-Fix-from_tty-argument-to-gdb.execute-in-Python.patch --]
[-- Type: text/x-patch, Size: 4873 bytes --]

From 7efaa3657c8b72c128675b27a2cb2f21e2e05b14 Mon Sep 17 00:00:00 2001
From: Gareth Rees <grees@undo.io>
Date: Tue, 8 Sep 2020 15:52:44 +0100
Subject: [PATCH] gdb: Fix from_tty argument to gdb.execute in Python.

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, "starti" and similar commands
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, these commands 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
nullptr was accidentally moved from execute_user_command() to
execute_control_commands() along with the other scoped restores.

Accordingly, the simplest way to fix this is to partially reverse
commit 56bcdbea2b by moving the code for setting the instream to
nullptr 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 nullptr here as this breaks the from_tty argument
        to gdb.execute in Python.
        (execute_user_command): set instream to nullptr 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                |  9 +++++----
 gdb/testsuite/gdb.python/python.exp | 13 +++++++++++++
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index da4a41023a..f8ac610d4d 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -392,10 +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 (&current_ui->instream, nullptr);
   scoped_restore save_async = make_scoped_restore (&current_ui->async, 0);
   scoped_restore save_nesting
     = make_scoped_restore (&command_nest_depth, command_nest_depth + 1);
@@ -464,6 +460,11 @@ 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 nullptr, indicating execution of a
+     user-defined function.  */
+  scoped_restore restore_instream
+    = make_scoped_restore (&current_ui->instream, nullptr);
+
   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


  reply	other threads:[~2020-09-24 11:16 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-22 10:23 Gareth Rees
2020-09-22 13:51 ` Andrew Burgess
2020-09-22 15:58   ` Gareth Rees
2020-09-23 11:20     ` Gareth Rees
2020-09-24 11:09       ` Andrew Burgess
2020-09-24 11:16         ` Gareth Rees [this message]
2020-09-25 12:53           ` Andrew Burgess
2020-09-25 14:31             ` Gareth Rees
2020-09-26 18:07               ` Joel Brobecker
2020-09-27 16:39                 ` Andrew Burgess
2020-09-28 20:16                   ` Joel Brobecker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAA8DSpFzDGNhyZESWFO6fw++65r_tbz7YXv-_Wn+V2btL88Y0A@mail.gmail.com \
    --to=grees@undo.io \
    --cc=andrew.burgess@embecosm.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox