[PROTOTYPE] Make "info breakpoints" show breakpoint's specs (Re: [PATCH] gdb: Don't skip prologue for explicit line breakpoints in assembler)

Message ID 85fe230e-ab67-0552-fe71-81f9f3762dd2@redhat.com
State New, archived
Headers

Commit Message

Pedro Alves June 24, 2019, 7:54 p.m. UTC
  On 6/24/19 8:16 PM, Pedro Alves wrote:
> 
> I've thought for a few years that "info breakpoints" should show
> BOTH the canonical spec behind each breakpoint, and the actual
> location(s) where the breakpoint is inserted.  It wouldn't
> be that hard, even.  I cooked up a prototype patch for that.
> I'll post it as a follow up.

Here's what I meant.

Here are for example 3 breakpoints that I set while debugging gdb:

 (top-gdb) info breakpoints
 1       breakpoint     keep y   0x0000000000575127 in internal_error(char const*, int, char const*, ...) at src/gdb/common/errors.c:54
 2       breakpoint     keep y   0x0000000000575127 in internal_error(char const*, int, char const*, ...) at src/gdb/common/errors.c:54
 3       breakpoint     keep y   <MULTIPLE>         
 3.1                         y   0x0000000000575127 in internal_error(char const*, int, char const*, ...) at src/gdb/common/errors.c:54
 3.2                         y   0x00007ffff6d50410 in PyErr_SetObject at /usr/src/debug/python2-2.7.15-4.fc27.x86_64/Python/errors.c:54
 (top-gdb)

From looking at those, you have no idea how I created the breakpoints in
the first place, which specs I used.  The first two look like the same,
but really aren't.  I don't recall which was which though.
And what's with the third, showing seemingly unrelated functions?

GDB of course knows the breakpoints were set differently.  It needs to
remember the spec in order to re_set the locations properly.  And it
needs to remember the spec in order to save the breakpoints, like:

 (top-gdb) save breakpoints bpts.cmd
 Saved to file 'bpts.cmd'.

Let's look at the file, see how the breakpoint had been created:

 (top-gdb) shell cat bpts.cmd
 break internal_error
 break -qualified internal_error
 break errors.c:54
 (top-gdb) 

Ah, see how the "-qualified" information was lost from
"info breakpoints" output?  And how now it's obvious
why we have two locations for breakpoint 3?

Why not show this in "info breakpoints" too?

A way to do that would be always show separate lines for
breakpoint and locations, as if all breakpoints had multiple
locations, even if they only have one location.  The "What"
column for the owner breakpoint could then show the canonical
location string:

 (top-gdb) info breakpoints 
 Num     Type           Disp Enb Address            What
 1       breakpoint     keep y                      internal_error
 1.1                         y   0x00000000005755a5 in internal_error(char const*, int, char const*, ...) at /home/pedro/gdb/mygit/src/gdb/common/errors.c:54
 2       breakpoint     keep y                      -qualified internal_error
 2.1                         y   0x00000000005755a5 in internal_error(char const*, int, char const*, ...) at /home/pedro/gdb/mygit/src/gdb/common/errors.c:54
 3       breakpoint     keep y                      errors.c:54
 3.1                         y   0x00000000005755a5 in internal_error(char const*, int, char const*, ...) at /home/pedro/gdb/mygit/src/gdb/common/errors.c:54
 3.2                         y   0x00007ffff6d50410 in PyErr_SetObject at /usr/src/debug/python2-2.7.15-4.fc27.x86_64/Python/errors.c:54

Breakpoints would no longer move from single-location to multiple-location
"display modes" as location were added/removed (e.g., when a shared library
is loaded), they'd always be in multi-location display mode.

Note we currently have a special case when we show the location of
a breakpoint with a single location separately, it's when a breakpoint
has a single location that is disabled.  The above gets rid of the
special case, by always applying the special case, you could say.  :-)

Here's what it would look like in that case we're discussing,
where the user sets a breapoint on a comment or empty line:

 (top-gdb) b 27
 Breakpoint 4 at 0x469aa6: file /home/pedro/gdb/mygit/src/gdb/gdb.c, line 28.
 (top-gdb) info breakpoints 4
 Num     Type           Disp Enb Address            What
 4       breakpoint     keep y                      /home/pedro/gdb/mygit/src/gdb/gdb.c:27
 4.1                         y   0x0000000000469aa6 in main(int, char**) at /home/pedro/gdb/mygit/src/gdb/gdb.c:28

Note how GDB remembers line 27, but we don't currently show it, unlike
in the snippet above with patched gdb.

If we had this, then I think it would also make sense to
add a way to list breakpoints and skip showing the locations, for
a more condensed, higher level view of the breakpoints.
E.g., try "b main" while debugging gdb.  You'll end up with
24 locations...:

(top-gdb) b main
Breakpoint 3 at 0x469aa6: main. (24 locations)
(top-gdb) info breakpoints 
Num     Type           Disp Enb Address            What
1       breakpoint     keep y                      internal_error
1.1                         y   0x00000000005755a5 in internal_error(char const*, int, char const*, ...) at /home/pedro/gdb/mygit/src/gdb/common/errors.c:54
2       breakpoint     keep y                      info_command
        silent
        return
2.1                         y   0x0000000000545204 in info_command(char const*, int) at /home/pedro/gdb/mygit/src/gdb/cli/cli-cmds.c:201
3       breakpoint     keep y                      main
3.1                         y   0x0000000000469aa6 in main(int, char**) at /home/pedro/gdb/mygit/src/gdb/gdb.c:28
3.2                         y   0x00000000009875fa in selftests::string_view::capacity_1::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/capacity/1.cc:167
3.3                         y   0x00000000009879ac in selftests::string_view::cons_1::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/cons/char/1.cc:62
3.4                         y   0x0000000000987a67 in selftests::string_view::cons_2::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/cons/char/2.cc:41
3.5                         y   0x0000000000987aa2 in selftests::string_view::cons_3::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/cons/char/3.cc:34
3.6                         y   0x0000000000987c2b in selftests::string_view::element_access_1::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/element_access/char/1.cc:66
3.7                         y   0x0000000000987c3f in selftests::string_view::element_access_empty::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/element_access/char/empty.cc:27
3.8                         y   0x0000000000987de3 in selftests::string_view::element_access_front_back::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/element_access/char/front_back.cc:38
3.9                         y   0x000000000098819b in selftests::string_view::inserters_2::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/inserters/char/2.cc:84
3.10                        y   0x00000000009882cf in selftests::string_view::modifiers_remove_prefix::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/modifiers/remove_prefix/char/1.cc:58
3.11                        y   0x00000000009883e0 in selftests::string_view::modifiers_remove_suffix::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/modifiers/remove_suffix/char/1.cc:58
3.12                        y   0x0000000000988b99 in selftests::string_view::operations_compare_1::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operations/compare/char/1.cc:127
3.13                        y   0x0000000000988cc6 in selftests::string_view::operations_compare_13650::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operations/compare/char/13650.cc:45
3.14                        y   0x0000000000988d82 in selftests::string_view::operations_copy_1::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operations/copy/char/1.cc:41
3.15                        y   0x0000000000988e20 in selftests::string_view::operations_data_1::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operations/data/char/1.cc:39
3.16                        y   0x0000000000989370 in selftests::string_view::operations_find_1::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operations/find/char/1.cc:160
3.17                        y   0x0000000000989861 in selftests::string_view::operations_find_2::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operations/find/char/2.cc:158
3.18                        y   0x0000000000989e43 in selftests::string_view::operations_find_3::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operations/find/char/3.cc:158
3.19                        y   0x0000000000989ebb in selftests::string_view::operations_find_4::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operations/find/char/4.cc:40
3.20                        y   0x000000000098a41b in selftests::string_view::operations_rfind_1::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operations/rfind/char/1.cc:90
3.21                        y   0x000000000098a623 in selftests::string_view::operations_rfind_2::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operations/rfind/char/2.cc:48
3.22                        y   0x000000000098a970 in selftests::string_view::operations_rfind_3::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operations/rfind/char/3.cc:62
3.23                        y   0x000000000098ac52 in selftests::string_view::operations_substr_1::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operations/substr/char/1.cc:74
3.24                        y   0x000000000098cd7a in selftests::string_view::operators_2::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operators/char/2.cc:366

With a "-hide-locations" switch, a user could do this to get the locations
out of the way:

 (top-gdb) info breakpoints -hide-locations 
 Num     Type           Disp Enb Address            What
 1       breakpoint     keep y                      internal_error
 2       breakpoint     keep y                      info_command
         silent
         return
 3       breakpoint     keep y                      main


(We could have switches for only showing enabled or disabled breakpoints
too, btw.)

Below's the prototype patch.

I didn't think about what to do with watchpoints, catchpoints,
etc.  Currently they show like an unpatched gdb:

 3       catchpoint     keep y                      syscall "<any syscall>" 
 4       hw watchpoint  keep y                      *main

I did notice that the patch currently crashes with catchpoints that
are implemented via breakpoints, for example, and possibly
other types:

 (top-gdb) catch throw 
 Catchpoint 5 (throw)
 (top-gdb) info breakpoints 
 5       breakpoint     keep y   ../../gdb/breakpoint.c:6387: internal-error: void print_one_breakpoint_location(breakpoint*, bp_location*, int, bp_location**, int): Assertion `b->loc == NULL || b->loc->next == NULL' failed.
 A problem internal to GDB has been detected,
 further debugging may prove unreliable.
 Quit this debugging session? (y or n) 

whoops.  :-)  I did say this was a prototype, right?  :-)

From 8087acd6926d1054979ed42076a9bb0b28c4551e Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Sat, 22 Jun 2019 21:10:58 +0100
Subject: [PATCH] bkpts

---
 gdb/breakpoint.c     | 114 ++++++++++++++++++++++++++++++++++++++-------------
 gdb/cli/cli-option.c |  12 +++++-
 gdb/cli/cli-option.h |   6 +++
 3 files changed, 102 insertions(+), 30 deletions(-)
  

Comments

Andrew Burgess July 1, 2019, 11:37 a.m. UTC | #1
* Pedro Alves <palves@redhat.com> [2019-06-24 20:54:43 +0100]:

> On 6/24/19 8:16 PM, Pedro Alves wrote:
> > 
> > I've thought for a few years that "info breakpoints" should show
> > BOTH the canonical spec behind each breakpoint, and the actual
> > location(s) where the breakpoint is inserted.  It wouldn't
> > be that hard, even.  I cooked up a prototype patch for that.
> > I'll post it as a follow up.
> 
> Here's what I meant.
> 
> Here are for example 3 breakpoints that I set while debugging gdb:
> 
>  (top-gdb) info breakpoints
>  1       breakpoint     keep y   0x0000000000575127 in internal_error(char const*, int, char const*, ...) at src/gdb/common/errors.c:54
>  2       breakpoint     keep y   0x0000000000575127 in internal_error(char const*, int, char const*, ...) at src/gdb/common/errors.c:54
>  3       breakpoint     keep y   <MULTIPLE>         
>  3.1                         y   0x0000000000575127 in internal_error(char const*, int, char const*, ...) at src/gdb/common/errors.c:54
>  3.2                         y   0x00007ffff6d50410 in PyErr_SetObject at /usr/src/debug/python2-2.7.15-4.fc27.x86_64/Python/errors.c:54
>  (top-gdb)
> 
> From looking at those, you have no idea how I created the breakpoints in
> the first place, which specs I used.  The first two look like the same,
> but really aren't.  I don't recall which was which though.
> And what's with the third, showing seemingly unrelated functions?
> 
> GDB of course knows the breakpoints were set differently.  It needs to
> remember the spec in order to re_set the locations properly.  And it
> needs to remember the spec in order to save the breakpoints, like:
> 
>  (top-gdb) save breakpoints bpts.cmd
>  Saved to file 'bpts.cmd'.
> 
> Let's look at the file, see how the breakpoint had been created:
> 
>  (top-gdb) shell cat bpts.cmd
>  break internal_error
>  break -qualified internal_error
>  break errors.c:54
>  (top-gdb) 
> 
> Ah, see how the "-qualified" information was lost from
> "info breakpoints" output?  And how now it's obvious
> why we have two locations for breakpoint 3?
> 
> Why not show this in "info breakpoints" too?
> 
> A way to do that would be always show separate lines for
> breakpoint and locations, as if all breakpoints had multiple
> locations, even if they only have one location.  The "What"
> column for the owner breakpoint could then show the canonical
> location string:
> 
>  (top-gdb) info breakpoints 
>  Num     Type           Disp Enb Address            What
>  1       breakpoint     keep y                      internal_error
>  1.1                         y   0x00000000005755a5 in internal_error(char const*, int, char const*, ...) at /home/pedro/gdb/mygit/src/gdb/common/errors.c:54
>  2       breakpoint     keep y                      -qualified internal_error
>  2.1                         y   0x00000000005755a5 in internal_error(char const*, int, char const*, ...) at /home/pedro/gdb/mygit/src/gdb/common/errors.c:54
>  3       breakpoint     keep y                      errors.c:54
>  3.1                         y   0x00000000005755a5 in internal_error(char const*, int, char const*, ...) at /home/pedro/gdb/mygit/src/gdb/common/errors.c:54
>  3.2                         y   0x00007ffff6d50410 in PyErr_SetObject at /usr/src/debug/python2-2.7.15-4.fc27.x86_64/Python/errors.c:54

I think this would be a good change.

Thanks,
Andrew


> 
> Breakpoints would no longer move from single-location to multiple-location
> "display modes" as location were added/removed (e.g., when a shared library
> is loaded), they'd always be in multi-location display mode.
> 
> Note we currently have a special case when we show the location of
> a breakpoint with a single location separately, it's when a breakpoint
> has a single location that is disabled.  The above gets rid of the
> special case, by always applying the special case, you could say.  :-)
> 
> Here's what it would look like in that case we're discussing,
> where the user sets a breapoint on a comment or empty line:
> 
>  (top-gdb) b 27
>  Breakpoint 4 at 0x469aa6: file /home/pedro/gdb/mygit/src/gdb/gdb.c, line 28.
>  (top-gdb) info breakpoints 4
>  Num     Type           Disp Enb Address            What
>  4       breakpoint     keep y                      /home/pedro/gdb/mygit/src/gdb/gdb.c:27
>  4.1                         y   0x0000000000469aa6 in main(int, char**) at /home/pedro/gdb/mygit/src/gdb/gdb.c:28
> 
> Note how GDB remembers line 27, but we don't currently show it, unlike
> in the snippet above with patched gdb.
> 
> If we had this, then I think it would also make sense to
> add a way to list breakpoints and skip showing the locations, for
> a more condensed, higher level view of the breakpoints.
> E.g., try "b main" while debugging gdb.  You'll end up with
> 24 locations...:
> 
> (top-gdb) b main
> Breakpoint 3 at 0x469aa6: main. (24 locations)
> (top-gdb) info breakpoints 
> Num     Type           Disp Enb Address            What
> 1       breakpoint     keep y                      internal_error
> 1.1                         y   0x00000000005755a5 in internal_error(char const*, int, char const*, ...) at /home/pedro/gdb/mygit/src/gdb/common/errors.c:54
> 2       breakpoint     keep y                      info_command
>         silent
>         return
> 2.1                         y   0x0000000000545204 in info_command(char const*, int) at /home/pedro/gdb/mygit/src/gdb/cli/cli-cmds.c:201
> 3       breakpoint     keep y                      main
> 3.1                         y   0x0000000000469aa6 in main(int, char**) at /home/pedro/gdb/mygit/src/gdb/gdb.c:28
> 3.2                         y   0x00000000009875fa in selftests::string_view::capacity_1::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/capacity/1.cc:167
> 3.3                         y   0x00000000009879ac in selftests::string_view::cons_1::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/cons/char/1.cc:62
> 3.4                         y   0x0000000000987a67 in selftests::string_view::cons_2::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/cons/char/2.cc:41
> 3.5                         y   0x0000000000987aa2 in selftests::string_view::cons_3::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/cons/char/3.cc:34
> 3.6                         y   0x0000000000987c2b in selftests::string_view::element_access_1::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/element_access/char/1.cc:66
> 3.7                         y   0x0000000000987c3f in selftests::string_view::element_access_empty::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/element_access/char/empty.cc:27
> 3.8                         y   0x0000000000987de3 in selftests::string_view::element_access_front_back::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/element_access/char/front_back.cc:38
> 3.9                         y   0x000000000098819b in selftests::string_view::inserters_2::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/inserters/char/2.cc:84
> 3.10                        y   0x00000000009882cf in selftests::string_view::modifiers_remove_prefix::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/modifiers/remove_prefix/char/1.cc:58
> 3.11                        y   0x00000000009883e0 in selftests::string_view::modifiers_remove_suffix::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/modifiers/remove_suffix/char/1.cc:58
> 3.12                        y   0x0000000000988b99 in selftests::string_view::operations_compare_1::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operations/compare/char/1.cc:127
> 3.13                        y   0x0000000000988cc6 in selftests::string_view::operations_compare_13650::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operations/compare/char/13650.cc:45
> 3.14                        y   0x0000000000988d82 in selftests::string_view::operations_copy_1::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operations/copy/char/1.cc:41
> 3.15                        y   0x0000000000988e20 in selftests::string_view::operations_data_1::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operations/data/char/1.cc:39
> 3.16                        y   0x0000000000989370 in selftests::string_view::operations_find_1::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operations/find/char/1.cc:160
> 3.17                        y   0x0000000000989861 in selftests::string_view::operations_find_2::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operations/find/char/2.cc:158
> 3.18                        y   0x0000000000989e43 in selftests::string_view::operations_find_3::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operations/find/char/3.cc:158
> 3.19                        y   0x0000000000989ebb in selftests::string_view::operations_find_4::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operations/find/char/4.cc:40
> 3.20                        y   0x000000000098a41b in selftests::string_view::operations_rfind_1::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operations/rfind/char/1.cc:90
> 3.21                        y   0x000000000098a623 in selftests::string_view::operations_rfind_2::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operations/rfind/char/2.cc:48
> 3.22                        y   0x000000000098a970 in selftests::string_view::operations_rfind_3::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operations/rfind/char/3.cc:62
> 3.23                        y   0x000000000098ac52 in selftests::string_view::operations_substr_1::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operations/substr/char/1.cc:74
> 3.24                        y   0x000000000098cd7a in selftests::string_view::operators_2::main() at /home/pedro/gdb/mygit/src/gdb/unittests/basic_string_view/operators/char/2.cc:366
> 
> With a "-hide-locations" switch, a user could do this to get the locations
> out of the way:
> 
>  (top-gdb) info breakpoints -hide-locations 
>  Num     Type           Disp Enb Address            What
>  1       breakpoint     keep y                      internal_error
>  2       breakpoint     keep y                      info_command
>          silent
>          return
>  3       breakpoint     keep y                      main
> 
> 
> (We could have switches for only showing enabled or disabled breakpoints
> too, btw.)
> 
> Below's the prototype patch.
> 
> I didn't think about what to do with watchpoints, catchpoints,
> etc.  Currently they show like an unpatched gdb:
> 
>  3       catchpoint     keep y                      syscall "<any syscall>" 
>  4       hw watchpoint  keep y                      *main
> 
> I did notice that the patch currently crashes with catchpoints that
> are implemented via breakpoints, for example, and possibly
> other types:
> 
>  (top-gdb) catch throw 
>  Catchpoint 5 (throw)
>  (top-gdb) info breakpoints 
>  5       breakpoint     keep y   ../../gdb/breakpoint.c:6387: internal-error: void print_one_breakpoint_location(breakpoint*, bp_location*, int, bp_location**, int): Assertion `b->loc == NULL || b->loc->next == NULL' failed.
>  A problem internal to GDB has been detected,
>  further debugging may prove unreliable.
>  Quit this debugging session? (y or n) 
> 
> whoops.  :-)  I did say this was a prototype, right?  :-)
> 
> From 8087acd6926d1054979ed42076a9bb0b28c4551e Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Sat, 22 Jun 2019 21:10:58 +0100
> Subject: [PATCH] bkpts
> 
> ---
>  gdb/breakpoint.c     | 114 ++++++++++++++++++++++++++++++++++++++-------------
>  gdb/cli/cli-option.c |  12 +++++-
>  gdb/cli/cli-option.h |   6 +++
>  3 files changed, 102 insertions(+), 30 deletions(-)
> 
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 8422db8b571..d28c4736d43 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -5973,6 +5973,33 @@ output_thread_groups (struct ui_out *uiout,
>      }
>  }
>  
> +/* The options for the "info breakpoints" command.  */
> +
> +struct info_breakpoints_opts
> +{
> +  /* For "-hide-locations".  */
> +  int hide_locations = 0;
> +};
> +
> +static const gdb::option::option_def info_breakpoints_option_defs[] = {
> +
> +  gdb::option::flag_option_def<info_breakpoints_opts> {
> +    "hide-locations",
> +    [] (info_breakpoints_opts *opts) { return &opts->hide_locations; },
> +    N_("Hide breakpoint locations."),
> +  },
> +
> +};
> +
> +/* Create an option_def_group for the "info breakpoints" options, with
> +   OPTS as context.  */
> +
> +static inline gdb::option::option_def_group
> +make_info_breakpoints_options_def_group (info_breakpoints_opts *opts)
> +{
> +  return {{info_breakpoints_option_defs}, opts};
> +}
> +
>  /* Print B to gdb_stdout.  */
>  
>  static void
> @@ -5993,14 +6020,11 @@ print_one_breakpoint_location (struct breakpoint *b,
>    get_user_print_options (&opts);
>  
>    gdb_assert (!loc || loc_number != 0);
> -  /* See comment in print_one_breakpoint concerning treatment of
> -     breakpoints with single disabled location.  */
> -  if (loc == NULL 
> -      && (b->loc != NULL 
> -	  && (b->loc->next != NULL || !b->loc->enabled)))
> -    header_of_multiple = 1;
>    if (loc == NULL)
> -    loc = b->loc;
> +    {
> +      header_of_multiple = 1;
> +      loc = b->loc;
> +    }
>  
>    annotate_record ();
>  
> @@ -6098,7 +6122,7 @@ print_one_breakpoint_location (struct breakpoint *b,
>  	  {
>  	    annotate_field (4);
>  	    if (header_of_multiple)
> -	      uiout->field_string ("addr", "<MULTIPLE>");
> +	      uiout->field_skip ("addr");
>  	    else if (b->loc == NULL || loc->shlib_disabled)
>  	      uiout->field_string ("addr", "<PENDING>");
>  	    else
> @@ -6106,7 +6130,9 @@ print_one_breakpoint_location (struct breakpoint *b,
>  				      loc->gdbarch, loc->address);
>  	  }
>  	annotate_field (5);
> -	if (!header_of_multiple)
> +	if (header_of_multiple)
> +	  uiout->field_string ("what", event_location_to_string (b->location.get ()));
> +	else
>  	  print_breakpoint_location (b, loc);
>  	if (b->loc)
>  	  *last_loc = b->loc;
> @@ -6333,7 +6359,8 @@ bool fix_multi_location_breakpoint_output_globally = false;
>  static void
>  print_one_breakpoint (struct breakpoint *b,
>  		      struct bp_location **last_loc, 
> -		      int allflag)
> +		      int allflag,
> +		      const info_breakpoints_opts &ib_opts)
>  {
>    struct ui_out *uiout = current_uiout;
>    bool use_fixed_output
> @@ -6348,21 +6375,15 @@ print_one_breakpoint (struct breakpoint *b,
>    if (!use_fixed_output)
>      bkpt_tuple_emitter.reset ();
>  
> -  /* If this breakpoint has custom print function,
> -     it's already printed.  Otherwise, print individual
> -     locations, if any.  */
> -  if (b->ops == NULL || b->ops->print_one == NULL)
> +  /* If this breakpoint has a custom print function, it's already
> +     printed.  Otherwise, print individual locations, if any, and if
> +     not explicitly disabled by the user.  */
> +  if ((b->ops == NULL || b->ops->print_one == NULL)
> +      && !ib_opts.hide_locations)
>      {
> -      /* If breakpoint has a single location that is disabled, we
> -	 print it as if it had several locations, since otherwise it's
> -	 hard to represent "breakpoint enabled, location disabled"
> -	 situation.
> -
> -	 Note that while hardware watchpoints have several locations
> -	 internally, that's not a property exposed to user.  */
> -      if (b->loc 
> -	  && !is_hardware_watchpoint (b)
> -	  && (b->loc->next || !b->loc->enabled))
> +      /* While hardware watchpoints have several locations internally,
> +	 that's not a property exposed to the user.  */
> +      if (!is_hardware_watchpoint (b))
>  	{
>  	  gdb::optional<ui_out_emit_list> locations_list;
>  
> @@ -6410,8 +6431,9 @@ breakpoint_address_bits (struct breakpoint *b)
>  void
>  print_breakpoint (breakpoint *b)
>  {
> +  info_breakpoints_opts ib_opts;
>    struct bp_location *dummy_loc = NULL;
> -  print_one_breakpoint (b, &dummy_loc, 0);
> +  print_one_breakpoint (b, &dummy_loc, 0, ib_opts);
>  }
>  
>  /* Return true if this breakpoint was set by the user, false if it is
> @@ -6452,6 +6474,17 @@ breakpoint_1 (const char *args, int allflag,
>  
>    get_user_print_options (&opts);
>  
> +  info_breakpoints_opts ib_opts;
> +
> +  auto grp = make_info_breakpoints_options_def_group (&ib_opts);
> +
> +  gdb::option::process_options
> +    (&args, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, grp);
> +
> +  if (args != nullptr
> +      && args[0] == '-' && (!allflag || !isdigit (args[1])))
> +    gdb::option::error_unrecognized_option_at (args);
> +
>    /* Compute the number of rows in the table, as well as the size
>       required for address fields.  */
>    nr_printable_breakpoints = 0;
> @@ -6549,7 +6582,7 @@ breakpoint_1 (const char *args, int allflag,
>  	/* We only print out user settable breakpoints unless the
>  	   allflag is set.  */
>  	if (allflag || user_breakpoint_p (b))
> -	  print_one_breakpoint (b, &last_loc, allflag);
> +	  print_one_breakpoint (b, &last_loc, allflag, ib_opts);
>        }
>    }
>  
> @@ -6579,6 +6612,29 @@ breakpoint_1 (const char *args, int allflag,
>    return nr_printable_breakpoints;
>  }
>  
> +/* Completer for the "info breakpoints" command.  */
> +
> +static void
> +info_breakpoints_command_completer (struct cmd_list_element *ignore,
> +				    completion_tracker &tracker,
> +				    const char *text, const char *word_ignored)
> +{
> +  const auto grp = make_info_breakpoints_options_def_group (nullptr);
> +
> +  if (gdb::option::complete_options
> +      (tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, grp))
> +    return;
> +
> +  /* Convenience to let the user know what the command can accept.  */
> +  if (*text == '\0')
> +    {
> +      gdb::option::complete_on_all_options (tracker, grp);
> +      /* Keep this "ID" in sync with what "help info breakpoints"
> +	 says.  */
> +      tracker.add_completion (make_unique_xstrdup ("ID"));
> +    }
> +}
> +
>  /* Display the value of default-collect in a way that is generally
>     compatible with the breakpoint list.  */
>  
> @@ -15611,7 +15667,7 @@ Convenience variable \"$bpnum\" contains the number of the last\n\
>  breakpoint set."));
>      }
>  
> -  add_info ("breakpoints", info_breakpoints_command, _("\
> +  c = add_info ("breakpoints", info_breakpoints_command, _("\
>  Status of specified breakpoints (all user-settable breakpoints if no argument).\n\
>  The \"Type\" column indicates one of:\n\
>  \tbreakpoint     - normal breakpoint\n\
> @@ -15626,10 +15682,11 @@ are set to the address of the last breakpoint listed unless the command\n\
>  is prefixed with \"server \".\n\n\
>  Convenience variable \"$bpnum\" contains the number of the last\n\
>  breakpoint set."));
> +  set_cmd_completer_handle_brkchars (c, info_breakpoints_command_completer);
>  
>    add_info_alias ("b", "breakpoints", 1);
>  
> -  add_cmd ("breakpoints", class_maintenance, maintenance_info_breakpoints, _("\
> +  c = add_cmd ("breakpoints", class_maintenance, maintenance_info_breakpoints, _("\
>  Status of all breakpoints, or breakpoint number NUMBER.\n\
>  The \"Type\" column indicates one of:\n\
>  \tbreakpoint     - normal breakpoint\n\
> @@ -15649,6 +15706,7 @@ is prefixed with \"server \".\n\n\
>  Convenience variable \"$bpnum\" contains the number of the last\n\
>  breakpoint set."),
>  	   &maintenanceinfolist);
> +  set_cmd_completer_handle_brkchars (c, info_breakpoints_command_completer);
>  
>    add_prefix_cmd ("catch", class_breakpoint, catch_command, _("\
>  Set catchpoints to catch events."),
> diff --git a/gdb/cli/cli-option.c b/gdb/cli/cli-option.c
> index 9a53ec0592d..d43edb78641 100644
> --- a/gdb/cli/cli-option.c
> +++ b/gdb/cli/cli-option.c
> @@ -121,6 +121,14 @@ complete_on_all_options (completion_tracker &tracker,
>    complete_on_options (options_group, tracker, opt + 1, opt);
>  }
>  
> +/* See cli-option.h.  */
> +
> +void
> +error_unrecognized_option_at (const char *at)
> +{
> +  error (_("Unrecognized option at: %s"), at);
> +}
> +
>  /* Parse ARGS, guided by OPTIONS_GROUP.  HAVE_DELIMITER is true if the
>     whole ARGS line included the "--" options-terminator delimiter.  */
>  
> @@ -136,7 +144,7 @@ parse_option (gdb::array_view<const option_def_group> options_group,
>    else if (**args != '-')
>      {
>        if (have_delimiter)
> -	error (_("Unrecognized option at: %s"), *args);
> +	error_unrecognized_option_at (*args);
>        return {};
>      }
>    else if (check_for_argument (args, "--"))
> @@ -182,7 +190,7 @@ parse_option (gdb::array_view<const option_def_group> options_group,
>    if (match == nullptr)
>      {
>        if (have_delimiter || mode != PROCESS_OPTIONS_UNKNOWN_IS_OPERAND)
> -	error (_("Unrecognized option at: %s"), *args);
> +	error_unrecognized_option_at (*args);
>  
>        return {};
>      }
> diff --git a/gdb/cli/cli-option.h b/gdb/cli/cli-option.h
> index 1bfbfce1ce5..06f12751c36 100644
> --- a/gdb/cli/cli-option.h
> +++ b/gdb/cli/cli-option.h
> @@ -314,6 +314,12 @@ extern void
>    complete_on_all_options (completion_tracker &tracker,
>  			   gdb::array_view<const option_def_group> options_group);
>  
> +/* Throw an error indicating an unrecognized option was detected at
> +   AT.  Use this in conjunction with UNKNOWN_IS_OPERAND instead of
> +   UNKNOWN_IS_ERROR when the operand may or may not begin with '-'
> +   depending on some condition determined at run time.  */
> +extern void error_unrecognized_option_at (const char *at);
> +
>  /* Return a string with the result of replacing %OPTIONS% in HELP_TMLP
>     with an auto-generated "help" string fragment for all the options
>     in OPTIONS_GROUP.  */
> -- 
> 2.14.5
>
  
Pedro Alves July 3, 2019, 10:37 p.m. UTC | #2
On 6/24/19 8:54 PM, Pedro Alves wrote:
> I did notice that the patch currently crashes with catchpoints that
> are implemented via breakpoints, for example, and possibly
> other types:
> 
>  (top-gdb) catch throw 
>  Catchpoint 5 (throw)
>  (top-gdb) info breakpoints 
>  5       breakpoint     keep y   ../../gdb/breakpoint.c:6387: internal-error: void print_one_breakpoint_location(breakpoint*, bp_location*, int, bp_location**, int): Assertion `b->loc == NULL || b->loc->next == NULL' failed.
>  A problem internal to GDB has been detected,
>  further debugging may prove unreliable.
>  Quit this debugging session? (y or n) 
> 
> whoops.  :-)  I did say this was a prototype, right?  :-)

Actually, turns out this is a preexisting problem.

GDB is not expecting to find more than one location for a catchpoint
backed by breakpoints.  But it is now finding two locations.
The breakpoint is set at "__cxa_begin_catch", and if we do that manually,
we see what are those locations:

(top-gdb) b __cxa_begin_catch
Breakpoint 5 at 0xb126c0 (2 locations)
(top-gdb) info breakpoints 
Num     Type           Disp Enb Address            What
1       breakpoint     keep y   <MULTIPLE>         
1.1                         y   0x0000000000b126c0 <__cxa_begin_catch>
1.2                         y   0x00007ffff2f4ddb0 in __cxxabiv1::__cxa_begin_catch(void*) at ../../../../libstdc++-v3/libsupc++/eh_catch.cc:41

Looks like a regression caused by the -qualified stuff.
I guess we want a breakpoint at <__cxa_begin_catch> only.
Should be easy to fix.

But I also suggest that "info breakpoints" shouldn't be showing any
address for "catch catch" breakpoints (and catch throw, etc.).
It's an implementation detail that these catchpoints are implemented
as magic breakpoints.

Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 8422db8b571..d28c4736d43 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5973,6 +5973,33 @@  output_thread_groups (struct ui_out *uiout,
     }
 }
 
+/* The options for the "info breakpoints" command.  */
+
+struct info_breakpoints_opts
+{
+  /* For "-hide-locations".  */
+  int hide_locations = 0;
+};
+
+static const gdb::option::option_def info_breakpoints_option_defs[] = {
+
+  gdb::option::flag_option_def<info_breakpoints_opts> {
+    "hide-locations",
+    [] (info_breakpoints_opts *opts) { return &opts->hide_locations; },
+    N_("Hide breakpoint locations."),
+  },
+
+};
+
+/* Create an option_def_group for the "info breakpoints" options, with
+   OPTS as context.  */
+
+static inline gdb::option::option_def_group
+make_info_breakpoints_options_def_group (info_breakpoints_opts *opts)
+{
+  return {{info_breakpoints_option_defs}, opts};
+}
+
 /* Print B to gdb_stdout.  */
 
 static void
@@ -5993,14 +6020,11 @@  print_one_breakpoint_location (struct breakpoint *b,
   get_user_print_options (&opts);
 
   gdb_assert (!loc || loc_number != 0);
-  /* See comment in print_one_breakpoint concerning treatment of
-     breakpoints with single disabled location.  */
-  if (loc == NULL 
-      && (b->loc != NULL 
-	  && (b->loc->next != NULL || !b->loc->enabled)))
-    header_of_multiple = 1;
   if (loc == NULL)
-    loc = b->loc;
+    {
+      header_of_multiple = 1;
+      loc = b->loc;
+    }
 
   annotate_record ();
 
@@ -6098,7 +6122,7 @@  print_one_breakpoint_location (struct breakpoint *b,
 	  {
 	    annotate_field (4);
 	    if (header_of_multiple)
-	      uiout->field_string ("addr", "<MULTIPLE>");
+	      uiout->field_skip ("addr");
 	    else if (b->loc == NULL || loc->shlib_disabled)
 	      uiout->field_string ("addr", "<PENDING>");
 	    else
@@ -6106,7 +6130,9 @@  print_one_breakpoint_location (struct breakpoint *b,
 				      loc->gdbarch, loc->address);
 	  }
 	annotate_field (5);
-	if (!header_of_multiple)
+	if (header_of_multiple)
+	  uiout->field_string ("what", event_location_to_string (b->location.get ()));
+	else
 	  print_breakpoint_location (b, loc);
 	if (b->loc)
 	  *last_loc = b->loc;
@@ -6333,7 +6359,8 @@  bool fix_multi_location_breakpoint_output_globally = false;
 static void
 print_one_breakpoint (struct breakpoint *b,
 		      struct bp_location **last_loc, 
-		      int allflag)
+		      int allflag,
+		      const info_breakpoints_opts &ib_opts)
 {
   struct ui_out *uiout = current_uiout;
   bool use_fixed_output
@@ -6348,21 +6375,15 @@  print_one_breakpoint (struct breakpoint *b,
   if (!use_fixed_output)
     bkpt_tuple_emitter.reset ();
 
-  /* If this breakpoint has custom print function,
-     it's already printed.  Otherwise, print individual
-     locations, if any.  */
-  if (b->ops == NULL || b->ops->print_one == NULL)
+  /* If this breakpoint has a custom print function, it's already
+     printed.  Otherwise, print individual locations, if any, and if
+     not explicitly disabled by the user.  */
+  if ((b->ops == NULL || b->ops->print_one == NULL)
+      && !ib_opts.hide_locations)
     {
-      /* If breakpoint has a single location that is disabled, we
-	 print it as if it had several locations, since otherwise it's
-	 hard to represent "breakpoint enabled, location disabled"
-	 situation.
-
-	 Note that while hardware watchpoints have several locations
-	 internally, that's not a property exposed to user.  */
-      if (b->loc 
-	  && !is_hardware_watchpoint (b)
-	  && (b->loc->next || !b->loc->enabled))
+      /* While hardware watchpoints have several locations internally,
+	 that's not a property exposed to the user.  */
+      if (!is_hardware_watchpoint (b))
 	{
 	  gdb::optional<ui_out_emit_list> locations_list;
 
@@ -6410,8 +6431,9 @@  breakpoint_address_bits (struct breakpoint *b)
 void
 print_breakpoint (breakpoint *b)
 {
+  info_breakpoints_opts ib_opts;
   struct bp_location *dummy_loc = NULL;
-  print_one_breakpoint (b, &dummy_loc, 0);
+  print_one_breakpoint (b, &dummy_loc, 0, ib_opts);
 }
 
 /* Return true if this breakpoint was set by the user, false if it is
@@ -6452,6 +6474,17 @@  breakpoint_1 (const char *args, int allflag,
 
   get_user_print_options (&opts);
 
+  info_breakpoints_opts ib_opts;
+
+  auto grp = make_info_breakpoints_options_def_group (&ib_opts);
+
+  gdb::option::process_options
+    (&args, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, grp);
+
+  if (args != nullptr
+      && args[0] == '-' && (!allflag || !isdigit (args[1])))
+    gdb::option::error_unrecognized_option_at (args);
+
   /* Compute the number of rows in the table, as well as the size
      required for address fields.  */
   nr_printable_breakpoints = 0;
@@ -6549,7 +6582,7 @@  breakpoint_1 (const char *args, int allflag,
 	/* We only print out user settable breakpoints unless the
 	   allflag is set.  */
 	if (allflag || user_breakpoint_p (b))
-	  print_one_breakpoint (b, &last_loc, allflag);
+	  print_one_breakpoint (b, &last_loc, allflag, ib_opts);
       }
   }
 
@@ -6579,6 +6612,29 @@  breakpoint_1 (const char *args, int allflag,
   return nr_printable_breakpoints;
 }
 
+/* Completer for the "info breakpoints" command.  */
+
+static void
+info_breakpoints_command_completer (struct cmd_list_element *ignore,
+				    completion_tracker &tracker,
+				    const char *text, const char *word_ignored)
+{
+  const auto grp = make_info_breakpoints_options_def_group (nullptr);
+
+  if (gdb::option::complete_options
+      (tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, grp))
+    return;
+
+  /* Convenience to let the user know what the command can accept.  */
+  if (*text == '\0')
+    {
+      gdb::option::complete_on_all_options (tracker, grp);
+      /* Keep this "ID" in sync with what "help info breakpoints"
+	 says.  */
+      tracker.add_completion (make_unique_xstrdup ("ID"));
+    }
+}
+
 /* Display the value of default-collect in a way that is generally
    compatible with the breakpoint list.  */
 
@@ -15611,7 +15667,7 @@  Convenience variable \"$bpnum\" contains the number of the last\n\
 breakpoint set."));
     }
 
-  add_info ("breakpoints", info_breakpoints_command, _("\
+  c = add_info ("breakpoints", info_breakpoints_command, _("\
 Status of specified breakpoints (all user-settable breakpoints if no argument).\n\
 The \"Type\" column indicates one of:\n\
 \tbreakpoint     - normal breakpoint\n\
@@ -15626,10 +15682,11 @@  are set to the address of the last breakpoint listed unless the command\n\
 is prefixed with \"server \".\n\n\
 Convenience variable \"$bpnum\" contains the number of the last\n\
 breakpoint set."));
+  set_cmd_completer_handle_brkchars (c, info_breakpoints_command_completer);
 
   add_info_alias ("b", "breakpoints", 1);
 
-  add_cmd ("breakpoints", class_maintenance, maintenance_info_breakpoints, _("\
+  c = add_cmd ("breakpoints", class_maintenance, maintenance_info_breakpoints, _("\
 Status of all breakpoints, or breakpoint number NUMBER.\n\
 The \"Type\" column indicates one of:\n\
 \tbreakpoint     - normal breakpoint\n\
@@ -15649,6 +15706,7 @@  is prefixed with \"server \".\n\n\
 Convenience variable \"$bpnum\" contains the number of the last\n\
 breakpoint set."),
 	   &maintenanceinfolist);
+  set_cmd_completer_handle_brkchars (c, info_breakpoints_command_completer);
 
   add_prefix_cmd ("catch", class_breakpoint, catch_command, _("\
 Set catchpoints to catch events."),
diff --git a/gdb/cli/cli-option.c b/gdb/cli/cli-option.c
index 9a53ec0592d..d43edb78641 100644
--- a/gdb/cli/cli-option.c
+++ b/gdb/cli/cli-option.c
@@ -121,6 +121,14 @@  complete_on_all_options (completion_tracker &tracker,
   complete_on_options (options_group, tracker, opt + 1, opt);
 }
 
+/* See cli-option.h.  */
+
+void
+error_unrecognized_option_at (const char *at)
+{
+  error (_("Unrecognized option at: %s"), at);
+}
+
 /* Parse ARGS, guided by OPTIONS_GROUP.  HAVE_DELIMITER is true if the
    whole ARGS line included the "--" options-terminator delimiter.  */
 
@@ -136,7 +144,7 @@  parse_option (gdb::array_view<const option_def_group> options_group,
   else if (**args != '-')
     {
       if (have_delimiter)
-	error (_("Unrecognized option at: %s"), *args);
+	error_unrecognized_option_at (*args);
       return {};
     }
   else if (check_for_argument (args, "--"))
@@ -182,7 +190,7 @@  parse_option (gdb::array_view<const option_def_group> options_group,
   if (match == nullptr)
     {
       if (have_delimiter || mode != PROCESS_OPTIONS_UNKNOWN_IS_OPERAND)
-	error (_("Unrecognized option at: %s"), *args);
+	error_unrecognized_option_at (*args);
 
       return {};
     }
diff --git a/gdb/cli/cli-option.h b/gdb/cli/cli-option.h
index 1bfbfce1ce5..06f12751c36 100644
--- a/gdb/cli/cli-option.h
+++ b/gdb/cli/cli-option.h
@@ -314,6 +314,12 @@  extern void
   complete_on_all_options (completion_tracker &tracker,
 			   gdb::array_view<const option_def_group> options_group);
 
+/* Throw an error indicating an unrecognized option was detected at
+   AT.  Use this in conjunction with UNKNOWN_IS_OPERAND instead of
+   UNKNOWN_IS_ERROR when the operand may or may not begin with '-'
+   depending on some condition determined at run time.  */
+extern void error_unrecognized_option_at (const char *at);
+
 /* Return a string with the result of replacing %OPTIONS% in HELP_TMLP
    with an auto-generated "help" string fragment for all the options
    in OPTIONS_GROUP.  */