Re: [patch] python prompt additions at first prompt.

Matt Rice
2011-08-29T16:23:12Z

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

On Fri, Aug 12, 2011 at 8:07 AM, Matt Rice <ratmice@gmail.com> wrote:
> On Fri, Aug 12, 2011 at 7:44 AM, Pedro Alves <pedro@codesourcery.com> wrote:
>> On Friday 12 August 2011 14:10:17, Matt Rice wrote:
>>> it seems clearer to return early in all sync_execution cases, and
>>> limit the potential for introducing the double prompting type of bug.
>>> I haven't been able to find any problems with this approach.
>>
>> I think this breaks the pagination prompt in async mode.  Try setting
>> a breakpoint in a loop, with a command list that just continues:
>>
>> (gdb) b inloop
>> (gdb) commands
>> Type commands for breakpoint(s) 2, one per line.
>> End with a line saying just "end".
>>>c
>>>end
>> (gdb) c
>
> Yeah it does, thank you for pointing this out.
>

Here is a version of my previous patch, which doesn't suffer from this failure.
I have added comments explaining that we really do want to display an
empty prompt in this case.

hopefully this clears up any confusion...

2011-08-28 Matt Rice <ratmice@gmail.com>

* event-top.c (cli_command_loop): Replace readline setup with
direct call to display_gdb_prompt.
(display_gdb_prompt): Do not call observer mechanism during
synchronous execution.

2011-08-28 Matt Rice <ratmice@gmail.com>

* lib/prompt.exp: New file for testing the first prompt.
* gdb.python/py-prompt.exp: Ditto.
* gdb.python/py-prompt.c: Ditto (copy of ext-attach.c).

[-- Attachment #2: foo.diff --]
[-- Type: text/x-patch, Size: 11518 bytes --]

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 37882728..6496988 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -185,27 +185,7 @@ rl_callback_read_char_wrapper (gdb_client_data client_data)
void
cli_command_loop (void)
{
- /* If we are using readline, set things up and display the first
- prompt, otherwise just print the prompt. */
- if (async_command_editing_p)
- {
- int length;
- char *a_prompt;
- char *gdb_prompt = get_prompt (0);
-
- /* Tell readline what the prompt to display is and what function
- it will need to call after a whole line is read. This also
- displays the first prompt. */
- length = strlen (get_prefix (0))
- + strlen (gdb_prompt) + strlen (get_suffix(0)) + 1;
- a_prompt = (char *) alloca (length);
- strcpy (a_prompt, get_prefix (0));
- strcat (a_prompt, gdb_prompt);
- strcat (a_prompt, get_suffix (0));
- rl_callback_handler_install (a_prompt, input_handler);
- }
- else
- display_gdb_prompt (0);
+ display_gdb_prompt (0);

/* Now it's time to start the event loop. */
start_event_loop ();
@@ -272,8 +252,11 @@ display_gdb_prompt (char *new_prompt)
/* Get the prompt before the observers are called as observer hook
functions may change the prompt. Do not call observers on an
explicit prompt change as passed to this function, as this forms
- a temporary prompt, IE, displayed but not set. */
- if (! new_prompt)
+ a temporary prompt, IE, displayed but not set. Do not call
+ observers for a prompt change if sync_execution is set, it will
+ call us again with sync_execution not set when it wants to
+ display an actual prompt. */
+ if (! sync_execution && ! new_prompt)
{
char *post_gdb_prompt = NULL;
char *pre_gdb_prompt = xstrdup (get_prompt (0));
@@ -288,6 +271,10 @@ display_gdb_prompt (char *new_prompt)
xfree (pre_gdb_prompt);
}

+ /* In the sync_execution && !is_running case we need to display the prompt
+ event though it may be "" to avoid a double prompt, while installing the
+ callback handlers, in the async_editing_p case for pagination,
+ So fall through. */
if (sync_execution && is_running (inferior_ptid))
{
/* This is to trick readline into not trying to display the
diff --git a/gdb/testsuite/gdb.python/py-prompt.c b/gdb/testsuite/gdb.python/py-prompt.c
new file mode 100644
index 0000000..8d84f09
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-prompt.c
@@ -0,0 +1,31 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2007, 2009, 2010, 2011 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 program is intended to be started outside of gdb, and then
+ attached to by gdb. It loops for a while, but not forever. */
+
+#include <unistd.h>
+
+int main ()
+{
+ int i;
+
+ for (i = 0; i < 120; i++)
+ sleep (1);
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-prompt.exp b/gdb/testsuite/gdb.python/py-prompt.exp
new file mode 100644
index 0000000..c1ca105
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-prompt.exp
@@ -0,0 +1,131 @@
+# Copyright (C) 2011 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 the mechanism
+# for defining the prompt in Python.
+
+if $tracelevel then {
+ strace $tracelevel
+}
+
+set testfile "py-prompt"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+
+
+load_lib gdb-python.exp
+load_lib prompt.exp
+
+# Start with a fresh gdb.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+gdb_exit
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+ untested py-prompt.exp
+ return -1
+}
+
+global GDBFLAGS
+set saved_gdbflags $GDBFLAGS
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set height 0\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set width 0\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"python p = list()\""]
+set prompt_func "python def foo(x): global p; p.append(x); return \'(Foo) \'"
+set GDBFLAGS [concat $GDBFLAGS " -ex \"$prompt_func\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"python gdb.prompt_hook=foo\""]
+
+set tmp_gdbflags $GDBFLAGS
+set gdb_prompt_fail $gdb_prompt
+
+global gdb_prompt
+# Does not include the space at the end of the prompt.
+# gdb_test expects it not to be there.
+set gdb_prompt "\[(\]Foo\[)\]"
+
+set GDBFLAGS [concat $tmp_gdbflags " -ex \"set editing on\""]
+prompt_gdb_start
+gdb_test "python x = len(p); print gdb.execute(\"show prompt\", to_string = True)" \
+ ".*prompt is \"$gdb_prompt \".*" \
+ "show prompt gets the correct result"
+gdb_test "python print x, len(p)" "1 2" \
+ "retrieving the prompt causes no extra prompt_hook calls"
+gdb_test "python print \"'\" + str(p\[0\]) + \"'\"" "'$gdb_prompt_fail '" \
+ "prompt_hook argument is default prompt."
+gdb_exit
+
+
+set GDBFLAGS [concat $tmp_gdbflags " -ex \"set editing off\""]
+prompt_gdb_start
+gdb_test "python x = len(p); print gdb.execute(\"show prompt\", to_string = True)" \
+ ".*prompt is \"$gdb_prompt \".*" \
+ "show prompt gets the correct result 2"
+gdb_test "python print x, len(p)" "1 2" \
+ "retrieving the prompt causes no extra prompt_hook calls 2"
+gdb_test "python print \"'\" + str(p\[0\]) + \"'\"" "'$gdb_prompt_fail '" \
+ "prompt_hook argument is default prompt. 2"
+gdb_exit
+
+# Start the program running and then wait for a bit, to be sure
+# that it can be attached to.
+set testpid [eval exec $binfile &]
+exec sleep 2
+if { [istarget "*-*-cygwin*"] } {
+ # testpid is the Cygwin PID, GDB uses the Windows PID, which might be
+ # different due to the way fork/exec works.
+ set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ]
+}
+
+set GDBFLAGS [concat $tmp_gdbflags " -ex \"set target-async on\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set pagination off\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set editing on\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"attach $testpid\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"continue&\""]
+
+# sync_execution = 1 is_running = 1
+prompt_gdb_start
+gdb_test "python x = len(p); print gdb.execute(\"show prompt\", to_string = True)" \
+ ".*prompt is \"$gdb_prompt \".*" \
+ "show prompt gets the correct result 3"
+gdb_test "python print x, len(p)" "1 2" \
+ "retrieving the prompt causes no extra prompt_hook calls 3"
+gdb_test "python print \"'\" + str(p\[0\]) + \"'\"" "'$gdb_prompt_fail '" \
+ "prompt_hook argument is default prompt. 3"
+gdb_exit
+
+set GDBFLAGS [concat $tmp_gdbflags " -ex \"set target-async on\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set pagination off\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set editing on\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"attach $testpid\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"interrupt\""]
+
+# sync_execution = 1 is_running = 0
+prompt_gdb_start
+gdb_test "python x = len(p); print gdb.execute(\"show prompt\", to_string = True)" \
+ ".*prompt is \"$gdb_prompt \".*" \
+ "show prompt gets the correct result 4"
+gdb_test "python print x, len(p)" "1 2" \
+ "retrieving the prompt causes no extra prompt_hook calls 4"
+gdb_test "python print \"'\" + str(p\[0\]) + \"'\"" "'$gdb_prompt_fail '" \
+ "prompt_hook argument is default prompt. 4"
+gdb_exit
+
+set GDBFLAGS $saved_gdbflags
+return 0
diff --git a/gdb/testsuite/lib/prompt.exp b/gdb/testsuite/lib/prompt.exp
new file mode 100644
index 0000000..727bbb3
--- /dev/null
+++ b/gdb/testsuite/lib/prompt.exp
@@ -0,0 +1,92 @@
+# Copyright (C) 2011 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/>.
+
+# Specialized subroutines for launching gdb and testing the very first prompt.
+
+
+#
+# start gdb -- start gdb running, prompt procedure
+# this procedure differs from the default in that you must pass 'set height 0',
+# and 'set width 0', yourself in GDBFLAGS, and it has a gdb_prompt_fail variable,
+#
+# uses pass if it sees $gdb_prompt, and fail if it sees $gdb_prompt_fail.
+#
+proc default_prompt_gdb_start { } {
+ global verbose
+ global GDB
+ global INTERNAL_GDBFLAGS GDBFLAGS
+ global gdb_prompt
+ global gdb_prompt_fail
+ global timeout
+ global gdb_spawn_id;
+
+ gdb_stop_suppressing_tests;
+
+ verbose "Spawning $GDB $INTERNAL_GDBFLAGS $GDBFLAGS"
+
+ if [info exists gdb_spawn_id] {
+ return 0;
+ }
+
+ if ![is_remote host] {
+ if { [which $GDB] == 0 } then {
+ perror "$GDB does not exist."
+ exit 1
+ }
+ }
+ set res [remote_spawn host "$GDB $INTERNAL_GDBFLAGS $GDBFLAGS [host_info gdb_opts]"];
+ if { $res < 0 || $res == "" } {
+ perror "Spawning $GDB failed."
+ return 1;
+ }
+ gdb_expect 360 {
+ -re ".*$gdb_prompt_fail.*$gdb_prompt_fail.*" {
+ fail "double prompted fail prompt"
+ }
+ -re ".*$gdb_prompt.*$gdb_prompt.*" {
+ fail "double prompted"
+ }
+ -re "\[\r\n\]$gdb_prompt_fail $" {
+ fail "GDB initializing first prompt"
+ }
+ -re "\[\r\n\]$gdb_prompt $" {
+ pass "GDB initializing first prompt"
+ }
+ -re "$gdb_prompt $" {
+ perror "GDB never initialized."
+ return -1
+ }
+ -re "$gdb_prompt_fail $" {
+ perror "GDB never initialized."
+ return -1
+ }
+ timeout {
+ perror "(timeout) GDB never initialized after 10 seconds."
+ remote_close host;
+ return -1
+ }
+ }
+ set gdb_spawn_id -1;
+ return 0;
+}
+
+#
+# Overridable function. You can override this function in your
+# baseboard file.
+#
+proc prompt_gdb_start { } {
+ default_prompt_gdb_start
+}
+