[3/7] btrace: Enable event tracing on Linux for Intel PT.

  Event tracing allows GDB to show information about interesting asyncronous
events when tracing with Intel PT.  Subsequent patches will add support for
displaying each type of event.

Enabling event-tracing unconditionally would result in rather noisy output, as
breakpoints themselves result in interrupt events.  Which is why this patch adds
a set/show command to allow the user to enable/disable event-tracing before
starting a recording. The event-tracing setting has no effect on an already
active recording.  The default setting is off.   As event tracing will use the
auxiliary infrastructure added by ptwrite, the user can still disable printing
events, even when event-tracing was enabled, by using the /a switch for the
record instruction-history/function-call-history commands.
---
 gdb/doc/gdb.texinfo          | 41 +++++++++++++++++++++++++----
 gdb/features/btrace-conf.dtd |  1 +
 gdb/nat/linux-btrace.c       | 11 ++++++++
 gdb/record-btrace.c          | 44 +++++++++++++++++++++++++++++++
 gdb/remote.c                 | 50 +++++++++++++++++++++++++++++++++++-
 gdbserver/linux-low.cc       |  2 ++
 gdbserver/server.cc          | 14 ++++++++++
 gdbsupport/btrace-common.h   |  3 +++
 8 files changed, 160 insertions(+), 6 deletions(-)
  

> From: Felix Willgerodt <felix.willgerodt@intel.com>
> Date: Mon,  2 Sep 2024 13:49:45 +0200
> 
> Event tracing allows GDB to show information about interesting asyncronous
                                                                 ^^^^^^^^^^^
Typo.

>  gdb/doc/gdb.texinfo          | 41 +++++++++++++++++++++++++----
>  gdb/features/btrace-conf.dtd |  1 +
>  gdb/nat/linux-btrace.c       | 11 ++++++++
>  gdb/record-btrace.c          | 44 +++++++++++++++++++++++++++++++
>  gdb/remote.c                 | 50 +++++++++++++++++++++++++++++++++++-
>  gdbserver/linux-low.cc       |  2 ++
>  gdbserver/server.cc          | 14 ++++++++++
>  gdbsupport/btrace-common.h   |  3 +++
>  8 files changed, 160 insertions(+), 6 deletions(-)

Thanks, the documentation parts are OK.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
  

> Date: Mon, 02 Sep 2024 15:09:12 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: gdb-patches@sourceware.org, markus.t.metzger@intel.com
> 
> > From: Felix Willgerodt <felix.willgerodt@intel.com>
> > Date: Mon,  2 Sep 2024 13:49:45 +0200
> > 
> > Event tracing allows GDB to show information about interesting asyncronous
>                                                                  ^^^^^^^^^^^
> Typo.
> 
> >  gdb/doc/gdb.texinfo          | 41 +++++++++++++++++++++++++----
> >  gdb/features/btrace-conf.dtd |  1 +
> >  gdb/nat/linux-btrace.c       | 11 ++++++++
> >  gdb/record-btrace.c          | 44 +++++++++++++++++++++++++++++++
> >  gdb/remote.c                 | 50 +++++++++++++++++++++++++++++++++++-
> >  gdbserver/linux-low.cc       |  2 ++
> >  gdbserver/server.cc          | 14 ++++++++++
> >  gdbsupport/btrace-common.h   |  3 +++
> >  8 files changed, 160 insertions(+), 6 deletions(-)
> 
> Thanks, the documentation parts are OK.

Actually, one question: should this be mentioned in NEWS?
  

> -----Original Message-----
> From: Eli Zaretskii <eliz@gnu.org>
> Sent: Montag, 2. September 2024 14:11
> To: Willgerodt, Felix <felix.willgerodt@intel.com>
> Cc: gdb-patches@sourceware.org; Metzger, Markus T
> <markus.t.metzger@intel.com>
> Subject: Re: [PATCH 3/7] btrace: Enable event tracing on Linux for Intel PT.
> 
> > Date: Mon, 02 Sep 2024 15:09:12 +0300
> > From: Eli Zaretskii <eliz@gnu.org>
> > Cc: gdb-patches@sourceware.org, markus.t.metzger@intel.com
> >
> > > From: Felix Willgerodt <felix.willgerodt@intel.com>
> > > Date: Mon,  2 Sep 2024 13:49:45 +0200
> > >
> > > Event tracing allows GDB to show information about interesting asyncronous
> >                                                                  ^^^^^^^^^^^
> > Typo.
> >
> > >  gdb/doc/gdb.texinfo          | 41 +++++++++++++++++++++++++----
> > >  gdb/features/btrace-conf.dtd |  1 +
> > >  gdb/nat/linux-btrace.c       | 11 ++++++++
> > >  gdb/record-btrace.c          | 44 +++++++++++++++++++++++++++++++
> > >  gdb/remote.c                 | 50 +++++++++++++++++++++++++++++++++++-
> > >  gdbserver/linux-low.cc       |  2 ++
> > >  gdbserver/server.cc          | 14 ++++++++++
> > >  gdbsupport/btrace-common.h   |  3 +++
> > >  8 files changed, 160 insertions(+), 6 deletions(-)
> >
> > Thanks, the documentation parts are OK.
> 
> Actually, one question: should this be mentioned in NEWS?

Hi Eli,

Thanks for the review, I have fixed the typo locally. And yes, this probably warrants
a NEWS entry. I have written this now in my local branch:

diff --git a/gdb/NEWS b/gdb/NEWS
index 2f5d5ebbcef..8f45593cf02 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,11 @@
 
 *** Changes since GDB 15
 
+* GDB now supports printing of asynchronous events from the Intel Processor
+  Trace during 'record instruction-history', 'record function-call-history'
+  and all stepping commands.  This can be controlled with the new
+  "set record btrace pt event-tracing" command.
+
 * GDB now supports printing of ptwrite payloads from the Intel Processor
   Trace during 'record instruction-history', 'record function-call-history'
   and all stepping commands.  The payload is also accessible in Python as a

Regards,
Felix

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
  

> From: "Willgerodt, Felix" <felix.willgerodt@intel.com>
> CC: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>, "Metzger,
>  Markus T" <markus.t.metzger@intel.com>
> Date: Mon, 2 Sep 2024 13:32:46 +0000
> 
> Thanks for the review, I have fixed the typo locally. And yes, this probably warrants
> a NEWS entry. I have written this now in my local branch:
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 2f5d5ebbcef..8f45593cf02 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -3,6 +3,11 @@
>  
>  *** Changes since GDB 15
>  
> +* GDB now supports printing of asynchronous events from the Intel Processor
> +  Trace during 'record instruction-history', 'record function-call-history'
> +  and all stepping commands.  This can be controlled with the new
> +  "set record btrace pt event-tracing" command.
> +
>  * GDB now supports printing of ptwrite payloads from the Intel Processor
>    Trace during 'record instruction-history', 'record function-call-history'
>    and all stepping commands.  The payload is also accessible in Python as a

LGTM, thanks.
  

Hello Felix,

>+  if (conf->event_tracing)
>+    {
>+      if (linux_supports_pt_feature ("event_trace"))
>+	{
>+	  tinfo->attr.config |= linux_read_pt_config_bitmask ("event");
>+	  tinfo->conf.pt.event_tracing = conf->event_tracing;

We know that conf->event_tracing is true, here; we checked it above.

>+	}
>+      else
>+	error (_("Event tracing for record btrace pt is not supported."));

Have you checked that this correctly results in the 'record btrace' command
getting aborted in the remote debug scenario?


>+static bool event_tracing = false;
>+
>+/* The "record pt event-tracing" show value function.  */
>+
>+static void
>+show_record_pt_event_tracing_value (struct ui_file *file, int from_tty,
>+				    struct cmd_list_element *c,
>+				    const char *value)
>+{
>+#if (LIBIPT_VERSION >= 0x201)
>+  gdb_printf (file, _("record pt event-tracing is %s.\n"), value);
>+#else
>+  gdb_printf (_("Event-tracing support was disabled at compile time.\n"));
>+#endif /* defined (LIBIPT_VERSION >= 0x201) */

I'm wondering if it is clear that GDB's compile-time is meant here.  Maybe
something like 'Event tracing is not supported by GDB.' or 'Event tracing
support has been disabled in GDB.' would be clearer.

We use a similar phrase in gdb/btrace.c to indicate lack of PT support, but it
is more difficult to change existing strings, so I'm not sure whether it makes
sense to adjust that one, too.

>+  /* Event tracing is a user setting, warn if it is set but the target
>+     doesn't support it.  */
>+  if ((m_features.packet_support (PACKET_Qbtrace_conf_pt_event_tracing)
>+       != PACKET_ENABLE)
>+	&& conf->pt.event_tracing)
>+    error (_("Target does not support event-tracing."));

The comment says warn, but the code errors out.

Regards,
Markus.

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
  

> -----Original Message-----
> From: Metzger, Markus T <markus.t.metzger@intel.com>
> Sent: Dienstag, 10. September 2024 12:48
> To: Willgerodt, Felix <felix.willgerodt@intel.com>
> Cc: gdb-patches@sourceware.org
> Subject: RE: [PATCH 3/7] btrace: Enable event tracing on Linux for Intel PT.
> 
> Hello Felix,
> 
> >+  if (conf->event_tracing)
> >+    {
> >+      if (linux_supports_pt_feature ("event_trace"))
> >+	{
> >+	  tinfo->attr.config |= linux_read_pt_config_bitmask ("event");
> >+	  tinfo->conf.pt.event_tracing = conf->event_tracing;
> 
> We know that conf->event_tracing is true, here; we checked it above.

Fixed locally.

> >+	}
> >+      else
> >+	error (_("Event tracing for record btrace pt is not supported."));
> 
> Have you checked that this correctly results in the 'record btrace' command
> getting aborted in the remote debug scenario?

I tested it both on the same host and on two different machines.
This is the behaviour for both:

0x00007ffff7fe3290 in _start () from target:/lib64/ld-linux-x86-64.so.2
(gdb) set record btrace pt event-tracing on
(gdb) rec pt
Could not enable branch tracing for Thread 537568.537568: Event tracing for record btrace pt is not supported.
(gdb)  

This looks correct to me.

> >+static bool event_tracing = false;
> >+
> >+/* The "record pt event-tracing" show value function.  */
> >+
> >+static void
> >+show_record_pt_event_tracing_value (struct ui_file *file, int from_tty,
> >+				    struct cmd_list_element *c,
> >+				    const char *value)
> >+{
> >+#if (LIBIPT_VERSION >= 0x201)
> >+  gdb_printf (file, _("record pt event-tracing is %s.\n"), value);
> >+#else
> >+  gdb_printf (_("Event-tracing support was disabled at compile time.\n"));
> >+#endif /* defined (LIBIPT_VERSION >= 0x201) */
> 
> I'm wondering if it is clear that GDB's compile-time is meant here.  Maybe
> something like 'Event tracing is not supported by GDB.' or 'Event tracing
> support has been disabled in GDB.' would be clearer.
> 
> We use a similar phrase in gdb/btrace.c to indicate lack of PT support, but it
> is more difficult to change existing strings, so I'm not sure whether it makes
> sense to adjust that one, too.

You make a good point, I just copied it but never liked it either. I will use
'Event tracing is not supported by GDB.'.

> >+  /* Event tracing is a user setting, warn if it is set but the target
> >+     doesn't support it.  */
> >+  if ((m_features.packet_support (PACKET_Qbtrace_conf_pt_event_tracing)
> >+       != PACKET_ENABLE)
> >+	&& conf->pt.event_tracing)
> >+    error (_("Target does not support event-tracing."));
> 
> The comment says warn, but the code errors out.

I will make it a warning.

Thanks,
Felix
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
  

Hello Felix,

>> >+	}
>> >+      else
>> >+	error (_("Event tracing for record btrace pt is not supported."));
>>
>> Have you checked that this correctly results in the 'record btrace' command
>> getting aborted in the remote debug scenario?
>
>I tested it both on the same host and on two different machines.
>This is the behaviour for both:
>
>0x00007ffff7fe3290 in _start () from target:/lib64/ld-linux-x86-64.so.2
>(gdb) set record btrace pt event-tracing on
>(gdb) rec pt
>Could not enable branch tracing for Thread 537568.537568: Event tracing for
>record btrace pt is not supported.
>(gdb)
>
>This looks correct to me.

This looks good.

Thanks,
Markus.

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928