Prevent turning record on while threads are running (PR 20869)

Message ID A78C989F6D9628469189715575E55B234001353D@IRSMSX104.ger.corp.intel.com
State New, archived
Headers

Commit Message

Metzger, Markus T Nov. 30, 2016, 9:54 a.m. UTC
  > -----Original Message-----
> From: Simon Marchi [mailto:simon.marchi@ericsson.com]
> Sent: Tuesday, November 29, 2016 4:08 PM
> To: gdb-patches@sourceware.org
> Cc: Metzger, Markus T <markus.t.metzger@intel.com>; Simon Marchi
> <simon.marchi@ericsson.com>
> Subject: [PATCH] Prevent turning record on while threads are running (PR 20869)

Hi Simon,

Thanks for looking into this.

> I also wanted to know whether it was supported to start btrace bts/pt
> recording with threads running.  When I try it with btrace bts, it works
> halfway.  "record btrace bts" gives me the error:
> 
>   Couldn't get registers: No such process.
> 
> If I do the command again, gdb doesn't complain and then I'm able to
> interrupt the target and use reverse-next.  However, since the
> initialization function was interrupted halfway, I am not sure that
> everything is setup correctly.  If we want to allow it, we would first
> need to look into this issue.

The issue with record-btrace is that tracing can be enabled on the target also for
running threads.  When we try to insert the first record for the current PC so the
trace starts at the location from where we disabled tracing, regcache_read_pc
throws the above error.  Since record_btrace_open has not installed a cleanup
to disable tracing again for this thread, we leave it enabled.

(gdb) rec bts
[record-btrace] open
[btrace] enable thread 1 (Thread 0x7ffff7fcd740 (LWP 110345))
[btrace] compute ftrace
[btrace] enable thread 2 (Thread 0x7ffff77fd700 (LWP 110346))
[btrace] disable thread 1 (Thread 0x7ffff7fcd740 (LWP 110345))
[btrace] clear thread 1 (Thread 0x7ffff7fcd740 (LWP 110345))
Couldn't get registers: No such process.

The next time we try to enable tracing, that running thread is already traced so
we skip it.  If we have more than two threads running, we disable tracing again
on this thread when we fail on the next running thread.  But we always leave the
highest-number running thread traced.

(gdb) 
[record-btrace] open
[btrace] enable thread 1 (Thread 0x7ffff7fcd740 (LWP 110345))
[btrace] compute ftrace
[btrace] enable thread 4 (Thread 0x7ffff67fb700 (LWP 110348))
[btrace] disable thread 3 (Thread 0x7ffff6ffc700 (LWP 110347))
[btrace] clear thread 3 (Thread 0x7ffff6ffc700 (LWP 110347))
[btrace] disable thread 2 (Thread 0x7ffff77fd700 (LWP 110346))
[btrace] clear thread 2 (Thread 0x7ffff77fd700 (LWP 110346))
[btrace] disable thread 1 (Thread 0x7ffff7fcd740 (LWP 110345))
[btrace] clear thread 1 (Thread 0x7ffff7fcd740 (LWP 110345))
Couldn't get registers: No such process.

Eventually, all running threads are already traced and we succeed to enable tracing.

The trace for running threads will start from their next branch target instead of from their
current position (for BTS).  This shouldn't matter unless the thread was actually waiting
and not executing any code when we started tracing.  We should be able to allow it like
this:


Do you want to extend your patch to allow tracing to be enabled for btrace or
would you rather have me fix this on top of your patch?

Thanks,
Markus.

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
  

Patch

diff --git a/gdb/btrace.c b/gdb/btrace.c
index 39d537c..b005627 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -1474,8 +1474,28 @@  btrace_enable (struct thread_info *tp, const struct btrace_config *conf)
 
   /* Add an entry for the current PC so we start tracing from where we
      enabled it.  */
-  if (tp->btrace.target != NULL)
-    btrace_add_pc (tp);
+  TRY
+    {
+      if (tp->btrace.target != NULL)
+       btrace_add_pc (tp);
+    }
+  CATCH (error, RETURN_MASK_ERROR)
+    {
+      /* We may fail to add the initial entry, for example if TP is currently
+        running.
+
+        For BTRACE_FORMAT_PT this doesn't matter.
+
+        For BTRACE_FORMAT_BTS we won't be able to stitch the initial trace
+        chunk to this initial entry so tracing will start at the next branch
+        target instead of at the current PC.  Since TP is currently running,
+        this shouldn't make a difference.
+
+        If TP were waiting most of the time and made only a little bit of
+        progress before it was stopped, we'd lose the instructions until the
+        first branch.  */
+    }
+  END_CATCH
 }
 
 /* See btrace.h.  */