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

["Metzger, Markus T" <markus.t.metzger@intel.com>]

> -----Original Message-----
> From: Jan Kratochvil [mailto:jan.kratochvil@redhat.com]
> Sent: Thursday, March 07, 2013 10:07 AM


> > I'm not quite sure what the OS will do if tracing is requested twice for the same
> > thread, i.e. repeated "record stop" followed by "record btrace" .  We may get
> > an error or we may get a second buffer, thus leaking resources (i.e. the mmapped
> > trace buffer).  We will need another target method to disable branch tracing
> > that is called by the "record stop" command.
> 
> Yes, thanks for catching it, "record stop" needs to call real btrace_disable
> before unpushing the target.
> 
> 
> > In a gdbserver --multi session, I would expect that we're leaking the mmapped trace
> > buffer also on re-run.
> 
> During re-run the first process gets killed first, so btrace_clear on the GDB
> side is appropriate in such case.

The problem is that your patch wouldn't try to actually disable tracing on the target.
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.

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


> @@ -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.


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.
Discard all threads will now use the teardown path instead of the disable path.
The error will abort the thread traversal in to_close.

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

Is there a reliable way of determining whether the target connection is down?
If there is, I could disable normally as long as the target connection is up and
only leak resources when the connection is already down. I might not even need
the second disable path in that case.


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.

This would be quite elegant but I have no clue about the side effects this
might have.


> On the gdbserver side there is linux_disable_btrace still called:
> #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
> #7  in kill_inferior (pid=28282) at target.c:190
> #8  in handle_v_kill (own_buf=0x667370 "vKill;6e7a") at server.c:2273
> #9  in handle_v_requests (own_buf=0x667370 "vKill;6e7a", packet_len=10, new_packet_len=0x7fffffffd7c8) at server.c:2345
> #10 in process_serial_event () at server.c:3484
> 
> So I find that OK.
> 
> 
> > I think this can be handled with an appropriate to_create_inferior method.
> 
> I do not see a reason for that hook - could you be more specific?

I was only thinking about re-running an already running process.  But the issue
is more general and thus requires a more general solution.

Regards,
Markus.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052