This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
PR gdb/13175 (Re: Eliminate tui_command_loop)
On Sunday 11 September 2011 14:24:22, Matt Rice wrote:
> On Sat, Sep 10, 2011 at 11:47 AM, Matt Rice <ratmice@gmail.com> wrote:
>
> >
> > I could easily be missing something though and it's fine.
> >
>
> ^ correct answer... it's handled in tui_setup_io().
Yep.
> the attached patch based on Pedro's WIP, fixes a small case that he missed.
> and makes interp.exp hit all of these failures
Thanks!
> this is PR gdb/13175
> (though Nick's reproducer hits the case you missed).
Okay.
> I modified the existing prompt checks in this file, because the
> gdb_expect was causing the test to take 30 seconds to complete, which
> is now down to 0 seconds... hopefully that change is ok...
Sorry, it isn't. You got the long delay because your test has a
racy `.*'. If you had written it like the other tests, like:
set cmd "interpreter-exec mi1 \"-break-insert main\""
gdb_test_multiple $cmd $cmd {
-re ".done.bkpt=.number=.\[0-9\].*\r\n$gdb_prompt " {
^^
pass "$cmd"
gdb_expect 1 {
-re "\r\n$gdb_prompt $" { }
}
}
}
then that .* could (and would often) eat one prompt, while
the $gdb_prompt afterward would consume the other. By the
time you got to the "gdb_expect 1", there was no input left
in the buffer, so you'd hang there, until that timed out.
Since there's no fail in that gdb_expect, the problem was
sort of silent.
Your change isn't correct, because gdb_test_multiple
has:
-re "\r\n$gdb_prompt $" {
if ![string match "" $message] then {
fail "$message"
}
set result 1
}
And that could match the first prompt if just the right amount of
characters end up in the buffer ($ anchors on the end of current
contents on the expect buffer), before the second prompt is output.
If you run your test with the race reproducer from PR12649, you'll see
the test failing consistently due to this issue (it forces expect to
add one new character at a time to the buffer).
The easiest fix is to keep using the old mechanism, getting rid of
all .*'s in the expected regex. See new version of the patch below.
> with this Pedro's patch runs through the testsuite no new failures...
I see the same.
> feel free to merge in with your patch or have me commit the tests
> seperately, whatever is easiest for you really...
It was easier to merge it in. I did a couple more cosmetic tweaks
in the code patch, and made the test use prepare_for_testing, along
with removing a few redundant ${testfile}.exp's in gdb.sum output.
Tested on x86_64-linux and applied.
Thanks again for identifying the problem and for the tests!
--
Pedro Alves
gdb/
2011-09-12 Pedro Alves <pedro@codesourcery.com>
Matt Rice <ratmice@gmail.com>
PR gdb/13175
* interps.c (struct interp) <interpreter_out>: Delete field.
(interp_new): Remove the data and uiout parameters and adjust.
(interp_set): Only set the current_uiout from the interpreter's
uiout after initializing the interpreter. Adjust call to
init_proc.
(interp_ui_out): Adjust to call procs->ui_out_proc.
(interp_data, interp_name): New.
* interps.h (interp_init_ftype): Add `self' parameter.
(interp_ui_out_ftype): New typedef.
(struct interp_procs) <ui_out_proc>: New method pointer.
(interp_new): Remove the data and uiout parameters.
(interp_data, interp_name): Declare.
* tui/tui-interp.c (tui_init): Adjust prototype.
(tui_ui_out): New.
(_initialize_tui_interp): Install tui_ui_out. Don't instanciate
tui_out here. Adjust call to interp_new.
* tui/tui-io.c (tui_initialize_io): Don't set current_uiout here.
* cli/cli-interp.c (cli_interpreter_init): Adjust prototype.
(cli_ui_out): New.
(_initialize_cli_interp): Install it. Adjust call to interp_new.
* mi/mi-common.h (struct mi_interp) <uiout>: New field.
* mi/mi-interp.c (mi_interpreter_init): Adjust prototype.
Initialize mi->uiout depending on the mi_version as extracted from
the interpreter's name.
(mi_ui_out): New.
(_initialize_mi_interp): Install mi_ui_out. Adjust calls to
interp_new. Don't allocate the ui_out's of the interpreters here.
gdb/testsuite/
2011-09-12 Matt Rice <ratmice@gmail.com>
Pedro Alves <pedro@codesourcery.com>
PR gdb/13175
* gdb.base/interp.exp: New tests.
* gdb.base/interp.c: New file.
---
gdb/cli/cli-interp.c | 12 ++++++--
gdb/interps.c | 38 +++++++++++++++++----------
gdb/interps.h | 16 ++++++++---
gdb/mi/mi-common.h | 3 ++
gdb/mi/mi-interp.c | 42 ++++++++++++++++++++++++------
gdb/testsuite/gdb.base/interp.c | 24 +++++++++++++++++
gdb/testsuite/gdb.base/interp.exp | 52 +++++++++++++++++++++++++++++++++++++-
gdb/tui/tui-interp.c | 17 ++++++++++--
gdb/tui/tui-io.c | 2 -
9 files changed, 171 insertions(+), 35 deletions(-)
Index: src/gdb/interps.c
===================================================================
--- src.orig/gdb/interps.c 2011-09-12 13:28:27.000000000 +0100
+++ src/gdb/interps.c 2011-09-12 20:29:07.216237941 +0100
@@ -67,11 +67,6 @@ struct interp
/* Has the init_proc been run? */
int inited;
- /* This is the ui_out used to collect results for this interpreter.
- It can be a formatter for stdout, as is the case for the console
- & mi outputs, or it might be a result formatter. */
- struct ui_out *interpreter_out;
-
const struct interp_procs *procs;
int quiet_p;
};
@@ -97,16 +92,14 @@ static int interpreter_initialized = 0;
fills the fields from the inputs, and returns a pointer to the
interpreter. */
struct interp *
-interp_new (const char *name, void *data, struct ui_out *uiout,
- const struct interp_procs *procs)
+interp_new (const char *name, const struct interp_procs *procs)
{
struct interp *new_interp;
new_interp = XMALLOC (struct interp);
new_interp->name = xstrdup (name);
- new_interp->data = data;
- new_interp->interpreter_out = uiout;
+ new_interp->data = NULL;
new_interp->quiet_p = 0;
new_interp->procs = procs;
new_interp->inited = 0;
@@ -184,19 +177,20 @@ interp_set (struct interp *interp, int t
interpreter_p = xstrdup (current_interpreter->name);
}
- current_uiout = interp->interpreter_out;
-
/* Run the init proc. If it fails, try to restore the old interp. */
if (!interp->inited)
{
if (interp->procs->init_proc != NULL)
{
- interp->data = interp->procs->init_proc (top_level);
+ interp->data = interp->procs->init_proc (interp, top_level);
}
interp->inited = 1;
}
+ /* Do this only after the interpreter is initialized. */
+ current_uiout = interp->procs->ui_out_proc (interp);
+
/* Clear out any installed interpreter hooks/event handlers. */
clear_interpreter_hooks ();
@@ -254,9 +248,25 @@ struct ui_out *
interp_ui_out (struct interp *interp)
{
if (interp != NULL)
- return interp->interpreter_out;
+ return interp->procs->ui_out_proc (interp);
+
+ return current_interpreter->procs->ui_out_proc (current_interpreter);
+}
+
+/* Returns the interpreter's cookie. */
- return current_interpreter->interpreter_out;
+void *
+interp_data (struct interp *interp)
+{
+ return interp->data;
+}
+
+/* Returns the interpreter's name. */
+
+const char *
+interp_name (struct interp *interp)
+{
+ return interp->name;
}
/* Returns true if the current interp is the passed in name. */
Index: src/gdb/interps.h
===================================================================
--- src.orig/gdb/interps.h 2011-09-12 13:28:26.000000000 +0100
+++ src/gdb/interps.h 2011-09-12 13:28:28.416229191 +0100
@@ -36,13 +36,14 @@ extern struct gdb_exception interp_exec
const char *command);
extern int interp_quiet_p (struct interp *interp);
-typedef void *(interp_init_ftype) (int top_level);
+typedef void *(interp_init_ftype) (struct interp *self, int top_level);
typedef int (interp_resume_ftype) (void *data);
typedef int (interp_suspend_ftype) (void *data);
typedef int (interp_prompt_p_ftype) (void *data);
typedef struct gdb_exception (interp_exec_ftype) (void *data,
const char *command);
typedef void (interp_command_loop_ftype) (void *data);
+typedef struct ui_out *(interp_ui_out_ftype) (struct interp *self);
struct interp_procs
{
@@ -51,16 +52,23 @@ struct interp_procs
interp_suspend_ftype *suspend_proc;
interp_exec_ftype *exec_proc;
interp_prompt_p_ftype *prompt_proc_p;
+
+ /* Returns the ui_out currently used to collect results for this
+ interpreter. It can be a formatter for stdout, as is the case
+ for the console & mi outputs, or it might be a result
+ formatter. */
+ interp_ui_out_ftype *ui_out_proc;
+
interp_command_loop_ftype *command_loop_proc;
};
-extern struct interp *interp_new (const char *name, void *data,
- struct ui_out *uiout,
- const struct interp_procs *procs);
+extern struct interp *interp_new (const char *name, const struct interp_procs *procs);
extern void interp_add (struct interp *interp);
extern int interp_set (struct interp *interp, int top_level);
extern struct interp *interp_lookup (const char *name);
extern struct ui_out *interp_ui_out (struct interp *interp);
+extern void *interp_data (struct interp *interp);
+extern const char *interp_name (struct interp *interp);
extern int current_interp_named_p (const char *name);
extern int current_interp_display_prompt_p (void);
Index: src/gdb/tui/tui-interp.c
===================================================================
--- src.orig/gdb/tui/tui-interp.c 2011-09-12 13:28:26.000000000 +0100
+++ src/gdb/tui/tui-interp.c 2011-09-12 13:28:28.416229191 +0100
@@ -52,7 +52,7 @@ static int tui_is_toplevel = 0;
/* These implement the TUI interpreter. */
static void *
-tui_init (int top_level)
+tui_init (struct interp *self, int top_level)
{
tui_is_toplevel = top_level;
@@ -126,6 +126,15 @@ tui_display_prompt_p (void *data)
return 1;
}
+static struct ui_out *
+tui_ui_out (struct interp *self)
+{
+ if (tui_active)
+ return tui_out;
+ else
+ return tui_old_uiout;
+}
+
static struct gdb_exception
tui_exec (void *data, const char *command_str)
{
@@ -144,11 +153,13 @@ _initialize_tui_interp (void)
tui_suspend,
tui_exec,
tui_display_prompt_p,
+ tui_ui_out,
};
+ struct interp *tui_interp;
/* Create a default uiout builder for the TUI. */
- tui_out = tui_out_new (gdb_stdout);
- interp_add (interp_new (INTERP_TUI, NULL, tui_out, &procs));
+ tui_interp = interp_new (INTERP_TUI, &procs);
+ interp_add (tui_interp);
if (interpreter_p && strcmp (interpreter_p, INTERP_TUI) == 0)
tui_start_enabled = 1;
Index: src/gdb/tui/tui-io.c
===================================================================
--- src.orig/gdb/tui/tui-io.c 2011-09-12 13:28:26.000000000 +0100
+++ src/gdb/tui/tui-io.c 2011-09-12 20:32:42.866238015 +0100
@@ -607,7 +607,7 @@ tui_initialize_io (void)
/* Create the default UI. It is not created because we installed a
deprecated_init_ui_hook. */
- tui_old_uiout = current_uiout = cli_out_new (gdb_stdout);
+ tui_old_uiout = cli_out_new (gdb_stdout);
#ifdef TUI_USE_PIPE_FOR_READLINE
/* Temporary solution for readline writing to stdout: redirect
Index: src/gdb/cli/cli-interp.c
===================================================================
--- src.orig/gdb/cli/cli-interp.c 2011-09-12 13:28:27.000000000 +0100
+++ src/gdb/cli/cli-interp.c 2011-09-12 13:28:28.416229191 +0100
@@ -40,7 +40,7 @@ static struct gdb_exception safe_execute
/* These implement the cli out interpreter: */
static void *
-cli_interpreter_init (int top_level)
+cli_interpreter_init (struct interp *self, int top_level)
{
return NULL;
}
@@ -135,6 +135,11 @@ safe_execute_command (struct ui_out *com
return e;
}
+static struct ui_out *
+cli_ui_out (struct interp *self)
+{
+ return cli_uiout;
+}
/* Standard gdb initialization hook. */
extern initialize_file_ftype _initialize_cli_interp; /* -Wmissing-prototypes */
@@ -147,13 +152,14 @@ _initialize_cli_interp (void)
cli_interpreter_resume, /* resume_proc */
cli_interpreter_suspend, /* suspend_proc */
cli_interpreter_exec, /* exec_proc */
- cli_interpreter_display_prompt_p /* prompt_proc_p */
+ cli_interpreter_display_prompt_p, /* prompt_proc_p */
+ cli_ui_out /* ui_out_proc */
};
struct interp *cli_interp;
/* Create a default uiout builder for the CLI. */
cli_uiout = cli_out_new (gdb_stdout);
- cli_interp = interp_new (INTERP_CONSOLE, NULL, cli_uiout, &procs);
+ cli_interp = interp_new (INTERP_CONSOLE, &procs);
interp_add (cli_interp);
}
Index: src/gdb/mi/mi-common.h
===================================================================
--- src.orig/gdb/mi/mi-common.h 2011-09-12 13:28:26.000000000 +0100
+++ src/gdb/mi/mi-common.h 2011-09-12 20:32:06.506238003 +0100
@@ -52,6 +52,9 @@ struct mi_interp
struct ui_file *targ;
struct ui_file *event_channel;
+ /* MI's builder. */
+ struct ui_out *uiout;
+
/* This is the interpreter for the mi... */
struct interp *mi2_interp;
struct interp *mi1_interp;
Index: src/gdb/mi/mi-interp.c
===================================================================
--- src.orig/gdb/mi/mi-interp.c 2011-09-12 13:28:27.000000000 +0100
+++ src/gdb/mi/mi-interp.c 2011-09-12 13:40:23.000000000 +0100
@@ -72,9 +72,11 @@ static void mi_breakpoint_modified (stru
static int report_initial_inferior (struct inferior *inf, void *closure);
static void *
-mi_interpreter_init (int top_level)
+mi_interpreter_init (struct interp *interp, int top_level)
{
struct mi_interp *mi = XMALLOC (struct mi_interp);
+ const char *name;
+ int mi_version;
/* HACK: We need to force stdout/stderr to point at the console. This avoids
any potential side effects caused by legacy code that is still
@@ -90,6 +92,22 @@ mi_interpreter_init (int top_level)
mi->targ = mi_console_file_new (raw_stdout, "@", '"');
mi->event_channel = mi_console_file_new (raw_stdout, "=", 0);
+ name = interp_name (interp);
+ /* INTERP_MI selects the most recent released version. "mi2" was
+ released as part of GDB 6.0. */
+ if (strcmp (name, INTERP_MI) == 0)
+ mi_version = 2;
+ else if (strcmp (name, INTERP_MI1) == 0)
+ mi_version = 1;
+ else if (strcmp (name, INTERP_MI2) == 0)
+ mi_version = 2;
+ else if (strcmp (name, INTERP_MI3) == 0)
+ mi_version = 3;
+ else
+ gdb_assert_not_reached ("unhandled MI version");
+
+ mi->uiout = mi_out_new (mi_version);
+
if (top_level)
{
observer_attach_new_thread (mi_new_thread);
@@ -701,6 +719,14 @@ report_initial_inferior (struct inferior
return 0;
}
+static struct ui_out *
+mi_ui_out (struct interp *interp)
+{
+ struct mi_interp *mi = interp_data (interp);
+
+ return mi->uiout;
+}
+
extern initialize_file_ftype _initialize_mi_interp; /* -Wmissing-prototypes */
void
@@ -712,15 +738,13 @@ _initialize_mi_interp (void)
mi_interpreter_resume, /* resume_proc */
mi_interpreter_suspend, /* suspend_proc */
mi_interpreter_exec, /* exec_proc */
- mi_interpreter_prompt_p /* prompt_proc_p */
+ mi_interpreter_prompt_p, /* prompt_proc_p */
+ mi_ui_out /* ui_out_proc */
};
/* The various interpreter levels. */
- interp_add (interp_new (INTERP_MI1, NULL, mi_out_new (1), &procs));
- interp_add (interp_new (INTERP_MI2, NULL, mi_out_new (2), &procs));
- interp_add (interp_new (INTERP_MI3, NULL, mi_out_new (3), &procs));
-
- /* "mi" selects the most recent released version. "mi2" was
- released as part of GDB 6.0. */
- interp_add (interp_new (INTERP_MI, NULL, mi_out_new (2), &procs));
+ interp_add (interp_new (INTERP_MI1, &procs));
+ interp_add (interp_new (INTERP_MI2, &procs));
+ interp_add (interp_new (INTERP_MI3, &procs));
+ interp_add (interp_new (INTERP_MI, &procs));
}
Index: src/gdb/testsuite/gdb.base/interp.c
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.base/interp.c 2011-09-12 20:34:18.000000000 +0100
@@ -0,0 +1,24 @@
+/* This test program is part of GDB, the GNU debugger.
+
+ Copyright 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/>.
+*/
+
+int
+main (int argc, const char **argv)
+{
+ return 0;
+}
Index: src/gdb/testsuite/gdb.base/interp.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.base/interp.exp 2011-09-12 20:34:00.000000000 +0100
+++ src/gdb/testsuite/gdb.base/interp.exp 2011-09-12 20:34:18.000000000 +0100
@@ -20,7 +20,11 @@ if $tracelevel then {
strace $tracelevel
}
-gdb_start
+set testfile "interp"
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${testfile}.c {debug}] } {
+ return -1
+}
# Do not use gdb_test for this test, since it has two prompts.
set cmd "interpreter-exec mi \"-var-update *\""
@@ -45,4 +49,50 @@ gdb_test_multiple "interpreter-exec mi \
}
}
+set cmd "interpreter-exec mi \"-stack-info-frame\""
+gdb_test_multiple $cmd $cmd {
+ -re ".error,msg=.No registers\..\r\n$gdb_prompt " {
+ pass "$cmd"
+ gdb_expect 1 {
+ -re "\r\n$gdb_prompt $" { }
+ }
+ }
+}
+
+set cmd "interpreter-exec mi1 \"-break-insert main\""
+gdb_test_multiple $cmd $cmd {
+ -re ".done.bkpt=.number=.\[0-9\]\[^\n\]+\r\n$gdb_prompt " {
+ pass "$cmd"
+ gdb_expect 1 {
+ -re "\r\n$gdb_prompt $" { }
+ }
+ }
+}
+
+set cmd "interpreter-exec mi2 \"-break-insert main\""
+gdb_test_multiple $cmd $cmd {
+ -re ".done.bkpt=.number=.\[0-9\]\[^\n\]+\r\n$gdb_prompt " {
+ pass "$cmd"
+ gdb_expect 1 {
+ -re "\r\n$gdb_prompt $" { }
+ }
+ }
+}
+
+set cmd "interpreter-exec mi3 \"-break-insert main\""
+gdb_test_multiple $cmd $cmd {
+ -re ".done.bkpt=.number=.\[0-9\]\[^\n\]+\r\n$gdb_prompt " {
+ pass "$cmd"
+ gdb_expect 1 {
+ -re "\r\n$gdb_prompt $" { }
+ }
+ }
+}
+
+if ![runto_main] then {
+ fail "run to main"
+ return -1;
+}
+
+gdb_test "list" ".*\[0-9\].*main \\(int argc.*" "can list sources"
gdb_exit