Re: [PATCH] Fix tracepoint tstart again get gdb_assert

[Pedro Alves <pedro at codesourcery dot com>]

On Friday 09 December 2011 09:53:23, Yao Qi wrote:
> On 12/08/2011 11:40 AM, Hui Zhu wrote:
> > Hi guys,
> > 
> > I am sorry that I didn't talk clear about the issue of tstatus.
> > When we use tstatus, it can auto set the tracepoint back to stop when
> > it full or got error.  Then user can use tstart without tstop that set
> >  loc->inserted.  So we will still meet that issue if we just set
> > loc->inserted in tstop.
> > 
> > For examp:
> > This gdb is just set  loc->inserted in tstop but not tstatus.
> > (gdb) tstart
> > (gdb) tstop
> > (gdb) tstart
> > (gdb) tstop
> > (gdb) tstart
> > xxx
> > xxx
> > (gdb) tstatus
> > Trace stopped because the buffer was full.
> > Buffer contains 0 trace frames (of 49072 created total).
> > Trace buffer has 908408 bytes of 10414080 bytes free (91% full).
> > Trace will stop if GDB disconnects.
> > Not looking at any trace frame.
> > (gdb) tstart
> > ../../src/gdb/tracepoint.c:1770: internal-error: start_tracing:
> > Assertion `!loc->inserted' failed.
> > A problem internal to GDB has been detected,
> > further debugging may prove unreliable.
> > Quit this debugging session? (y or n) n
> > 
> > ../../src/gdb/tracepoint.c:1770: internal-error: start_tracing:
> > Assertion `!loc->inserted' failed.
> > A problem internal to GDB has been detected,
> > further debugging may prove unreliable.
> > Create a core file of GDB? (y or n) n
> > 
> 
> This is the 2nd version of my patch.  Compared with 1st version (sent
> four hours ago), this version covers more cases.  In this patch, a new
> observer `trace_stopped' is added.  In stop_tracing and
> target_get_trace_status, observer `trace_stopped' is notified, and then,
> `inserted' flag is cleared.  This looks more clear than version 1.

I think this is overkill.  It's just impossible to be absolutely
certain a trace run is currently ongoing.  If you do "tstatus;tstart",
the trace can still stop between the two commands.  Is there
anything that we benefit from clearing the inserted flag
on tstatus?  I think that if we clear them only, and
always, at "tstart" time, we're good.  We already clear all
the inserted flag of all breakpoints and tracepoints on target
open ("target remote ...", etc.), from within
breakpoint.c:breakpoint_init_inferior -- that's why you don't
see this assertion if you disconnect and reconnect, and find
the target was tracing.

So I think we should just clear the inserted flag of all
tracepoints in start_tracing, right after the target_trace_init
call.  Possibly even merge the new loop with the existing loop
that downloads tracepoints.

-- 
Pedro Alves