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

[Tom Tromey <tromey at redhat dot com>]

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