| Message ID | 532897FA.6000809@redhat.com |
|---|---|
| State | Committed |
| Headers | show |
On 03/18/2014 07:01 PM, Pedro Alves wrote: > On 03/04/2014 06:32 PM, Tom Tromey wrote: >> 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: >> > I thought about this mirroring of breakpoints to the console > only done in async, and given we're trying to make the CLI vs MI > output match, it seemed a little silly to not do the same in > sync mode. So I removed the target_can_async_p() check above, > and extended the comments to explain why we do this. I then > extended the tests to make sure the CLI output appears at > the right times, and that it doesn't appear when it shouldn't. > See below. Let me know what you think. Tom told me off list he's OK with this version. I've pushed it in. Getting closer... Pedro Alves > >> + 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); >> + } >> + } > > ------------------ > [PATCH] PR gdb/13860: don't lose '-interpreter-exec console > EXECUTION_COMMAND''s output in async mode. > > 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. Deciding what to mirror to the > console wrt to breakpoints and random stops gets messy real fast. > E.g., say "s" trips on a breakpoint. We'd clearly want to mirror the > event to the console in this case. But what about more complicated > cases like "s&; thread n; s&", and one of those steps spawning a new > thread, and that thread hitting a breakpoint? It's impossible in > general to track whether the thread had any relation to the commands > that had been executed. So I think we should just simplify and always > mirror breakpoints and random events to the console. > > 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 added. > > Tested on x86_64 Fedora 17, no regressions. > > 2014-03-18 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): 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. > > 2014-03-18 Pedro Alves <palves@redhat.com> > > PR gdb/13860 > * gdb.mi/mi-cli.exp (line_callee4_next_step): New global. > (top level): Test that output related to execution commands is > sent to the console with CLI commands, but not with MI commands. > Test that breakpoint events are always mirrored to the console. > Also expect the new source line to be output after a "next" in > async mode too. Make it a pass/fail test. > * gdb.mi/mi-solib.exp: Test that the CLI solib event note is > output. > * lib/mi-support.exp (mi_gdb_expect_cli_output): New procedure. > --- > 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 | 77 +++++++++++++++++++++++++++++++++++++++ > gdb/testsuite/gdb.mi/mi-cli.exp | 52 ++++++++++++++++++++++---- > gdb/testsuite/gdb.mi/mi-solib.exp | 11 ++++++ > gdb/testsuite/lib/mi-support.exp | 24 ++++++++++++ > 9 files changed, 225 insertions(+), 9 deletions(-) > > diff --git a/gdb/cli-out.c b/gdb/cli-out.c > index 5943fa7..eedbd2c 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 */ > @@ -367,7 +378,7 @@ const 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 9b43a68..9f5dee6 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 f783260..fed4a27 100644 > --- a/gdb/infrun.c > +++ b/gdb/infrun.c > @@ -433,6 +433,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) > { > @@ -484,6 +485,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, > @@ -495,6 +497,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; > @@ -539,6 +542,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 > { > @@ -1999,6 +2003,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); > } > @@ -2200,6 +2206,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=%s, step=%d)\n", > diff --git a/gdb/interps.c b/gdb/interps.c > index e446747..f6b941c 100644 > --- a/gdb/interps.c > +++ b/gdb/interps.c > @@ -318,6 +318,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) > @@ -351,9 +374,20 @@ interp_set_quiet (struct interp *interp, int quiet) > struct gdb_exception > interp_exec (struct interp *interp, const char *command_str) > { > + struct gdb_exception ex; > + struct interp *save_command_interp; > + > gdb_assert (interp->procs->exec_proc != NULL); > > - return interp->procs->exec_proc (interp->data, command_str); > + /* 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; > } > > /* A convenience routine that nulls out all the common command hooks. > diff --git a/gdb/interps.h b/gdb/interps.h > index 568b5df..13edf42 100644 > --- a/gdb/interps.h > +++ b/gdb/interps.h > @@ -96,6 +96,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 862beaf..80fffa3 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. */ > @@ -231,6 +232,10 @@ mi_cmd_interpreter_exec (char *command, char **argv, int argc) > error (_("-interpreter-exec: could not find interpreter \"%s\""), > 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 > @@ -415,6 +420,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) > { > @@ -445,6 +470,58 @@ 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 > + { > + /* Breakpoint hits should always be mirrored to the console. > + Deciding what to mirror to the console wrt to breakpoints > + and random stops gets messy real fast. E.g., say "s" > + trips on a breakpoint. We'd clearly want to mirror the > + event to the console in this case. But what about more > + complicated cases like "s&; thread n; s&", and one of > + those steps spawning a new thread, and that thread > + hitting a breakpoint? It's impossible in general to > + track whether the thread had any relation to the commands > + that had been executed. So we just simplify and always > + mirror breakpoints and random events to the console. > + > + Also, CLI execution commands (-interpreter-exec console > + "next", for example) in async mode have the opposite > + issue as described in the "then" branch above -- > + 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). */ > + struct thread_info *tp = inferior_thread (); > + > + if ((!tp->control.stop_step > + && !tp->control.proceed_to_finish) > + || (tp->control.command_interp != NULL > + && tp->control.command_interp != top_level_interpreter ())) > + { > + 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 cb8d3af..50313ae 100644 > --- a/gdb/testsuite/gdb.mi/mi-cli.exp > +++ b/gdb/testsuite/gdb.mi/mi-cli.exp > @@ -69,6 +69,7 @@ set line_main_callme_2 [expr $line_main_return + 1] > set line_callee4_head [gdb_get_line_number "callee4 ("] > set line_callee4_body [expr $line_callee4_head + 2] > set line_callee4_next [expr $line_callee4_body + 1] > +set line_callee4_next_step [expr $line_callee4_next + 3] > > mi_gdb_test "-interpreter-exec console \"set args foobar\"" \ > ".*=cmd-param-changed,param=\"args\",value=\"foobar\".*\\^done" \ > @@ -153,13 +154,44 @@ if {$async} { > mi_execute_to "interpreter-exec console step" $reason "callee4" "" ".*basics.c" $line_callee4_next \ > "" "check *stopped from CLI command" > > +mi_send_resuming_command "exec-step" "-exec-step to line \$line_callee4_next_step" > + > +# Test that the new current source line is _not_ output, given we > +# executed MI's -exec-next, not CLI's 'next' command. > + > +set output [mi_gdb_expect_cli_output "\\*stopped" "collect CLI output for -exec-step"] > + > +set test "-exec-step does not produce CLI step output" > +if {[regexp "A + B" "$output"]} { > + fail $test > +} else { > + pass $test > +} > + > +mi_expect_stop "end-stepping-range" "callee4" "" ".*basics.c" $line_callee4_next_step \ > + "" "check *stopped from CLI command 2" > + > mi_gdb_test "600-break-insert -t basics.c:$line_main_hello" \ > {600\^done,bkpt=.number="3",type="breakpoint".*\}} \ > "-break-insert -t basics.c:\$line_main_hello" > > -mi_execute_to "exec-continue" "breakpoint-hit" "main" "" ".*basics.c" \ > - $line_main_hello { "" "disp=\"del\"" } \ > - "-exec-continue to line \$line_main_hello" > +# Test that breakpoint events are always mirrored to the CLI output > +# stream (both sync and async modes). > +mi_send_resuming_command "exec-continue" "-exec-continue to line \$line_main_hello" > + > +set output [mi_gdb_expect_cli_output "\\*stopped" "collect CLI output for breakpoint hit"] > +set test "breakpoint hit produces CLI output" > +set pattern "\\\\nTemporary breakpoint 3, main \\(\\) at \[^\n\]+basics.c:$line_main_hello\\\\n\[^\n\]+Hello" > + > +if {[regexp $pattern $output]} { > + pass $test > +} else { > + fail $test > +} > + > +# Test the MI output. > +mi_expect_stop "breakpoint-hit" "main" "" ".*basics.c" \ > + $line_main_hello { "" "disp=\"del\"" } "Temporary breakpoint output hit in MI" > > # Test that the token is output even for CLI commands > # Also test that *stopped includes frame information. > @@ -167,10 +199,16 @@ 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 to the console > +# stream, given we executed the console 'next' command, not > +# -exec-next. > +set test "34 next: CLI output" > +gdb_expect { > + -re "~\"$line_main_return\[^\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 608d2b7..a684a3d 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/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp > index 213073a..1f6dfa7 100644 > --- a/gdb/testsuite/lib/mi-support.exp > +++ b/gdb/testsuite/lib/mi-support.exp > @@ -799,6 +799,30 @@ proc mi_gdb_test { args } { > return $result > } > > +# Collect output sent to the console output stream until UNTIL is > +# seen. UNTIL is a regular expression. MESSAGE is the message to be > +# printed in case of timeout. > + > +proc mi_gdb_expect_cli_output {until message} { > + > + set output "" > + gdb_expect { > + -re "~\"(\[^\r\n\]+)\"\r\n" { > + append output $expect_out(1,string) > + exp_continue > + } > + -notransfer -re "$until" { > + # Done > + } > + timeout { > + fail "$message (timeout)" > + return "" > + } > + } > + > + return $output > +} > + > # > # MI run command. (A modified version of gdb_run_cmd) > # >
diff --git a/gdb/cli-out.c b/gdb/cli-out.c index 5943fa7..eedbd2c 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 */ @@ -367,7 +378,7 @@ const 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 9b43a68..9f5dee6 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 f783260..fed4a27 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -433,6 +433,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) { @@ -484,6 +485,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, @@ -495,6 +497,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; @@ -539,6 +542,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 { @@ -1999,6 +2003,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); } @@ -2200,6 +2206,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=%s, step=%d)\n", diff --git a/gdb/interps.c b/gdb/interps.c index e446747..f6b941c 100644 --- a/gdb/interps.c +++ b/gdb/interps.c @@ -318,6 +318,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) @@ -351,9 +374,20 @@ interp_set_quiet (struct interp *interp, int quiet) struct gdb_exception interp_exec (struct interp *interp, const char *command_str) { + struct gdb_exception ex; + struct interp *save_command_interp; + gdb_assert (interp->procs->exec_proc != NULL); - return interp->procs->exec_proc (interp->data, command_str); + /* 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; } /* A convenience routine that nulls out all the common command hooks. diff --git a/gdb/interps.h b/gdb/interps.h index 568b5df..13edf42 100644 --- a/gdb/interps.h +++ b/gdb/interps.h @@ -96,6 +96,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 862beaf..80fffa3 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. */ @@ -231,6 +232,10 @@ mi_cmd_interpreter_exec (char *command, char **argv, int argc) error (_("-interpreter-exec: could not find interpreter \"%s\""), 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 @@ -415,6 +420,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) { @@ -445,6 +470,58 @@ 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 + { + /* Breakpoint hits should always be mirrored to the console. + Deciding what to mirror to the console wrt to breakpoints + and random stops gets messy real fast. E.g., say "s" + trips on a breakpoint. We'd clearly want to mirror the + event to the console in this case. But what about more + complicated cases like "s&; thread n; s&", and one of + those steps spawning a new thread, and that thread + hitting a breakpoint? It's impossible in general to + track whether the thread had any relation to the commands + that had been executed. So we just simplify and always + mirror breakpoints and random events to the console. + + Also, CLI execution commands (-interpreter-exec console + "next", for example) in async mode have the opposite + issue as described in the "then" branch above -- + 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). */ + struct thread_info *tp = inferior_thread (); + + if ((!tp->control.stop_step + && !tp->control.proceed_to_finish) + || (tp->control.command_interp != NULL + && tp->control.command_interp != top_level_interpreter ())) + { + 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 cb8d3af..50313ae 100644 --- a/gdb/testsuite/gdb.mi/mi-cli.exp +++ b/gdb/testsuite/gdb.mi/mi-cli.exp @@ -69,6 +69,7 @@ set line_main_callme_2 [expr $line_main_return + 1] set line_callee4_head [gdb_get_line_number "callee4 ("] set line_callee4_body [expr $line_callee4_head + 2] set line_callee4_next [expr $line_callee4_body + 1] +set line_callee4_next_step [expr $line_callee4_next + 3] mi_gdb_test "-interpreter-exec console \"set args foobar\"" \ ".*=cmd-param-changed,param=\"args\",value=\"foobar\".*\\^done" \ @@ -153,13 +154,44 @@ if {$async} { mi_execute_to "interpreter-exec console step" $reason "callee4" "" ".*basics.c" $line_callee4_next \ "" "check *stopped from CLI command" +mi_send_resuming_command "exec-step" "-exec-step to line \$line_callee4_next_step" + +# Test that the new current source line is _not_ output, given we +# executed MI's -exec-next, not CLI's 'next' command. + +set output [mi_gdb_expect_cli_output "\\*stopped" "collect CLI output for -exec-step"] + +set test "-exec-step does not produce CLI step output" +if {[regexp "A + B" "$output"]} { + fail $test +} else { + pass $test +} + +mi_expect_stop "end-stepping-range" "callee4" "" ".*basics.c" $line_callee4_next_step \ + "" "check *stopped from CLI command 2" + mi_gdb_test "600-break-insert -t basics.c:$line_main_hello" \ {600\^done,bkpt=.number="3",type="breakpoint".*\}} \ "-break-insert -t basics.c:\$line_main_hello" -mi_execute_to "exec-continue" "breakpoint-hit" "main" "" ".*basics.c" \ - $line_main_hello { "" "disp=\"del\"" } \ - "-exec-continue to line \$line_main_hello" +# Test that breakpoint events are always mirrored to the CLI output +# stream (both sync and async modes). +mi_send_resuming_command "exec-continue" "-exec-continue to line \$line_main_hello" + +set output [mi_gdb_expect_cli_output "\\*stopped" "collect CLI output for breakpoint hit"] +set test "breakpoint hit produces CLI output" +set pattern "\\\\nTemporary breakpoint 3, main \\(\\) at \[^\n\]+basics.c:$line_main_hello\\\\n\[^\n\]+Hello" + +if {[regexp $pattern $output]} { + pass $test +} else { + fail $test +} + +# Test the MI output. +mi_expect_stop "breakpoint-hit" "main" "" ".*basics.c" \ + $line_main_hello { "" "disp=\"del\"" } "Temporary breakpoint output hit in MI" # Test that the token is output even for CLI commands # Also test that *stopped includes frame information. @@ -167,10 +199,16 @@ 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 to the console +# stream, given we executed the console 'next' command, not +# -exec-next. +set test "34 next: CLI output" +gdb_expect { + -re "~\"$line_main_return\[^\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 608d2b7..a684a3d 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/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp index 213073a..1f6dfa7 100644 --- a/gdb/testsuite/lib/mi-support.exp +++ b/gdb/testsuite/lib/mi-support.exp @@ -799,6 +799,30 @@ proc mi_gdb_test { args } { return $result } +# Collect output sent to the console output stream until UNTIL is +# seen. UNTIL is a regular expression. MESSAGE is the message to be +# printed in case of timeout. + +proc mi_gdb_expect_cli_output {until message} { + + set output "" + gdb_expect { + -re "~\"(\[^\r\n\]+)\"\r\n" { + append output $expect_out(1,string) + exp_continue + } + -notransfer -re "$until" { + # Done + } + timeout { + fail "$message (timeout)" + return "" + } + } + + return $output +} + # # MI run command. (A modified version of gdb_run_cmd) #