[PATCH] btrace: diagnose "record btrace pt" without libipt

[PATCH] btrace: diagnose "record btrace pt" without libipt

[Markus Metzger]

If GDB has been configured without libipt support, i.e. HAVE_LIBIPT is
undefined, and is running on a system that supports Intel(R) Processor Trace,
GDB will run into an internal error when trying to decode the trace.

    (gdb) record btrace
    (gdb) s
    usage (name=0x7fffffffe954 "fib-64")
        at src/fib.c:12
    12          fprintf(stderr, "usage: %s <num>\n", name);
    (gdb) info record
    Active record target: record-btrace
    Recording format: Intel(R) Processor Trace.
    Buffer size: 16kB.
    gdb/btrace.c:971: internal-error: Unexpected branch trace format.
    A problem internal to GDB has been detected,
    further debugging may prove unreliable.
    Quit this debugging session? (y or n)

This requires a system with Linux kernel 4.1 or later running on a 5th
Generation Intel Core processor or later.

When trying to enable branch tracing, in addition to checking the target
support for the requested branch tracing format, also check whether GDB
supports. it.

2015-11-20  Markus Metzger  <markus.t.metzger@intel.com>

gdb/
	* btrace.c (btrace_enable): Check whether HAVE_LIBIPT is defined.

testsuite/
	* gdb.exp (skip_btrace_pt_tests): Check for a "GDB does not support"
	error.
---
 gdb/btrace.c              | 5 +++++
 gdb/testsuite/lib/gdb.exp | 3 +++
 2 files changed, 8 insertions(+)

diff --git a/gdb/btrace.c b/gdb/btrace.c
index 2bf7177..35431cb 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -1035,6 +1035,11 @@ btrace_enable (struct thread_info *tp, const struct btrace_config *conf)
   if (tp->btrace.target != NULL)
     return;
 
+#if !defined (HAVE_LIBIPT)
+  if (conf->format == BTRACE_FORMAT_PT)
+    error (_("GDB does not support Intel(R) Processor Trace."));
+#endif /* !defined (HAVE_LIBIPT) */
+
   if (!target_supports_btrace (conf->format))
     error (_("Target does not support branch tracing."));
 
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index a420181..90e8644 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2826,6 +2826,9 @@ gdb_caching_proc skip_btrace_pt_tests {
         -re "Could not enable branch tracing.*\r\n$gdb_prompt $" {
             set skip_btrace_tests 1
         }
+        -re "GDB does not support.*\r\n$gdb_prompt $" {
+            set skip_btrace_tests 1
+        }
         -re "^record btrace pt\r\n$gdb_prompt $" {
             set skip_btrace_tests 0
         }
-- 
1.8.3.1

Re: [PATCH] btrace: diagnose "record btrace pt" without libipt

[Pedro Alves]

On 11/20/2015 09:17 AM, Markus Metzger wrote:

> support for the requested branch tracing format, also check whether GDB
> supports. it.

( Nit: spurious '.'. )

> 
> 2015-11-20  Markus Metzger  <markus.t.metzger@intel.com>
> 
> gdb/
> 	* btrace.c (btrace_enable): Check whether HAVE_LIBIPT is defined.
> 
> testsuite/
> 	* gdb.exp (skip_btrace_pt_tests): Check for a "GDB does not support"
> 	error.

LGTM.

Thanks,
Pedro Alves

Re: [PATCH] btrace: diagnose "record btrace pt" without libipt

[Pedro Alves]

On 11/20/2015 09:17 AM, Markus Metzger wrote:
> If GDB has been configured without libipt support, i.e. HAVE_LIBIPT is
> undefined, and is running on a system that supports Intel(R) Processor Trace,
> GDB will run into an internal error when trying to decode the trace.
> 
>     (gdb) record btrace
>     (gdb) s
>     usage (name=0x7fffffffe954 "fib-64")
>         at src/fib.c:12
>     12          fprintf(stderr, "usage: %s <num>\n", name);
>     (gdb) info record
>     Active record target: record-btrace
>     Recording format: Intel(R) Processor Trace.
>     Buffer size: 16kB.
>     gdb/btrace.c:971: internal-error: Unexpected branch trace format.
>     A problem internal to GDB has been detected,
>     further debugging may prove unreliable.
>     Quit this debugging session? (y or n)
> 
> This requires a system with Linux kernel 4.1 or later running on a 5th
> Generation Intel Core processor or later.
> 
> When trying to enable branch tracing, in addition to checking the target
> support for the requested branch tracing format, also check whether GDB
> supports. it.

BTW, this made me wonder what happens if you're remote debugging with
gdbserver, and then:

 #1 - enable btrace pt
 #2 - disconnect with "disconnect"
 #3 - restart gdb
 #4 - reconnect to gdbserver

- Does gdb sync the "btrace-enabled" state with the server?  Or does it get out
  of sync and confused?

- What if btrace pt was enabled on the inferior, and the gdb that reconnects
  in #4 above is compiled _without_ libipt?  Will "info record" still crash?

Thanks,
Pedro Alves

RE: [PATCH] btrace: diagnose "record btrace pt" without libipt

[Metzger, Markus T]

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Friday, November 20, 2015 12:36 PM
> To: Metzger, Markus T
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH] btrace: diagnose "record btrace pt" without libipt
> 
> On 11/20/2015 09:17 AM, Markus Metzger wrote:
> > If GDB has been configured without libipt support, i.e. HAVE_LIBIPT is
> > undefined, and is running on a system that supports Intel(R) Processor
> Trace,
> > GDB will run into an internal error when trying to decode the trace.
> >
> >     (gdb) record btrace
> >     (gdb) s
> >     usage (name=0x7fffffffe954 "fib-64")
> >         at src/fib.c:12
> >     12          fprintf(stderr, "usage: %s <num>\n", name);
> >     (gdb) info record
> >     Active record target: record-btrace
> >     Recording format: Intel(R) Processor Trace.
> >     Buffer size: 16kB.
> >     gdb/btrace.c:971: internal-error: Unexpected branch trace format.
> >     A problem internal to GDB has been detected,
> >     further debugging may prove unreliable.
> >     Quit this debugging session? (y or n)
> >
> > This requires a system with Linux kernel 4.1 or later running on a 5th
> > Generation Intel Core processor or later.
> >
> > When trying to enable branch tracing, in addition to checking the target
> > support for the requested branch tracing format, also check whether GDB
> > supports. it.
> 
> BTW, this made me wonder what happens if you're remote debugging with
> gdbserver, and then:
> 
>  #1 - enable btrace pt
>  #2 - disconnect with "disconnect"
>  #3 - restart gdb
>  #4 - reconnect to gdbserver
> 
> - Does gdb sync the "btrace-enabled" state with the server?  Or does it get
> out
>   of sync and confused?

The record target is popped off the target stack on disconnect.  This disables
branch tracing.  The below log is for BTS but the logic is the same for PT.

	(gdb) rec b
	[record-btrace] open
	[record-btrace] open
	[btrace] enable thread 1 (Thread 135520)
	Sending packet: $Qbtrace-conf:bts:size=0x10000#e3...Packet received: OK
	Sending packet: $Qbtrace:bts#45...Packet received: OK
	Sending packet: $qXfer:btrace-conf:read::0,fff#5c...Packet received: l<!DOCTYPE btrace-conf SYSTEM "btrace-	conf.dtd">\n<btrace-conf version="1.0">\n<bts size="0x10000" />\n</btrace-conf>\n\000
	[btrace] compute ftrace
	[record-btrace] attach thread observer
	(gdb) info rec
	Active record target: record-btrace
	[record-btrace] info
	Recording format: Branch Trace Store.
	Buffer size: 64kB.
	[btrace] fetch thread 1 (Thread 135520)
	Sending packet: $qXfer:btrace:read:delta:0,fff#93...Packet received: l<!DOCTYPE btrace SYSTEM "btrace.dtd">\n<btrace 	version="1.0">\n<block begin="0x0" end="0x7ffff7ddc423"/>\n</btrace>\n\000
	Recorded 0 instructions in 0 functions (0 gaps) for thread 1 (Thread 135520).
	(gdb) disconnect 
	Sending packet: $qTStatus#49...Packet received: 	T0;tnotrun:0;tframes:0;tcreated:0;tfree:500000;tsize:500000;circular:0;disconn:0;starttime:0;stoptime:0;username:;notes::
	record: disconnect record-btrace
	record: stop record-btrace
	[record-btrace] stop recording
	[record-btrace] detach thread observer
	[btrace] disable thread 1 (Thread 135520)
	Sending packet: $Qbtrace:off#37...Packet received: OK
	[btrace] clear thread 1 (Thread 135520)
	record: unpush record-btrace
	Ending remote debugging.
	[btrace] free objfile
	[btrace] free objfile
	(gdb)

After reconnecting, you need to enable btrace again.

Regards,
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, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH] btrace: diagnose "record btrace pt" without libipt

[Pedro Alves]

On 11/20/2015 12:11 PM, Metzger, Markus T wrote:

> The record target is popped off the target stack on disconnect.  This disables
> branch tracing.  The below log is for BTS but the logic is the same for PT.
> 	Sending packet: $Qbtrace:off#37...Packet received: OK

> 
> After reconnecting, you need to enable btrace again.

I see.  But then it sounds like we'll have the problem if the connection
drops unexpectedly -- gdb won't be able to tell the server to
disable Qbtrace.  I guess the easiest way to emulate is kill gdb:

 #1 - enable btrace pt
 #2 - connection is terminated unexpectedly / kill gdb
 #3 - reconnect to gdbserver

a) Does the new gdb get out of sync and confused?

b) What if btrace pt was enabled on the inferior, and the gdb that
   reconnects in #3 above is compiled _without_ libipt?
   Will "info record" still crash?  What I'm thinking is that
   a similar format check may be necessary around this code path,
   not only at PT enable time.

Thanks,
Pedro Alves

RE: [PATCH] btrace: diagnose "record btrace pt" without libipt

[Metzger, Markus T]

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Friday, November 20, 2015 2:12 PM
> To: Metzger, Markus T
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH] btrace: diagnose "record btrace pt" without libipt
> 
> On 11/20/2015 12:11 PM, Metzger, Markus T wrote:
> 
> > The record target is popped off the target stack on disconnect.  This
> disables
> > branch tracing.  The below log is for BTS but the logic is the same for PT.
> > 	Sending packet: $Qbtrace:off#37...Packet received: OK
> 
> >
> > After reconnecting, you need to enable btrace again.
> 
> I see.  But then it sounds like we'll have the problem if the connection
> drops unexpectedly -- gdb won't be able to tell the server to
> disable Qbtrace.  I guess the easiest way to emulate is kill gdb:
> 
>  #1 - enable btrace pt
>  #2 - connection is terminated unexpectedly / kill gdb
>  #3 - reconnect to gdbserver
> 
> a) Does the new gdb get out of sync and confused?

Yes.

GDB doesn't recognize that the remote agent is already tracing.
This also prevents enabling branch tracing after reconnect.

    (gdb) info rec
    No record target is currently active.
    (gdb) rec b
    [record-btrace] open
    [record-btrace] open
    [btrace] enable thread 1 (Thread 141484)
    Sending packet: $Qbtrace-conf:bts:size=0x10000#e3...Packet received: OK
    Sending packet: $Qbtrace:bts#45...Packet received: E.Btrace already enabled.
    Could not enable branch tracing for Thread 141484: Btrace already enabled.
    (gdb) info rec
    No record target is currently active.
    (gdb)


Thanks for pointing this out.  Let's try to fix it...

Handling the "E.Btrace already enabled" error in remote.c shouldn't be too hard.
This would at least allow another "record btrace" after reconnect - and it should
keep the trace logs.  This takes the same code path as a new enable so the check
in this patch should suffice.

A non-PT enabled GDB would try to fall back to BTS, though, so we could either
implicitly disable PT in the target to allow it or give an error, which again leaves
branch tracing unusable in this GDB session.  But it would keep the trace logs if
the user accidentally chose the wrong GDB for reconnecting.

It would be nice if GDB could detect that record btrace is already enabled and push
the record-btrace target automatically.  I guess this requires some indication about
the record status from gdbserver.  Is there some other target that does this automatic
push on (re-)connect that I could use as reference?

This still leaves the question how GDB should behave if it doesn't support the tracing
format that's already enabled in the GDBserver it just connected to.

Regards,
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, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH] btrace: diagnose "record btrace pt" without libipt

[Pedro Alves]

On 11/20/2015 01:59 PM, Metzger, Markus T wrote:

> Thanks for pointing this out.  Let's try to fix it...
> 
> Handling the "E.Btrace already enabled" error in remote.c shouldn't be too hard.
> This would at least allow another "record btrace" after reconnect - and it should
> keep the trace logs.  This takes the same code path as a new enable so the check
> in this patch should suffice.
> 
> A non-PT enabled GDB would try to fall back to BTS, though, so we could either
> implicitly disable PT in the target to allow it or give an error, which again leaves
> branch tracing unusable in this GDB session.  But it would keep the trace logs if
> the user accidentally chose the wrong GDB for reconnecting.
> 
> It would be nice if GDB could detect that record btrace is already enabled and push
> the record-btrace target automatically.  I guess this requires some indication about
> the record status from gdbserver.  Is there some other target that does this automatic
> push on (re-)connect that I could use as reference?

E.g., linux-thread-db.c pushes itself from a new_objfile observer (thread_db_new_objfile),
and  the spu multiarch target pushes itself from an inferior_created observer
(spu_multiarch_inferior_created).

> 
> This still leaves the question how GDB should behave if it doesn't support the tracing
> format that's already enabled in the GDBserver it just connected to.
> 

I guess warn/error when you try to view/consume it, but still let you disable
the tracing?

Thanks,
Pedro Alves

RE: [PATCH] btrace: diagnose "record btrace pt" without libipt

[Metzger, Markus T]

-----Original Message-----
From: Pedro Alves [mailto:palves@redhat.com] 
Sent: Wednesday, November 25, 2015 5:38 PM
To: Metzger, Markus T <markus.t.metzger@intel.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH] btrace: diagnose "record btrace pt" without libipt


> It would be nice if GDB could detect that record btrace is already 
> enabled and push the record-btrace target automatically.  I guess this 
> requires some indication about the record status from gdbserver.  Is 
> there some other target that does this automatic push on (re-)connect that I could use as reference?

E.g., linux-thread-db.c pushes itself from a new_objfile observer (thread_db_new_objfile), and  the spu multiarch target pushes itself from an inferior_created observer (spu_multiarch_inferior_created).

Thanks.  I'll look into those examples.  It'll take a bit but I don't think this is urgent.


Are you OK to push the patch to avoid the internal error and handle the re-connect
case in a separate series?

That patch might make sense for 7.10.1, as well.  OK to push onto the 7.10 branch?

Regards,
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, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH] btrace: diagnose "record btrace pt" without libipt

[Pedro Alves]

On 11/26/2015 07:12 AM, Metzger, Markus T wrote:
> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com] 
> Sent: Wednesday, November 25, 2015 5:38 PM
> To: Metzger, Markus T <markus.t.metzger@intel.com>
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH] btrace: diagnose "record btrace pt" without libipt
> 
> 
>> It would be nice if GDB could detect that record btrace is already 
>> enabled and push the record-btrace target automatically.  I guess this 
>> requires some indication about the record status from gdbserver.  Is 
>> there some other target that does this automatic push on (re-)connect that I could use as reference?
> 
> E.g., linux-thread-db.c pushes itself from a new_objfile observer (thread_db_new_objfile), and  the spu multiarch target pushes itself from an inferior_created observer (spu_multiarch_inferior_created).
> 
> Thanks.  I'll look into those examples.  It'll take a bit but I don't think this is urgent.

Certainly not urgent.

> 
> 
> Are you OK to push the patch to avoid the internal error and handle the re-connect
> case in a separate series?

Yes.

> 
> That patch might make sense for 7.10.1, as well.  OK to push onto the 7.10 branch?

OK.  In that case, as per standard procedure, please file a PR and
list it the 7.10 release wiki page.

Thanks,
Pedro Alves