Re: [PATCH 0/4 V7] MI notification on trace started/stopped

[Pedro Alves <palves@redhat.com>]

On 12/12/2013 01:49 PM, Yao Qi wrote:
> Hi,
> I examined all previous versions of this series and corresponding
> reviews.  I find that V2 is the closet one to acceptance.  After
> v2, I pursued "notification annex" and supported query on it,
> which is a blind alley. (I don't recall why I choose this way then).
> 
> I update my patches to follow V2 and address comments I got during
> the review.  The major comment raised by Pedro is that "GDB and
> the remote negotiate the set of supported notifications".

Also, I still think ordering issues between trace start/stop and
stop events needs to be considered.  Let me expand.

With trace events sent on a separate channel, we can get things
like this, with a single-threaded program being traced:

 #1 - tstart, resume program
 #2 - trace stops because buffer full
 #3 - gdbserver queues trace stop notification.
 #4 - breakpoint at "next_session" is hit.
 #5 - gdb happens to process breakpoint notification first.
 #6 - The "next_session" breakpoint has a breakpoint
      command that does: "tstop; tsave; tstart; continue"
      (program iterates, and another collection sample begins)
 #7 - GDB emits MI =trace-stopped then =trace-started in response
      to tstop/tstart above.
 #8 - gdb processes the pending trace stop notification, emits
      MI =trace-stopped to frontend.
 #9 - The frontend is confused, thinking the trace session is
      stopped, when it is in fact running.

Now, even if we sent trace stops down the %Stop
notification, that wouldn't happen.  GDB would always process
the trace stop notification before the breakpoint.

But, considering multi-threaded programs, a different thread
can stop the trace immediately after the breakpoint at
next_session hits and triggers a %Stop notification,
and so sending trace notifications using %Stop wouldn't
fix the frontend confusion in this case, as the stop would
still be processed before the trace stop.

What I'm just now thinking would fix it would be if the
remote _also_ triggered trace _start_ notifications in
addition to trace stops:

 #1 - tstart, resume program
 #2 - trace stops because buffer full
 #3 - gdbserver queues trace stop notification.
 #4 - breakpoint at "next_session" is hit.
 #5 - gdb happens to process breakpoint notification first.
 #6 - The "next_session" breakpoint has a breakpoint
      command that does: "tstop; tsave; tstart; continue"
      (program iterates, and another collection sample begins)
 #7 - GDB emits MI =trace-stopped then =trace-started in response
      to tstop/tstart above.
 #8 - gdb processes the pending trace stop notification, emits
      MI =trace-stopped to frontend.
 #9 - gdb processes a trace start notification, emits
      MI =trace-start to frontend.
 #10 - frontend displays the trace session as running.


Now, looking at this, we see MI may get this trace event
sequence:

...
 =trace-stopped  (caused by tstop in bp command)
 =trace-start    (caused by tstart in bp command)
 =trace-stopped  (triggered by target)
 =trace-start    (triggered by target)

even though the tracing only stopped and started once.

To fully fix that, MI trace events triggered
by GDB actions could be distinguishable from MI trace
events triggered by the target (e.g, an attribute),
and the frontend could only look at target-triggered
events.  I'm not sure that's really necessary, and it can
always be added later, but it may be nice to have.

Another way to fix the ordering issue I just thought would
be to have a sequence number associated with each trace
session, and send those along trace start/stop notifications,
so that a delayed =trace-stopped generated from the target
would be older than the current trace session, so
it would be ignored.  Not sure how I feel about that.
Not sure how I feel about the other solution either.  :-)

Seems to me something needs to be done though.

-- 
Pedro Alves