Re: Crash of GDB with gdbserver btrace enabled [Re: [patch v9 00/23] branch tracing support for Atom]

[Jan Kratochvil <jan.kratochvil@redhat.com>]

On Thu, 07 Mar 2013 10:40:53 +0100, Metzger, Markus T wrote:
> The problem is that your patch wouldn't try to actually disable tracing on the target.

That is a responsibility of remote gdbserver.  If gdbserver needs then it
should disable/discard btrace on its own (which AFAIK it does).


> Gdbserver would still have the trace buffer mmapped.  I don't know if the OS can
> unmap those buffers when the traced process goes away - the buffers are owned
> by another process that is still running.

I have shown in the gdbserver backtrace that gdbserver's linux_mourn calls
linux_disable_btrace so I do not see why you think the btrace buffers remain
mapped on the gdbserver side:
	#0  linux_disable_btrace (tinfo=0x66fd40) at ../common/linux-btrace.c:509
	#1  in remove_thread (thread=0x66f900) at inferiors.c:165
	#2  in delete_lwp (lwp=0x66f7d0) at linux-low.c:334
	#3  in delete_lwp_callback (entry=0x66f7d0, proc=0x66f660) at linux-low.c:1234
	#4  in find_inferior (list=0x666540 <all_lwps>, func=0x425c61 <delete_lwp_callback>, arg=0x66f660) at inferiors.c:185
	#5  in linux_mourn (process=0x66f660) at linux-low.c:1248
	#6  in linux_kill (pid=28282) at linux-low.c:1058

There sure may be a case I forgot about but from your problem description I do
not see such case.


> This does not only affect re-run.  We should have the same issue when a pthread
> is joined.

The same like above, I do not see a reproducer of such case.



> > @@ -447,6 +447,12 @@ btrace_clear (struct thread_info *tp)
> > 
> >    btinfo = &tp->btrace;
> > 
> > +  if (btinfo->target != NULL)
> > +    {
> > +      target_clear_btrace (btinfo->target);
> > +      btinfo->target = NULL;
> > +    }
> 
> This is also something we don't want to do.  It would disable tracing when we
> just want to update the trace.

OK, I see - so there should be another function instead of reusing existing
btrace_clear.


> I have a patch that adds another disable path similar to your patch. That other
> path is used in clear_thread_inferior_resources.  So in to_close, we first try to
> disable branch tracing normally.  Should we realize that the target is gone, GDB
> will pop all targets, discard all threads, and throw an error.

The point was that during mourn inferior with gdbserver there should be no
gdb->gdbserver communication about btrace as it is gdbserver's problem.
GDB just asked gdbserver to mourn the inferior.


> Discard all threads will now use the teardown path instead of the disable path.
> The error will abort the thread traversal in to_close.

Also during to_close AFAIK no error should ever happen.


> The remaining problem is that this will leak branch trace buffers for joined
> pthreads in the remote case.

Again, gdbserver should track everything around its debuggee, gdb just asks
gdbserver to kill the inferior.


> Is there a reliable way of determining whether the target connection is down?

No, it may break in the middle of disabling btrace etc.  And there is no
reason for gdb to care about.


> Another approach would be to change pop_target to unpush the top target
> instead of just closing it.  This would prevent btrace from finding a target to
> disable tracing and all I would need to do is remove the tcomplain call in
> target_disable_btrace.

I do not see why, this will not run possible cleanups executed from its
to_close.


Thanks,
Jan