[PATCH] btrace: initiate teardown when closing record btrace target

[PATCH] btrace: initiate teardown when closing record btrace target

[Markus Metzger]

The to_teardown_btrace target method is used to free btrace resources
during shutdown when target record has already been unpushed and we
can't reliably talk to a remote target to disable branch tracing.

Tracing resources are freed for each thread when the thread is removed;
both on the GDB side and on the gdbserver side.

In the remote case, the remote target that provides to_teardown_btrace
to free the GDB side resources has already been unpushed when threads
are destroyed.  This results in a complaint "You can't do this ..." and
in a few bytes of memory leaked for each thread.

Initiate btrace teardown in record_btrace_close, so the remote target is
still in place.

2014-01-20  Markus Metzger  <markus.t.metzger@intel.com>

	* record-btrace.c (record_btrace_close): Call btrace_teardown
	for all threads.
---
 gdb/record-btrace.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 3a93fdb..365b6be 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -213,11 +213,16 @@ record_btrace_stop_recording (void)
 static void
 record_btrace_close (void)
 {
+  struct thread_info *tp;
+
   /* Make sure automatic recording gets disabled even if we did not stop
      recording before closing the record-btrace target.  */
   record_btrace_auto_disable ();
 
-  /* We already stopped recording.  */
+  /* We should have already stopped recording.
+     Tear down btrace in case we have not.  */
+  ALL_THREADS (tp)
+    btrace_teardown (tp);
 }
 
 /* The to_info_record method of target record-btrace.  */
-- 
1.8.3.1

Re: [PATCH] btrace: initiate teardown when closing record btrace target

[Jan Kratochvil]

On Mon, 20 Jan 2014 10:00:21 +0100, Markus Metzger wrote:
> In the remote case, the remote target that provides to_teardown_btrace
> to free the GDB side resources has already been unpushed when threads
> are destroyed.  This results in a complaint "You can't do this ..." and
> in a few bytes of memory leaked for each thread.

Could you provide an example when this error happens?

I can reproduce it with plain gdb+gdbserver but not with btrace functionality.


Thanks,
Jan

RE: [PATCH] btrace: initiate teardown when closing record btrace target

[Metzger, Markus T]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Jan Kratochvil
> Sent: Tuesday, January 21, 2014 3:17 PM


> On Mon, 20 Jan 2014 10:00:21 +0100, Markus Metzger wrote:
> > In the remote case, the remote target that provides to_teardown_btrace
> > to free the GDB side resources has already been unpushed when threads
> > are destroyed.  This results in a complaint "You can't do this ..." and
> > in a few bytes of memory leaked for each thread.
> 
> Could you provide an example when this error happens?
> 
> I can reproduce it with plain gdb+gdbserver but not with btrace functionality.

It happens with a simple hello-world.

$ gdbserver :2001 hello
$ gdb hello
(gdb) target remote :2001
...
(gdb) rec b
(gdb) kill
Kill the program being debugged? (y or n) y
You can't do that when your target is 'exec'.
(gdb)

When I set a breakpoint at tcomplain and go up, I'm in taret_teardown_btrace.

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

Re: [PATCH] btrace: initiate teardown when closing record btrace target

[Jan Kratochvil]

Hello Markus,

On Mon, 20 Jan 2014 10:00:21 +0100, Markus Metzger wrote:
> Initiate btrace teardown in record_btrace_close, so the remote target is
> still in place.
> 
> 2014-01-20  Markus Metzger  <markus.t.metzger@intel.com>
> 
> 	* record-btrace.c (record_btrace_close): Call btrace_teardown
> 	for all threads.

yes, fine with the patch.

The placement in record_btrace_close() is also symmetrical to btrace_enable()
in record_btrace_open().


Thanks,
Jan