* [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).