[PATCH 3/3] btrace: update tail call heuristic

[PATCH 3/3] btrace: update tail call heuristic

[Markus Metzger]

An unconditional jump to the start of a function typically indicates a tail
call.

If we can't determine the start of the function at the destination address, we
used to treat it as a tail call, as well.  This results in lots of tail calls
for code for which we don't have symbol information.

Restrict the heuristic to only consider jumps as tail calls that switch
functions in the case where we can't determine the start of a function.  This
effecticely disables tail call detection for code without symbol information.

2016-01-19  Markus Metzger  <markus.t.metzger@intel.com>

gdb/
	* btrace.c (ftrace_update_function): Update tail call heuristic.
---
 gdb/btrace.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/gdb/btrace.c b/gdb/btrace.c
index a683bfa..b584386 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -528,10 +528,17 @@ ftrace_update_function (struct btrace_function *bfun, CORE_ADDR pc)
 
 	    start = get_pc_function_start (pc);
 
+	    /* A jump to the start of a function is (typically) a tail call.  */
+	    if (start == pc)
+	      return ftrace_new_tailcall (bfun, mfun, fun);
+
 	    /* If we can't determine the function for PC, we treat a jump at
-	       the end of the block as tail call.  */
-	    if (start == 0 || start == pc)
+	       the end of the block as tail call if we're switching functions
+	       and as an intra-function branch if we don't.  */
+	    if (start == 0 && ftrace_function_switched (bfun, mfun, fun))
 	      return ftrace_new_tailcall (bfun, mfun, fun);
+
+	    break;
 	  }
 	}
     }
-- 
1.8.3.1

[PATCH 2/3] btrace: allow leading trace gaps

[Markus Metzger]

GDB ignores trace gaps from decode errors or overflows at the beginning of the
trace.  There isn't really a gap in the trace; the trace just starts a bit
later than expected.

In cases where there is no trace at all or where the trace is smaller than
expected, this may hide the reason for the missing trace.

Allow leading trace gaps.  They will be shown as decode warnings and by the
record function-call-history command.

    (gdb) info record
    Active record target: record-btrace
    Recording format: Intel Processor Trace.
    Buffer size: 16kB.
    warning: Decode error (-6) at instruction 0 (offset = 0x58, pc = 0x0): unexpected packet context.
    warning: Decode error (-6) at instruction 0 (offset = 0xb0, pc = 0x0): unexpected packet context.
    warning: Decode error (-6) at instruction 0 (offset = 0x168, pc = 0x0): unexpected packet context.
    warning: Decode error (-6) at instruction 54205 (offset = 0xe08, pc = 0x0): unexpected packet context.
    warning: Decode error (-6) at instruction 54205 (offset = 0xe60, pc = 0x0): unexpected packet context.
    warning: Decode error (-6) at instruction 54205 (offset = 0xed8, pc = 0x0): unexpected packet context.
    Recorded 91582 instructions in 1111 functions (6 gaps) for thread 1 (process 15710).
    (gdb) record function-call-history /c 1
    1       [decode error (-6): unexpected packet context]
    2       [decode error (-6): unexpected packet context]
    3       [decode error (-6): unexpected packet context]
    4           _dl_addr
    5             ??
    6           _dl_addr
    7         ??
    8           ??
    9         ??
    10      ??

Leading trace gaps will not be shown by the record instruction-history command
without further changes.

2016-01-19  Markus Metzger  <markus.t.metzger@intel.com>

gdb/
	* btrace.c (btrace_compute_ftrace_bts, ftrace_add_pt): Allow leading gaps.
	* record-btrace.c (record_btrace_single_step_forward)
	(record_btrace_single_step_backward): Jump back to last instruction if
	step ends at a gap.
	(record_btrace_goto_begin): Skip gaps.
---
 gdb/btrace.c        | 50 +++++++++++++++++++++++++-------------------------
 gdb/record-btrace.c | 33 +++++++++++++++++++++++++++------
 2 files changed, 52 insertions(+), 31 deletions(-)

diff --git a/gdb/btrace.c b/gdb/btrace.c
index af8f16a..a683bfa 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -625,17 +625,17 @@ btrace_compute_ftrace_bts (struct thread_info *tp,
 	  /* We should hit the end of the block.  Warn if we went too far.  */
 	  if (block->end < pc)
 	    {
-	      /* Indicate the gap in the trace - unless we're at the
-		 beginning.  */
-	      if (begin != NULL)
-		{
-		  end = ftrace_new_gap (end, BDE_BTS_OVERFLOW);
-		  ngaps += 1;
+	      /* Indicate the gap in the trace.  */
+	      end = ftrace_new_gap (end, BDE_BTS_OVERFLOW);
+	      if (begin == NULL)
+		begin = end;
+
+	      ngaps += 1;
+
+	      warning (_("Recorded trace may be corrupted at instruction "
+			 "%u (pc = %s)."), end->insn_offset - 1,
+		       core_addr_to_string_nz (pc));
 
-		  warning (_("Recorded trace may be corrupted at instruction "
-			     "%u (pc = %s)."), end->insn_offset - 1,
-			   core_addr_to_string_nz (pc));
-		}
 	      break;
 	    }
 
@@ -795,19 +795,22 @@ ftrace_add_pt (struct pt_insn_decoder *decoder,
 			     "= 0x%" PRIx64 ", pc = 0x%" PRIx64 ")."),
 			   end->insn_offset - 1, offset, insn.ip);
 		}
+	    }
 
-	      /* Indicate trace overflows.  */
-	      if (insn.resynced)
-		{
-		  *pend = end = ftrace_new_gap (end, BDE_PT_OVERFLOW);
-		  *ngaps += 1;
+	  /* Indicate trace overflows.  */
+	  if (insn.resynced)
+	    {
+	      *pend = end = ftrace_new_gap (end, BDE_PT_OVERFLOW);
+	      if (begin == NULL)
+		*pbegin = begin = end;
 
-		  pt_insn_get_offset (decoder, &offset);
+	      *ngaps += 1;
 
-		  warning (_("Overflow at instruction %u (offset = 0x%" PRIx64
-			     ", pc = 0x%" PRIx64 ")."), end->insn_offset - 1,
-			   offset, insn.ip);
-		}
+	      pt_insn_get_offset (decoder, &offset);
+
+	      warning (_("Overflow at instruction %u (offset = 0x%" PRIx64
+			 ", pc = 0x%" PRIx64 ")."), end->insn_offset - 1,
+		       offset, insn.ip);
 	    }
 
 	  upd = ftrace_update_function (end, insn.ip);
@@ -833,13 +836,10 @@ ftrace_add_pt (struct pt_insn_decoder *decoder,
       if (errcode == -pte_eos)
 	break;
 
-      /* If the gap is at the very beginning, we ignore it - we will have
-	 less trace, but we won't have any holes in the trace.  */
-      if (begin == NULL)
-	continue;
-
       /* Indicate the gap in the trace.  */
       *pend = end = ftrace_new_gap (end, errcode);
+      if (begin == NULL)
+	*pbegin = begin = end;
       *ngaps += 1;
 
       pt_insn_get_offset (decoder, &offset);
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 77b5180..898379d 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -2260,7 +2260,7 @@ record_btrace_replay_at_breakpoint (struct thread_info *tp)
 static struct target_waitstatus
 record_btrace_single_step_forward (struct thread_info *tp)
 {
-  struct btrace_insn_iterator *replay, end;
+  struct btrace_insn_iterator *replay, end, start;
   struct btrace_thread_info *btinfo;
 
   btinfo = &tp->btrace;
@@ -2274,7 +2274,9 @@ record_btrace_single_step_forward (struct thread_info *tp)
   if (record_btrace_replay_at_breakpoint (tp))
     return btrace_step_stopped ();
 
-  /* Skip gaps during replay.  */
+  /* Skip gaps during replay.  If we end up at a gap (at the end of the trace),
+     jump back to the instruction at which we started.  */
+  start = *replay;
   do
     {
       unsigned int steps;
@@ -2283,7 +2285,10 @@ record_btrace_single_step_forward (struct thread_info *tp)
 	 of the execution history.  */
       steps = btrace_insn_next (replay, 1);
       if (steps == 0)
-	return btrace_step_no_history ();
+	{
+	  *replay = start;
+	  return btrace_step_no_history ();
+	}
     }
   while (btrace_insn_get (replay) == NULL);
 
@@ -2304,7 +2309,7 @@ record_btrace_single_step_forward (struct thread_info *tp)
 static struct target_waitstatus
 record_btrace_single_step_backward (struct thread_info *tp)
 {
-  struct btrace_insn_iterator *replay;
+  struct btrace_insn_iterator *replay, start;
   struct btrace_thread_info *btinfo;
 
   btinfo = &tp->btrace;
@@ -2315,14 +2320,19 @@ record_btrace_single_step_backward (struct thread_info *tp)
     replay = record_btrace_start_replaying (tp);
 
   /* If we can't step any further, we reached the end of the history.
-     Skip gaps during replay.  */
+     Skip gaps during replay.  If we end up at a gap (at the beginning of
+     the trace), jump back to the instruction at which we started.  */
+  start = *replay;
   do
     {
       unsigned int steps;
 
       steps = btrace_insn_prev (replay, 1);
       if (steps == 0)
-	return btrace_step_no_history ();
+	{
+	  *replay = start;
+	  return btrace_step_no_history ();
+	}
     }
   while (btrace_insn_get (replay) == NULL);
 
@@ -2732,6 +2742,17 @@ record_btrace_goto_begin (struct target_ops *self)
   tp = require_btrace_thread ();
 
   btrace_insn_begin (&begin, &tp->btrace);
+
+  /* Skip gaps at the beginning of the trace.  */
+  while (btrace_insn_get (&begin) == NULL)
+    {
+      unsigned int steps;
+
+      steps = btrace_insn_next (&begin, 1);
+      if (steps == 0)
+	error (_("No trace."));
+    }
+
   record_btrace_set_replay (tp, &begin);
 }
 
-- 
1.8.3.1

[PATCH 1/3] btrace: fix gap indication

[Markus Metzger]

Trace gaps due to overflows or non-contiguous trace are ignored in the 'info
record' command.  Fix that.

Also add a warning when decoding the trace and print the instruction number
preceding the trace gap in that warning message.  It looks like this:

    (gdb) info record
    Active record target: record-btrace
    Recording format: Intel Processor Trace.
    Buffer size: 16kB.
    warning: Decode error (-13) at instruction 101044 (offset = 0x29f0, pc = 0x7ffff728a642): no memory mapped at this address.
    Recorded 101044 instructions in 2093 functions (1 gaps) for thread 1 (process 5360).
    (gdb) record instruction-history 101044
    101044     0x00007ffff728a640:  pop    %r13
    [decode error (-13): no memory mapped at this address]

Remove the dead code that was supposed to print a gaps warning at the end of
trace decode.  This isn't really needed since we now print a warning for each
gap.

2016-01-19  Markus Metzger  <markus.t.metzger@intel.com>

gdb/
	* btrace.c (ftrace_add_pt): Fix gap indication.  Add warning for non-
	contiguous trace and overflow.  Rephrase trace decode warning and print
	instruction number.  Remove dead gaps warning.
	(btrace_compute_ftrace_bts): Rephrase warnings and print instruction
	number.
---
 gdb/btrace.c | 54 +++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 35 insertions(+), 19 deletions(-)

diff --git a/gdb/btrace.c b/gdb/btrace.c
index 7c4da09..af8f16a 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -629,11 +629,12 @@ btrace_compute_ftrace_bts (struct thread_info *tp,
 		 beginning.  */
 	      if (begin != NULL)
 		{
-		  warning (_("Recorded trace may be corrupted around %s."),
-			   core_addr_to_string_nz (pc));
-
 		  end = ftrace_new_gap (end, BDE_BTS_OVERFLOW);
 		  ngaps += 1;
+
+		  warning (_("Recorded trace may be corrupted at instruction "
+			     "%u (pc = %s)."), end->insn_offset - 1,
+			   core_addr_to_string_nz (pc));
 		}
 	      break;
 	    }
@@ -671,14 +672,15 @@ btrace_compute_ftrace_bts (struct thread_info *tp,
 	  /* We can't continue if we fail to compute the size.  */
 	  if (size <= 0)
 	    {
-	      warning (_("Recorded trace may be incomplete around %s."),
-		       core_addr_to_string_nz (pc));
-
 	      /* Indicate the gap in the trace.  We just added INSN so we're
 		 not at the beginning.  */
 	      end = ftrace_new_gap (end, BDE_BTS_INSN_SIZE);
 	      ngaps += 1;
 
+	      warning (_("Recorded trace may be incomplete at instructionn %u "
+			 "(pc = %s)."), end->insn_offset - 1,
+		       core_addr_to_string_nz (pc));
+
 	      break;
 	    }
 
@@ -749,11 +751,10 @@ ftrace_add_pt (struct pt_insn_decoder *decoder,
 {
   struct btrace_function *begin, *end, *upd;
   uint64_t offset;
-  int errcode, nerrors;
+  int errcode;
 
   begin = *pbegin;
   end = *pend;
-  nerrors = 0;
   for (;;)
     {
       struct btrace_insn btinsn;
@@ -784,11 +785,29 @@ ftrace_add_pt (struct pt_insn_decoder *decoder,
 		 flag.  The ENABLED instruction flag means that we continued
 		 from some other instruction.  Indicate this as a trace gap.  */
 	      if (insn.enabled)
-		*pend = end = ftrace_new_gap (end, BDE_PT_DISABLED);
+		{
+		  *pend = end = ftrace_new_gap (end, BDE_PT_DISABLED);
+		  *ngaps += 1;
+
+		  pt_insn_get_offset (decoder, &offset);
+
+		  warning (_("Non-contiguous trace at instruction %u (offset "
+			     "= 0x%" PRIx64 ", pc = 0x%" PRIx64 ")."),
+			   end->insn_offset - 1, offset, insn.ip);
+		}
 
 	      /* Indicate trace overflows.  */
 	      if (insn.resynced)
-		*pend = end = ftrace_new_gap (end, BDE_PT_OVERFLOW);
+		{
+		  *pend = end = ftrace_new_gap (end, BDE_PT_OVERFLOW);
+		  *ngaps += 1;
+
+		  pt_insn_get_offset (decoder, &offset);
+
+		  warning (_("Overflow at instruction %u (offset = 0x%" PRIx64
+			     ", pc = 0x%" PRIx64 ")."), end->insn_offset - 1,
+			   offset, insn.ip);
+		}
 	    }
 
 	  upd = ftrace_update_function (end, insn.ip);
@@ -819,19 +838,16 @@ ftrace_add_pt (struct pt_insn_decoder *decoder,
       if (begin == NULL)
 	continue;
 
-      pt_insn_get_offset (decoder, &offset);
-
-      warning (_("Failed to decode Intel Processor Trace near trace "
-		 "offset 0x%" PRIx64 " near recorded PC 0x%" PRIx64 ": %s."),
-	       offset, insn.ip, pt_errstr (pt_errcode (errcode)));
-
       /* Indicate the gap in the trace.  */
       *pend = end = ftrace_new_gap (end, errcode);
       *ngaps += 1;
-    }
 
-  if (nerrors > 0)
-    warning (_("The recorded execution trace may have gaps."));
+      pt_insn_get_offset (decoder, &offset);
+
+      warning (_("Decode error (%d) at instruction %u (offset = 0x%" PRIx64
+		 ", pc = 0x%" PRIx64 "): %s."), errcode, end->insn_offset - 1,
+	       offset, insn.ip, pt_errstr (pt_errcode (errcode)));
+    }
 }
 
 /* A callback function to allow the trace decoder to read the inferior's
-- 
1.8.3.1