[PATCH 00/10] Extensions for PTWRITE

[PATCH 00/10] Extensions for PTWRITE

[felix.willgerodt]

From: Felix Willgerodt <felix.willgerodt@intel.com>

Hi all,

this is a set of patches extending the GDB record functionality for the new
x86 instruction PTWRITE.  PTWRITE allows the user to write any value into
the Intel Processor Trace.  This patch series enables the user to access,
store and display these values in GDB/Python.

Regards,
Felix

Felix Willgerodt (10):
  btrace: Introduce auxiliary instructions.
  btrace: Enable auxiliary instructions in record instruction-history.
  btrace: Enable auxiliary instructions in record function-call-history.
  btrace: Handle stepping and goto for auxiliary instructions.
  python: Introduce gdb.RecordAuxiliary class.
  python: Add clear_trace() to gdb.Record.
  btrace, linux: Enable ptwrite packets.
  btrace, python: Enable ptwrite listener registration.
  btrace, python: Enable calling the ptwrite listener.
  btrace: Extend event decoding for ptwrite.

 gdb/NEWS                                      |   6 +
 gdb/btrace.c                                  |  52 ++
 gdb/btrace.h                                  |  42 +-
 gdb/data-directory/Makefile.in                |   1 +
 gdb/doc/gdb.texinfo                           |   8 +-
 gdb/doc/python.texi                           | 129 +++++
 gdb/extension-priv.h                          |   4 +
 gdb/extension.c                               |  24 +
 gdb/extension.h                               |   3 +
 gdb/guile/guile.c                             |   1 +
 gdb/nat/linux-btrace.c                        |  32 ++
 gdb/python/lib/gdb/ptwrite.py                 |  83 +++
 gdb/python/py-record-btrace.c                 | 138 ++++-
 gdb/python/py-record-btrace.h                 |   6 +
 gdb/python/py-record.c                        | 118 ++++-
 gdb/python/py-record.h                        |   3 +
 gdb/python/python-internal.h                  |   3 +
 gdb/python/python.c                           |   2 +
 gdb/record-btrace.c                           |  83 ++-
 gdb/record.c                                  |   5 +
 gdb/record.h                                  |   5 +-
 gdb/testsuite/gdb.btrace/ptwrite.c            |  40 ++
 gdb/testsuite/gdb.btrace/ptwrite.exp          | 212 ++++++++
 gdb/testsuite/gdb.btrace/x86_64-ptwrite.S     | 479 ++++++++++++++++++
 gdb/testsuite/gdb.python/py-record-btrace.exp |   6 +-
 gdb/testsuite/lib/gdb.exp                     |  92 ++++
 26 files changed, 1552 insertions(+), 25 deletions(-)
 create mode 100644 gdb/python/lib/gdb/ptwrite.py
 create mode 100644 gdb/testsuite/gdb.btrace/ptwrite.c
 create mode 100644 gdb/testsuite/gdb.btrace/ptwrite.exp
 create mode 100644 gdb/testsuite/gdb.btrace/x86_64-ptwrite.S

-- 
2.20.1

[PATCH 08/10] btrace, python: Enable ptwrite listener registration.

[felix.willgerodt]

From: Felix Willgerodt <felix.willgerodt@intel.com>

With this patch a default ptwrite listener is registered upon start of GDB.
It prints the plain ptwrite payload as hex.  The default listener can be
overwritten by registering a custom listener in python or by registering
"None", for no output at all.  Registering a listener function creates per
thread copies to allow unique internal states per thread.

2019-05-29  Felix Willgerodt  <felix.willgerodt@intel.com>

gdb/ChangeLog:
	* btrace.c (ftrace_add_pt): Add call to
	apply_ext_lang_ptwrite_listener.
	* btrace.h (btrace_thread_info): New member ptw_listener.
	* data-directory/Makefile.in: Add ptwrite.py.
	* extension-priv.h (struct extension_language_ops): New member
	apply_ptwrite_listener.
	* extension.c (apply_ext_lang_ptwrite_listener): New function.
	* extension.h (apply_ext_lang_ptwrite_listener): New declaration.
	* guile/guile.c (guile_extension_ops): Add
	gdbscm_load_ptwrite_listener.
	* python/lib/gdb/ptwrite.py: New file.
	* python/py-record-btrace.c (recpy_initialize_listener,
	get_ptwrite_listener): New function.
	* python/py-record-btrace.h (recpy_initialize_listener): New
	declaration.
	* python/py-record.c (gdbpy_load_ptwrite_listener): New function.
	* python/python-internal.h (gdbpy_load_ptwrite_listener): New
	declaration.
	* python/python.c (python_extension_ops): New member
	gdbpy_load_ptwrite_listener.

---
 gdb/btrace.c                   |  3 ++
 gdb/btrace.h                   |  3 ++
 gdb/data-directory/Makefile.in |  1 +
 gdb/extension-priv.h           |  4 ++
 gdb/extension.c                | 24 ++++++++++
 gdb/extension.h                |  3 ++
 gdb/guile/guile.c              |  1 +
 gdb/python/lib/gdb/ptwrite.py  | 83 ++++++++++++++++++++++++++++++++++
 gdb/python/py-record-btrace.c  | 40 ++++++++++++++++
 gdb/python/py-record-btrace.h  |  3 ++
 gdb/python/py-record.c         | 29 ++++++++++++
 gdb/python/python-internal.h   |  3 ++
 gdb/python/python.c            |  2 +
 13 files changed, 199 insertions(+)
 create mode 100644 gdb/python/lib/gdb/ptwrite.py

diff --git a/gdb/btrace.c b/gdb/btrace.c
index d3e2f46c678..5bdef1c1768 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -34,6 +34,7 @@
 #include "common/rsp-low.h"
 #include "gdbcmd.h"
 #include "cli/cli-utils.h"
+#include "extension.h"
 
 /* For maintenance commands.  */
 #include "record-btrace.h"
@@ -1308,6 +1309,8 @@ ftrace_add_pt (struct btrace_thread_info *btinfo,
   uint64_t offset;
   int status;
 
+  apply_ext_lang_ptwrite_listener (inferior_ptid);
+
   for (;;)
     {
       struct pt_insn insn;
diff --git a/gdb/btrace.h b/gdb/btrace.h
index d29a1633ecf..7a80f5a44e1 100644
--- a/gdb/btrace.h
+++ b/gdb/btrace.h
@@ -354,6 +354,9 @@ struct btrace_thread_info
      stepping through the execution history.  */
   std::vector<std::string> aux_data;
 
+  /* PyObject pointer to the ptwrite listener function.  */
+  void *ptw_listener = nullptr;
+
   /* The function level offset.  When added to each function's LEVEL,
      this normalizes the function levels such that the smallest level
      becomes zero.  */
diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in
index 88147125ccd..39170e786e8 100644
--- a/gdb/data-directory/Makefile.in
+++ b/gdb/data-directory/Makefile.in
@@ -74,6 +74,7 @@ PYTHON_FILE_LIST = \
 	gdb/frames.py \
 	gdb/printing.py \
 	gdb/prompt.py \
+	gdb/ptwrite.py \
 	gdb/types.py \
 	gdb/unwinder.py \
 	gdb/xmethod.py \
diff --git a/gdb/extension-priv.h b/gdb/extension-priv.h
index 97594f853a2..ced4cd10390 100644
--- a/gdb/extension-priv.h
+++ b/gdb/extension-priv.h
@@ -187,6 +187,10 @@ struct extension_language_ops
      enum ext_lang_frame_args args_type,
      struct ui_out *out, int frame_low, int frame_high);
 
+  /* Used for registering the ptwrite listener to the current thread.  */
+  enum ext_lang_bt_status (*apply_ptwrite_listener)
+    (const struct extension_language_defn *, ptid_t inferior_ptid);
+
   /* Update values held by the extension language when OBJFILE is discarded.
      New global types must be created for every such value, which must then be
      updated to use the new types.
diff --git a/gdb/extension.c b/gdb/extension.c
index 8637bc53f2e..e463f0d9b87 100644
--- a/gdb/extension.c
+++ b/gdb/extension.c
@@ -570,6 +570,30 @@ apply_ext_lang_frame_filter (struct frame_info *frame,
   return EXT_LANG_BT_NO_FILTERS;
 }
 
+/* Used for registering the ptwrite listener to the current thread.  */
+
+enum ext_lang_bt_status
+apply_ext_lang_ptwrite_listener (ptid_t inferior_ptid)
+{
+  int i;
+  const struct extension_language_defn *extlang;
+
+  ALL_ENABLED_EXTENSION_LANGUAGES (i, extlang)
+    {
+      enum ext_lang_bt_status status;
+
+      if (extlang->ops->apply_ptwrite_listener == NULL)
+	continue;
+
+      status = extlang->ops->apply_ptwrite_listener (extlang, inferior_ptid);
+
+      if (status != EXT_LANG_BT_NO_FILTERS)
+	return status;
+    }
+
+  return EXT_LANG_BT_NO_FILTERS;
+}
+
 /* Update values held by the extension language when OBJFILE is discarded.
    New global types must be created for every such value, which must then be
    updated to use the new types.
diff --git a/gdb/extension.h b/gdb/extension.h
index 2f1b71851c6..cb5fe89a16d 100644
--- a/gdb/extension.h
+++ b/gdb/extension.h
@@ -291,6 +291,9 @@ extern enum ext_lang_bt_status apply_ext_lang_frame_filter
    enum ext_lang_frame_args args_type,
    struct ui_out *out, int frame_low, int frame_high);
 
+enum ext_lang_bt_status apply_ext_lang_ptwrite_listener
+  (ptid_t inferior_ptid);
+
 extern void preserve_ext_lang_values (struct objfile *, htab_t copied_types);
 
 extern const struct extension_language_defn *get_breakpoint_cond_ext_lang
diff --git a/gdb/guile/guile.c b/gdb/guile/guile.c
index 9247fd64122..9ba63da9fcd 100644
--- a/gdb/guile/guile.c
+++ b/gdb/guile/guile.c
@@ -150,6 +150,7 @@ const struct extension_language_ops guile_extension_ops =
   gdbscm_apply_val_pretty_printer,
 
   NULL, /* gdbscm_apply_frame_filter, */
+  NULL, /* gdbscm_load_ptwrite_listener, */
 
   gdbscm_preserve_values,
 
diff --git a/gdb/python/lib/gdb/ptwrite.py b/gdb/python/lib/gdb/ptwrite.py
new file mode 100644
index 00000000000..a485ce89ee9
--- /dev/null
+++ b/gdb/python/lib/gdb/ptwrite.py
@@ -0,0 +1,83 @@
+# Ptwrite utilities.
+# Copyright (C) 2018 Free Software Foundation, Inc.
+
+# 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/>.
+
+"""Utilities for working with ptwrite listeners."""
+
+import gdb
+from copy import deepcopy
+
+
+def default_listener(payload, ip):
+    """Default listener that is active upon starting GDB."""
+    return "payload: {:#x}".format(payload)
+
+# This dict contains the per thread copies of the listener function and the
+# global template listener, from which the copies are created.
+_ptwrite_listener = {"global" : default_listener}
+
+
+def _update_listener_dict(thread_list):
+    """Helper function to update the listener dict.
+
+    Discards listener copies of threads that already exited and registers
+    copies of the listener for new threads."""
+    # thread_list[x].ptid returns the tuple (pid, lwp, tid)
+    lwp_list = [i.ptid[1] for i in thread_list]
+
+    # clean-up old listeners
+    for key in _ptwrite_listener:
+        if key not in lwp_list and not "global":
+            _ptwrite_listener.pop(key)
+
+    # Register listener for new threads
+    for key in lwp_list:
+        if key not in _ptwrite_listener.keys():
+            _ptwrite_listener[key] = deepcopy(_ptwrite_listener["global"])
+
+
+def _clear_traces(thread_list):
+    """Helper function to clear the trace of all threads."""
+    current_thread = gdb.selected_thread()
+
+    for thread in thread_list:
+        thread.switch()
+        gdb.current_recording().clear_trace()
+
+    current_thread.switch()
+
+
+def register_listener(listener):
+    """Register the ptwrite listener function."""
+    global _ptwrite_listener
+
+    if listener is not None and not callable(listener):
+        raise TypeError("Listener must be callable!")
+
+    thread_list = gdb.Inferior.threads(gdb.selected_inferior())
+    _clear_traces(thread_list)
+
+    _ptwrite_listener.clear()
+    _ptwrite_listener["global"] = listener
+
+    _update_listener_dict(thread_list)
+
+
+def get_listener():
+    """Returns the listeners of the current thread."""
+    thread_list = gdb.Inferior.threads(gdb.selected_inferior())
+    _update_listener_dict(thread_list)
+
+    return _ptwrite_listener[gdb.selected_thread().ptid[1]]
diff --git a/gdb/python/py-record-btrace.c b/gdb/python/py-record-btrace.c
index 2a4af551146..c7ad47d64a0 100644
--- a/gdb/python/py-record-btrace.c
+++ b/gdb/python/py-record-btrace.c
@@ -772,6 +772,46 @@ recpy_bt_function_call_history (PyObject *self, void *closure)
   return btpy_list_new (tinfo, first, last, 1, &recpy_func_type);
 }
 
+/* Helper function returning the current ptwrite listener.  Returns nullptr
+   in case of errors.  */
+
+PyObject *
+get_ptwrite_listener ()
+{
+  PyObject *module = PyImport_ImportModule ("gdb.ptwrite");
+
+  if (PyErr_Occurred ())
+  {
+    gdbpy_print_stack ();
+    return nullptr;
+  }
+
+  PyObject *default_ptw_listener = PyObject_CallMethod (module,	"get_listener",
+							NULL);
+
+  gdb_Py_DECREF (module);
+
+  if (PyErr_Occurred ())
+    gdbpy_print_stack ();
+
+  return default_ptw_listener;
+}
+
+/* Helper function to initialize the ptwrite listener.  Returns nullptr in
+   case of errors.  */
+
+PyObject *
+recpy_initialize_listener (ptid_t inferior_ptid)
+{
+  struct thread_info * const tinfo = find_thread_ptid (inferior_ptid);
+
+  gdb_assert (tinfo != nullptr);
+
+  tinfo->btrace.ptw_listener = get_ptwrite_listener ();
+
+  return (PyObject *) tinfo->btrace.ptw_listener;
+}
+
 /* Implementation of BtraceRecord.goto (self, BtraceInstruction) -> None.  */
 
 PyObject *
diff --git a/gdb/python/py-record-btrace.h b/gdb/python/py-record-btrace.h
index 8056a5b75cb..8dff1917729 100644
--- a/gdb/python/py-record-btrace.h
+++ b/gdb/python/py-record-btrace.h
@@ -91,4 +91,7 @@ extern PyObject *recpy_bt_func_prev (PyObject *self, void *closure);
 /* Implementation of RecordFunctionSegment.next [RecordFunctionSegment].  */
 extern PyObject *recpy_bt_func_next (PyObject *self, void *closure);
 
+/* Helper function returning the ptwrite listener by thread id.  */
+extern PyObject *recpy_initialize_listener (ptid_t inferior_ptid);
+
 #endif /* PYTHON_PY_RECORD_BTRACE_H */
diff --git a/gdb/python/py-record.c b/gdb/python/py-record.c
index 855b4089d87..86dc0dfaa9a 100644
--- a/gdb/python/py-record.c
+++ b/gdb/python/py-record.c
@@ -735,3 +735,32 @@ gdbpy_stop_recording (PyObject *self, PyObject *args)
 
   Py_RETURN_NONE;
 }
+
+/* Used for registering the default ptwrite listener to the current thread.  A
+   pointer to this function is stored in the python extension interface.  */
+
+enum ext_lang_bt_status
+gdbpy_load_ptwrite_listener (const struct extension_language_defn *extlang,
+			     ptid_t inferior_ptid)
+{
+  struct gdbarch *gdbarch = NULL;
+
+  if (!gdb_python_initialized)
+    return EXT_LANG_BT_NO_FILTERS;
+
+  try
+    {
+      gdbarch = target_gdbarch ();
+    }
+  catch (const gdb_exception &except)
+    {
+      /* Let gdb try to print the stack trace.  */
+      return EXT_LANG_BT_NO_FILTERS;
+    }
+
+  gdbpy_enter enter_py (gdbarch, current_language);
+
+  recpy_initialize_listener (inferior_ptid);
+
+  return EXT_LANG_BT_OK;
+}
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 69ff1fe30de..552e2d8b76a 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -397,6 +397,9 @@ extern enum ext_lang_rc gdbpy_apply_val_pretty_printer
    struct value *val,
    const struct value_print_options *options,
    const struct language_defn *language);
+extern enum ext_lang_bt_status gdbpy_load_ptwrite_listener
+  (const struct extension_language_defn *extlang,
+   ptid_t inferior_ptid);
 extern enum ext_lang_bt_status gdbpy_apply_frame_filter
   (const struct extension_language_defn *,
    struct frame_info *frame, frame_filter_flags flags,
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 4dad8ec10d1..2f2fa4bb236 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -178,6 +178,8 @@ const struct extension_language_ops python_extension_ops =
 
   gdbpy_apply_frame_filter,
 
+  gdbpy_load_ptwrite_listener,
+
   gdbpy_preserve_values,
 
   gdbpy_breakpoint_has_cond,
-- 
2.20.1

RE: [PATCH 08/10] btrace, python: Enable ptwrite listener registration.

[Metzger, Markus T]

Hello Felix,

> With this patch a default ptwrite listener is registered upon start of GDB.
> It prints the plain ptwrite payload as hex.  The default listener can be
> overwritten by registering a custom listener in python or by registering
> "None", for no output at all.  Registering a listener function creates per
> thread copies to allow unique internal states per thread.
> 
> 2019-05-29  Felix Willgerodt  <felix.willgerodt@intel.com>
> 
> gdb/ChangeLog:
> 	* btrace.c (ftrace_add_pt): Add call to
> 	apply_ext_lang_ptwrite_listener.
> 	* btrace.h (btrace_thread_info): New member ptw_listener.
> 	* data-directory/Makefile.in: Add ptwrite.py.
> 	* extension-priv.h (struct extension_language_ops): New member
> 	apply_ptwrite_listener.
> 	* extension.c (apply_ext_lang_ptwrite_listener): New function.
> 	* extension.h (apply_ext_lang_ptwrite_listener): New declaration.
> 	* guile/guile.c (guile_extension_ops): Add
> 	gdbscm_load_ptwrite_listener.
> 	* python/lib/gdb/ptwrite.py: New file.
> 	* python/py-record-btrace.c (recpy_initialize_listener,
> 	get_ptwrite_listener): New function.
> 	* python/py-record-btrace.h (recpy_initialize_listener): New
> 	declaration.
> 	* python/py-record.c (gdbpy_load_ptwrite_listener): New function.
> 	* python/python-internal.h (gdbpy_load_ptwrite_listener): New
> 	declaration.
> 	* python/python.c (python_extension_ops): New member
> 	gdbpy_load_ptwrite_listener.
> 
> ---
>  gdb/btrace.c                   |  3 ++
>  gdb/btrace.h                   |  3 ++
>  gdb/data-directory/Makefile.in |  1 +
>  gdb/extension-priv.h           |  4 ++
>  gdb/extension.c                | 24 ++++++++++
>  gdb/extension.h                |  3 ++
>  gdb/guile/guile.c              |  1 +
>  gdb/python/lib/gdb/ptwrite.py  | 83
> ++++++++++++++++++++++++++++++++++
>  gdb/python/py-record-btrace.c  | 40 ++++++++++++++++
>  gdb/python/py-record-btrace.h  |  3 ++
>  gdb/python/py-record.c         | 29 ++++++++++++
>  gdb/python/python-internal.h   |  3 ++
>  gdb/python/python.c            |  2 +
>  13 files changed, 199 insertions(+)
>  create mode 100644 gdb/python/lib/gdb/ptwrite.py

When registering a new PTWRITE listener, could GDB check the libipt version and give
an error if it does not support PTW?

> @@ -1308,6 +1309,8 @@ ftrace_add_pt (struct btrace_thread_info *btinfo,
>    uint64_t offset;
>    int status;
> 
> +  apply_ext_lang_ptwrite_listener (inferior_ptid);

We may decode the trace in chunks by calling ftrace_add_pt () multiple times.
This would need to preserve (the state of) the installed listener.

> @@ -0,0 +1,83 @@
> +# Ptwrite utilities.
> +# Copyright (C) 2018 Free Software Foundation, Inc.

It's 2019, meanwhile.

> +
> +# 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/>.
> +
> +"""Utilities for working with ptwrite listeners."""
> +
> +import gdb
> +from copy import deepcopy
> +
> +
> +def default_listener(payload, ip):

I somehow expected some PtwritePrinter class.

> +    """Default listener that is active upon starting GDB."""
> +    return "payload: {:#x}".format(payload)

Should we return the payload (in hex) without the prefix?

> @@ -735,3 +735,32 @@ gdbpy_stop_recording (PyObject *self, PyObject *args)
> 
>    Py_RETURN_NONE;
>  }
> +
> +/* Used for registering the default ptwrite listener to the current thread.  A
> +   pointer to this function is stored in the python extension interface.  */
> +
> +enum ext_lang_bt_status
> +gdbpy_load_ptwrite_listener (const struct extension_language_defn *extlang,
> +			     ptid_t inferior_ptid)

PTWRITE is very PT specific.  Any chance this could go into py-record-btrace.c?

> +{
> +  struct gdbarch *gdbarch = NULL;
> +
> +  if (!gdb_python_initialized)
> +    return EXT_LANG_BT_NO_FILTERS;
> +
> +  try
> +    {
> +      gdbarch = target_gdbarch ();
> +    }
> +  catch (const gdb_exception &except)
> +    {
> +      /* Let gdb try to print the stack trace.  */
> +      return EXT_LANG_BT_NO_FILTERS;

I fail to understand the comment.

> +    }
> +
> +  gdbpy_enter enter_py (gdbarch, current_language);
> +
> +  recpy_initialize_listener (inferior_ptid);
> +
> +  return EXT_LANG_BT_OK;
> +}

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

Re: [PATCH 08/10] btrace, python: Enable ptwrite listener registration.

[Willgerodt, Felix]

On 6/4/19 2:36 PM, Metzger, Markus T wrote:
> Hello Felix,
>
>> With this patch a default ptwrite listener is registered upon start of GDB.
>> It prints the plain ptwrite payload as hex.  The default listener can be
>> overwritten by registering a custom listener in python or by registering
>> "None", for no output at all.  Registering a listener function creates per
>> thread copies to allow unique internal states per thread.
>>
>> 2019-05-29  Felix Willgerodt  <felix.willgerodt@intel.com>
>>
>> gdb/ChangeLog:
>> * btrace.c (ftrace_add_pt): Add call to
>> apply_ext_lang_ptwrite_listener.
>> * btrace.h (btrace_thread_info): New member ptw_listener.
>> * data-directory/Makefile.in: Add ptwrite.py.
>> * extension-priv.h (struct extension_language_ops): New member
>> apply_ptwrite_listener.
>> * extension.c (apply_ext_lang_ptwrite_listener): New function.
>> * extension.h (apply_ext_lang_ptwrite_listener): New declaration.
>> * guile/guile.c (guile_extension_ops): Add
>> gdbscm_load_ptwrite_listener.
>> * python/lib/gdb/ptwrite.py: New file.
>> * python/py-record-btrace.c (recpy_initialize_listener,
>> get_ptwrite_listener): New function.
>> * python/py-record-btrace.h (recpy_initialize_listener): New
>> declaration.
>> * python/py-record.c (gdbpy_load_ptwrite_listener): New function.
>> * python/python-internal.h (gdbpy_load_ptwrite_listener): New
>> declaration.
>> * python/python.c (python_extension_ops): New member
>> gdbpy_load_ptwrite_listener.
>>
>> ---
>>   gdb/btrace.c                   |  3 ++
>>   gdb/btrace.h                   |  3 ++
>>   gdb/data-directory/Makefile.in |  1 +
>>   gdb/extension-priv.h           |  4 ++
>>   gdb/extension.c                | 24 ++++++++++
>>   gdb/extension.h                |  3 ++
>>   gdb/guile/guile.c              |  1 +
>>   gdb/python/lib/gdb/ptwrite.py  | 83
>> ++++++++++++++++++++++++++++++++++
>>   gdb/python/py-record-btrace.c  | 40 ++++++++++++++++
>>   gdb/python/py-record-btrace.h  |  3 ++
>>   gdb/python/py-record.c         | 29 ++++++++++++
>>   gdb/python/python-internal.h   |  3 ++
>>   gdb/python/python.c            |  2 +
>>   13 files changed, 199 insertions(+)
>>   create mode 100644 gdb/python/lib/gdb/ptwrite.py

Markus> When registering a new PTWRITE listener, could GDB check the libipt version and give
Markus> an error if it does not support PTW?

In the current design GDB doesn't know if a there is a new listener (different to the default),
until it needs to use it. We would need to expose the libipt version somehow in python.
Not sure how that should look like, in CLI we only have it exposed in "maintenance info btrace".


>> @@ -1308,6 +1309,8 @@ ftrace_add_pt (struct btrace_thread_info *btinfo,
>>     uint64_t offset;
>>     int status;
>>
>> +  apply_ext_lang_ptwrite_listener (inferior_ptid);

Markus > We may decode the trace in chunks by calling ftrace_add_pt () multiple times.
Markus > This would need to preserve (the state of) the installed listener.

The state is already preserved. This just fetches the (C++) pointer to
be able to call the listener from GDB. If the listener wasn't changed,
the same pointer/object will be fetched as in the first call to ftrace_add_pt
().


>> @@ -0,0 +1,83 @@
>> +# Ptwrite utilities.
>> +# Copyright (C) 2018 Free Software Foundation, Inc.

Markus> It's 2019, meanwhile.

Fixed.

>> +
>> +# 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/>.
>> +
>> +"""Utilities for working with ptwrite listeners."""
>> +
>> +import gdb
>> +from copy import deepcopy
>> +
>> +
>> +def default_listener(payload, ip):

Markus> I somehow expected some PtwritePrinter class.

I don't see any advantage of classifying the default one. And as we allow every
callable, I don't see anything else we need to provide here. Please elaborate if you do.


>> +    """Default listener that is active upon starting GDB."""
>> +    return "payload: {:#x}".format(payload)

Markus> Should we return the payload (in hex) without the prefix?

I don't really have a preference. Done.


>> @@ -735,3 +735,32 @@ gdbpy_stop_recording (PyObject *self, PyObject *args)
>>
>>     Py_RETURN_NONE;
>>   }
>> +
>> +/* Used for registering the default ptwrite listener to the current thread.  A
>> +   pointer to this function is stored in the python extension interface.  */
>> +
>> +enum ext_lang_bt_status
>> +gdbpy_load_ptwrite_listener (const struct extension_language_defn *extlang,
>> +     ptid_t inferior_ptid)

Markus> PTWRITE is very PT specific.  Any chance this could go into py-record-btrace.c?

Done.

>> +{
>> +  struct gdbarch *gdbarch = NULL;
>> +
>> +  if (!gdb_python_initialized)
>> +    return EXT_LANG_BT_NO_FILTERS;
>> +
>> +  try
>> +    {
>> +      gdbarch = target_gdbarch ();
>> +    }
>> +  catch (const gdb_exception &except)
>> +    {
>> +      /* Let gdb try to print the stack trace.  */
>> +      return EXT_LANG_BT_NO_FILTERS;

Markus> I fail to understand the comment.

I copied it from gdbpy_apply_frame_filter() when creating the function. Removed.

Thanks,
Felix
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH 03/10] btrace: Enable auxiliary instructions in record function-call-history.

[felix.willgerodt]

From: Felix Willgerodt <felix.willgerodt@intel.com>

Print the auxiliary data when a btrace_insn of type BTRACE_INSN_AUX
is encountered in the function-call-history.  Printing is
active by default, it can be silenced with the /s modifier.

This patch is in preparation for the ptwrite feature, which is based on
auxiliary instructions.

2019-05-29  Felix Willgerodt  <felix.willgerodt@intel.com>

gdb/ChangeLog:
	* btrace.h (btrace_function_flag): Add BFUN_AUX_DECODED.
	* record-btrace.c (btrace_print_aux_insn): New function.
	(btrace_call_history): Handle BTRACE_INSN_AUX.
	* record.c: (get_call_history_modifiers): Add /s modifier.
	* record.h (record_print_flag): New value RECORD_DONT_PRINT_AUX.

gdb/doc/ChangeLog:
	* gdb.texinfo (record function-call-history): Document /s modifier.

---
 gdb/btrace.h        |  6 +++++-
 gdb/doc/gdb.texinfo |  5 +++--
 gdb/record-btrace.c | 21 +++++++++++++++++++++
 gdb/record.c        |  5 +++++
 gdb/record.h        |  5 ++++-
 5 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/gdb/btrace.h b/gdb/btrace.h
index 2a0e8664bf8..d29a1633ecf 100644
--- a/gdb/btrace.h
+++ b/gdb/btrace.h
@@ -104,7 +104,11 @@ enum btrace_function_flag
 
   /* The 'up' link points to a tail call.  This obviously only makes sense
      if bfun_up_links_to_ret is clear.  */
-  BFUN_UP_LINKS_TO_TAILCALL = (1 << 1)
+  BFUN_UP_LINKS_TO_TAILCALL = (1 << 1),
+
+  /* Indicates that at least one auxiliary instruction is in the current
+     function segment.  */
+  BFUN_AUX_DECODED = (1 << 2)
 };
 DEF_ENUM_FLAGS_TYPE (enum btrace_function_flag, btrace_function_flags);
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index c31ed39b251..20056e33a88 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -7300,8 +7300,9 @@ for this instruction sequence (if the @code{/l} modifier is
 specified), and the instructions numbers that form the sequence (if
 the @code{/i} modifier is specified).  The function names are indented
 to reflect the call stack depth if the @code{/c} modifier is
-specified.  The @code{/l}, @code{/i}, and @code{/c} modifiers can be
-given together.
+specified.  Printing auxiliary information is enabled by default and can be
+omitted with the @code{/s} modifier.  The @code{/l}, @code{/i}, @code{/s} and
+@code{/c} modifiers can be given together.
 
 @smallexample
 (@value{GDBP}) @b{list 1, 10}
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 5fb967c40f0..5f70c4838a9 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -1139,6 +1139,23 @@ btrace_get_bfun_name (const struct btrace_function *bfun)
     return "??";
 }
 
+static void
+btrace_print_aux_insn (struct ui_out *uiout,
+		       const struct btrace_function *bfun,
+		       const struct btrace_thread_info *btinfo)
+{
+  for (const auto &i : bfun->insn)
+    {
+      if (i.iclass == BTRACE_INSN_AUX)
+	{
+	  uiout->text ("\t\t[");
+	  uiout->field_fmt ("aux-insn", "%s",
+			    btinfo->aux_data[i.aux_data_index].c_str ());
+	  uiout->text ("]\n");
+	}
+    }
+}
+
 /* Disassemble a section of the recorded function trace.  */
 
 static void
@@ -1214,6 +1231,10 @@ btrace_call_history (struct ui_out *uiout,
 	}
 
       uiout->text ("\n");
+
+      if (((flags & RECORD_DONT_PRINT_AUX) == 0)
+	  && ((bfun->flags & BFUN_AUX_DECODED) != 0))
+	btrace_print_aux_insn(uiout, bfun, btinfo);
     }
 }
 
diff --git a/gdb/record.c b/gdb/record.c
index 828c19968a3..de02ca60513 100644
--- a/gdb/record.c
+++ b/gdb/record.c
@@ -650,6 +650,9 @@ get_call_history_modifiers (const char **arg)
 	    case 'c':
 	      modifiers |= RECORD_PRINT_INDENT_CALLS;
 	      break;
+	    case 's':
+	      modifiers |= RECORD_DONT_PRINT_AUX;
+	      break;
 	    default:
 	      error (_("Invalid modifier: %c."), *args);
 	    }
@@ -880,6 +883,8 @@ Without modifiers, it prints the function name.\n\
 With a /l modifier, the source file and line number range is included.\n\
 With a /i modifier, the instruction number range is included.\n\
 With a /c modifier, the output is indented based on the call stack depth.\n\
+With a /s modifier, omits output of auxiliary data, which is enabled \
+by default.\n\
 With no argument, prints ten more lines after the previous ten-line print.\n\
 \"record function-call-history -\" prints ten lines before a previous ten-line \
 print.\n\
diff --git a/gdb/record.h b/gdb/record.h
index 03f96e8ab78..7c4449480a3 100644
--- a/gdb/record.h
+++ b/gdb/record.h
@@ -61,7 +61,10 @@ enum record_print_flag
   RECORD_PRINT_INSN_RANGE = (1 << 1),
 
   /* Indent based on call stack depth (if applicable).  */
-  RECORD_PRINT_INDENT_CALLS = (1 << 2)
+  RECORD_PRINT_INDENT_CALLS = (1 << 2),
+
+  /* Deactivate printing auxiliary data (if applicable).  */
+  RECORD_DONT_PRINT_AUX = (1 << 3)
 };
 DEF_ENUM_FLAGS_TYPE (enum record_print_flag, record_print_flags);
 
-- 
2.20.1

Re: [PATCH 03/10] btrace: Enable auxiliary instructions in record function-call-history.

[Eli Zaretskii]

> From: felix.willgerodt@intel.com
> Cc: markus.t.metzger@intel.com, Felix Willgerodt <felix.willgerodt@intel.com>
> Date: Wed, 29 May 2019 10:47:46 +0200
> 
> gdb/doc/ChangeLog:
> 	* gdb.texinfo (record function-call-history): Document /s modifier.

This part is OK, thanks.

RE: [PATCH 03/10] btrace: Enable auxiliary instructions in record function-call-history.

[Metzger, Markus T]

Hello Felix,

> Print the auxiliary data when a btrace_insn of type BTRACE_INSN_AUX
> is encountered in the function-call-history.  Printing is
> active by default, it can be silenced with the /s modifier.

The /s modifier is used in 'record instruction-history' to mean 'interleave sources'.  I guess
we'd want this behavior there, as well.  And we'd also want the same modifier for both
commands.

> +static void
> +btrace_print_aux_insn (struct ui_out *uiout,
> +		       const struct btrace_function *bfun,
> +		       const struct btrace_thread_info *btinfo)
> +{
> +  for (const auto &i : bfun->insn)
> +    {
> +      if (i.iclass == BTRACE_INSN_AUX)
> +	{
> +	  uiout->text ("\t\t[");

We print the number and a single tab for gaps.  I'd do the same here.

> +	  uiout->field_fmt ("aux-insn", "%s",

Should this be spelled "aux-data" to align with the naming of the field?

> +			    btinfo->aux_data[i.aux_data_index].c_str ());
> +	  uiout->text ("]\n");
> +	}
> +    }
> +}
> +
>  /* Disassemble a section of the recorded function trace.  */
> 
>  static void
> @@ -1214,6 +1231,10 @@ btrace_call_history (struct ui_out *uiout,
>  	}
> 
>        uiout->text ("\n");
> +
> +      if (((flags & RECORD_DONT_PRINT_AUX) == 0)
> +	  && ((bfun->flags & BFUN_AUX_DECODED) != 0))
> +	btrace_print_aux_insn(uiout, bfun, btinfo);

This ignores the 'record function-call-history-size' setting.

>      }
>  }

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

RE: [PATCH 03/10] btrace: Enable auxiliary instructions in record function-call-history.

[Willgerodt, Felix]

On 6/4/19 2:35 PM, Metzger, Markus T wrote:
>Hello Felix,
>
>> Print the auxiliary data when a btrace_insn of type BTRACE_INSN_AUX is 
>> encountered in the function-call-history.  Printing is active by 
>> default, it can be silenced with the /s modifier.

Markus> The /s modifier is used in 'record instruction-history' to mean 'interleave sources'.  I guess we'd want this behavior there, as well.  And we'd also want the same modifier for both commands.

Done

>> +static void
>> +btrace_print_aux_insn (struct ui_out *uiout,
>> +		       const struct btrace_function *bfun,
>> +		       const struct btrace_thread_info *btinfo) {
>> +  for (const auto &i : bfun->insn)
> >+    {
> >+      if (i.iclass == BTRACE_INSN_AUX)
> >+	{
> >+	  uiout->text ("\t\t[");

Markus> We print the number and a single tab for gaps.  I'd do the same here.

I don't think we want that. I had that in an earlier revision, but we decided against it back then.
AUX insns are not function segments like gaps. There could be plenty of AUX insns in one function,
increasing the number by a lot. We would need to manipulate the bfun->number, as the aux
insn is a insn inside bfun, but a gap is an empty bfun.

Currently it looks like this:

record function-call-history 1,4
1       main
2       ptwrite64
                [payload: 0x42]
3       main
4       ptwrite32
               [payload: 0x43]
               [payload: 0x33]
5       main

This makes it consistent with the silenced version:

record function-call-history /a 1,4
1       main
2       ptwrite64
3       main
4       ptwrite32
5       main

>
>> +	  uiout->field_fmt ("aux-insn", "%s",
>
>Should this be spelled "aux-data" to align with the naming of the field?
>>
>>+			    btinfo->aux_data[i.aux_data_index].c_str ());
>>+	  uiout->text ("]\n");
>>+	}
>>+    }
>>+}
>>+
>> /* Disassemble a section of the recorded function trace.  */
>>
>> static void
>> @@ -1214,6 +1231,10 @@ btrace_call_history (struct ui_out *uiout,
>> 	}
>> 
>>        uiout->text ("\n");
>> +
>> +      if (((flags & RECORD_DONT_PRINT_AUX) == 0)
>> +	  && ((bfun->flags & BFUN_AUX_DECODED) != 0))
>> +	btrace_print_aux_insn(uiout, bfun, btinfo);

Markus> This ignores the 'record function-call-history-size' setting.


Isn't the size handled by the loop? The current behaviour looks fine to me:

(gdb) set record function-call-history-size 2
(gdb) record function-call-history 1
1    	main
2    	ptwrite64
		[payload: 0x42]
(gdb) set record function-call-history-size 3
(gdb) record function-call-history 1         
1	main
2	ptwrite64
		[payload: 0x42]
3	main

I don't think we want to start cutting the auxiliary data from a function.

Thanks,
Felix
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PATCH 03/10] btrace: Enable auxiliary instructions in record function-call-history.

[Metzger, Markus T]

Hello Felix,

Thanks for the patches.  I'm afraid reviews may take a while but I wanted to
comment on this one.

>Markus> We print the number and a single tab for gaps.  I'd do the same here.
>
>I don't think we want that. I had that in an earlier revision, but we decided
>against it back then.
>AUX insns are not function segments like gaps. There could be plenty of AUX
>insns in one function,
>increasing the number by a lot. We would need to manipulate the bfun-
>>number, as the aux
>insn is a insn inside bfun, but a gap is an empty bfun.
>
>Currently it looks like this:
>
>record function-call-history 1,4
>1       main
>2       ptwrite64
>                [payload: 0x42]
>3       main
>4       ptwrite32
>               [payload: 0x43]
>               [payload: 0x33]
>5       main

I agree this output looks nice.  One problem with it, though, is that the user asked
for 4 lines of output and we give 8.

That's also what triggered this comment ...

>>> +      if (((flags & RECORD_DONT_PRINT_AUX) == 0)
>>> +  && ((bfun->flags & BFUN_AUX_DECODED) != 0))
>>> +btrace_print_aux_insn(uiout, bfun, btinfo);
>
>Markus> This ignores the 'record function-call-history-size' setting.

I agree that those AUX records are neither instructions nor gaps.  If we viewed
this similar to interleaved source, getting more output lines than you asked for
is OK.

Regards,
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PATCH 03/10] btrace: Enable auxiliary instructions in record function-call-history.

[Willgerodt, Felix]

Markus> I agree this output looks nice.  One problem with it, though, is that the user asked
Markus> for 4 lines of output and we give 8.

I had assumed this is only about functions, not lines. According to the docs:

record function-call-history begin, end
Prints functions beginning with function number begin until function number end.
The function number end is included.

The docs mention mostly functions, but are a bit inconsistent and mention lines occasionally.
Which is understandable, as they are equivalent without my patches.
The size setting only mentions lines, I didn't realize that before:

set record function-call-history-size size
set record function-call-history-size unlimited
Define how many lines to print in the record function-call-history command. (...)

For the instruction-history the docs don't ever mention lines, e.g.: 

set record instruction-history-size size
set record instruction-history-size unlimited
Define how many instructions to disassemble in the record instruction-history command. (...)

I propose to update the documentation to not mention lines (but functions) for
'set record function-call-history-size'. I know changing "existing documented behavior" is
usually not the best idea, but in my view it clearly improves usability in this case. 
It could be seen as improving the docs - not changing behavior - anyway, as they are inconsistent.
What do you think?

Regards,
Felix


Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PATCH 03/10] btrace: Enable auxiliary instructions in record function-call-history.

[Metzger, Markus T]

>I propose to update the documentation to not mention lines (but functions) for
>'set record function-call-history-size'. I know changing "existing documented
>behavior" is
>usually not the best idea, but in my view it clearly improves usability in this case.
>It could be seen as improving the docs - not changing behavior - anyway, as they
>are inconsistent.
>What do you think?

Sounds reasonable to me.

I'd consider it a clarification since for /m or /s, we also do not count the source
lines that are printed.

Thanks,
Markus.

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH 09/10] btrace, python: Enable calling the ptwrite listener.

[felix.willgerodt]

From: Felix Willgerodt <felix.willgerodt@intel.com>

Adding a new function to btinfo that allows to call the python ptwrite listener
from inside GDB.

2019-05-29  Felix Willgerodt  <felix.willgerodt@intel.com>

gdb/ChangeLog:
	* btrace.h (btrace_thread_info): New member ptw_callback_fun.
	* python/py-record-btrace.c (recpy_call_listener): New function.
	(recpy_initialize_listener): Save recpy_call_listener in btinfo.

---
 gdb/btrace.h                  |  8 +++++
 gdb/python/py-record-btrace.c | 63 +++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+)

diff --git a/gdb/btrace.h b/gdb/btrace.h
index 7a80f5a44e1..7ad3cb65a28 100644
--- a/gdb/btrace.h
+++ b/gdb/btrace.h
@@ -354,6 +354,14 @@ struct btrace_thread_info
      stepping through the execution history.  */
   std::vector<std::string> aux_data;
 
+  /* Function pointer to the ptwrite callback.  Returns the string returned
+     by the ptwrite listener function or nullptr if no string is supposed to
+     be printed.  */
+  gdb::unique_xmalloc_ptr<char> (*ptw_callback_fun) (
+						const uint64_t *payload,
+						const uint64_t *ip,
+						const void *ptw_listener);
+
   /* PyObject pointer to the ptwrite listener function.  */
   void *ptw_listener = nullptr;
 
diff --git a/gdb/python/py-record-btrace.c b/gdb/python/py-record-btrace.c
index c7ad47d64a0..42a0c29a348 100644
--- a/gdb/python/py-record-btrace.c
+++ b/gdb/python/py-record-btrace.c
@@ -772,6 +772,68 @@ recpy_bt_function_call_history (PyObject *self, void *closure)
   return btpy_list_new (tinfo, first, last, 1, &recpy_func_type);
 }
 
+/* Calling the ptwrite listener.  Returns a pointer to the string that will be
+   printed or nullptr if nothing should be printed.  */
+gdb::unique_xmalloc_ptr<char>
+recpy_call_listener (const uint64_t *payload, const uint64_t *ip,
+		     const void *ptw_listener)
+{
+  if ((PyObject *) ptw_listener == Py_None)
+    return nullptr;
+  else if ((PyObject *) ptw_listener == nullptr)
+    error (_("No valid ptwrite listener."));
+
+  /* As Python is started as a seperate thread, we need to
+     acquire the GIL to safely call the listener function.  */
+  PyGILState_STATE gstate = PyGILState_Ensure ();
+
+  PyObject *py_ip = Py_None;
+  PyObject *py_payload = PyLong_FromUnsignedLongLong (*payload);
+  Py_INCREF (Py_None);
+
+  if (ip != nullptr)
+    py_ip = PyLong_FromUnsignedLongLong (*ip);
+
+  PyObject *py_result = PyObject_CallFunctionObjArgs ((PyObject *) ptw_listener,
+						      py_payload, py_ip, NULL);
+
+  if (PyErr_Occurred ())
+    {
+      gdbpy_print_stack ();
+      gdb_Py_DECREF (py_ip);
+      gdb_Py_DECREF (py_payload);
+      gdb_Py_DECREF (py_result);
+      PyGILState_Release (gstate);
+      error (_("Error while executing Python code."));
+    }
+
+  gdb_Py_DECREF (py_ip);
+  gdb_Py_DECREF (py_payload);
+
+  if (py_result == Py_None)
+    {
+      gdb_Py_DECREF (py_result);
+      PyGILState_Release (gstate);
+      return nullptr;
+    }
+
+  gdb::unique_xmalloc_ptr<char> resultstring = gdbpy_obj_to_string (py_result);
+
+  if (PyErr_Occurred ())
+    {
+      gdbpy_print_stack ();
+      gdb_Py_DECREF (py_result);
+      PyGILState_Release (gstate);
+      error (_("Error while executing Python code."));
+    }
+
+  if (py_result != nullptr)
+    gdb_Py_DECREF (py_result);
+  PyGILState_Release (gstate);
+
+  return resultstring;
+}
+
 /* Helper function returning the current ptwrite listener.  Returns nullptr
    in case of errors.  */
 
@@ -807,6 +869,7 @@ recpy_initialize_listener (ptid_t inferior_ptid)
 
   gdb_assert (tinfo != nullptr);
 
+  tinfo->btrace.ptw_callback_fun = &recpy_call_listener;
   tinfo->btrace.ptw_listener = get_ptwrite_listener ();
 
   return (PyObject *) tinfo->btrace.ptw_listener;
-- 
2.20.1

RE: [PATCH 09/10] btrace, python: Enable calling the ptwrite listener.

[Metzger, Markus T]

Hello Felix,

> diff --git a/gdb/btrace.h b/gdb/btrace.h
> index 7a80f5a44e1..7ad3cb65a28 100644
> --- a/gdb/btrace.h
> +++ b/gdb/btrace.h
> @@ -354,6 +354,14 @@ struct btrace_thread_info
>       stepping through the execution history.  */
>    std::vector<std::string> aux_data;
> 
> +  /* Function pointer to the ptwrite callback.  Returns the string returned
> +     by the ptwrite listener function or nullptr if no string is supposed to
> +     be printed.  */
> +  gdb::unique_xmalloc_ptr<char> (*ptw_callback_fun) (
> +						const uint64_t *payload,
> +						const uint64_t *ip,
> +						const void *ptw_listener);
> +
>    /* PyObject pointer to the ptwrite listener function.  */
>    void *ptw_listener = nullptr;
> 
> diff --git a/gdb/python/py-record-btrace.c b/gdb/python/py-record-btrace.c
> index c7ad47d64a0..42a0c29a348 100644
> --- a/gdb/python/py-record-btrace.c
> +++ b/gdb/python/py-record-btrace.c
> @@ -772,6 +772,68 @@ recpy_bt_function_call_history (PyObject *self, void
> *closure)
>    return btpy_list_new (tinfo, first, last, 1, &recpy_func_type);
>  }
> 
> +/* Calling the ptwrite listener.  Returns a pointer to the string that will be
> +   printed or nullptr if nothing should be printed.  */
> +gdb::unique_xmalloc_ptr<char>
> +recpy_call_listener (const uint64_t *payload, const uint64_t *ip,
> +		     const void *ptw_listener)
> +{
> +  if ((PyObject *) ptw_listener == Py_None)
> +    return nullptr;
> +  else if ((PyObject *) ptw_listener == nullptr)
> +    error (_("No valid ptwrite listener."));

PTW_LISTENER is declared void * in struct btrace_info.  Couldn't we treat nullptr
as not-present?

> +
> +  /* As Python is started as a seperate thread, we need to
> +     acquire the GIL to safely call the listener function.  */
> +  PyGILState_STATE gstate = PyGILState_Ensure ();
> +
> +  PyObject *py_ip = Py_None;
> +  PyObject *py_payload = PyLong_FromUnsignedLongLong (*payload);
> +  Py_INCREF (Py_None);

Are we dropping that reference again in the case that PY_IP gets overwritten?

> +
> +  if (ip != nullptr)
> +    py_ip = PyLong_FromUnsignedLongLong (*ip);

It wouldn't hurt to document that IP can be NULL whereas PAYLOAD cannot
in the function's comment.

> +
> +  PyObject *py_result = PyObject_CallFunctionObjArgs ((PyObject *)
> ptw_listener,
> +						      py_payload, py_ip, NULL);
> +
> +  if (PyErr_Occurred ())
> +    {
> +      gdbpy_print_stack ();
> +      gdb_Py_DECREF (py_ip);
> +      gdb_Py_DECREF (py_payload);
> +      gdb_Py_DECREF (py_result);
> +      PyGILState_Release (gstate);
> +      error (_("Error while executing Python code."));
> +    }
> +
> +  gdb_Py_DECREF (py_ip);
> +  gdb_Py_DECREF (py_payload);
> +
> +  if (py_result == Py_None)
> +    {
> +      gdb_Py_DECREF (py_result);
> +      PyGILState_Release (gstate);
> +      return nullptr;
> +    }
> +
> +  gdb::unique_xmalloc_ptr<char> resultstring = gdbpy_obj_to_string
> (py_result);
> +
> +  if (PyErr_Occurred ())
> +    {
> +      gdbpy_print_stack ();
> +      gdb_Py_DECREF (py_result);
> +      PyGILState_Release (gstate);
> +      error (_("Error while executing Python code."));
> +    }
> +
> +  if (py_result != nullptr)
> +    gdb_Py_DECREF (py_result);

Shouldn't we check that before using PY_RESULT above?

> +  PyGILState_Release (gstate);
> +
> +  return resultstring;
> +}
> +
>  /* Helper function returning the current ptwrite listener.  Returns nullptr
>     in case of errors.  */
> 
> @@ -807,6 +869,7 @@ recpy_initialize_listener (ptid_t inferior_ptid)
> 
>    gdb_assert (tinfo != nullptr);
> 
> +  tinfo->btrace.ptw_callback_fun = &recpy_call_listener;
>    tinfo->btrace.ptw_listener = get_ptwrite_listener ();
> 
>    return (PyObject *) tinfo->btrace.ptw_listener;
> --
> 2.20.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, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH 09/10] btrace, python: Enable calling the ptwrite listener.

[Willgerodt, Felix]

On 6/4/19 2:37 PM, Metzger, Markus T wrote:
> Hello Felix,
>
>> diff --git a/gdb/btrace.h b/gdb/btrace.h
>> index 7a80f5a44e1..7ad3cb65a28 100644
>> --- a/gdb/btrace.h
>> +++ b/gdb/btrace.h
>> @@ -354,6 +354,14 @@ struct btrace_thread_info
>>        stepping through the execution history.  */
>>     std::vector<std::string> aux_data;
>>
>> +  /* Function pointer to the ptwrite callback.  Returns the string returned
>> +     by the ptwrite listener function or nullptr if no string is supposed to
>> +     be printed.  */
>> +  gdb::unique_xmalloc_ptr<char> (*ptw_callback_fun) (
>> +const uint64_t *payload,
>> +const uint64_t *ip,
>> +const void *ptw_listener);
>> +
>>     /* PyObject pointer to the ptwrite listener function.  */
>>     void *ptw_listener = nullptr;
>>
>> diff --git a/gdb/python/py-record-btrace.c b/gdb/python/py-record-btrace.c
>> index c7ad47d64a0..42a0c29a348 100644
>> --- a/gdb/python/py-record-btrace.c
>> +++ b/gdb/python/py-record-btrace.c
>> @@ -772,6 +772,68 @@ recpy_bt_function_call_history (PyObject *self, void
>> *closure)
>>     return btpy_list_new (tinfo, first, last, 1, &recpy_func_type);
>>   }
>>
>> +/* Calling the ptwrite listener.  Returns a pointer to the string that will be
>> +   printed or nullptr if nothing should be printed.  */
>> +gdb::unique_xmalloc_ptr<char>
>> +recpy_call_listener (const uint64_t *payload, const uint64_t *ip,
>> +     const void *ptw_listener)
>> +{
>> +  if ((PyObject *) ptw_listener == Py_None)
>> +    return nullptr;
>> +  else if ((PyObject *) ptw_listener == nullptr)
>> +    error (_("No valid ptwrite listener."));

Markus> PTW_LISTENER is declared void * in struct btrace_info.  Couldn't we treat nullptr
Markus> as not-present?

If ptw_listener is a nullptr, something went rather wrong. We have a listener active by
default and can only "deactivate" the listener by registering Py_None. I don't think this
extra check hurts. But I don't have a strong opinion if you want it removed.


>> +
>> +  /* As Python is started as a seperate thread, we need to
>> +     acquire the GIL to safely call the listener function.  */
>> +  PyGILState_STATE gstate = PyGILState_Ensure ();
>> +
>> +  PyObject *py_ip = Py_None;
>> +  PyObject *py_payload = PyLong_FromUnsignedLongLong (*payload);
>> +  Py_INCREF (Py_None);

Markus> Are we dropping that reference again in the case that PY_IP gets overwritten?

We are now, thanks.

>> +
>> +  if (ip != nullptr)
>> +    py_ip = PyLong_FromUnsignedLongLong (*ip);

Markus> It wouldn't hurt to document that IP can be NULL whereas PAYLOAD cannot
Markus> in the function's comment.

Done

>> +
>> +  PyObject *py_result = PyObject_CallFunctionObjArgs ((PyObject *)
>> ptw_listener,
>> +      py_payload, py_ip, NULL);
>> +
>> +  if (PyErr_Occurred ())
>> +    {
>> +      gdbpy_print_stack ();
>> +      gdb_Py_DECREF (py_ip);
>> +      gdb_Py_DECREF (py_payload);
>> +      gdb_Py_DECREF (py_result);
>> +      PyGILState_Release (gstate);
>> +      error (_("Error while executing Python code."));
>> +    }
>> +
>> +  gdb_Py_DECREF (py_ip);
>> +  gdb_Py_DECREF (py_payload);
>> +
>> +  if (py_result == Py_None)
>> +    {
>> +      gdb_Py_DECREF (py_result);
>> +      PyGILState_Release (gstate);
>> +      return nullptr;
>> +    }
>> +
>> +  gdb::unique_xmalloc_ptr<char> resultstring = gdbpy_obj_to_string
>> (py_result);
>> +
>> +  if (PyErr_Occurred ())
>> +    {
>> +      gdbpy_print_stack ();
>> +      gdb_Py_DECREF (py_result);
>> +      PyGILState_Release (gstate);
>> +      error (_("Error while executing Python code."));
>> +    }
>> +
>> +  if (py_result != nullptr)
>> +    gdb_Py_DECREF (py_result);

Markus> Shouldn't we check that before using PY_RESULT above?

I removed the check as it shouldn't be necessary at all.
PyObject_CallFunctionObjArgs only returns nullptr if an error occured,
but that is already checked via PyErr_Occurred ().

Thanks,
Felix
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH 07/10] btrace, linux: Enable ptwrite packets.

[felix.willgerodt]

From: Felix Willgerodt <felix.willgerodt@intel.com>

Enable ptwrite in the PT config if the kernel supports it.

2019-05-29  Felix Willgerodt  <felix.willgerodt@intel.com>

gdb/ChangeLog:
	* nat/linux-btrace.c (linux_supports_ptw): New function.
	(linux_enable_pt): Set attr.config if ptwrite is supported.

---
 gdb/nat/linux-btrace.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/gdb/nat/linux-btrace.c b/gdb/nat/linux-btrace.c
index ef291ec2310..6eb5334e0ae 100644
--- a/gdb/nat/linux-btrace.c
+++ b/gdb/nat/linux-btrace.c
@@ -32,6 +32,10 @@
 
 #include <sys/syscall.h>
 
+#if defined (HAVE_LIBIPT)
+#include <intel-pt.h>
+#endif
+
 #if HAVE_LINUX_PERF_EVENT_H && defined(SYS_perf_event_open)
 #include <unistd.h>
 #include <sys/mman.h>
@@ -409,6 +413,31 @@ cpu_supports_bts (void)
     }
 }
 
+#if defined (PERF_ATTR_SIZE_VER5)
+/* Check whether the linux target supports Intel Processor Trace PTWRITE.  */
+
+static bool
+linux_supports_ptwrite ()
+{
+  static const char filename[]
+      = "/sys/bus/event_source/devices/intel_pt/caps/ptwrite";
+  gdb_file_up file = gdb_fopen_cloexec (filename, "r");
+
+  if (file.get () == nullptr)
+    return false;
+
+  int status, found = fscanf (file.get (), "%d", &status);
+
+  if (found != 1)
+    {
+      warning (_("Failed to determine ptwrite support from %s."), filename);
+      return false;
+    }
+
+  return status == 1;
+}
+#endif /* defined (PERF_ATTR_SIZE_VER5) */
+
 /* The perf_event_open syscall failed.  Try to print a helpful error
    message.  */
 
@@ -601,6 +630,9 @@ linux_enable_pt (ptid_t ptid, const struct btrace_config_pt *conf)
   pt->attr.exclude_hv = 1;
   pt->attr.exclude_idle = 1;
 
+  if (linux_supports_ptwrite ())
+    pt->attr.config |= 0x1000;
+
   errno = 0;
   scoped_fd fd (syscall (SYS_perf_event_open, &pt->attr, pid, -1, -1, 0));
   if (fd.get () < 0)
-- 
2.20.1

RE: [PATCH 07/10] btrace, linux: Enable ptwrite packets.

[Metzger, Markus T]

Hello Felix,

> diff --git a/gdb/nat/linux-btrace.c b/gdb/nat/linux-btrace.c
> index ef291ec2310..6eb5334e0ae 100644
> --- a/gdb/nat/linux-btrace.c
> +++ b/gdb/nat/linux-btrace.c
> @@ -32,6 +32,10 @@
> 
>  #include <sys/syscall.h>
> 
> +#if defined (HAVE_LIBIPT)
> +#include <intel-pt.h>
> +#endif

This code is used for gdbserver, as well, which doesn't link against libipt.so.  Also, we don't
seem to be using anything from that header - and we shouldn't.

> +
>  #if HAVE_LINUX_PERF_EVENT_H && defined(SYS_perf_event_open)
>  #include <unistd.h>
>  #include <sys/mman.h>
> @@ -409,6 +413,31 @@ cpu_supports_bts (void)
>      }
>  }
> 
> +#if defined (PERF_ATTR_SIZE_VER5)

This guard shouldn't be needed.  We're not using anything from struct perf_event_attr here.

> +/* Check whether the linux target supports Intel Processor Trace PTWRITE.  */
> +
> +static bool
> +linux_supports_ptwrite ()
> +{
> +  static const char filename[]
> +      = "/sys/bus/event_source/devices/intel_pt/caps/ptwrite";
> +  gdb_file_up file = gdb_fopen_cloexec (filename, "r");
> +
> +  if (file.get () == nullptr)
> +    return false;
> +
> +  int status, found = fscanf (file.get (), "%d", &status);
> +
> +  if (found != 1)
> +    {
> +      warning (_("Failed to determine ptwrite support from %s."), filename);
> +      return false;
> +    }
> +
> +  return status == 1;
> +}
> +#endif /* defined (PERF_ATTR_SIZE_VER5) */
> +
>  /* The perf_event_open syscall failed.  Try to print a helpful error
>     message.  */
> 
> @@ -601,6 +630,9 @@ linux_enable_pt (ptid_t ptid, const struct
> btrace_config_pt *conf)
>    pt->attr.exclude_hv = 1;
>    pt->attr.exclude_idle = 1;
> 
> +  if (linux_supports_ptwrite ())
> +    pt->attr.config |= 0x1000;

Doing this unconditionally may break GDBs that have been built with an old libipt that
doesn't support the PTW packet.

We should probably have GDB request PTW via CONF.  This may need extending RSP.

> +
>    errno = 0;
>    scoped_fd fd (syscall (SYS_perf_event_open, &pt->attr, pid, -1, -1, 0));
>    if (fd.get () < 0)
> --
> 2.20.1

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

RE: [PATCH 07/10] btrace, linux: Enable ptwrite packets.

[Willgerodt, Felix]

On 6/4/19 2:35 PM, Metzger, Markus T wrote:
>> diff --git a/gdb/nat/linux-btrace.c b/gdb/nat/linux-btrace.c index 
>> ef291ec2310..6eb5334e0ae 100644
>> --- a/gdb/nat/linux-btrace.c
>> +++ b/gdb/nat/linux-btrace.c
>> @@ -32,6 +32,10 @@
>> 
>>  #include <sys/syscall.h>
>> 
>> +#if defined (HAVE_LIBIPT)
>> +#include <intel-pt.h>
>> +#endif

Markus> This code is used for gdbserver, as well, which doesn't link against libipt.so.  Also, we don't seem to be using anything from that header - and we shouldn't.

Was a leftover rebase mistake on my side. Removed.

>> +
>>  #if HAVE_LINUX_PERF_EVENT_H && defined(SYS_perf_event_open)  #include 
>> <unistd.h>  #include <sys/mman.h> @@ -409,6 +413,31 @@ 
>> cpu_supports_bts (void)
>>      }
>>  }
>> 
>> +#if defined (PERF_ATTR_SIZE_VER5)

Markus> This guard shouldn't be needed.  We're not using anything from struct perf_event_attr here.

Removed.

>> +  if (linux_supports_ptwrite ())
>> +    pt->attr.config |= 0x1000;

Markus> Doing this unconditionally may break GDBs that have been built with an old libipt that doesn't support the PTW packet.
Markus> We should probably have GDB request PTW via CONF.  This may need extending RSP.

I finally got around to extending RSP. Sorry for taking so long.

Thanks,
Felix
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH 06/10] python: Add clear_trace() to gdb.Record.

[felix.willgerodt]

From: Felix Willgerodt <felix.willgerodt@intel.com>

This function allows to clear the trace data from python, forcing to
re-decode the trace for successive commands.

2019-05-29  Felix Willgerodt  <felix.willgerodt@intel.com>

gdb/ChangeLog:
	* py-record-btrace.c (recpy_bt_clear): New function.
	* py-record-btrace.h (recpy_bt_clear): New export.
	* py-record.c (recpy_clear): New function.
	(recpy_record_methods): Add clear_trace.

testsuite/ChangeLog:
	* gdb.python/py-record-btrace.exp: Add tests for btrace_clear.

gdb/doc/ChangeLog:
	* python.texi (gdb.Record): Document btrace_clear().

---
 gdb/doc/python.texi                           |  5 +++++
 gdb/python/py-record-btrace.c                 | 13 +++++++++++++
 gdb/python/py-record-btrace.h                 |  3 +++
 gdb/python/py-record.c                        | 16 ++++++++++++++++
 gdb/testsuite/gdb.python/py-record-btrace.exp |  6 +++++-
 5 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index d4149f364e1..e892caf9e18 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -3439,6 +3439,11 @@ A @code{gdb.Record} object has the following methods:
 Move the replay position to the given @var{instruction}.
 @end defun
 
+@defun Record.clear_trace ()
+Clear the trace data of the current recording.  This forces re-decoding of the
+trace for successive commands.
+@end defun
+
 The common @code{gdb.Instruction} class that recording method specific
 instruction objects inherit from, has the following attributes:
 
diff --git a/gdb/python/py-record-btrace.c b/gdb/python/py-record-btrace.c
index 81e43a00516..2a4af551146 100644
--- a/gdb/python/py-record-btrace.c
+++ b/gdb/python/py-record-btrace.c
@@ -811,6 +811,19 @@ recpy_bt_goto (PyObject *self, PyObject *args)
   Py_RETURN_NONE;
 }
 
+/* Implementation of BtraceRecord.clear (self) -> None.  */
+
+PyObject *
+recpy_bt_clear (PyObject *self, PyObject *args)
+{
+  const recpy_record_object * const record = (recpy_record_object *) self;
+  thread_info *const tinfo = record->thread;
+
+  btrace_clear (tinfo);
+
+  Py_RETURN_NONE;
+}
+
 /* BtraceList methods.  */
 
 struct PyMethodDef btpy_list_methods[] =
diff --git a/gdb/python/py-record-btrace.h b/gdb/python/py-record-btrace.h
index 1d8560dace3..8056a5b75cb 100644
--- a/gdb/python/py-record-btrace.h
+++ b/gdb/python/py-record-btrace.h
@@ -31,6 +31,9 @@ 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 BtraceRecord.clear (self) -> None.  */
+extern PyObject *recpy_bt_clear (PyObject *self, PyObject *args);
+
 /* Implementation of record.instruction_history [list].  */
 extern PyObject *recpy_bt_instruction_history (PyObject *self, void *closure);
 
diff --git a/gdb/python/py-record.c b/gdb/python/py-record.c
index 8b1b3c4e651..855b4089d87 100644
--- a/gdb/python/py-record.c
+++ b/gdb/python/py-record.c
@@ -127,6 +127,19 @@ recpy_goto (PyObject *self, PyObject *value)
   return PyErr_Format (PyExc_NotImplementedError, _("Not implemented."));
 }
 
+/* Implementation of record.clear_trace () -> None.  */
+
+static PyObject *
+recpy_clear (PyObject *self, PyObject *value)
+{
+  const recpy_record_object * const obj = (recpy_record_object *) self;
+
+  if (obj->method == RECORD_METHOD_BTRACE)
+    return recpy_bt_clear (self, value);
+
+  return PyErr_Format (PyExc_NotImplementedError, _("Not implemented."));
+}
+
 /* Implementation of record.replay_position [instruction]  */
 
 static PyObject *
@@ -538,6 +551,9 @@ static PyMethodDef recpy_record_methods[] = {
   { "goto", recpy_goto, METH_VARARGS,
     "goto (instruction|function_call) -> None.\n\
 Rewind to given location."},
+  { "clear_trace", recpy_clear, METH_VARARGS,
+    "clear_trace () -> None.\n\
+Clears the trace."},
   { NULL }
 };
 
diff --git a/gdb/testsuite/gdb.python/py-record-btrace.exp b/gdb/testsuite/gdb.python/py-record-btrace.exp
index f6267d664a4..72bb69d5908 100644
--- a/gdb/testsuite/gdb.python/py-record-btrace.exp
+++ b/gdb/testsuite/gdb.python/py-record-btrace.exp
@@ -90,7 +90,11 @@ with_test_prefix "instruction " {
     }
     gdb_test "python print(i.decoded)" ".*"
     gdb_test "python print(i.size)" "$decimal"
-    gdb_test "python print(i.is_speculative)" "False"
+    gdb_test "python print(i.is_speculative)" "False"    
+    gdb_test_no_output "python gdb.current_recording().clear_trace()"
+    gdb_test "python insn = r.instruction_history"
+    gdb_test_no_output "python i = insn\[0\]"
+    gdb_test "python print(i.size)" "$decimal"
 }
 
 with_test_prefix "function call" {
-- 
2.20.1

Re: [PATCH 06/10] python: Add clear_trace() to gdb.Record.

[Eli Zaretskii]

> From: felix.willgerodt@intel.com
> Cc: markus.t.metzger@intel.com, Felix Willgerodt <felix.willgerodt@intel.com>
> Date: Wed, 29 May 2019 10:47:49 +0200
> 
> gdb/doc/ChangeLog:
> 	* python.texi (gdb.Record): Document btrace_clear().

OK for this part.

RE: [PATCH 06/10] python: Add clear_trace() to gdb.Record.

[Metzger, Markus T]

Hello Felix,

> diff --git a/gdb/python/py-record-btrace.c b/gdb/python/py-record-btrace.c
> index 81e43a00516..2a4af551146 100644
> --- a/gdb/python/py-record-btrace.c
> +++ b/gdb/python/py-record-btrace.c
> @@ -811,6 +811,19 @@ recpy_bt_goto (PyObject *self, PyObject *args)
>    Py_RETURN_NONE;
>  }
> 
> +/* Implementation of BtraceRecord.clear (self) -> None.  */

This does not match the documentation, which uses 'clear_trace'.  More
instances below.

Personally, I like 'clear' better since it is shorter and the context should be
clear enough given that it is a Record method.

> @@ -127,6 +127,19 @@ recpy_goto (PyObject *self, PyObject *value)
>    return PyErr_Format (PyExc_NotImplementedError, _("Not implemented."));
>  }
> 
> +/* Implementation of record.clear_trace () -> None.  */
> +
> +static PyObject *
> +recpy_clear (PyObject *self, PyObject *value)

Comment and function name don't match.

> diff --git a/gdb/testsuite/gdb.python/py-record-btrace.exp
> b/gdb/testsuite/gdb.python/py-record-btrace.exp
> index f6267d664a4..72bb69d5908 100644
> --- a/gdb/testsuite/gdb.python/py-record-btrace.exp
> +++ b/gdb/testsuite/gdb.python/py-record-btrace.exp
> @@ -90,7 +90,11 @@ with_test_prefix "instruction " {
>      }
>      gdb_test "python print(i.decoded)" ".*"
>      gdb_test "python print(i.size)" "$decimal"
> -    gdb_test "python print(i.is_speculative)" "False"
> +    gdb_test "python print(i.is_speculative)" "False"
> +    gdb_test_no_output "python gdb.current_recording().clear_trace()"

Why are we not using 'r' as we do below?

I wonder what would happen if we accessed 'i', 'c', 'insn', or 'call' here.

> +    gdb_test "python insn = r.instruction_history"
> +    gdb_test_no_output "python i = insn\[0\]"
> +    gdb_test "python print(i.size)" "$decimal"
>  }
> 
>  with_test_prefix "function call" {
> --
> 2.20.1

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

Re: [PATCH 06/10] python: Add clear_trace() to gdb.Record.

[Willgerodt, Felix]

On 6/4/19 2:36 PM, Metzger, Markus T wrote:
> Hello Felix,
>
>> diff --git a/gdb/python/py-record-btrace.c b/gdb/python/py-record-btrace.c
>> index 81e43a00516..2a4af551146 100644
>> --- a/gdb/python/py-record-btrace.c
>> +++ b/gdb/python/py-record-btrace.c
>> @@ -811,6 +811,19 @@ recpy_bt_goto (PyObject *self, PyObject *args)
>>     Py_RETURN_NONE;
>>   }
>>
>> +/* Implementation of BtraceRecord.clear (self) -> None.  */

Markus> This does not match the documentation, which uses 'clear_trace'.  More
Markus> instances below.
Markus>
Markus> Personally, I like 'clear' better since it is shorter and the context should be
Markus> clear enough given that it is a Record method.

Changed it to clear.

>> @@ -127,6 +127,19 @@ recpy_goto (PyObject *self, PyObject *value)
>>     return PyErr_Format (PyExc_NotImplementedError, _("Not implemented."));
>>   }
>>
>> +/* Implementation of record.clear_trace () -> None.  */
>> +
>> +static PyObject *
>> +recpy_clear (PyObject *self, PyObject *value)

Markus> Comment and function name don't match.

Done

>
>> diff --git a/gdb/testsuite/gdb.python/py-record-btrace.exp
>> b/gdb/testsuite/gdb.python/py-record-btrace.exp
>> index f6267d664a4..72bb69d5908 100644
>> --- a/gdb/testsuite/gdb.python/py-record-btrace.exp
>> +++ b/gdb/testsuite/gdb.python/py-record-btrace.exp
>> @@ -90,7 +90,11 @@ with_test_prefix "instruction " {
>>       }
>>       gdb_test "python print(i.decoded)" ".*"
>>       gdb_test "python print(i.size)" "$decimal"
>> -    gdb_test "python print(i.is_speculative)" "False"
>> +    gdb_test "python print(i.is_speculative)" "False"
>> +    gdb_test_no_output "python gdb.current_recording().clear_trace()"

Markus> Why are we not using 'r' as we do below?

No reason, just missed that. Done.

Markus> I wonder what would happen if we accessed 'i', 'c', 'insn', or 'call' here.

The objects are still there but they lost their trace reference until
re-decoding is triggered. An example with 'i':

(gdb) pi
 >>> print i
<gdb.RecordInstruction object at 0x7f71605806e8>
 >>> print i.decoded
movl   $0x0,-0x4(%rbp)
 >>> r.clear()
 >>> print i
<gdb.RecordInstruction object at 0x7f71605806e8>
 >>> print i.decoded
Traceback (most recent call last):
   File "<stdin>", line 1, in <module>
gdb.error: No such instruction.
 >>> insn = r.instruction_history
 >>> print i.decoded
movl   $0x0,-0x4(%rbp)


Thanks,
Felix
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH 01/10] btrace: Introduce auxiliary instructions.

[felix.willgerodt]

From: Felix Willgerodt <felix.willgerodt@intel.com>

Auxiliary instructions are pseudo instructions pointing to auxiliary data.
This auxiliary data can be printed in all commands displaying (record
function-call-history, record instruction-history) or stepping through
(stepi etc.) the execution history, which will be introduced in the next
commits.

This patch is in preparation for the ptwrite feature, which is based on
auxiliary instructions.

2019-05-29  Felix Willgerodt  <felix.willgerodt@intel.com>

gdb/ChangeLog:
	* btrace.c (btrace_clear_history): Clear aux_data.
	* btrace.h (btrace_insn_class): Add BTRACE_INSN_AUX.
	(btrace_insn): New union.
	(btrace_thread_info): New member aux_data.

gdb/doc/ChangeLog:
	* gdb.texinfo (Process Record and Replay): Document printing of auxiliary
	information.

---
 gdb/btrace.c        |  2 ++
 gdb/btrace.h        | 25 +++++++++++++++++++++----
 gdb/doc/gdb.texinfo |  3 +++
 3 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/gdb/btrace.c b/gdb/btrace.c
index c6d564e7062..d3e2f46c678 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -1810,6 +1810,8 @@ btrace_clear_history (struct btrace_thread_info *btinfo)
   btinfo->insn_history = NULL;
   btinfo->call_history = NULL;
   btinfo->replay = NULL;
+
+  btinfo->aux_data.clear ();
 }
 
 /* Clear the branch trace maintenance histories in BTINFO.  */
diff --git a/gdb/btrace.h b/gdb/btrace.h
index 7b38b14ac01..2a0e8664bf8 100644
--- a/gdb/btrace.h
+++ b/gdb/btrace.h
@@ -52,7 +52,10 @@ enum btrace_insn_class
   BTRACE_INSN_RETURN,
 
   /* The instruction is an unconditional jump.  */
-  BTRACE_INSN_JUMP
+  BTRACE_INSN_JUMP,
+
+  /* The instruction is a pseudo instruction containing auxiliary data.  */
+  BTRACE_INSN_AUX
 };
 
 /* Instruction flags.  */
@@ -68,9 +71,19 @@ DEF_ENUM_FLAGS_TYPE (enum btrace_insn_flag, btrace_insn_flags);
    This represents a single instruction in a branch trace.  */
 struct btrace_insn
 {
-  /* The address of this instruction.  */
-  CORE_ADDR pc;
-
+  union
+  {
+    /* The address of this instruction.  Applies to btrace_insn with
+       iclass == BTRACE_INSN_OTHER or
+       iclass == BTRACE_INSN_CALL or
+       iclass == BTRACE_INSN_RETURN or
+       iclass == BTRACE_INSN_JUMP.  */
+    CORE_ADDR pc;
+
+    /* Index into btrace_info::aux_data.  Applies to btrace_insn with
+       iclass == BTRACE_INSN_AUX.  */
+    uint64_t aux_data_index;
+  };
   /* The size of this instruction in bytes.  */
   gdb_byte size;
 
@@ -333,6 +346,10 @@ struct btrace_thread_info
      function segment i will be at index (i - 1).  */
   std::vector<btrace_function> functions;
 
+  /* Optional auxiliary data that is printed in all commands displaying or
+     stepping through the execution history.  */
+  std::vector<std::string> aux_data;
+
   /* The function level offset.  When added to each function's LEVEL,
      this normalizes the function levels such that the smallest level
      becomes zero.  */
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 3c4535ec506..c31ed39b251 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -6831,6 +6831,9 @@ Moxie, PowerPC, PowerPC64, S/390, and x86 (i386/amd64) running
 GNU/Linux.  Process record and replay can be used both when native
 debugging, and when remote debugging via @code{gdbserver}.
 
+When recording an inferior, GDB may print additional auxiliary information
+during stepping commands and commands displaying the execution history.
+
 For architecture environments that support process record and replay,
 @value{GDBN} provides the following commands:
 
-- 
2.20.1

Re: [PATCH 01/10] btrace: Introduce auxiliary instructions.

[Eli Zaretskii]

> From: felix.willgerodt@intel.com
> Cc: markus.t.metzger@intel.com, Felix Willgerodt <felix.willgerodt@intel.com>
> Date: Wed, 29 May 2019 10:47:44 +0200
> 
> index 3c4535ec506..c31ed39b251 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -6831,6 +6831,9 @@ Moxie, PowerPC, PowerPC64, S/390, and x86 (i386/amd64) running
>  GNU/Linux.  Process record and replay can be used both when native
>  debugging, and when remote debugging via @code{gdbserver}.
>  
> +When recording an inferior, GDB may print additional auxiliary information
                               ^^^
That should be "@value{GDBN}" instead.

Thanks.

RE: [PATCH 01/10] btrace: Introduce auxiliary instructions.

[Metzger, Markus T]

Hello Felix,

> Auxiliary instructions are pseudo instructions pointing to auxiliary data.
> This auxiliary data can be printed in all commands displaying (record
> function-call-history, record instruction-history) or stepping through
> (stepi etc.) the execution history, which will be introduced in the next
> commits.
> 
> This patch is in preparation for the ptwrite feature, which is based on
> auxiliary instructions.
> 
> 2019-05-29  Felix Willgerodt  <felix.willgerodt@intel.com>
> 
> gdb/ChangeLog:
> 	* btrace.c (btrace_clear_history): Clear aux_data.
> 	* btrace.h (btrace_insn_class): Add BTRACE_INSN_AUX.
> 	(btrace_insn): New union.
> 	(btrace_thread_info): New member aux_data.
> 
> gdb/doc/ChangeLog:
> 	* gdb.texinfo (Process Record and Replay): Document printing of auxiliary
> 	information.
> 
> ---
>  gdb/btrace.c        |  2 ++
>  gdb/btrace.h        | 25 +++++++++++++++++++++----
>  gdb/doc/gdb.texinfo |  3 +++
>  3 files changed, 26 insertions(+), 4 deletions(-)

LGTM.

Markus.

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

[PATCH 04/10] btrace: Handle stepping and goto for auxiliary instructions.

[felix.willgerodt]

From: Felix Willgerodt <felix.willgerodt@intel.com>

Print the auxiliary data when stepping. Don't allow to goto an auxiliary
instruction.

This patch is in preparation for the ptwrite feature, which is based on
auxiliary instructions.

2019-05-29  Felix Willgerodt  <felix.willgerodt@intel.com>

gdb/ChangeLog:
	* record-btrace.c: (record_btrace_single_step_forward): Handle
	BTRACE_INSN_AUX.
	(record_btrace_single_step_backward): Likewise.
	(record_btrace_goto): Likewise.

---
 gdb/record-btrace.c | 52 ++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 44 insertions(+), 8 deletions(-)

diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 5f70c4838a9..573326924a2 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -2366,6 +2366,8 @@ record_btrace_single_step_forward (struct thread_info *tp)
 {
   struct btrace_insn_iterator *replay, end, start;
   struct btrace_thread_info *btinfo;
+  const struct btrace_function *bfun;
+  struct btrace_insn next_insn;
 
   btinfo = &tp->btrace;
   replay = btinfo->replay;
@@ -2378,13 +2380,26 @@ record_btrace_single_step_forward (struct thread_info *tp)
   if (record_btrace_replay_at_breakpoint (tp))
     return btrace_step_stopped ();
 
-  /* Skip gaps during replay.  If we end up at a gap (at the end of the trace),
-     jump back to the instruction at which we started.  */
   start = *replay;
+
+  /* Skip gaps during replay.  If we end up at a gap (at the end of the trace),
+     jump back to the instruction at which we started.  If we're stepping a
+     BTRACE_INSN_AUX instruction, print the ptwrite string and skip the
+     instruction.  */
   do
     {
       unsigned int steps;
 
+      /* If we're stepping a BTRACE_INSN_AUX instruction, print the auxiliary
+	 data and skip the instruction.  If we are at the end of the trace, jump
+	 back to the instruction at which we started.  */
+      bfun = &replay->btinfo->functions[replay->call_index];
+      next_insn = bfun->insn[replay->insn_index + 1];
+
+      if (next_insn.iclass == BTRACE_INSN_AUX)
+	printf_unfiltered (
+	    "[%s]\n", btinfo->aux_data[next_insn.aux_data_index].c_str ());
+
       /* We will bail out here if we continue stepping after reaching the end
 	 of the execution history.  */
       steps = btrace_insn_next (replay, 1);
@@ -2394,7 +2409,8 @@ record_btrace_single_step_forward (struct thread_info *tp)
 	  return btrace_step_no_history ();
 	}
     }
-  while (btrace_insn_get (replay) == NULL);
+  while (btrace_insn_get (replay) == NULL
+	 || next_insn.iclass == BTRACE_INSN_AUX);
 
   /* Determine the end of the instruction trace.  */
   btrace_insn_end (&end, btinfo);
@@ -2415,6 +2431,8 @@ record_btrace_single_step_backward (struct thread_info *tp)
 {
   struct btrace_insn_iterator *replay, start;
   struct btrace_thread_info *btinfo;
+  const struct btrace_function *bfun;
+  struct btrace_insn prev_insn;
 
   btinfo = &tp->btrace;
   replay = btinfo->replay;
@@ -2452,6 +2470,22 @@ record_btrace_single_step_backward (struct thread_info *tp)
   if (record_btrace_replay_at_breakpoint (tp))
     return btrace_step_stopped ();
 
+  /* Check if we're stepping a BTRACE_INSN_AUX instruction and skip it.  */
+  bfun = &replay->btinfo->functions[replay->call_index];
+  prev_insn = bfun->insn[replay->insn_index];
+
+  if (prev_insn.iclass == BTRACE_INSN_AUX)
+    {
+      printf_unfiltered ("[%s]\n",
+			 btinfo->aux_data[prev_insn.aux_data_index].c_str ());
+      unsigned int steps = btrace_insn_prev (replay, 1);
+      if (steps == 0)
+	{
+	  *replay = start;
+	  return btrace_step_no_history ();
+	}
+    }
+
   return btrace_step_spurious ();
 }
 
@@ -2853,25 +2887,27 @@ record_btrace_target::goto_record_end ()
 /* The goto_record method of target record-btrace.  */
 
 void
-record_btrace_target::goto_record (ULONGEST insn)
+record_btrace_target::goto_record (ULONGEST insn_number)
 {
   struct thread_info *tp;
   struct btrace_insn_iterator it;
   unsigned int number;
   int found;
 
-  number = insn;
+  number = insn_number;
 
   /* Check for wrap-arounds.  */
-  if (number != insn)
+  if (number != insn_number)
     error (_("Instruction number out of range."));
 
   tp = require_btrace_thread ();
 
   found = btrace_find_insn_by_number (&it, &tp->btrace, number);
+  const struct btrace_insn *insn = btrace_insn_get (&it);
 
-  /* Check if the instruction could not be found or is a gap.  */
-  if (found == 0 || btrace_insn_get (&it) == NULL)
+  /* Check if the instruction could not be found or is a gap or an
+     auxilliary instruction.  */
+  if ((found == 0) || (insn == NULL) || (insn->iclass == BTRACE_INSN_AUX))
     error (_("No such instruction."));
 
   record_btrace_set_replay (tp, &it);
-- 
2.20.1

RE: [PATCH 04/10] btrace: Handle stepping and goto for auxiliary instructions.

[Metzger, Markus T]

Hello Felix,

> @@ -2378,13 +2380,26 @@ record_btrace_single_step_forward (struct thread_info
> *tp)
>    if (record_btrace_replay_at_breakpoint (tp))
>      return btrace_step_stopped ();
> 
> -  /* Skip gaps during replay.  If we end up at a gap (at the end of the trace),
> -     jump back to the instruction at which we started.  */
>    start = *replay;

Please preserve the order of comment and code.

> +
> +  /* Skip gaps during replay.  If we end up at a gap (at the end of the trace),
> +     jump back to the instruction at which we started.  If we're stepping a
> +     BTRACE_INSN_AUX instruction, print the ptwrite string and skip the

Should this say 'print the aux data'?

> +     instruction.  */
>    do
>      {
>        unsigned int steps;
> 
> +      /* If we're stepping a BTRACE_INSN_AUX instruction, print the auxiliary
> +	 data and skip the instruction.  If we are at the end of the trace, jump
> +	 back to the instruction at which we started.  */
> +      bfun = &replay->btinfo->functions[replay->call_index];
> +      next_insn = bfun->insn[replay->insn_index + 1];

The below btrace_insn_next () already advances REPLAY to the next instruction.  You may
use btrace_insn_next (replay) to get that instruction.

> +
> +      if (next_insn.iclass == BTRACE_INSN_AUX)
> +	printf_unfiltered (
> +	    "[%s]\n", btinfo->aux_data[next_insn.aux_data_index].c_str ());
> +
>        /* We will bail out here if we continue stepping after reaching the end
>  	 of the execution history.  */
>        steps = btrace_insn_next (replay, 1);
> @@ -2394,7 +2409,8 @@ record_btrace_single_step_forward (struct thread_info
> *tp)
>  	  return btrace_step_no_history ();
>  	}
>      }
> -  while (btrace_insn_get (replay) == NULL);
> +  while (btrace_insn_get (replay) == NULL
> +	 || next_insn.iclass == BTRACE_INSN_AUX);

Since we now call btrace_insn_get (replay) inside the loop, we should rewrite this.

> @@ -2452,6 +2470,22 @@ record_btrace_single_step_backward (struct
> thread_info *tp)
>    if (record_btrace_replay_at_breakpoint (tp))
>      return btrace_step_stopped ();
> 
> +  /* Check if we're stepping a BTRACE_INSN_AUX instruction and skip it.  */
> +  bfun = &replay->btinfo->functions[replay->call_index];
> +  prev_insn = bfun->insn[replay->insn_index];

Same here.

> +
> +  if (prev_insn.iclass == BTRACE_INSN_AUX)
> +    {
> +      printf_unfiltered ("[%s]\n",
> +			 btinfo->aux_data[prev_insn.aux_data_index].c_str ());
> +      unsigned int steps = btrace_insn_prev (replay, 1);
> +      if (steps == 0)
> +	{
> +	  *replay = start;
> +	  return btrace_step_no_history ();
> +	}
> +    }
> +
>    return btrace_step_spurious ();
>  }
> 
> @@ -2853,25 +2887,27 @@ record_btrace_target::goto_record_end ()
>  /* The goto_record method of target record-btrace.  */
> 
>  void
> -record_btrace_target::goto_record (ULONGEST insn)
> +record_btrace_target::goto_record (ULONGEST insn_number)
>  {
>    struct thread_info *tp;
>    struct btrace_insn_iterator it;
>    unsigned int number;
>    int found;
> 
> -  number = insn;
> +  number = insn_number;
> 
>    /* Check for wrap-arounds.  */
> -  if (number != insn)
> +  if (number != insn_number)
>      error (_("Instruction number out of range."));
> 
>    tp = require_btrace_thread ();
> 
>    found = btrace_find_insn_by_number (&it, &tp->btrace, number);
> +  const struct btrace_insn *insn = btrace_insn_get (&it);

You need to check FOUND before dereferencing IT.

> 
> -  /* Check if the instruction could not be found or is a gap.  */
> -  if (found == 0 || btrace_insn_get (&it) == NULL)
> +  /* Check if the instruction could not be found or is a gap or an
> +     auxilliary instruction.  */
> +  if ((found == 0) || (insn == NULL) || (insn->iclass == BTRACE_INSN_AUX))
>      error (_("No such instruction."));
> 
>    record_btrace_set_replay (tp, &it);
> --
> 2.20.1

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

RE: [PATCH 04/10] btrace: Handle stepping and goto for auxiliary instructions.

[Willgerodt, Felix]

I should have fixed all comments.

Thanks,
Felix

On 6/4/19 2:35 PM, Metzger, Markus T wrote:

Hello Felix,

> @@ -2378,13 +2380,26 @@ record_btrace_single_step_forward (struct 
> thread_info
> *tp)
>    if (record_btrace_replay_at_breakpoint (tp))
>      return btrace_step_stopped ();
> 
> -  /* Skip gaps during replay.  If we end up at a gap (at the end of the trace),
> -     jump back to the instruction at which we started.  */
>    start = *replay;

Please preserve the order of comment and code.

> +
> +  /* Skip gaps during replay.  If we end up at a gap (at the end of the trace),
> +     jump back to the instruction at which we started.  If we're stepping a
> +     BTRACE_INSN_AUX instruction, print the ptwrite string and skip 
> + the

Should this say 'print the aux data'?

> +     instruction.  */
>    do
>      {
>        unsigned int steps;
> 
> +      /* If we're stepping a BTRACE_INSN_AUX instruction, print the auxiliary
> +	 data and skip the instruction.  If we are at the end of the trace, jump
> +	 back to the instruction at which we started.  */
> +      bfun = &replay->btinfo->functions[replay->call_index];
> +      next_insn = bfun->insn[replay->insn_index + 1];

The below btrace_insn_next () already advances REPLAY to the next instruction.  You may use btrace_insn_next (replay) to get that instruction.

> +
> +      if (next_insn.iclass == BTRACE_INSN_AUX)
> +	printf_unfiltered (
> +	    "[%s]\n", btinfo->aux_data[next_insn.aux_data_index].c_str ());
> +
>        /* We will bail out here if we continue stepping after reaching the end
>  	 of the execution history.  */
>        steps = btrace_insn_next (replay, 1); @@ -2394,7 +2409,8 @@ 
> record_btrace_single_step_forward (struct thread_info
> *tp)
>  	  return btrace_step_no_history ();
>  	}
>      }
> -  while (btrace_insn_get (replay) == NULL);
> +  while (btrace_insn_get (replay) == NULL
> +	 || next_insn.iclass == BTRACE_INSN_AUX);

Since we now call btrace_insn_get (replay) inside the loop, we should rewrite this.

> @@ -2452,6 +2470,22 @@ record_btrace_single_step_backward (struct 
> thread_info *tp)
>    if (record_btrace_replay_at_breakpoint (tp))
>      return btrace_step_stopped ();
> 
> +  /* Check if we're stepping a BTRACE_INSN_AUX instruction and skip 
> + it.  */  bfun = &replay->btinfo->functions[replay->call_index];
> +  prev_insn = bfun->insn[replay->insn_index];

Same here.

> +
> +  if (prev_insn.iclass == BTRACE_INSN_AUX)
> +    {
> +      printf_unfiltered ("[%s]\n",
> +			 btinfo->aux_data[prev_insn.aux_data_index].c_str ());
> +      unsigned int steps = btrace_insn_prev (replay, 1);
> +      if (steps == 0)
> +	{
> +	  *replay = start;
> +	  return btrace_step_no_history ();
> +	}
> +    }
> +
>    return btrace_step_spurious ();
>  }
> 
> @@ -2853,25 +2887,27 @@ record_btrace_target::goto_record_end ()
>  /* The goto_record method of target record-btrace.  */
> 
>  void
> -record_btrace_target::goto_record (ULONGEST insn)
> +record_btrace_target::goto_record (ULONGEST insn_number)
>  {
>    struct thread_info *tp;
>    struct btrace_insn_iterator it;
>    unsigned int number;
>    int found;
> 
> -  number = insn;
> +  number = insn_number;
> 
>    /* Check for wrap-arounds.  */
> -  if (number != insn)
> +  if (number != insn_number)
>      error (_("Instruction number out of range."));
> 
>    tp = require_btrace_thread ();
> 
>    found = btrace_find_insn_by_number (&it, &tp->btrace, number);
> +  const struct btrace_insn *insn = btrace_insn_get (&it);

You need to check FOUND before dereferencing IT.

> 
> -  /* Check if the instruction could not be found or is a gap.  */
> -  if (found == 0 || btrace_insn_get (&it) == NULL)
> +  /* Check if the instruction could not be found or is a gap or an
> +     auxilliary instruction.  */
> +  if ((found == 0) || (insn == NULL) || (insn->iclass == 
> + BTRACE_INSN_AUX))
>      error (_("No such instruction."));
> 
>    record_btrace_set_replay (tp, &it);
> --
> 2.20.1

Thanks,
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH 10/10] btrace: Extend event decoding for ptwrite.

[felix.willgerodt]

From: Felix Willgerodt <felix.willgerodt@intel.com>

Call the ptwrite listener function whenever a ptwrite event is decoded.
The returned string is written to the aux_data string table and a
corresponding auxiliary instruction is appended to the function segment.

2019-05-29  Felix Willgerodt  <felix.willgerodt@intel.com>

gdb/ChangeLog:
	* btrace.c (handle_pt_insn_events): Handle ptev_ptwrite.

gdb/testsuite/ChangeLog:
	* gdb.btrace/ptwrite.c: New file.
	* gdb.btrace/ptwrite.exp: New file.
	* gdb.btrace/x86_64-ptwrite.S: New file.
	* lib/gdb.exp (skip_btrace_ptw_tests): New function.

gdb/doc/ChangeLog:
	* python.texi (gdb.ptwrite): New documentation.

---
 gdb/NEWS                                  |   6 +
 gdb/btrace.c                              |  47 +++
 gdb/doc/python.texi                       | 111 +++++
 gdb/testsuite/gdb.btrace/ptwrite.c        |  40 ++
 gdb/testsuite/gdb.btrace/ptwrite.exp      | 212 ++++++++++
 gdb/testsuite/gdb.btrace/x86_64-ptwrite.S | 479 ++++++++++++++++++++++
 gdb/testsuite/lib/gdb.exp                 |  92 +++++
 7 files changed, 987 insertions(+)
 create mode 100644 gdb/testsuite/gdb.btrace/ptwrite.c
 create mode 100644 gdb/testsuite/gdb.btrace/ptwrite.exp
 create mode 100644 gdb/testsuite/gdb.btrace/x86_64-ptwrite.S

diff --git a/gdb/NEWS b/gdb/NEWS
index 6546b7243a9..1fca2f0cfd2 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,12 @@
 
 *** Changes since GDB 8.3
 
+* GDB now supports printing of ptwrite payloads from the Intel Processor
+  Trace during 'record instruction-history', 'record  function-call-history',
+  all stepping commands and in  Python.  Printing is customizable via a
+  ptwrite listener function in  Python.  By default, the raw ptwrite
+  payload is printed for each ptwrite that is encountered.
+
 * New built-in convenience variables $_gdb_major and $_gdb_minor
   provide the GDB version.  They are handy for conditionally using
   features available only in or since specific GDB versions, in
diff --git a/gdb/btrace.c b/gdb/btrace.c
index 5bdef1c1768..e098abba514 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -1244,6 +1244,53 @@ handle_pt_insn_events (struct btrace_thread_info *btinfo,
 		   bfun->insn_offset - 1, offset);
 
 	  break;
+
+	case ptev_ptwrite:
+	  {
+	    uint64_t *ip = nullptr;
+	    gdb::unique_xmalloc_ptr<char> ptw_string = nullptr;
+	    struct btrace_insn ptw_insn;
+
+	    /* Lookup the ip.  */
+	    if (event.ip_suppressed == 0)
+	      ip = &event.variant.ptwrite.ip;
+	    else if (!btinfo->functions.empty ()
+		     && !btinfo->functions.back ().insn.empty ())
+	      ip = &btinfo->functions.back ().insn.back ().pc;
+	    else
+	      /* This should never happen.  */
+	      error (_("Failed to obtain the PC of the current instruction."));
+
+	    try
+	      {
+		if (btinfo->ptw_callback_fun != nullptr)
+		  ptw_string = btinfo->ptw_callback_fun (
+					  &event.variant.ptwrite.payload, ip,
+					  btinfo->ptw_listener);
+	      }
+	    catch (const gdb_exception_error &error)
+	      {
+		warning (_("Failed to call ptwrite listener."));
+	      }
+
+	    if (ptw_string == nullptr)
+	      break;
+
+	    btinfo->aux_data.emplace_back (ptw_string.get ());
+
+	    /* Update insn list with ptw payload insn.  */
+	    ptw_insn.aux_data_index = btinfo->aux_data.size () - 1;
+	    ptw_insn.flags = 0;
+	    ptw_insn.iclass = BTRACE_INSN_AUX;
+	    ptw_insn.size = 0;
+
+	    bfun = ftrace_update_function (btinfo, *ip);
+	    bfun->flags |= BFUN_AUX_DECODED;
+
+	    ftrace_update_insns (bfun, ptw_insn);
+
+	    break;
+	  }
 	}
     }
 #endif /* defined (HAVE_PT_INSN_EVENT) */
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index e892caf9e18..8fb052c3b83 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -5683,6 +5683,7 @@ registering objfile-specific pretty-printers and frame-filters.
 * gdb.printing::       Building and registering pretty-printers.
 * gdb.types::          Utilities for working with types.
 * gdb.prompt::         Utilities for prompt value substitution.
+* gdb.ptwrite::        Utilities for ptwrite listener registration.
 @end menu
 
 @node gdb.printing
@@ -5874,3 +5875,113 @@ substitute_prompt (``frame: \f,
 "frame: main, print arguments: scalars"
 @end smallexample
 @end table
+
+@node gdb.ptwrite
+@subsubsection gdb.ptwrite
+@cindex gdb.ptwrite
+
+This module provides additional functionality for recording programs that make
+use of the PTWRITE instruction. PTWRITE is a x86 instruction that allows to
+write values into the Intel Processor Trace (@pxref{Process Record and Replay}).
+The @value{NGCC} built-in functions for it are:
+@smallexample
+void __builtin_ia32_ptwrite32 (unsigned)
+void __builtin_ia32_ptwrite64 (unsigned long long)
+@end smallexample
+
+If an inferior uses the instruction, @value{GDBN} inserts the raw payload value
+as auxiliary information into the execution history.  Auxiliary information
+is by default printed during 'record instruction-history',
+'record function-call-history' and all stepping commands and is accessible
+in Python as a @code{RecordAuxiliary} object. 
+
+@exdent Sample program:
+@smallexample
+void
+ptwrite64 (unsigned long long value)
+@{
+  __builtin_ia32_ptwrite64 (value);
+@}
+
+int
+main (void)
+@{
+  ptwrite64 (0x42);
+  return 0; /* break here.  */
+@}
+@end smallexample
+
+@exdent @value{GDBN} output after recording the sample program in pt format:
+@smallexample
+(gdb) record instruction-history 12,14
+12         0x000000000040074c <ptwrite64+16>:	ptwrite %rbx
+13         [payload: 0x42]
+14         0x0000000000400751 <ptwrite64+21>:	mov    -0x8(%rbp),%rbx
+(gdb) record function-call-history
+1       main
+2       ptwrite64
+                [payload: 0x42]
+3       main
+@end smallexample
+
+The @code{gdb.ptwrite} module allows customizing the default output of ptwrite
+auxiliary information. A custom Python function can be registered via
+@code{gdb.ptwrite.register_listener()} as the @dfn{ptwrite listener function}.
+This function will be called with the ptwrite payload and IP as arguments
+during trace decoding.
+
+@table @code
+
+@item register_listener (@var{listener})
+Used to register the ptwrite listener.  The listener can be any callable
+object that accepts two arguments.   It can return a string, which will be
+printed by @value{GDBN} during the aforementioned commands, or @code{None},
+resulting in no output.  @code{None} can also be registered to deactivate
+printing.
+
+@item get_listener ()
+Returns the currently active ptwrite listener function.
+
+@item default_listener (@var{payload}, @var{ip})
+The listener function active by default.
+@end table
+
+When recording multithreaded programs, @value{GDBN} creates a new copy of the
+listener function for each thread to allow for independent internal states.
+There is currently no support for registering different listeners for different
+threads. The listener can however distinguish between multiple threads
+with the help of @code{gdb.selected_thread().global_num} or similar.
+
+@exdent For example:
+@smallexample
+(gdb) python-interactive
+>>> def my_listener(payload, ip):
+...     if gdb.selected_thread().global_num == 1:
+...         return "payload: @{0@}, ip: @{:#x@}".format(payload, ip)
+...     else:
+...         return None
+...
+>>> import gdb.ptwrite
+>>> gdb.ptwrite.register_listener(my_listener)
+>>>
+(gdb) record instruction-history 127,129
+127	   0x000000000040116c <ptwrite32(unsigned int)+13>:	ptwrite %ebx
+128        [payload: 4919, ip: 0x40074c]
+129	   0x0000000000401170 <ptwrite32(unsigned int)+17>:	nop
+(gdb) info threads 
+* 1    Thread 0x7ffff7fd8740 (LWP 25796) "ptwrite_threads" task (arg=0x0)
+    at bin/ptwrite/ptwrite_threads.c:45
+  2    Thread 0x7ffff6eb8700 (LWP 25797) "ptwrite_threads" task (arg=0x0)
+    at bin/ptwrite/ptwrite_threads.c:45
+(gdb) thread 2
+[Switching to thread 2 (Thread 0x7ffff6eb8700 (LWP 25797))]
+#0  task (arg=0x0) at ptwrite_threads.c:45
+45	  return NULL;
+(gdb) record instruction-history 1753,1755
+1753	   0x000000000040116c <ptwrite32(unsigned int)+13>:	ptwrite %ebx
+1754	   0x0000000000401170 <ptwrite32(unsigned int)+17>:	nop
+1755	   0x0000000000401171 <ptwrite32(unsigned int)+18>:	pop    %rbx
+@end smallexample
+
+This GDB feature is dependent on hardware and operating system support and
+requires the Intel Processor Trace decoder library in version 2.0.0 or newer.
diff --git a/gdb/testsuite/gdb.btrace/ptwrite.c b/gdb/testsuite/gdb.btrace/ptwrite.c
new file mode 100644
index 00000000000..5e7ecb75e02
--- /dev/null
+++ b/gdb/testsuite/gdb.btrace/ptwrite.c
@@ -0,0 +1,40 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2018 Free Software Foundation, Inc.
+
+ 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 <stdint.h>
+
+void
+ptwrite64 (uint64_t value)
+{
+  asm volatile ("PTWRITE %0;" : : "b" (value));
+}
+
+void
+ptwrite32 (uint32_t value)
+{
+  asm volatile ("PTWRITE %0;" : : "b" (value));
+}
+
+int
+main (void)
+{
+
+  ptwrite64 (0x42);
+  ptwrite32 (0x43);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.btrace/ptwrite.exp b/gdb/testsuite/gdb.btrace/ptwrite.exp
new file mode 100644
index 00000000000..4cbd030be44
--- /dev/null
+++ b/gdb/testsuite/gdb.btrace/ptwrite.exp
@@ -0,0 +1,212 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2018 Free Software Foundation, Inc.
+#
+# 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/>.
+
+load_lib gdb-python.exp
+
+if { [skip_btrace_pt_tests] } {
+    unsupported "Target does not support record btrace pt."
+    return -1
+}
+
+if { [skip_btrace_ptw_tests] } {
+    unsupported "Hardware does not support ptwrite instructions."
+    return -1
+}
+
+set comp_flags "additional_flags=-I${srcdir}/.."
+set opts {}
+
+if [info exists COMPILE] {
+    # make check RUNTESTFLAGS="gdb.btrace/ptwrite.exp COMPILE=1"
+    standard_testfile ptwrite.c
+    lappend opts debug comp_flags
+} elseif {[istarget "i?86-*-*"] || [istarget "x86_64-*-*"]} {
+	if {[is_amd64_regs_target]} {
+		standard_testfile x86_64-ptwrite.S
+	} else {
+		unsupported "target architecture not supported"
+		return -1
+	}
+} else {
+    unsupported "target architecture not supported"
+    return -1
+}
+
+if [prepare_for_testing "failed to prepare" $testfile $srcfile $opts] {
+    unsupported "Compiler does not support ptwrite."
+    return -1
+}
+
+
+if { ![runto_main] } {
+    untested "failed to run to main"
+    return -1
+}
+
+# This needs to be after runto_main
+if { [skip_python_tests] } {
+    unsupported "Configuration doesn't support python scripting."
+    return -1
+}
+
+
+### 1. Default testrun
+
+# Setup recording
+gdb_test_no_output "set record instruction-history-size unlimited" "Default: set unlimited"
+gdb_test_no_output "record btrace pt" "Default: record btrace pt"
+gdb_test "next" ".*" "Default: first next"
+gdb_test "next" ".*" "Default: second next"
+
+
+# Test libipt version (must be >= 2.0.0)
+set libipt_supports_ptwrite 0
+
+gdb_test_multiple "maint info btrace" "check libipt version" {
+    -re ".*Version: \[01\]\.\[0-9\]+\.\[0-9\]+.*"
+    {
+	set libipt_supports_ptwrite 0
+    }
+    -re ".*Version: \[0-9\]+\.\[0-9\]+\.\[0-9\]+.*"
+    {
+	set libipt_supports_ptwrite 1
+    }
+    default {
+	set libipt_supports_ptwrite 0
+    }
+}
+
+if { $libipt_supports_ptwrite == 0 } {
+    unsupported "libipt version doesn't support ptwrite decoding."
+    return -1
+}
+
+# Test record instruction-history
+gdb_test "record instruction-history 1" [multi_line \
+  ".*\[0-9\]+\t   $hex <ptwrite64\\+\[0-9\]+>:\tptwrite %\[a-z\]+" \
+  "\[0-9\]+\t   \\\[payload: 0x42\\\]" \
+  ".*\[0-9\]+\t   $hex <ptwrite32\\+\[0-9\]+>:\tptwrite %\[a-z\]+" \
+  "\[0-9\]+\t   \\\[payload: 0x43\\\].*" \
+  ] "Default: record instruction-history 1"
+
+# Test function call history
+gdb_test "record function-call-history 1,4" [multi_line \
+  "1\tmain" \
+  "2\tptwrite64" \
+  "\t\t\\\[payload: 0x42\\\]" \
+  "3\tmain" \
+  "4\tptwrite32" \
+  "\t\t\\\[payload: 0x43\\\]" \
+  ] "Default: record function-call-history 1,4"
+
+gdb_test "record function-call-history /s 1,4" [multi_line \
+  "1\tmain" \
+  "2\tptwrite64" \
+  "3\tmain" \
+  "4\tptwrite32" \
+  ] "Default: record function-call-history /s 1,4"
+
+# Test payload printing during stepping
+gdb_test "record goto 10" "No such instruction\."
+gdb_test "record goto 9" ".*ptwrite64.* at .*ptwrite.c:23.*"
+gdb_test "stepi" ".*\\\[payload: 0x42\\\].*"
+gdb_test "reverse-stepi" ".*\\\[payload: 0x42\\\].*"
+gdb_test "continue" ".*\\\[payload: 0x42\\\].*\\\[payload: 0x43\\\].*"
+gdb_test "reverse-continue" ".*\\\[payload: 0x43\\\].*\\\[payload: 0x42\\\].*"
+
+# Test auxiliary type in python
+gdb_py_test_multiple "Default: auxiliary type in python" \
+  "python" "" \
+  "h = gdb.current_recording().instruction_history" "" \
+  "for insn in h: print(insn)" "" \
+  "end" [multi_line \
+  ".*RecordAuxiliary.*" \
+  ".*RecordAuxiliary.*" \
+  ]
+
+
+### 2. Test listener registration
+### 2.1 Custom listener
+
+gdb_py_test_multiple "Custom: register listener in python" \
+  "python" "" \
+  "def my_listener(payload, ip):" "" \
+  "    if  payload == 66:" "" \
+  "        return \"payload: {0}, ip: {1:#x}\".format(payload, ip)" "" \
+  "    else:" "" \
+  "        return None" "" \
+  "import gdb.ptwrite" "" \
+  "gdb.ptwrite.register_listener(my_listener)" "" \
+  "end" ""
+
+gdb_test "record instruction-history 1" [multi_line \
+  ".*\[0-9\]+\t   $hex <ptwrite64\\+\[0-9\]+>:\tptwrite %\[a-z\]+" \
+  "\[0-9\]+\t   \\\[payload: 66, ip: $hex\\\]" \
+  ".*\[0-9\]+\t   $hex <ptwrite32\\+\[0-9\]+>:\tptwrite %\[a-z\]+" \
+  "\[0-9\]+\t   $hex <ptwrite32\\+\[0-9\]+>:.*" \
+  ] "Custom: record instruction-history 1"
+
+### 2.2 None as listener
+
+gdb_py_test_multiple "None: register listener in python" \
+  "python" "" \
+  "import gdb.ptwrite" "" \
+  "gdb.ptwrite.register_listener(None)" "" \
+  "end" ""
+
+gdb_test "record instruction-history 1" [multi_line \
+  ".*\[0-9\]+\t   $hex <ptwrite64\\+\[0-9\]+>:\tptwrite %\[a-z\]+" \
+  "\[0-9\]+\t   $hex <ptwrite64\\+\[0-9\]+>:.*" \
+  "\[0-9\]+\t   $hex <ptwrite32\\+\[0-9\]+>:\tptwrite %\[a-z\]+" \
+  "\[0-9\]+\t   $hex <ptwrite32\\+\[0-9\]+>:.*" \
+  ] "None: record instruction-history 1"
+
+### 2.3 Lambdas as listener
+
+gdb_py_test_multiple "Lambdas: register listener in python" \
+  "python" "" \
+  "import gdb.ptwrite" "" \
+  "gdb.ptwrite.register_listener(lambda payload, ip: \"{}\".format(payload + 2))" "" \
+  "end" ""
+
+gdb_test "record instruction-history 1" [multi_line \
+  ".*\[0-9\]+\t   $hex <ptwrite64\\+\[0-9\]+>:\tptwrite %\[a-z\]+" \
+  "\[0-9\]+\t   \\\[68\\\]" \
+  ".*\[0-9\]+\t   $hex <ptwrite32\\+\[0-9\]+>:\tptwrite %\[a-z\]+" \
+  "\[0-9\]+\t   \\\[69\\\].*" \
+  ] "Lambdas: record instruction-history 1"
+
+### 2.4 Functors as listener
+
+gdb_py_test_multiple "Functors: register listener in python" \
+  "python" "" \
+  "import gdb.ptwrite" "" \
+  "class foobar(object):" "" \
+  "    def __init__(self):" "" \
+  "        self.variable = 0" "" \
+  "    def __call__(self, payload, ip):" "" \
+  "        self.variable += 1" "" \
+  "        return \"{}, {}\".format(self.variable, payload)" "" \
+  "gdb.ptwrite.register_listener(foobar())" "" \
+  "end" ""
+
+gdb_test "record instruction-history 1" [multi_line \
+  ".*\[0-9\]+\t   $hex <ptwrite64\\+\[0-9\]+>:\tptwrite %\[a-z\]+" \
+  "\[0-9\]+\t   \\\[1, 66\\\]" \
+  ".*\[0-9\]+\t   $hex <ptwrite32\\+\[0-9\]+>:\tptwrite %\[a-z\]+" \
+  "\[0-9\]+\t   \\\[2, 67\\\].*" \
+  ] "Functors: record instruction-history 1"
diff --git a/gdb/testsuite/gdb.btrace/x86_64-ptwrite.S b/gdb/testsuite/gdb.btrace/x86_64-ptwrite.S
new file mode 100644
index 00000000000..138ed17a42a
--- /dev/null
+++ b/gdb/testsuite/gdb.btrace/x86_64-ptwrite.S
@@ -0,0 +1,479 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2018 Free Software Foundation, Inc.
+
+   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/>.
+
+
+   This file has been generated using gcc version 8.1.1 20180502:
+   gcc -S -dA -g ptwrite.c -o x86_64-ptwrite.S.  */
+
+	.file	"ptwrite.c"
+	.text
+.Ltext0:
+	.globl	ptwrite64
+	.type	ptwrite64, @function
+ptwrite64:
+.LFB0:
+	.file 1 "ptwrite.c"
+	# ptwrite.c:22:1
+	.loc 1 22 1
+	.cfi_startproc
+# BLOCK 2 seq:0
+# PRED: ENTRY (FALLTHRU)
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset 6, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register 6
+	pushq	%rbx
+	.cfi_offset 3, -24
+	movq	%rdi, -16(%rbp)
+	# ptwrite.c:23:3
+	.loc 1 23 3
+	movq	-16(%rbp), %rax
+	movq	%rax, %rbx
+#APP
+# 23 "ptwrite.c" 1
+	PTWRITE %rbx;
+# 0 "" 2
+	# ptwrite.c:24:1
+	.loc 1 24 1
+#NO_APP
+	nop
+	popq	%rbx
+	popq	%rbp
+	.cfi_def_cfa 7, 8
+# SUCC: EXIT [always] 
+	ret
+	.cfi_endproc
+.LFE0:
+	.size	ptwrite64, .-ptwrite64
+	.globl	ptwrite32
+	.type	ptwrite32, @function
+ptwrite32:
+.LFB1:
+	# ptwrite.c:28:1
+	.loc 1 28 1
+	.cfi_startproc
+# BLOCK 2 seq:0
+# PRED: ENTRY (FALLTHRU)
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset 6, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register 6
+	pushq	%rbx
+	.cfi_offset 3, -24
+	movl	%edi, -12(%rbp)
+	# ptwrite.c:29:3
+	.loc 1 29 3
+	movl	-12(%rbp), %eax
+	movl	%eax, %ebx
+#APP
+# 29 "ptwrite.c" 1
+	PTWRITE %ebx;
+# 0 "" 2
+	# ptwrite.c:30:1
+	.loc 1 30 1
+#NO_APP
+	nop
+	popq	%rbx
+	popq	%rbp
+	.cfi_def_cfa 7, 8
+# SUCC: EXIT [always] 
+	ret
+	.cfi_endproc
+.LFE1:
+	.size	ptwrite32, .-ptwrite32
+	.globl	main
+	.type	main, @function
+main:
+.LFB2:
+	# ptwrite.c:34:1
+	.loc 1 34 1
+	.cfi_startproc
+# BLOCK 2 seq:0
+# PRED: ENTRY (FALLTHRU)
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset 6, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register 6
+	# ptwrite.c:36:3
+	.loc 1 36 3
+	movl	$66, %edi
+	call	ptwrite64
+	# ptwrite.c:37:3
+	.loc 1 37 3
+	movl	$67, %edi
+	call	ptwrite32
+	# ptwrite.c:39:10
+	.loc 1 39 10
+	movl	$0, %eax
+	# ptwrite.c:40:1
+	.loc 1 40 1
+	popq	%rbp
+	.cfi_def_cfa 7, 8
+# SUCC: EXIT [always] 
+	ret
+	.cfi_endproc
+.LFE2:
+	.size	main, .-main
+.Letext0:
+	.file 2 "/usr/include/bits/types.h"
+	.file 3 "/usr/include/bits/stdint-uintn.h"
+	.section	.debug_info,"",@progbits
+.Ldebug_info0:
+	.long	0x10f	# Length of Compilation Unit Info
+	.value	0x4	# DWARF version number
+	.long	.Ldebug_abbrev0	# Offset Into Abbrev. Section
+	.byte	0x8	# Pointer Size (in bytes)
+	.uleb128 0x1	# (DIE (0xb) DW_TAG_compile_unit)
+	.long	.LASF13	# DW_AT_producer: "GNU C17 8.1.1 20180502 (Red Hat 8.1.1-1) -mtune=generic -march=x86-64 -g"
+	.byte	0xc	# DW_AT_language
+	.long	.LASF14	# DW_AT_name: "ptwrite.c"
+	.long	.LASF15	# DW_AT_comp_dir: "gdb/gdb/testsuite/gdb.btrace"
+	.quad	.Ltext0	# DW_AT_low_pc
+	.quad	.Letext0-.Ltext0	# DW_AT_high_pc
+	.long	.Ldebug_line0	# DW_AT_stmt_list
+	.uleb128 0x2	# (DIE (0x2d) DW_TAG_base_type)
+	.byte	0x1	# DW_AT_byte_size
+	.byte	0x8	# DW_AT_encoding
+	.long	.LASF0	# DW_AT_name: "unsigned char"
+	.uleb128 0x2	# (DIE (0x34) DW_TAG_base_type)
+	.byte	0x2	# DW_AT_byte_size
+	.byte	0x7	# DW_AT_encoding
+	.long	.LASF1	# DW_AT_name: "short unsigned int"
+	.uleb128 0x2	# (DIE (0x3b) DW_TAG_base_type)
+	.byte	0x4	# DW_AT_byte_size
+	.byte	0x7	# DW_AT_encoding
+	.long	.LASF2	# DW_AT_name: "unsigned int"
+	.uleb128 0x2	# (DIE (0x42) DW_TAG_base_type)
+	.byte	0x8	# DW_AT_byte_size
+	.byte	0x7	# DW_AT_encoding
+	.long	.LASF3	# DW_AT_name: "long unsigned int"
+	.uleb128 0x2	# (DIE (0x49) DW_TAG_base_type)
+	.byte	0x1	# DW_AT_byte_size
+	.byte	0x6	# DW_AT_encoding
+	.long	.LASF4	# DW_AT_name: "signed char"
+	.uleb128 0x2	# (DIE (0x50) DW_TAG_base_type)
+	.byte	0x2	# DW_AT_byte_size
+	.byte	0x5	# DW_AT_encoding
+	.long	.LASF5	# DW_AT_name: "short int"
+	.uleb128 0x3	# (DIE (0x57) DW_TAG_base_type)
+	.byte	0x4	# DW_AT_byte_size
+	.byte	0x5	# DW_AT_encoding
+	.ascii "int\0"	# DW_AT_name
+	.uleb128 0x4	# (DIE (0x5e) DW_TAG_typedef)
+	.long	.LASF7	# DW_AT_name: "__uint32_t"
+	.byte	0x2	# DW_AT_decl_file (/usr/include/bits/types.h)
+	.byte	0x29	# DW_AT_decl_line
+	.byte	0x16	# DW_AT_decl_column
+	.long	0x3b	# DW_AT_type
+	.uleb128 0x2	# (DIE (0x6a) DW_TAG_base_type)
+	.byte	0x8	# DW_AT_byte_size
+	.byte	0x5	# DW_AT_encoding
+	.long	.LASF6	# DW_AT_name: "long int"
+	.uleb128 0x4	# (DIE (0x71) DW_TAG_typedef)
+	.long	.LASF8	# DW_AT_name: "__uint64_t"
+	.byte	0x2	# DW_AT_decl_file (/usr/include/bits/types.h)
+	.byte	0x2c	# DW_AT_decl_line
+	.byte	0x1b	# DW_AT_decl_column
+	.long	0x42	# DW_AT_type
+	.uleb128 0x2	# (DIE (0x7d) DW_TAG_base_type)
+	.byte	0x1	# DW_AT_byte_size
+	.byte	0x6	# DW_AT_encoding
+	.long	.LASF9	# DW_AT_name: "char"
+	.uleb128 0x4	# (DIE (0x84) DW_TAG_typedef)
+	.long	.LASF10	# DW_AT_name: "uint32_t"
+	.byte	0x3	# DW_AT_decl_file (/usr/include/bits/stdint-uintn.h)
+	.byte	0x1a	# DW_AT_decl_line
+	.byte	0x14	# DW_AT_decl_column
+	.long	0x5e	# DW_AT_type
+	.uleb128 0x4	# (DIE (0x90) DW_TAG_typedef)
+	.long	.LASF11	# DW_AT_name: "uint64_t"
+	.byte	0x3	# DW_AT_decl_file (/usr/include/bits/stdint-uintn.h)
+	.byte	0x1b	# DW_AT_decl_line
+	.byte	0x14	# DW_AT_decl_column
+	.long	0x71	# DW_AT_type
+	.uleb128 0x5	# (DIE (0x9c) DW_TAG_subprogram)
+			# DW_AT_external
+	.long	.LASF16	# DW_AT_name: "main"
+	.byte	0x1	# DW_AT_decl_file (ptwrite.c)
+	.byte	0x21	# DW_AT_decl_line
+	.byte	0x1	# DW_AT_decl_column
+			# DW_AT_prototyped
+	.long	0x57	# DW_AT_type
+	.quad	.LFB2	# DW_AT_low_pc
+	.quad	.LFE2-.LFB2	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_tail_call_sites
+	.uleb128 0x6	# (DIE (0xba) DW_TAG_subprogram)
+			# DW_AT_external
+	.long	.LASF17	# DW_AT_name: "ptwrite32"
+	.byte	0x1	# DW_AT_decl_file (ptwrite.c)
+	.byte	0x1b	# DW_AT_decl_line
+	.byte	0x1	# DW_AT_decl_column
+			# DW_AT_prototyped
+	.quad	.LFB1	# DW_AT_low_pc
+	.quad	.LFE1-.LFB1	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.long	0xe8	# DW_AT_sibling
+	.uleb128 0x7	# (DIE (0xd8) DW_TAG_formal_parameter)
+	.long	.LASF12	# DW_AT_name: "value"
+	.byte	0x1	# DW_AT_decl_file (ptwrite.c)
+	.byte	0x1b	# DW_AT_decl_line
+	.byte	0x15	# DW_AT_decl_column
+	.long	0x84	# DW_AT_type
+	.uleb128 0x2	# DW_AT_location
+	.byte	0x91	# DW_OP_fbreg
+	.sleb128 -28
+	.byte	0	# end of children of DIE 0xba
+	.uleb128 0x8	# (DIE (0xe8) DW_TAG_subprogram)
+			# DW_AT_external
+	.long	.LASF18	# DW_AT_name: "ptwrite64"
+	.byte	0x1	# DW_AT_decl_file (ptwrite.c)
+	.byte	0x15	# DW_AT_decl_line
+	.byte	0x1	# DW_AT_decl_column
+			# DW_AT_prototyped
+	.quad	.LFB0	# DW_AT_low_pc
+	.quad	.LFE0-.LFB0	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.uleb128 0x7	# (DIE (0x102) DW_TAG_formal_parameter)
+	.long	.LASF12	# DW_AT_name: "value"
+	.byte	0x1	# DW_AT_decl_file (ptwrite.c)
+	.byte	0x15	# DW_AT_decl_line
+	.byte	0x15	# DW_AT_decl_column
+	.long	0x90	# DW_AT_type
+	.uleb128 0x2	# DW_AT_location
+	.byte	0x91	# DW_OP_fbreg
+	.sleb128 -32
+	.byte	0	# end of children of DIE 0xe8
+	.byte	0	# end of children of DIE 0xb
+	.section	.debug_abbrev,"",@progbits
+.Ldebug_abbrev0:
+	.uleb128 0x1	# (abbrev code)
+	.uleb128 0x11	# (TAG: DW_TAG_compile_unit)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x25	# (DW_AT_producer)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x13	# (DW_AT_language)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x1b	# (DW_AT_comp_dir)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x7	# (DW_FORM_data8)
+	.uleb128 0x10	# (DW_AT_stmt_list)
+	.uleb128 0x17	# (DW_FORM_sec_offset)
+	.byte	0
+	.byte	0
+	.uleb128 0x2	# (abbrev code)
+	.uleb128 0x24	# (TAG: DW_TAG_base_type)
+	.byte	0	# DW_children_no
+	.uleb128 0xb	# (DW_AT_byte_size)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3e	# (DW_AT_encoding)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.byte	0
+	.byte	0
+	.uleb128 0x3	# (abbrev code)
+	.uleb128 0x24	# (TAG: DW_TAG_base_type)
+	.byte	0	# DW_children_no
+	.uleb128 0xb	# (DW_AT_byte_size)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3e	# (DW_AT_encoding)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0x8	# (DW_FORM_string)
+	.byte	0
+	.byte	0
+	.uleb128 0x4	# (abbrev code)
+	.uleb128 0x16	# (TAG: DW_TAG_typedef)
+	.byte	0	# DW_children_no
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x39	# (DW_AT_decl_column)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.byte	0
+	.byte	0
+	.uleb128 0x5	# (abbrev code)
+	.uleb128 0x2e	# (TAG: DW_TAG_subprogram)
+	.byte	0	# DW_children_no
+	.uleb128 0x3f	# (DW_AT_external)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x39	# (DW_AT_decl_column)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x27	# (DW_AT_prototyped)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x7	# (DW_FORM_data8)
+	.uleb128 0x40	# (DW_AT_frame_base)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.uleb128 0x2116	# (DW_AT_GNU_all_tail_call_sites)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.byte	0
+	.byte	0
+	.uleb128 0x6	# (abbrev code)
+	.uleb128 0x2e	# (TAG: DW_TAG_subprogram)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x3f	# (DW_AT_external)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x39	# (DW_AT_decl_column)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x27	# (DW_AT_prototyped)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x7	# (DW_FORM_data8)
+	.uleb128 0x40	# (DW_AT_frame_base)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.uleb128 0x2117	# (DW_AT_GNU_all_call_sites)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x1	# (DW_AT_sibling)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.byte	0
+	.byte	0
+	.uleb128 0x7	# (abbrev code)
+	.uleb128 0x5	# (TAG: DW_TAG_formal_parameter)
+	.byte	0	# DW_children_no
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x39	# (DW_AT_decl_column)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x2	# (DW_AT_location)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.byte	0
+	.byte	0
+	.uleb128 0x8	# (abbrev code)
+	.uleb128 0x2e	# (TAG: DW_TAG_subprogram)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x3f	# (DW_AT_external)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x39	# (DW_AT_decl_column)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x27	# (DW_AT_prototyped)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x7	# (DW_FORM_data8)
+	.uleb128 0x40	# (DW_AT_frame_base)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.uleb128 0x2117	# (DW_AT_GNU_all_call_sites)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.byte	0
+	.byte	0
+	.byte	0
+	.section	.debug_aranges,"",@progbits
+	.long	0x2c	# Length of Address Ranges Info
+	.value	0x2	# DWARF aranges version
+	.long	.Ldebug_info0	# Offset of Compilation Unit Info
+	.byte	0x8	# Size of Address
+	.byte	0	# Size of Segment Descriptor
+	.value	0	# Pad to 16 byte boundary
+	.value	0
+	.quad	.Ltext0	# Address
+	.quad	.Letext0-.Ltext0	# Length
+	.quad	0
+	.quad	0
+	.section	.debug_line,"",@progbits
+.Ldebug_line0:
+	.section	.debug_str,"MS",@progbits,1
+.LASF2:
+	.string	"unsigned int"
+.LASF7:
+	.string	"__uint32_t"
+.LASF15:
+	.string	"gdb/gdb/testsuite/gdb.btrace"
+.LASF14:
+	.string	"ptwrite.c"
+.LASF18:
+	.string	"ptwrite64"
+.LASF3:
+	.string	"long unsigned int"
+.LASF11:
+	.string	"uint64_t"
+.LASF0:
+	.string	"unsigned char"
+.LASF16:
+	.string	"main"
+.LASF10:
+	.string	"uint32_t"
+.LASF6:
+	.string	"long int"
+.LASF8:
+	.string	"__uint64_t"
+.LASF13:
+	.string	"GNU C17 8.1.1 20180502 (Red Hat 8.1.1-1) -mtune=generic -march=x86-64 -g"
+.LASF1:
+	.string	"short unsigned int"
+.LASF4:
+	.string	"signed char"
+.LASF12:
+	.string	"value"
+.LASF5:
+	.string	"short int"
+.LASF17:
+	.string	"ptwrite32"
+.LASF9:
+	.string	"char"
+	.ident	"GCC: (GNU) 8.1.1 20180502 (Red Hat 8.1.1-1)"
+	.section	.note.GNU-stack,"",@progbits
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index c703a7e6338..ba7ccfd46ba 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2874,6 +2874,98 @@ gdb_caching_proc skip_btrace_pt_tests {
     return $skip_btrace_tests
 }
 
+# Run a test on the target to see if it supports ptwrite instructions.
+# Return 0 if so, 1 if it does not.  Based on 'check_vmx_hw_available'
+# from the GCC testsuite.
+
+gdb_caching_proc skip_btrace_ptw_tests {
+    global srcdir subdir gdb_prompt inferior_exited_re
+
+    set me "skip_ptw_tests"
+
+    # Set up, compile, and execute a test program.
+    # Include the current process ID in the file names to prevent conflicts
+    # with invocations for multiple testsuites.
+    set src [standard_temp_file ptw[pid].c]
+    set exe [standard_temp_file ptw[pid].x]
+
+    gdb_produce_source $src {
+	#include <stdbool.h>
+	#include <stdint.h>
+	#include <stdio.h>
+	#include "x86-cpuid.h"
+
+	#define bit_PTW (1 << 4)
+
+	bool
+	have_ptw ()
+	{
+	  unsigned int eax, ebx, ecx, edx;
+
+	  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
+	      return false;
+
+	  if ((ecx & bit_OSXSAVE) != bit_OSXSAVE)
+	      return false;
+
+	  if (__get_cpuid_max (0, NULL) < 0x14)
+	      return false;
+
+	  __cpuid_count (0x14, 0, eax, ebx, ecx, edx);
+
+	  return (ebx & bit_PTW) == bit_PTW;
+	}
+
+	int
+	main ()
+	{
+	  return have_ptw ();
+	}
+
+    }
+
+    verbose "$me:  compiling testfile $src" 2
+
+    set test_flags [list additional_flags=-I${srcdir}/../nat]
+
+    set compile_flags [concat {debug nowarnings quiet} ${test_flags}]
+    set lines [gdb_compile $src $exe executable $compile_flags]
+
+    if ![string match "" $lines] then {
+	verbose "$me:  testfile compilation failed, returning 1" 2
+	file delete $src
+	return 1
+    }
+
+    # No error message, compilation succeeded so now run it via gdb.
+    gdb_exit
+    gdb_start
+    gdb_reinitialize_dir $srcdir/$subdir
+    gdb_load $exe
+    if ![runto_main] {
+	file delete $src
+	return 1
+    }
+    file delete $src
+
+    # In case of an unexpected output, we return 2 as a fail value.
+    set skip_ptw_tests 2
+    gdb_test_multiple "print (int) have_ptw ()" "check ptw support" {
+	-re ".. = 1\r\n$gdb_prompt $" {
+	    set skip_ptw_tests 0
+	}
+	-re ".. = 0\r\n$gdb_prompt $" {
+	    set skip_ptw_tests 1
+	}
+    }
+
+    gdb_exit
+    remote_file build delete $exe
+
+    verbose "$me:  returning $skip_ptw_tests" 2
+    return $skip_ptw_tests
+}
+
 # Run a test on the target to see if it supports Aarch64 SVE hardware.
 # Return 0 if so, 1 if it does not.  Note this causes a restart of GDB.
 
-- 
2.20.1

Re: [PATCH 10/10] btrace: Extend event decoding for ptwrite.

[Eli Zaretskii]

> From: felix.willgerodt@intel.com
> Cc: markus.t.metzger@intel.com, Felix Willgerodt <felix.willgerodt@intel.com>
> Date: Wed, 29 May 2019 10:47:53 +0200
> 
> +* GDB now supports printing of ptwrite payloads from the Intel Processor
> +  Trace during 'record instruction-history', 'record  function-call-history',
                                                       ^^
Extra blank.

> +  all stepping commands and in  Python.  Printing is customizable via a
                          ^
A comma missing there.
> +use of the PTWRITE instruction. PTWRITE is a x86 instruction that allows to
                                 ^^
Two spaces between sentences, please.

> +The @value{NGCC} built-in functions for it are:
> +@smallexample
> +void __builtin_ia32_ptwrite32 (unsigned)
> +void __builtin_ia32_ptwrite64 (unsigned long long)
> +@end smallexample

Should there be a semi-colon at the end of each function prototype?

> +as auxiliary information into the execution history.  Auxiliary information
> +is by default printed during 'record instruction-history',
> +'record function-call-history' and all stepping commands and is accessible

The two "record" commands should be in @code and without quotes.
Also, please add a comma before the last "and".

> +in Python as a @code{RecordAuxiliary} object. 
> +
> +@exdent Sample program:
> +@smallexample
> +void
> +ptwrite64 (unsigned long long value)
> +@{
> +  __builtin_ia32_ptwrite64 (value);
> +@}
> +
> +int
> +main (void)
> +@{
> +  ptwrite64 (0x42);
> +  return 0; /* break here.  */
> +@}
> +@end smallexample
> +
> +@exdent @value{GDBN} output after recording the sample program in pt format:
> +@smallexample
> +(gdb) record instruction-history 12,14
> +12         0x000000000040074c <ptwrite64+16>:	ptwrite %rbx
> +13         [payload: 0x42]
> +14         0x0000000000400751 <ptwrite64+21>:	mov    -0x8(%rbp),%rbx
> +(gdb) record function-call-history
> +1       main
> +2       ptwrite64
> +                [payload: 0x42]
> +3       main
> +@end smallexample

Long examples such as the two above should use @group to avoid being
split between two pages in an unfortunate place.

> +auxiliary information. A custom Python function can be registered via
                        ^^
Two spaces.

> +@code{gdb.ptwrite.register_listener()} as the @dfn{ptwrite listener function}.

Every term you put in @dfn should have an index entry, as this is
generally terminology people will later look up in the manual.

> +This function will be called with the ptwrite payload and IP as arguments
> +during trace decoding.

If "IP" stands for "instruction pointer", I think that we prefer "PC"
instead.

> +@table @code
> +
> +@item register_listener (@var{listener})
> +Used to register the ptwrite listener.  The listener can be any callable
> +object that accepts two arguments.   It can return a string, which will be
                                     ^^^
Extra space.

> +printed by @value{GDBN} during the aforementioned commands, or @code{None},
> +resulting in no output.  @code{None} can also be registered to deactivate
> +printing.
> +
> +@item get_listener ()
> +Returns the currently active ptwrite listener function.
> +
> +@item default_listener (@var{payload}, @var{ip})
> +The listener function active by default.
> +@end table

Why is this a @table, and not a series of @defun's?

> +threads. The listener can however distinguish between multiple threads
          ^^
Missing space.

> +with the help of @code{gdb.selected_thread().global_num} or similar.

Please add a cross-reference to where gdb.selected_thread is described
in the manual.

Thanks.

RE: [PATCH 10/10] btrace: Extend event decoding for ptwrite.

[Metzger, Markus T]

Hello Felix,

> @@ -1244,6 +1244,53 @@ handle_pt_insn_events (struct btrace_thread_info
> *btinfo,
>  		   bfun->insn_offset - 1, offset);
> 
>  	  break;
> +
> +	case ptev_ptwrite:
> +	  {
> +	    uint64_t *ip = nullptr;
> +	    gdb::unique_xmalloc_ptr<char> ptw_string = nullptr;
> +	    struct btrace_insn ptw_insn;
> +
> +	    /* Lookup the ip.  */
> +	    if (event.ip_suppressed == 0)
> +	      ip = &event.variant.ptwrite.ip;
> +	    else if (!btinfo->functions.empty ()
> +		     && !btinfo->functions.back ().insn.empty ())
> +	      ip = &btinfo->functions.back ().insn.back ().pc;

Libipt will provide the PTWRITE instruction's IP.  We may get PTW events without IP
but those wouldn't bind to the previous instruction - or libipt would have filled in
the event IP.

If EVENT.IP_SUPPRESSED is set, there is no IP.

> +	    else
> +	      /* This should never happen.  */
> +	      error (_("Failed to obtain the PC of the current instruction."));

We explicitly allow a nullptr IP in the callback function.

> +
> +	    try
> +	      {
> +		if (btinfo->ptw_callback_fun != nullptr)
> +		  ptw_string = btinfo->ptw_callback_fun (
> +					  &event.variant.ptwrite.payload, ip,
> +					  btinfo->ptw_listener);
> +	      }
> +	    catch (const gdb_exception_error &error)
> +	      {
> +		warning (_("Failed to call ptwrite listener."));

This indicates a bug in the user's ptwrite-listener object.  Should we print a
backtrace and fail with an error?

> +	      }
> +
> +	    if (ptw_string == nullptr)
> +	      break;
> +
> +	    btinfo->aux_data.emplace_back (ptw_string.get ());
> +
> +	    /* Update insn list with ptw payload insn.  */
> +	    ptw_insn.aux_data_index = btinfo->aux_data.size () - 1;
> +	    ptw_insn.flags = 0;

Can we preserve the SPECULATIVE flag?

> +	    ptw_insn.iclass = BTRACE_INSN_AUX;
> +	    ptw_insn.size = 0;
> +
> +	    bfun = ftrace_update_function (btinfo, *ip);
> +	    bfun->flags |= BFUN_AUX_DECODED;
> +
> +	    ftrace_update_insns (bfun, ptw_insn);
> +
> +	    break;
> +	  }
>  	}
>      }
>  #endif /* defined (HAVE_PT_INSN_EVENT) */

[...]

> +When recording multithreaded programs, @value{GDBN} creates a new copy of

Also when recording a single threaded program.  Maybe just remove this part.

> the
> +listener function for each thread to allow for independent internal states.
> +There is currently no support for registering different listeners for different
> +threads. The listener can however distinguish between multiple threads

You write 'currently'.  Are there plans to support different listeners for different threads?

> +with the help of @code{gdb.selected_thread().global_num} or similar.
> +
> +@exdent For example:
> +@smallexample
> +(gdb) python-interactive
> +>>> def my_listener(payload, ip):
> +...     if gdb.selected_thread().global_num == 1:
> +...         return "payload: @{0@}, ip: @{:#x@}".format(payload, ip)
> +...     else:
> +...         return None
> +...

An example where the listener accumulates payloads would be nice.

> +++ b/gdb/testsuite/gdb.btrace/ptwrite.c
> @@ -0,0 +1,40 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> + Copyright 2018 Free Software Foundation, Inc.

It's 2019, meanwhile.

> +
> + 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 <stdint.h>
> +
> +void
> +ptwrite64 (uint64_t value)
> +{
> +  asm volatile ("PTWRITE %0;" : : "b" (value));
> +}
> +
> +void
> +ptwrite32 (uint32_t value)
> +{
> +  asm volatile ("PTWRITE %0;" : : "b" (value));
> +}
> +
> +int
> +main (void)
> +{
> +

Spurious empty line.

> +  ptwrite64 (0x42);
> +  ptwrite32 (0x43);
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.btrace/ptwrite.exp
> b/gdb/testsuite/gdb.btrace/ptwrite.exp
> new file mode 100644
> index 00000000000..4cbd030be44
> --- /dev/null
> +++ b/gdb/testsuite/gdb.btrace/ptwrite.exp
> @@ -0,0 +1,212 @@
> +# This testcase is part of GDB, the GNU debugger.
> +#
> +# Copyright 2018 Free Software Foundation, Inc.
> +#
> +# 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/>.
> +
> +load_lib gdb-python.exp
> +
> +if { [skip_btrace_pt_tests] } {
> +    unsupported "Target does not support record btrace pt."
> +    return -1
> +}
> +
> +if { [skip_btrace_ptw_tests] } {
> +    unsupported "Hardware does not support ptwrite instructions."
> +    return -1
> +}
> +
> +set comp_flags "additional_flags=-I${srcdir}/.."

If we need this only in the COMPILE case, I'd move it there; or just inline it.

> +set opts {}
> +
> +if [info exists COMPILE] {
> +    # make check RUNTESTFLAGS="gdb.btrace/ptwrite.exp COMPILE=1"
> +    standard_testfile ptwrite.c
> +    lappend opts debug comp_flags
> +} elseif {[istarget "i?86-*-*"] || [istarget "x86_64-*-*"]} {
> +	if {[is_amd64_regs_target]} {
> +		standard_testfile x86_64-ptwrite.S
> +	} else {
> +		unsupported "target architecture not supported"
> +		return -1
> +	}
> +} else {
> +    unsupported "target architecture not supported"
> +    return -1
> +}
> +
> +if [prepare_for_testing "failed to prepare" $testfile $srcfile $opts] {
> +    unsupported "Compiler does not support ptwrite."

I don't think we need an 'unsupported' here.  We should get 'failed to prepare', already.

> +    return -1
> +}
> +
> +
> +if { ![runto_main] } {
> +    untested "failed to run to main"
> +    return -1
> +}
> +
> +# This needs to be after runto_main
> +if { [skip_python_tests] } {
> +    unsupported "Configuration doesn't support python scripting."
> +    return -1
> +}
> +
> +
> +### 1. Default testrun
> +
> +# Setup recording

You can use with_test_prefix to avoid repeating the prefix in each test.

> +gdb_test_no_output "set record instruction-history-size unlimited" "Default:
> set unlimited"
> +gdb_test_no_output "record btrace pt" "Default: record btrace pt"
> +gdb_test "next" ".*" "Default: first next"
> +gdb_test "next" ".*" "Default: second next"


> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index c703a7e6338..ba7ccfd46ba 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -2874,6 +2874,98 @@ gdb_caching_proc skip_btrace_pt_tests {
>      return $skip_btrace_tests
>  }
> 
> +# Run a test on the target to see if it supports ptwrite instructions.
> +# Return 0 if so, 1 if it does not.  Based on 'check_vmx_hw_available'
> +# from the GCC testsuite.
> +
> +gdb_caching_proc skip_btrace_ptw_tests {
> +    global srcdir subdir gdb_prompt inferior_exited_re
> +
> +    set me "skip_ptw_tests"
> +
> +    # Set up, compile, and execute a test program.
> +    # Include the current process ID in the file names to prevent conflicts
> +    # with invocations for multiple testsuites.
> +    set src [standard_temp_file ptw[pid].c]
> +    set exe [standard_temp_file ptw[pid].x]
> +
> +    gdb_produce_source $src {
> +	#include <stdbool.h>
> +	#include <stdint.h>
> +	#include <stdio.h>
> +	#include "x86-cpuid.h"
> +
> +	#define bit_PTW (1 << 4)
> +
> +	bool
> +	have_ptw ()
> +	{
> +	  unsigned int eax, ebx, ecx, edx;
> +
> +	  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
> +	      return false;
> +
> +	  if ((ecx & bit_OSXSAVE) != bit_OSXSAVE)
> +	      return false;
> +
> +	  if (__get_cpuid_max (0, NULL) < 0x14)
> +	      return false;
> +
> +	  __cpuid_count (0x14, 0, eax, ebx, ecx, edx);
> +
> +	  return (ebx & bit_PTW) == bit_PTW;
> +	}
> +
> +	int
> +	main ()
> +	{
> +	  return have_ptw ();
> +	}
> +
> +    }
> +
> +    verbose "$me:  compiling testfile $src" 2
> +
> +    set test_flags [list additional_flags=-I${srcdir}/../nat]
> +
> +    set compile_flags [concat {debug nowarnings quiet} ${test_flags}]
> +    set lines [gdb_compile $src $exe executable $compile_flags]
> +
> +    if ![string match "" $lines] then {
> +	verbose "$me:  testfile compilation failed, returning 1" 2
> +	file delete $src
> +	return 1
> +    }
> +
> +    # No error message, compilation succeeded so now run it via gdb.
> +    gdb_exit
> +    gdb_start
> +    gdb_reinitialize_dir $srcdir/$subdir
> +    gdb_load $exe
> +    if ![runto_main] {
> +	file delete $src
> +	return 1
> +    }
> +    file delete $src
> +
> +    # In case of an unexpected output, we return 2 as a fail value.
> +    set skip_ptw_tests 2
> +    gdb_test_multiple "print (int) have_ptw ()" "check ptw support" {
> +	-re ".. = 1\r\n$gdb_prompt $" {
> +	    set skip_ptw_tests 0
> +	}
> +	-re ".. = 0\r\n$gdb_prompt $" {
> +	    set skip_ptw_tests 1
> +	}
> +    }
> +
> +    gdb_exit
> +    remote_file build delete $exe
> +
> +    verbose "$me:  returning $skip_ptw_tests" 2
> +    return $skip_ptw_tests

We still don't know if the OS supports it, do we?

> +}
> +
>  # Run a test on the target to see if it supports Aarch64 SVE hardware.
>  # Return 0 if so, 1 if it does not.  Note this causes a restart of GDB.
> 
> --
> 2.20.1

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

RE: [PATCH 10/10] btrace: Extend event decoding for ptwrite.

[Willgerodt, Felix]

On 6/4/19 2:35 PM, Metzger, Markus T wrote:
>> @@ -1244,6 +1244,53 @@ handle_pt_insn_events (struct 
>> btrace_thread_info *btinfo,
>> 		   bfun->insn_offset - 1, offset);
>> 
>>  	  break;
>> +
>> +	case ptev_ptwrite:
>> +	  {
>> +	    uint64_t *ip = nullptr;
>> +	    gdb::unique_xmalloc_ptr<char> ptw_string = nullptr;
>> +	    struct btrace_insn ptw_insn;
>> +
>> +	    /* Lookup the ip.  */
>> +	    if (event.ip_suppressed == 0)
>> +	      ip = &event.variant.ptwrite.ip;
>> +	    else if (!btinfo->functions.empty ()
>> +		     && !btinfo->functions.back ().insn.empty ())
>> +	      ip = &btinfo->functions.back ().insn.back ().pc;

Markus> Libipt will provide the PTWRITE instruction's IP.  We may get PTW events without IP but those wouldn't bind to the previous instruction - or libipt would have filled in the event IP.
Markus > If EVENT.IP_SUPPRESSED is set, there is no IP.

This is a fallback to get the IP from somewhere. In the case that ip is suppressed - but recording worked fine - this should still provide the user with the right IP. That said, I am fine with removing it, it is probably not a common occurrence.

>> +	    else
>> +	      /* This should never happen.  */
>> +	      error (_("Failed to obtain the PC of the current 
>> +instruction."));

Markus> We explicitly allow a nullptr IP in the callback function.

Right removed.

>> +
>> +	    try
>> +	      {
>> +		if (btinfo->ptw_callback_fun != nullptr)
>> +		  ptw_string = btinfo->ptw_callback_fun (
>> +					  &event.variant.ptwrite.payload, ip,
>> +					  btinfo->ptw_listener);
>> +	      }
>> +	    catch (const gdb_exception_error &error)
>> +	      {
>> +		warning (_("Failed to call ptwrite listener."));

Markus> This indicates a bug in the user's ptwrite-listener object.  Should we print a backtrace and fail with an error?

Python code errors are actually already caught and printed in py-record-btrace.c already. I think the idea was some protection against possible errors with the callback registration or other weird GDB problems. I removed the try/catch, as I think that shouldn't really happen, it is all GDB internal.

>> +	      }
>> +
>> +	    if (ptw_string == nullptr)
>> +	      break;
>> +
>> +	    btinfo->aux_data.emplace_back (ptw_string.get ());
>> +
>> +	    /* Update insn list with ptw payload insn.  */
>> +	    ptw_insn.aux_data_index = btinfo->aux_data.size () - 1;
>> +	    ptw_insn.flags = 0;

Markus> Can we preserve the SPECULATIVE flag?

Yes, done now.

>> +When recording multithreaded programs, @value{GDBN} creates a new 
>> +copy of

Markus>Also when recording a single threaded program.  Maybe just remove this part.

Done

>> the
>> +listener function for each thread to allow for independent internal states.
>> +There is currently no support for registering different listeners for 
>> +different threads. The listener can however distinguish between 
>> +multiple threads

Markus> You write 'currently'.  Are there plans to support different listeners for different threads?

I removed 'currently'.

>> +with the help of @code{gdb.selected_thread().global_num} or similar.
>> +
>> +@exdent For example:
>> +@smallexample
>> +(gdb) python-interactive
>> +>>> def my_listener(payload, ip):
>> +...     if gdb.selected_thread().global_num == 1:
>> +...         return "payload: @{0@}, ip: @{:#x@}".format(payload, ip)
>> +...     else:
>> +...         return None
>> +...

Markus> An example where the listener accumulates payloads would be nice.

I rewrote this one to include a counter that is incremented. For the accumulated payloads you would really need
to show the payload values and therefore the source code for it to make sense. This made the example too long.

>> +++ b/gdb/testsuite/gdb.btrace/ptwrite.c
>> @@ -0,0 +1,40 @@
>> +/* This testcase is part of GDB, the GNU debugger.
>> +
>> + Copyright 2018 Free Software Foundation, Inc.

Markus> It's 2019, meanwhile.

Done

> +int
> +main (void)
> +{
> +

>Spurious empty line.

Done

>> +
>> +if { [skip_btrace_ptw_tests] } {
>> +    unsupported "Hardware does not support ptwrite instructions."
>> +    return -1
>> +}
>> +
>> +set comp_flags "additional_flags=-I${srcdir}/.."

Markus> If we need this only in the COMPILE case, I'd move it there; or just inline it.

Removed, was a left over from earlier revisions.

>> +if [prepare_for_testing "failed to prepare" $testfile $srcfile $opts] {
>> +    unsupported "Compiler does not support ptwrite."

Markus> I don't think we need an 'unsupported' here.  We should get 'failed to prepare', already.

Fixed

> +### 1. Default testrun
> +
> +# Setup recording

Markus> You can use with_test_prefix to avoid repeating the prefix in each test.

Using with_test_prefix would have resulted in some ugly formatting with 
the multi-line test statements. I preferred to remove the prefix from the 
tests that don't need it to have a unique name.


>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp 
>> index c703a7e6338..ba7ccfd46ba 100644
>> --- a/gdb/testsuite/lib/gdb.exp
>> +++ b/gdb/testsuite/lib/gdb.exp
>> @@ -2874,6 +2874,98 @@ gdb_caching_proc skip_btrace_pt_tests {
>>      return $skip_btrace_tests
>>  }
>> 
>> +# Run a test on the target to see if it supports ptwrite instructions.
>> +# Return 0 if so, 1 if it does not.  Based on 'check_vmx_hw_available'
>> +# from the GCC testsuite.
>> +
>> +gdb_caching_proc skip_btrace_ptw_tests {
>> +    global srcdir subdir gdb_prompt inferior_exited_re
>> +
>> +    set me "skip_ptw_tests"
>> +
>> +    # Set up, compile, and execute a test program.
>> +    # Include the current process ID in the file names to prevent conflicts
>> +    # with invocations for multiple testsuites.
>> +    set src [standard_temp_file ptw[pid].c]
>> +    set exe [standard_temp_file ptw[pid].x]
>> +
>> +    gdb_produce_source $src {
>> +	#include <stdbool.h>
>> +	#include <stdint.h>
>> +	#include <stdio.h>
>> +	#include "x86-cpuid.h"
>> +
>> +	#define bit_PTW (1 << 4) 
>> +
>> +	bool
>> +	have_ptw ()
>> +	{
>> +	  unsigned int eax, ebx, ecx, edx;
>> +
>> +	  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
>> +	      return false;
>> +
>> +	  if ((ecx & bit_OSXSAVE) != bit_OSXSAVE)
>> +	      return false;
>> +
>> +	  if (__get_cpuid_max (0, NULL) < 0x14)
>> +	      return false;
>> +
>> +	  __cpuid_count (0x14, 0, eax, ebx, ecx, edx);
>> +
>> +	  return (ebx & bit_PTW) == bit_PTW;
>> +	}
>> +
>> +	int
>> +	main ()
>> +	{
>> +	  return have_ptw ();
>> +	}
>> +
>> +    }
>> +
>> +    verbose "$me:  compiling testfile $src" 2
>> +
>> +    set test_flags [list additional_flags=-I${srcdir}/../nat]
>> +
>> +    set compile_flags [concat {debug nowarnings quiet} ${test_flags}]
>> +    set lines [gdb_compile $src $exe executable $compile_flags]
>> +
>> +    if ![string match "" $lines] then {
>> +	verbose "$me:  testfile compilation failed, returning 1" 2
>> +	file delete $src
>> +	return 1
>> +    }
>> +
>> +    # No error message, compilation succeeded so now run it via gdb.
>> +    gdb_exit
>> +    gdb_start
>> +    gdb_reinitialize_dir $srcdir/$subdir
>> +    gdb_load $exe
>> +    if ![runto_main] {
>> +	file delete $src
>> +	return 1
>> +    }
>> +    file delete $src
>> +
>> +    # In case of an unexpected output, we return 2 as a fail value.
>> +    set skip_ptw_tests 2
>> +    gdb_test_multiple "print (int) have_ptw ()" "check ptw support" {
>> +	-re ".. = 1\r\n$gdb_prompt $" {
>> +	    set skip_ptw_tests 0
>> +	}
>> +	-re ".. = 0\r\n$gdb_prompt $" {
>> +	    set skip_ptw_tests 1
>> +	}
>> +    }
>> +
>> +    gdb_exit
>> +    remote_file build delete $exe
>> +
>> +    verbose "$me:  returning $skip_ptw_tests" 2
>> +    return $skip_ptw_tests

Markus> We still don't know if the OS supports it, do we?

True, I rewrote the whole function to do it similar to other functions we added in the meanwhile,
and added checks whether "/sys/bus/event_source/devices/intel_pt/caps/ptwrite" is set.

Thanks,
Felix
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH 05/10] python: Introduce gdb.RecordAuxiliary class.

[felix.willgerodt]

From: Felix Willgerodt <felix.willgerodt@intel.com>

Auxiliary instructions are no real instructions and get their own object
class, similar to gaps. gdb.Record.instruction_history is now possibly a
list of gdb.RecordInstruction, gdb.RecordGap or gdb.RecordAuxiliary
objects.

This patch is in preparation for the new ptwrite feature, which is based on
auxiliary instructions.

2019-05-29  Felix Willgerodt  <felix.willgerodt@intel.com>

gdb/ChangeLog:
	* py-record-btrace.c (btpy_insn_or_gap_new): Removed.
	(btpy_item_new): New function.
	(btpy_list_item): Call btpy_item_new instead of recpy_insn_new.
	(recpy_bt_replay_position): Call btpy_item_new instead of
	btpy_insn_or_gap_new.
	(recpy_bt_begin): Call btpy_item_new instead of btpy_insn_or_gap_new.
	(recpy_bt_end): Call btpy_item_new instead of btpy_insn_or_gap_new.
	* py-record.c (recpy_aux_type): New static object.
	(recpy_aux_object): New typedef.
	(recpy_aux_new, recpy_aux_number, recpy_aux_data): New function.
	(recpy_aux_getset): New static object.
	(gdbpy_initialize_record): Initialize gdb.RecordAuxiliary type.

gdb/doc/ChangeLog:
	* python.texi (gdb.RecordAuxiliary): New documentation.

---
 gdb/doc/python.texi           | 13 +++++++
 gdb/python/py-record-btrace.c | 22 +++++++----
 gdb/python/py-record.c        | 73 ++++++++++++++++++++++++++++++++++-
 gdb/python/py-record.h        |  3 ++
 4 files changed, 103 insertions(+), 8 deletions(-)

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 98e52bb770c..d4149f364e1 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -3495,6 +3495,19 @@ the current recording method.
 A human readable string with the reason for the gap.
 @end defvar
 
+Some @value{GDBN} features write auxiliary information into the execution
+history.  This information is represented by a @code{gdb.RecordAuxiliary} object
+in the instruction list.  It has the following attributes:
+
+@defvar RecordAuxiliary.number
+An integer identifying this auxiliary.  @code{number} corresponds to the numbers
+seen in @code{record instruction-history} (@pxref{Process Record and Replay}).
+@end defvar
+
+@defvar RecordAuxiliary.data
+A string representation of the auxiliary data.
+@end defvar
+
 A @code{gdb.RecordFunctionSegment} object has the following attributes:
 
 @defvar RecordFunctionSegment.number
diff --git a/gdb/python/py-record-btrace.c b/gdb/python/py-record-btrace.c
index e7153fb6d7b..81e43a00516 100644
--- a/gdb/python/py-record-btrace.c
+++ b/gdb/python/py-record-btrace.c
@@ -150,10 +150,11 @@ btrace_func_from_recpy_func (const PyObject * const pyobject)
 }
 
 /* Looks at the recorded item with the number NUMBER and create a
-   gdb.RecordInstruction or gdb.RecordGap object for it accordingly.  */
+   gdb.RecordInstruction, gdb.RecordGap or gdb.RecordAuxiliary object
+   for it accordingly.  */
 
 static PyObject *
-btpy_insn_or_gap_new (thread_info *tinfo, Py_ssize_t number)
+btpy_item_new (thread_info *tinfo, Py_ssize_t number)
 {
   btrace_insn_iterator iter;
   int err_code;
@@ -172,6 +173,14 @@ btpy_insn_or_gap_new (thread_info *tinfo, Py_ssize_t number)
       return recpy_gap_new (err_code, err_string, number);
     }
 
+  const struct btrace_insn *insn = btrace_insn_get (&iter);
+
+  gdb_assert (insn != nullptr);
+
+  if (insn->iclass == BTRACE_INSN_AUX)
+    return recpy_aux_new (iter.btinfo->aux_data[insn->aux_data_index].c_str (),
+			  number);
+
   return recpy_insn_new (tinfo, RECORD_METHOD_BTRACE, number);
 }
 
@@ -470,7 +479,7 @@ btpy_list_item (PyObject *self, Py_ssize_t index)
   number = obj->first + (obj->step * index);
 
   if (obj->element_type == &recpy_insn_type)
-    return recpy_insn_new (obj->thread, RECORD_METHOD_BTRACE, number);
+    return btpy_item_new (obj->thread, number);
   else
     return recpy_func_new (obj->thread, RECORD_METHOD_BTRACE, number);
 }
@@ -658,8 +667,7 @@ recpy_bt_replay_position (PyObject *self, void *closure)
   if (tinfo->btrace.replay == NULL)
     Py_RETURN_NONE;
 
-  return btpy_insn_or_gap_new (tinfo,
-			       btrace_insn_number (tinfo->btrace.replay));
+  return btpy_item_new (tinfo, btrace_insn_number (tinfo->btrace.replay));
 }
 
 /* Implementation of
@@ -681,7 +689,7 @@ recpy_bt_begin (PyObject *self, void *closure)
     Py_RETURN_NONE;
 
   btrace_insn_begin (&iterator, &tinfo->btrace);
-  return btpy_insn_or_gap_new (tinfo, btrace_insn_number (&iterator));
+  return btpy_item_new (tinfo, btrace_insn_number (&iterator));
 }
 
 /* Implementation of
@@ -703,7 +711,7 @@ recpy_bt_end (PyObject *self, void *closure)
     Py_RETURN_NONE;
 
   btrace_insn_end (&iterator, &tinfo->btrace);
-  return btpy_insn_or_gap_new (tinfo, btrace_insn_number (&iterator));
+  return btpy_item_new (tinfo, btrace_insn_number (&iterator));
 }
 
 /* Implementation of
diff --git a/gdb/python/py-record.c b/gdb/python/py-record.c
index d46a03e75ac..8b1b3c4e651 100644
--- a/gdb/python/py-record.c
+++ b/gdb/python/py-record.c
@@ -49,6 +49,12 @@ PyTypeObject recpy_gap_type = {
   PyVarObject_HEAD_INIT (NULL, 0)
 };
 
+/* Python RecordAuxiliary type.  */
+
+PyTypeObject recpy_aux_type = {
+  PyVarObject_HEAD_INIT (NULL, 0)
+};
+
 /* Python RecordGap object.  */
 typedef struct
 {
@@ -64,6 +70,18 @@ typedef struct
   Py_ssize_t number;
 } recpy_gap_object;
 
+/* Python RecordAuxiliary object.  */
+typedef struct
+{
+  PyObject_HEAD
+
+  /* Auxiliary data.  */
+  const char *data;
+
+  /* Element number.  */
+  Py_ssize_t number;
+} recpy_aux_object;
+
 /* Implementation of record.method.  */
 
 static PyObject *
@@ -477,6 +495,43 @@ recpy_gap_reason_string (PyObject *self, void *closure)
   return PyString_FromString (obj->reason_string);
 }
 
+/* Create a new gdb.Auxiliary object.  */
+
+PyObject *
+recpy_aux_new (const char *data, Py_ssize_t number)
+{
+  recpy_aux_object * const obj = PyObject_New (recpy_aux_object,
+					       &recpy_aux_type);
+
+  if (obj == NULL)
+   return NULL;
+
+  obj->data = data;
+  obj->number = number;
+
+  return (PyObject *) obj;
+}
+
+/* Implementation of Auxiliary.number [int].  */
+
+static PyObject *
+recpy_aux_number (PyObject *self, void *closure)
+{
+  const recpy_aux_object * const obj = (const recpy_aux_object *) self;
+
+  return PyInt_FromSsize_t (obj->number);
+}
+
+/* Implementation of Auxiliary.data [str].  */
+
+static PyObject *
+recpy_aux_data (PyObject *self, void *closure)
+{
+  const recpy_aux_object * const obj = (const recpy_aux_object *) self;
+
+  return PyString_FromString (obj->data);
+}
+
 /* Record method list.  */
 
 static PyMethodDef recpy_record_methods[] = {
@@ -542,6 +597,14 @@ static gdb_PyGetSetDef recpy_gap_getset[] = {
   { NULL }
 };
 
+/* RecordAuxiliary member list.  */
+
+static gdb_PyGetSetDef recpy_aux_getset[] = {
+  { "number", recpy_aux_number, NULL, "element number", NULL},
+  { "data", recpy_aux_data, NULL, "data", NULL},
+  { NULL }
+};
+
 /* Sets up the record API in the gdb module.  */
 
 int
@@ -581,10 +644,18 @@ gdbpy_initialize_record (void)
   recpy_gap_type.tp_doc = "GDB recorded gap object";
   recpy_gap_type.tp_getset = recpy_gap_getset;
 
+  recpy_aux_type.tp_new = PyType_GenericNew;
+  recpy_aux_type.tp_flags = Py_TPFLAGS_DEFAULT;
+  recpy_aux_type.tp_basicsize = sizeof (recpy_aux_object);
+  recpy_aux_type.tp_name = "gdb.RecordAuxiliary";
+  recpy_aux_type.tp_doc = "GDB recorded auxiliary object";
+  recpy_aux_type.tp_getset = recpy_aux_getset;
+
   if (PyType_Ready (&recpy_record_type) < 0
       || PyType_Ready (&recpy_insn_type) < 0
       || PyType_Ready (&recpy_func_type) < 0
-      || PyType_Ready (&recpy_gap_type) < 0)
+      || PyType_Ready (&recpy_gap_type) < 0
+      || PyType_Ready (&recpy_aux_type) < 0)
     return -1;
   else
     return 0;
diff --git a/gdb/python/py-record.h b/gdb/python/py-record.h
index 74da1bdc4d6..6e0ff23ad37 100644
--- a/gdb/python/py-record.h
+++ b/gdb/python/py-record.h
@@ -71,4 +71,7 @@ extern PyObject *recpy_func_new (thread_info *thread, enum record_method method,
 extern PyObject *recpy_gap_new (int reason_code, const char *reason_string,
 				Py_ssize_t number);
 
+/* Create a new gdb.RecordGap object.  */
+extern PyObject *recpy_aux_new (const char *data, Py_ssize_t number);
+
 #endif /* PYTHON_PY_RECORD_H */
-- 
2.20.1

Re: [PATCH 05/10] python: Introduce gdb.RecordAuxiliary class.

[Eli Zaretskii]

> From: felix.willgerodt@intel.com
> Cc: markus.t.metzger@intel.com, Felix Willgerodt <felix.willgerodt@intel.com>
> Date: Wed, 29 May 2019 10:47:48 +0200
> 
> gdb/doc/ChangeLog:
> 	* python.texi (gdb.RecordAuxiliary): New documentation.

OK for this part, thanks.

RE: [PATCH 05/10] python: Introduce gdb.RecordAuxiliary class.

[Metzger, Markus T]

Hello Felix,

> Auxiliary instructions are no real instructions and get their own object
> class, similar to gaps. gdb.Record.instruction_history is now possibly a
> list of gdb.RecordInstruction, gdb.RecordGap or gdb.RecordAuxiliary
> objects.
> 
> This patch is in preparation for the new ptwrite feature, which is based on
> auxiliary instructions.
> 
> 2019-05-29  Felix Willgerodt  <felix.willgerodt@intel.com>
> 
> gdb/ChangeLog:
> 	* py-record-btrace.c (btpy_insn_or_gap_new): Removed.
> 	(btpy_item_new): New function.
> 	(btpy_list_item): Call btpy_item_new instead of recpy_insn_new.
> 	(recpy_bt_replay_position): Call btpy_item_new instead of
> 	btpy_insn_or_gap_new.
> 	(recpy_bt_begin): Call btpy_item_new instead of btpy_insn_or_gap_new.
> 	(recpy_bt_end): Call btpy_item_new instead of btpy_insn_or_gap_new.
> 	* py-record.c (recpy_aux_type): New static object.
> 	(recpy_aux_object): New typedef.
> 	(recpy_aux_new, recpy_aux_number, recpy_aux_data): New function.
> 	(recpy_aux_getset): New static object.
> 	(gdbpy_initialize_record): Initialize gdb.RecordAuxiliary type.
> 
> gdb/doc/ChangeLog:
> 	* python.texi (gdb.RecordAuxiliary): New documentation.
> 
> ---
>  gdb/doc/python.texi           | 13 +++++++
>  gdb/python/py-record-btrace.c | 22 +++++++----
>  gdb/python/py-record.c        | 73 ++++++++++++++++++++++++++++++++++-
>  gdb/python/py-record.h        |  3 ++
>  4 files changed, 103 insertions(+), 8 deletions(-)

Looks good to me but I can't approve it.

> @@ -172,6 +173,14 @@ btpy_insn_or_gap_new (thread_info *tinfo, Py_ssize_t
> number)
>        return recpy_gap_new (err_code, err_string, number);
>      }
> 
> +  const struct btrace_insn *insn = btrace_insn_get (&iter);
> +
> +  gdb_assert (insn != nullptr);

This must be true because we checked btrace_insn_get_error () before.  To make it
more obvious, we could rewrite the function to call btrace_insn_get () first and check
the returned btrace_insn pointer.  There's no need to check the error code, I think.
In the worst case, we'll end up with a gap that says 'success'.

> @@ -470,7 +479,7 @@ btpy_list_item (PyObject *self, Py_ssize_t index)
>    number = obj->first + (obj->step * index);
> 
>    if (obj->element_type == &recpy_insn_type)
> -    return recpy_insn_new (obj->thread, RECORD_METHOD_BTRACE, number);
> +    return btpy_item_new (obj->thread, number);
>    else
>      return recpy_func_new (obj->thread, RECORD_METHOD_BTRACE, number);
>  }

Isn't this code just creating list item objects?

Thanks,
Markus.

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

Re: [PATCH 05/10] python: Introduce gdb.RecordAuxiliary class.

[Willgerodt, Felix]

On 6/4/19 2:35 PM, Metzger, Markus T wrote:
> Hello Felix,
>
>> Auxiliary instructions are no real instructions and get their own object
>> class, similar to gaps. gdb.Record.instruction_history is now possibly a
>> list of gdb.RecordInstruction, gdb.RecordGap or gdb.RecordAuxiliary
>> objects.
>>
>> This patch is in preparation for the new ptwrite feature, which is based on
>> auxiliary instructions.
>>
>> 2019-05-29  Felix Willgerodt  <felix.willgerodt@intel.com>
>>
>> gdb/ChangeLog:
>> * py-record-btrace.c (btpy_insn_or_gap_new): Removed.
>> (btpy_item_new): New function.
>> (btpy_list_item): Call btpy_item_new instead of recpy_insn_new.
>> (recpy_bt_replay_position): Call btpy_item_new instead of
>> btpy_insn_or_gap_new.
>> (recpy_bt_begin): Call btpy_item_new instead of btpy_insn_or_gap_new.
>> (recpy_bt_end): Call btpy_item_new instead of btpy_insn_or_gap_new.
>> * py-record.c (recpy_aux_type): New static object.
>> (recpy_aux_object): New typedef.
>> (recpy_aux_new, recpy_aux_number, recpy_aux_data): New function.
>> (recpy_aux_getset): New static object.
>> (gdbpy_initialize_record): Initialize gdb.RecordAuxiliary type.
>>
>> gdb/doc/ChangeLog:
>> * python.texi (gdb.RecordAuxiliary): New documentation.
>>
>> ---
>>   gdb/doc/python.texi           | 13 +++++++
>>   gdb/python/py-record-btrace.c | 22 +++++++----
>>   gdb/python/py-record.c        | 73 ++++++++++++++++++++++++++++++++++-
>>   gdb/python/py-record.h        |  3 ++
>>   4 files changed, 103 insertions(+), 8 deletions(-)
>
> Looks good to me but I can't approve it.
>
>> @@ -172,6 +173,14 @@ btpy_insn_or_gap_new (thread_info *tinfo, Py_ssize_t
>> number)
>>         return recpy_gap_new (err_code, err_string, number);
>>       }
>>
>> +  const struct btrace_insn *insn = btrace_insn_get (&iter);
>> +
>> +  gdb_assert (insn != nullptr);

Markus> This must be true because we checked btrace_insn_get_error () before.  To make it
Markus> more obvious, we could rewrite the function to call btrace_insn_get () first and check
Markus> the returned btrace_insn pointer.  There's no need to check the error code, I think.
Markus> In the worst case, we'll end up with a gap that says 'success'.

You are right, removed it.

>> @@ -470,7 +479,7 @@ btpy_list_item (PyObject *self, Py_ssize_t index)
>>     number = obj->first + (obj->step * index);
>>
>>     if (obj->element_type == &recpy_insn_type)
>> -    return recpy_insn_new (obj->thread, RECORD_METHOD_BTRACE, number);
>> +    return btpy_item_new (obj->thread, number);
>>     else
>>       return recpy_func_new (obj->thread, RECORD_METHOD_BTRACE, number);
>>   }

Markus> Isn't this code just creating list item objects?

Yes, but when creating the list we now don't want to add RecordAuxiliary
objects as RecordInstruction objects. The only list objects created are of type
recpy_insn_type or recpy_func_type.


Thanks,
Felix
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH 02/10] btrace: Enable auxiliary instructions in record instruction-history.

[felix.willgerodt]

From: Felix Willgerodt <felix.willgerodt@intel.com>

Print the auxiliary data when a btrace_insn of type BTRACE_INSN_AUX
is encountered in the instruction-history.

This patch is in preparation for the ptwrite feature, which is based on
auxiliary instructions.

2019-05-29  Felix Willgerodt  <felix.willgerodt@intel.com>

gdb/ChangeLog:
	* record-btrace.c (btrace_insn_history): Handle BTRACE_INSN_AUX.

---
 gdb/record-btrace.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 21085d5c62c..5fb967c40f0 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -811,6 +811,16 @@ btrace_insn_history (struct ui_out *uiout,
 	  btrace_ui_out_decode_error (uiout, btrace_insn_get_error (&it),
 				      conf->format);
 	}
+      else if (insn->iclass == BTRACE_INSN_AUX)
+	{
+	  uiout->field_fmt ("insn-number", "%u", btrace_insn_number (&it));
+	  uiout->text ("\t");
+	  uiout->spaces (3);
+	  uiout->text ("[");
+	  uiout->field_fmt ("aux-insn", "%s",
+			    it.btinfo->aux_data[insn->aux_data_index].c_str ());
+	  uiout->text ("]\n");
+	}
       else
 	{
 	  struct disasm_insn dinsn;
-- 
2.20.1

RE: [PATCH 02/10] btrace: Enable auxiliary instructions in record instruction-history.

[Metzger, Markus T]

Hello Felix,

> Print the auxiliary data when a btrace_insn of type BTRACE_INSN_AUX
> is encountered in the instruction-history.
> 
> This patch is in preparation for the ptwrite feature, which is based on
> auxiliary instructions.
> 
> 2019-05-29  Felix Willgerodt  <felix.willgerodt@intel.com>
> 
> gdb/ChangeLog:
> 	* record-btrace.c (btrace_insn_history): Handle BTRACE_INSN_AUX.
> 
> ---
>  gdb/record-btrace.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)

LGTM with one tiny nit below.

> @@ -811,6 +811,16 @@ btrace_insn_history (struct ui_out *uiout,
>  	  btrace_ui_out_decode_error (uiout, btrace_insn_get_error (&it),
>  				      conf->format);
>  	}
> +      else if (insn->iclass == BTRACE_INSN_AUX)
> +	{
> +	  uiout->field_fmt ("insn-number", "%u", btrace_insn_number (&it));
> +	  uiout->text ("\t");
> +	  uiout->spaces (3);
> +	  uiout->text ("[");
> +	  uiout->field_fmt ("aux-insn", "%s",

The field is called aux_data.   Should they be aligned?

> +			    it.btinfo->aux_data[insn->aux_data_index].c_str ());
> +	  uiout->text ("]\n");
> +	}
>        else
>  	{
>  	  struct disasm_insn dinsn;
> --
> 2.20.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, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PATCH 02/10] btrace: Enable auxiliary instructions in record instruction-history.

[Willgerodt, Felix]

On 6/4/19 2:35 PM, Metzger, Markus T wrote:
>> Print the auxiliary data when a btrace_insn of type BTRACE_INSN_AUX is 
>> encountered in the instruction-history.
>> 
>> This patch is in preparation for the ptwrite feature, which is based 
>> on auxiliary instructions.
>> 
>> 2019-05-29  Felix Willgerodt  <felix.willgerodt@intel.com>
>> 
>> gdb/ChangeLog:
>> 	* record-btrace.c (btrace_insn_history): Handle BTRACE_INSN_AUX.
>> 
>> ---
>>  gdb/record-btrace.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)

Markus> LGTM with one tiny nit below.

>> @@ -811,6 +811,16 @@ btrace_insn_history (struct ui_out *uiout,
>>  	  btrace_ui_out_decode_error (uiout, btrace_insn_get_error (&it),
>>  				      conf->format);
>>  	}
>> +      else if (insn->iclass == BTRACE_INSN_AUX)
>> +	{
>> +	  uiout->field_fmt ("insn-number", "%u", btrace_insn_number (&it));
>> +	  uiout->text ("\t");
>> +	  uiout->spaces (3);
>> +	  uiout->text ("[");
>> +	  uiout->field_fmt ("aux-insn", "%s",

Markus> The field is called aux_data.   Should they be aligned?

Done.

Thanks,
Felix


Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928