From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11510 invoked by alias); 29 Jul 2013 16:45:39 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 11467 invoked by uid 89); 29 Jul 2013 16:45:39 -0000 X-Spam-SWARE-Status: No, score=-4.7 required=5.0 tests=AWL,BAYES_50,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RDNS_NONE,SPF_HELO_PASS,SPF_PASS autolearn=no version=3.3.1 Received: from Unknown (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Mon, 29 Jul 2013 16:45:37 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r6TGjTKl030419 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 29 Jul 2013 12:45:29 -0400 Received: from barimba.redhat.com (ovpn-113-128.phx2.redhat.com [10.3.113.128]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r6TGjPbI023425; Mon, 29 Jul 2013 12:45:28 -0400 From: Tom Tromey To: gdb-patches@sourceware.org Cc: Pedro Alves Subject: [PATCH 5/8] PR gdb/13860: don't lose '-interpreter-exec console EXECUTION_COMMAND''s output in async mode. Date: Mon, 29 Jul 2013 16:45:00 -0000 Message-Id: <1375116324-32092-6-git-send-email-tromey@redhat.com> In-Reply-To: <1375116324-32092-1-git-send-email-tromey@redhat.com> References: <1375116324-32092-1-git-send-email-tromey@redhat.com> X-SW-Source: 2013-07/txt/msg00719.txt.bz2 From: Pedro Alves 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 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