* [PATCH] Update btrace data in maintenance btrace commands
@ 2021-04-12 14:01 Stephen Röttger
2021-05-25 13:06 ` [PING] " Stephen Röttger
2021-05-29 2:13 ` Simon Marchi
0 siblings, 2 replies; 7+ messages in thread
From: Stephen Röttger @ 2021-04-12 14:01 UTC (permalink / raw)
To: gdb-patches; +Cc: Stephen Roettger
From: Stephen Roettger <sroettger@google.com>
When calling `maintenance btrace packet-history` or `maintenance info btrace`,
the commands wouldn't show any data unless it was updated previously by some
other means, for example by running `info record`.
To fix this, use the require_btrace function from record-brace.h which
will update the data before returning the btrace_thread_info pointer.
Here's an example of the issue:
```
$ gdb -nx /usr/bin/ls
(gdb) break malloc
Breakpoint 1 at 0x46c8
(gdb) run
Starting program: /usr/bin/ls
Breakpoint 1, 0x00007ffff7fec240 in malloc () from /lib64/ld-linux-x86-64.so.2
(gdb) record btrace bts
(gdb) c
Continuing.
Breakpoint 1, 0x00007ffff7fec240 in malloc () from /lib64/ld-linux-x86-64.so.2
(gdb) maintenance btrace packet-history
No trace.
(gdb) maintenance info btrace
Format: Branch Trace Store.
Aborted
```
gdb/ChangeLog:
* btrace.c (maint_btrace_packet_history_cmd): use require_btrace
(maint_info_btrace_cmd): use require_btrace
* record-btrace.c (struct btrace_thread_info): mark non-static
* record-btrace.h (require_btrace): export function
---
gdb/btrace.c | 13 ++-----------
gdb/record-btrace.c | 2 +-
gdb/record-btrace.h | 7 +++++++
3 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/gdb/btrace.c b/gdb/btrace.c
index c697f37f46c..177e818f0cf 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -3231,12 +3231,8 @@ maint_btrace_packet_history_cmd (const char *arg, int from_tty)
struct btrace_thread_info *btinfo;
unsigned int size, begin, end, from, to;
- thread_info *tp = find_thread_ptid (current_inferior (), inferior_ptid);
- if (tp == NULL)
- error (_("No thread."));
-
+ btinfo = require_btrace ();
size = 10;
- btinfo = &tp->btrace;
btrace_maint_update_packets (btinfo, &begin, &end, &from, &to);
if (begin == end)
@@ -3372,12 +3368,7 @@ maint_info_btrace_cmd (const char *args, int from_tty)
if (args != NULL && *args != 0)
error (_("Invalid argument."));
- if (inferior_ptid == null_ptid)
- error (_("No thread."));
-
- thread_info *tp = inferior_thread ();
-
- btinfo = &tp->btrace;
+ btinfo = require_btrace ();
conf = btrace_conf (btinfo);
if (conf == NULL)
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index b7b3c91f85d..79a6e47a4a6 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -269,7 +269,7 @@ require_btrace_thread (void)
Throws an error if there is no thread or no trace. This function never
returns NULL. */
-static struct btrace_thread_info *
+struct btrace_thread_info *
require_btrace (void)
{
struct thread_info *tp;
diff --git a/gdb/record-btrace.h b/gdb/record-btrace.h
index 8e7bbd90e60..b56e720e5a5 100644
--- a/gdb/record-btrace.h
+++ b/gdb/record-btrace.h
@@ -29,4 +29,11 @@ extern void record_btrace_push_target (void);
NULL if the cpu was configured as auto. */
extern const struct btrace_cpu *record_btrace_get_cpu (void);
+/* Update the branch trace for the current thread and return a pointer to its
+ branch trace information struct.
+
+ Throws an error if there is no thread or no trace. This function never
+ returns NULL. */
+extern struct btrace_thread_info * require_btrace (void);
+
#endif /* RECORD_BTRACE_H */
--
2.31.1.295.g9ea45b61b8-goog
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PING] [PATCH] Update btrace data in maintenance btrace commands
2021-04-12 14:01 [PATCH] Update btrace data in maintenance btrace commands Stephen Röttger
@ 2021-05-25 13:06 ` Stephen Röttger
2021-05-29 2:13 ` Simon Marchi
1 sibling, 0 replies; 7+ messages in thread
From: Stephen Röttger @ 2021-05-25 13:06 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 3720 bytes --]
On Mon, Apr 12, 2021 at 4:01 PM Stephen Röttger <sroettger@google.com>
wrote:
> From: Stephen Roettger <sroettger@google.com>
>
> When calling `maintenance btrace packet-history` or `maintenance info
> btrace`,
> the commands wouldn't show any data unless it was updated previously by
> some
> other means, for example by running `info record`.
> To fix this, use the require_btrace function from record-brace.h which
> will update the data before returning the btrace_thread_info pointer.
>
> Here's an example of the issue:
> ```
> $ gdb -nx /usr/bin/ls
> (gdb) break malloc
> Breakpoint 1 at 0x46c8
> (gdb) run
> Starting program: /usr/bin/ls
>
> Breakpoint 1, 0x00007ffff7fec240 in malloc () from
> /lib64/ld-linux-x86-64.so.2
> (gdb) record btrace bts
> (gdb) c
> Continuing.
>
> Breakpoint 1, 0x00007ffff7fec240 in malloc () from
> /lib64/ld-linux-x86-64.so.2
> (gdb) maintenance btrace packet-history
> No trace.
> (gdb) maintenance info btrace
> Format: Branch Trace Store.
> Aborted
> ```
>
> gdb/ChangeLog:
>
> * btrace.c (maint_btrace_packet_history_cmd): use require_btrace
> (maint_info_btrace_cmd): use require_btrace
> * record-btrace.c (struct btrace_thread_info): mark non-static
> * record-btrace.h (require_btrace): export function
>
> ---
> gdb/btrace.c | 13 ++-----------
> gdb/record-btrace.c | 2 +-
> gdb/record-btrace.h | 7 +++++++
> 3 files changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/gdb/btrace.c b/gdb/btrace.c
> index c697f37f46c..177e818f0cf 100644
> --- a/gdb/btrace.c
> +++ b/gdb/btrace.c
> @@ -3231,12 +3231,8 @@ maint_btrace_packet_history_cmd (const char *arg,
> int from_tty)
> struct btrace_thread_info *btinfo;
> unsigned int size, begin, end, from, to;
>
> - thread_info *tp = find_thread_ptid (current_inferior (), inferior_ptid);
> - if (tp == NULL)
> - error (_("No thread."));
> -
> + btinfo = require_btrace ();
> size = 10;
> - btinfo = &tp->btrace;
>
> btrace_maint_update_packets (btinfo, &begin, &end, &from, &to);
> if (begin == end)
> @@ -3372,12 +3368,7 @@ maint_info_btrace_cmd (const char *args, int
> from_tty)
> if (args != NULL && *args != 0)
> error (_("Invalid argument."));
>
> - if (inferior_ptid == null_ptid)
> - error (_("No thread."));
> -
> - thread_info *tp = inferior_thread ();
> -
> - btinfo = &tp->btrace;
> + btinfo = require_btrace ();
>
> conf = btrace_conf (btinfo);
> if (conf == NULL)
> diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
> index b7b3c91f85d..79a6e47a4a6 100644
> --- a/gdb/record-btrace.c
> +++ b/gdb/record-btrace.c
> @@ -269,7 +269,7 @@ require_btrace_thread (void)
> Throws an error if there is no thread or no trace. This function never
> returns NULL. */
>
> -static struct btrace_thread_info *
> +struct btrace_thread_info *
> require_btrace (void)
> {
> struct thread_info *tp;
> diff --git a/gdb/record-btrace.h b/gdb/record-btrace.h
> index 8e7bbd90e60..b56e720e5a5 100644
> --- a/gdb/record-btrace.h
> +++ b/gdb/record-btrace.h
> @@ -29,4 +29,11 @@ extern void record_btrace_push_target (void);
> NULL if the cpu was configured as auto. */
> extern const struct btrace_cpu *record_btrace_get_cpu (void);
>
> +/* Update the branch trace for the current thread and return a pointer to
> its
> + branch trace information struct.
> +
> + Throws an error if there is no thread or no trace. This function never
> + returns NULL. */
> +extern struct btrace_thread_info * require_btrace (void);
> +
> #endif /* RECORD_BTRACE_H */
> --
> 2.31.1.295.g9ea45b61b8-goog
>
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4002 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Update btrace data in maintenance btrace commands
2021-04-12 14:01 [PATCH] Update btrace data in maintenance btrace commands Stephen Röttger
2021-05-25 13:06 ` [PING] " Stephen Röttger
@ 2021-05-29 2:13 ` Simon Marchi
2021-06-08 14:05 ` Metzger, Markus T
1 sibling, 1 reply; 7+ messages in thread
From: Simon Marchi @ 2021-05-29 2:13 UTC (permalink / raw)
To: Stephen Röttger, gdb-patches
On 2021-04-12 10:01 a.m., Stephen Röttger via Gdb-patches wrote:
> diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
> index b7b3c91f85d..79a6e47a4a6 100644
> --- a/gdb/record-btrace.c
> +++ b/gdb/record-btrace.c
> @@ -269,7 +269,7 @@ require_btrace_thread (void)
> Throws an error if there is no thread or no trace. This function never
> returns NULL. */
>
> -static struct btrace_thread_info *
> +struct btrace_thread_info *
> require_btrace (void)
Since the doc is now in the header, write:
/* See record-btrace.h. */
here.
I'd LGTM this patch, but I'd like Markus (the btrace specialist) to
give it a look (I think he's away, it will be in a few weeks).
Simon
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] Update btrace data in maintenance btrace commands
2021-05-29 2:13 ` Simon Marchi
@ 2021-06-08 14:05 ` Metzger, Markus T
2021-06-09 9:41 ` Stephen Röttger
0 siblings, 1 reply; 7+ messages in thread
From: Metzger, Markus T @ 2021-06-08 14:05 UTC (permalink / raw)
To: Stephen Röttger; +Cc: gdb-patches, Simon Marchi
Hello Stephen,
>On 2021-04-12 10:01 a.m., Stephen Röttger via Gdb-patches wrote:
>> diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
>> index b7b3c91f85d..79a6e47a4a6 100644
>> --- a/gdb/record-btrace.c
>> +++ b/gdb/record-btrace.c
>> @@ -269,7 +269,7 @@ require_btrace_thread (void)
>> Throws an error if there is no thread or no trace. This function never
>> returns NULL. */
>>
>> -static struct btrace_thread_info *
>> +struct btrace_thread_info *
>> require_btrace (void)
>
>Since the doc is now in the header, write:
>
>/* See record-btrace.h. */
>
>here.
>
>I'd LGTM this patch, but I'd like Markus (the btrace specialist) to
>give it a look (I think he's away, it will be in a few weeks).
Those maintenance commands are not intended for users. You'd really only
need them to debug issues with PT decode or recording. My main use-case is
to clear the history, then turn on debugging and logging, and re-decode the
trace. Re-fetching shouldn't hurt but isn't necessary.
Your patch LGTM (with the comment fixed) but I wonder whether there's
anything to fix, at all. If you end up needing to use those maint commands,
something else may be wrong.
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Update btrace data in maintenance btrace commands
2021-06-08 14:05 ` Metzger, Markus T
@ 2021-06-09 9:41 ` Stephen Röttger
2021-08-03 8:49 ` Stephen Röttger
0 siblings, 1 reply; 7+ messages in thread
From: Stephen Röttger @ 2021-06-09 9:41 UTC (permalink / raw)
To: Metzger, Markus T; +Cc: gdb-patches, Simon Marchi
[-- Attachment #1: Type: text/plain, Size: 2209 bytes --]
Ah that makes sense. Thanks Markus and Simon.
You're right, in the end I didn't rely on these commands. Though when I started
playing with btrace, it did make me think btrace was broken on my machine and
it took me a bit to figure out what's going on. So I think this change can
still be valuable to save others the headache :).
Let me know what you think, if you think it's valuable I'll send a v2 with the
proposed change.
Cheers,
Stephen
On Tue, Jun 8, 2021 at 4:05 PM Metzger, Markus T
<markus.t.metzger@intel.com> wrote:
>
> Hello Stephen,
>
> >On 2021-04-12 10:01 a.m., Stephen Röttger via Gdb-patches wrote:
> >> diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
> >> index b7b3c91f85d..79a6e47a4a6 100644
> >> --- a/gdb/record-btrace.c
> >> +++ b/gdb/record-btrace.c
> >> @@ -269,7 +269,7 @@ require_btrace_thread (void)
> >> Throws an error if there is no thread or no trace. This function never
> >> returns NULL. */
> >>
> >> -static struct btrace_thread_info *
> >> +struct btrace_thread_info *
> >> require_btrace (void)
> >
> >Since the doc is now in the header, write:
> >
> >/* See record-btrace.h. */
> >
> >here.
> >
> >I'd LGTM this patch, but I'd like Markus (the btrace specialist) to
> >give it a look (I think he's away, it will be in a few weeks).
>
> Those maintenance commands are not intended for users. You'd really only
> need them to debug issues with PT decode or recording. My main use-case is
> to clear the history, then turn on debugging and logging, and re-decode the
> trace. Re-fetching shouldn't hurt but isn't necessary.
>
> Your patch LGTM (with the comment fixed) but I wonder whether there's
> anything to fix, at all. If you end up needing to use those maint commands,
> something else may be wrong.
>
> 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
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4002 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Update btrace data in maintenance btrace commands
2021-06-09 9:41 ` Stephen Röttger
@ 2021-08-03 8:49 ` Stephen Röttger
2021-08-03 10:05 ` Metzger, Markus T
0 siblings, 1 reply; 7+ messages in thread
From: Stephen Röttger @ 2021-08-03 8:49 UTC (permalink / raw)
To: Metzger, Markus T; +Cc: gdb-patches, Simon Marchi
[-- Attachment #1: Type: text/plain, Size: 2620 bytes --]
> Let me know what you think, if you think it's valuable I'll send a v2
with the
proposed change.
Hey, just checking if you would like this patch, should I make the proposed
change?
On Wed, Jun 9, 2021 at 11:41 AM Stephen Röttger <sroettger@google.com>
wrote:
> Ah that makes sense. Thanks Markus and Simon.
>
> You're right, in the end I didn't rely on these commands. Though when I
> started
> playing with btrace, it did make me think btrace was broken on my machine
> and
> it took me a bit to figure out what's going on. So I think this change can
> still be valuable to save others the headache :).
> Let me know what you think, if you think it's valuable I'll send a v2 with
> the
> proposed change.
>
> Cheers,
> Stephen
>
> On Tue, Jun 8, 2021 at 4:05 PM Metzger, Markus T
> <markus.t.metzger@intel.com> wrote:
> >
> > Hello Stephen,
> >
> > >On 2021-04-12 10:01 a.m., Stephen Röttger via Gdb-patches wrote:
> > >> diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
> > >> index b7b3c91f85d..79a6e47a4a6 100644
> > >> --- a/gdb/record-btrace.c
> > >> +++ b/gdb/record-btrace.c
> > >> @@ -269,7 +269,7 @@ require_btrace_thread (void)
> > >> Throws an error if there is no thread or no trace. This function
> never
> > >> returns NULL. */
> > >>
> > >> -static struct btrace_thread_info *
> > >> +struct btrace_thread_info *
> > >> require_btrace (void)
> > >
> > >Since the doc is now in the header, write:
> > >
> > >/* See record-btrace.h. */
> > >
> > >here.
> > >
> > >I'd LGTM this patch, but I'd like Markus (the btrace specialist) to
> > >give it a look (I think he's away, it will be in a few weeks).
> >
> > Those maintenance commands are not intended for users. You'd really only
> > need them to debug issues with PT decode or recording. My main use-case
> is
> > to clear the history, then turn on debugging and logging, and re-decode
> the
> > trace. Re-fetching shouldn't hurt but isn't necessary.
> >
> > Your patch LGTM (with the comment fixed) but I wonder whether there's
> > anything to fix, at all. If you end up needing to use those maint
> commands,
> > something else may be wrong.
> >
> > 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
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4002 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] Update btrace data in maintenance btrace commands
2021-08-03 8:49 ` Stephen Röttger
@ 2021-08-03 10:05 ` Metzger, Markus T
0 siblings, 0 replies; 7+ messages in thread
From: Metzger, Markus T @ 2021-08-03 10:05 UTC (permalink / raw)
To: Stephen Röttger; +Cc: gdb-patches, Simon Marchi
Hello Stephen,
I’m OK with the patch if you want to send an updated version.
Thanks,
Markus.
From: Stephen Röttger <sroettger@google.com>
Sent: Tuesday, August 3, 2021 10:50 AM
To: Metzger, Markus T <markus.t.metzger@intel.com>
Cc: gdb-patches@sourceware.org; Simon Marchi <simon.marchi@polymtl.ca>
Subject: Re: [PATCH] Update btrace data in maintenance btrace commands
> Let me know what you think, if you think it's valuable I'll send a v2 with the
proposed change.
Hey, just checking if you would like this patch, should I make the proposed change?
On Wed, Jun 9, 2021 at 11:41 AM Stephen Röttger <sroettger@google.com<mailto:sroettger@google.com>> wrote:
Ah that makes sense. Thanks Markus and Simon.
You're right, in the end I didn't rely on these commands. Though when I started
playing with btrace, it did make me think btrace was broken on my machine and
it took me a bit to figure out what's going on. So I think this change can
still be valuable to save others the headache :).
Let me know what you think, if you think it's valuable I'll send a v2 with the
proposed change.
Cheers,
Stephen
On Tue, Jun 8, 2021 at 4:05 PM Metzger, Markus T
<markus.t.metzger@intel.com<mailto:markus.t.metzger@intel.com>> wrote:
>
> Hello Stephen,
>
> >On 2021-04-12 10:01 a.m., Stephen Röttger via Gdb-patches wrote:
> >> diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
> >> index b7b3c91f85d..79a6e47a4a6 100644
> >> --- a/gdb/record-btrace.c
> >> +++ b/gdb/record-btrace.c
> >> @@ -269,7 +269,7 @@ require_btrace_thread (void)
> >> Throws an error if there is no thread or no trace. This function never
> >> returns NULL. */
> >>
> >> -static struct btrace_thread_info *
> >> +struct btrace_thread_info *
> >> require_btrace (void)
> >
> >Since the doc is now in the header, write:
> >
> >/* See record-btrace.h. */
> >
> >here.
> >
> >I'd LGTM this patch, but I'd like Markus (the btrace specialist) to
> >give it a look (I think he's away, it will be in a few weeks).
>
> Those maintenance commands are not intended for users. You'd really only
> need them to debug issues with PT decode or recording. My main use-case is
> to clear the history, then turn on debugging and logging, and re-decode the
> trace. Re-fetching shouldn't hurt but isn't necessary.
>
> Your patch LGTM (with the comment fixed) but I wonder whether there's
> anything to fix, at all. If you end up needing to use those maint commands,
> something else may be wrong.
>
> 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> <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
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-08-03 10:06 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-12 14:01 [PATCH] Update btrace data in maintenance btrace commands Stephen Röttger
2021-05-25 13:06 ` [PING] " Stephen Röttger
2021-05-29 2:13 ` Simon Marchi
2021-06-08 14:05 ` Metzger, Markus T
2021-06-09 9:41 ` Stephen Röttger
2021-08-03 8:49 ` Stephen Röttger
2021-08-03 10:05 ` 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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).