[PATCH:MI] Event notification

[PATCH:MI] Event notification

[Nick Roberts]

This patch adds event notification for changes in selected frame and thread.
In annotations, file and line information are output when the program stops and
when the selected frame is changed, e.g., with "up" or "down".  Currently in MI
this information is only output for the former case.  For threads, -thread-info
doesn't give the selected frame and this may currently change when execution
stops.  In any case, providing a notification it means that the frontend
doesn't need to interrogate Gdb.

A further advantage of using notifications is that they are output even when a
CLI command is issued from the console.  I've discussed these ideas on the
mailing list before but not used observers and in the past I've got a bit hung
up on trying to detect changes in stack (too difficult).

I've not run the testsuite as I imagine it will break in many places.  I want
to get this idea of decoupling MI output from the input command accepted first.
In this respect, it is somewhat similar to asynchronous output in asynchronous
mode.

I've not dealt with breakpoint changes but these could be worked similarly.
Indeed if you look at breakpoint.c and mi-cmd-break.c you can see that this
was started (but not completed) using events.  The command "-break-insert"
uses deprecated_set_gdb_event_hooks and I'm quite sure that the plan was
that "-break-enable" and "-break-disable" should too, as currently when
breakpoint_modify_event and breakpoint_delete_event are called the hooks
aren't set.  It should be quite easy to convert breakpoint.c at these
places to use observers.

This patch is on top of my earlier one (Avoid breakpoint query in MI)
for interp.c and interp.h.  For the moment, it uses interp_top_level.

-- 
Nick                                           http://www.inet.net.nz/~nickrob


2008-04-18  Nick Roberts  <nickrob@snap.net.nz>

	* mi/mi-interp.c (mi_interpreter_init): Register observers for
	frame and thread changes.
	(mi_frame_changed, mi_thread_changed): New static functions.

	* frame.c (select_frame): Notify observer for frame changes.

	* thread.c (do_captured_thread_select): Only print output to CLI
	Notify observer for thread changes.

	* infrun.c (normal_stop): Notify observer for thread changes.

2008-04-18  Nick Roberts  <nickrob@snap.net.nz>

	* observer.texi (GDB Observers): New observers frame_changed and
	thread_changed.


Index: mi/mi-interp.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-interp.c,v
retrieving revision 1.27
diff -p -r1.27 mi-interp.c
*** mi/mi-interp.c	4 Apr 2008 21:59:25 -0000	1.27
--- mi/mi-interp.c	17 Apr 2008 23:16:48 -0000
*************** static void mi_insert_notify_hooks (void
*** 67,72 ****
--- 67,74 ----
  static void mi_remove_notify_hooks (void);
  
  static void mi_new_thread (struct thread_info *t);
+ static void mi_frame_changed ();
+ static void mi_thread_changed ();
  
  static void *
  mi_interpreter_init (int top_level)
*************** mi_interpreter_init (int top_level)
*** 88,94 ****
    mi->event_channel = mi_console_file_new (raw_stdout, "=", 0);
  
    if (top_level)
!     observer_attach_new_thread (mi_new_thread);
  
    return mi;
  }
--- 90,100 ----
    mi->event_channel = mi_console_file_new (raw_stdout, "=", 0);
  
    if (top_level)
!     {
!       observer_attach_new_thread (mi_new_thread);
!       observer_attach_frame_changed (mi_frame_changed);
!       observer_attach_thread_changed (mi_thread_changed);
!     }
  
    return mi;
  }
*************** mi_new_thread (struct thread_info *t)
*** 316,321 ****
--- 322,372 ----
    gdb_flush (mi->event_channel);
  }
  
+ static void
+ mi_frame_changed ()
+ {
+   struct mi_interp *mi = top_level_interpreter_data ();
+   struct interp *interp_to_use;
+   struct ui_out *old_uiout, *temp_uiout;
+   int version;
+ 
+   if (target_has_registers && target_has_stack && target_has_memory)
+     {
+       fprintf_unfiltered (mi->event_channel, "frame-changed");
+       interp_to_use = interp_top_level ();
+       old_uiout = uiout;
+       temp_uiout = interp_ui_out (interp_to_use);
+       version = mi_version (temp_uiout);
+       temp_uiout = mi_out_new (version);
+       uiout = temp_uiout;
+       print_frame_info (get_selected_frame (NULL), 1, LOC_AND_ADDRESS, 0);
+       mi_out_put (uiout, mi->event_channel);
+       uiout = old_uiout;
+       gdb_flush (mi->event_channel);
+     }
+ }
+ 
+ static void
+ mi_thread_changed ()
+ {
+   struct mi_interp *mi = top_level_interpreter_data ();
+   struct interp *interp_to_use;
+   struct ui_out *old_uiout, *temp_uiout;
+   int version;
+ 
+   fprintf_unfiltered (mi->event_channel, "thread-changed");
+   interp_to_use = interp_top_level ();
+   old_uiout = uiout;
+   temp_uiout = interp_ui_out (interp_to_use);
+   version = mi_version (temp_uiout);
+   temp_uiout = mi_out_new (version);
+   uiout = temp_uiout;
+   ui_out_field_int (uiout, "new-thread-id", pid_to_thread_id (inferior_ptid));
+   mi_out_put (uiout, mi->event_channel);
+   uiout = old_uiout;
+   gdb_flush (mi->event_channel);
+ }
+ 
  extern initialize_file_ftype _initialize_mi_interp; /* -Wmissing-prototypes */
  
  void
Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.238
diff -p -r1.238 frame.c
*** frame.c	13 Mar 2008 12:22:12 -0000	1.238
--- frame.c	17 Apr 2008 23:16:49 -0000
*************** select_frame (struct frame_info *fi)
*** 998,1003 ****
--- 998,1005 ----
  	{
  	  set_language (s->language);
  	}
+ 
+       observer_notify_frame_changed ();
      }
  }
  	
Index: thread.c
===================================================================
RCS file: /cvs/src/src/gdb/thread.c,v
retrieving revision 1.65
diff -p -r1.65 thread.c
*** thread.c	23 Mar 2008 09:53:52 -0000	1.65
--- thread.c	17 Apr 2008 23:16:50 -0000
*************** do_captured_thread_select (struct ui_out
*** 756,761 ****
--- 756,762 ----
  {
    int num;
    struct thread_info *tp;
+   char buf[16];
  
    num = value_as_long (parse_and_eval (tidstr));
  
*************** do_captured_thread_select (struct ui_out
*** 770,780 ****
    switch_to_thread (tp->ptid);
  
    ui_out_text (uiout, "[Switching to thread ");
!   ui_out_field_int (uiout, "new-thread-id", pid_to_thread_id (inferior_ptid));
    ui_out_text (uiout, " (");
    ui_out_text (uiout, target_tid_to_str (inferior_ptid));
    ui_out_text (uiout, ")]");
  
    print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC);
    return GDB_RC_OK;
  }
--- 771,784 ----
    switch_to_thread (tp->ptid);
  
    ui_out_text (uiout, "[Switching to thread ");
!   sprintf (buf, "%d",  pid_to_thread_id (inferior_ptid));
!   ui_out_text (uiout, buf);
    ui_out_text (uiout, " (");
    ui_out_text (uiout, target_tid_to_str (inferior_ptid));
    ui_out_text (uiout, ")]");
  
+   observer_notify_thread_changed (inferior_ptid);
+ 
    print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC);
    return GDB_RC_OK;
  }
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.268
diff -p -r1.268 infrun.c
*** infrun.c	15 Apr 2008 14:32:12 -0000	1.268
--- infrun.c	17 Apr 2008 23:16:52 -0000
*************** normal_stop (void)
*** 3106,3111 ****
--- 3106,3112 ----
        target_terminal_ours_for_output ();
        printf_filtered (_("[Switching to %s]\n"),
  		       target_pid_to_str (inferior_ptid));
+       observer_notify_thread_changed (inferior_ptid);
        previous_inferior_ptid = inferior_ptid;
      }
  
Index: doc/observer.texi
===================================================================
RCS file: /cvs/src/src/gdb/doc/observer.texi,v
retrieving revision 1.14
diff -p -r1.14 observer.texi
*** doc/observer.texi	14 Mar 2008 17:21:07 -0000	1.14
--- doc/observer.texi	17 Apr 2008 23:16:52 -0000
*************** previously loaded symbol table data has 
*** 133,135 ****
--- 133,143 ----
  The thread specified by @var{t} has been created.
  @end deftypefun
  
+ @deftypefun void frame_changed ()
+ A new frame has been selected.
+ @end deftypefun
+ 
+ @deftypefun void thread_changed ()
+ A new thread has been selected.
+ @end deftypefun
+ 

Re: [PATCH:MI] Event notification

[Daniel Jacobowitz]

I don't think anyone responded to this.

On Fri, Apr 18, 2008 at 11:20:38AM +1200, Nick Roberts wrote:
> I've not run the testsuite as I imagine it will break in many places.  I want
> to get this idea of decoupling MI output from the input command accepted first.
> In this respect, it is somewhat similar to asynchronous output in asynchronous
> mode.

I'm fine with this idea.  The thread changed observer looks fine.  I
think that the frame changed observer will fire too often;
select_frame is often used with cleanups to temporarily select another
frame.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH:MI] Event notification

[Vladimir Prus]

Nick Roberts wrote:

> This patch adds event notification for changes in selected frame and thread.
> In annotations, file and line information are output when the program stops and
> when the selected frame is changed, e.g., with "up" or "down".  Currently in MI
> this information is only output for the former case.  For threads, -thread-info
> doesn't give the selected frame and this may currently change when execution
> stops.  

I think that when execution stops, all threads have frame 0 as selected, no?

> In any case, providing a notification it means that the frontend 
> doesn't need to interrogate Gdb.
> 
> A further advantage of using notifications is that they are output even when a
> CLI command is issued from the console.  I've discussed these ideas on the
> mailing list before but not used observers and in the past I've got a bit hung
> up on trying to detect changes in stack (too difficult).
> 
> I've not run the testsuite as I imagine it will break in many places.  I want
> to get this idea of decoupling MI output from the input command accepted first.
> In this respect, it is somewhat similar to asynchronous output in asynchronous
> mode.

Some interesting questions arise, with the first one -- what is the exact 
meaning of those new notifications and what is the expected reaction in frontend?
For example, suppose you have a bunch of fixed variable objects in different
threads. Then, -var-update * will switch between threads to evaluate the variable
objects. This, I think, will produce a bunch of thread change and frame change
notifications? What will frontend do? If a frontend updates the current
line indicator as result of those notifications, then "-var-update *" will
cause the line indicator to jump around in a fairly interesting way :-)

One way to address this is to suppress those notification for implicit
gdb activity. I don't see how we can easily do this. Another way would
be instruct the frontends to ignore those notifications during some commands.
But then, I'm not sure how to do this without creating a huge table of
commands that implicitly change thread/stack, and without running the risk
of making the frontend act funny if we forget a command, or unintentionally
make some other command switch thread/frames.

- Volodya

Re: [PATCH:MI] Event notification

[Daniel Jacobowitz]

On Sun, May 04, 2008 at 03:30:56PM +0400, Vladimir Prus wrote:
> I think that when execution stops, all threads have frame 0 as selected, no?

Most of the time, but not necessarily.  For Ada we automatically
select the exception site on catch/throw events.  For C++ we do not,
but we ought to.

> Some interesting questions arise, with the first one -- what is the exact 
> meaning of those new notifications and what is the expected reaction in frontend?
> For example, suppose you have a bunch of fixed variable objects in different
> threads. Then, -var-update * will switch between threads to evaluate the variable
> objects. This, I think, will produce a bunch of thread change and frame change
> notifications? What will frontend do? If a frontend updates the current
> line indicator as result of those notifications, then "-var-update *" will
> cause the line indicator to jump around in a fairly interesting way :-)
> 
> One way to address this is to suppress those notification for implicit
> gdb activity. I don't see how we can easily do this. Another way would
> be instruct the frontends to ignore those notifications during some commands.
> But then, I'm not sure how to do this without creating a huge table of
> commands that implicitly change thread/stack, and without running the risk
> of making the frontend act funny if we forget a command, or unintentionally
> make some other command switch thread/frames.

IIRC Nick added the thread switch events only when we print out
[Switching] messages for the CLI, which are already at the sites of
explicit activity.

The other way to do it is to generate these events at prompts, either
before the result or after the prompt.  We're about to display a
prompt so we check the selected frame against what was selected last
time we displayed a prompt.  It changed?  Show an event.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH:MI] Event notification

[Nick Roberts]

 > > This patch adds event notification for changes in selected frame and
 > > thread.  In annotations, file and line information are output when the
 > > program stops and when the selected frame is changed, e.g., with "up" or
 > > "down".  Currently in MI this information is only output for the former
 > > case.  For threads, -thread-info doesn't give the selected frame and this
 > > may currently change when execution stops.
 > 
 > I think that when execution stops, all threads have frame 0 as selected, no?

All threads may have frame 0, but the frame details are different, e.g.
currently:

-thread-select 2
^done,frame={level="0",addr="0x08048968",func="myproc",args=[{name="ptr_i",value="0xbff4f73c"}],file="pthreadtest.c",fullname="/home/nickrob/C/pthreadtest.c",line="91"}
(gdb) 
-thread-select 4
^done,frame={level="0",addr="0x08048859",func="myproc",args=[{name="ptr_i",value="0xbff4f744"}],file="pthreadtest.c",fullname="/home/nickrob/C/pthreadtest.c",line="53"}
(gdb) 

 > > In any case, providing a notification it means that the frontend 
 > > doesn't need to interrogate Gdb.
 > > 
 > > A further advantage of using notifications is that they are output even
 > > when a CLI command is issued from the console.  I've discussed these ideas
 > > on the mailing list before but not used observers and in the past I've got
 > > a bit hung up on trying to detect changes in stack (too difficult).
 > > 
 > > I've not run the testsuite as I imagine it will break in many places.  I
 > > want to get this idea of decoupling MI output from the input command
 > > accepted first.  In this respect, it is somewhat similar to asynchronous
 > > output in asynchronous mode.
 > 
 > Some interesting questions arise, with the first one -- what is the exact
 > meaning of those new notifications and what is the expected reaction in
 > frontend?  For example, suppose you have a bunch of fixed variable objects
 > in different threads. Then, -var-update * will switch between threads to
 > evaluate the variable objects. This, I think, will produce a bunch of thread
 > change and frame change notifications?

It currently issues a load of frame change notifications but no thread change
ones, just because the observer isn't registered in switch_to_thread.

 >                                        What will frontend do? If a frontend
 > updates the current line indicator as result of those notifications, then
 > "-var-update *" will cause the line indicator to jump around in a fairly
 > interesting way :-)

In annotations, you can prefix a command with "server" so that it doesn't
get added to the command history.  I guess you could have a similar
convention here: a token of 0 means don't print notifications, e.g,

^0-var-update --all-values *

If we do this now, it will be backward compatible because AFAIK released Gdb
doesn't print event notifications.

 > One way to address this is to suppress those notification for implicit
 > gdb activity. I don't see how we can easily do this. Another way would
 > be instruct the frontends to ignore those notifications during some commands.
 > But then, I'm not sure how to do this without creating a huge table of
 > commands that implicitly change thread/stack, and without running the risk
 > of making the frontend act funny if we forget a command, or unintentionally
 > make some other command switch thread/frames.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [PATCH:MI] Event notification

[Nick Roberts]

 >                                                             ... I
 > think that the frame changed observer will fire too often;
 > select_frame is often used with cleanups to temporarily select another
 > frame.

How about conditioning it so that it only fires when the target isn't running,
e.g., with the up, down, frame, -stack-select-frame etc commands?

Something like:

      if (!target_can_async_p () && !target_executing)
	observer_notify_frame_changed ();

which I realise isn't right because target_executing is for async mode.

In synchronous mode, is there a way to distinguish between the target
stopping normally and stopping while GDB executes?

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [PATCH:MI] Event notification

[Daniel Jacobowitz]

On Tue, Jun 10, 2008 at 01:39:34PM +1200, Nick Roberts wrote:
>  >                                                             ... I
>  > think that the frame changed observer will fire too often;
>  > select_frame is often used with cleanups to temporarily select another
>  > frame.
> 
> How about conditioning it so that it only fires when the target isn't running,
> e.g., with the up, down, frame, -stack-select-frame etc commands?

It's not about when the target is running.  Consider updating varobjs;
for every varobj with an associated frame, we will select the frame
and it will pass through select_frame.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH:MI] Event notification

[Nick Roberts]

 > > How about conditioning it so that it only fires when the target isn't
 > > running, e.g., with the up, down, frame, -stack-select-frame etc commands?
 > 
 > It's not about when the target is running.  Consider updating varobjs;
 > for every varobj with an associated frame, we will select the frame
 > and it will pass through select_frame.

Perhaps the answer then is to put notification explicitly in each CLI and MI
command (as I've proposed for thread_command and mi_cmd_thread_select).
However, it's always possible to just issue -stack-info-frame after execution
has stopped and maybe that's never expensive enough to justify such a brute
force approach.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [PATCH:MI] Event notification

[Pedro Alves]

A Tuesday 10 June 2008 03:53:27, Nick Roberts wrote:

> Perhaps the answer then is to put notification explicitly in each CLI and
> MI command (as I've proposed for thread_command and mi_cmd_thread_select).

I'm working on a patch that's needed for non-stop that splits the notion of
user/frontend selected thread and frame, from the internally
selected thread and frame.  With it in place, it should be easy to place an 
observer call in the function called to record the current thread and frame.

-- 
Pedro Alves