Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] gdb, btrace: support libipt v2.2 events
@ 2026-04-30 11:24 Markus Metzger
  2026-04-30 18:11 ` Keith Seitz
  0 siblings, 1 reply; 8+ messages in thread
From: Markus Metzger @ 2026-04-30 11:24 UTC (permalink / raw)
  To: gdb-patches

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

diff --git a/gdb/btrace.c b/gdb/btrace.c
index 6a8f0f549d1..f9c6f0e5b6a 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -1597,6 +1597,59 @@ handle_pt_insn_events (struct btrace_thread_info *btinfo,
 	    break;
 	  }
 #endif /* defined (LIBIPT_VERSION >= 0x201) */
+
+#if (LIBIPT_VERSION >= 0x202)
+	case ptev_trig:
+	  {
+	    std::string aux_string = std::string (_("trig: trbv = "))
+	      + hex_string (event.variant.trig.trbv);
+
+	    if (event.variant.trig.mult != 0)
+	      aux_string += std::string (", mult");
+
+	    if (event.ip_suppressed == 0)
+	      {
+		pc = event.variant.trig.ip;
+		aux_string += std::string (", ip = ") + hex_string (*pc);
+	      }
+
+	    if (event.variant.trig.icnt != 0)
+	      aux_string += std::string (", icnt = ")
+		+ hex_string (event.variant.trig.icnt);
+
+	    handle_pt_aux_insn (btinfo, aux_string, pc);
+	    break;
+	  }
+
+	case ptev_swintr:
+	  {
+	    std::string aux_string = std::string (_("swintr: vector = "))
+	      + hex_string (event.variant.swintr.vector);
+
+	    if (event.ip_suppressed == 0)
+	      {
+		pc = event.variant.swintr.ip;
+		aux_string += std::string (", ip = ") + hex_string (*pc);
+	      }
+
+	    handle_pt_aux_insn (btinfo, aux_string, pc);
+	    break;
+	  }
+
+	case ptev_syscall:
+	  {
+	    std::string aux_string = std::string (_("syscall"));
+
+	    if (event.ip_suppressed == 0)
+	      {
+		pc = event.variant.syscall.ip;
+		aux_string += std::string (": ip = ") + hex_string (*pc);
+	      }
+
+	    handle_pt_aux_insn (btinfo, aux_string, pc);
+	    break;
+	  }
+#endif /* (LIBIPT_VERSION >= 0x202) */
 	}
     }
 #endif /* defined (HAVE_PT_INSN_EVENT) */
-- 
2.34.1

Intel Deutschland GmbH
Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany
Tel: +49 89 991 430, www.intel.de
Managing Directors: Harry Demas, Jeffrey Schneiderman, Yin Chong Sorrell
Chairperson of the Supervisory Board: Nicole Lau
Registered Seat: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


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

* Re: [PATCH] gdb, btrace: support libipt v2.2 events
  2026-04-30 11:24 [PATCH] gdb, btrace: support libipt v2.2 events Markus Metzger
@ 2026-04-30 18:11 ` Keith Seitz
  2026-05-01 16:39   ` Tom Tromey
  2026-05-04  7:16   ` Metzger, Markus T
  0 siblings, 2 replies; 8+ messages in thread
From: Keith Seitz @ 2026-04-30 18:11 UTC (permalink / raw)
  To: Markus Metzger, gdb-patches

Hi,

On 4/30/26 4:24 AM, Markus Metzger wrote:
> ---
>   gdb/btrace.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 53 insertions(+)

There appears to be no commit message other than the (admittedly
rather complete) title?

I think a brief NEWS entry is warranted, too.

There is one tiny nit (below), but otherwise, I think you should approve 
your patch. ;-)

Reviewed-By: Keith Seitz <keiths@redhat.com>

Keith

> 
> diff --git a/gdb/btrace.c b/gdb/btrace.c
> index 6a8f0f549d1..f9c6f0e5b6a 100644
> --- a/gdb/btrace.c
> +++ b/gdb/btrace.c
> @@ -1597,6 +1597,59 @@ handle_pt_insn_events (struct btrace_thread_info *btinfo,
>   	    break;
>   	  }
>   #endif /* defined (LIBIPT_VERSION >= 0x201) */
> +
> +#if (LIBIPT_VERSION >= 0x202)
> +	case ptev_trig:
> +	  {
> +	    std::string aux_string = std::string (_("trig: trbv = "))
> +	      + hex_string (event.variant.trig.trbv);
> +
> +	    if (event.variant.trig.mult != 0)
> +	      aux_string += std::string (", mult");
> +
> +	    if (event.ip_suppressed == 0)
> +	      {
> +		pc = event.variant.trig.ip;
> +		aux_string += std::string (", ip = ") + hex_string (*pc);
> +	      }
> +
> +	    if (event.variant.trig.icnt != 0)
> +	      aux_string += std::string (", icnt = ")
> +		+ hex_string (event.variant.trig.icnt);
> +
> +	    handle_pt_aux_insn (btinfo, aux_string, pc);
> +	    break;
> +	  }
> +
> +	case ptev_swintr:
> +	  {
> +	    std::string aux_string = std::string (_("swintr: vector = "))
> +	      + hex_string (event.variant.swintr.vector);
> +
> +	    if (event.ip_suppressed == 0)
> +	      {
> +		pc = event.variant.swintr.ip;
> +		aux_string += std::string (", ip = ") + hex_string (*pc);
> +	      }
> +
> +	    handle_pt_aux_insn (btinfo, aux_string, pc);
> +	    break;
> +	  }
> +
> +	case ptev_syscall:
> +	  {
> +	    std::string aux_string = std::string (_("syscall"));
> +
> +	    if (event.ip_suppressed == 0)
> +	      {
> +		pc = event.variant.syscall.ip;
> +		aux_string += std::string (": ip = ") + hex_string (*pc);
> +	      }
> +
> +	    handle_pt_aux_insn (btinfo, aux_string, pc);
> +	    break;
> +	  }
> +#endif /* (LIBIPT_VERSION >= 0x202) */

Elsewhere in the file, this comment includes "defined".

>   	}
>       }
>   #endif /* defined (HAVE_PT_INSN_EVENT) */


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

* Re: [PATCH] gdb, btrace: support libipt v2.2 events
  2026-04-30 18:11 ` Keith Seitz
@ 2026-05-01 16:39   ` Tom Tromey
  2026-05-04  7:16     ` Metzger, Markus T
  2026-05-04  7:16   ` Metzger, Markus T
  1 sibling, 1 reply; 8+ messages in thread
From: Tom Tromey @ 2026-05-01 16:39 UTC (permalink / raw)
  To: Keith Seitz; +Cc: Markus Metzger, gdb-patches

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

>> +	    std::string aux_string = std::string (_("trig: trbv = "))
>> +	      + hex_string (event.variant.trig.trbv);

Normally here we'd wrap the RSH in parens, per the coding standards.

>> +#endif /* (LIBIPT_VERSION >= 0x202) */

Keith> Elsewhere in the file, this comment includes "defined".

True but those spots seem to be in error.

Tom

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

* RE: [PATCH] gdb, btrace: support libipt v2.2 events
  2026-05-01 16:39   ` Tom Tromey
@ 2026-05-04  7:16     ` Metzger, Markus T
  2026-05-04 16:08       ` Simon Marchi
  2026-05-05 13:28       ` Tom Tromey
  0 siblings, 2 replies; 8+ messages in thread
From: Metzger, Markus T @ 2026-05-04  7:16 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches, Keith Seitz

Hello Tom,

>>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:
>
>>> +	    std::string aux_string = std::string (_("trig: trbv = "))
>>> +	      + hex_string (event.variant.trig.trbv);
>
>Normally here we'd wrap the RSH in parens, per the coding standards.

I'd need to break at the =, then, like this:

	    std::string aux_string
	      = (std::string (_("trig: trbv = "))
		 + hex_string (event.variant.trig.trbv));

The patch matches the surrounding style, so if you want this fixed, I'd need
to fix it everywhere in a separate patch.


>>> +#endif /* (LIBIPT_VERSION >= 0x202) */
>
>Keith> Elsewhere in the file, this comment includes "defined".
>
>True but those spots seem to be in error.

I fixed those in a separate obvious patch:

diff --git a/gdb/btrace.c b/gdb/btrace.c
index f9c6f0e5b6a..ef152c531fb 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -1283,7 +1283,7 @@ decode_interrupt_vector (const uint8_t vector)
 
   return nullptr;
 }
-#endif /* defined (LIBIPT_VERSION >= 0x201) */
+#endif /* (LIBIPT_VERSION >= 0x201) */
 
 /* Handle instruction decode events (libipt-v2).  */
 
@@ -1596,7 +1596,7 @@ handle_pt_insn_events (struct btrace_thread_info *btinfo,
            handle_pt_aux_insn (btinfo, aux_string, pc);
            break;
          }
-#endif /* defined (LIBIPT_VERSION >= 0x201) */
+#endif /* (LIBIPT_VERSION >= 0x201) */
 
 #if (LIBIPT_VERSION >= 0x202)
        case ptev_trig:
@@ -3064,7 +3064,7 @@ pt_print_packet (const struct pt_packet *packet)
                  packet->payload.ptw.payload,
                  packet->payload.ptw.ip ? (" ip") : (""));
       break;
-#endif /* defined (LIBIPT_VERSION >= 0x200)  */
+#endif /* (LIBIPT_VERSION >= 0x200)  */
 
 #if (LIBIPT_VERSION >= 0x201)
     case ppt_cfe:
@@ -3077,7 +3077,7 @@ pt_print_packet (const struct pt_packet *packet)
       gdb_printf (("evd %u: 0x%" PRIx64 ""), packet->payload.evd.type,
                  packet->payload.evd.payload);
       break;
-#endif /* defined (LIBIPT_VERSION >= 0x201)  */
+#endif /* (LIBIPT_VERSION >= 0x201)  */
     }
 }

Markus.
Intel Deutschland GmbH
Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany
Tel: +49 89 991 430, www.intel.de
Managing Directors: Harry Demas, Jeffrey Schneiderman, Yin Chong Sorrell
Chairperson of the Supervisory Board: Nicole Lau
Registered Seat: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


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

* RE: [PATCH] gdb, btrace: support libipt v2.2 events
  2026-04-30 18:11 ` Keith Seitz
  2026-05-01 16:39   ` Tom Tromey
@ 2026-05-04  7:16   ` Metzger, Markus T
  1 sibling, 0 replies; 8+ messages in thread
From: Metzger, Markus T @ 2026-05-04  7:16 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches

Hello Keith,

>On 4/30/26 4:24 AM, Markus Metzger wrote:
>> ---
>>   gdb/btrace.c | 53
>++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 53 insertions(+)
>
>There appears to be no commit message other than the (admittedly
>rather complete) title?
>
>I think a brief NEWS entry is warranted, too.

I extended the former and added the latter.  Will send a v2 so we can review
the NEWS wording.

>Reviewed-By: Keith Seitz <keiths@redhat.com>

Thanks for your review,
Makus.

Intel Deutschland GmbH
Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany
Tel: +49 89 991 430, www.intel.de
Managing Directors: Harry Demas, Jeffrey Schneiderman, Yin Chong Sorrell
Chairperson of the Supervisory Board: Nicole Lau
Registered Seat: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

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

* Re: [PATCH] gdb, btrace: support libipt v2.2 events
  2026-05-04  7:16     ` Metzger, Markus T
@ 2026-05-04 16:08       ` Simon Marchi
  2026-05-05 13:28       ` Tom Tromey
  1 sibling, 0 replies; 8+ messages in thread
From: Simon Marchi @ 2026-05-04 16:08 UTC (permalink / raw)
  To: Metzger, Markus T, Tom Tromey; +Cc: gdb-patches, Keith Seitz

On 5/4/26 3:16 AM, Metzger, Markus T wrote:
> Hello Tom,
> 
>>>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:
>>
>>>> +	    std::string aux_string = std::string (_("trig: trbv = "))
>>>> +	      + hex_string (event.variant.trig.trbv);
>>
>> Normally here we'd wrap the RSH in parens, per the coding standards.
> 
> I'd need to break at the =, then, like this:
> 
> 	    std::string aux_string
> 	      = (std::string (_("trig: trbv = "))
> 		 + hex_string (event.variant.trig.trbv));
> 
> The patch matches the surrounding style, so if you want this fixed, I'd need
> to fix it everywhere in a separate patch.

Just noting that you could also use `string_printf` for this, which
sometimes gives more readable code than using concatenation (and
perhaps one day we'll use {fmt}/std::format, one can dream).

Simon

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

* Re: [PATCH] gdb, btrace: support libipt v2.2 events
  2026-05-04  7:16     ` Metzger, Markus T
  2026-05-04 16:08       ` Simon Marchi
@ 2026-05-05 13:28       ` Tom Tromey
  2026-05-08  5:07         ` Metzger, Markus T
  1 sibling, 1 reply; 8+ messages in thread
From: Tom Tromey @ 2026-05-05 13:28 UTC (permalink / raw)
  To: Metzger, Markus T; +Cc: Tom Tromey, gdb-patches, Keith Seitz

> I'd need to break at the =, then, like this:

> 	    std::string aux_string
> 	      = (std::string (_("trig: trbv = "))
> 		 + hex_string (event.variant.trig.trbv));

> The patch matches the surrounding style, so if you want this fixed, I'd need
> to fix it everywhere in a separate patch.

In gdb it's pretty normal for some code to be weird or wrong.
For example tdep files are less reviewed than others.

However the presence of some code that isn't standards-conformant
doesn't mean that new code should be.  You see this a lot with things
like NULL/nullptr -- using nullptr in new code doesn't imply that all
the old code has to be updated at the same time.

So IMO new code should follow the relevant coding standards, even if
other code in the same function does not.

thanks,
Tom

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

* RE: [PATCH] gdb, btrace: support libipt v2.2 events
  2026-05-05 13:28       ` Tom Tromey
@ 2026-05-08  5:07         ` Metzger, Markus T
  0 siblings, 0 replies; 8+ messages in thread
From: Metzger, Markus T @ 2026-05-08  5:07 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

Hello Tom,

>> I'd need to break at the =, then, like this:
>
>> 	    std::string aux_string
>> 	      = (std::string (_("trig: trbv = "))
>> 		 + hex_string (event.variant.trig.trbv));
>
>> The patch matches the surrounding style, so if you want this fixed, I'd need
>> to fix it everywhere in a separate patch.
>
>In gdb it's pretty normal for some code to be weird or wrong.
>For example tdep files are less reviewed than others.
>
>However the presence of some code that isn't standards-conformant
>doesn't mean that new code should be.  You see this a lot with things
>like NULL/nullptr -- using nullptr in new code doesn't imply that all
>the old code has to be updated at the same time.
>
>So IMO new code should follow the relevant coding standards, even if
>other code in the same function does not.

I changed this in v5 [1].

[1]: https://sourceware.org/pipermail/gdb-patches/2026-May/227179.html

Regards,
Markus.
Intel Deutschland GmbH
Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany
Tel: +49 89 991 430, www.intel.de
Managing Directors: Harry Demas, Jeffrey Schneiderman, Yin Chong Sorrell
Chairperson of the Supervisory Board: Nicole Lau
Registered Seat: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


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

end of thread, other threads:[~2026-05-08  5:08 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-04-30 11:24 [PATCH] gdb, btrace: support libipt v2.2 events Markus Metzger
2026-04-30 18:11 ` Keith Seitz
2026-05-01 16:39   ` Tom Tromey
2026-05-04  7:16     ` Metzger, Markus T
2026-05-04 16:08       ` Simon Marchi
2026-05-05 13:28       ` Tom Tromey
2026-05-08  5:07         ` Metzger, Markus T
2026-05-04  7:16   ` Metzger, Markus T

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox