This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[PATCH 3/3] PR gdb/13860: don't lose '-interpreter-exec console EXECUTION_COMMAND''s output in async mode.
- From: Pedro Alves <palves at redhat dot com>
- To: gdb-patches at sourceware dot org
- Date: Wed, 09 May 2012 17:55:17 +0100
- Subject: [PATCH 3/3] PR gdb/13860: don't lose '-interpreter-exec console EXECUTION_COMMAND''s output in async mode.
- References: <20120509165457.25915.32885.stgit@brno.lan>
The other part of PR gdb/13860 is about console execution commands
in MI getting their output half lost. E.g., take the finish command,
executed on a frontend's GDB console:
sync:
finish
&"finish\n"
~"Run till exit from #0 usleep (useconds=10) at ../sysdeps/unix/sysv/linux/usleep.c:27\n"
^running
*running,thread-id="1"
(gdb)
~"0x00000000004004d7 in foo () at stepinf.c:6\n"
~"6\t usleep (10);\n"
~"Value returned is $1 = 0\n"
*stopped,reason="function-finished",frame={addr="0x00000000004004d7",func="foo",args=[],file="stepinf.c",fullname="/home/pedro/gdb/tests/stepinf.c",line="6"},thread-id="1",stopped-threads="all",core="1"
async:
finish
&"finish\n"
~"Run till exit from #0 usleep (useconds=10) at ../sysdeps/unix/sysv/linux/usleep.c:27\n"
^running
*running,thread-id="1"
(gdb)
*stopped,reason="function-finished",frame={addr="0x00000000004004d7",func="foo",args=[],file="stepinf.c",fullname="/home/pedro/gdb/tests/stepinf.c",line="6"},gdb-result-var="$1",return-value="0",thread-id="1",stopped-threads="all",core="0"
Note how all the "Value returned" etc. output is missing in async mode.
The same happens with e.g., catchpoints:
=breakpoint-modified,bkpt={number="1",type="catchpoint",disp="keep",enabled="y",what="22016",times="1"}
~"\nCatchpoint "
~"1 (forked process 22016), 0x0000003791cbd8a6 in __libc_fork () at ../nptl/sysdeps/unix/sysv/linux/fork.c:131\n"
~"131\t pid = ARCH_FORK ();\n"
*stopped,reason="fork",disp="keep",bkptno="1",newpid="22016",frame={addr="0x0000003791cbd8a6",func="__libc_fork",args=[],file="../nptl/sysdeps/unix/sysv/linux/fork.c",fullname="/usr/src/debug/glibc-2.14-394-g8f3b1ff/nptl/sysdeps/unix/sysv/linux/fork.c",line="131"},thread-id="1",stopped-threads="all",core="0"
where all those ~ lines are missing in async mode, or just the "step"
current line indication:
s
&"s\n"
^running
*running,thread-id="all"
(gdb)
~"13\t foo ();\n"
*stopped,frame={addr="0x00000000004004ef",func="main",args=[{name="argc",value="1"},{name="argv",value="0x7fffffffdd78"}],file="stepinf.c",fullname="/home/pedro/gdb/tests/stepinf.c",line="13"},thread-id="1",stopped-threads="all",core="3"
(gdb)
Or in the case of the PRs example, the "Stopped due to shared library
event" note:
start
&"start\n"
~"Temporary breakpoint 1 at 0x400608: file ../../../src/gdb/testsuite/gdb.mi/solib-main.c, line 21.\n"
=breakpoint-created,bkpt={number="1",type="breakpoint",disp="del",enabled="y",addr="0x0000000000400608",func="main",file="../../../src/gdb/testsuite/gdb.mi/solib-main.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/solib-main.c",line="21",times="0",original-location="main"}
~"Starting program: /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/solib-main \n"
=thread-group-started,id="i1",pid="21990"
=thread-created,id="1",group-id="i1"
^running
*running,thread-id="all"
(gdb)
=library-loaded,id="/lib64/ld-linux-x86-64.so.2",target-name="/lib64/ld-linux-x86-64.so.2",host-name="/lib64/ld-linux-x86-64.so.2",symbols-loaded="0",thread-group="i1"
~"Stopped due to shared library event (no libraries added or removed)\n"
*stopped,reason="solib-event",thread-id="1",stopped-threads="all",core="3"
(gdb)
IMO, if you're typing execution commands in a frontend's console, you
expect to see their output. Indeed it's what you get in sync mode. I
think async mode should do the same.
That's what this patch does.
Notes:
- mi->out is the same as gdb_stdout when MI is the current
interpreter. I think that referring to that directly is cleaner.
An earlier revision of this patch made the changes that are now
done in mi_on_normal_stop directly in infrun.c:normal_stop, and so
not having an obvious place to put the new uiout by then, and not
wanting to abuse CLI's uiout, I thought of making a temporary
uiout when necessary. This meant that I needed a way to destroy
uiout objects. Nothing else currently destroy's them, as all
uiout objects are long lived along side the interpreters. Now
that the code in question that creates the temporary uiout is all
within mi-interp.c, I could instead make the uiout that wraps
mi->out be long lived too, and thus not need ui-out.h|c changes.
Let me know if you have a preference.
- Hopefuly the rest of the patch is more or less obvious given the
comments I added.
Tested on x86_64 Fedora 16, no regressions.
2012-05-09 Pedro Alves <palves@redhat.com>
PR gdb/13860
* gdbthread.h (struct thread_control_state): New field
`command_interp'.
* infrun.c (follow_fork): Copy the new thread control field to the
child fork thread.
(clear_proceed_status_thread): Clear the new thread control field.
(proceed): Set the new thread control field.
* interps.h (command_interp): Declare.
* interps.c (command_interpreter): New global.
(command_interp): New function.
(interp_exec): Set `command_interpreter' while here.
* ui-out.h (ui_out_dtor_ftype): New typedef.
(struct ui_out_impl): New field `dtor'.
(ui_out_free): Declare.
* ui-out.c (default_ui_out_impl): Install NULL `dtor' handler.
(ui_out_free): New function.
* cli-out.c (cli_uiout_dtor): New function.
(cli_ui_out_impl): Install it.
* mi/mi-interp.c: Include cli-out.h.
(mi_cmd_interpreter_exec): Add comment.
(restore_current_uiout_cleanup): New function.
(ui_out_free_cleanup): New function.
(mi_on_normal_stop): In async mode, if finishing an execution
command started by a CLI command, or any kind of breakpoint-like
event triggered, print the stop event to the output (CLI) stream.
* mi/mi-out.c (mi_ui_out_impl): Install NULL `dtor' handler.
gdb/testsuite/
* gdb.mi/mi-cli.exp: Also expect the new source line to be output
after a "next", in async mode. Make it a pass/fail test.
* gdb.mi/mi-solib.exp: Test that the CLI solib event note is
output.
---
gdb/cli-out.c | 12 +++++++
gdb/gdbthread.h | 5 +++
gdb/infrun.c | 14 ++++++++
gdb/interps.c | 36 ++++++++++++++++++++
gdb/interps.h | 2 +
gdb/mi/mi-interp.c | 66 +++++++++++++++++++++++++++++++++++++
gdb/mi/mi-out.c | 1 +
gdb/testsuite/gdb.mi/mi-cli.exp | 13 +++++--
gdb/testsuite/gdb.mi/mi-solib.exp | 11 ++++++
gdb/ui-out.c | 11 ++++++
gdb/ui-out.h | 6 +++
11 files changed, 172 insertions(+), 5 deletions(-)
diff --git a/gdb/cli-out.c b/gdb/cli-out.c
index 925206c..d6fad6d 100644
--- a/gdb/cli-out.c
+++ b/gdb/cli-out.c
@@ -41,6 +41,17 @@ static void out_field_fmt (struct ui_out *uiout, int fldno,
const char *fldname,
const char *format,...) ATTRIBUTE_PRINTF (4, 5);
+/* The destructor. */
+
+static void
+cli_uiout_dtor (struct ui_out *ui_out)
+{
+ cli_out_data *data = ui_out_data (ui_out);
+
+ VEC_free (ui_filep, data->streams);
+ xfree (data);
+}
+
/* These are the CLI output functions */
/* Mark beginning of a table */
@@ -355,6 +366,7 @@ field_separator (void)
struct ui_out_impl cli_ui_out_impl =
{
+ cli_uiout_dtor,
cli_table_begin,
cli_table_body,
cli_table_end,
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index fb8de16..a61566f 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -114,6 +114,11 @@ struct thread_control_state
/* Chain containing status of breakpoint(s) the thread stopped
at. */
bpstat stop_bpstat;
+
+ /* The interpreter that issued the execution command. NULL if the
+ thread was resumed as a result of a command applied to some other
+ thread (e.g., "next" with scheduler-locking off). */
+ struct interp *command_interp;
};
/* Inferior thread specific part of `struct infcall_suspend_state'.
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 607ef44..45ff472 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -438,6 +438,7 @@ follow_fork (void)
CORE_ADDR step_range_start = 0;
CORE_ADDR step_range_end = 0;
struct frame_id step_frame_id = { 0 };
+ struct interp *command_interp = NULL;
if (!non_stop)
{
@@ -489,6 +490,7 @@ follow_fork (void)
step_frame_id = tp->control.step_frame_id;
exception_resume_breakpoint
= clone_momentary_breakpoint (tp->control.exception_resume_breakpoint);
+ command_interp = tp->control.command_interp;
/* For now, delete the parent's sr breakpoint, otherwise,
parent/child sr breakpoints are considered duplicates,
@@ -500,6 +502,7 @@ follow_fork (void)
tp->control.step_range_end = 0;
tp->control.step_frame_id = null_frame_id;
delete_exception_resume_breakpoint (tp);
+ tp->control.command_interp = NULL;
}
parent = inferior_ptid;
@@ -544,6 +547,7 @@ follow_fork (void)
tp->control.step_frame_id = step_frame_id;
tp->control.exception_resume_breakpoint
= exception_resume_breakpoint;
+ tp->control.command_interp = command_interp;
}
else
{
@@ -1970,6 +1974,8 @@ clear_proceed_status_thread (struct thread_info *tp)
tp->control.proceed_to_finish = 0;
+ tp->control.command_interp = NULL;
+
/* Discard any remaining commands or status from previous stop. */
bpstat_clear (&tp->control.stop_bpstat);
}
@@ -2170,6 +2176,14 @@ proceed (CORE_ADDR addr, enum target_signal siggnal, int step)
regcache_write_pc (regcache, addr);
}
+ /* Record the interpreter that issued the execution command that
+ caused this thread to resume. If the top level interpreter is
+ MI/async, and the execution command was a CLI command
+ (next/step/etc.), we'll want to print stop event output to the MI
+ console channel (the stepped-to line, etc.), as if the user
+ entered the execution command on a real GDB console. */
+ inferior_thread ()->control.command_interp = command_interp ();
+
if (debug_infrun)
fprintf_unfiltered (gdb_stdlog,
"infrun: proceed (addr=%s, signal=%d, step=%d)\n",
diff --git a/gdb/interps.c b/gdb/interps.c
index 23e5a10..fe9f3f4 100644
--- a/gdb/interps.c
+++ b/gdb/interps.c
@@ -305,6 +305,29 @@ current_interp_display_prompt_p (void)
data);
}
+/* The interpreter that is active while `interp_exec' is active, NULL
+ at all other times. */
+static struct interp *command_interpreter;
+
+/* The interpreter that was active when a command was executed.
+ Normally that'd always be CURRENT_INTERPRETER, except that MI's
+ -interpreter-exec command doesn't actually flip the current
+ interpreter when running its sub-command. The
+ `command_interpreter' global tracks when interp_exec is called
+ (IOW, when -interpreter-exec is called). If that is set, it is
+ INTERP in '-interpreter-exec INTERP "CMD"' or in 'interpreter-exec
+ INTERP "CMD". Otherwise, interp_exec isn't active, and so the
+ interpreter running the command is the current interpreter. */
+
+struct interp *
+command_interp (void)
+{
+ if (command_interpreter != NULL)
+ return command_interpreter;
+ else
+ return current_interpreter;
+}
+
/* Run the current command interpreter's main loop. */
void
current_interp_command_loop (void)
@@ -352,7 +375,18 @@ interp_exec (struct interp *interp, const char *command_str)
{
if (interp->procs->exec_proc != NULL)
{
- return interp->procs->exec_proc (interp->data, command_str);
+ struct gdb_exception ex;
+ struct interp *save_command_interp;
+
+ /* See `command_interp' for why we do this. */
+ save_command_interp = command_interpreter;
+ command_interpreter = interp;
+
+ ex = interp->procs->exec_proc (interp->data, command_str);
+
+ command_interpreter = save_command_interp;
+
+ return ex;
}
return exception_none;
}
diff --git a/gdb/interps.h b/gdb/interps.h
index bbf0838..76c99bc 100644
--- a/gdb/interps.h
+++ b/gdb/interps.h
@@ -78,6 +78,8 @@ extern void current_interp_command_loop (void);
extern void *top_level_interpreter_data (void);
extern struct interp *top_level_interpreter (void);
+extern struct interp *command_interp (void);
+
/* True if the current interpreter is in async mode, false if in sync
mode. If in sync mode, running a synchronous execution command
(with execute_command, e.g, "next") will not return until the
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index a451cc3..cfcc58f 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -35,6 +35,7 @@
#include "gdbthread.h"
#include "solist.h"
#include "gdb.h"
+#include "cli-out.h"
/* These are the interpreter setup, etc. functions for the MI
interpreter. */
@@ -238,6 +239,10 @@ mi_cmd_interpreter_exec (char *command, char **argv, int argc)
"does not support command execution"),
argv[0]);
+ /* Note that unlike the CLI version of this command, we don't
+ actually set INTERP_TO_USE as the current interpreter, as we
+ still want gdb_stdout, etc. to point at MI streams. */
+
/* Insert the MI out hooks, making sure to also call the
interpreter's hooks if it has any. */
/* KRS: We shouldn't need this... Events should be installed and
@@ -427,6 +432,26 @@ mi_inferior_removed (struct inferior *inf)
gdb_flush (mi->event_channel);
}
+/* Cleanup that restores a previous current uiout. */
+
+static void
+restore_current_uiout_cleanup (void *arg)
+{
+ struct ui_out *saved_uiout = arg;
+
+ current_uiout = saved_uiout;
+}
+
+/* Cleanup that destroys the a ui_out object. */
+
+static void
+ui_out_free_cleanup (void *arg)
+{
+ struct ui_out *uiout = arg;
+
+ ui_out_free (uiout);
+}
+
static void
mi_on_normal_stop (struct bpstats *bs, int print_frame)
{
@@ -457,6 +482,47 @@ mi_on_normal_stop (struct bpstats *bs, int print_frame)
current_uiout = saved_uiout;
}
+ /* Otherwise, frame information has already been printed by
+ normal_stop. */
+ else if (target_can_async_p ())
+ {
+ /* However, CLI execution commands (-interpreter-exec
+ console "next", for example) in async mode have the
+ opposite issue. normal_stop has already printed frame
+ information to MI uiout, but nothing has printed the same
+ information to the CLI channel. We should print the
+ source line to the console when stepping or other similar
+ commands, iff the step was started by a console command
+ (but not if it was started with -exec-step or similar).
+ Breakpoint hits should always be mirrored to the
+ console. */
+ struct thread_info *tp = inferior_thread ();
+
+ if ((tp->control.command_interp != NULL
+ && tp->control.command_interp != top_level_interpreter ())
+ || (!tp->control.stop_step
+ && !tp->control.proceed_to_finish))
+ {
+ struct mi_interp *mi = top_level_interpreter_data ();
+ struct target_waitstatus last;
+ ptid_t last_ptid;
+ struct ui_out *cli_uiout;
+ struct cleanup *old_chain;
+
+ /* Sets the current uiout to a new temporary CLI uiout
+ assigned to STREAM. */
+ cli_uiout = cli_out_new (mi->out);
+ old_chain = make_cleanup (ui_out_free_cleanup, cli_uiout);
+
+ make_cleanup (restore_current_uiout_cleanup, current_uiout);
+ current_uiout = cli_uiout;
+
+ get_last_target_status (&last_ptid, &last);
+ print_stop_event (&last);
+
+ do_cleanups (old_chain);
+ }
+ }
ui_out_field_int (mi_uiout, "thread-id",
pid_to_thread_id (inferior_ptid));
diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c
index b39e05a..b19503c 100644
--- a/gdb/mi/mi-out.c
+++ b/gdb/mi/mi-out.c
@@ -73,6 +73,7 @@ static int mi_redirect (struct ui_out *uiout, struct ui_file *outstream);
struct ui_out_impl mi_ui_out_impl =
{
+ NULL,
mi_table_begin,
mi_table_body,
mi_table_end,
diff --git a/gdb/testsuite/gdb.mi/mi-cli.exp b/gdb/testsuite/gdb.mi/mi-cli.exp
index db022f8..5e3e31c 100644
--- a/gdb/testsuite/gdb.mi/mi-cli.exp
+++ b/gdb/testsuite/gdb.mi/mi-cli.exp
@@ -178,10 +178,15 @@ mi_gdb_test "34 next" \
".*34\\\^running.*\\*running,thread-id=\"all\"" \
"34 next: run"
-if {!$async} {
- gdb_expect {
- -re "~\[^\r\n\]+\r\n" {
- }
+# Test that the new current source line is output, given we executed
+# the console 'next' command, not -exec-next.
+set test "34 next: CLI output"
+gdb_expect {
+ -re "~\"67\[^\r\n\]+\r\n" {
+ pass $test
+ }
+ timeout {
+ fail "$test (timeout)"
}
}
diff --git a/gdb/testsuite/gdb.mi/mi-solib.exp b/gdb/testsuite/gdb.mi/mi-solib.exp
index 1e79125..2db1d15 100644
--- a/gdb/testsuite/gdb.mi/mi-solib.exp
+++ b/gdb/testsuite/gdb.mi/mi-solib.exp
@@ -60,4 +60,15 @@ mi_gdb_test "777-gdb-set stop-on-solib-events 1" "777\\^done" \
# commands still cause the correct MI output to be generated.
mi_run_with_cli
+# Also test that the CLI solib event note is output.
+set test "CLI prints solib event"
+gdb_expect {
+ -re "~\"Stopped due to shared library event \\(no libraries added or removed\\)\\\\n" {
+ pass "$test"
+ }
+ timeout {
+ fail "$test (timeout)"
+ }
+}
+
mi_expect_stop solib-event .* .* .* .* .* "check for solib event"
diff --git a/gdb/ui-out.c b/gdb/ui-out.c
index aa53f3f..9a68bb7 100644
--- a/gdb/ui-out.c
+++ b/gdb/ui-out.c
@@ -191,6 +191,7 @@ static void default_flush (struct ui_out *uiout);
struct ui_out_impl default_ui_out_impl =
{
+ NULL, /* dtor */
default_table_begin,
default_table_body,
default_table_end,
@@ -1135,6 +1136,16 @@ ui_out_new (struct ui_out_impl *impl, void *data,
return uiout;
}
+/* Free UIOUT. */
+
+void
+ui_out_free (struct ui_out *uiout)
+{
+ if (uiout->impl->dtor != NULL)
+ uiout->impl->dtor (uiout);
+ xfree (uiout);
+}
+
/* Standard gdb initialization hook. */
void
diff --git a/gdb/ui-out.h b/gdb/ui-out.h
index 4b9725f..5996ab2 100644
--- a/gdb/ui-out.h
+++ b/gdb/ui-out.h
@@ -181,6 +181,7 @@ extern int ui_out_is_mi_like_p (struct ui_out *uiout);
/* Type definition of all implementation functions. */
+typedef void (ui_out_dtor_ftype) (struct ui_out * uiout);
typedef void (table_begin_ftype) (struct ui_out * uiout,
int nbrofcols, int nr_rows,
const char *tblid);
@@ -230,6 +231,7 @@ typedef int (redirect_ftype) (struct ui_out * uiout,
struct ui_out_impl
{
+ ui_out_dtor_ftype *dtor;
table_begin_ftype *table_begin;
table_body_ftype *table_body;
table_end_ftype *table_end;
@@ -261,6 +263,10 @@ extern struct ui_out *ui_out_new (struct ui_out_impl *impl,
void *data,
int flags);
+/* Free UIOUT. */
+
+extern void ui_out_free (struct ui_out *uiout);
+
/* Redirect the ouptut of a ui_out object temporarily. */
extern int ui_out_redirect (struct ui_out *uiout, struct ui_file *outstream);