public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v6 1/9] btrace: Count gaps as one instruction explicitly.
  2017-02-13 12:38 [PATCH v6 0/9] Python bindings for btrace recordings Tim Wiederhake
  2017-02-13 12:38 ` [PATCH v6 2/9] btrace: Export btrace_decode_error function Tim Wiederhake
@ 2017-02-13 12:38 ` Tim Wiederhake
  2017-02-13 12:38 ` [PATCH v6 5/9] Add method to query current recording method to target_ops Tim Wiederhake
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Tim Wiederhake @ 2017-02-13 12:38 UTC (permalink / raw)
  To: gdb-patches; +Cc: markus.t.metzger, palves, xdje42

This gives all instructions, including gaps, a unique number.  Add a function
to retrieve the error code if a btrace instruction iterator points to an
invalid instruction.

2017-02-13  Tim Wiederhake  <tim.wiederhake@intel.com>

gdb/ChangeLog:

	* btrace.c (ftrace_call_num_insn, btrace_insn_get_error): New function.
	(ftrace_new_function, btrace_insn_number, btrace_insn_cmp,
	btrace_find_insn_by_number): Remove special case for gaps.
	* btrace.h (btrace_insn_get_error): New export.
	(btrace_insn_number, btrace_find_insn_by_number): Adjust comment.
	* record-btrace.c (btrace_insn_history): Print number for gaps.
	(record_btrace_info, record_btrace_goto): Handle gaps.


---
 gdb/btrace.c        | 84 ++++++++++++++++++-----------------------------------
 gdb/btrace.h        |  9 ++++--
 gdb/record-btrace.c | 34 ++++++++--------------
 3 files changed, 46 insertions(+), 81 deletions(-)

diff --git a/gdb/btrace.c b/gdb/btrace.c
index 6d621e4..3704d9a 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -141,6 +141,21 @@ ftrace_debug (const struct btrace_function *bfun, const char *prefix)
 		prefix, fun, file, level, ibegin, iend);
 }
 
+/* Return the number of instructions in a given function call segment.  */
+
+static unsigned int
+ftrace_call_num_insn (const struct btrace_function* bfun)
+{
+  if (bfun == NULL)
+    return 0;
+
+  /* A gap is always counted as one instruction.  */
+  if (bfun->errcode != 0)
+    return 1;
+
+  return VEC_length (btrace_insn_s, bfun->insn);
+}
+
 /* Return non-zero if BFUN does not match MFUN and FUN,
    return zero otherwise.  */
 
@@ -216,8 +231,7 @@ ftrace_new_function (struct btrace_function *prev,
       prev->flow.next = bfun;
 
       bfun->number = prev->number + 1;
-      bfun->insn_offset = (prev->insn_offset
-			   + VEC_length (btrace_insn_s, prev->insn));
+      bfun->insn_offset = prev->insn_offset + ftrace_call_num_insn (prev);
       bfun->level = prev->level;
     }
 
@@ -2176,18 +2190,18 @@ btrace_insn_get (const struct btrace_insn_iterator *it)
 
 /* See btrace.h.  */
 
-unsigned int
-btrace_insn_number (const struct btrace_insn_iterator *it)
+int
+btrace_insn_get_error (const struct btrace_insn_iterator *it)
 {
-  const struct btrace_function *bfun;
-
-  bfun = it->function;
+  return it->function->errcode;
+}
 
-  /* Return zero if the iterator points to a gap in the trace.  */
-  if (bfun->errcode != 0)
-    return 0;
+/* See btrace.h.  */
 
-  return bfun->insn_offset + it->index;
+unsigned int
+btrace_insn_number (const struct btrace_insn_iterator *it)
+{
+  return it->function->insn_offset + it->index;
 }
 
 /* See btrace.h.  */
@@ -2382,37 +2396,6 @@ btrace_insn_cmp (const struct btrace_insn_iterator *lhs,
   lnum = btrace_insn_number (lhs);
   rnum = btrace_insn_number (rhs);
 
-  /* A gap has an instruction number of zero.  Things are getting more
-     complicated if gaps are involved.
-
-     We take the instruction number offset from the iterator's function.
-     This is the number of the first instruction after the gap.
-
-     This is OK as long as both lhs and rhs point to gaps.  If only one of
-     them does, we need to adjust the number based on the other's regular
-     instruction number.  Otherwise, a gap might compare equal to an
-     instruction.  */
-
-  if (lnum == 0 && rnum == 0)
-    {
-      lnum = lhs->function->insn_offset;
-      rnum = rhs->function->insn_offset;
-    }
-  else if (lnum == 0)
-    {
-      lnum = lhs->function->insn_offset;
-
-      if (lnum == rnum)
-	lnum -= 1;
-    }
-  else if (rnum == 0)
-    {
-      rnum = rhs->function->insn_offset;
-
-      if (rnum == lnum)
-	rnum -= 1;
-    }
-
   return (int) (lnum - rnum);
 }
 
@@ -2424,26 +2407,15 @@ btrace_find_insn_by_number (struct btrace_insn_iterator *it,
 			    unsigned int number)
 {
   const struct btrace_function *bfun;
-  unsigned int end, length;
 
   for (bfun = btinfo->end; bfun != NULL; bfun = bfun->flow.prev)
-    {
-      /* Skip gaps. */
-      if (bfun->errcode != 0)
-	continue;
-
-      if (bfun->insn_offset <= number)
-	break;
-    }
+    if (bfun->insn_offset <= number)
+      break;
 
   if (bfun == NULL)
     return 0;
 
-  length = VEC_length (btrace_insn_s, bfun->insn);
-  gdb_assert (length > 0);
-
-  end = bfun->insn_offset + length;
-  if (end <= number)
+  if (bfun->insn_offset + ftrace_call_num_insn (bfun) <= number)
     return 0;
 
   it->function = bfun;
diff --git a/gdb/btrace.h b/gdb/btrace.h
index dcc776a..f9af46c 100644
--- a/gdb/btrace.h
+++ b/gdb/btrace.h
@@ -405,9 +405,12 @@ extern void parse_xml_btrace_conf (struct btrace_config *conf, const char *xml);
 extern const struct btrace_insn *
   btrace_insn_get (const struct btrace_insn_iterator *);
 
+/* Return the error code for a branch trace instruction iterator.  Returns zero
+   if there is no error, i.e. the instruction is valid.  */
+extern int btrace_insn_get_error (const struct btrace_insn_iterator *);
+
 /* Return the instruction number for a branch trace iterator.
-   Returns one past the maximum instruction number for the end iterator.
-   Returns zero if the iterator does not point to a valid instruction.  */
+   Returns one past the maximum instruction number for the end iterator.  */
 extern unsigned int btrace_insn_number (const struct btrace_insn_iterator *);
 
 /* Initialize a branch trace instruction iterator to point to the begin/end of
@@ -433,7 +436,7 @@ extern unsigned int btrace_insn_prev (struct btrace_insn_iterator *,
 extern int btrace_insn_cmp (const struct btrace_insn_iterator *lhs,
 			    const struct btrace_insn_iterator *rhs);
 
-/* Find an instruction in the function branch trace by its number.
+/* Find an instruction or gap in the function branch trace by its number.
    If the instruction is found, initialize the branch trace instruction
    iterator to point to this instruction and return non-zero.
    Return zero otherwise.  */
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index daa0696..1dbf5d8 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -443,28 +443,12 @@ record_btrace_info (struct target_ops *self)
       calls = btrace_call_number (&call);
 
       btrace_insn_end (&insn, btinfo);
-
       insns = btrace_insn_number (&insn);
-      if (insns != 0)
-	{
-	  /* The last instruction does not really belong to the trace.  */
-	  insns -= 1;
-	}
-      else
-	{
-	  unsigned int steps;
 
-	  /* Skip gaps at the end.  */
-	  do
-	    {
-	      steps = btrace_insn_prev (&insn, 1);
-	      if (steps == 0)
-		break;
-
-	      insns = btrace_insn_number (&insn);
-	    }
-	  while (insns == 0);
-	}
+      /* If the last instruction is not a gap, it is the current instruction
+	 that is not actually part of the record.  */
+      if (btrace_insn_get (&insn) != NULL)
+	insns -= 1;
 
       gaps = btinfo->ngaps;
     }
@@ -737,7 +721,11 @@ btrace_insn_history (struct ui_out *uiout,
 	  /* We have trace so we must have a configuration.  */
 	  gdb_assert (conf != NULL);
 
-	  btrace_ui_out_decode_error (uiout, it.function->errcode,
+	  uiout->field_fmt ("insn-number", "%u",
+			    btrace_insn_number (&it));
+	  uiout->text ("\t");
+
+	  btrace_ui_out_decode_error (uiout, btrace_insn_get_error (&it),
 				      conf->format);
 	}
       else
@@ -2828,7 +2816,9 @@ record_btrace_goto (struct target_ops *self, ULONGEST insn)
   tp = require_btrace_thread ();
 
   found = btrace_find_insn_by_number (&it, &tp->btrace, number);
-  if (found == 0)
+
+  /* Check if the instruction could not be found or is a gap.  */
+  if (found == 0 || btrace_insn_get (&it) == NULL)
     error (_("No such instruction."));
 
   record_btrace_set_replay (tp, &it);
-- 
2.7.4

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v6 5/9] Add method to query current recording method to target_ops.
  2017-02-13 12:38 [PATCH v6 0/9] Python bindings for btrace recordings Tim Wiederhake
  2017-02-13 12:38 ` [PATCH v6 2/9] btrace: Export btrace_decode_error function Tim Wiederhake
  2017-02-13 12:38 ` [PATCH v6 1/9] btrace: Count gaps as one instruction explicitly Tim Wiederhake
@ 2017-02-13 12:38 ` Tim Wiederhake
  2017-02-13 12:38 ` [PATCH v6 9/9] Add documentation for new record Python bindings Tim Wiederhake
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Tim Wiederhake @ 2017-02-13 12:38 UTC (permalink / raw)
  To: gdb-patches; +Cc: markus.t.metzger, palves, xdje42

2017-02-13  Tim Wiederhake  <tim.wiederhake@intel.com>

gdb/ChangeLog

	* record-btrace.c (record_btrace_record_method): New function.
	(init_record_btrace_ops): Initialize to_record_method.
	* record-full.c (record_full_record_method): New function.
	(init_record_full_ops, init_record_full_core_ops): Add
	record_full_record_method.
	* record.h (enum record_method): New enum.
	* target-debug.h (target_debug_print_enum_record_method: New define.
	* target-delegates.c: Regenerate.
	* target.c (target_record_method): New function.
	* target.h: Include record.h.
	(struct target_ops) <to_record_method>: New field.
	(target_record_method): New export.


---
 gdb/record-btrace.c    | 18 ++++++++++++++++++
 gdb/record-full.c      | 10 ++++++++++
 gdb/record.h           | 13 +++++++++++++
 gdb/target-debug.h     |  2 ++
 gdb/target-delegates.c | 33 +++++++++++++++++++++++++++++++++
 gdb/target.c           |  8 ++++++++
 gdb/target.h           |  8 ++++++++
 7 files changed, 92 insertions(+)

diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 19b9222..f7683f2 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -1258,6 +1258,23 @@ record_btrace_call_history_from (struct target_ops *self,
   record_btrace_call_history_range (self, begin, end, flags);
 }
 
+/* The to_record_method method of target record-btrace.  */
+
+static enum record_method
+record_btrace_record_method (struct target_ops *self, ptid_t ptid)
+{
+  const struct btrace_config *config;
+  struct thread_info * const tp = find_thread_ptid (ptid);
+
+  if (tp == NULL)
+    error (_("No thread."));
+
+  if (tp->btrace.target == NULL)
+    return RECORD_METHOD_NONE;
+
+  return RECORD_METHOD_BTRACE;
+}
+
 /* The to_record_is_replaying method of target record-btrace.  */
 
 static int
@@ -2833,6 +2850,7 @@ init_record_btrace_ops (void)
   ops->to_call_history = record_btrace_call_history;
   ops->to_call_history_from = record_btrace_call_history_from;
   ops->to_call_history_range = record_btrace_call_history_range;
+  ops->to_record_method = record_btrace_record_method;
   ops->to_record_is_replaying = record_btrace_is_replaying;
   ops->to_record_will_replay = record_btrace_will_replay;
   ops->to_record_stop_replaying = record_btrace_stop_replaying_all;
diff --git a/gdb/record-full.c b/gdb/record-full.c
index fdd613b..bd95acc 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -1803,6 +1803,14 @@ record_full_execution_direction (struct target_ops *self)
   return record_full_execution_dir;
 }
 
+/* The to_record_method method of target record-full.  */
+
+enum record_method
+record_full_record_method (struct target_ops *self, ptid_t ptid)
+{
+  return RECORD_METHOD_FULL;
+}
+
 static void
 record_full_info (struct target_ops *self)
 {
@@ -1992,6 +2000,7 @@ init_record_full_ops (void)
   record_full_ops.to_get_bookmark = record_full_get_bookmark;
   record_full_ops.to_goto_bookmark = record_full_goto_bookmark;
   record_full_ops.to_execution_direction = record_full_execution_direction;
+  record_full_ops.to_record_method = record_full_record_method;
   record_full_ops.to_info_record = record_full_info;
   record_full_ops.to_save_record = record_full_save;
   record_full_ops.to_delete_record = record_full_delete;
@@ -2242,6 +2251,7 @@ init_record_full_core_ops (void)
   record_full_core_ops.to_goto_bookmark = record_full_goto_bookmark;
   record_full_core_ops.to_execution_direction
     = record_full_execution_direction;
+  record_full_core_ops.to_record_method = record_full_record_method;
   record_full_core_ops.to_info_record = record_full_info;
   record_full_core_ops.to_delete_record = record_full_delete;
   record_full_core_ops.to_record_is_replaying = record_full_is_replaying;
diff --git a/gdb/record.h b/gdb/record.h
index cff4506..a54a08f 100644
--- a/gdb/record.h
+++ b/gdb/record.h
@@ -37,6 +37,19 @@ extern struct cmd_list_element *info_record_cmdlist;
 extern const struct frame_unwind record_btrace_frame_unwind;
 extern const struct frame_unwind record_btrace_tailcall_frame_unwind;
 
+/* A list of different recording methods.  */
+enum record_method
+{
+  /* No or unknown record method.  */
+  RECORD_METHOD_NONE,
+
+  /* Record method "full".  */
+  RECORD_METHOD_FULL,
+
+  /* Record method "btrace".  */
+  RECORD_METHOD_BTRACE
+};
+
 /* A list of flags specifying what record target methods should print.  */
 enum record_print_flag
 {
diff --git a/gdb/target-debug.h b/gdb/target-debug.h
index 857cece..6923608 100644
--- a/gdb/target-debug.h
+++ b/gdb/target-debug.h
@@ -148,6 +148,8 @@
   target_debug_do_print (host_address_to_string (X))
 #define target_debug_print_enum_btrace_format(X)	\
   target_debug_do_print (plongest (X))
+#define target_debug_print_enum_record_method(X)	\
+  target_debug_do_print (plongest (X))
 #define target_debug_print_const_struct_btrace_config_p(X)	\
   target_debug_do_print (host_address_to_string (X))
 #define target_debug_print_const_struct_btrace_target_info_p(X)	\
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 73e45dd..470b7e4 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -3579,6 +3579,35 @@ debug_btrace_conf (struct target_ops *self, const struct btrace_target_info *arg
   return result;
 }
 
+static enum record_method
+delegate_record_method (struct target_ops *self, ptid_t arg1)
+{
+  self = self->beneath;
+  return self->to_record_method (self, arg1);
+}
+
+static enum record_method
+tdefault_record_method (struct target_ops *self, ptid_t arg1)
+{
+  return RECORD_METHOD_NONE;
+}
+
+static enum record_method
+debug_record_method (struct target_ops *self, ptid_t arg1)
+{
+  enum record_method result;
+  fprintf_unfiltered (gdb_stdlog, "-> %s->to_record_method (...)\n", debug_target.to_shortname);
+  result = debug_target.to_record_method (&debug_target, arg1);
+  fprintf_unfiltered (gdb_stdlog, "<- %s->to_record_method (", debug_target.to_shortname);
+  target_debug_print_struct_target_ops_p (&debug_target);
+  fputs_unfiltered (", ", gdb_stdlog);
+  target_debug_print_ptid_t (arg1);
+  fputs_unfiltered (") = ", gdb_stdlog);
+  target_debug_print_enum_record_method (result);
+  fputs_unfiltered ("\n", gdb_stdlog);
+  return result;
+}
+
 static void
 delegate_stop_recording (struct target_ops *self)
 {
@@ -4386,6 +4415,8 @@ install_delegators (struct target_ops *ops)
     ops->to_read_btrace = delegate_read_btrace;
   if (ops->to_btrace_conf == NULL)
     ops->to_btrace_conf = delegate_btrace_conf;
+  if (ops->to_record_method == NULL)
+    ops->to_record_method = delegate_record_method;
   if (ops->to_stop_recording == NULL)
     ops->to_stop_recording = delegate_stop_recording;
   if (ops->to_info_record == NULL)
@@ -4565,6 +4596,7 @@ install_dummy_methods (struct target_ops *ops)
   ops->to_teardown_btrace = tdefault_teardown_btrace;
   ops->to_read_btrace = tdefault_read_btrace;
   ops->to_btrace_conf = tdefault_btrace_conf;
+  ops->to_record_method = tdefault_record_method;
   ops->to_stop_recording = tdefault_stop_recording;
   ops->to_info_record = tdefault_info_record;
   ops->to_save_record = tdefault_save_record;
@@ -4723,6 +4755,7 @@ init_debug_target (struct target_ops *ops)
   ops->to_teardown_btrace = debug_teardown_btrace;
   ops->to_read_btrace = debug_read_btrace;
   ops->to_btrace_conf = debug_btrace_conf;
+  ops->to_record_method = debug_record_method;
   ops->to_stop_recording = debug_stop_recording;
   ops->to_info_record = debug_info_record;
   ops->to_save_record = debug_save_record;
diff --git a/gdb/target.c b/gdb/target.c
index 3c409f0..0ff8515 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3789,6 +3789,14 @@ target_delete_record (void)
 
 /* See target.h.  */
 
+enum record_method
+target_record_method (ptid_t ptid)
+{
+  return current_target.to_record_method (&current_target, ptid);
+}
+
+/* See target.h.  */
+
 int
 target_record_is_replaying (ptid_t ptid)
 {
diff --git a/gdb/target.h b/gdb/target.h
index 8df117e..943a0e2 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -72,6 +72,7 @@ struct inferior;
 #include "vec.h"
 #include "gdb_signals.h"
 #include "btrace.h"
+#include "record.h"
 #include "command.h"
 
 #include "break-common.h" /* For enum target_hw_bp_type.  */
@@ -1149,6 +1150,10 @@ struct target_ops
 						   const struct btrace_target_info *)
       TARGET_DEFAULT_RETURN (NULL);
 
+    /* Current recording method.  */
+    enum record_method (*to_record_method) (struct target_ops *, ptid_t ptid)
+      TARGET_DEFAULT_RETURN (RECORD_METHOD_NONE);
+
     /* Stop trace recording.  */
     void (*to_stop_recording) (struct target_ops *)
       TARGET_DEFAULT_IGNORE ();
@@ -2495,6 +2500,9 @@ extern int target_supports_delete_record (void);
 /* See to_delete_record in struct target_ops.  */
 extern void target_delete_record (void);
 
+/* See to_record_method.  */
+extern enum record_method target_record_method (ptid_t ptid);
+
 /* See to_record_is_replaying in struct target_ops.  */
 extern int target_record_is_replaying (ptid_t ptid);
 
-- 
2.7.4

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v6 2/9] btrace: Export btrace_decode_error function.
  2017-02-13 12:38 [PATCH v6 0/9] Python bindings for btrace recordings Tim Wiederhake
@ 2017-02-13 12:38 ` Tim Wiederhake
  2017-02-13 12:38 ` [PATCH v6 1/9] btrace: Count gaps as one instruction explicitly Tim Wiederhake
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Tim Wiederhake @ 2017-02-13 12:38 UTC (permalink / raw)
  To: gdb-patches; +Cc: markus.t.metzger, palves, xdje42

2017-02-13  Tim Wiederhake  <tim.wiederhake@intel.com>

gdb/ChangeLog:

	* record-btrace.c (btrace_ui_out_decode_error): Move most of it ...
	* btrace.c (btrace_decode_error): ... here.  New function.
	* btrace.h (btrace_decode_error): New export.


---
 gdb/btrace.c        | 49 ++++++++++++++++++++++++++++++++++++++++++++
 gdb/btrace.h        |  5 +++++
 gdb/record-btrace.c | 58 +++--------------------------------------------------
 3 files changed, 57 insertions(+), 55 deletions(-)

diff --git a/gdb/btrace.c b/gdb/btrace.c
index 3704d9a..3c32f09 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -1723,6 +1723,55 @@ btrace_maint_clear (struct btrace_thread_info *btinfo)
 
 /* See btrace.h.  */
 
+const char *
+btrace_decode_error (enum btrace_format format, int errcode)
+{
+  switch (format)
+    {
+    case BTRACE_FORMAT_BTS:
+      switch (errcode)
+	{
+	case BDE_BTS_OVERFLOW:
+	  return _("instruction overflow");
+
+	case BDE_BTS_INSN_SIZE:
+	  return _("unknown instruction");
+
+	default:
+	  break;
+	}
+      break;
+
+#if defined (HAVE_LIBIPT)
+    case BTRACE_FORMAT_PT:
+      switch (errcode)
+	{
+	case BDE_PT_USER_QUIT:
+	  return _("trace decode cancelled");
+
+	case BDE_PT_DISABLED:
+	  return _("disabled");
+
+	case BDE_PT_OVERFLOW:
+	  return _("overflow");
+
+	default:
+	  if (errcode < 0)
+	    return pt_errstr (pt_errcode (errcode));
+	  break;
+	}
+      break;
+#endif /* defined (HAVE_LIBIPT)  */
+
+    default:
+      break;
+    }
+
+  return _("unknown");
+}
+
+/* See btrace.h.  */
+
 void
 btrace_fetch (struct thread_info *tp)
 {
diff --git a/gdb/btrace.h b/gdb/btrace.h
index f9af46c..1c0b6b3 100644
--- a/gdb/btrace.h
+++ b/gdb/btrace.h
@@ -384,6 +384,11 @@ extern void btrace_disable (struct thread_info *);
    target_teardown_btrace instead of target_disable_btrace.  */
 extern void btrace_teardown (struct thread_info *);
 
+/* Return a human readable error string for the given ERRCODE in FORMAT.
+   The pointer will never be NULL and must not be freed.  */
+
+extern const char *btrace_decode_error (enum btrace_format format, int errcode);
+
 /* Fetch the branch trace for a single thread.  */
 extern void btrace_fetch (struct thread_info *);
 
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 1dbf5d8..19b9222 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -468,63 +468,11 @@ static void
 btrace_ui_out_decode_error (struct ui_out *uiout, int errcode,
 			    enum btrace_format format)
 {
-  const char *errstr;
-  int is_error;
-
-  errstr = _("unknown");
-  is_error = 1;
-
-  switch (format)
-    {
-    default:
-      break;
-
-    case BTRACE_FORMAT_BTS:
-      switch (errcode)
-	{
-	default:
-	  break;
-
-	case BDE_BTS_OVERFLOW:
-	  errstr = _("instruction overflow");
-	  break;
-
-	case BDE_BTS_INSN_SIZE:
-	  errstr = _("unknown instruction");
-	  break;
-	}
-      break;
-
-#if defined (HAVE_LIBIPT)
-    case BTRACE_FORMAT_PT:
-      switch (errcode)
-	{
-	case BDE_PT_USER_QUIT:
-	  is_error = 0;
-	  errstr = _("trace decode cancelled");
-	  break;
-
-	case BDE_PT_DISABLED:
-	  is_error = 0;
-	  errstr = _("disabled");
-	  break;
-
-	case BDE_PT_OVERFLOW:
-	  is_error = 0;
-	  errstr = _("overflow");
-	  break;
-
-	default:
-	  if (errcode < 0)
-	    errstr = pt_errstr (pt_errcode (errcode));
-	  break;
-	}
-      break;
-#endif /* defined (HAVE_LIBIPT)  */
-    }
+  const char *errstr = btrace_decode_error (format, errcode);
 
   uiout->text (_("["));
-  if (is_error)
+  /* ERRCODE > 0 indicates notifications on BTRACE_FORMAT_PT.  */
+  if (!(format == BTRACE_FORMAT_PT && errcode > 0))
     {
       uiout->text (_("decode error ("));
       uiout->field_int ("errcode", errcode);
-- 
2.7.4

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v6 3/9] btrace: Use binary search to find instruction.
  2017-02-13 12:38 [PATCH v6 0/9] Python bindings for btrace recordings Tim Wiederhake
                   ` (3 preceding siblings ...)
  2017-02-13 12:38 ` [PATCH v6 9/9] Add documentation for new record Python bindings Tim Wiederhake
@ 2017-02-13 12:38 ` Tim Wiederhake
  2017-02-13 12:38 ` [PATCH v6 6/9] python: Create Python bindings for record history Tim Wiederhake
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Tim Wiederhake @ 2017-02-13 12:38 UTC (permalink / raw)
  To: gdb-patches; +Cc: markus.t.metzger, palves, xdje42

Currently, btrace_find_insn_by_number will iterate over all function call
segments to find the one that contains the needed instruction.  This linear
search is too slow for the upcoming Python bindings that will use this
function to access instructions.  This patch introduces a vector in struct
btrace_thread_info that holds pointers to all recorded function segments and
allows to use binary search.

The proper solution is to turn the underlying tree into a vector of objects
and use indices for access.  This requires more work.  A patch set is
currently being worked on and will be published later.

2017-02-13  Tim Wiederhake  <tim.wiederhake@intel.com>

gdb/ChangeLog:
	* btrace.c (btrace_fetch): Copy function call segments pointer
	into a vector.
	(btrace_clear): Clear the vector.
	(btrace_find_insn_by_number): Use binary search to find the correct
	function call segment.
	* btrace.h (brace_fun_p): New typedef.
	(struct btrace_thread_info) <functions>: New field.


---
 gdb/btrace.c | 45 +++++++++++++++++++++++++++++++++++++++------
 gdb/btrace.h |  7 +++++++
 2 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/gdb/btrace.c b/gdb/btrace.c
index 3c32f09..e242a89 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -1837,13 +1837,19 @@ btrace_fetch (struct thread_info *tp)
   /* Compute the trace, provided we have any.  */
   if (!btrace_data_empty (&btrace))
     {
+      struct btrace_function *bfun;
+
       /* Store the raw trace data.  The stored data will be cleared in
 	 btrace_clear, so we always append the new trace.  */
       btrace_data_append (&btinfo->data, &btrace);
       btrace_maint_clear (btinfo);
 
+      VEC_truncate (btrace_fun_p, btinfo->functions, 0);
       btrace_clear_history (btinfo);
       btrace_compute_ftrace (tp, &btrace);
+
+      for (bfun = btinfo->begin; bfun != NULL; bfun = bfun->flow.next)
+	VEC_safe_push (btrace_fun_p, btinfo->functions, bfun);
     }
 
   do_cleanups (cleanup);
@@ -1866,6 +1872,8 @@ btrace_clear (struct thread_info *tp)
 
   btinfo = &tp->btrace;
 
+  VEC_free (btrace_fun_p, btinfo->functions);
+
   it = btinfo->begin;
   while (it != NULL)
     {
@@ -2456,20 +2464,45 @@ btrace_find_insn_by_number (struct btrace_insn_iterator *it,
 			    unsigned int number)
 {
   const struct btrace_function *bfun;
+  unsigned int upper, lower;
 
-  for (bfun = btinfo->end; bfun != NULL; bfun = bfun->flow.prev)
-    if (bfun->insn_offset <= number)
-      break;
+  if (VEC_empty (btrace_fun_p, btinfo->functions))
+      return 0;
 
-  if (bfun == NULL)
+  lower = 0;
+  bfun = VEC_index (btrace_fun_p, btinfo->functions, lower);
+  if (number < bfun->insn_offset)
     return 0;
 
-  if (bfun->insn_offset + ftrace_call_num_insn (bfun) <= number)
+  upper = VEC_length (btrace_fun_p, btinfo->functions) - 1;
+  bfun = VEC_index (btrace_fun_p, btinfo->functions, upper);
+  if (number >= bfun->insn_offset + ftrace_call_num_insn (bfun))
     return 0;
 
+  /* We assume that there are no holes in the numbering.  */
+  for (;;)
+    {
+      const unsigned int average = lower + (upper - lower) / 2;
+
+      bfun = VEC_index (btrace_fun_p, btinfo->functions, average);
+
+      if (number < bfun->insn_offset)
+	{
+	  upper = average - 1;
+	  continue;
+	}
+
+      if (number >= bfun->insn_offset + ftrace_call_num_insn (bfun))
+	{
+	  lower = average + 1;
+	  continue;
+	}
+
+      break;
+    }
+
   it->function = bfun;
   it->index = number - bfun->insn_offset;
-
   return 1;
 }
 
diff --git a/gdb/btrace.h b/gdb/btrace.h
index 1c0b6b3..07ed10c 100644
--- a/gdb/btrace.h
+++ b/gdb/btrace.h
@@ -187,6 +187,9 @@ struct btrace_function
   btrace_function_flags flags;
 };
 
+typedef struct btrace_function *btrace_fun_p;
+DEF_VEC_P (btrace_fun_p);
+
 /* A branch trace instruction iterator.  */
 struct btrace_insn_iterator
 {
@@ -337,6 +340,10 @@ struct btrace_thread_info
   struct btrace_function *begin;
   struct btrace_function *end;
 
+  /* Vector of pointer to decoded function segments.  These are in execution
+     order with the first element == BEGIN and the last element == END.  */
+  VEC (btrace_fun_p) *functions;
+
   /* The function level offset.  When added to each function's LEVEL,
      this normalizes the function levels such that the smallest level
      becomes zero.  */
-- 
2.7.4

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v6 6/9] python: Create Python bindings for record history.
  2017-02-13 12:38 [PATCH v6 0/9] Python bindings for btrace recordings Tim Wiederhake
                   ` (4 preceding siblings ...)
  2017-02-13 12:38 ` [PATCH v6 3/9] btrace: Use binary search to find instruction Tim Wiederhake
@ 2017-02-13 12:38 ` Tim Wiederhake
  2017-02-13 12:38 ` [PATCH v6 4/9] Add record_start and record_stop functions Tim Wiederhake
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Tim Wiederhake @ 2017-02-13 12:38 UTC (permalink / raw)
  To: gdb-patches; +Cc: markus.t.metzger, palves, xdje42

This patch adds three new functions to the gdb module in Python:
	- start_recording
	- stop_recording
	- current_recording
start_recording and current_recording return an object of the new type
gdb.Record, which can be used to access the recorded data.

2017-02-13  Tim Wiederhake  <tim.wiederhake@intel.com>

gdb/ChangeLog

	* Makefile.in (SUBDIR_PYTHON_OBS): Add python/py-record.o.
	(SUBDIR_PYTHON_SRCS): Add python/py-record.c.
	* python/py-record.c: New file.
	* python/python-internal.h (gdbpy_start_recording,
	gdbpy_current_recording, gdpy_stop_recording,
	gdbpy_initialize_record): New export.
	* python/python.c (_initialize_python): Add gdbpy_initialize_record.
	(python_GdbMethods): Add gdbpy_start_recording,
	gdbpy_current_recording and gdbpy_stop_recording.


---
 gdb/Makefile.in              |   2 +
 gdb/python/py-record.c       | 227 +++++++++++++++++++++++++++++++++++++++++++
 gdb/python/python-internal.h |   5 +
 gdb/python/python.c          |  13 +++
 4 files changed, 247 insertions(+)
 create mode 100644 gdb/python/py-record.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index e0fe442..b4419f0 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -460,6 +460,7 @@ SUBDIR_PYTHON_OBS = \
 	py-param.o \
 	py-prettyprint.o \
 	py-progspace.o \
+	py-record.o \
 	py-signalevent.o \
 	py-stopevent.o \
 	py-symbol.o \
@@ -500,6 +501,7 @@ SUBDIR_PYTHON_SRCS = \
 	python/py-param.c \
 	python/py-prettyprint.c \
 	python/py-progspace.c \
+	python/py-record.c \
 	python/py-signalevent.c \
 	python/py-stopevent.c \
 	python/py-symbol.c \
diff --git a/gdb/python/py-record.c b/gdb/python/py-record.c
new file mode 100644
index 0000000..27ecbb6
--- /dev/null
+++ b/gdb/python/py-record.c
@@ -0,0 +1,227 @@
+/* Python interface to record targets.
+
+   Copyright 2016-2017 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include "inferior.h"
+#include "record.h"
+#include "python-internal.h"
+#include "target.h"
+
+/* Python Record object.  */
+
+typedef struct
+{
+  PyObject_HEAD
+
+  /* The ptid this object refers to.  */
+  ptid_t ptid;
+
+  /* The current recording method.  */
+  enum record_method method;
+} recpy_record_object;
+
+/* Python Record type.  */
+
+static PyTypeObject recpy_record_type = {
+  PyVarObject_HEAD_INIT (NULL, 0)
+};
+
+/* Implementation of record.ptid.  */
+
+static PyObject *
+recpy_ptid (PyObject *self, void* closure)
+{
+  const recpy_record_object * const obj = (recpy_record_object *) self;
+
+  return gdbpy_create_ptid_object (obj->ptid);
+}
+
+/* Implementation of record.method.  */
+
+static PyObject *
+recpy_method (PyObject *self, void* closure)
+{
+  return PyErr_Format (PyExc_NotImplementedError, _("Not implemented."));
+}
+
+/* Implementation of record.format.  */
+
+static PyObject *
+recpy_format (PyObject *self, void* closure)
+{
+  return PyErr_Format (PyExc_NotImplementedError, _("Not implemented."));
+}
+
+/* Implementation of record.goto (instruction) -> None.  */
+
+static PyObject *
+recpy_goto (PyObject *self, PyObject *value)
+{
+  return PyErr_Format (PyExc_NotImplementedError, _("Not implemented."));
+}
+
+/* Implementation of record.replay_position [instruction]  */
+
+static PyObject *
+recpy_replay_position (PyObject *self, void *closure)
+{
+  return PyErr_Format (PyExc_NotImplementedError, _("Not implemented."));
+}
+
+/* Implementation of record.instruction_history [list].  */
+
+static PyObject *
+recpy_instruction_history (PyObject *self, void* closure)
+{
+  return PyErr_Format (PyExc_NotImplementedError, _("Not implemented."));
+}
+
+/* Implementation of record.function_call_history [list].  */
+
+static PyObject *
+recpy_function_call_history (PyObject *self, void* closure)
+{
+  return PyErr_Format (PyExc_NotImplementedError, _("Not implemented."));
+}
+
+/* Implementation of record.begin [instruction].  */
+
+static PyObject *
+recpy_begin (PyObject *self, void* closure)
+{
+  return PyErr_Format (PyExc_NotImplementedError, _("Not implemented."));
+}
+
+/* Implementation of record.end [instruction].  */
+
+static PyObject *
+recpy_end (PyObject *self, void* closure)
+{
+  return PyErr_Format (PyExc_NotImplementedError, _("Not implemented."));
+}
+
+/* Record method list.  */
+
+static PyMethodDef recpy_record_methods[] = {
+  { "goto", recpy_goto, METH_VARARGS,
+    "goto (instruction|function_call) -> None.\n\
+Rewind to given location."},
+  { NULL }
+};
+
+/* Record member list.  */
+
+static PyGetSetDef recpy_record_getset[] = {
+  { "ptid", recpy_ptid, NULL, "Current thread.", NULL },
+  { "method", recpy_method, NULL, "Current recording method.", NULL },
+  { "format", recpy_format, NULL, "Current recording format.", NULL },
+  { "replay_position", recpy_replay_position, NULL, "Current replay position.",
+    NULL },
+  { "instruction_history", recpy_instruction_history, NULL,
+    "List of instructions in current recording.", NULL },
+  { "function_call_history", recpy_function_call_history, NULL,
+    "List of function calls in current recording.", NULL },
+  { "begin", recpy_begin, NULL,
+    "First instruction in current recording.", NULL },
+  { "end", recpy_end, NULL,
+    "One past the last instruction in current recording.  This is typically \
+the current instruction and is used for e.g. record.goto (record.end).", NULL },
+  { NULL }
+};
+
+/* Sets up the record API in the gdb module.  */
+
+int
+gdbpy_initialize_record (void)
+{
+  recpy_record_type.tp_new = PyType_GenericNew;
+  recpy_record_type.tp_flags = Py_TPFLAGS_DEFAULT;
+  recpy_record_type.tp_basicsize = sizeof (recpy_record_object);
+  recpy_record_type.tp_name = "gdb.Record";
+  recpy_record_type.tp_doc = "GDB record object";
+  recpy_record_type.tp_methods = recpy_record_methods;
+  recpy_record_type.tp_getset = recpy_record_getset;
+
+  return PyType_Ready (&recpy_record_type);
+}
+
+/* Implementation of gdb.start_recording (method) -> gdb.Record.  */
+
+PyObject *
+gdbpy_start_recording (PyObject *self, PyObject *args)
+{
+  const char *method = NULL;
+  const char *format = NULL;
+  PyObject *ret = NULL;
+
+  if (!PyArg_ParseTuple (args, "|ss", &method, &format))
+    return NULL;
+
+  TRY
+    {
+      record_start (method, format, 0);
+      ret = gdbpy_current_recording (self, args);
+    }
+  CATCH (except, RETURN_MASK_ALL)
+    {
+      gdbpy_convert_exception (except);
+    }
+  END_CATCH
+
+  return ret;
+}
+
+/* Implementation of gdb.current_recording (self) -> gdb.Record.  */
+
+PyObject *
+gdbpy_current_recording (PyObject *self, PyObject *args)
+{
+  recpy_record_object *ret = NULL;
+
+  if (find_record_target () == NULL)
+    Py_RETURN_NONE;
+
+  ret = PyObject_New (recpy_record_object, &recpy_record_type);
+  ret->ptid = inferior_ptid;
+  ret->method = target_record_method (inferior_ptid);
+
+  return (PyObject *) ret;
+}
+
+/* Implementation of gdb.stop_recording (self) -> None.  */
+
+PyObject *
+gdbpy_stop_recording (PyObject *self, PyObject *args)
+{
+  PyObject *ret = NULL;
+
+  TRY
+    {
+      record_stop (0);
+      ret = Py_None;
+      Py_INCREF (Py_None);
+    }
+  CATCH (except, RETURN_MASK_ALL)
+    {
+      gdbpy_convert_exception (except);
+    }
+  END_CATCH
+
+  return ret;
+}
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index e2ebc1b..c6af9f7 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -369,6 +369,9 @@ PyObject *gdbpy_frame_stop_reason_string (PyObject *, PyObject *);
 PyObject *gdbpy_lookup_symbol (PyObject *self, PyObject *args, PyObject *kw);
 PyObject *gdbpy_lookup_global_symbol (PyObject *self, PyObject *args,
 				      PyObject *kw);
+PyObject *gdbpy_start_recording (PyObject *self, PyObject *args);
+PyObject *gdbpy_current_recording (PyObject *self, PyObject *args);
+PyObject *gdbpy_stop_recording (PyObject *self, PyObject *args);
 PyObject *gdbpy_newest_frame (PyObject *self, PyObject *args);
 PyObject *gdbpy_selected_frame (PyObject *self, PyObject *args);
 PyObject *gdbpy_block_for_pc (PyObject *self, PyObject *args);
@@ -436,6 +439,8 @@ int gdbpy_initialize_values (void)
   CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
 int gdbpy_initialize_frames (void)
   CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
+int gdbpy_initialize_record (void)
+  CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
 int gdbpy_initialize_symtabs (void)
   CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
 int gdbpy_initialize_commands (void)
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 1f5ab42..f8b9f7e 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1628,6 +1628,7 @@ do_start_initialization ()
       || gdbpy_initialize_values () < 0
       || gdbpy_initialize_frames () < 0
       || gdbpy_initialize_commands () < 0
+      || gdbpy_initialize_record () < 0
       || gdbpy_initialize_symbols () < 0
       || gdbpy_initialize_symtabs () < 0
       || gdbpy_initialize_blocks () < 0
@@ -1910,6 +1911,18 @@ Return the selected frame object." },
     "stop_reason_string (Integer) -> String.\n\
 Return a string explaining unwind stop reason." },
 
+  { "start_recording", gdbpy_start_recording, METH_VARARGS,
+    "start_recording ([method] [, format]) -> gdb.Record.\n\
+Start recording with the given method.  If no method is given, will fall back\n\
+to the system default method.  If no format is given, will fall back to the\n\
+default format for the given method."},
+  { "current_recording", gdbpy_current_recording, METH_NOARGS,
+    "current_recording () -> gdb.Record.\n\
+Return current recording object." },
+  { "stop_recording", gdbpy_stop_recording, METH_NOARGS,
+    "stop_recording () -> None.\n\
+Stop current recording." },
+
   { "lookup_type", (PyCFunction) gdbpy_lookup_type,
     METH_VARARGS | METH_KEYWORDS,
     "lookup_type (name [, block]) -> type\n\
-- 
2.7.4

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v6 4/9] Add record_start and record_stop functions.
  2017-02-13 12:38 [PATCH v6 0/9] Python bindings for btrace recordings Tim Wiederhake
                   ` (5 preceding siblings ...)
  2017-02-13 12:38 ` [PATCH v6 6/9] python: Create Python bindings for record history Tim Wiederhake
@ 2017-02-13 12:38 ` Tim Wiederhake
       [not found] ` <1486989450-11313-8-git-send-email-tim.wiederhake@intel.com>
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Tim Wiederhake @ 2017-02-13 12:38 UTC (permalink / raw)
  To: gdb-patches; +Cc: markus.t.metzger, palves, xdje42

2017-02-13  Tim Wiederhake  <tim.wiederhake@intel.com>

gdb/ChangeLog

	* record.h (record_start, record_stop): New export.
	* record.c (record_start, record_stop): New function.


---
 gdb/record.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 gdb/record.h |  8 ++++++++
 2 files changed, 50 insertions(+)

diff --git a/gdb/record.c b/gdb/record.c
index 15271b2..eff8fe1 100644
--- a/gdb/record.c
+++ b/gdb/record.c
@@ -93,6 +93,48 @@ record_preopen (void)
 
 /* See record.h.  */
 
+void
+record_start (const char *method, const char *format, int from_tty)
+{
+  if (method == NULL)
+    {
+      if (format == NULL)
+	execute_command_to_string ("record", from_tty);
+      else
+	error (_("Invalid format."));
+    }
+  else if (strcmp (method, "full") == 0)
+    {
+      if (format == NULL)
+	execute_command_to_string ("record full", from_tty);
+      else
+	error (_("Invalid format."));
+    }
+  else if (strcmp (method, "btrace") == 0)
+    {
+      if (format == NULL)
+	execute_command_to_string ("record btrace", from_tty);
+      else if (strcmp (format, "bts") == 0)
+	execute_command_to_string ("record btrace bts", from_tty);
+      else if (strcmp (format, "pt") == 0)
+	execute_command_to_string ("record btrace pt", from_tty);
+      else
+	error (_("Invalid format."));
+    }
+  else
+    error (_("Invalid method."));
+}
+
+/* See record.h.  */
+
+void
+record_stop (int from_tty)
+{
+  execute_command_to_string ("record stop", from_tty);
+}
+
+/* See record.h.  */
+
 int
 record_read_memory (struct gdbarch *gdbarch,
 		    CORE_ADDR memaddr, gdb_byte *myaddr,
diff --git a/gdb/record.h b/gdb/record.h
index e1d4aae..cff4506 100644
--- a/gdb/record.h
+++ b/gdb/record.h
@@ -91,4 +91,12 @@ extern struct target_ops *find_record_target (void);
    it does anything.  */
 extern void record_preopen (void);
 
+/* Start recording with the given METHOD and FORMAT.  NULL means default method
+   or format.  Throw on failure or invalid method / format.  */
+extern void record_start (const char *method, const char *format,
+			  int from_tty);
+
+/* Stop recording.  Throw on failure.  */
+extern void record_stop (int from_tty);
+
 #endif /* _RECORD_H_ */
-- 
2.7.4

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v6 9/9] Add documentation for new record Python bindings.
  2017-02-13 12:38 [PATCH v6 0/9] Python bindings for btrace recordings Tim Wiederhake
                   ` (2 preceding siblings ...)
  2017-02-13 12:38 ` [PATCH v6 5/9] Add method to query current recording method to target_ops Tim Wiederhake
@ 2017-02-13 12:38 ` Tim Wiederhake
  2017-02-13 14:48   ` Eli Zaretskii
  2017-03-03 11:10   ` Yao Qi
  2017-02-13 12:38 ` [PATCH v6 3/9] btrace: Use binary search to find instruction Tim Wiederhake
                   ` (5 subsequent siblings)
  9 siblings, 2 replies; 21+ messages in thread
From: Tim Wiederhake @ 2017-02-13 12:38 UTC (permalink / raw)
  To: gdb-patches; +Cc: markus.t.metzger, palves, xdje42

2017-02-13  Tim Wiederhake  <tim.wiederhake@intel.com>

gdb/ChangeLog:

	* NEWS: Add record Python bindings entry.

gdb/doc/ChangeLog:

	* python.texi (Recordings In Python): New section.


---
 gdb/NEWS            |   4 +
 gdb/doc/python.texi | 245 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 249 insertions(+)

diff --git a/gdb/NEWS b/gdb/NEWS
index 08f97c0..f412887 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,10 @@
 
 *** Changes since GDB 7.12
 
+* Python Scripting
+
+  ** New functions to start, stop and access a running btrace recording.
+
 * GDB now supports recording and replaying rdrand and rdseed Intel 64
   instructions.
 
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index fae45aa..d1cbadb 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -151,6 +151,7 @@ optional arguments while skipping others.  Example:
 * Inferiors In Python::         Python representation of inferiors (processes)
 * Events In Python::            Listening for events from @value{GDBN}.
 * Threads In Python::           Accessing inferior threads from Python.
+* Recordings In Python::        Accessing recordings from Python.
 * Commands In Python::          Implementing new commands in Python.
 * Parameters In Python::        Adding new @value{GDBN} parameters.
 * Functions In Python::         Writing new convenience functions.
@@ -3062,6 +3063,250 @@ Return a Boolean indicating whether the thread is running.
 Return a Boolean indicating whether the thread is exited.
 @end defun
 
+@node Recordings In Python
+@subsubsection Recordings In Python
+@cindex recordings in python
+
+The following recordings-related functions
+(@pxref{Process Record and Replay}) are available in the @code{gdb}
+module:
+
+@defun gdb.start_recording (@r{[}method@r{]}, @r{[}format@r{]})
+Start a recording using the given @var{method} and @var{format}.  If
+no @var{format} is given, the default format for the recording method
+is used.  If no @var{method} is given, the default method will be used.
+Returns a @code{gdb.Record} object on success.  Throw an exception on
+failure.
+
+The following strings can be passed as @var{method}:
+
+@itemize @bullet
+@item
+@code{"full"}
+@item
+@code{"btrace"}: Possible values for @var{format}: @code{"pt"},
+@code{"bts"} or leave out for default format.
+@end itemize
+@end defun
+
+@defun gdb.current_recording ()
+Access a currently running recording.  Return a @code{gdb.Record}
+object on success.  Return @code{None} if no recording is currently
+active.
+@end defun
+
+@defun gdb.stop_recording ()
+Stop the current recording.  Throw an exception if no recording is
+currently active.  All record objects become invalid after this call.
+@end defun
+
+A @code{gdb.Record} object has the following attributes:
+
+@defvar Record.ptid
+ID of the thread associated with this object as a tuple of three integers.  The
+first is the Process ID (PID); the second is the Lightweight Process ID (LWPID),
+and the third is the Thread ID (TID). Either the LWPID or TID may be 0, which
+indicates that the operating system does not use that identifier.
+@end defvar
+
+@defvar Record.method
+A string with the current recording method, e.g.@: @code{full} or
+@code{btrace}.
+@end defvar
+
+@defvar Record.format
+A string with the current recording format, e.g.@: @code{bt}, @code{pts} or
+@code{None}.
+@end defvar
+
+@defvar Record.begin
+A method specific instruction object representing the first instruction
+in this recording.
+@end defvar
+
+@defvar Record.end
+A method specific instruction object representing the current
+instruction, that is not actually part of the recording.
+@end defvar
+
+@defvar Record.replay_position
+The instruction representing the current replay position.  If there is
+no replay active, this will be @code{None}.
+@end defvar
+
+@defvar Record.instruction_history
+A list with all recorded instructions.
+@end defvar
+
+@defvar Record.function_call_history
+A list with all recorded function call segments.
+@end defvar
+
+A @code{gdb.Record} object has the following methods:
+
+@defun Record.goto (instruction)
+Move the replay position to the given @var{instruction}.
+@end defun
+
+The attributes and methods of instruction objects depend on the current
+recording method.  Currently, only btrace instructions are supported.
+
+A @code{gdb.BtraceInstruction} object has the following attributes:
+
+@defvar BtraceInstruction.number
+An integer identifying this instruction.  @var{number} corresponds to
+the numbers seen in @code{record instruction-history}
+(@pxref{Process Record and Replay}).
+@end defvar
+
+@defvar BtraceInstruction.error
+An integer identifying the error code for gaps in the history.
+@code{None} for regular instructions.
+@end defvar
+
+@defvar BtraceInstruction.sal
+A @code{gdb.Symtab_and_line} object representing the associated symtab
+and line of this instruction.  May be @code{None} if the instruction
+is a gap.
+@end defvar
+
+@defvar BtraceInstruction.pc
+An integer representing this instruction's address.  May be @code{None}
+if the instruction is a gap or the debug symbols could not be read.
+@end defvar
+
+@defvar BtraceInstruction.data
+A buffer with the raw instruction data.  May be @code{None} if the
+instruction is a gap.
+@end defvar
+
+@defvar BtraceInstruction.decoded
+A human readable string with the disassembled instruction.  Contains the
+error message for gaps.
+@end defvar
+
+@defvar BtraceInstruction.size
+The size of the instruction in bytes.  Will be @code{None} if the
+instruction is a gap.
+@end defvar
+
+@defvar BtraceInstruction.is_speculative
+A boolean indicating whether the instruction was executed
+speculatively.  Will be @code{None} for gaps.
+@end defvar
+
+The attributes and methods of function call objects depend on the
+current recording format.  Currently, only btrace function calls are
+supported.
+
+A @code{gdb.BtraceFunctionCall} object has the following attributes:
+
+@defvar BtraceFunctionCall.number
+An integer identifying this function call.  @var{number} corresponds to
+the numbers seen in @code{record function-call-history}
+(@pxref{Process Record and Replay}).
+@end defvar
+
+@defvar BtraceFunctionCall.symbol
+A @code{gdb.Symbol} object representing the associated symbol.  May be
+@code{None} if the function call is a gap or the debug symbols could
+not be read.
+@end defvar
+
+@defvar BtraceFunctionCall.level
+An integer representing the function call's stack level.  May be
+@code{None} if the function call is a gap.
+@end defvar
+
+@defvar BtraceFunctionCall.instructions
+A list of @code{gdb.BtraceInstruction} objects associated with this function
+call.
+@end defvar
+
+@defvar BtraceFunctionCall.up
+A @code{gdb.BtraceFunctionCall} object representing the caller's
+function segment.  If the call has not been recorded, this will be the
+function segment to which control returns.  If neither the call nor the
+return have been recorded, this will be @code{None}.
+@end defvar
+
+@defvar BtraceFunctionCall.prev_sibling
+A @code{gdb.BtraceFunctionCall} object representing the previous
+segment of this function call.  May be @code{None}.
+@end defvar
+
+@defvar BtraceFunctionCall.next_sibling
+A @code{gdb.BtraceFunctionCall} object representing the next segment of
+this function call.  May be @code{None}.
+@end defvar
+
+The following example demonstrates the usage of these objects and
+functions to create a function that will rewind a record to the last
+time a function in a different file was executed.  This would typically
+be used to track the execution of user provided callback functions in a
+library which typically are not visible in a back trace.
+
+@smallexample
+def bringback ():
+    rec = gdb.current_recording ()
+    if not rec:
+        return
+
+    insn = rec.instruction_history
+    if len (insn) == 0:
+        return
+
+    try:
+        position = insn.index (rec.replay_position)
+    except:
+        position = -1
+    try:
+        filename = insn[position].sal.symtab.fullname ()
+    except:
+        filename = None
+
+    for i in reversed (insn[:position]):
+	try:
+            current = i.sal.symtab.fullname ()
+	except:
+            current = None
+
+        if filename == current:
+            continue
+
+        rec.goto (i)
+        return
+@end smallexample
+
+Another possible application is to write a function that counts the
+number of code executions in a given line range.  This line range can
+contain parts of functions or span across several functions and is not
+limited to be contiguous.
+
+@smallexample
+def countrange (filename, linerange):
+    count = 0
+
+    def filter_only (file_name):
+        for call in gdb.current_recording ().function_call_history:
+            try:
+                if file_name in call.symbol.symtab.fullname ():
+                    yield call
+            except:
+                pass
+
+    for c in filter_only (filename):
+        for i in c.instructions:
+            try:
+                if i.sal.line in linerange:
+                    count += 1
+                    break;
+            except:
+                    pass
+
+    return count
+@end smallexample
+
 @node Commands In Python
 @subsubsection Commands In Python
 
-- 
2.7.4

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v6 0/9] Python bindings for btrace recordings
@ 2017-02-13 12:38 Tim Wiederhake
  2017-02-13 12:38 ` [PATCH v6 2/9] btrace: Export btrace_decode_error function Tim Wiederhake
                   ` (9 more replies)
  0 siblings, 10 replies; 21+ messages in thread
From: Tim Wiederhake @ 2017-02-13 12:38 UTC (permalink / raw)
  To: gdb-patches; +Cc: markus.t.metzger, palves, xdje42

Hello everyone!

This patch series adds Python bindings for btrace recordings.

V1 of this series can be found here:
https://sourceware.org/ml/gdb-patches/2016-10/msg00733.html

V2 of this series can be found here:
https://sourceware.org/ml/gdb-patches/2016-11/msg00084.html

V3 of this series can be found here:
https://sourceware.org/ml/gdb-patches/2016-11/msg00605.html

V4 of this series can be found here:
https://sourceware.org/ml/gdb-patches/2017-01/msg00044.html

V5 of this series can be found here:
https://sourceware.org/ml/gdb-patches/2017-01/msg00616.html

Changes from V5:
 - Skip gdb.python/py-record-full.exp if recording is not supported.
 - s/symbol/SAL/ in comment in py-record-btrace.c.
 - Replaced mem_fileopen by string_file in py-record-btrace.c.
 - Simplified btpy_list_count and added comment in py-record-btrace.c.
 - Removed unused variable in recpy_bt_goto (leftover from previous refactoring)
   in py-record-btrace.c.
 - Moved opening curly braces to new line in py-record-btrace.c.

The documentation parts are already approved by Eli.

Is this good to go?

Tim

Tim Wiederhake (9):
  btrace: Count gaps as one instruction explicitly.
  btrace: Export btrace_decode_error function.
  btrace: Use binary search to find instruction.
  Add record_start and record_stop functions.
  Add method to query current recording method to target_ops.
  python: Create Python bindings for record history.
  python: Implement btrace Python bindings for record history.
  python: Add tests for record Python bindings
  Add documentation for new record Python bindings.

 gdb/Makefile.in                               |    6 +
 gdb/NEWS                                      |    4 +
 gdb/btrace.c                                  |  170 +++--
 gdb/btrace.h                                  |   21 +-
 gdb/doc/python.texi                           |  245 ++++++
 gdb/python/py-record-btrace.c                 | 1001 +++++++++++++++++++++++++
 gdb/python/py-record-btrace.h                 |   49 ++
 gdb/python/py-record-full.c                   |   39 +
 gdb/python/py-record-full.h                   |   31 +
 gdb/python/py-record.c                        |  275 +++++++
 gdb/python/python-internal.h                  |    9 +
 gdb/python/python.c                           |   14 +
 gdb/record-btrace.c                           |  110 +--
 gdb/record-full.c                             |   10 +
 gdb/record.c                                  |   42 ++
 gdb/record.h                                  |   21 +
 gdb/target-debug.h                            |    2 +
 gdb/target-delegates.c                        |   33 +
 gdb/target.c                                  |    8 +
 gdb/target.h                                  |    8 +
 gdb/testsuite/gdb.python/py-record-btrace.c   |   46 ++
 gdb/testsuite/gdb.python/py-record-btrace.exp |  146 ++++
 gdb/testsuite/gdb.python/py-record-full.c     |   46 ++
 gdb/testsuite/gdb.python/py-record-full.exp   |   58 ++
 24 files changed, 2256 insertions(+), 138 deletions(-)
 create mode 100644 gdb/python/py-record-btrace.c
 create mode 100644 gdb/python/py-record-btrace.h
 create mode 100644 gdb/python/py-record-full.c
 create mode 100644 gdb/python/py-record-full.h
 create mode 100644 gdb/python/py-record.c
 create mode 100644 gdb/testsuite/gdb.python/py-record-btrace.c
 create mode 100644 gdb/testsuite/gdb.python/py-record-btrace.exp
 create mode 100644 gdb/testsuite/gdb.python/py-record-full.c
 create mode 100644 gdb/testsuite/gdb.python/py-record-full.exp

-- 
2.7.4

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 9/9] Add documentation for new record Python bindings.
  2017-02-13 12:38 ` [PATCH v6 9/9] Add documentation for new record Python bindings Tim Wiederhake
@ 2017-02-13 14:48   ` Eli Zaretskii
  2017-03-03 11:10   ` Yao Qi
  1 sibling, 0 replies; 21+ messages in thread
From: Eli Zaretskii @ 2017-02-13 14:48 UTC (permalink / raw)
  To: Tim Wiederhake; +Cc: gdb-patches, markus.t.metzger, palves, xdje42

> From: Tim Wiederhake <tim.wiederhake@intel.com>
> Cc: markus.t.metzger@intel.com, palves@redhat.com, xdje42@gmail.com
> Date: Mon, 13 Feb 2017 13:37:30 +0100
> 
> 2017-02-13  Tim Wiederhake  <tim.wiederhake@intel.com>
> 
> gdb/ChangeLog:
> 
> 	* NEWS: Add record Python bindings entry.
> 
> gdb/doc/ChangeLog:
> 
> 	* python.texi (Recordings In Python): New section.

Thanks, I believe I already approved a previous version of this.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 7/9] python: Implement btrace Python bindings for record history.
       [not found] ` <1486989450-11313-8-git-send-email-tim.wiederhake@intel.com>
@ 2017-02-13 17:03   ` Doug Evans
  2017-02-13 17:12   ` Doug Evans
  1 sibling, 0 replies; 21+ messages in thread
From: Doug Evans @ 2017-02-13 17:03 UTC (permalink / raw)
  To: Tim Wiederhake; +Cc: gdb-patches, markus.t.metzger, palves

Tim Wiederhake <tim.wiederhake@intel.com> writes:
> This patch implements the gdb.Record Python object methods and fields for
> record target btrace.  Also, implement a stub for record target full.
>
> 2017-02-13  Tim Wiederhake  <tim.wiederhake@intel.com>
>
> gdb/ChangeLog:
>
> 	* Makefile.in (SUBDIR_PYTHON_OBS): Add py-record-btrace.o,
> 	py-record-full.o.
> 	(SUBDIR_PYTHON_SRCS): Add py-record-btrace.c, py-record-full.c.
> 	* python/py-record-btrace.c, python/py-record-btrace.h,
> 	python/py-record-full.c, python/py-record-full.h: New file.
> 	* python/py-record.c: Add include for py-record-btrace.h and
> 	py-record-full.h.
> 	(recpy_method, recpy_format, recpy_goto, recpy_replay_position,
> 	recpy_instruction_history, recpy_function_call_history, recpy_begin,
> 	recpy_end): Use functions from py-record-btrace.c and py-record-full.c.
> 	* python/python-internal.h (PyInt_FromSsize_t, PyInt_AsSsize_t):
> 	New definition.
> 	(gdbpy_initialize_btrace): New export.
> 	* python/python.c (_initialize_python): Add gdbpy_initialize_btrace.

LGTM

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 7/9] python: Implement btrace Python bindings for record history.
       [not found] ` <1486989450-11313-8-git-send-email-tim.wiederhake@intel.com>
  2017-02-13 17:03   ` [PATCH v6 7/9] python: Implement btrace Python bindings for record history Doug Evans
@ 2017-02-13 17:12   ` Doug Evans
  1 sibling, 0 replies; 21+ messages in thread
From: Doug Evans @ 2017-02-13 17:12 UTC (permalink / raw)
  To: Tim Wiederhake; +Cc: gdb-patches, markus.t.metzger, palves

> This patch implements the gdb.Record Python object methods and fields for
> record target btrace.  Also, implement a stub for record target full.
>
> 2017-02-13  Tim Wiederhake  <tim.wiederhake@intel.com>
>
> gdb/ChangeLog:
>
> 	* Makefile.in (SUBDIR_PYTHON_OBS): Add py-record-btrace.o,
> 	py-record-full.o.
> 	(SUBDIR_PYTHON_SRCS): Add py-record-btrace.c, py-record-full.c.
> 	* python/py-record-btrace.c, python/py-record-btrace.h,
> 	python/py-record-full.c, python/py-record-full.h: New file.
> 	* python/py-record.c: Add include for py-record-btrace.h and
> 	py-record-full.h.
> 	(recpy_method, recpy_format, recpy_goto, recpy_replay_position,
> 	recpy_instruction_history, recpy_function_call_history, recpy_begin,
> 	recpy_end): Use functions from py-record-btrace.c and py-record-full.c.
> 	* python/python-internal.h (PyInt_FromSsize_t, PyInt_AsSsize_t):
> 	New definition.
> 	(gdbpy_initialize_btrace): New export.
> 	* python/python.c (_initialize_python): Add gdbpy_initialize_btrace.

In the immortal words of Shrek, "hold the phone". :-)
I see there's more comments than just one in the reply here:
https://sourceware.org/ml/gdb-patches/2017-02/msg00347.html

but the subject line got mixed with patch 8/9.
https://sourceware.org/ml/gdb-patches/2017-02/msg00334.html

This one, 7/9, still LGTM.

> ---
>  gdb/Makefile.in               |    4 +
>  gdb/python/py-record-btrace.c | 1001 +++++++++++++++++++++++++++++++++++++++++
>  gdb/python/py-record-btrace.h |   49 ++
>  gdb/python/py-record-full.c   |   39 ++
>  gdb/python/py-record-full.h   |   31 ++
>  gdb/python/py-record.c        |   48 ++
>  gdb/python/python-internal.h  |    4 +
>  gdb/python/python.c           |    1 +
>  8 files changed, 1177 insertions(+)
>  create mode 100644 gdb/python/py-record-btrace.c
>  create mode 100644 gdb/python/py-record-btrace.h
>  create mode 100644 gdb/python/py-record-full.c
>  create mode 100644 gdb/python/py-record-full.h
>
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index b4419f0..43253d3 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -461,6 +461,8 @@ SUBDIR_PYTHON_OBS = \
>  	py-prettyprint.o \
>  	py-progspace.o \
>  	py-record.o \
> +	py-record-btrace.o \
> +	py-record-full.o \
>  	py-signalevent.o \
>  	py-stopevent.o \
>  	py-symbol.o \
> @@ -502,6 +504,8 @@ SUBDIR_PYTHON_SRCS = \
>  	python/py-prettyprint.c \
>  	python/py-progspace.c \
>  	python/py-record.c \
> +	python/py-record-btrace.c \
> +	python/py-record-full.c \
>  	python/py-signalevent.c \
>  	python/py-stopevent.c \
>  	python/py-symbol.c \
> diff --git a/gdb/python/py-record-btrace.c b/gdb/python/py-record-btrace.c
> new file mode 100644
> index 0000000..6158f31
> --- /dev/null
> +++ b/gdb/python/py-record-btrace.c
> @@ -0,0 +1,1001 @@
> +/* Python interface to btrace instruction history.
> +
> +   Copyright 2016-2017 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include "defs.h"
> +#include "gdbcore.h"
> +#include "gdbcmd.h"
> +#include "gdbthread.h"
> +#include "btrace.h"
> +#include "py-record-btrace.h"
> +#include "disasm.h"
> +
> +#if defined (IS_PY3K)
> +
> +#define BTPY_PYSLICE(x) (x)
> +
> +#else
> +
> +#define BTPY_PYSLICE(x) ((PySliceObject *) x)
> +
> +#endif
> +
> +#define BTPY_REQUIRE_VALID_INSN(obj, iter)				\
> +    do {								\
> +      struct thread_info *tinfo = find_thread_ptid (obj->ptid);		\
> +      if (tinfo == NULL || btrace_is_empty (tinfo))			\
> +	return PyErr_Format (gdbpy_gdb_error, _("Empty branch trace."));\
> +      if (0 == btrace_find_insn_by_number (&iter, &tinfo->btrace,	\
> +					   obj->number))		\
> +	return PyErr_Format (gdbpy_gdb_error, _("No such instruction."));\
> +    } while (0)
> +
> +#define BTPY_REQUIRE_VALID_CALL(obj, iter)				\
> +    do {								\
> +      struct thread_info *tinfo = find_thread_ptid (obj->ptid);		\
> +      if (tinfo == NULL || btrace_is_empty (tinfo))			\
> +	return PyErr_Format (gdbpy_gdb_error, _("Empty branch trace."));\
> +      if (0 == btrace_find_call_by_number (&iter, &tinfo->btrace,	\
> +					   obj->number))		\
> +	return PyErr_Format (gdbpy_gdb_error, _("No such call segment."));\
> +    } while (0)
> +
> +/* This can either be a btrace instruction or a function call segment,
> +   depending on the chosen type.  */
> +
> +typedef struct {
> +  PyObject_HEAD
> +
> +  /* The thread this object belongs to.  */
> +  ptid_t ptid;
> +
> +  /* Instruction number or function call segment number, depending on the type
> +     of this object.  */
> +  Py_ssize_t number;
> +} btpy_object;
> +
> +/* Python object for btrace record lists.  */
> +
> +typedef struct {
> +  PyObject_HEAD
> +
> +  /* The thread this list belongs to.  */
> +  ptid_t ptid;
> +
> +  /* The first index being part of this list.  */
> +  Py_ssize_t first;
> +
> +  /* The last index begin part of this list.  */
> +  Py_ssize_t last;
> +
> +  /* Stride size.  */
> +  Py_ssize_t step;
> +
> +  /* Either &BTPY_CALL_TYPE or &BTPY_INSN_TYPE.  */
> +  PyTypeObject* element_type;
> +} btpy_list_object;
> +
> +/* Python type for btrace instructions.  */
> +
> +static PyTypeObject btpy_insn_type = {
> +  PyVarObject_HEAD_INIT (NULL, 0)
> +};
> +
> +/* Python type for btrace function-calls.  */
> +
> +static PyTypeObject btpy_call_type = {
> +  PyVarObject_HEAD_INIT (NULL, 0)
> +};
> +
> +/* Python type for btrace lists.  */
> +
> +static PyTypeObject btpy_list_type = {
> +  PyVarObject_HEAD_INIT (NULL, 0)
> +};
> +
> +/* Create a new gdb.BtraceInstruction or gdb.BtraceFunctionCall object,
> +   depending on TYPE.  */
> +
> +static PyObject *
> +btpy_new (ptid_t ptid, Py_ssize_t number, PyTypeObject* type)
> +{
> +  btpy_object * const obj = PyObject_New (btpy_object, type);
> +
> +  if (obj == NULL)
> +    return NULL;
> +
> +  obj->ptid = ptid;
> +  obj->number = number;
> +
> +  return (PyObject *) obj;
> +}
> +
> +/* Create a new gdb.BtraceInstruction object.  */
> +
> +static PyObject *
> +btpy_insn_new (ptid_t ptid, Py_ssize_t number)
> +{
> +  return btpy_new (ptid, number, &btpy_insn_type);
> +}
> +
> +/* Create a new gdb.BtraceFunctionCall object.  */
> +
> +static PyObject *
> +btpy_call_new (ptid_t ptid, Py_ssize_t number)
> +{
> +  return btpy_new (ptid, number, &btpy_call_type);
> +}
> +
> +/* Create a new gdb.BtraceList object.  */
> +
> +static PyObject *
> +btpy_list_new (ptid_t ptid, Py_ssize_t first, Py_ssize_t last, Py_ssize_t step,
> +	       PyTypeObject *element_type)
> +{
> +  btpy_list_object * const obj = PyObject_New (btpy_list_object,
> +					       &btpy_list_type);
> +
> +  if (obj == NULL)
> +    return NULL;
> +
> +  obj->ptid = ptid;
> +  obj->first = first;
> +  obj->last = last;
> +  obj->step = step;
> +  obj->element_type = element_type;
> +
> +  return (PyObject *) obj;
> +}
> +
> +/* Implementation of BtraceInstruction.number [int] and
> +   BtraceFunctionCall.number [int].  */
> +
> +static PyObject *
> +btpy_number (PyObject *self, void *closure)
> +{
> +  const btpy_object * const obj = (btpy_object *) self;
> +
> +  return PyInt_FromSsize_t (obj->number);
> +}
> +
> +/* Implementation of BtraceInstruction.__hash__ () -> int and
> +   BtraceFunctionCall.__hash__ () -> int.  */
> +
> +static Py_hash_t
> +btpy_hash (PyObject *self)
> +{
> +  const btpy_object * const obj = (btpy_object *) self;
> +
> +  return obj->number;
> +}
> +
> +/* Implementation of BtraceInstruction.error [int].  Returns the
> +   error code for gaps.  */
> +
> +static PyObject *
> +btpy_insn_error (PyObject *self, void *closure)
> +{
> +  const btpy_object * const obj = (btpy_object *) self;
> +  struct btrace_insn_iterator iter;
> +  int error;
> +
> +  BTPY_REQUIRE_VALID_INSN (obj, iter);
> +
> +  error = btrace_insn_get_error (&iter);
> +
> +  if (error == 0)
> +    Py_RETURN_NONE;
> +
> +  return PyInt_FromLong (error);
> +}
> +
> +/* Implementation of BtraceInstruction.sal [gdb.Symtab_and_line].
> +   Return the SAL associated with this instruction.  */
> +
> +static PyObject *
> +btpy_insn_sal (PyObject *self, void *closure)
> +{
> +  const btpy_object * const obj = (btpy_object *) self;
> +  const struct btrace_insn *insn;
> +  struct btrace_insn_iterator iter;
> +  PyObject *result = NULL;
> +
> +  BTPY_REQUIRE_VALID_INSN (obj, iter);
> +
> +  insn = btrace_insn_get (&iter);
> +  if (insn == NULL)
> +    Py_RETURN_NONE;
> +
> +  TRY
> +    {
> +      result = symtab_and_line_to_sal_object (find_pc_line (insn->pc, 0));
> +    }
> +  CATCH (except, RETURN_MASK_ALL)
> +    {
> +      GDB_PY_HANDLE_EXCEPTION (except);
> +    }
> +  END_CATCH
> +
> +  return result;
> +}
> +
> +/* Implementation of BtraceInstruction.pc [int].  Returns
> +   the instruction address.  */
> +
> +static PyObject *
> +btpy_insn_pc (PyObject *self, void *closure)
> +{
> +  const btpy_object * const obj = (btpy_object *) self;
> +  const struct btrace_insn *insn;
> +  struct btrace_insn_iterator iter;
> +
> +  BTPY_REQUIRE_VALID_INSN (obj, iter);
> +
> +  insn = btrace_insn_get (&iter);
> +  if (insn == NULL)
> +    Py_RETURN_NONE;
> +
> +  return gdb_py_long_from_ulongest (insn->pc);
> +}
> +
> +/* Implementation of BtraceInstruction.size [int].  Returns
> +   the instruction size.  */
> +
> +static PyObject *
> +btpy_insn_size (PyObject *self, void *closure)
> +{
> +  const btpy_object * const obj = (btpy_object *) self;
> +  const struct btrace_insn *insn;
> +  struct btrace_insn_iterator iter;
> +
> +  BTPY_REQUIRE_VALID_INSN (obj, iter);
> +
> +  insn = btrace_insn_get (&iter);
> +  if (insn == NULL)
> +    Py_RETURN_NONE;
> +
> +  return PyInt_FromLong (insn->size);
> +}
> +
> +/* Implementation of BtraceInstruction.is_speculative [bool].
> +   Returns if this instruction was executed speculatively.  */
> +
> +static PyObject *
> +btpy_insn_is_speculative (PyObject *self, void *closure)
> +{
> +  const btpy_object * const obj = (btpy_object *) self;
> +  const struct btrace_insn *insn;
> +  struct btrace_insn_iterator iter;
> +
> +  BTPY_REQUIRE_VALID_INSN (obj, iter);
> +
> +  insn = btrace_insn_get (&iter);
> +  if (insn == NULL)
> +    Py_RETURN_NONE;
> +
> +  if (insn->flags & BTRACE_INSN_FLAG_SPECULATIVE)
> +    Py_RETURN_TRUE;
> +  else
> +    Py_RETURN_FALSE;
> +}
> +
> +/* Implementation of BtraceInstruction.data [buffer].
> +   Returns raw instruction data.  */
> +
> +static PyObject *
> +btpy_insn_data (PyObject *self, void *closure)
> +{
> +  const btpy_object * const obj = (btpy_object *) self;
> +  const struct btrace_insn *insn;
> +  struct btrace_insn_iterator iter;
> +  gdb_byte *buffer = NULL;
> +  PyObject *object;
> +
> +  BTPY_REQUIRE_VALID_INSN (obj, iter);
> +
> +  insn = btrace_insn_get (&iter);
> +  if (insn == NULL)
> +    Py_RETURN_NONE;
> +
> +  TRY
> +    {
> +      buffer = (gdb_byte *) xmalloc (insn->size);
> +      read_memory (insn->pc, buffer, insn->size);
> +    }
> +  CATCH (except, RETURN_MASK_ALL)
> +    {
> +      xfree (buffer);
> +      GDB_PY_HANDLE_EXCEPTION (except);
> +    }
> +  END_CATCH
> +
> +  object = PyBytes_FromStringAndSize ((const char*) buffer, insn->size);
> +  xfree (buffer);
> +
> +  if (object == NULL)
> +    return NULL;
> +
> +  return PyMemoryView_FromObject (object);
> +}
> +
> +/* Implementation of BtraceInstruction.decode [str].  Returns
> +   the instruction as human readable string.  */
> +
> +static PyObject *
> +btpy_insn_decode (PyObject *self, void *closure)
> +{
> +  const btpy_object * const obj = (btpy_object *) self;
> +  const struct btrace_insn *insn;
> +  struct btrace_insn_iterator iter;
> +  string_file strfile;
> +
> +  BTPY_REQUIRE_VALID_INSN (obj, iter);
> +
> +  insn = btrace_insn_get (&iter);
> +  if (insn == NULL)
> +    {
> +      int error_code = btrace_insn_get_error (&iter);
> +      const struct btrace_config *config;
> +
> +      config = btrace_conf (&find_thread_ptid (obj->ptid)->btrace);
> +      return PyBytes_FromString (btrace_decode_error (config->format,
> +						      error_code));
> +    }
> +
> +  TRY
> +    {
> +      gdb_print_insn (target_gdbarch (), insn->pc, &strfile, NULL);
> +    }
> +  CATCH (except, RETURN_MASK_ALL)
> +    {
> +      gdbpy_convert_exception (except);
> +      return NULL;
> +    }
> +  END_CATCH
> +
> +
> +  return PyBytes_FromString (strfile.string ().c_str ());
> +}
> +
> +/* Implementation of BtraceFunctionCall.level [int].  Returns the
> +   call level.  */
> +
> +static PyObject *
> +btpy_call_level (PyObject *self, void *closure)
> +{
> +  const btpy_object * const obj = (btpy_object *) self;
> +  const struct btrace_function *func;
> +  struct btrace_call_iterator iter;
> +
> +  BTPY_REQUIRE_VALID_CALL (obj, iter);
> +
> +  func = btrace_call_get (&iter);
> +  if (func == NULL)
> +    Py_RETURN_NONE;
> +
> +  return PyInt_FromLong (iter.btinfo->level + func->level);
> +}
> +
> +/* Implementation of BtraceFunctionCall.symbol [gdb.Symbol].  Returns
> +   the symbol associated with this function call.  */
> +
> +static PyObject *
> +btpy_call_symbol (PyObject *self, void *closure)
> +{
> +  const btpy_object * const obj = (btpy_object *) self;
> +  const struct btrace_function *func;
> +  struct btrace_call_iterator iter;
> +
> +  BTPY_REQUIRE_VALID_CALL (obj, iter);
> +
> +  func = btrace_call_get (&iter);
> +  if (func == NULL)
> +    Py_RETURN_NONE;
> +
> +  if (func->sym == NULL)
> +    Py_RETURN_NONE;
> +
> +  return symbol_to_symbol_object (func->sym);
> +}
> +
> +/* Implementation of BtraceFunctionCall.instructions [list].
> +   Return the list of instructions that belong to this function call.  */
> +
> +static PyObject *
> +btpy_call_instructions (PyObject *self, void *closure)
> +{
> +  const btpy_object * const obj = (btpy_object *) self;
> +  const struct btrace_function *func;
> +  struct btrace_call_iterator iter;
> +  unsigned int len;
> +
> +  BTPY_REQUIRE_VALID_CALL (obj, iter);
> +
> +  func = btrace_call_get (&iter);
> +  if (func == NULL)
> +    Py_RETURN_NONE;
> +
> +  len = VEC_length (btrace_insn_s, func->insn);
> +
> +  /* Gaps count as one instruction.  */
> +  if (len == 0)
> +    len = 1;
> +
> +  return btpy_list_new (obj->ptid, func->insn_offset, func->insn_offset + len,
> +			1, &btpy_insn_type);
> +}
> +
> +/* Implementation of BtraceFunctionCall.up [gdb.BtraceRecordCall].
> +   Return the caller / returnee of this function.  */
> +
> +static PyObject *
> +btpy_call_up (PyObject *self, void *closure)
> +{
> +  const btpy_object * const obj = (btpy_object *) self;
> +  const struct btrace_function *func;
> +  struct btrace_call_iterator iter;
> +
> +  BTPY_REQUIRE_VALID_CALL (obj, iter);
> +
> +  func = btrace_call_get (&iter);
> +  if (func == NULL)
> +    Py_RETURN_NONE;
> +
> +  if (func->up == NULL)
> +    Py_RETURN_NONE;
> +
> +  return btpy_call_new (obj->ptid, func->up->number);
> +}
> +
> +/* Implementation of BtraceFunctionCall.prev_sibling [BtraceFunctionCall].
> +   Return a previous segment of this function.  */
> +
> +static PyObject *
> +btpy_call_prev_sibling (PyObject *self, void *closure)
> +{
> +  const btpy_object * const obj = (btpy_object *) self;
> +  const struct btrace_function *func;
> +  struct btrace_call_iterator iter;
> +
> +  BTPY_REQUIRE_VALID_CALL (obj, iter);
> +
> +  func = btrace_call_get (&iter);
> +  if (func == NULL)
> +    Py_RETURN_NONE;
> +
> +  if (func->segment.prev == NULL)
> +    Py_RETURN_NONE;
> +
> +  return btpy_call_new (obj->ptid, func->segment.prev->number);
> +}
> +
> +/* Implementation of BtraceFunctionCall.next_sibling [BtraceFunctionCall].
> +   Return a following segment of this function.  */
> +
> +static PyObject *
> +btpy_call_next_sibling (PyObject *self, void *closure)
> +{
> +  const btpy_object * const obj = (btpy_object *) self;
> +  const struct btrace_function *func;
> +  struct btrace_call_iterator iter;
> +
> +  BTPY_REQUIRE_VALID_CALL (obj, iter);
> +
> +  func = btrace_call_get (&iter);
> +  if (func == NULL)
> +    Py_RETURN_NONE;
> +
> +  if (func->segment.next == NULL)
> +    Py_RETURN_NONE;
> +
> +  return btpy_call_new (obj->ptid, func->segment.next->number);
> +}
> +
> +/* Python rich compare function to allow for equality and inequality checks
> +   in Python.  */
> +
> +static PyObject *
> +btpy_richcompare (PyObject *self, PyObject *other, int op)
> +{
> +  const btpy_object * const obj1 = (btpy_object *) self;
> +  const btpy_object * const obj2 = (btpy_object *) other;
> +
> +  if (Py_TYPE (self) != Py_TYPE (other))
> +    {
> +      Py_INCREF (Py_NotImplemented);
> +      return Py_NotImplemented;
> +    }
> +
> +  switch (op)
> +  {
> +    case Py_EQ:
> +      if (ptid_equal (obj1->ptid, obj2->ptid) && obj1->number == obj2->number)
> +	Py_RETURN_TRUE;
> +      else
> +	Py_RETURN_FALSE;
> +
> +    case Py_NE:
> +      if (!ptid_equal (obj1->ptid, obj2->ptid) || obj1->number != obj2->number)
> +	Py_RETURN_TRUE;
> +      else
> +	Py_RETURN_FALSE;
> +
> +    default:
> +      break;
> +  }
> +
> +  Py_INCREF (Py_NotImplemented);
> +  return Py_NotImplemented;
> +}
> +
> +/* Implementation of BtraceList.__len__ (self) -> int.  */
> +
> +static Py_ssize_t
> +btpy_list_length (PyObject *self)
> +{
> +  const btpy_list_object * const obj = (btpy_list_object *) self;
> +  const Py_ssize_t distance = obj->last - obj->first;
> +  const Py_ssize_t result = distance / obj->step;
> +
> +  if ((distance % obj->step) == 0)
> +    return result;
> +
> +  return result + 1;
> +}
> +
> +/* Implementation of
> +   BtraceList.__getitem__ (self, key) -> BtraceInstruction and
> +   BtraceList.__getitem__ (self, key) -> BtraceFunctionCall.  */
> +
> +static PyObject *
> +btpy_list_item (PyObject *self, Py_ssize_t index)
> +{
> +  const btpy_list_object * const obj = (btpy_list_object *) self;
> +  struct thread_info * const tinfo = find_thread_ptid (obj->ptid);
> +
> +  if (index < 0 || index >= btpy_list_length (self))
> +    return PyErr_Format (PyExc_IndexError, _("Index out of range: %zd."),
> +			 index);
> +
> +  return btpy_new (obj->ptid, obj->first + (obj->step * index),
> +		   obj->element_type);
> +}
> +
> +/* Implementation of BtraceList.__getitem__ (self, slice) -> BtraceList.  */
> +
> +static PyObject *
> +btpy_list_slice (PyObject *self, PyObject *value)
> +{
> +  const btpy_list_object * const obj = (btpy_list_object *) self;
> +  const Py_ssize_t length = btpy_list_length (self);
> +  Py_ssize_t start, stop, step, slicelength;
> +
> +  if (PyInt_Check (value))
> +    {
> +      Py_ssize_t index = PyInt_AsSsize_t (value);
> +
> +      /* Emulate Python behavior for negative indices.  */
> +      if (index < 0)
> +	index += length;
> +
> +      return btpy_list_item (self, index);
> +    }
> +
> +  if (!PySlice_Check (value))
> +    return PyErr_Format (PyExc_TypeError, _("Index must be int or slice."));
> +
> +  if (0 != PySlice_GetIndicesEx (BTPY_PYSLICE (value), length, &start, &stop,
> +				 &step, &slicelength))
> +    return NULL;
> +
> +  return btpy_list_new (obj->ptid, obj->first + obj->step * start,
> +			obj->first + obj->step * stop, obj->step * step,
> +			obj->element_type);
> +}
> +
> +/* Helper function that returns the position of an element in a BtraceList
> +   or -1 if the element is not in the list.  */
> +
> +static LONGEST
> +btpy_list_position (PyObject *self, PyObject *value)
> +{
> +  const btpy_list_object * const list_obj = (btpy_list_object *) self;
> +  const btpy_object * const obj = (btpy_object *) value;
> +  Py_ssize_t index = obj->number;
> +
> +  if (list_obj->element_type != Py_TYPE (value))
> +    return -1;
> +
> +  if (!ptid_equal (list_obj->ptid, obj->ptid))
> +    return -1;
> +
> +  if (index < list_obj->first || index > list_obj->last)
> +    return -1;
> +
> +  index -= list_obj->first;
> +
> +  if (index % list_obj->step != 0)
> +    return -1;
> +
> +  return index / list_obj->step;
> +}
> +
> +/* Implementation of "in" operator for BtraceLists.  */
> +
> +static int
> +btpy_list_contains (PyObject *self, PyObject *value)
> +{
> +  if (btpy_list_position (self, value) < 0)
> +    return 0;
> +
> +  return 1;
> +}
> +
> +/* Implementation of BtraceLists.index (self, value) -> int.  */
> +
> +static PyObject *
> +btpy_list_index (PyObject *self, PyObject *value)
> +{
> +  const LONGEST index = btpy_list_position (self, value);
> +
> +  if (index < 0)
> +    return PyErr_Format (PyExc_ValueError, _("Not in list."));
> +
> +  return gdb_py_long_from_longest (index);
> +}
> +
> +/* Implementation of BtraceList.count (self, value) -> int.  */
> +
> +static PyObject *
> +btpy_list_count (PyObject *self, PyObject *value)
> +{
> +  /* We know that if an element is in the list, it is so exactly one time,
> +     enabling us to reuse the "is element of" check.  */
> +  return PyInt_FromLong (btpy_list_contains (self, value));
> +}
> +
> +/* Python rich compare function to allow for equality and inequality checks
> +   in Python.  */
> +
> +static PyObject *
> +btpy_list_richcompare (PyObject *self, PyObject *other, int op)
> +{
> +  const btpy_list_object * const obj1 = (btpy_list_object *) self;
> +  const btpy_list_object * const obj2 = (btpy_list_object *) other;
> +
> +  if (Py_TYPE (self) != Py_TYPE (other))
> +    {
> +      Py_INCREF (Py_NotImplemented);
> +      return Py_NotImplemented;
> +    }
> +
> +  switch (op)
> +  {
> +    case Py_EQ:
> +      if (ptid_equal (obj1->ptid, obj2->ptid)
> +	  && obj1->element_type == obj2->element_type
> +	  && obj1->first == obj2->first
> +	  && obj1->last == obj2->last
> +	  && obj1->step == obj2->step)
> +	Py_RETURN_TRUE;
> +      else
> +	Py_RETURN_FALSE;
> +
> +    case Py_NE:
> +      if (!ptid_equal (obj1->ptid, obj2->ptid)
> +	  || obj1->element_type != obj2->element_type
> +	  || obj1->first != obj2->first
> +	  || obj1->last != obj2->last
> +	  || obj1->step != obj2->step)
> +	Py_RETURN_TRUE;
> +      else
> +	Py_RETURN_FALSE;
> +
> +    default:
> +      break;
> +  }
> +
> +  Py_INCREF (Py_NotImplemented);
> +  return Py_NotImplemented;
> +}
> +
> +/* Implementation of
> +   BtraceRecord.method [str].  */
> +
> +PyObject *
> +recpy_bt_method (PyObject *self, void *closure)
> +{
> +  return PyString_FromString ("btrace");
> +}
> +
> +/* Implementation of
> +   BtraceRecord.format [str].  */
> +
> +PyObject *
> +recpy_bt_format (PyObject *self, void *closure)
> +{
> +  const struct thread_info * const tinfo = find_thread_ptid (inferior_ptid);
> +  const struct btrace_config * config;
> +
> +  if (tinfo == NULL)
> +    Py_RETURN_NONE;
> +
> +  config = btrace_conf (&tinfo->btrace);
> +
> +  if (config == NULL)
> +    Py_RETURN_NONE;
> +
> +  return PyString_FromString (btrace_format_short_string (config->format));
> +}
> +
> +/* Implementation of
> +   BtraceRecord.replay_position [BtraceInstruction].  */
> +
> +PyObject *
> +recpy_bt_replay_position (PyObject *self, void *closure)
> +{
> +  const struct thread_info * const tinfo = find_thread_ptid (inferior_ptid);
> +
> +  if (tinfo == NULL)
> +    Py_RETURN_NONE;
> +
> +  if (tinfo->btrace.replay == NULL)
> +    Py_RETURN_NONE;
> +
> +  return btpy_insn_new (inferior_ptid,
> +			btrace_insn_number (tinfo->btrace.replay));
> +}
> +
> +/* Implementation of
> +   BtraceRecord.begin [BtraceInstruction].  */
> +
> +PyObject *
> +recpy_bt_begin (PyObject *self, void *closure)
> +{
> +  struct thread_info * const tinfo = find_thread_ptid (inferior_ptid);
> +  struct btrace_insn_iterator iterator;
> +
> +  if (tinfo == NULL)
> +    Py_RETURN_NONE;
> +
> +  btrace_fetch (tinfo);
> +
> +  if (btrace_is_empty (tinfo))
> +    Py_RETURN_NONE;
> +
> +  btrace_insn_begin (&iterator, &tinfo->btrace);
> +  return btpy_insn_new (inferior_ptid, btrace_insn_number (&iterator));
> +}
> +
> +/* Implementation of
> +   BtraceRecord.end [BtraceInstruction].  */
> +
> +PyObject *
> +recpy_bt_end (PyObject *self, void *closure)
> +{
> +  struct thread_info * const tinfo = find_thread_ptid (inferior_ptid);
> +  struct btrace_insn_iterator iterator;
> +
> +  if (tinfo == NULL)
> +    Py_RETURN_NONE;
> +
> +  btrace_fetch (tinfo);
> +
> +  if (btrace_is_empty (tinfo))
> +    Py_RETURN_NONE;
> +
> +  btrace_insn_end (&iterator, &tinfo->btrace);
> +  return btpy_insn_new (inferior_ptid, btrace_insn_number (&iterator));
> +}
> +
> +/* Implementation of
> +   BtraceRecord.instruction_history [list].  */
> +
> +PyObject *
> +recpy_bt_instruction_history (PyObject *self, void *closure)
> +{
> +  struct thread_info * const tinfo = find_thread_ptid (inferior_ptid);
> +  struct btrace_insn_iterator iterator;
> +  unsigned long first = 0;
> +  unsigned long last = 0;
> +
> +   if (tinfo == NULL)
> +     Py_RETURN_NONE;
> +
> +   btrace_fetch (tinfo);
> +
> +   if (btrace_is_empty (tinfo))
> +     Py_RETURN_NONE;
> +
> +   btrace_insn_begin (&iterator, &tinfo->btrace);
> +   first = btrace_insn_number (&iterator);
> +
> +   btrace_insn_end (&iterator, &tinfo->btrace);
> +   last = btrace_insn_number (&iterator);
> +
> +   return btpy_list_new (inferior_ptid, first, last, 1, &btpy_insn_type);
> +}
> +
> +/* Implementation of
> +   BtraceRecord.function_call_history [list].  */
> +
> +PyObject *
> +recpy_bt_function_call_history (PyObject *self, void *closure)
> +{
> +    struct thread_info * const tinfo = find_thread_ptid (inferior_ptid);
> +    struct btrace_call_iterator iterator;
> +    unsigned long first = 0;
> +    unsigned long last = 0;
> +
> +    if (tinfo == NULL)
> +      Py_RETURN_NONE;
> +
> +    btrace_fetch (tinfo);
> +
> +    if (btrace_is_empty (tinfo))
> +      Py_RETURN_NONE;
> +
> +    btrace_call_begin (&iterator, &tinfo->btrace);
> +    first = btrace_call_number (&iterator);
> +
> +    btrace_call_end (&iterator, &tinfo->btrace);
> +    last = btrace_call_number (&iterator);
> +
> +    return btpy_list_new (inferior_ptid, first, last, 1, &btpy_call_type);
> +}
> +
> +/* Implementation of BtraceRecord.goto (self, BtraceInstruction) -> None.  */
> +
> +PyObject *
> +recpy_bt_goto (PyObject *self, PyObject *args)
> +{
> +  struct thread_info * const tinfo = find_thread_ptid (inferior_ptid);
> +  const btpy_object *obj;
> +
> +  if (tinfo == NULL || btrace_is_empty (tinfo))
> +	return PyErr_Format (gdbpy_gdb_error, _("Empty branch trace."));
> +
> +  if (!PyArg_ParseTuple (args, "O", &obj))
> +    return NULL;
> +
> +  if (Py_TYPE (obj) != &btpy_insn_type)
> +    return PyErr_Format (PyExc_TypeError, _("Argument must be instruction."));
> +
> +  TRY
> +    {
> +      struct btrace_insn_iterator iter;
> +
> +      btrace_insn_end (&iter, &tinfo->btrace);
> +
> +      if (btrace_insn_number (&iter) == obj->number)
> +	target_goto_record_end ();
> +      else
> +	target_goto_record (obj->number);
> +    }
> +  CATCH (except, RETURN_MASK_ALL)
> +    {
> +      GDB_PY_HANDLE_EXCEPTION (except);
> +    }
> +  END_CATCH
> +
> +  Py_RETURN_NONE;
> +}
> +
> +/* BtraceInstruction members.  */
> +
> +struct PyGetSetDef btpy_insn_getset[] =
> +{
> +  { "number", btpy_number, NULL, "instruction number", NULL},
> +  { "error", btpy_insn_error, NULL, "error number for gaps", NULL},
> +  { "sal", btpy_insn_sal, NULL, "associated symbol and line", NULL},
> +  { "pc", btpy_insn_pc, NULL, "instruction address", NULL},
> +  { "data", btpy_insn_data, NULL, "raw instruction data", NULL},
> +  { "decoded", btpy_insn_decode, NULL, "decoded instruction or error message \
> +if the instruction is a gap", NULL},
> +  { "size", btpy_insn_size, NULL, "instruction size in byte", NULL},
> +  { "is_speculative", btpy_insn_is_speculative, NULL, "if the instruction was \
> +executed speculatively", NULL},
> +  {NULL}
> +};
> +
> +/* BtraceFunctionCall members.  */
> +
> +static PyGetSetDef btpy_call_getset[] =
> +{
> +  { "number", btpy_number, NULL, "function call number", NULL},
> +  { "level", btpy_call_level, NULL, "call stack level", NULL},
> +  { "symbol", btpy_call_symbol, NULL, "associated line and symbol", NULL},
> +  { "instructions", btpy_call_instructions, NULL, "list of instructions in \
> +this function segment", NULL},
> +  { "up", btpy_call_up, NULL, "caller or returned-to function segment", NULL},
> +  { "prev_sibling", btpy_call_prev_sibling, NULL, "previous segment of this \
> +function", NULL},
> +  { "next_sibling", btpy_call_next_sibling, NULL, "next segment of this \
> +function", NULL},
> +  {NULL}
> +};
> +
> +/* BtraceList methods.  */
> +
> +struct PyMethodDef btpy_list_methods[] =
> +{
> +  { "count", btpy_list_count, METH_O, "count number of occurences"},
> +  { "index", btpy_list_index, METH_O, "index of entry"},
> +  {NULL}
> +};
> +
> +/* BtraceList sequence methods.  */
> +
> +static PySequenceMethods btpy_list_sequence_methods =
> +{
> +  NULL
> +};
> +
> +/* BtraceList mapping methods.  Necessary for slicing.  */
> +
> +static PyMappingMethods btpy_list_mapping_methods =
> +{
> +  NULL
> +};
> +
> +/* Sets up the btrace record API.  */
> +
> +int
> +gdbpy_initialize_btrace (void)
> +{
> +  btpy_insn_type.tp_new = PyType_GenericNew;
> +  btpy_insn_type.tp_flags = Py_TPFLAGS_DEFAULT;
> +  btpy_insn_type.tp_basicsize = sizeof (btpy_object);
> +  btpy_insn_type.tp_name = "gdb.BtraceInstruction";
> +  btpy_insn_type.tp_doc = "GDB btrace instruction object";
> +  btpy_insn_type.tp_getset = btpy_insn_getset;
> +  btpy_insn_type.tp_richcompare = btpy_richcompare;
> +  btpy_insn_type.tp_hash = btpy_hash;
> +
> +  btpy_call_type.tp_new = PyType_GenericNew;
> +  btpy_call_type.tp_flags = Py_TPFLAGS_DEFAULT;
> +  btpy_call_type.tp_basicsize = sizeof (btpy_object);
> +  btpy_call_type.tp_name = "gdb.BtraceFunctionCall";
> +  btpy_call_type.tp_doc = "GDB btrace call object";
> +  btpy_call_type.tp_getset = btpy_call_getset;
> +  btpy_call_type.tp_richcompare = btpy_richcompare;
> +  btpy_call_type.tp_hash = btpy_hash;
> +
> +  btpy_list_type.tp_new = PyType_GenericNew;
> +  btpy_list_type.tp_flags = Py_TPFLAGS_DEFAULT;
> +  btpy_list_type.tp_basicsize = sizeof (btpy_list_object);
> +  btpy_list_type.tp_name = "gdb.BtraceObjectList";
> +  btpy_list_type.tp_doc = "GDB btrace list object";
> +  btpy_list_type.tp_methods = btpy_list_methods;
> +  btpy_list_type.tp_as_sequence = &btpy_list_sequence_methods;
> +  btpy_list_type.tp_as_mapping = &btpy_list_mapping_methods;
> +  btpy_list_type.tp_richcompare = btpy_list_richcompare;
> +
> +  btpy_list_sequence_methods.sq_item = btpy_list_item;
> +  btpy_list_sequence_methods.sq_length = btpy_list_length;
> +  btpy_list_sequence_methods.sq_contains = btpy_list_contains;
> +
> +  btpy_list_mapping_methods.mp_subscript = btpy_list_slice;
> +
> +  if (PyType_Ready (&btpy_insn_type) < 0
> +      || PyType_Ready (&btpy_call_type) < 0
> +      || PyType_Ready (&btpy_list_type) < 0)
> +    return -1;
> +  else
> +    return 0;
> +}
> diff --git a/gdb/python/py-record-btrace.h b/gdb/python/py-record-btrace.h
> new file mode 100644
> index 0000000..aefb1e4
> --- /dev/null
> +++ b/gdb/python/py-record-btrace.h
> @@ -0,0 +1,49 @@
> +/* Python interface to btrace record targets.
> +
> +   Copyright 2016-2017 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef GDB_PY_RECORD_BTRACE_H
> +#define GDB_PY_RECORD_BTRACE_H
> +
> +#include "python-internal.h"
> +
> +/* Implementation of record.method [str].  */
> +extern PyObject *recpy_bt_method (PyObject *self, void *closure);
> +
> +/* Implementation of record.format [str].  */
> +extern PyObject *recpy_bt_format (PyObject *self, void *closure);
> +
> +/* Implementation of record.goto (instruction) -> None.  */
> +extern PyObject *recpy_bt_goto (PyObject *self, PyObject *value);
> +
> +/* Implementation of record.instruction_history [list].  */
> +extern PyObject *recpy_bt_instruction_history (PyObject *self, void *closure);
> +
> +/* Implementation of record.function_call_history [list].  */
> +extern PyObject *recpy_bt_function_call_history (PyObject *self, void *closure);
> +
> +/* Implementation of record.replay_position [instruction].  */
> +extern PyObject *recpy_bt_replay_position (PyObject *self, void *closure);
> +
> +/* Implementation of record.begin [instruction].  */
> +extern PyObject *recpy_bt_begin (PyObject *self, void *closure);
> +
> +/* Implementation of record.end [instruction].  */
> +extern PyObject *recpy_bt_end (PyObject *self, void *closure);
> +
> +#endif /* GDB_PY_RECORD_BTRACE_H */
> diff --git a/gdb/python/py-record-full.c b/gdb/python/py-record-full.c
> new file mode 100644
> index 0000000..06e6693
> --- /dev/null
> +++ b/gdb/python/py-record-full.c
> @@ -0,0 +1,39 @@
> +/* Python interface to btrace instruction history.
> +
> +   Copyright 2016-2017 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include "defs.h"
> +#include "py-record-full.h"
> +
> +/* Implementation of
> +   BtraceRecord.method [str].  */
> +
> +PyObject *
> +recpy_full_method (PyObject *self, void *closure)
> +{
> +  return PyString_FromString ("full");
> +}
> +
> +/* Implementation of
> +   BtraceRecord.format [str].  */
> +
> +PyObject *
> +recpy_full_format (PyObject *self, void *closure)
> +{
> +  return PyString_FromString ("full");
> +}
> diff --git a/gdb/python/py-record-full.h b/gdb/python/py-record-full.h
> new file mode 100644
> index 0000000..30acc51
> --- /dev/null
> +++ b/gdb/python/py-record-full.h
> @@ -0,0 +1,31 @@
> +/* Python interface to full record targets.
> +
> +   Copyright 2016-2017 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef GDB_PY_RECORD_FULL_H
> +#define GDB_PY_RECORD_FULL_H
> +
> +#include "python-internal.h"
> +
> +/* Implementation of record.method [str].  */
> +extern PyObject *recpy_full_method (PyObject *self, void *value);
> +
> +/* Implementation of record.format [str].  */
> +extern PyObject *recpy_full_format (PyObject *self, void *value);
> +
> +#endif /* GDB_PY_RECORD_FULL_H */
> diff --git a/gdb/python/py-record.c b/gdb/python/py-record.c
> index 27ecbb6..72922a4 100644
> --- a/gdb/python/py-record.c
> +++ b/gdb/python/py-record.c
> @@ -21,6 +21,8 @@
>  #include "inferior.h"
>  #include "record.h"
>  #include "python-internal.h"
> +#include "py-record-btrace.h"
> +#include "py-record-full.h"
>  #include "target.h"
>  
>  /* Python Record object.  */
> @@ -57,6 +59,14 @@ recpy_ptid (PyObject *self, void* closure)
>  static PyObject *
>  recpy_method (PyObject *self, void* closure)
>  {
> +  const recpy_record_object * const obj = (recpy_record_object *) self;
> +
> +  if (obj->method == RECORD_METHOD_FULL)
> +    return recpy_full_method (self, closure);
> +
> +  if (obj->method == RECORD_METHOD_BTRACE)
> +    return recpy_bt_method (self, closure);
> +
>    return PyErr_Format (PyExc_NotImplementedError, _("Not implemented."));
>  }
>  
> @@ -65,6 +75,14 @@ recpy_method (PyObject *self, void* closure)
>  static PyObject *
>  recpy_format (PyObject *self, void* closure)
>  {
> +  const recpy_record_object * const obj = (recpy_record_object *) self;
> +
> +  if (obj->method == RECORD_METHOD_FULL)
> +    return recpy_full_format (self, closure);
> +
> +  if (obj->method == RECORD_METHOD_BTRACE)
> +    return recpy_bt_format (self, closure);
> +
>    return PyErr_Format (PyExc_NotImplementedError, _("Not implemented."));
>  }
>  
> @@ -73,6 +91,11 @@ recpy_format (PyObject *self, void* closure)
>  static PyObject *
>  recpy_goto (PyObject *self, PyObject *value)
>  {
> +  const recpy_record_object * const obj = (recpy_record_object *) self;
> +
> +  if (obj->method == RECORD_METHOD_BTRACE)
> +    return recpy_bt_goto (self, value);
> +
>    return PyErr_Format (PyExc_NotImplementedError, _("Not implemented."));
>  }
>  
> @@ -81,6 +104,11 @@ recpy_goto (PyObject *self, PyObject *value)
>  static PyObject *
>  recpy_replay_position (PyObject *self, void *closure)
>  {
> +  const recpy_record_object * const obj = (recpy_record_object *) self;
> +
> +  if (obj->method == RECORD_METHOD_BTRACE)
> +    return recpy_bt_replay_position (self, closure);
> +
>    return PyErr_Format (PyExc_NotImplementedError, _("Not implemented."));
>  }
>  
> @@ -89,6 +117,11 @@ recpy_replay_position (PyObject *self, void *closure)
>  static PyObject *
>  recpy_instruction_history (PyObject *self, void* closure)
>  {
> +  const recpy_record_object * const obj = (recpy_record_object *) self;
> +
> +  if (obj->method == RECORD_METHOD_BTRACE)
> +    return recpy_bt_instruction_history (self, closure);
> +
>    return PyErr_Format (PyExc_NotImplementedError, _("Not implemented."));
>  }
>  
> @@ -97,6 +130,11 @@ recpy_instruction_history (PyObject *self, void* closure)
>  static PyObject *
>  recpy_function_call_history (PyObject *self, void* closure)
>  {
> +  const recpy_record_object * const obj = (recpy_record_object *) self;
> +
> +  if (obj->method == RECORD_METHOD_BTRACE)
> +    return recpy_bt_function_call_history (self, closure);
> +
>    return PyErr_Format (PyExc_NotImplementedError, _("Not implemented."));
>  }
>  
> @@ -105,6 +143,11 @@ recpy_function_call_history (PyObject *self, void* closure)
>  static PyObject *
>  recpy_begin (PyObject *self, void* closure)
>  {
> +  const recpy_record_object * const obj = (recpy_record_object *) self;
> +
> +  if (obj->method == RECORD_METHOD_BTRACE)
> +    return recpy_bt_begin (self, closure);
> +
>    return PyErr_Format (PyExc_NotImplementedError, _("Not implemented."));
>  }
>  
> @@ -113,6 +156,11 @@ recpy_begin (PyObject *self, void* closure)
>  static PyObject *
>  recpy_end (PyObject *self, void* closure)
>  {
> +  const recpy_record_object * const obj = (recpy_record_object *) self;
> +
> +  if (obj->method == RECORD_METHOD_BTRACE)
> +    return recpy_bt_end (self, closure);
> +
>    return PyErr_Format (PyExc_NotImplementedError, _("Not implemented."));
>  }
>  
> diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
> index c6af9f7..4dd413d 100644
> --- a/gdb/python/python-internal.h
> +++ b/gdb/python/python-internal.h
> @@ -104,7 +104,9 @@
>  
>  #define PyInt_Check PyLong_Check
>  #define PyInt_FromLong PyLong_FromLong
> +#define PyInt_FromSsize_t PyLong_FromSsize_t
>  #define PyInt_AsLong PyLong_AsLong
> +#define PyInt_AsSsize_t PyLong_AsSsize_t
>  
>  #define PyString_FromString PyUnicode_FromString
>  #define PyString_Decode PyUnicode_Decode
> @@ -439,6 +441,8 @@ int gdbpy_initialize_values (void)
>    CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
>  int gdbpy_initialize_frames (void)
>    CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
> +int gdbpy_initialize_btrace (void)
> +  CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
>  int gdbpy_initialize_record (void)
>    CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
>  int gdbpy_initialize_symtabs (void)
> diff --git a/gdb/python/python.c b/gdb/python/python.c
> index f8b9f7e..f878f9a 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -1629,6 +1629,7 @@ do_start_initialization ()
>        || gdbpy_initialize_frames () < 0
>        || gdbpy_initialize_commands () < 0
>        || gdbpy_initialize_record () < 0
> +      || gdbpy_initialize_btrace () < 0
>        || gdbpy_initialize_symbols () < 0
>        || gdbpy_initialize_symtabs () < 0
>        || gdbpy_initialize_blocks () < 0

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 8/9] python: Add tests for record Python bindings
       [not found] ` <1486989450-11313-9-git-send-email-tim.wiederhake@intel.com>
@ 2017-02-13 17:17   ` Doug Evans
  0 siblings, 0 replies; 21+ messages in thread
From: Doug Evans @ 2017-02-13 17:17 UTC (permalink / raw)
  To: Tim Wiederhake; +Cc: gdb-patches, markus.t.metzger, palves

Tim Wiederhake <tim.wiederhake@intel.com> writes:
> 2017-02-13  Tim Wiederhake  <tim.wiederhake@intel.com>
>
> gdb/testsuite/ChangeLog:
>
>         * gdb.python/py-record-btrace.c, gdb.python/py-record-btrace.exp,
> 	gdb.python/py-record-full.c, gdb.python/py-record-full.exp: New file.

LGTM

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 0/9] Python bindings for btrace recordings
  2017-02-13 12:38 [PATCH v6 0/9] Python bindings for btrace recordings Tim Wiederhake
                   ` (8 preceding siblings ...)
       [not found] ` <1486989450-11313-9-git-send-email-tim.wiederhake@intel.com>
@ 2017-02-13 17:18 ` Doug Evans
  2017-02-14 10:20   ` Wiederhake, Tim
  9 siblings, 1 reply; 21+ messages in thread
From: Doug Evans @ 2017-02-13 17:18 UTC (permalink / raw)
  To: Tim Wiederhake; +Cc: gdb-patches, markus.t.metzger, palves

Tim Wiederhake <tim.wiederhake@intel.com> writes:
> Hello everyone!
>
> This patch series adds Python bindings for btrace recordings.
>
> V1 of this series can be found here:
> https://sourceware.org/ml/gdb-patches/2016-10/msg00733.html
>
> V2 of this series can be found here:
> https://sourceware.org/ml/gdb-patches/2016-11/msg00084.html
>
> V3 of this series can be found here:
> https://sourceware.org/ml/gdb-patches/2016-11/msg00605.html
>
> V4 of this series can be found here:
> https://sourceware.org/ml/gdb-patches/2017-01/msg00044.html
>
> V5 of this series can be found here:
> https://sourceware.org/ml/gdb-patches/2017-01/msg00616.html
>
> Changes from V5:
>  - Skip gdb.python/py-record-full.exp if recording is not supported.
>  - s/symbol/SAL/ in comment in py-record-btrace.c.
>  - Replaced mem_fileopen by string_file in py-record-btrace.c.
>  - Simplified btpy_list_count and added comment in py-record-btrace.c.
>  - Removed unused variable in recpy_bt_goto (leftover from previous refactoring)
>    in py-record-btrace.c.
>  - Moved opening curly braces to new line in py-record-btrace.c.
>
> The documentation parts are already approved by Eli.
>
> Is this good to go?

LGTM

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH v6 0/9] Python bindings for btrace recordings
  2017-02-13 17:18 ` [PATCH v6 0/9] Python bindings for btrace recordings Doug Evans
@ 2017-02-14 10:20   ` Wiederhake, Tim
  2017-02-14 16:22     ` Doug Evans
  0 siblings, 1 reply; 21+ messages in thread
From: Wiederhake, Tim @ 2017-02-14 10:20 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches, Metzger, Markus T, palves

Hi all,

> > Is this good to go?
> LGTM

... And in return I made a mess. What should have been a "git push --dry-run" to prepare for pushing this, ended up as an actual push. The commits now miss the actual ChangeLog entries and contain some "Change-Id" lines. Before I start fiddeling and probably make things worse, could you tell me how to properly resolve this?

Thanks,
Tim

> -----Original Message-----
> From: Doug Evans [mailto:xdje42@gmail.com]
> Sent: Monday, February 13, 2017 6:18 PM
> To: Wiederhake, Tim <tim.wiederhake@intel.com>
> Cc: gdb-patches@sourceware.org; Metzger, Markus T
> <markus.t.metzger@intel.com>; palves@redhat.com
> Subject: Re: [PATCH v6 0/9] Python bindings for btrace recordings
> 
> Tim Wiederhake <tim.wiederhake@intel.com> writes:
> > Hello everyone!
> >
> > This patch series adds Python bindings for btrace recordings.
> >
> > V1 of this series can be found here:
> > https://sourceware.org/ml/gdb-patches/2016-10/msg00733.html
> >
> > V2 of this series can be found here:
> > https://sourceware.org/ml/gdb-patches/2016-11/msg00084.html
> >
> > V3 of this series can be found here:
> > https://sourceware.org/ml/gdb-patches/2016-11/msg00605.html
> >
> > V4 of this series can be found here:
> > https://sourceware.org/ml/gdb-patches/2017-01/msg00044.html
> >
> > V5 of this series can be found here:
> > https://sourceware.org/ml/gdb-patches/2017-01/msg00616.html
> >
> > Changes from V5:
> >  - Skip gdb.python/py-record-full.exp if recording is not supported.
> >  - s/symbol/SAL/ in comment in py-record-btrace.c.
> >  - Replaced mem_fileopen by string_file in py-record-btrace.c.
> >  - Simplified btpy_list_count and added comment in py-record-btrace.c.
> >  - Removed unused variable in recpy_bt_goto (leftover from previous
> refactoring)
> >    in py-record-btrace.c.
> >  - Moved opening curly braces to new line in py-record-btrace.c.
> >
> > The documentation parts are already approved by Eli.
> >
> > Is this good to go?
> 
> LGTM
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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 0/9] Python bindings for btrace recordings
  2017-02-14 10:20   ` Wiederhake, Tim
@ 2017-02-14 16:22     ` Doug Evans
  2017-02-15  7:35       ` Wiederhake, Tim
  0 siblings, 1 reply; 21+ messages in thread
From: Doug Evans @ 2017-02-14 16:22 UTC (permalink / raw)
  To: Wiederhake, Tim; +Cc: gdb-patches, Metzger, Markus T, palves

On Tue, Feb 14, 2017 at 2:18 AM, Wiederhake, Tim
<tim.wiederhake@intel.com> wrote:
> Hi all,
>
>> > Is this good to go?
>> LGTM
>
> ... And in return I made a mess. What should have been a "git push --dry-run" to prepare for pushing this, ended up as an actual push. The commits now miss the actual ChangeLog entries and contain some "Change-Id" lines. Before I start fiddeling and probably make things worse, could you tell me how to properly resolve this?
>
> Thanks,
> Tim

Hi.
I suppose technically the Right thing to do is to revert it all and
recommit with the ChangeLog changes.
But it's not like we never make post-commit corrections to just ChangeLogs.
Perhaps not on this scale, but I'm totally ok with just ignoring the
Change-Id lines and adding the ChangeLog entries as a separate patch.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH v6 0/9] Python bindings for btrace recordings
  2017-02-14 16:22     ` Doug Evans
@ 2017-02-15  7:35       ` Wiederhake, Tim
  0 siblings, 0 replies; 21+ messages in thread
From: Wiederhake, Tim @ 2017-02-15  7:35 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches, Metzger, Markus T, palves

Hi Doug,

thanks for your answer. I just pushed a commit with the missing ChangeLogs as the less intrusive solution.
To make sure that this doesn't happen again, I set up a pre-push script that will yell at me if I forget to include ChangeLogs or have any lines in the commit message that are not supposed to be there.

Regards,
Tim
> -----Original Message-----
> From: Doug Evans [mailto:xdje42@gmail.com]
> Sent: Tuesday, February 14, 2017 5:22 PM
> To: Wiederhake, Tim <tim.wiederhake@intel.com>
> Cc: gdb-patches@sourceware.org; Metzger, Markus T
> <markus.t.metzger@intel.com>; palves@redhat.com
> Subject: Re: [PATCH v6 0/9] Python bindings for btrace recordings
> 
> On Tue, Feb 14, 2017 at 2:18 AM, Wiederhake, Tim
> <tim.wiederhake@intel.com> wrote:
> > Hi all,
> >
> >> > Is this good to go?
> >> LGTM
> >
> > ... And in return I made a mess. What should have been a "git push --dry-
> run" to prepare for pushing this, ended up as an actual push. The commits
> now miss the actual ChangeLog entries and contain some "Change-Id" lines.
> Before I start fiddeling and probably make things worse, could you tell me
> how to properly resolve this?
> >
> > Thanks,
> > Tim
> 
> Hi.
> I suppose technically the Right thing to do is to revert it all and recommit with
> the ChangeLog changes.
> But it's not like we never make post-commit corrections to just ChangeLogs.
> Perhaps not on this scale, but I'm totally ok with just ignoring the Change-Id
> lines and adding the ChangeLog entries as a separate patch.
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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 9/9] Add documentation for new record Python bindings.
  2017-02-13 12:38 ` [PATCH v6 9/9] Add documentation for new record Python bindings Tim Wiederhake
  2017-02-13 14:48   ` Eli Zaretskii
@ 2017-03-03 11:10   ` Yao Qi
  2017-03-06  8:56     ` Wiederhake, Tim
  1 sibling, 1 reply; 21+ messages in thread
From: Yao Qi @ 2017-03-03 11:10 UTC (permalink / raw)
  To: Tim Wiederhake; +Cc: gdb-patches, markus.t.metzger, palves, xdje42

Tim Wiederhake <tim.wiederhake@intel.com> writes:

Hi Tim,
I read them again this morning, and have a design question in general,
why are they specific to btrace?

> +The attributes and methods of instruction objects depend on the current
> +recording method.  Currently, only btrace instructions are supported.
> +
> +A @code{gdb.BtraceInstruction} object has the following attributes:
> +
> +@defvar BtraceInstruction.number
> +An integer identifying this instruction.  @var{number} corresponds to
> +the numbers seen in @code{record instruction-history}
> +(@pxref{Process Record and Replay}).
> +@end defvar
> +

Why is it btrace specific?  Instructions in history of any record
methods can have a number or id, isn't?

> +@defvar BtraceInstruction.error
> +An integer identifying the error code for gaps in the history.
> +@code{None} for regular instructions.
> +@end defvar

> +
> +@defvar BtraceInstruction.sal
> +A @code{gdb.Symtab_and_line} object representing the associated symtab
> +and line of this instruction.  May be @code{None} if the instruction
> +is a gap.
> +@end defvar
> +
> +@defvar BtraceInstruction.pc
> +An integer representing this instruction's address.  May be @code{None}
> +if the instruction is a gap or the debug symbols could not be read.
> +@end defvar

Again, is it btrace specific?  Any instruction will have an address,
regardless of debug symbol.

Secondly, we have BtraceInstruction.pc, do we still need
BtraceInstruction.sal?  The sal can be got from other python api,
although these api may not exist.

Can add a base class like RecordInstruction, which have some basic
attributes for all record methods, and BtraceInstruction extends it to
add more btrace specific attributes.

> +
> +@defvar BtraceInstruction.data
> +A buffer with the raw instruction data.  May be @code{None} if the
> +instruction is a gap.
> +@end defvar
> +
> +@defvar BtraceInstruction.decoded
> +A human readable string with the disassembled instruction.  Contains the
> +error message for gaps.
> +@end defvar
> +
> +@defvar BtraceInstruction.size
> +The size of the instruction in bytes.  Will be @code{None} if the
> +instruction is a gap.
> +@end defvar
> +
> +@defvar BtraceInstruction.is_speculative
> +A boolean indicating whether the instruction was executed
> +speculatively.  Will be @code{None} for gaps.
> +@end defvar

Why are these four attributes about Btrace or even record?  To me, they
are about instruction decode or disassembly.

> +
> +The attributes and methods of function call objects depend on the
> +current recording format.  Currently, only btrace function calls are
> +supported.
> +
> +A @code{gdb.BtraceFunctionCall} object has the following attributes:
> +
> +@defvar BtraceFunctionCall.number
> +An integer identifying this function call.  @var{number} corresponds to
> +the numbers seen in @code{record function-call-history}
> +(@pxref{Process Record and Replay}).
> +@end defvar
> +
> +@defvar BtraceFunctionCall.symbol
> +A @code{gdb.Symbol} object representing the associated symbol.  May be
> +@code{None} if the function call is a gap or the debug symbols could
> +not be read.
> +@end defvar
> +
> +@defvar BtraceFunctionCall.level
> +An integer representing the function call's stack level.  May be
> +@code{None} if the function call is a gap.
> +@end defvar
> +
> +@defvar BtraceFunctionCall.instructions
> +A list of @code{gdb.BtraceInstruction} objects associated with this function
> +call.
> +@end defvar
> +
> +@defvar BtraceFunctionCall.up
> +A @code{gdb.BtraceFunctionCall} object representing the caller's
> +function segment.  If the call has not been recorded, this will be the
> +function segment to which control returns.  If neither the call nor the
> +return have been recorded, this will be @code{None}.
> +@end defvar
> +
> +@defvar BtraceFunctionCall.prev_sibling
> +A @code{gdb.BtraceFunctionCall} object representing the previous
> +segment of this function call.  May be @code{None}.
> +@end defvar
> +
> +@defvar BtraceFunctionCall.next_sibling
> +A @code{gdb.BtraceFunctionCall} object representing the next segment of
> +this function call.  May be @code{None}.
> +@end defvar
> +

Sorry, I don't understand BtraceFunctionCall, can you give an example?
BtraceFunctionCall.symbol and .level indicates it represents a function
call rather than a function.  In the following example,

f2 () { f1 (); }
f3 () { f1 (); }

main () { f2 (); f3 (); }

Here are my understandings,

 - There are two BtraceFunctionCall objects bf1 and bf2 for f1, the first
   one, bf1, is about the call in f2 and the second one, bf2, is about the
   call in f3.
 - bf2.up is about the call in f2, and bf3.up is about the call in f3.
 - bf2.next_sibling == bf3 and bf3.prev_sibling == b2

Are they correct?

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH v6 9/9] Add documentation for new record Python bindings.
  2017-03-03 11:10   ` Yao Qi
@ 2017-03-06  8:56     ` Wiederhake, Tim
  2017-03-07 11:53       ` Yao Qi
  0 siblings, 1 reply; 21+ messages in thread
From: Wiederhake, Tim @ 2017-03-06  8:56 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches, Metzger, Markus T, palves, xdje42

Hi Yao,

> I read them again this morning, and have a design question in general, why
> are they specific to btrace?
> (...)
> Can add a base class like RecordInstruction, which have some basic attributes
> for all record methods, and BtraceInstruction extends it to add more btrace
> specific attributes.

Unfortunately, the interfaces to the "full" and "btrace" recording differ quite
much. So even for "common" attributes, the code for the base class instruction
and function segment objects would have to "if (method == ...)" through all
recording methods, cluttering it, making it harder to maintain and potentially
cause trouble if we introduce another recording format in the future when
functions in the base class suddenly are not implemented for all methods
anymore.

My idea was to make use of Python's philosophy of duck-typing. Having specific
(C wrapper) objects for each recording method solves the problem of how to
obtain the requested data from the recording "back end". If we name respective
functions and attributes the same in these classes, we "quack like a duck" and
create the same common interface for the user from Python's perspective.

> Sorry, I don't understand BtraceFunctionCall, can you give an example?
> BtraceFunctionCall.symbol and .level indicates it represents a function call
> rather than a function.  In the following example,
> 
> f2 () { f1 (); }
> f3 () { f1 (); }
> 
> main () { f2 (); f3 (); }
>
> (...)

Your example creates nine function segments as can be seen in
`record function-call-history /lic`:
1	main	inst 1,2	at 
2	  f2	inst 3,6	at 
3	    f1	inst 7,11	at 
4	  f2	inst 12,14	at 
5	main	inst 15,16	at 
6	  f3	inst 17,20	at 
7	    f1	inst 21,25	at 
8	  f3	inst 26,28	at 
9	main	inst 29,30	at 

The objects in (Python) gdb.current_recording().function_call_history represent
one line in that list each. That is, each time execution flow returns to a
function a new segment for this function call is created. The segments that
belong to the same function call (i.e. #1 -> #5 -> #9 or #2 -> #4 or #6 -> #8)
are linked via the prev_sibling and next_sibling attribute of the objects.

"up" is the link to the caller or returned-to function call segment. For #2 and
#4 in the example, this would be "#1".

I hope I was able to clear things up,
Tim

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Yao Qi
> Sent: Friday, March 3, 2017 12:10 PM
> To: Wiederhake, Tim <tim.wiederhake@intel.com>
> Cc: gdb-patches@sourceware.org; Metzger, Markus T
> <markus.t.metzger@intel.com>; palves@redhat.com; xdje42@gmail.com
> Subject: Re: [PATCH v6 9/9] Add documentation for new record Python
> bindings.
> 
> Tim Wiederhake <tim.wiederhake@intel.com> writes:
> 
> Hi Tim,
> I read them again this morning, and have a design question in general, why
> are they specific to btrace?
> 
> > +The attributes and methods of instruction objects depend on the
> > +current recording method.  Currently, only btrace instructions are
> supported.
> > +
> > +A @code{gdb.BtraceInstruction} object has the following attributes:
> > +
> > +@defvar BtraceInstruction.number
> > +An integer identifying this instruction.  @var{number} corresponds to
> > +the numbers seen in @code{record instruction-history} (@pxref{Process
> > +Record and Replay}).
> > +@end defvar
> > +
> 
> Why is it btrace specific?  Instructions in history of any record methods can
> have a number or id, isn't?
> 
> > +@defvar BtraceInstruction.error
> > +An integer identifying the error code for gaps in the history.
> > +@code{None} for regular instructions.
> > +@end defvar
> 
> > +
> > +@defvar BtraceInstruction.sal
> > +A @code{gdb.Symtab_and_line} object representing the associated
> > +symtab and line of this instruction.  May be @code{None} if the
> > +instruction is a gap.
> > +@end defvar
> > +
> > +@defvar BtraceInstruction.pc
> > +An integer representing this instruction's address.  May be
> > +@code{None} if the instruction is a gap or the debug symbols could not be
> read.
> > +@end defvar
> 
> Again, is it btrace specific?  Any instruction will have an address, regardless of
> debug symbol.
> 
> Secondly, we have BtraceInstruction.pc, do we still need
> BtraceInstruction.sal?  The sal can be got from other python api, although
> these api may not exist.
> 
> Can add a base class like RecordInstruction, which have some basic attributes
> for all record methods, and BtraceInstruction extends it to add more btrace
> specific attributes.
> 
> > +
> > +@defvar BtraceInstruction.data
> > +A buffer with the raw instruction data.  May be @code{None} if the
> > +instruction is a gap.
> > +@end defvar
> > +
> > +@defvar BtraceInstruction.decoded
> > +A human readable string with the disassembled instruction.  Contains
> > +the error message for gaps.
> > +@end defvar
> > +
> > +@defvar BtraceInstruction.size
> > +The size of the instruction in bytes.  Will be @code{None} if the
> > +instruction is a gap.
> > +@end defvar
> > +
> > +@defvar BtraceInstruction.is_speculative A boolean indicating whether
> > +the instruction was executed speculatively.  Will be @code{None} for
> > +gaps.
> > +@end defvar
> 
> Why are these four attributes about Btrace or even record?  To me, they are
> about instruction decode or disassembly.
> 
> > +
> > +The attributes and methods of function call objects depend on the
> > +current recording format.  Currently, only btrace function calls are
> > +supported.
> > +
> > +A @code{gdb.BtraceFunctionCall} object has the following attributes:
> > +
> > +@defvar BtraceFunctionCall.number
> > +An integer identifying this function call.  @var{number} corresponds
> > +to the numbers seen in @code{record function-call-history}
> > +(@pxref{Process Record and Replay}).
> > +@end defvar
> > +
> > +@defvar BtraceFunctionCall.symbol
> > +A @code{gdb.Symbol} object representing the associated symbol.  May
> > +be @code{None} if the function call is a gap or the debug symbols
> > +could not be read.
> > +@end defvar
> > +
> > +@defvar BtraceFunctionCall.level
> > +An integer representing the function call's stack level.  May be
> > +@code{None} if the function call is a gap.
> > +@end defvar
> > +
> > +@defvar BtraceFunctionCall.instructions A list of
> > +@code{gdb.BtraceInstruction} objects associated with this function
> > +call.
> > +@end defvar
> > +
> > +@defvar BtraceFunctionCall.up
> > +A @code{gdb.BtraceFunctionCall} object representing the caller's
> > +function segment.  If the call has not been recorded, this will be
> > +the function segment to which control returns.  If neither the call
> > +nor the return have been recorded, this will be @code{None}.
> > +@end defvar
> > +
> > +@defvar BtraceFunctionCall.prev_sibling A
> > +@code{gdb.BtraceFunctionCall} object representing the previous
> > +segment of this function call.  May be @code{None}.
> > +@end defvar
> > +
> > +@defvar BtraceFunctionCall.next_sibling A
> > +@code{gdb.BtraceFunctionCall} object representing the next segment of
> > +this function call.  May be @code{None}.
> > +@end defvar
> > +
> 
> Sorry, I don't understand BtraceFunctionCall, can you give an example?
> BtraceFunctionCall.symbol and .level indicates it represents a function call
> rather than a function.  In the following example,
> 
> f2 () { f1 (); }
> f3 () { f1 (); }
> 
> main () { f2 (); f3 (); }
> 
> Here are my understandings,
> 
>  - There are two BtraceFunctionCall objects bf1 and bf2 for f1, the first
>    one, bf1, is about the call in f2 and the second one, bf2, is about the
>    call in f3.
>  - bf2.up is about the call in f2, and bf3.up is about the call in f3.
>  - bf2.next_sibling == bf3 and bf3.prev_sibling == b2
> 
> Are they correct?
> 
> --
> Yao (齐尧)
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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 9/9] Add documentation for new record Python bindings.
  2017-03-06  8:56     ` Wiederhake, Tim
@ 2017-03-07 11:53       ` Yao Qi
  2017-03-07 17:23         ` Wiederhake, Tim
  0 siblings, 1 reply; 21+ messages in thread
From: Yao Qi @ 2017-03-07 11:53 UTC (permalink / raw)
  To: Wiederhake, Tim; +Cc: gdb-patches, Metzger, Markus T, palves, xdje42

"Wiederhake, Tim" <tim.wiederhake@intel.com> writes:

>> I read them again this morning, and have a design question in general, why
>> are they specific to btrace?
>> (...)
>> Can add a base class like RecordInstruction, which have some basic attributes
>> for all record methods, and BtraceInstruction extends it to add more btrace
>> specific attributes.
>
> Unfortunately, the interfaces to the "full" and "btrace" recording differ quite
> much. So even for "common" attributes, the code for the base class instruction
> and function segment objects would have to "if (method == ...)" through all
> recording methods, cluttering it, making it harder to maintain and
> potentially

Do you have a concrete example?  If you have to "if (method == ...)" in
python code (outside of gdb), there is something wrong on design in
python code.  I believe there are some design patterns for this kind of
issue "if (method == ...)".

> cause trouble if we introduce another recording format in the future when
> functions in the base class suddenly are not implemented for all methods
> anymore.
>
> My idea was to make use of Python's philosophy of duck-typing. Having specific
> (C wrapper) objects for each recording method solves the problem of how to
> obtain the requested data from the recording "back end". If we name respective
> functions and attributes the same in these classes, we "quack like a duck" and
> create the same common interface for the user from Python's perspective.
>

but you can't have a common interface because the python api is btrace
specific.  Suppose we have gdb.FullInstruction for "full", what
attributes do we have? .number and .pc maybe?  With the python api
design like this, how can I write a python code to trace data regardless
of which method is being used?  Ideal design in my mind is like this (I
hope concrete example like this helps discussion)

class RecordInstruction(object)

      # fields .pc and .number

class FullRecordInstruction(RecordInstruction)

class BtraceInstruction(RecordInstruction)

      # field .error

Client python code can get a list of RecordInstruction to process,
without being aware which method is used.

Use gdb.find_pc_line (pc) to get gdb.Symtab_and_line, so we don't need
to have BtraceInstruction.sal.

BtraceInstruction .data, .decoded, .size, and .is_speculative, shouldn't
be in BtraceInstruction.  They are about instruction.  We need to put
them into a separate class, like gdb.Instruction, and add some function
to get gdb.Instruction from pc.

I'll give comments to BtraceFunctionCall later.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH v6 9/9] Add documentation for new record Python bindings.
  2017-03-07 11:53       ` Yao Qi
@ 2017-03-07 17:23         ` Wiederhake, Tim
  2017-03-17 16:50           ` Yao Qi
  0 siblings, 1 reply; 21+ messages in thread
From: Wiederhake, Tim @ 2017-03-07 17:23 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches, Metzger, Markus T, palves, xdje42

Hi Yao!

I believe that there is a misunderstanding.  Let me pick up the idea of giving
concrete examples.  With the current implementation, a simple use case would
look like this:

(gdb) record btrace
[RECORD SOME PROGRAM FLOW]
(gdb) python-interactive
>>> recorded_instructions = gdb.current_recording().instruction_history
>>> for instruction in recorded_instructions:
...   print(i.number, hex(i.pc), i.sal)
...
[OUTPUT]

For the sake of the argument, let's assume that "number", "pc" and "sal" are not
unique to btrace and that support for "full" is implemented in the way that I
suggested: "FullInstruction" has some functions / data members that happen to
have the same names as their "BtraceInstruction" counterparts but these python
classes are not related in any way in terms of class inheritance. The user now
wants to record with method "full" and "instruction_history" returns not a list
of BtraceInstruction objects but a list of FullInstruction objects; the input
looks like this:

(gdb) record full
[RECORD SOME PROGRAM FLOW]
(gdb) python-interactive
>>> recorded_instructions = gdb.current_recording().instruction_history
>>> for instruction in recorded_instructions:
...   print(i.number, hex(i.pc), i.sal)
...
[OUTPUT]

It is exactly the same. Now if we were to have some Python instruction base
class, let's call it "RecordInstruction", and two Python sub classes,
"BtraceInstruction" and "FullInstruction", the example would still look exactly
like this:

(gdb) record [full or btrace]
[RECORD SOME PROGRAM FLOW]
(gdb) python-interactive
>>> recorded_instructions = gdb.current_recording().instruction_history
>>> for instruction in recorded_instructions:
...   print(i.number, hex(i.pc), i.sal)
...
[OUTPUT]

The user has no benefit from the existance of a python base class. They never
receive an instance of this base class to see what the common interface is. The
pythonic way -- as far as I understand it -- is to rely on the existance of
functions / data members of the objects, not on the actual type of the object.
That is what I meant with "duck typing". Both BtraceInstruction and
FullInstruction behave in the same way for a couple of functions / data members,
they "quack like a duck" and therefore "are a duck", no matter that they are
instances of two unrelated classes.

What exactly forms this common interface has yet to defined and codified in the
documentation. But this is in my opinion the beauty of this approach: All
functions / data members of BtraceInstruction that are not part of this common
interface automatically are "just there" and "additional functionality".

The approach of having a common base class /would/ have some benefit, if all
three of "RecordInstruction", "BtraceInstruction" and "FullInstruction" were
native Python classes: Less code in BtraceInstruction and FullInstruction. But
BtraceInstruction and eventually FullInstruction are "Built-In"s from Python's
perspective. That is, not real Python classes but some C code that interfaces
with the Python interpreter.

Let's assume that "number", "pc" and "sal" were to be the common interface and
therefore data members of "RecordInstruction". The C code for this Python class
now needs to know which recording is currently active, since there is no

current_target.to_get_pc_for_recorded_instruction_by_number(struct target_ops*,
							unsigned int);

So the C code for the common "RecordInstruction.pc" would look something like
this:

switch (current_recording_method)
  {
    case METHOD_BRACE:
	/* BTRACE specific code to retrieve PC for a given recorded instruction */
	struct btrace_insn_iterator iter;
	find_insn_by_number(&iter, number_of_the_recorded_instruction);
	struct btrace_insn* insn = btrace_insn_get(&iter);
	return insn->pc;
    case METHOD_FULL:
	/* FULL specific code to retrieve PC for a given recorded instruction */
	....
    default:
	/* throw some python exception */
  }

To add more recording related functions to target_ops is probably not the best
way to go, since we still need some shim to turn the returned C structs into
Python objects and we impose some requirements on the recording targets like
"requires random access to the recorded instruction" and so on. Additionally,
we would still require custom code for a recording method, if that recording
method can provide additional information that we want to make accessible to
the user.

From my point of view, there is no benefit for the user from the common base
class approach. Both cases exhibit the same functionality but the one with the
common base class adds lots of complexity and added code to maintain.

So, no "if (method = ...)" in python code; the manual tells you how write
(python) code that works regardless of the recording method in a yet to be
written and to be decided upon paragraph about the "common interface"; and the
python API itself is actually not at all btrace specific, as one could argue
that all implemented functions and data members are currently just "added
functionality for the btrace case".

I hope I could clear things up and thanks for reading through this,

Tim


> -----Original Message-----
> From: Yao Qi [mailto:qiyaoltc@gmail.com]
> Sent: Tuesday, March 7, 2017 12:54 PM
> To: Wiederhake, Tim <tim.wiederhake@intel.com>
> Cc: gdb-patches@sourceware.org; Metzger, Markus T
> <markus.t.metzger@intel.com>; palves@redhat.com; xdje42@gmail.com
> Subject: Re: [PATCH v6 9/9] Add documentation for new record Python
> bindings.
> 
> "Wiederhake, Tim" <tim.wiederhake@intel.com> writes:
> 
> >> I read them again this morning, and have a design question in
> >> general, why are they specific to btrace?
> >> (...)
> >> Can add a base class like RecordInstruction, which have some basic
> >> attributes for all record methods, and BtraceInstruction extends it
> >> to add more btrace specific attributes.
> >
> > Unfortunately, the interfaces to the "full" and "btrace" recording
> > differ quite much. So even for "common" attributes, the code for the
> > base class instruction and function segment objects would have to "if
> > (method == ...)" through all recording methods, cluttering it, making
> > it harder to maintain and potentially
> 
> Do you have a concrete example?  If you have to "if (method == ...)" in
> python code (outside of gdb), there is something wrong on design in python
> code.  I believe there are some design patterns for this kind of issue "if
> (method == ...)".
> 
> > cause trouble if we introduce another recording format in the future
> > when functions in the base class suddenly are not implemented for all
> > methods anymore.
> >
> > My idea was to make use of Python's philosophy of duck-typing. Having
> > specific (C wrapper) objects for each recording method solves the
> > problem of how to obtain the requested data from the recording "back
> > end". If we name respective functions and attributes the same in these
> > classes, we "quack like a duck" and create the same common interface for
> the user from Python's perspective.
> >
> 
> but you can't have a common interface because the python api is btrace
> specific.  Suppose we have gdb.FullInstruction for "full", what attributes do
> we have? .number and .pc maybe?  With the python api design like this, how
> can I write a python code to trace data regardless of which method is being
> used?  Ideal design in my mind is like this (I hope concrete example like this
> helps discussion)
> 
> class RecordInstruction(object)
> 
>       # fields .pc and .number
> 
> class FullRecordInstruction(RecordInstruction)
> 
> class BtraceInstruction(RecordInstruction)
> 
>       # field .error
> 
> Client python code can get a list of RecordInstruction to process, without
> being aware which method is used.
> 
> Use gdb.find_pc_line (pc) to get gdb.Symtab_and_line, so we don't need to
> have BtraceInstruction.sal.
> 
> BtraceInstruction .data, .decoded, .size, and .is_speculative, shouldn't be in
> BtraceInstruction.  They are about instruction.  We need to put them into a
> separate class, like gdb.Instruction, and add some function to get
> gdb.Instruction from pc.
> 
> I'll give comments to BtraceFunctionCall later.
> 
> --
> Yao (齐尧)
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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 9/9] Add documentation for new record Python bindings.
  2017-03-07 17:23         ` Wiederhake, Tim
@ 2017-03-17 16:50           ` Yao Qi
  0 siblings, 0 replies; 21+ messages in thread
From: Yao Qi @ 2017-03-17 16:50 UTC (permalink / raw)
  To: Wiederhake, Tim; +Cc: gdb-patches, Metzger, Markus T, palves, xdje42

"Wiederhake, Tim" <tim.wiederhake@intel.com> writes:

> For the sake of the argument, let's assume that "number", "pc" and "sal" are not
> unique to btrace and that support for "full" is implemented in the way that I
> suggested: "FullInstruction" has some functions / data members that happen to
> have the same names as their "BtraceInstruction" counterparts but these python
> classes are not related in any way in terms of class inheritance. The user now
> wants to record with method "full" and "instruction_history" returns not a list
> of BtraceInstruction objects but a list of FullInstruction objects; the input
> looks like this:

That is not the use case of OOP.  I, as a user, write a python script to
get instructions (which have attributes "number", "pc", and "sal"), I
don't have to know which record method is used.  Just start record,
want gdb gives me a list of instructions, and iterate them to parse
these attributes of each instruction.

>
> (gdb) record full
> [RECORD SOME PROGRAM FLOW]
> (gdb) python-interactive
>>>> recorded_instructions = gdb.current_recording().instruction_history
>>>> for instruction in recorded_instructions:
> ...   print(i.number, hex(i.pc), i.sal)
> ...
> [OUTPUT]
>
> It is exactly the same. Now if we were to have some Python instruction base
> class, let's call it "RecordInstruction", and two Python sub classes,
> "BtraceInstruction" and "FullInstruction", the example would still look exactly
> like this:
>
> (gdb) record [full or btrace]
> [RECORD SOME PROGRAM FLOW]
> (gdb) python-interactive
>>>> recorded_instructions = gdb.current_recording().instruction_history
>>>> for instruction in recorded_instructions:
> ...   print(i.number, hex(i.pc), i.sal)
> ...
> [OUTPUT]
>
> The user has no benefit from the existance of a python base class. They never
> receive an instance of this base class to see what the common interface is. The
> pythonic way -- as far as I understand it -- is to rely on the existance of
> functions / data members of the objects, not on the actual type of the
> object.

The benefit is that we can easily enforce the instruction attributes for
new added record methods.  If I want to add "FullInstruction",
"BarInstruction", or "FooInstruction", how can we make sure these
classes have these mandatory or basic attributes "number", "pc", and
"sal" so that the following python script can still be used with new
record format?

recorded_instructions = gdb.current_recording().instruction_history
for instruction in recorded_instructions:
   print(i.number, hex(i.pc), i.sal)

Secondly, the document of these interfaces becomes simpler with class
inheritance.  We only need to document these mandatory attributes in
base class once.  In your current approach, we need to document these
attributes separately in each class.

> That is what I meant with "duck typing". Both BtraceInstruction and
> FullInstruction behave in the same way for a couple of functions / data members,
> they "quack like a duck" and therefore "are a duck", no matter that they are
> instances of two unrelated classes.
>

This is from consumer's point of view.  I am looking at this from the
producer's or interface provider's point of view.

> What exactly forms this common interface has yet to defined and codified in the
> documentation. But this is in my opinion the beauty of this approach: All
> functions / data members of BtraceInstruction that are not part of this common
> interface automatically are "just there" and "additional functionality".
>
> The approach of having a common base class /would/ have some benefit, if all
> three of "RecordInstruction", "BtraceInstruction" and "FullInstruction" were
> native Python classes: Less code in BtraceInstruction and FullInstruction. But
> BtraceInstruction and eventually FullInstruction are "Built-In"s from Python's
> perspective. That is, not real Python classes but some C code that interfaces
> with the Python interpreter.
>
> Let's assume that "number", "pc" and "sal" were to be the common interface and
> therefore data members of "RecordInstruction". The C code for this Python class
> now needs to know which recording is currently active, since there is no
>
> current_target.to_get_pc_for_recorded_instruction_by_number(struct target_ops*,
> 							unsigned int);
>

We can change GDB C code, like target_ops, whenever we want, however, we
can't change external python interface.  I am focusing on the external
python interface now.

>
> From my point of view, there is no benefit for the user from the common base
> class approach. Both cases exhibit the same functionality but the one with the
> common base class adds lots of complexity and added code to maintain.
>
> So, no "if (method = ...)" in python code; the manual tells you how write
> (python) code that works regardless of the recording method in a yet to be
> written and to be decided upon paragraph about the "common interface"; and the
> python API itself is actually not at all btrace specific, as one could argue
> that all implemented functions and data members are currently just "added
> functionality for the btrace case".

I'll do some experimental hacking to help me fully understanding whether
my suggestion is a good idea or bad idea.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2017-03-17 16:50 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-13 12:38 [PATCH v6 0/9] Python bindings for btrace recordings Tim Wiederhake
2017-02-13 12:38 ` [PATCH v6 2/9] btrace: Export btrace_decode_error function Tim Wiederhake
2017-02-13 12:38 ` [PATCH v6 1/9] btrace: Count gaps as one instruction explicitly Tim Wiederhake
2017-02-13 12:38 ` [PATCH v6 5/9] Add method to query current recording method to target_ops Tim Wiederhake
2017-02-13 12:38 ` [PATCH v6 9/9] Add documentation for new record Python bindings Tim Wiederhake
2017-02-13 14:48   ` Eli Zaretskii
2017-03-03 11:10   ` Yao Qi
2017-03-06  8:56     ` Wiederhake, Tim
2017-03-07 11:53       ` Yao Qi
2017-03-07 17:23         ` Wiederhake, Tim
2017-03-17 16:50           ` Yao Qi
2017-02-13 12:38 ` [PATCH v6 3/9] btrace: Use binary search to find instruction Tim Wiederhake
2017-02-13 12:38 ` [PATCH v6 6/9] python: Create Python bindings for record history Tim Wiederhake
2017-02-13 12:38 ` [PATCH v6 4/9] Add record_start and record_stop functions Tim Wiederhake
     [not found] ` <1486989450-11313-8-git-send-email-tim.wiederhake@intel.com>
2017-02-13 17:03   ` [PATCH v6 7/9] python: Implement btrace Python bindings for record history Doug Evans
2017-02-13 17:12   ` Doug Evans
     [not found] ` <1486989450-11313-9-git-send-email-tim.wiederhake@intel.com>
2017-02-13 17:17   ` [PATCH v6 8/9] python: Add tests for record Python bindings Doug Evans
2017-02-13 17:18 ` [PATCH v6 0/9] Python bindings for btrace recordings Doug Evans
2017-02-14 10:20   ` Wiederhake, Tim
2017-02-14 16:22     ` Doug Evans
2017-02-15  7:35       ` Wiederhake, Tim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).