[PATCH 2/3] config, btrace: check for pt_insn_event in libipt

[PATCH 2/3] config, btrace: check for pt_insn_event in libipt

[Markus Metzger]

Version 2 of libipt adds an event system to instruction flow decoders and
deprecates indicating events via flags in struct pt_insn.  Add configuration
checks to determine which version we have.

2017-04-28  Markus Metzger  <markus.t.metzger@intel.com>

gdb/
	* configure.ac: Check for pt_insn_event, struct pt_insn.enabled,
	and struct pt_insn.resynced.
	* configure: Regenerated.
	* config.in: Regenerated.


---
 gdb/config.in    |   9 ++++
 gdb/configure    | 148 ++++++++++++++++++++++++++++++++++---------------------
 gdb/configure.ac |   6 +++
 3 files changed, 106 insertions(+), 57 deletions(-)

diff --git a/gdb/config.in b/gdb/config.in
index 3f8a723..4a40956 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -381,6 +381,9 @@
 /* Define if sys/ptrace.h defines the PT_GETXMMREGS request. */
 #undef HAVE_PT_GETXMMREGS
 
+/* Define to 1 if you have the `pt_insn_event' function. */
+#undef HAVE_PT_INSN_EVENT
+
 /* Define to 1 if you have the `pwrite' function. */
 #undef HAVE_PWRITE
 
@@ -462,6 +465,12 @@
 /* Define to 1 if `struct ptrace_lwpinfo' is a member of `pl_tdname'. */
 #undef HAVE_STRUCT_PTRACE_LWPINFO_PL_TDNAME
 
+/* Define to 1 if `struct pt_insn' is a member of `enabled'. */
+#undef HAVE_STRUCT_PT_INSN_ENABLED
+
+/* Define to 1 if `struct pt_insn' is a member of `resynced'. */
+#undef HAVE_STRUCT_PT_INSN_RESYNCED
+
 /* Define to 1 if your system has struct reg in <machine/reg.h>. */
 #undef HAVE_STRUCT_REG
 
diff --git a/gdb/configure b/gdb/configure
index 9f05b4b..5473c0e 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -2241,6 +2241,63 @@ $as_echo "$ac_res" >&6; }
 
 } # ac_fn_c_check_func
 
+# ac_fn_c_check_member LINENO AGGR MEMBER VAR INCLUDES
+# ----------------------------------------------------
+# Tries to find if the field MEMBER exists in type AGGR, after including
+# INCLUDES, setting cache variable VAR accordingly.
+ac_fn_c_check_member ()
+{
+  as_lineno=${as_lineno-"$1"} as_lineno_stack=as_lineno_stack=$as_lineno_stack
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for $2.$3" >&5
+$as_echo_n "checking for $2.$3... " >&6; }
+if { as_var=$4; eval "test \"\${$as_var+set}\" = set"; }; then :
+  $as_echo_n "(cached) " >&6
+else
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+$5
+int
+main ()
+{
+static $2 ac_aggr;
+if (ac_aggr.$3)
+return 0;
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+  eval "$4=yes"
+else
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+$5
+int
+main ()
+{
+static $2 ac_aggr;
+if (sizeof ac_aggr.$3)
+return 0;
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+  eval "$4=yes"
+else
+  eval "$4=no"
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+fi
+eval ac_res=\$$4
+	       { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_res" >&5
+$as_echo "$ac_res" >&6; }
+  eval $as_lineno_stack; test "x$as_lineno_stack" = x && { as_lineno=; unset as_lineno;}
+
+} # ac_fn_c_check_member
+
 # ac_fn_c_check_type LINENO TYPE VAR INCLUDES
 # -------------------------------------------
 # Tests whether TYPE exists after having included INCLUDES, setting cache
@@ -2339,63 +2396,6 @@ $as_echo "$ac_res" >&6; }
   eval $as_lineno_stack; test "x$as_lineno_stack" = x && { as_lineno=; unset as_lineno;}
 
 } # ac_fn_c_check_decl
-
-# ac_fn_c_check_member LINENO AGGR MEMBER VAR INCLUDES
-# ----------------------------------------------------
-# Tries to find if the field MEMBER exists in type AGGR, after including
-# INCLUDES, setting cache variable VAR accordingly.
-ac_fn_c_check_member ()
-{
-  as_lineno=${as_lineno-"$1"} as_lineno_stack=as_lineno_stack=$as_lineno_stack
-  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for $2.$3" >&5
-$as_echo_n "checking for $2.$3... " >&6; }
-if { as_var=$4; eval "test \"\${$as_var+set}\" = set"; }; then :
-  $as_echo_n "(cached) " >&6
-else
-  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
-/* end confdefs.h.  */
-$5
-int
-main ()
-{
-static $2 ac_aggr;
-if (ac_aggr.$3)
-return 0;
-  ;
-  return 0;
-}
-_ACEOF
-if ac_fn_c_try_compile "$LINENO"; then :
-  eval "$4=yes"
-else
-  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
-/* end confdefs.h.  */
-$5
-int
-main ()
-{
-static $2 ac_aggr;
-if (sizeof ac_aggr.$3)
-return 0;
-  ;
-  return 0;
-}
-_ACEOF
-if ac_fn_c_try_compile "$LINENO"; then :
-  eval "$4=yes"
-else
-  eval "$4=no"
-fi
-rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
-fi
-rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
-fi
-eval ac_res=\$$4
-	       { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_res" >&5
-$as_echo "$ac_res" >&6; }
-  eval $as_lineno_stack; test "x$as_lineno_stack" = x && { as_lineno=; unset as_lineno;}
-
-} # ac_fn_c_check_member
 cat >config.log <<_ACEOF
 This file contains any messages produced by compilers while
 running configure, to aid debugging if configure makes a mistake.
@@ -11433,6 +11433,40 @@ $as_echo "$LIBIPT" >&6; }
       { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: libipt is missing or unusable; some features may be unavailable." >&5
 $as_echo "$as_me: WARNING: libipt is missing or unusable; some features may be unavailable." >&2;}
     fi
+  else
+    save_LIBS=$LIBS
+    LIBS="$LIBS $LIBIPT"
+    for ac_func in pt_insn_event
+do :
+  ac_fn_c_check_func "$LINENO" "pt_insn_event" "ac_cv_func_pt_insn_event"
+if test "x$ac_cv_func_pt_insn_event" = x""yes; then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE_PT_INSN_EVENT 1
+_ACEOF
+
+fi
+done
+
+    ac_fn_c_check_member "$LINENO" "struct pt_insn" "enabled" "ac_cv_member_struct_pt_insn_enabled" "$ac_includes_default"
+if test "x$ac_cv_member_struct_pt_insn_enabled" = x""yes; then :
+
+cat >>confdefs.h <<_ACEOF
+#define HAVE_STRUCT_PT_INSN_ENABLED 1
+_ACEOF
+
+
+fi
+ac_fn_c_check_member "$LINENO" "struct pt_insn" "resynced" "ac_cv_member_struct_pt_insn_resynced" "$ac_includes_default"
+if test "x$ac_cv_member_struct_pt_insn_resynced" = x""yes; then :
+
+cat >>confdefs.h <<_ACEOF
+#define HAVE_STRUCT_PT_INSN_RESYNCED 1
+_ACEOF
+
+
+fi
+
+    LIBS=$save_LIBS
   fi
 fi
 
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 50f6f59..b4bf4ae 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -1272,6 +1272,12 @@ else
     else
       AC_MSG_WARN([libipt is missing or unusable; some features may be unavailable.])
     fi
+  else
+    save_LIBS=$LIBS
+    LIBS="$LIBS $LIBIPT"
+    AC_CHECK_FUNCS(pt_insn_event)
+    AC_CHECK_MEMBERS([struct pt_insn.enabled, struct pt_insn.resynced])
+    LIBS=$save_LIBS
   fi
 fi
 
-- 
1.8.3.1

[PATCH 3/3] btrace: support decoder events

[Markus Metzger]

Newer versions of libipt support instruction flow decoder events instead of
indicating those events with flags in struct pt_insn.  Add support for them in
GDB.

2017-04-28  Markus Metzger  <markus.t.metzger@intel.com>

gdb/
	* btrace.c (handle_pt_insn_events): New.
	(ftrace_add_pt): Call handle_pt_insn_events.  Rename ERRCODE into
	STATUS.  Split into this and ...
	(handle_pt_insn_event_flags): ... this.


---
 gdb/btrace.c | 174 +++++++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 127 insertions(+), 47 deletions(-)

diff --git a/gdb/btrace.c b/gdb/btrace.c
index 6780e22..21d0be9 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -1132,6 +1132,116 @@ pt_btrace_insn (const struct pt_insn &insn)
 	  pt_btrace_insn_flags (insn)};
 }
 
+/* Handle instruction decode events (libipt-v2).  */
+
+static int
+handle_pt_insn_events (struct pt_insn_decoder *decoder,
+		       struct btrace_function **pbegin,
+		       struct btrace_function **pend,
+		       VEC (bfun_s) **gaps, int status)
+{
+#if defined (HAVE_PT_INSN_EVENT)
+  while (status & pts_event_pending)
+    {
+      struct pt_event event;
+      uint64_t offset;
+
+      status = pt_insn_event (decoder, &event, sizeof (event));
+      if (status < 0)
+	break;
+
+      switch (event.type)
+	{
+	default:
+	  break;
+
+	case ptev_enabled:
+	  if (*pbegin != NULL && event.variant.enabled.resumed == 0)
+	    {
+	      *pend = ftrace_new_gap (*pend, BDE_PT_DISABLED);
+	      if (*pbegin == NULL)
+		*pbegin = *pend;
+
+	      VEC_safe_push (bfun_s, *gaps, *pend);
+
+	      pt_insn_get_offset (decoder, &offset);
+
+	      warning (_("Non-contiguous trace at instruction %u (offset = 0x%"
+			 PRIx64 ")."), (*pend)->insn_offset - 1, offset);
+	    }
+
+	  break;
+
+	case ptev_overflow:
+	  *pend = ftrace_new_gap (*pend, BDE_PT_OVERFLOW);
+	  if (*pbegin == NULL)
+	    *pbegin = *pend;
+
+	  VEC_safe_push (bfun_s, *gaps, *pend);
+
+	  pt_insn_get_offset (decoder, &offset);
+
+	  warning (_("Overflow at instruction %u (offset = 0x%" PRIx64 ")."),
+		   (*pend)->insn_offset - 1, offset);
+
+	  break;
+	}
+    }
+#endif /* defined (HAVE_PT_INSN_EVENT) */
+
+  return status;
+}
+
+/* Handle events indicated by flags in INSN (libipt-v1).  */
+
+static void
+handle_pt_insn_event_flags (struct pt_insn_decoder *decoder,
+			    const struct pt_insn *insn,
+			    struct btrace_function **pbegin,
+			    struct btrace_function **pend,
+			    VEC (bfun_s) **gaps)
+{
+#if defined (HAVE_STRUCT_PT_INSN_ENABLED)
+  /* Tracing is disabled and re-enabled each time we enter the kernel.  Most
+     times, we continue from the same instruction we stopped before.  This is
+     indicated via the RESUMED instruction flag.  The ENABLED instruction flag
+     means that we continued from some other instruction.  Indicate this as a
+     trace gap except when tracing just started.  */
+  if (insn.enabled && (*begin != NULL))
+    {
+      uint64_t offset;
+
+      *end = ftrace_new_gap (*end, BDE_PT_DISABLED);
+
+      VEC_safe_push (bfun_s, *gaps, *end);
+
+      pt_insn_get_offset (decoder, &offset);
+
+      warning (_("Non-contiguous trace at instruction %u (offset = 0x%" PRIx64
+		 ", pc = 0x%" PRIx64 ")."), (*end)->insn_offset - 1, offset,
+	       insn.ip);
+    }
+#endif /* defined (HAVE_STRUCT_PT_INSN_ENABLED) */
+
+#if defined (HAVE_STRUCT_PT_INSN_RESYNCED)
+  /* Indicate trace overflows.  */
+  if (insn.resynced)
+    {
+      uint64_t offset;
+
+      *end = ftrace_new_gap (*end, BDE_PT_OVERFLOW);
+      if (*begin == NULL)
+	*begin = *end;
+
+      VEC_safe_push (bfun_s, *gaps, *end);
+
+      pt_insn_get_offset (decoder, &offset);
+
+      warning (_("Overflow at instruction %u (offset = 0x%" PRIx64 ", pc = 0x%"
+		 PRIx64 ")."), (*end)->insn_offset - 1, offset, insn.ip);
+    }
+#endif /* defined (HAVE_STRUCT_PT_INSN_RESYNCED) */
+}
 
 /* Add function branch trace using DECODER.  */
 
@@ -1142,64 +1252,34 @@ ftrace_add_pt (struct pt_insn_decoder *decoder,
 	       VEC (bfun_s) **gaps)
 {
   uint64_t offset;
-  int errcode;
+  int status;
 
   for (;;)
     {
       struct pt_insn insn;
 
-      errcode = pt_insn_sync_forward (decoder);
-      if (errcode < 0)
+      status = pt_insn_sync_forward (decoder);
+      if (status < 0)
 	{
-	  if (errcode != -pte_eos)
+	  if (status != -pte_eos)
 	    warning (_("Failed to synchronize onto the Intel Processor "
-		       "Trace stream: %s."), pt_errstr (pt_errcode (errcode)));
+		       "Trace stream: %s."), pt_errstr (pt_errcode (status)));
 	  break;
 	}
 
       for (;;)
 	{
-	  errcode = pt_insn_next (decoder, &insn, sizeof(insn));
-	  if (errcode < 0)
+	  /* Handle events from the previous iteration or synchronization.  */
+	  status = handle_pt_insn_events (decoder, begin, end, gaps, status);
+	  if (status < 0)
 	    break;
 
-	  /* Look for gaps in the trace - unless we're at the beginning.  */
-	  if (*begin != NULL)
-	    {
-	      /* Tracing is disabled and re-enabled each time we enter the
-		 kernel.  Most times, we continue from the same instruction we
-		 stopped before.  This is indicated via the RESUMED instruction
-		 flag.  The ENABLED instruction flag means that we continued
-		 from some other instruction.  Indicate this as a trace gap.  */
-	      if (insn.enabled)
-		{
-		  *end = ftrace_new_gap (*end, BDE_PT_DISABLED);
-
-		  VEC_safe_push (bfun_s, *gaps, *end);
-
-		  pt_insn_get_offset (decoder, &offset);
-
-		  warning (_("Non-contiguous trace at instruction %u (offset "
-			     "= 0x%" PRIx64 ", pc = 0x%" PRIx64 ")."),
-			   (*end)->insn_offset - 1, offset, insn.ip);
-		}
-	    }
-
-	  /* Indicate trace overflows.  */
-	  if (insn.resynced)
-	    {
-	      *end = ftrace_new_gap (*end, BDE_PT_OVERFLOW);
-	      if (*begin == NULL)
-		*begin = *end;
-
-	      VEC_safe_push (bfun_s, *gaps, *end);
-
-	      pt_insn_get_offset (decoder, &offset);
+	  status = pt_insn_next (decoder, &insn, sizeof(insn));
+	  if (status < 0)
+	    break;
 
-	      warning (_("Overflow at instruction %u (offset = 0x%" PRIx64
-			 ", pc = 0x%" PRIx64 ")."), (*end)->insn_offset - 1,
-		       offset, insn.ip);
-	    }
+	  /* Handle events indicated by flags in INSN.  */
+	  handle_pt_insn_event_flags (decoder, &insn, begin, end, gaps);
 
 	  *end = ftrace_update_function (*end, insn.ip);
 	  if (*begin == NULL)
@@ -1212,11 +1292,11 @@ ftrace_add_pt (struct pt_insn_decoder *decoder,
 	  ftrace_update_insns (*end, &btinsn);
 	}
 
-      if (errcode == -pte_eos)
+      if (status == -pte_eos)
 	break;
 
       /* Indicate the gap in the trace.  */
-      *end = ftrace_new_gap (*end, errcode);
+      *end = ftrace_new_gap (*end, status);
       if (*begin == NULL)
 	*begin = *end;
 
@@ -1225,9 +1305,9 @@ ftrace_add_pt (struct pt_insn_decoder *decoder,
       pt_insn_get_offset (decoder, &offset);
 
       warning (_("Decode error (%d) at instruction %u (offset = 0x%" PRIx64
-		 ", pc = 0x%" PRIx64 "): %s."), errcode,
+		 ", pc = 0x%" PRIx64 "): %s."), status,
 	       (*end)->insn_offset - 1, offset, insn.ip,
-	       pt_errstr (pt_errcode (errcode)));
+	       pt_errstr (pt_errcode (status)));
     }
 }
 
-- 
1.8.3.1

[PATCH 1/3] btrace: work directly on begin and end pointers

[Markus Metzger]

In ftrace_add_pt, we store copies of *PBEGIN and *PEND in local variables BEGIN
and END respectively.  The intent was to help the compiler figure out that BEGIN
and END are not changed over external function calls.

This is still true for most calls, yet if we want to allow changing *PBEGIN and
*PEND inside a function, we'd have to reload BEGIN and END after that call.
Stop using local copies to avoid that complication.

2017-04-28  Markus Metzger  <markus.t.metzger@intel.com>

gdb/
	* btrace.c (ftrace_add_pt): Rename arguments PBEGIN and PEND into
	BEGIN and END.  Use them directly.


---
 gdb/btrace.c | 53 +++++++++++++++++++++++------------------------------
 1 file changed, 23 insertions(+), 30 deletions(-)

diff --git a/gdb/btrace.c b/gdb/btrace.c
index 238df0a..6780e22 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -1137,16 +1137,13 @@ pt_btrace_insn (const struct pt_insn &insn)
 
 static void
 ftrace_add_pt (struct pt_insn_decoder *decoder,
-	       struct btrace_function **pbegin,
-	       struct btrace_function **pend, int *plevel,
+	       struct btrace_function **begin,
+	       struct btrace_function **end, int *plevel,
 	       VEC (bfun_s) **gaps)
 {
-  struct btrace_function *begin, *end, *upd;
   uint64_t offset;
   int errcode;
 
-  begin = *pbegin;
-  end = *pend;
   for (;;)
     {
       struct pt_insn insn;
@@ -1167,7 +1164,7 @@ ftrace_add_pt (struct pt_insn_decoder *decoder,
 	    break;
 
 	  /* Look for gaps in the trace - unless we're at the beginning.  */
-	  if (begin != NULL)
+	  if (*begin != NULL)
 	    {
 	      /* Tracing is disabled and re-enabled each time we enter the
 		 kernel.  Most times, we continue from the same instruction we
@@ -1176,65 +1173,61 @@ ftrace_add_pt (struct pt_insn_decoder *decoder,
 		 from some other instruction.  Indicate this as a trace gap.  */
 	      if (insn.enabled)
 		{
-		  *pend = end = ftrace_new_gap (end, BDE_PT_DISABLED);
+		  *end = ftrace_new_gap (*end, BDE_PT_DISABLED);
 
-		  VEC_safe_push (bfun_s, *gaps, end);
+		  VEC_safe_push (bfun_s, *gaps, *end);
 
 		  pt_insn_get_offset (decoder, &offset);
 
 		  warning (_("Non-contiguous trace at instruction %u (offset "
 			     "= 0x%" PRIx64 ", pc = 0x%" PRIx64 ")."),
-			   end->insn_offset - 1, offset, insn.ip);
+			   (*end)->insn_offset - 1, offset, insn.ip);
 		}
 	    }
 
 	  /* Indicate trace overflows.  */
 	  if (insn.resynced)
 	    {
-	      *pend = end = ftrace_new_gap (end, BDE_PT_OVERFLOW);
-	      if (begin == NULL)
-		*pbegin = begin = end;
+	      *end = ftrace_new_gap (*end, BDE_PT_OVERFLOW);
+	      if (*begin == NULL)
+		*begin = *end;
 
-	      VEC_safe_push (bfun_s, *gaps, end);
+	      VEC_safe_push (bfun_s, *gaps, *end);
 
 	      pt_insn_get_offset (decoder, &offset);
 
 	      warning (_("Overflow at instruction %u (offset = 0x%" PRIx64
-			 ", pc = 0x%" PRIx64 ")."), end->insn_offset - 1,
+			 ", pc = 0x%" PRIx64 ")."), (*end)->insn_offset - 1,
 		       offset, insn.ip);
 	    }
 
-	  upd = ftrace_update_function (end, insn.ip);
-	  if (upd != end)
-	    {
-	      *pend = end = upd;
-
-	      if (begin == NULL)
-		*pbegin = begin = upd;
-	    }
+	  *end = ftrace_update_function (*end, insn.ip);
+	  if (*begin == NULL)
+	    *begin = *end;
 
 	  /* Maintain the function level offset.  */
-	  *plevel = std::min (*plevel, end->level);
+	  *plevel = std::min (*plevel, (*end)->level);
 
 	  btrace_insn btinsn = pt_btrace_insn (insn);
-	  ftrace_update_insns (end, &btinsn);
+	  ftrace_update_insns (*end, &btinsn);
 	}
 
       if (errcode == -pte_eos)
 	break;
 
       /* Indicate the gap in the trace.  */
-      *pend = end = ftrace_new_gap (end, errcode);
-      if (begin == NULL)
-	*pbegin = begin = end;
+      *end = ftrace_new_gap (*end, errcode);
+      if (*begin == NULL)
+	*begin = *end;
 
-      VEC_safe_push (bfun_s, *gaps, end);
+      VEC_safe_push (bfun_s, *gaps, *end);
 
       pt_insn_get_offset (decoder, &offset);
 
       warning (_("Decode error (%d) at instruction %u (offset = 0x%" PRIx64
-		 ", pc = 0x%" PRIx64 "): %s."), errcode, end->insn_offset - 1,
-	       offset, insn.ip, pt_errstr (pt_errcode (errcode)));
+		 ", pc = 0x%" PRIx64 "): %s."), errcode,
+	       (*end)->insn_offset - 1, offset, insn.ip,
+	       pt_errstr (pt_errcode (errcode)));
     }
 }
 
-- 
1.8.3.1

RE: [PATCH 3/3] btrace: support decoder events

[Metzger, Markus T]

Ping.

Thanks,
Markus.

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Markus Metzger
> Sent: Friday, April 28, 2017 4:44 PM
> To: gdb-patches@sourceware.org
> Subject: [PATCH 3/3] btrace: support decoder events
> 
> Newer versions of libipt support instruction flow decoder events instead of
> indicating those events with flags in struct pt_insn.  Add support for them in
> GDB.
> 
> 2017-04-28  Markus Metzger  <markus.t.metzger@intel.com>
> 
> gdb/
> 	* btrace.c (handle_pt_insn_events): New.
> 	(ftrace_add_pt): Call handle_pt_insn_events.  Rename ERRCODE into
> 	STATUS.  Split into this and ...
> 	(handle_pt_insn_event_flags): ... this.
> 
> 
> ---
>  gdb/btrace.c | 174 +++++++++++++++++++++++++++++++++++++++++++-------------
> ---
>  1 file changed, 127 insertions(+), 47 deletions(-)
> 
> diff --git a/gdb/btrace.c b/gdb/btrace.c
> index 6780e22..21d0be9 100644
> --- a/gdb/btrace.c
> +++ b/gdb/btrace.c
> @@ -1132,6 +1132,116 @@ pt_btrace_insn (const struct pt_insn &insn)
>  	  pt_btrace_insn_flags (insn)};
>  }
> 
> +/* Handle instruction decode events (libipt-v2).  */
> +
> +static int
> +handle_pt_insn_events (struct pt_insn_decoder *decoder,
> +		       struct btrace_function **pbegin,
> +		       struct btrace_function **pend,
> +		       VEC (bfun_s) **gaps, int status)
> +{
> +#if defined (HAVE_PT_INSN_EVENT)
> +  while (status & pts_event_pending)
> +    {
> +      struct pt_event event;
> +      uint64_t offset;
> +
> +      status = pt_insn_event (decoder, &event, sizeof (event));
> +      if (status < 0)
> +	break;
> +
> +      switch (event.type)
> +	{
> +	default:
> +	  break;
> +
> +	case ptev_enabled:
> +	  if (*pbegin != NULL && event.variant.enabled.resumed == 0)
> +	    {
> +	      *pend = ftrace_new_gap (*pend, BDE_PT_DISABLED);
> +	      if (*pbegin == NULL)
> +		*pbegin = *pend;
> +
> +	      VEC_safe_push (bfun_s, *gaps, *pend);
> +
> +	      pt_insn_get_offset (decoder, &offset);
> +
> +	      warning (_("Non-contiguous trace at instruction %u (offset = 0x%"
> +			 PRIx64 ")."), (*pend)->insn_offset - 1, offset);
> +	    }
> +
> +	  break;
> +
> +	case ptev_overflow:
> +	  *pend = ftrace_new_gap (*pend, BDE_PT_OVERFLOW);
> +	  if (*pbegin == NULL)
> +	    *pbegin = *pend;
> +
> +	  VEC_safe_push (bfun_s, *gaps, *pend);
> +
> +	  pt_insn_get_offset (decoder, &offset);
> +
> +	  warning (_("Overflow at instruction %u (offset = 0x%" PRIx64 ")."),
> +		   (*pend)->insn_offset - 1, offset);
> +
> +	  break;
> +	}
> +    }
> +#endif /* defined (HAVE_PT_INSN_EVENT) */
> +
> +  return status;
> +}
> +
> +/* Handle events indicated by flags in INSN (libipt-v1).  */
> +
> +static void
> +handle_pt_insn_event_flags (struct pt_insn_decoder *decoder,
> +			    const struct pt_insn *insn,
> +			    struct btrace_function **pbegin,
> +			    struct btrace_function **pend,
> +			    VEC (bfun_s) **gaps)
> +{
> +#if defined (HAVE_STRUCT_PT_INSN_ENABLED)
> +  /* Tracing is disabled and re-enabled each time we enter the kernel.  Most
> +     times, we continue from the same instruction we stopped before.  This is
> +     indicated via the RESUMED instruction flag.  The ENABLED instruction flag
> +     means that we continued from some other instruction.  Indicate this as a
> +     trace gap except when tracing just started.  */
> +  if (insn.enabled && (*begin != NULL))
> +    {
> +      uint64_t offset;
> +
> +      *end = ftrace_new_gap (*end, BDE_PT_DISABLED);
> +
> +      VEC_safe_push (bfun_s, *gaps, *end);
> +
> +      pt_insn_get_offset (decoder, &offset);
> +
> +      warning (_("Non-contiguous trace at instruction %u (offset = 0x%" PRIx64
> +		 ", pc = 0x%" PRIx64 ")."), (*end)->insn_offset - 1, offset,
> +	       insn.ip);
> +    }
> +#endif /* defined (HAVE_STRUCT_PT_INSN_ENABLED) */
> +
> +#if defined (HAVE_STRUCT_PT_INSN_RESYNCED)
> +  /* Indicate trace overflows.  */
> +  if (insn.resynced)
> +    {
> +      uint64_t offset;
> +
> +      *end = ftrace_new_gap (*end, BDE_PT_OVERFLOW);
> +      if (*begin == NULL)
> +	*begin = *end;
> +
> +      VEC_safe_push (bfun_s, *gaps, *end);
> +
> +      pt_insn_get_offset (decoder, &offset);
> +
> +      warning (_("Overflow at instruction %u (offset = 0x%" PRIx64 ", pc = 0x%"
> +		 PRIx64 ")."), (*end)->insn_offset - 1, offset, insn.ip);
> +    }
> +#endif /* defined (HAVE_STRUCT_PT_INSN_RESYNCED) */
> +}
> 
>  /* Add function branch trace using DECODER.  */
> 
> @@ -1142,64 +1252,34 @@ ftrace_add_pt (struct pt_insn_decoder *decoder,
>  	       VEC (bfun_s) **gaps)
>  {
>    uint64_t offset;
> -  int errcode;
> +  int status;
> 
>    for (;;)
>      {
>        struct pt_insn insn;
> 
> -      errcode = pt_insn_sync_forward (decoder);
> -      if (errcode < 0)
> +      status = pt_insn_sync_forward (decoder);
> +      if (status < 0)
>  	{
> -	  if (errcode != -pte_eos)
> +	  if (status != -pte_eos)
>  	    warning (_("Failed to synchronize onto the Intel Processor "
> -		       "Trace stream: %s."), pt_errstr (pt_errcode (errcode)));
> +		       "Trace stream: %s."), pt_errstr (pt_errcode (status)));
>  	  break;
>  	}
> 
>        for (;;)
>  	{
> -	  errcode = pt_insn_next (decoder, &insn, sizeof(insn));
> -	  if (errcode < 0)
> +	  /* Handle events from the previous iteration or synchronization.  */
> +	  status = handle_pt_insn_events (decoder, begin, end, gaps, status);
> +	  if (status < 0)
>  	    break;
> 
> -	  /* Look for gaps in the trace - unless we're at the beginning.  */
> -	  if (*begin != NULL)
> -	    {
> -	      /* Tracing is disabled and re-enabled each time we enter the
> -		 kernel.  Most times, we continue from the same instruction we
> -		 stopped before.  This is indicated via the RESUMED instruction
> -		 flag.  The ENABLED instruction flag means that we continued
> -		 from some other instruction.  Indicate this as a trace gap.  */
> -	      if (insn.enabled)
> -		{
> -		  *end = ftrace_new_gap (*end, BDE_PT_DISABLED);
> -
> -		  VEC_safe_push (bfun_s, *gaps, *end);
> -
> -		  pt_insn_get_offset (decoder, &offset);
> -
> -		  warning (_("Non-contiguous trace at instruction %u (offset "
> -			     "= 0x%" PRIx64 ", pc = 0x%" PRIx64 ")."),
> -			   (*end)->insn_offset - 1, offset, insn.ip);
> -		}
> -	    }
> -
> -	  /* Indicate trace overflows.  */
> -	  if (insn.resynced)
> -	    {
> -	      *end = ftrace_new_gap (*end, BDE_PT_OVERFLOW);
> -	      if (*begin == NULL)
> -		*begin = *end;
> -
> -	      VEC_safe_push (bfun_s, *gaps, *end);
> -
> -	      pt_insn_get_offset (decoder, &offset);
> +	  status = pt_insn_next (decoder, &insn, sizeof(insn));
> +	  if (status < 0)
> +	    break;
> 
> -	      warning (_("Overflow at instruction %u (offset = 0x%" PRIx64
> -			 ", pc = 0x%" PRIx64 ")."), (*end)->insn_offset - 1,
> -		       offset, insn.ip);
> -	    }
> +	  /* Handle events indicated by flags in INSN.  */
> +	  handle_pt_insn_event_flags (decoder, &insn, begin, end, gaps);
> 
>  	  *end = ftrace_update_function (*end, insn.ip);
>  	  if (*begin == NULL)
> @@ -1212,11 +1292,11 @@ ftrace_add_pt (struct pt_insn_decoder *decoder,
>  	  ftrace_update_insns (*end, &btinsn);
>  	}
> 
> -      if (errcode == -pte_eos)
> +      if (status == -pte_eos)
>  	break;
> 
>        /* Indicate the gap in the trace.  */
> -      *end = ftrace_new_gap (*end, errcode);
> +      *end = ftrace_new_gap (*end, status);
>        if (*begin == NULL)
>  	*begin = *end;
> 
> @@ -1225,9 +1305,9 @@ ftrace_add_pt (struct pt_insn_decoder *decoder,
>        pt_insn_get_offset (decoder, &offset);
> 
>        warning (_("Decode error (%d) at instruction %u (offset = 0x%" PRIx64
> -		 ", pc = 0x%" PRIx64 "): %s."), errcode,
> +		 ", pc = 0x%" PRIx64 "): %s."), status,
>  	       (*end)->insn_offset - 1, offset, insn.ip,
> -	       pt_errstr (pt_errcode (errcode)));
> +	       pt_errstr (pt_errcode (status)));
>      }
>  }
> 
> --
> 1.8.3.1

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

Re: [PATCH 3/3] btrace: support decoder events

[Pedro Alves]

On 05/23/2017 03:10 PM, Metzger, Markus T wrote:
> Ping.

I won't pretend to understand most of this stuff, but I skimmed it
and at least nothing jumped out at me as obviously wrong.  Fine with
me put it in.

Thanks,
Pedro Alves