From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id q4ZZIV9+bF8SegAAWB0awg (envelope-from ) for ; Thu, 24 Sep 2020 07:09:19 -0400 Received: by simark.ca (Postfix, from userid 112) id 6C92F1EE05; Thu, 24 Sep 2020 07:09:19 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 3DBE31E509 for ; Thu, 24 Sep 2020 07:09:18 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id E4DD33857C65; Thu, 24 Sep 2020 11:09:17 +0000 (GMT) Received: from mail-wm1-x344.google.com (mail-wm1-x344.google.com [IPv6:2a00:1450:4864:20::344]) by sourceware.org (Postfix) with ESMTPS id DFBF63850408 for ; Thu, 24 Sep 2020 11:09:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org DFBF63850408 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wm1-x344.google.com with SMTP id d4so3083885wmd.5 for ; Thu, 24 Sep 2020 04:09:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=XMmJIgntF07uwufZHCucxSM9JR8LrqRQHiGBvVpxwzY=; b=ICcDXRQ6Xs5bF0gmVnEBzm8QwIf2Q18+5dXYMdsgzrgb/UaoOOVCzQ3GO3YBIjfdDj lAYHlYcqnTbKV1yp/lboh1g3OMKFN7SYgJVOkigmVdmVdelY60SUjus7eM7BrofA1sGz TCVrc6FjMVUjRQxRQYuN3eAgN6MlSDCK8BYwUPA96/llUzw4opCN/frSBQgLNQqOfOpr WWnNy/yTZyXoAdK0n3X1bnwg+VS4LjZd16d6PheUGgs8tsjGjL2w7EyEnwzalFZNHU/u G0si7q8HmwJ1zyhgj0O1EOkbYYHPF2tB4UaY5Bf70Nf3b3N7UR1btxM/FvGCFuUhd9S8 cO2w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=XMmJIgntF07uwufZHCucxSM9JR8LrqRQHiGBvVpxwzY=; b=NzVspX54QkSGg8zQXnn7elH/ndv/3Osn8Reeuckgund7pZ3rh4OSTbpTm8oR6KulM1 o+xzMALeMtx0pMXphkTsTT7N52H8cVnT3vRRyh28KrCRR3IVY07M+mTTHhCezikZ6nJH m2PSeDRFplWcFOtqCx1X4NBOl7F+Tor8NTh28LosI3r/2J1o9nQbcQ/LI5+ptvqObPUg YH1IjIKds8Gtq8AQr3ponyghKl8/1EwdikTZsUv2wczo2wlN/9wO4MdmrGI48aJNjfWZ OcX2y+lTjlcY8RzY/k4EbOjA5/DWeWEZNFFrObUARm8aauCALqOsz6ivI24x1BCfre77 X8/A== X-Gm-Message-State: AOAM530b1ib52mzAPJH4yr5qVZtpXx7VvONWugM6T/y2IeuguyzqkcvM 5HbsFvdRc4Blw1HY8NmIfHXg7HG8wpP8Vg== X-Google-Smtp-Source: ABdhPJzKogVsaR7sZMVQ5R6tidxE83NylLC++UT0TletP5RmoQ6fjfQWqRsQ124pW3D4Wed1dRYF7Q== X-Received: by 2002:a7b:cbcb:: with SMTP id n11mr4126645wmi.5.1600945753829; Thu, 24 Sep 2020 04:09:13 -0700 (PDT) Received: from localhost (host31-53-80-104.range31-53.btcentralplus.com. [31.53.80.104]) by smtp.gmail.com with ESMTPSA id s26sm3032767wmh.44.2020.09.24.04.09.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 24 Sep 2020 04:09:12 -0700 (PDT) Date: Thu, 24 Sep 2020 12:09:12 +0100 From: Andrew Burgess To: Gareth Rees Subject: Re: [PATCH] gdb: Fix from_tty argument to gdb.execute in Python. Message-ID: <20200924110912.GK1540618@embecosm.com> References: <20200922102343.8440-1-grees@undo.io> <20200922135157.GH1540618@embecosm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Operating-System: Linux/5.8.9-101.fc31.x86_64 (x86_64) X-Uptime: 12:06:46 up 5 days, 19:14, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: gdb-patches@sourceware.org Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" * Gareth Rees [2020-09-23 12:20:06 +0100]: > Here's the updated patch with just the instream change. > From 92db8f79ee0dbfaaf689210a4656513a55a82d69 Mon Sep 17 00:00:00 2001 > From: Gareth Rees > 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 0 > 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 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 | 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..97541fcca3 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 (¤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); > @@ -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 0, indicating execution of a > + user-defined function. */ > + scoped_restore restore_instream > + = make_scoped_restore (¤t_ui->instream, nullptr); Could you change the comment to read: /* Set the instream to nullptr, indicating execution of a user-defined function. */ I'm happy to see this merged with this change. If anyone else thinks we should move either of the other two parts back they can do so in a follow up commit. 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 >