[patch v3 18/23] record-btrace: extend unwinder

[Markus Metzger <markus dot t dot metzger at intel dot com>]

Extend the always failing unwinder to provide the PC based on the call structure
detected in the branch trace.

There are several open points:

An assertion in get_frame_id at frame.c:340 requires that a frame provides a
stack address.  The record-btrace unwinder can't provide this since the trace
does not contain data.  I incorrectly set stack_addr_p to 1 to avoid the
assertion.

When evaluating arguments for printing the stack back trace, there's an ugly
error displayed: "error reading variable: can't compute CFA for this frame".
The error is correct, we can't compute the CFA since we don't have the stack at
that time, but it is rather annoying at this place and makes the back trace
difficult to read.

Now that we set the PC to a different value and provide a fake unwinder, we have
the potential to affect almost every other command.  How can this be tested
sufficiently?  I added a few tests for the intended functionality, but nothing
so far to ensure that it does not break some other command when used in this
context.

Reviewed-by: Eli Zaretskii  <eliz@gnu.org>
2013-04-24  Markus Metzger  <markus.t.metzger@intel.com>

	* frame.h (enum frame_type) <BTRACE_FRAME>: New.
	* record-btrace.c: Include hashtab.h.
	(btrace_get_bfun_name): New.
	(btrace_call_history): Call btrace_get_bfun_name.
	(struct btrace_frame_cache): New.
	(bfcache): New.
	(bfcache_hash, bfcache_eq, bfcache_new): New.
	(btrace_get_frame_function): New.
	(record_btrace_frame_unwind_stop_reason): Allow unwinding.
	(record_btrace_frame_this_id): Compute own id.
	(record_btrace_frame_prev_register): Provide PC, throw_error
	for all other registers.
	(record_btrace_frame_sniffer): Detect btrace frames.
	(record_btrace_frame_dealloc_cache): New.
	(record_btrace_frame_unwind): Add new functions.
	(_initialize_record_btrace): Allocate cache.
	* btrace.c (btrace_clear): Call reinit_frame_cache.
	* NEWS: Announce it.

testsuite/
	* gdb.btrace/record_goto.exp: Add backtrace test.
	* gdb.btrace/tailcall.exp: Add backtrace test.


---
 gdb/NEWS                                 |    2 +
 gdb/btrace.c                             |    4 +
 gdb/frame.h                              |    4 +-
 gdb/record-btrace.c                      |  259 +++++++++++++++++++++++++++---
 gdb/testsuite/gdb.btrace/record_goto.exp |   13 ++
 gdb/testsuite/gdb.btrace/tailcall.exp    |   17 ++
 6 files changed, 279 insertions(+), 20 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 7780b3f..c2db8b2 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -13,6 +13,8 @@ Nios II ELF 			nios2*-*-elf
 Nios II GNU/Linux		nios2*-*-linux
 
 * The btrace record target supports the 'record goto' command.
+  For locations inside the execution trace, the back trace is computed
+  based on the information stored in the execution trace.
 
 * The command 'record function-call-history' supports a new modifier '/c' to
   indent the function names based on their call stack depth.
diff --git a/gdb/btrace.c b/gdb/btrace.c
index 216659a..8aa0b3c 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -752,6 +752,10 @@ btrace_clear (struct thread_info *tp)
 
   DEBUG ("clear thread %d (%s)", tp->num, target_pid_to_str (tp->ptid));
 
+  /* Make sure btrace frames that may hold a pointer into the branch
+     trace data are destroyed.  */
+  reinit_frame_cache ();
+
   btinfo = &tp->btrace;
 
   it = btinfo->begin;
diff --git a/gdb/frame.h b/gdb/frame.h
index 31b9cb7..db4cc52 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -216,7 +216,9 @@ enum frame_type
   ARCH_FRAME,
   /* Sentinel or registers frame.  This frame obtains register values
      direct from the inferior's registers.  */
-  SENTINEL_FRAME
+  SENTINEL_FRAME,
+  /* A branch tracing frame.  */
+  BTRACE_FRAME
 };
 
 /* For every stopped thread, GDB tracks two frames: current and
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 200b4e4..2bc813a 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -34,6 +34,7 @@
 #include "filenames.h"
 #include "regcache.h"
 #include "frame-unwind.h"
+#include "hashtab.h"
 
 /* The target_ops of record-btrace.  */
 static struct target_ops record_btrace_ops;
@@ -507,6 +508,28 @@ btrace_call_history_src_line (struct ui_out *uiout,
   ui_out_field_int (uiout, "max line", end);
 }
 
+/* Get the name of a branch trace function.  */
+
+static const char *
+btrace_get_bfun_name (const struct btrace_function *bfun)
+{
+  struct minimal_symbol *msym;
+  struct symbol *sym;
+
+  if (bfun == NULL)
+    return "<none>";
+
+  msym = bfun->msym;
+  sym = bfun->sym;
+
+  if (sym != NULL)
+    return SYMBOL_PRINT_NAME (sym);
+  else if (msym != NULL)
+    return SYMBOL_PRINT_NAME (msym);
+  else
+    return "<unknown>";
+}
+
 /* Disassemble a section of the recorded function trace.  */
 
 static void
@@ -524,12 +547,8 @@ btrace_call_history (struct ui_out *uiout,
   for (it = *begin; btrace_call_cmp (&it, end) != 0; btrace_call_next (&it, 1))
     {
       const struct btrace_function *bfun;
-      struct minimal_symbol *msym;
-      struct symbol *sym;
 
       bfun = btrace_call_get (&it);
-      msym = bfun->msym;
-      sym = bfun->sym;
 
       /* Print the function index.  */
       ui_out_field_uint (uiout, "index", bfun->number);
@@ -543,12 +562,7 @@ btrace_call_history (struct ui_out *uiout,
 	    ui_out_text (uiout, "  ");
 	}
 
-      if (sym != NULL)
-	ui_out_field_string (uiout, "function", SYMBOL_PRINT_NAME (sym));
-      else if (msym != NULL)
-	ui_out_field_string (uiout, "function", SYMBOL_PRINT_NAME (msym));
-      else
-	ui_out_field_string (uiout, "function", "<unknown>");
+      ui_out_field_string (uiout, "function", btrace_get_bfun_name (bfun));
 
       if ((flags & RECORD_PRINT_INSN_RANGE) != 0)
 	{
@@ -905,13 +919,100 @@ record_btrace_prepare_to_store (struct target_ops *ops,
       }
 }
 
+/* The branch trace frame cache.  */
+
+struct btrace_frame_cache
+{
+  /* The thread.  */
+  struct thread_info *tp;
+
+  /* The frame info.  */
+  struct frame_info *frame;
+
+  /* The branch trace function segment.  */
+  const struct btrace_function *bfun;
+
+  /* The return PC into this frame.  */
+  CORE_ADDR pc;
+};
+
+/* A struct btrace_frame_cache hash table indexed by NEXT.  */
+
+static htab_t bfcache;
+
+/* hash_f for htab_create_alloc of bfcache.  */
+
+static hashval_t
+bfcache_hash (const void *arg)
+{
+  const struct btrace_frame_cache *cache = arg;
+
+  return htab_hash_pointer (cache->frame);
+}
+
+/* eq_f for htab_create_alloc of bfcache.  */
+
+static int
+bfcache_eq (const void *arg1, const void *arg2)
+{
+  const struct btrace_frame_cache *cache1 = arg1;
+  const struct btrace_frame_cache *cache2 = arg2;
+
+  return cache1->frame == cache2->frame;
+}
+
+/* Create a new btrace frame cache.  */
+
+static struct btrace_frame_cache *
+bfcache_new (struct frame_info *frame)
+{
+  struct btrace_frame_cache *cache;
+  void **slot;
+
+  cache = FRAME_OBSTACK_ZALLOC (struct btrace_frame_cache);
+  cache->frame = frame;
+
+  slot = htab_find_slot (bfcache, cache, INSERT);
+  gdb_assert (*slot == NULL);
+  *slot = cache;
+
+  return cache;
+}
+
+/* Extract the branch trace function from a branch trace frame.  */
+
+static const struct btrace_function *
+btrace_get_frame_function (struct frame_info *frame)
+{
+  const struct btrace_frame_cache *cache;
+  const struct btrace_function *bfun;
+  struct btrace_frame_cache pattern;
+  void **slot;
+
+  pattern.frame = frame;
+
+  slot = htab_find_slot (bfcache, &pattern, NO_INSERT);
+  if (slot == NULL)
+    return NULL;
+
+  cache = *slot;
+  return cache->bfun;
+}
+
 /* Implement stop_reason method for record_btrace_frame_unwind.  */
 
 static enum unwind_stop_reason
 record_btrace_frame_unwind_stop_reason (struct frame_info *this_frame,
 					void **this_cache)
 {
-  return UNWIND_UNAVAILABLE;
+  const struct btrace_frame_cache *cache;
+
+  cache = *this_cache;
+
+  if (cache->bfun == NULL)
+    return UNWIND_UNAVAILABLE;
+
+  return UNWIND_NO_REASON;
 }
 
 /* Implement this_id method for record_btrace_frame_unwind.  */
@@ -920,7 +1021,21 @@ static void
 record_btrace_frame_this_id (struct frame_info *this_frame, void **this_cache,
 			     struct frame_id *this_id)
 {
-  /* Leave there the outer_frame_id value.  */
+  const struct btrace_frame_cache *cache;
+  CORE_ADDR stack, code, special;
+
+  cache = *this_cache;
+
+  stack = 0;
+  code = get_frame_func (this_frame);
+  special = (CORE_ADDR) cache->bfun;
+
+  *this_id = frame_id_build_special (stack, code, special);
+
+  DEBUG ("[frame] %s id: (!stack, pc=%s, special=%s)",
+	 btrace_get_bfun_name (cache->bfun),
+	 core_addr_to_string_nz (this_id->code_addr),
+	 core_addr_to_string_nz (this_id->special_addr));
 }
 
 /* Implement prev_register method for record_btrace_frame_unwind.  */
@@ -930,8 +1045,31 @@ record_btrace_frame_prev_register (struct frame_info *this_frame,
 				   void **this_cache,
 				   int regnum)
 {
-  throw_error (NOT_AVAILABLE_ERROR,
-              _("Registers are not available in btrace record history"));
+  const struct btrace_frame_cache *cache;
+  const struct btrace_function *bfun;
+  struct gdbarch *gdbarch;
+  CORE_ADDR pc;
+  int pcreg;
+
+  gdbarch = get_frame_arch (this_frame);
+  pcreg = gdbarch_pc_regnum (gdbarch);
+  if (pcreg < 0 || regnum != pcreg)
+    throw_error (NOT_AVAILABLE_ERROR,
+		 _("Registers are not available in btrace record history"));
+
+  cache = *this_cache;
+  bfun = cache->bfun;
+  if (bfun == NULL)
+    throw_error (NOT_AVAILABLE_ERROR,
+		 _("Registers are not available in btrace record history"));
+
+  pc = cache->pc;
+
+  DEBUG ("[frame] unwound PC for %s on level %d: %s",
+	 btrace_get_bfun_name (bfun), bfun->level,
+	 core_addr_to_string_nz (pc));
+
+  return frame_unwind_got_address (this_frame, regnum, pc);
 }
 
 /* Implement sniffer method for record_btrace_frame_unwind.  */
@@ -941,9 +1079,14 @@ record_btrace_frame_sniffer (const struct frame_unwind *self,
 			     struct frame_info *this_frame,
 			     void **this_cache)
 {
+  const struct btrace_thread_info *btinfo;
+  const struct btrace_insn_iterator *replay;
+  const struct btrace_insn *insn;
+  const struct btrace_function *bfun, *caller;
+  struct btrace_frame_cache *cache;
   struct thread_info *tp;
-  struct btrace_thread_info *btinfo;
-  struct btrace_insn_iterator *replay;
+  struct frame_info *next;
+  CORE_ADDR pc;
 
   /* This doesn't seem right.  Yet, I don't see how I could get from a frame
      to its thread.  */
@@ -951,7 +1094,81 @@ record_btrace_frame_sniffer (const struct frame_unwind *self,
   if (tp == NULL)
     return 0;
 
-  return btrace_is_replaying (tp);
+  replay = tp->btrace.replay;
+  if (replay == NULL)
+    return 0;
+
+  /* Find the next frame's branch trace function.  */
+  next = get_next_frame (this_frame);
+  if (next == NULL)
+    {
+      /* The sentinel frame below corresponds to our replay position.  */
+      bfun = replay->function;
+    }
+  else
+    {
+      /* This is an outer frame.  It must be the predecessor of another
+	 branch trace frame.  Let's get this frame's branch trace function
+	 so we can compute our own.  */
+      bfun = btrace_get_frame_function (next);
+    }
+
+  /* If we did not find a branch trace function, this is not our frame.  */
+  if (bfun == NULL)
+    return 0;
+
+  /* Go up to the calling function segment.  */
+  caller = bfun->up;
+  pc = 0;
+
+  /* Determine where to find the PC in the upper function segment.  */
+  if (caller != NULL)
+    {
+      if ((bfun->flags & BFUN_UP_LINKS_TO_RET) != 0)
+	{
+	  insn = VEC_index (btrace_insn_s, caller->insn, 0);
+	  pc = insn->pc;
+	}
+      else
+	{
+	  insn = VEC_last (btrace_insn_s, caller->insn);
+	  pc = insn->pc;
+
+	  /* We link directly to the jump instruction in the case of a tail
+	     call, since the next instruction will likely be outside of the
+	     caller function.  */
+	  if ((bfun->flags & BFUN_UP_LINKS_TO_TAILCALL) == 0)
+	    pc += gdb_insn_length (get_frame_arch (this_frame), pc);
+	}
+
+      DEBUG ("[frame] sniffed frame for %s on level %d",
+	     btrace_get_bfun_name (caller), caller->level);
+    }
+  else
+    DEBUG ("[frame] sniffed top btrace frame");
+
+  /* This is our frame.  Initialize the frame cache.  */
+  cache = bfcache_new (this_frame);
+  cache->tp = tp;
+  cache->bfun = caller;
+  cache->pc = pc;
+
+  *this_cache = cache;
+  return 1;
+}
+
+static void
+record_btrace_frame_dealloc_cache (struct frame_info *self, void *this_cache)
+{
+  struct btrace_frame_cache *cache;
+  void **slot;
+
+  cache = this_cache;
+
+  slot = htab_find_slot (bfcache, cache, NO_INSERT);
+  gdb_assert (slot != NULL);
+
+  htab_remove_elt (bfcache, cache);
 }
 
 /* btrace recording does not store previous memory content, neither the stack
@@ -962,12 +1179,13 @@ record_btrace_frame_sniffer (const struct frame_unwind *self,
 
 static const struct frame_unwind record_btrace_frame_unwind =
 {
-  NORMAL_FRAME,
+  BTRACE_FRAME,
   record_btrace_frame_unwind_stop_reason,
   record_btrace_frame_this_id,
   record_btrace_frame_prev_register,
   NULL,
-  record_btrace_frame_sniffer
+  record_btrace_frame_sniffer,
+  record_btrace_frame_dealloc_cache
 };
 
 /* The to_resume method of target record-btrace.  */
@@ -1181,4 +1399,7 @@ _initialize_record_btrace (void)
 
   init_record_btrace_ops ();
   add_target (&record_btrace_ops);
+
+  bfcache = htab_create_alloc (50, bfcache_hash, bfcache_eq, NULL,
+			       xcalloc, xfree);
 }
diff --git a/gdb/testsuite/gdb.btrace/record_goto.exp b/gdb/testsuite/gdb.btrace/record_goto.exp
index a9f9a64..8477a03 100644
--- a/gdb/testsuite/gdb.btrace/record_goto.exp
+++ b/gdb/testsuite/gdb.btrace/record_goto.exp
@@ -75,6 +75,19 @@ gdb_test "record instruction-history" "
 gdb_test "record goto 26" "
 .*fun3 \\(\\) at record_goto.c:35.*" "record_goto - goto 26"
 
+# check the back trace at that location
+gdb_test "backtrace" "
+#0.*fun3.*at record_goto.c:35.*\r
+#1.*fun4.*at record_goto.c:44.*\r
+#2.*main.*at record_goto.c:51.*\r
+Backtrace stopped: not enough registers or memory available to unwind further" "backtrace at 25"
+
+# walk the backtrace
+gdb_test "up" "
+.*fun4.*at record_goto.c:44.*" "up to fun4"
+gdb_test "up" "
+.*main.*at record_goto.c:51.*" "up to main"
+
 # the function call history should start at the new location
 gdb_test "record function-call-history /ci -" "
 8\t    fun3\tinst 19,21\r
diff --git a/gdb/testsuite/gdb.btrace/tailcall.exp b/gdb/testsuite/gdb.btrace/tailcall.exp
index cf9fdf3..ada4b14 100644
--- a/gdb/testsuite/gdb.btrace/tailcall.exp
+++ b/gdb/testsuite/gdb.btrace/tailcall.exp
@@ -47,3 +47,20 @@ gdb_test "record function-call-history /c 1" "
 1\t  foo\r
 2\t    bar\r
 3\tmain" "tailcall - calls indented"
+
+# go into bar
+gdb_test "record goto 3" "
+.*bar \\(\\) at .*x86-tailcall.c:24.*" "go to bar"
+
+# check the backtrace
+gdb_test "backtrace" "
+#0.*bar.*at .*x86-tailcall.c:24.*\r
+#1.*foo.*at .*x86-tailcall.c:29.*\r
+#2.*main.*at .*x86-tailcall.c:37.*\r
+Backtrace stopped: not enough registers or memory available to unwind further" "backtrace in bar"
+
+# walk the backtrace
+gdb_test "up" "
+.*foo \\(\\) at .*x86-tailcall.c:29.*" "up to foo"
+gdb_test "up" "
+.*main \\(\\) at .*x86-tailcall.c:37.*" "up to main"
-- 
1.7.1