Re: [PATCH] Fix tracepoint tstart again get gdb_assert

[Stan Shebs <stanshebs at earthlink dot net>]

On 12/6/11 8:08 PM, Yao Qi wrote:
> On 12/06/2011 11:56 PM, Hui Zhu wrote:
> > Hi,
> >
> > I use a gdb tstart again in a section an got:
> >
> > (gdb) tstart
> > (gdb) tstop
> > (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)
> Thanks for this patch!  It is related to my previous commit
>
>    [patch 4/8] Download tracepoint locations and track its status
>    http://sourceware.org/ml/gdb-patches/2011-11/msg00337.html
>
> > The reason is:
> > start_tracing:
> >        for (loc = b->loc; loc; loc = loc->next)
> > 	{
> > 	  /* Since tracepoint locations are never duplicated, `inserted'
> > 	     flag should be zero.  */
> > 	  gdb_assert (!loc->inserted);
> >
> > 	  target_download_tracepoint (loc);
> >
> > 	  loc->inserted = 1;
> > 	}
> >
> > But in stop_tracing and trace_status_command don't have code to set
> > inserted back to 0.
> Right, I agree that loc->inserted should be cleared on stop_tracing.
> However, I don't know why we have to clear loc->inserted in
> trace_status_command.  A few comments below.

I agree, stop_tracing should clear loc->inserted, and tstatus should 
not; in general an information-reading/displaying command should not be 
making state changes.  If the assert triggers without the tstatus patch, 
then we should understand how and why.  (We might be missing a 
reconnection case or some such?)

Also, I notice that while the testsuite has lots of tstart/tstop 
sequences, it normally starts a fresh GDB each time, so it was unable to 
catch this failure.  The patch to fix should add a sequence of a couple 
trace runs in a row to one of the existing test files.

Stan