[PATCH v2 6/9] PR gdb/13860: don't lose '-interpreter-exec console EXECUTION_COMMAND''s output in async mode.

Tom Tromey
2013-07-31T18:28:20Z

From: Pedro Alves <palves@redhat.com>

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 made a temporary uiout when
necessary.

- 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.

* 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 | 13 +++++++-
gdb/gdbthread.h | 5 +++
gdb/infrun.c | 14 +++++++++
gdb/interps.c | 36 ++++++++++++++++++++-
gdb/interps.h | 2 ++
gdb/mi/mi-interp.c | 66 +++++++++++++++++++++++++++++++++++++++
gdb/testsuite/gdb.mi/mi-cli.exp | 13 +++++---
gdb/testsuite/gdb.mi/mi-solib.exp | 11 +++++++
8 files changed, 154 insertions(+), 6 deletions(-)

diff --git a/gdb/cli-out.c b/gdb/cli-out.c
index 380352b..cedd3af 100644
--- a/gdb/cli-out.c
+++ b/gdb/cli-out.c
@@ -40,6 +40,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 */
@@ -370,7 +381,7 @@ struct ui_out_impl cli_ui_out_impl =
cli_wrap_hint,
cli_flush,
cli_redirect,
- 0,
+ cli_uiout_dtor,
0, /* Does not need MI hacks (i.e. needs CLI hacks). */
};

diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index c3b85dc..f3e8db1 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -122,6 +122,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 53ecc1d..0052ef2 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -428,6 +428,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)
{
@@ -479,6 +480,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,
@@ -490,6 +492,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;
@@ -534,6 +537,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
{
@@ -1993,6 +1997,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);
}
@@ -2194,6 +2200,14 @@ proceed (CORE_ADDR addr, enum gdb_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 25500d6..ca3b9b5 100644
--- a/gdb/interps.c
+++ b/gdb/interps.c
@@ -315,6 +315,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)
@@ -362,7 +385,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 58ac6b2..6a45a7b 100644
--- a/gdb/interps.h
+++ b/gdb/interps.h
@@ -97,6 +97,8 @@ extern int current_interp_set_logging (int start_log, struct ui_file *out,
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 0cd43f5..b4515d9 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -37,6 +37,7 @@
#include "gdb.h"
#include "objfiles.h"
#include "tracepoint.h"
+#include "cli-out.h"

/* These are the interpreter setup, etc. functions for the MI
interpreter. */
@@ -251,6 +252,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
@@ -453,6 +458,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_destroy (uiout);
+}
+
static void
mi_on_normal_stop (struct bpstats *bs, int print_frame)
{
@@ -483,6 +508,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/testsuite/gdb.mi/mi-cli.exp b/gdb/testsuite/gdb.mi/mi-cli.exp
index 5b809e2..bee296d 100644
--- a/gdb/testsuite/gdb.mi/mi-cli.exp
+++ b/gdb/testsuite/gdb.mi/mi-cli.exp
@@ -166,10 +166,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 cd400ea..0a8b43b 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"
--
1.8.1.4