This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v2] PR gdb/13860 - Make MI sync vs async output (closer to) the same.


On 05/23/2014 07:47 PM, Pedro Alves wrote:
> On 05/23/2014 04:46 AM, Yao Qi wrote:
>> The patch looks good.  One question and two suggestions below:
>>
>> On 05/23/2014 02:14 AM, Pedro Alves wrote:
>>> Fix this by making run control event printing roughly independent of
>>> whatever is the current interpreter or uiout.  That is, move these
>>> prints to interpreter observers, that know whether to print or be
>>> quiet, and if printing, which uiout to print to.  In the case of the
>>> console/tui interpreters, only print if the top interpreter.  For MI,
>>> always print.
>>
>> Currently, run control prints events directly to uiout and it has to
>> worry about different interpreters.  
> 
> Right.
> 
>> Looks your change is like
>> delegating print events to interpreter and let each interpreter decide
>> what uiout to use and how to print.
> 
> Yeah.
> 
>> If my understand is correct,
>> we'd better document somewhere in code that run control should delegate
>> events printing to interpreter, instead of printing events directly.
> 
> Good idea.  I've added it to the comment that introduces
> these print routines, like so:
> 
> --- c/gdb/infrun.c
> +++ w/gdb/infrun.c
> @@ -5904,7 +5904,11 @@ end_stepping_range (void)
>     The rest of the cases are dealt with later on in normal_stop and
>     print_it_typical.  Ideally there should be a call to one of these
>     print_*_reason functions functions from handle_inferior_event each time
> -   stop_stepping is called.  */
> +   stop_stepping is called.
> +
> +   Note that we don't call these directly, instead we delegate that to
> +   the interpreters, through observers.  Interpreters then call these
> +   with whatever uiout is right.  */
> 
> 
>>
>>>
>>> Breakpoint hits / normal stops are already handled similarly -- MI has
>>> a normal_stop observer that prints the event to both MI and the CLI,
>>> though that could be cleaned up further in the direction of this
>>> patch.
>>
>> Do you mean we can clean up print_stop_event in the same way you did in
>> this patch?
> 
> Yeah.  print_stop_event is already in a form close to this patch.
> Because it's called directly from within normal_stop for the CLI,
> MI has to go through contortions in mi_on_normal_stop to call
> it again with the right uiout.  I think we should be able to move
> the print_stop_event call out of normal_stop to 'normal_stop' CLI/TUI observers.
> That is, have the tui/console interpreters call print_stop_event
> from their own 'normal_stop' observers.  Then mi_on_normal_stop
> wouldn't need all that uiout flipping business:
> 
> static void
> mi_on_normal_stop (struct bpstats *bs, int print_frame)
> {
>   /* Since this can be called when CLI command is executing,
>      using cli interpreter, be sure to use MI uiout for output,
>      not the current one.  */
>   struct ui_out *mi_uiout = interp_ui_out (top_level_interpreter ());
> 
>       if (current_uiout != mi_uiout)
> 	{
> 	  /* The normal_stop function has printed frame information
> 	     into CLI uiout, or some other non-MI uiout.  There's no
> 	     way we can extract proper fields from random uiout
> 	     object, so we print the frame again.  In practice, this
> 	     can only happen when running a CLI command in MI.  */
> ...
> 	}
>       /* Otherwise, frame information has already been printed by
> 	 normal_stop.  */
> 
> 
> It'd just always call print_stop_event twice itself instead,
> like the other observers in this patch.
> 
>>
>>> diff --git a/gdb/infrun.h b/gdb/infrun.h
>>> index f0649f3..47509d9 100644
>>> --- a/gdb/infrun.h
>>> +++ b/gdb/infrun.h
>>> @@ -120,6 +120,29 @@ extern int stepping_past_instruction_at (struct address_space *aspace,
>>>  extern void set_step_info (struct frame_info *frame,
>>>  			   struct symtab_and_line sal);
>>>  
>>> +/* Several print_*_reason helper functions to print why the inferior
>>> +   has stopped to the passed in UIOUT.  The interpreters call
>>> +   these.  */
>>> +
>>> +/* Signal received, print why the inferior has stopped.  */
>>> +extern void print_signal_received_reason (struct ui_out *uiout,
>>> +					  enum gdb_signal siggnal);
>>> +
> ...
> 
>> I'd like to move these declarations to interps.h or interp-internal.h,
>> because these functions are only used by different interpreters, and
>> they can be invisible out side of interpreters.
> 
> I'd much rather not.  It's confusing to have the functions defined in
> infrun.c and declared elsewhere.  In fact, I just created infrun.h
> because of this:
> 
>   https://sourceware.org/ml/gdb-patches/2014-05/msg00542.html
> 
> It had happened more than once in the past that I've pointed out
> in review that infrun.c stuff went to inferior.h.  I believe
> foo.h+foo.c pairs makes things more obvious.
> 
> It may actually simplify things to move split the functions
> in CLI vs MI variants and move them to the respective
> interpreters instead.
> 

I pasted v1 by mistake...  Here's the real v2.  It's only
really the comment that changed though.  Oh and I remember
to add the PR number to the ChangeLog.


8<----------------
From: Pedro Alves <palves@redhat.com>
Subject: [PATCH] PR gdb/13860 - Make MI sync vs async output (closer to) the
 same.

This PR still not done.

Ignoring expected and desired differences like whether the prompt is
output after *stoppped records, GDB MI output is still different in
sync and async modes.

In sync mode, when a CLI execution command is entered, the "reason"
field is missing in the *stopped async record.  And in async mode, for
some events, like program exits, the corresponding CLI output is
missing in the CLI channel.

Vis, diff between sync vs async modes:

   run
   ^running
   *running,thread-id="1"
   (gdb)
   ...
 - ~"[Inferior 1 (process 15882) exited normally]\n"
   =thread-exited,id="1",group-id="i1"
   =thread-group-exited,id="i1",exit-code="0"
 - *stopped
 + *stopped,reason="exited-normally"

   si
   ...
   (gdb)
   ~"0x000000000045e033\t29\t  memset (&args, 0, sizeof args);\n"
 - *stopped,frame=...,thread-id="1",stopped-threads="all",core="0"
 + *stopped,reason="end-stepping-range",frame=...,thread-id="1",stopped-threads="all",core="0"
   (gdb)

In addition, in both cases, when a MI execution command is entered,
and a breakpoint triggers, the event is sent to the console too.  But
some events like program exits have the CLI output missing in the CLI
channel:

   -exec-run
   ^running
   *running,thread-id="1"
   (gdb)
   ...
   =thread-exited,id="1",group-id="i1"
   =thread-group-exited,id="i1",exit-code="0"
 - *stopped
 + *stopped,reason="exited-normally"

We'll want to make background commands always possible by default.
IOW, make target-async be the default.  But, in order to do that,
we'll need to emulate MI sync on top of an async target.  That means
we'll have yet another combination to care for in the testsuite.

Rather than making the testsuite cope with all these differences, I
thought it better to just fix GDB to always have the complete output,
no matter whether it's in sync or async mode.

This is all related to interpreter-exec, and the corresponding uiout
switching.  (Typing a CLI command directly in MI is shorthand for
running it through -interpreter-exec console.)

In sync mode, when a CLI command is active, normal_stop is called when
the current interpreter and uiout are CLI's.  So print_XXX_reason
prints the stop reason to CLI uiout (only), and we don't show it in
MI.

In async mode the stop event is processed when we're back in the MI
interpreter, so the stop reason is printed directly to the MI uiout.

Fix this by making run control event printing roughly independent of
whatever is the current interpreter or uiout.  That is, move these
prints to interpreter observers, that know whether to print or be
quiet, and if printing, which uiout to print to.  In the case of the
console/tui interpreters, only print if the top interpreter.  For MI,
always print.

Breakpoint hits / normal stops are already handled similarly -- MI has
a normal_stop observer that prints the event to both MI and the CLI,
though that could be cleaned up further in the direction of this
patch.

This also makes all of:

 (gdb) foo
and
 (gdb) interpreter-exec MI "-exec-foo"
and
 (gdb)
 -exec-foo
and
 (gdb)
 -interpreter-exec console "foo"

print as expected.

Tested on x86_64 Fedora 20, sync and async modes.

gdb/
2014-05-23  Pedro Alves  <palves@redhat.com>

	PR gdb/13860
	* cli/cli-interp.c: Include infrun.h and observer.h.
	(cli_uiout, cli_interp): New globals.
	(cli_on_signal_received, cli_on_end_stepping_range)
	(cli_on_signal_exited, cli_on_exited, cli_on_no_history): New
	functions.
	(cli_interpreter_init): Install them as 'end_stepping_range',
	'signal_received' 'signal_exited', 'exited' and 'no_history'
	observers.
	(_initialize_cli_interp): Remove cli_interp local.
	* infrun.c (handle_inferior_event): Call the several stop reason
	observers instead of printing the stop reason directly.
	(end_stepping_range): New function.
	(print_end_stepping_range_reason, print_signal_exited_reason)
	(print_exited_reason, print_signal_received_reason)
	(print_no_history_reason): Make static, and add an uiout
	parameter.  Print to that instead of to CURRENT_UIOUT.
	* infrun.h (print_end_stepping_range_reason)
	(print_signal_exited_reason, print_exited_reason)
	(print_signal_received_reason print_no_history_reason): New
	declarations.
	* mi/mi-common.h (struct mi_interp): Rename 'uiout' field to
	'mi_uiout'.
	<cli_uiout>: New field.
	* mi/mi-interp.c (mi_interpreter_init): Adjust.  Create the new
	uiout for CLI output.  Install 'signal_received',
	'end_stepping_range', 'signal_exited', 'exited' and 'no_history'
	observers.
	(find_mi_interpreter, mi_interp_data, mi_on_signal_received)
	(mi_on_end_stepping_range, mi_on_signal_exited, mi_on_exited)
	(mi_on_no_history): New functions.
	(ui_out_free_cleanup): Delete function.
	(mi_on_normal_stop): Don't allocate a new uiout for CLI output,
	instead use the one already stored in the MI interpreter data.
	(mi_ui_out): Adjust.
	* tui/tui-interp.c: Include infrun.h and observer.h.
	(tui_interp): New global.
	(tui_on_signal_received, tui_on_end_stepping_range)
	(tui_on_signal_exited, tui_on_exited)
	(tui_on_no_history): New functions.
	(tui_init): Install them as 'end_stepping_range',
	'signal_received' 'signal_exited', 'exited' and 'no_history'
	observers.
	(_initialize_tui_interp): Delete tui_interp local.

gdb/doc/
2014-05-23  Pedro Alves  <palves@redhat.com>

	PR gdb/13860
	* observer.texi (signal_received, end_stepping_range)
	(signal_exited, exited, no_history): New observer subjects.

gdb/testsuite/
2014-05-23  Pedro Alves  <palves@redhat.com>

	PR gdb/13860
	* gdb.mi/mi-cli.exp: Always expect "end-stepping-range" stop
	reason, even in sync mode.
---
 gdb/cli/cli-interp.c            |  64 ++++++++++++++++++-
 gdb/doc/observer.texi           |  20 ++++++
 gdb/infrun.c                    | 118 ++++++++++++++++------------------
 gdb/infrun.h                    |  22 +++++++
 gdb/mi/mi-common.h              |   5 +-
 gdb/mi/mi-interp.c              | 136 +++++++++++++++++++++++++++++++++++-----
 gdb/testsuite/gdb.mi/mi-cli.exp |  23 +------
 gdb/tui/tui-interp.c            |  64 ++++++++++++++++++-
 8 files changed, 349 insertions(+), 103 deletions(-)

diff --git a/gdb/cli/cli-interp.c b/gdb/cli/cli-interp.c
index d80ee58..d3badcd 100644
--- a/gdb/cli/cli-interp.c
+++ b/gdb/cli/cli-interp.c
@@ -25,21 +25,80 @@
 #include "top.h"		/* for "execute_command" */
 #include <string.h>
 #include "exceptions.h"
-
-struct ui_out *cli_uiout;
+#include "infrun.h"
+#include "observer.h"
 
 /* These are the ui_out and the interpreter for the console
    interpreter.  */
+struct ui_out *cli_uiout;
+static struct interp *cli_interp;
 
 /* Longjmp-safe wrapper for "execute_command".  */
 static struct gdb_exception safe_execute_command (struct ui_out *uiout,
 						  char *command, 
 						  int from_tty);
+
+/* Observers for several run control events.  If the interpreter is
+   quiet (i.e., another interpreter is being run with
+   interpreter-exec), print nothing.  */
+
+/* Observer for the signal_received notification.  */
+
+static void
+cli_on_signal_received (enum gdb_signal siggnal)
+{
+  if (!interp_quiet_p (cli_interp))
+    print_signal_received_reason (cli_uiout, siggnal);
+}
+
+/* Observer for the end_stepping_range notification.  */
+
+static void
+cli_on_end_stepping_range (void)
+{
+  if (!interp_quiet_p (cli_interp))
+    print_end_stepping_range_reason (cli_uiout);
+}
+
+/* Observer for the signalled notification.  */
+
+static void
+cli_on_signal_exited (enum gdb_signal siggnal)
+{
+  if (!interp_quiet_p (cli_interp))
+    print_signal_exited_reason (cli_uiout, siggnal);
+}
+
+/* Observer for the exited notification.  */
+
+static void
+cli_on_exited (int exitstatus)
+{
+  if (!interp_quiet_p (cli_interp))
+    print_exited_reason (cli_uiout, exitstatus);
+}
+
+/* Observer for the no_history notification.  */
+
+static void
+cli_on_no_history (void)
+{
+  if (!interp_quiet_p (cli_interp))
+    print_no_history_reason (cli_uiout);
+}
+
 /* These implement the cli out interpreter: */
 
 static void *
 cli_interpreter_init (struct interp *self, int top_level)
 {
+  /* If changing this, remember to update tui-interp.c as well.  */
+  observer_attach_end_stepping_range (cli_on_end_stepping_range);
+  observer_attach_signal_received (cli_on_signal_received);
+  observer_attach_signal_exited (cli_on_signal_exited);
+  observer_attach_exited (cli_on_exited);
+  observer_attach_no_history (cli_on_no_history);
+
   return NULL;
 }
 
@@ -155,7 +214,6 @@ _initialize_cli_interp (void)
     NULL,                       /* set_logging_proc */
     cli_command_loop            /* command_loop_proc */
   };
-  struct interp *cli_interp;
 
   /* Create a default uiout builder for the CLI.  */
   cli_uiout = cli_out_new (gdb_stdout);
diff --git a/gdb/doc/observer.texi b/gdb/doc/observer.texi
index 61acbb2..ee43fc5 100644
--- a/gdb/doc/observer.texi
+++ b/gdb/doc/observer.texi
@@ -94,6 +94,26 @@ the breakpoints were are stopped at, if any.  Second argument
 inferior has stopped.
 @end deftypefun
 
+@deftypefun void signal_received (enum gdb_signal @var{siggnal})
+The inferior was stopped by a signal.
+@end deftypefun
+
+@deftypefun void end_stepping_range (void)
+We are done with a step/next/si/ni command.
+@end deftypefun
+
+@deftypefun void signal_exited (enum gdb_signal @var{siggnal})
+The inferior was terminated by a signal.
+@end deftypefun
+
+@deftypefun void exited (int @var{exitstatus})
+The inferior program is finished.
+@end deftypefun
+
+@deftypefun void no_history (void)
+Reverse execution: target ran out of history info.
+@end deftypefun
+
 @deftypefun void target_changed (struct target_ops *@var{target})
 The target's register contents have changed.
 @end deftypefun
diff --git a/gdb/infrun.c b/gdb/infrun.c
index ec78148..461f6e7 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -88,15 +88,7 @@ static int currently_stepping (struct thread_info *tp);
 
 static void xdb_handle_command (char *args, int from_tty);
 
-static void print_exited_reason (int exitstatus);
-
-static void print_signal_exited_reason (enum gdb_signal siggnal);
-
-static void print_no_history_reason (void);
-
-static void print_signal_received_reason (enum gdb_signal siggnal);
-
-static void print_end_stepping_range_reason (void);
+static void end_stepping_range (void);
 
 void _initialize_infrun (void);
 
@@ -3528,7 +3520,7 @@ handle_inferior_event (struct execution_control_state *ecs)
 	  /* Support the --return-child-result option.  */
 	  return_child_result_value = ecs->ws.value.integer;
 
-	  print_exited_reason (ecs->ws.value.integer);
+	  observer_notify_exited (ecs->ws.value.integer);
 	}
       else
 	{
@@ -3557,7 +3549,7 @@ handle_inferior_event (struct execution_control_state *ecs)
 Cannot fill $_exitsignal with the correct signal number.\n"));
 	    }
 
-	  print_signal_exited_reason (ecs->ws.value.sig);
+	  observer_notify_signal_exited (ecs->ws.value.sig);
 	}
 
       gdb_flush (gdb_stdout);
@@ -3825,7 +3817,7 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
 	  singlestep_breakpoints_inserted_p = 0;
 	}
       stop_pc = regcache_read_pc (get_thread_regcache (ecs->ptid));
-      print_no_history_reason ();
+      observer_notify_no_history ();
       stop_stepping (ecs);
       return;
     }
@@ -4213,10 +4205,10 @@ handle_signal_stop (struct execution_control_state *ecs)
 
       if (signal_print[ecs->event_thread->suspend.stop_signal])
 	{
+	  /* The signal table tells us to print about this signal.  */
 	  printed = 1;
 	  target_terminal_ours_for_output ();
-	  print_signal_received_reason
-				     (ecs->event_thread->suspend.stop_signal);
+	  observer_notify_signal_received (ecs->event_thread->suspend.stop_signal);
 	}
       /* Always stop on signals if we're either just gaining control
 	 of the program, or the user explicitly requested this thread
@@ -4460,7 +4452,7 @@ process_event_stop_test (struct execution_control_state *ecs)
 	delete_step_resume_breakpoint (ecs->event_thread);
 
 	ecs->event_thread->control.stop_step = 1;
-	print_end_stepping_range_reason ();
+	end_stepping_range ();
 	stop_stepping (ecs);
       }
       return;
@@ -4625,7 +4617,7 @@ process_event_stop_test (struct execution_control_state *ecs)
 	  && execution_direction == EXEC_REVERSE)
 	{
 	  ecs->event_thread->control.stop_step = 1;
-	  print_end_stepping_range_reason ();
+	  end_stepping_range ();
 	  stop_stepping (ecs);
 	}
       else
@@ -4779,7 +4771,7 @@ process_event_stop_test (struct execution_control_state *ecs)
 	     well.  FENN */
 	  /* And this works the same backward as frontward.  MVS */
 	  ecs->event_thread->control.stop_step = 1;
-	  print_end_stepping_range_reason ();
+	  end_stepping_range ();
 	  stop_stepping (ecs);
 	  return;
 	}
@@ -4895,7 +4887,7 @@ process_event_stop_test (struct execution_control_state *ecs)
 	  && step_stop_if_no_debug)
 	{
 	  ecs->event_thread->control.stop_step = 1;
-	  print_end_stepping_range_reason ();
+	  end_stepping_range ();
 	  stop_stepping (ecs);
 	  return;
 	}
@@ -4991,7 +4983,7 @@ process_event_stop_test (struct execution_control_state *ecs)
 	     is set, we stop the step so that the user has a chance to
 	     switch in assembly mode.  */
 	  ecs->event_thread->control.stop_step = 1;
-	  print_end_stepping_range_reason ();
+	  end_stepping_range ();
 	  stop_stepping (ecs);
 	  return;
 	}
@@ -5012,7 +5004,7 @@ process_event_stop_test (struct execution_control_state *ecs)
       if (debug_infrun)
 	 fprintf_unfiltered (gdb_stdlog, "infrun: stepi/nexti\n");
       ecs->event_thread->control.stop_step = 1;
-      print_end_stepping_range_reason ();
+      end_stepping_range ();
       stop_stepping (ecs);
       return;
     }
@@ -5026,7 +5018,7 @@ process_event_stop_test (struct execution_control_state *ecs)
       if (debug_infrun)
 	 fprintf_unfiltered (gdb_stdlog, "infrun: no line number info\n");
       ecs->event_thread->control.stop_step = 1;
-      print_end_stepping_range_reason ();
+      end_stepping_range ();
       stop_stepping (ecs);
       return;
     }
@@ -5059,7 +5051,7 @@ process_event_stop_test (struct execution_control_state *ecs)
 	    step_into_inline_frame (ecs->ptid);
 
 	  ecs->event_thread->control.stop_step = 1;
-	  print_end_stepping_range_reason ();
+	  end_stepping_range ();
 	  stop_stepping (ecs);
 	  return;
 	}
@@ -5074,7 +5066,7 @@ process_event_stop_test (struct execution_control_state *ecs)
 	  else
 	    {
 	      ecs->event_thread->control.stop_step = 1;
-	      print_end_stepping_range_reason ();
+	      end_stepping_range ();
 	      stop_stepping (ecs);
 	    }
 	  return;
@@ -5101,7 +5093,7 @@ process_event_stop_test (struct execution_control_state *ecs)
       else
 	{
 	  ecs->event_thread->control.stop_step = 1;
-	  print_end_stepping_range_reason ();
+	  end_stepping_range ();
 	  stop_stepping (ecs);
 	}
       return;
@@ -5119,7 +5111,7 @@ process_event_stop_test (struct execution_control_state *ecs)
 	 fprintf_unfiltered (gdb_stdlog,
 			     "infrun: stepped to a different line\n");
       ecs->event_thread->control.stop_step = 1;
-      print_end_stepping_range_reason ();
+      end_stepping_range ();
       stop_stepping (ecs);
       return;
     }
@@ -5445,7 +5437,7 @@ handle_step_into_function (struct gdbarch *gdbarch,
     {
       /* We are already there: stop now.  */
       ecs->event_thread->control.stop_step = 1;
-      print_end_stepping_range_reason ();
+      end_stepping_range ();
       stop_stepping (ecs);
       return;
     }
@@ -5494,7 +5486,7 @@ handle_step_into_function_backward (struct gdbarch *gdbarch,
     {
       /* We're there already.  Just stop stepping now.  */
       ecs->event_thread->control.stop_step = 1;
-      print_end_stepping_range_reason ();
+      end_stepping_range ();
       stop_stepping (ecs);
     }
   else
@@ -5894,35 +5886,46 @@ prepare_to_wait (struct execution_control_state *ecs)
   ecs->wait_some_more = 1;
 }
 
+/* We are done with the step range of a step/next/si/ni command.
+   Called once for each n of a "step n" operation.  Notify observers
+   if not in the middle of doing a "step N" operation for N > 1.  */
+
+static void
+end_stepping_range (void)
+{
+  if (inferior_thread ()->step_multi
+      && inferior_thread ()->control.stop_step)
+    return;
+
+  observer_notify_end_stepping_range ();
+}
+
 /* Several print_*_reason functions to print why the inferior has stopped.
    We always print something when the inferior exits, or receives a signal.
    The rest of the cases are dealt with later on in normal_stop and
    print_it_typical.  Ideally there should be a call to one of these
    print_*_reason functions functions from handle_inferior_event each time
-   stop_stepping is called.  */
+   stop_stepping is called.
 
-/* Print why the inferior has stopped.  
-   We are done with a step/next/si/ni command, print why the inferior has
-   stopped.  For now print nothing.  Print a message only if not in the middle
-   of doing a "step n" operation for n > 1.  */
+   Note that we don't call these directly, instead we delegate that to
+   the interpreters, through observers.  Interpreters then call these
+   with whatever uiout is right.  */
 
-static void
-print_end_stepping_range_reason (void)
+void
+print_end_stepping_range_reason (struct ui_out *uiout)
 {
-  if ((!inferior_thread ()->step_multi
-       || !inferior_thread ()->control.stop_step)
-      && ui_out_is_mi_like_p (current_uiout))
-    ui_out_field_string (current_uiout, "reason",
-                         async_reason_lookup (EXEC_ASYNC_END_STEPPING_RANGE));
-}
+  /* For CLI-like interpreters, print nothing.  */
 
-/* The inferior was terminated by a signal, print why it stopped.  */
+  if (ui_out_is_mi_like_p (uiout))
+    {
+      ui_out_field_string (uiout, "reason",
+			   async_reason_lookup (EXEC_ASYNC_END_STEPPING_RANGE));
+    }
+}
 
-static void
-print_signal_exited_reason (enum gdb_signal siggnal)
+void
+print_signal_exited_reason (struct ui_out *uiout, enum gdb_signal siggnal)
 {
-  struct ui_out *uiout = current_uiout;
-
   annotate_signalled ();
   if (ui_out_is_mi_like_p (uiout))
     ui_out_field_string
@@ -5941,14 +5944,11 @@ print_signal_exited_reason (enum gdb_signal siggnal)
   ui_out_text (uiout, "The program no longer exists.\n");
 }
 
-/* The inferior program is finished, print why it stopped.  */
-
-static void
-print_exited_reason (int exitstatus)
+void
+print_exited_reason (struct ui_out *uiout, int exitstatus)
 {
   struct inferior *inf = current_inferior ();
   const char *pidstr = target_pid_to_str (pid_to_ptid (inf->pid));
-  struct ui_out *uiout = current_uiout;
 
   annotate_exited (exitstatus);
   if (exitstatus)
@@ -5977,14 +5977,9 @@ print_exited_reason (int exitstatus)
     }
 }
 
-/* Signal received, print why the inferior has stopped.  The signal table
-   tells us to print about it.  */
-
-static void
-print_signal_received_reason (enum gdb_signal siggnal)
+void
+print_signal_received_reason (struct ui_out *uiout, enum gdb_signal siggnal)
 {
-  struct ui_out *uiout = current_uiout;
-
   annotate_signal ();
 
   if (siggnal == GDB_SIGNAL_0 && !ui_out_is_mi_like_p (uiout))
@@ -6016,13 +6011,10 @@ print_signal_received_reason (enum gdb_signal siggnal)
   ui_out_text (uiout, ".\n");
 }
 
-/* Reverse execution: target ran out of history info, print why the inferior
-   has stopped.  */
-
-static void
-print_no_history_reason (void)
+void
+print_no_history_reason (struct ui_out *uiout)
 {
-  ui_out_text (current_uiout, "\nNo more reverse-execution history.\n");
+  ui_out_text (uiout, "\nNo more reverse-execution history.\n");
 }
 
 /* Print current location without a level number, if we have changed
diff --git a/gdb/infrun.h b/gdb/infrun.h
index f0649f3..66612a8 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -120,6 +120,28 @@ extern int stepping_past_instruction_at (struct address_space *aspace,
 extern void set_step_info (struct frame_info *frame,
 			   struct symtab_and_line sal);
 
+/* Several print_*_reason helper functions to print why the inferior
+   has stopped to the passed in UIOUT.  */
+
+/* Signal received, print why the inferior has stopped.  */
+extern void print_signal_received_reason (struct ui_out *uiout,
+					  enum gdb_signal siggnal);
+
+/* Print why the inferior has stopped.  We are done with a
+   step/next/si/ni command, print why the inferior has stopped.  */
+extern void print_end_stepping_range_reason (struct ui_out *uiout);
+
+/* The inferior was terminated by a signal, print why it stopped.  */
+extern void print_signal_exited_reason (struct ui_out *uiout,
+					enum gdb_signal siggnal);
+
+/* The inferior program is finished, print why it stopped.  */
+extern void print_exited_reason (struct ui_out *uiout, int exitstatus);
+
+/* Reverse execution: target ran out of history info, print why the
+   inferior has stopped.  */
+extern void print_no_history_reason (struct ui_out *uiout);
+
 extern void print_stop_event (struct target_waitstatus *ws);
 
 extern int signal_stop_state (int);
diff --git a/gdb/mi/mi-common.h b/gdb/mi/mi-common.h
index a6b0103..71e5d78 100644
--- a/gdb/mi/mi-common.h
+++ b/gdb/mi/mi-common.h
@@ -58,7 +58,10 @@ struct mi_interp
   struct ui_file *event_channel;
 
   /* MI's builder.  */
-  struct ui_out *uiout;
+  struct ui_out *mi_uiout;
+
+  /* MI's CLI builder (wraps OUT).  */
+  struct ui_out *cli_uiout;
 
   /* This is the interpreter for the mi... */
   struct interp *mi2_interp;
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 99479cc..e8e30e2 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -56,7 +56,13 @@ static int mi_interp_query_hook (const char *ctlstr, va_list ap)
 
 static void mi_insert_notify_hooks (void);
 static void mi_remove_notify_hooks (void);
+
+static void mi_on_signal_received (enum gdb_signal siggnal);
+static void mi_on_end_stepping_range (void);
+static void mi_on_signal_exited (enum gdb_signal siggnal);
+static void mi_on_exited (int exitstatus);
 static void mi_on_normal_stop (struct bpstats *bs, int print_frame);
+static void mi_on_no_history (void);
 
 static void mi_new_thread (struct thread_info *t);
 static void mi_thread_exit (struct thread_info *t, int silent);
@@ -118,7 +124,16 @@ mi_interpreter_init (struct interp *interp, int top_level)
   else
     gdb_assert_not_reached ("unhandled MI version");
 
-  mi->uiout = mi_out_new (mi_version);
+  mi->mi_uiout = mi_out_new (mi_version);
+  mi->cli_uiout = cli_out_new (mi->out);
+
+  /* There are installed even if MI is not the top level interpreter.
+     The callbacks themselves decide whether to be skipped.  */
+  observer_attach_signal_received (mi_on_signal_received);
+  observer_attach_end_stepping_range (mi_on_end_stepping_range);
+  observer_attach_signal_exited (mi_on_signal_exited);
+  observer_attach_exited (mi_on_exited);
+  observer_attach_no_history (mi_on_no_history);
 
   if (top_level)
     {
@@ -431,14 +446,111 @@ restore_current_uiout_cleanup (void *arg)
   current_uiout = saved_uiout;
 }
 
-/* Cleanup that destroys the a ui_out object.  */
+/* Return the MI interpreter, if it is active -- either because it's
+   the top-level interpreter or the interpreter executing the current
+   command.  Returns NULL if the MI interpreter is not being used.  */
+
+static struct interp *
+find_mi_interpreter (void)
+{
+  struct interp *interp;
+
+  interp = top_level_interpreter ();
+  if (ui_out_is_mi_like_p (interp_ui_out (interp)))
+    return interp;
+
+  interp = command_interp ();
+  if (ui_out_is_mi_like_p (interp_ui_out (interp)))
+    return interp;
+
+  return NULL;
+}
+
+/* Return the MI_INTERP structure of the active MI interpreter.
+   Returns NULL if MI is not active.  */
+
+static struct mi_interp *
+mi_interp_data (void)
+{
+  struct interp *interp = find_mi_interpreter ();
+
+  if (interp != NULL)
+    return interp_data (interp);
+  return NULL;
+}
+
+/* Observers for several run control events that print why the
+   inferior has stopped to both the the MI event channel and to the MI
+   console.  If the MI interpreter is not active, print nothing.  */
+
+/* Observer for the signal_received notification.  */
+
+static void
+mi_on_signal_received (enum gdb_signal siggnal)
+{
+  struct mi_interp *mi = mi_interp_data ();
+
+  if (mi == NULL)
+    return;
+
+  print_signal_received_reason (mi->mi_uiout, siggnal);
+  print_signal_received_reason (mi->cli_uiout, siggnal);
+}
+
+/* Observer for the end_stepping_range notification.  */
+
+static void
+mi_on_end_stepping_range (void)
+{
+  struct mi_interp *mi = mi_interp_data ();
+
+  if (mi == NULL)
+    return;
+
+  print_end_stepping_range_reason (mi->mi_uiout);
+  print_end_stepping_range_reason (mi->cli_uiout);
+}
+
+/* Observer for the signal_exited notification.  */
 
 static void
-ui_out_free_cleanup (void *arg)
+mi_on_signal_exited (enum gdb_signal siggnal)
 {
-  struct ui_out *uiout = arg;
+  struct mi_interp *mi = mi_interp_data ();
 
-  ui_out_destroy (uiout);
+  if (mi == NULL)
+    return;
+
+  print_signal_exited_reason (mi->mi_uiout, siggnal);
+  print_signal_exited_reason (mi->cli_uiout, siggnal);
+}
+
+/* Observer for the exited notification.  */
+
+static void
+mi_on_exited (int exitstatus)
+{
+  struct mi_interp *mi = mi_interp_data ();
+
+  if (mi == NULL)
+    return;
+
+  print_exited_reason (mi->mi_uiout, exitstatus);
+  print_exited_reason (mi->cli_uiout, exitstatus);
+}
+
+/* Observer for the no_history notification.  */
+
+static void
+mi_on_no_history (void)
+{
+  struct mi_interp *mi = mi_interp_data ();
+
+  if (mi == NULL)
+    return;
+
+  print_no_history_reason (mi->mi_uiout);
+  print_no_history_reason (mi->cli_uiout);
 }
 
 static void
@@ -506,16 +618,12 @@ mi_on_normal_stop (struct bpstats *bs, int print_frame)
 	      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;
+	      /* Set the current uiout to CLI uiout temporarily.  */
+	      old_chain = make_cleanup (restore_current_uiout_cleanup,
+					current_uiout);
+	      current_uiout = mi->cli_uiout;
 
 	      get_last_target_status (&last_ptid, &last);
 	      print_stop_event (&last);
@@ -972,7 +1080,7 @@ mi_ui_out (struct interp *interp)
 {
   struct mi_interp *mi = interp_data (interp);
 
-  return mi->uiout;
+  return mi->mi_uiout;
 }
 
 /* Save the original value of raw_stdout here when logging, so we can
diff --git a/gdb/testsuite/gdb.mi/mi-cli.exp b/gdb/testsuite/gdb.mi/mi-cli.exp
index 81f8898..107a579 100644
--- a/gdb/testsuite/gdb.mi/mi-cli.exp
+++ b/gdb/testsuite/gdb.mi/mi-cli.exp
@@ -136,22 +136,7 @@ mi_gdb_test "500-stack-select-frame 0" \
   {500\^done} \
   "-stack-select-frame 0"
 
-# When a CLI command is entered in MI session, the respose is different in
-# sync and async modes. In sync mode normal_stop is called when current
-# interpreter is CLI. So:
-#   - print_stop_reason prints stop reason in CLI uiout, and we don't show it
-#     in MI
-#   - The stop position is printed, and appears in MI 'console' channel.
-#
-# In async mode the stop event is processed when we're back to MI interpreter,
-# so the stop reason is printed into MI uiout an.
-if {$async} {
-    set reason "end-stepping-range"
-} else {
-    set reason ""
-}
-
-mi_execute_to "interpreter-exec console step" $reason "callee4" "" ".*basics.c" $line_callee4_next \
+mi_execute_to "interpreter-exec console step" "end-stepping-range" "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"
@@ -212,11 +197,7 @@ gdb_expect {
     }
 }
 
-# Note that the output does not include stop reason. This is fine.
-# The purpose of *stopped notification for CLI command is to make
-# sure that frontend knows that inferior is stopped, and knows where.
-# Supplementary information is not necessary.
-mi_expect_stop "$reason" "main" "" ".*basics.c" $line_main_return "" \
+mi_expect_stop "end-stepping-range" "main" "" ".*basics.c" $line_main_return "" \
     "34 next: stop"
 
 mi_gdb_test "-interpreter-exec console \"list\"" \
diff --git a/gdb/tui/tui-interp.c b/gdb/tui/tui-interp.c
index 2958972..cd11148 100644
--- a/gdb/tui/tui-interp.c
+++ b/gdb/tui/tui-interp.c
@@ -30,11 +30,18 @@
 #include "tui/tui.h"
 #include "tui/tui-io.h"
 #include "exceptions.h"
+#include "infrun.h"
+#include "observer.h"
+
+static struct ui_out *tui_ui_out (struct interp *self);
 
 /* Set to 1 when the TUI mode must be activated when we first start
    gdb.  */
 static int tui_start_enabled = 0;
 
+/* The TUI interpreter.  */
+static struct interp *tui_interp;
+
 /* Cleanup the tui before exiting.  */
 
 static void
@@ -48,6 +55,55 @@ tui_exit (void)
 /* True if TUI is the top-level interpreter.  */
 static int tui_is_toplevel = 0;
 
+/* Observers for several run control events.  If the interpreter is
+   quiet (i.e., another interpreter is being run with
+   interpreter-exec), print nothing.  */
+
+/* Observer for the signal_received notification.  */
+
+static void
+tui_on_signal_received (enum gdb_signal siggnal)
+{
+  if (!interp_quiet_p (tui_interp))
+    print_signal_received_reason (tui_ui_out (tui_interp), siggnal);
+}
+
+/* Observer for the end_stepping_range notification.  */
+
+static void
+tui_on_end_stepping_range (void)
+{
+  if (!interp_quiet_p (tui_interp))
+    print_end_stepping_range_reason (tui_ui_out (tui_interp));
+}
+
+/* Observer for the signal_exited notification.  */
+
+static void
+tui_on_signal_exited (enum gdb_signal siggnal)
+{
+  if (!interp_quiet_p (tui_interp))
+    print_signal_exited_reason (tui_ui_out (tui_interp), siggnal);
+}
+
+/* Observer for the exited notification.  */
+
+static void
+tui_on_exited (int exitstatus)
+{
+  if (!interp_quiet_p (tui_interp))
+    print_exited_reason (tui_ui_out (tui_interp), exitstatus);
+}
+
+/* Observer for the no_history notification.  */
+
+static void
+tui_on_no_history (void)
+{
+  if (!interp_quiet_p (tui_interp))
+    print_no_history_reason (tui_ui_out (tui_interp));
+}
+
 /* These implement the TUI interpreter.  */
 
 static void *
@@ -65,6 +121,13 @@ tui_init (struct interp *self, int top_level)
   if (ui_file_isatty (gdb_stdout))
     tui_initialize_readline ();
 
+  /* If changing this, remember to update cli-interp.c as well.  */
+  observer_attach_signal_received (tui_on_signal_received);
+  observer_attach_end_stepping_range (tui_on_end_stepping_range);
+  observer_attach_signal_exited (tui_on_signal_exited);
+  observer_attach_exited (tui_on_exited);
+  observer_attach_no_history (tui_on_no_history);
+
   return NULL;
 }
 
@@ -156,7 +219,6 @@ _initialize_tui_interp (void)
     NULL,
     cli_command_loop
   };
-  struct interp *tui_interp;
 
   /* Create a default uiout builder for the TUI.  */
   tui_interp = interp_new (INTERP_TUI, &procs);
-- 
1.9.0



-- 
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]