* [PATCH 0/2] info breakpoints improvements
@ 2022-05-19 21:55 Pedro Alves
2022-05-19 21:55 ` [PATCH 1/2] Always show locations for breakpoints & show canonical location spec Pedro Alves
` (3 more replies)
0 siblings, 4 replies; 39+ messages in thread
From: Pedro Alves @ 2022-05-19 21:55 UTC (permalink / raw)
To: gdb-patches
The subject of breakpoint line number sliding came up again in an
internal discussion, which reminded me of this WIP patch that I
originally posted here:
https://sourceware.org/pipermail/gdb-patches/2019-June/158638.html
Here's an updated version that is close to ready for merging. It
needs agreement, and, testsuite adjustments. Oh, and I realize now,
NEWS entries. Both patches already contain documentation changes. I
went through the whole "Set Breaks" chapter fixing up things to better
match current reality as I was documenting the new features.
Patch #1 changes GDB such that instead of:
(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
4 breakpoint keep y 0x000055555564107b in main(int, char**) at src/gdb/gdb.c:28
(top-gdb)
we get:
(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 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 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 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
4 breakpoint keep y gdb.c:27
4.1 y 0x000055555564107b in main(int, char**) at src/gdb/gdb.c:28
(top-gdb)
i.e., always show the locations in their own rows, AND print the
canonical location spec in the "What" column of the breakpoint's
header row. It also indents the location numbers by one space for
better grouping. The need for grouping is clearer if you have
breakpoints with dozens of locations, such as when you do "b main"
when debugging GDB.
See that commit's log for a lot more details.
Patch #2 introduces an "info breakpoints -hide-locations" option.
With that, you get just the breakpoint header rows, showing the
canonical location spec originally used to set the breakpoint, but not
what the spec expanded to:
(top-gdb) i b -h
Num Type Disp Enb What
1 breakpoint keep y internal_error
2 breakpoint keep y -qualified internal_error
3 breakpoint keep y errors.c:54
Before this can go in, I'll need to address testsuite fallout, of
course. I expect this will be a significant effort, so I'm hoping we
can come to agreement on whether the GDB change is OK before wasting
time adjusting the testsuite.
Pedro Alves (2):
Always show locations for breakpoints & show canonical location spec
Introduce "info breakpoints -hide-locations"
gdb/doc/gdb.texinfo | 208 ++++++++++++++++++++--------------
gdb/ada-lang.c | 9 +-
gdb/break-catch-exec.c | 10 +-
gdb/break-catch-fork.c | 10 +-
gdb/break-catch-load.c | 8 +-
gdb/break-catch-sig.c | 10 +-
gdb/break-catch-syscall.c | 9 +-
gdb/break-catch-throw.c | 10 +-
gdb/breakpoint.c | 228 +++++++++++++++++++++++++-------------
gdb/breakpoint.h | 5 +-
gdb/cli/cli-option.c | 12 +-
gdb/cli/cli-option.h | 6 +
12 files changed, 324 insertions(+), 201 deletions(-)
base-commit: ee3272d472e864bcbe86d6c647d7b48df715c72b
--
2.36.0
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 1/2] Always show locations for breakpoints & show canonical location spec
2022-05-19 21:55 [PATCH 0/2] info breakpoints improvements Pedro Alves
@ 2022-05-19 21:55 ` Pedro Alves
2022-05-20 6:45 ` Eli Zaretskii
2022-05-20 7:45 ` [PATCH " Metzger, Markus T
2022-05-19 21:55 ` [PATCH 2/2] Introduce "info breakpoints -hide-locations" Pedro Alves
` (2 subsequent siblings)
3 siblings, 2 replies; 39+ messages in thread
From: Pedro Alves @ 2022-05-19 21:55 UTC (permalink / raw)
To: gdb-patches
I've thought for a few years that "info breakpoints" should show BOTH
the canonical location spec behind each breakpoint, and the actual
resolved location(s) where the breakpoint is inserted. It currently
only shows the latter.
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. Breakpoints 1 and 2 look like
the same from that output, but really aren't. I'll have forgotten
after a while which was which though. And what's with the third
breakpoint, showing seemingly unrelated functions in its locations?
GDB of course knows all those breakpoints were set differently. It
needs to remember the location specs 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 breakpoints had been created:
(top-gdb) shell cat bpts.cmd
break internal_error
break -qualified internal_error
break errors.c:54
(top-gdb)
Ah, the "-qualified" information for breakpoint 2 was lost from "info
breakpoints" output. And now it's obvious why we have two locations
for breakpoint 3 -- that breakpoint was set by FILE:LINE, and GDB
resolved that to two locations in unrelated functions that happen to
be implemented in files with the same name.
I propose that we show this info in "info breakpoints" too, with a
conceptualy simple change that just removes special cases. To get
there, let's observe a fact -- there are a couple cases where "info
breakpoints" prints the breakpoint's locations in their own rows:
#1 - when the breakpoint has multiple locations:
(gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
1.1 y 0x0000000000a2d874 in func(int) at func.c:51
1.2 y 0x0000000000b2d640 in func(int) at func2.c:51
#2 - when the breakpoint only has one location, but it is disabled,
while the breakpoint itself is enabled. E.g.:
(gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y 0x0000000000a2d874 in func(int) at func.c:51
(gdb) disable 1.1
(gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
1.1 n 0x0000000000a2d874 in func(int) at func.c:51
(the last example shows <MULTIPLE> when there's only one location, but
let's ignore that...)
Note that when we print the locations in a separate row, the "What"
column for the breakpoint header row is empty. That seems to me to be
the perfect spot to put the breakpoint's location spec.
Then, if we make it such that breakpoints always print their locations
in separate rows, even if they only have one location, that space is
always there to use. The result for the confusing example at the top
would be this:
(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 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 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 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
(Note: this looks better on a terminal with color.)
Breakpoints would no longer move from single-location to
multiple-location "display modes" as locations were added/removed
(e.g., when a shared library is loaded), they'd always be in
multi-location display mode. And we'd get rid of the "breakpoint has
a single location that is disabled" special case.
Now, after using this for a while, I came to wish that the output
would give a better visual clue on each group of "breakpoint + its
locations". This commit implements that by simply indenting the
location number by one space, like so:
(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 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 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 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
The need for grouping is clearer if you have breakpoints with dozens
of locations, such as when you do "b main" when debugging GDB.
Another interesting case where this commit helps is when the user sets
a breakpoint by line number, and the line corresponds to a comment or
empty line, or to code that was optimized away. E.g., again when
debugging GDB:
(top-gdb) b 27
Breakpoint 4 at 0x469aa6: file src/gdb/gdb.c, line 28.
Note I asked for line 27 but got line 28.
"info breakpoints" currently says:
(top-gdb) info breakpoints 4
Num Type Disp Enb Address What
4 breakpoint keep y 0x0000000000469aa6 in main(int, char**) at src/gdb/gdb.c:28
Again here we lost the info about the original location spec, line 27.
While after this commit, we get:
(top-gdb) info breakpoints 4
Num Type Disp Enb Address What
4 breakpoint keep y src/gdb/gdb.c:27
4.1 y 0x0000000000469aa6 in main(int, char**) at src/gdb/gdb.c:28
Lancelot is working on a GDB change that will make GDB slide each
breakpoint location's line independently, so we could end up with
something like this, even:
(top-gdb) info breakpoints 4
Num Type Disp Enb Address What
4 breakpoint keep y func.cc:27
4.1 y 0x0000000000469aa6 in func<float>(float) at func.cc:28
4.2 y 0x000000000047abb8 in func<int>(int) at func.cc:30
The output for watchpoints and catchpoints is unmodified, they will
continue being printed like before, as we never print locations for
those:
Num Type Disp Enb Address What
2 catchpoint keep y syscall "<any syscall>"
3 hw watchpoint keep y *main
Before this commit can go in, I'll need to address testsuite fallout,
of course. I expect this will be a significant effort, so I'm hoping
we can come to agreement on whether the GDB change is OK before
wasting time adjusting the testsuite.
Documentation adjustments included. They also clean up a few tangent
things in the area of the docs I was adjusting to better match
reality.
Change-Id: Ic42ad8565e79ca67bfebb22cbb4794ea816fd08b
---
gdb/doc/gdb.texinfo | 183 +++++++++++++++++++++++++-------------------
gdb/breakpoint.c | 95 +++++++++++------------
2 files changed, 148 insertions(+), 130 deletions(-)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 90c307a7dd2..7bd791afa06 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -4343,17 +4343,24 @@ Vars,, Convenience Variables}, for a discussion of what you can do with
convenience variables.
@table @code
-@item break @var{location}
-Set a breakpoint at the given @var{location}, which can specify a
-function name, a line number, or an address of an instruction.
-(@xref{Specify Location}, for a list of all the possible ways to
-specify a @var{location}.) The breakpoint will stop your program just
-before it executes any of the code in the specified @var{location}.
-
-When using source languages that permit overloading of symbols, such as
-C@t{++}, a function name may refer to more than one possible place to break.
-@xref{Ambiguous Expressions,,Ambiguous Expressions}, for a discussion of
-that situation.
+@item break @var{location_spec}
+Set a breakpoint at all the locations the given @var{location_spec}
+resolves to. @var{location_spec} can specify a function name, a line
+number, an address of an instruction, and more. @xref{Specify
+Location}, for a list of all the possible ways to specify a
+@var{location}. The breakpoint will stop your program just before it
+executes any of the instructions at the resolved locations' addresses.
+
+When using source languages that permit overloading of symbols, such
+as C@t{++}, a function name may refer to more than one symbol, and
+thus more than one place to break. @xref{Ambiguous
+Expressions,,Ambiguous Expressions}, for a discussion of that
+situation.
+
+It is possible that a given function is instantiated more than once in
+the program, resulting in a breakpoint with multiple resolved
+locations. This can happen e.g., with inlined functions, or template
+functions.
It is also possible to insert a breakpoint that will stop the program
only if a specific thread (@pxref{Thread-Specific Breakpoints})
@@ -4407,11 +4414,11 @@ command:
@smallexample
(@value{GDBP}) info breakpoints
Num Type Disp Enb Address What
-1 breakpoint keep y <MULTIPLE>
+1 breakpoint keep y func
stop only if a == 10
-1.1 N* 0x00000000000011b6 in ...
-1.2 y 0x00000000000011c2 in ...
-1.3 N* 0x00000000000011ce in ...
+ 1.1 N* 0x00000000000011b6 in ...
+ 1.2 y 0x00000000000011c2 in ...
+ 1.3 N* 0x00000000000011ce in ...
(*): Breakpoint condition is invalid at this location.
@end smallexample
@@ -4545,32 +4552,45 @@ optionally be surrounded by spaces.
@cindex @code{$_} and @code{info breakpoints}
@item info breakpoints @r{[}@var{list}@dots{}@r{]}
@itemx info break @r{[}@var{list}@dots{}@r{]}
-Print a table of all breakpoints, watchpoints, and catchpoints set and
-not deleted. Optional argument @var{n} means print information only
-about the specified breakpoint(s) (or watchpoint(s) or catchpoint(s)).
-For each breakpoint, following columns are printed:
+Print a table of all breakpoints, watchpoints, tracepoints, and
+catchpoints set and not deleted. Optional argument @var{n} means
+print information only about the specified breakpoint(s) (or
+watchpoint(s) or tracepoint(s) or catchpoint(s)).
+
+A breakpoint location specification (@pxref{Specify Location}) may be
+resolved to several locations in your program. E.g., @code{break
+func} will find a location for each function named @code{func} in the
+program. For each code breakpoint and tracepoint (not for
+watchpoints, nor catchpoints), @value{GDBN} prints a header entry, and
+then one entry for each resolved location of the breakpoint. More on
+this further below.
+
+For each breakpoint and breakpoint location, the following columns are
+printed:
@table @emph
@item Breakpoint Numbers
@item Type
-Breakpoint, watchpoint, or catchpoint.
+Breakpoint, watchpoint, tracepoint, or catchpoint.
@item Disposition
Whether the breakpoint is marked to be disabled or deleted when hit.
@item Enabled or Disabled
-Enabled breakpoints are marked with @samp{y}. @samp{n} marks breakpoints
-that are not enabled.
+Enabled breakpoints and breakpoint locations are marked with @samp{y}.
+@samp{n} marks breakpoints and breakpoint locations that are not
+enabled.
@item Address
-Where the breakpoint is in your program, as a memory address. For a
-pending breakpoint whose address is not yet known, this field will
-contain @samp{<PENDING>}. Such breakpoint won't fire until a shared
-library that has the symbol or line referred by breakpoint is loaded.
-See below for details. A breakpoint with several locations will
-have @samp{<MULTIPLE>} in this field---see below for details.
+Where the breakpoint location is in your program, as a memory address.
+For a breakpoint whose location specification hasn't been resolved to
+any location yet, this field will contain @samp{<PENDING>}. Such
+breakpoint won't fire until its location spec is resolved to an actual
+location, such as e.g., when a shared library that has the symbol or
+line that matches the location specification is loaded. See below for
+details.
@item What
-Where the breakpoint is in the source for your program, as a file and
-line number. For a pending breakpoint, the original string passed to
-the breakpoint command will be listed as it cannot be resolved until
-the appropriate shared library is loaded in the future.
+For a breakpoint header row, the original location specification
+passed to the breakpoint command. For a breakpoint location row,
+where the breakpoint location is in the source for your program, as a
+file and line number.
@end table
@noindent
@@ -4583,8 +4603,7 @@ its condition evaluation mode in between parentheses.
Breakpoint commands, if any, are listed after that. A pending breakpoint is
allowed to have a condition specified for it. The condition is not parsed for
-validity until a shared library is loaded that allows the pending
-breakpoint to resolve to a valid location.
+validity until the pending breakpoint is resolved to a valid location.
@noindent
@code{info break} with a breakpoint
@@ -4637,23 +4656,31 @@ several places where that function is inlined.
In all those cases, @value{GDBN} will insert a breakpoint at all
the relevant locations.
-A breakpoint with multiple locations is displayed in the breakpoint
-table using several rows---one header row, followed by one row for
-each breakpoint location. The header row has @samp{<MULTIPLE>} in the
-address column. The rows for individual locations contain the actual
-addresses for locations, and show the functions to which those
-locations belong. The number column for a location is of the form
+@value{GDBN} may add new locations to existing breakpoints when new
+symbols are loaded. For example, if you have a breakpoint in a
+C@t{++} template function, and a newly loaded shared library has an
+instantiation of that template, a new location is added to the list of
+locations for the breakpoint. More on this further below.
+
+Regulars code breakpoints and tracepoints are displayed in the
+breakpoint table using several rows---one header row per breakpoint,
+followed by one row for each of the breakpoint's locations. The
+header row has an empty address column, and the original location
+specification in the what column. The rows for individual locations
+contain the addresses for the locations, and show the functions to
+which those addresses belong. The number column for a location is
+indented by one space to the right for grouping, and is of the form
@var{breakpoint-number}.@var{location-number}.
For example:
@smallexample
Num Type Disp Enb Address What
-1 breakpoint keep y <MULTIPLE>
+1 breakpoint keep y foo
stop only if i==1
breakpoint already hit 1 time
-1.1 y 0x080486a2 in void foo<int>() at t.cc:8
-1.2 y 0x080486ca in void foo<double>() at t.cc:8
+ 1.1 y 0x080486a2 in void foo<int>() at t.cc:8
+ 1.2 y 0x080486ca in void foo<double>() at t.cc:8
@end smallexample
You cannot delete the individual locations from a breakpoint. However,
@@ -4677,53 +4704,54 @@ any shared library is loaded or unloaded. Typically, you would
set a breakpoint in a shared library at the beginning of your
debugging session, when the library is not loaded, and when the
symbols from the library are not available. When you try to set
-breakpoint, @value{GDBN} will ask you if you want to set
-a so called @dfn{pending breakpoint}---breakpoint whose address
-is not yet resolved.
+a breakpoint, @value{GDBN} will ask you for confirmation of whether
+you want to set a so called @dfn{pending breakpoint}---a breakpoint with
+no resolved locations yet.
After the program is run, whenever a new shared library is loaded,
-@value{GDBN} reevaluates all the breakpoints. When a newly loaded
-shared library contains the symbol or line referred to by some
-pending breakpoint, that breakpoint is resolved and becomes an
-ordinary breakpoint. When a library is unloaded, all breakpoints
-that refer to its symbols or source lines become pending again.
-
-This logic works for breakpoints with multiple locations, too. For
-example, if you have a breakpoint in a C@t{++} template function, and
-a newly loaded shared library has an instantiation of that template,
-a new location is added to the list of locations for the breakpoint.
-
-Except for having unresolved address, pending breakpoints do not
+@value{GDBN} reevaluates all the breakpoints' location specs. When a
+newly loaded shared library contains the symbol or line referred to by
+some pending breakpoint, a location for the breakpoint is found and
+the breakpoint becomes an ordinary breakpoint. When a library is
+unloaded, breakpoint locations that refer to its symbols or source
+lines are not deleted -- they are instead displayed with a
+@code{<PENDING>} address. This is so that if the shared library is
+loaded again, the location's enabled/disabled state is preserved.
+
+Except for having no resolved locations, pending breakpoints do not
differ from regular breakpoints. You can set conditions or commands,
enable and disable them and perform other breakpoint operations.
@value{GDBN} provides some additional commands for controlling what
-happens when the @samp{break} command cannot resolve breakpoint
-address specification to an address:
+happens when the @samp{break} command cannot resolve its breakpoint
+location specification to any location:
@kindex set breakpoint pending
@kindex show breakpoint pending
@table @code
@item set breakpoint pending auto
-This is the default behavior. When @value{GDBN} cannot find the breakpoint
-location, it queries you whether a pending breakpoint should be created.
+This is the default behavior. When @value{GDBN} cannot find any
+location for the location specification, it queries you whether a
+pending breakpoint should be created.
@item set breakpoint pending on
-This indicates that an unrecognized breakpoint location should automatically
-result in a pending breakpoint being created.
+This indicates that when @value{GDBN} cannot resolve any location for
+the location specification, it should create a pending breakpoint
+without asking for confirmation.
@item set breakpoint pending off
-This indicates that pending breakpoints are not to be created. Any
-unrecognized breakpoint location results in an error. This setting does
-not affect any pending breakpoints previously created.
+This indicates that pending breakpoints are not to be created. If
+@value{GDBN} cannot resolve any location for the location
+specification, @value{GDBN} throws an error. This setting does not
+affect any pending breakpoints previously created.
@item show breakpoint pending
Show the current behavior setting for creating pending breakpoints.
@end table
The settings above only affect the @code{break} command and its
-variants. Once breakpoint is set, it will be automatically updated
-as shared libraries are loaded and unloaded.
+variants. Once a breakpoint is created, it will be automatically
+updated as shared libraries are loaded and unloaded.
@cindex automatic hardware breakpoints
For some targets, @value{GDBN} can automatically decide if hardware or
@@ -30544,11 +30572,10 @@ value is @samp{y}, or disabled, in which case the value is @samp{n}.
Note that this is not the same as the field @code{enable}.
@item addr
-The address of the breakpoint. This may be a hexidecimal number,
+The address of the breakpoint. This may be a hexadecimal number,
giving the address; or the string @samp{<PENDING>}, for a pending
-breakpoint; or the string @samp{<MULTIPLE>}, for a breakpoint with
-multiple locations. This field will not be present if no address can
-be determined. For example, a watchpoint does not have an address.
+breakpoint; This field will not be present if no address can be
+determined. For example, a watchpoint does not have an address.
@item addr_flags
Optional field containing any flags related to the address. These flags are
@@ -30629,16 +30656,14 @@ is not.
Some extra data, the exact contents of which are type-dependent.
@item locations
-This field is present if the breakpoint has multiple locations. It is also
-exceptionally present if the breakpoint is enabled and has a single, disabled
-location.
+This field is present for ordinary breakpoints and tracepoints.
The value is a list of locations. The format of a location is described below.
@end table
-A location in a multi-location breakpoint is represented as a tuple with the
-following fields:
+Each location in an ordinary breakpoint or tracepoint is represented
+as a tuple with the following fields:
@table @code
@@ -30660,7 +30685,7 @@ at this location.
@end table
@item addr
-The address of this location as an hexidecimal number.
+The address of this location as an hexadecimal number.
@item addr_flags
Optional field containing any flags related to the address. These flags are
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 9abc1443d96..724bc778a9b 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -6179,42 +6179,36 @@ print_one_breakpoint_location (struct breakpoint *b,
static char bpenables[] = "nynny";
struct ui_out *uiout = current_uiout;
- int header_of_multiple = 0;
- int part_of_multiple = (loc != NULL);
struct value_print_options opts;
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 || b->loc->disabled_by_cond)))
- header_of_multiple = 1;
- if (loc == NULL)
- loc = b->loc;
annotate_record ();
/* 1 */
annotate_field (0);
- if (part_of_multiple)
- uiout->field_fmt ("number", "%d.%d", b->number, loc_number);
+ if (loc != nullptr)
+ {
+ if (uiout->is_mi_like_p ())
+ uiout->field_fmt ("number", "%d.%d", b->number, loc_number);
+ else
+ uiout->field_fmt ("number", " %d.%d", b->number, loc_number);
+ }
else
uiout->field_signed ("number", b->number);
/* 2 */
annotate_field (1);
- if (part_of_multiple)
+ if (loc != nullptr)
uiout->field_skip ("type");
else
uiout->field_string ("type", bptype_string (b->type));
/* 3 */
annotate_field (2);
- if (part_of_multiple)
+ if (loc != nullptr)
uiout->field_skip ("disp");
else
uiout->field_string ("disp", bpdisp_text (b->disposition));
@@ -6225,7 +6219,7 @@ print_one_breakpoint_location (struct breakpoint *b,
display "N*" on CLI, where "*" refers to a footnote below the
table. For MI, simply display a "N" without a footnote. */
const char *N = (uiout->is_mi_like_p ()) ? "N" : "N*";
- if (part_of_multiple)
+ if (loc != nullptr)
uiout->field_string ("enabled", (loc->disabled_by_cond ? N
: (loc->enabled ? "y" : "n")));
else
@@ -6255,9 +6249,8 @@ print_one_breakpoint_location (struct breakpoint *b,
if (opts.addressprint)
{
annotate_field (4);
- if (header_of_multiple)
- uiout->field_string ("addr", "<MULTIPLE>",
- metadata_style.style ());
+ if (loc == nullptr)
+ uiout->field_skip ("addr");
else if (b->loc == NULL || loc->shlib_disabled)
uiout->field_string ("addr", "<PENDING>",
metadata_style.style ());
@@ -6266,14 +6259,22 @@ print_one_breakpoint_location (struct breakpoint *b,
loc->gdbarch, loc->address);
}
annotate_field (5);
- if (!header_of_multiple)
+ if (loc == nullptr)
+ {
+ if (b->location != nullptr)
+ uiout->field_string
+ ("what", event_location_to_string (b->location.get ()));
+ else
+ uiout->field_skip ("what");
+ }
+ else
print_breakpoint_location (b, loc);
if (b->loc)
*last_loc = b->loc;
}
}
- if (loc != NULL && !header_of_multiple)
+ if (loc != nullptr)
{
std::vector<int> inf_nums;
int mi_only = 1;
@@ -6296,8 +6297,7 @@ print_one_breakpoint_location (struct breakpoint *b,
mi_only = 0;
output_thread_groups (uiout, "thread-groups", inf_nums, mi_only);
}
-
- if (!part_of_multiple)
+ else
{
if (b->thread != -1)
{
@@ -6315,10 +6315,10 @@ print_one_breakpoint_location (struct breakpoint *b,
uiout->text ("\n");
- if (!part_of_multiple)
+ if (loc == nullptr)
b->print_one_detail (uiout);
- if (part_of_multiple && frame_id_p (b->frame_id))
+ if (loc != nullptr && frame_id_p (b->frame_id))
{
annotate_field (6);
uiout->text ("\tstop only in stack frame at ");
@@ -6329,7 +6329,7 @@ print_one_breakpoint_location (struct breakpoint *b,
uiout->text ("\n");
}
- if (!part_of_multiple && b->cond_string)
+ if (loc == nullptr && b->cond_string)
{
annotate_field (7);
if (is_tracepoint (b))
@@ -6351,7 +6351,7 @@ print_one_breakpoint_location (struct breakpoint *b,
uiout->text ("\n");
}
- if (!part_of_multiple && b->thread != -1)
+ if (loc == nullptr && b->thread != -1)
{
/* FIXME should make an annotation for this. */
uiout->text ("\tstop only in thread ");
@@ -6366,7 +6366,7 @@ print_one_breakpoint_location (struct breakpoint *b,
uiout->text ("\n");
}
- if (!part_of_multiple)
+ if (loc == nullptr)
{
if (b->hit_count)
{
@@ -6392,7 +6392,7 @@ print_one_breakpoint_location (struct breakpoint *b,
}
}
- if (!part_of_multiple && b->ignore_count)
+ if (loc == nullptr && b->ignore_count)
{
annotate_field (8);
uiout->message ("\tignore next %pF hits\n",
@@ -6402,7 +6402,7 @@ print_one_breakpoint_location (struct breakpoint *b,
/* Note that an enable count of 1 corresponds to "enable once"
behavior, which is reported by the combination of enablement and
disposition, so we don't need to mention it here. */
- if (!part_of_multiple && b->enable_count > 1)
+ if (loc == nullptr && b->enable_count > 1)
{
annotate_field (8);
uiout->text ("\tdisable after ");
@@ -6416,7 +6416,7 @@ print_one_breakpoint_location (struct breakpoint *b,
uiout->text (" hits\n");
}
- if (!part_of_multiple && is_tracepoint (b))
+ if (loc == nullptr && is_tracepoint (b))
{
struct tracepoint *tp = (struct tracepoint *) b;
@@ -6429,7 +6429,7 @@ print_one_breakpoint_location (struct breakpoint *b,
}
l = b->commands ? b->commands.get () : NULL;
- if (!part_of_multiple && l)
+ if (loc == nullptr && l)
{
annotate_field (9);
ui_out_emit_tuple tuple_emitter (uiout, "script");
@@ -6440,7 +6440,7 @@ print_one_breakpoint_location (struct breakpoint *b,
{
struct tracepoint *t = (struct tracepoint *) b;
- if (!part_of_multiple && t->pass_count)
+ if (loc == nullptr && t->pass_count)
{
annotate_field (10);
uiout->text ("\tpass count ");
@@ -6450,7 +6450,7 @@ print_one_breakpoint_location (struct breakpoint *b,
/* Don't display it when tracepoint or tracepoint location is
pending. */
- if (!header_of_multiple && loc != NULL && !loc->shlib_disabled)
+ if (loc != nullptr && !loc->shlib_disabled)
{
annotate_field (11);
@@ -6468,7 +6468,7 @@ print_one_breakpoint_location (struct breakpoint *b,
}
}
- if (uiout->is_mi_like_p () && !part_of_multiple)
+ if (uiout->is_mi_like_p () && loc == nullptr)
{
if (is_watchpoint (b))
{
@@ -6508,30 +6508,23 @@ 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 this breakpoint has custom print function, it's already
+ printed. Otherwise, print individual locations, if any, and if
+ not explicitly disabled by the user. */
if (!printed || allflag)
{
- /* 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
+ /* Note that while hardware watchpoints have several locations
internally, that's not a property exposed to users.
Likewise, while catchpoints may be implemented with
breakpoints (e.g., catch throw), that's not a property
exposed to users. We do however display the internal
breakpoint locations with "maint info breakpoints". */
- if (!is_hardware_watchpoint (b)
- && (!is_catchpoint (b) || is_exception_catchpoint (b)
- || is_ada_exception_catchpoint (b))
- && (allflag
- || (b->loc && (b->loc->next
- || !b->loc->enabled
- || b->loc->disabled_by_cond))))
+ if (!is_watchpoint (b)
+ && (!is_catchpoint (b)
+ || ((is_exception_catchpoint (b)
+ || is_ada_exception_catchpoint (b))
+ && allflag)))
{
gdb::optional<ui_out_emit_list> locations_list;
--
2.36.0
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 2/2] Introduce "info breakpoints -hide-locations"
2022-05-19 21:55 [PATCH 0/2] info breakpoints improvements Pedro Alves
2022-05-19 21:55 ` [PATCH 1/2] Always show locations for breakpoints & show canonical location spec Pedro Alves
@ 2022-05-19 21:55 ` Pedro Alves
2022-05-20 6:48 ` Eli Zaretskii
2022-05-20 5:57 ` [PATCH 0/2] info breakpoints improvements Eli Zaretskii
2022-05-24 8:38 ` Luis Machado
3 siblings, 1 reply; 39+ messages in thread
From: Pedro Alves @ 2022-05-19 21:55 UTC (permalink / raw)
To: gdb-patches
This commit adds a new option to "info breakpoints",
"-hide-locations". It's purpose is to tell GDB to skip printing
breakpoint locations, printing only the breakpoint header rows.
And then, since only code breakpoint locations print anything in the
"Address" column, "-hide-breakpoints" also disables the "Address"
column.
For example, when debugging GDB, you can use the new options to get
this:
(top-gdb) i b -h
Num Type Disp Enb What
1 breakpoint keep y internal_error
2 breakpoint keep y info_command
silent
return
3 breakpoint keep y main
breakpoint already hit 1 time
4 breakpoint keep y error
(top-gdb)
instead of:
(top-gdb) i b
Num Type Disp Enb Address What
1 breakpoint keep y internal_error
1.1 y 0x0000555555f81d12 in internal_error(char const*, int, char const*, ...) at /home/pedro/gdb/binutils-gdb/src/gdbsupport/errors.cc:51
2 breakpoint keep y info_command
silent
return
2.1 y 0x00005555557b3097 in info_command(char const*, int) at /home/pedro/gdb/binutils-gdb/src/gdb/cli/cli-cmds.c:217
3 breakpoint keep y main
breakpoint already hit 1 time
3.1 y 0x000055555564106c in main(int, char**) at /home/pedro/gdb/binutils-gdb/src/gdb/gdb.c:25
3.2 y 0x0000555555dba524 in selftests::string_view::capacity_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/capacity/1.cc:167
3.3 y 0x0000555555dba943 in selftests::string_view::cons_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/cons/char/1.cc:62
3.4 y 0x0000555555dbaa34 in selftests::string_view::cons_2::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/cons/char/2.cc:41
3.5 y 0x0000555555dbaa9a in selftests::string_view::cons_3::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/cons/char/3.cc:34
3.6 y 0x0000555555dbac6b in selftests::string_view::element_access_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/element_access/char/1.cc:66
3.7 y 0x0000555555dbac83 in selftests::string_view::element_access_empty::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/element_access/char/empty.cc:25
3.8 y 0x0000555555dbae91 in selftests::string_view::element_access_front_back::main()
at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/element_access/char/front_back.cc:38
3.9 y 0x0000555555dbb2bd in selftests::string_view::inserters_2::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/inserters/char/2.cc:84
3.10 y 0x0000555555dbb429 in selftests::string_view::modifiers_remove_prefix::main()
at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/modifiers/remove_prefix/char/1.cc:58
3.11 y 0x0000555555dbb575 in selftests::string_view::modifiers_remove_suffix::main()
at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/modifiers/remove_suffix/char/1.cc:58
3.12 y 0x0000555555dbbd38 in selftests::string_view::operations_compare_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/compare/char/1.cc:127
3.13 y 0x0000555555dbbe7b in selftests::string_view::operations_compare_13650::main()
at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/compare/char/13650.cc:45
3.14 y 0x0000555555dbbf6a in selftests::string_view::operations_copy_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/copy/char/1.cc:41
3.15 y 0x0000555555dbc03b in selftests::string_view::operations_data_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/data/char/1.cc:39
3.16 y 0x0000555555dbc5fe in selftests::string_view::operations_find_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/find/char/1.cc:160
3.17 y 0x0000555555dbcb60 in selftests::string_view::operations_find_2::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/find/char/2.cc:158
3.18 y 0x0000555555dbd1c1 in selftests::string_view::operations_find_3::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/find/char/3.cc:158
3.19 y 0x0000555555dbd26c in selftests::string_view::operations_find_4::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/find/char/4.cc:40
3.20 y 0x0000555555dbd83f in selftests::string_view::operations_rfind_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/rfind/char/1.cc:90
3.21 y 0x0000555555dbda98 in selftests::string_view::operations_rfind_2::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/rfind/char/2.cc:48
3.22 y 0x0000555555dbde4c in selftests::string_view::operations_rfind_3::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/rfind/char/3.cc:63
3.23 y 0x0000555555dbe189 in selftests::string_view::operations_substr_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/substr/char/1.cc:74
3.24 y 0x0000555555dbffdb in selftests::string_view::operators_2::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operators/char/2.cc:366
4 breakpoint keep y error
4.1 y 0x00005555557f5142 in gcc_c_plugin::error(char const*) const at /home/pedro/gdb/binutils-gdb/src/gdb/../include/gcc-c-fe.def:198
4.2 y 0x00005555557fe1f4 in gcc_cp_plugin::error(char const*) const at /home/pedro/gdb/binutils-gdb/src/gdb/../include/gcc-cp-fe.def:983
4.3 y 0x0000555555f81c5e in error(char const*, ...) at /home/pedro/gdb/binutils-gdb/src/gdbsupport/errors.cc:39
4.4 y 0x00007ffff65ee4e0 <icu_66::RBBIRuleScanner::error(UErrorCode)>
4.5 y 0x00007ffff685e070 <icu_66::RegexCompile::error(UErrorCode)>
4.6 y 0x00007ffff6cd4e86 in error at /usr/include/x86_64-linux-gnu/bits/error.h:40
4.7 y 0x00007ffff71c2190 in __error at error.c:274
Documentation change included. No testsuite changes yet until there's
agreement on the previous patch.
Change-Id: Ic42ad8565e79ca67bfebb22cbb4794ea816fd08b
---
gdb/doc/gdb.texinfo | 37 ++++++++---
gdb/ada-lang.c | 9 +--
gdb/break-catch-exec.c | 10 ++-
gdb/break-catch-fork.c | 10 ++-
gdb/break-catch-load.c | 8 +--
gdb/break-catch-sig.c | 10 ++-
gdb/break-catch-syscall.c | 9 ++-
gdb/break-catch-throw.c | 10 ++-
gdb/breakpoint.c | 133 +++++++++++++++++++++++++++++++-------
gdb/breakpoint.h | 5 +-
gdb/cli/cli-option.c | 12 +++-
gdb/cli/cli-option.h | 6 ++
12 files changed, 182 insertions(+), 77 deletions(-)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 7bd791afa06..94cfcff8d29 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -4550,8 +4550,8 @@ optionally be surrounded by spaces.
@kindex info breakpoints
@cindex @code{$_} and @code{info breakpoints}
-@item info breakpoints @r{[}@var{list}@dots{}@r{]}
-@itemx info break @r{[}@var{list}@dots{}@r{]}
+@item info breakpoints [-hide-locations] @r{[}@var{list}@dots{}@r{]}
+@itemx info break [-hide-locations] @r{[}@var{list}@dots{}@r{]}
Print a table of all breakpoints, watchpoints, tracepoints, and
catchpoints set and not deleted. Optional argument @var{n} means
print information only about the specified breakpoint(s) (or
@@ -4560,10 +4560,11 @@ watchpoint(s) or tracepoint(s) or catchpoint(s)).
A breakpoint location specification (@pxref{Specify Location}) may be
resolved to several locations in your program. E.g., @code{break
func} will find a location for each function named @code{func} in the
-program. For each code breakpoint and tracepoint (not for
-watchpoints, nor catchpoints), @value{GDBN} prints a header entry, and
-then one entry for each resolved location of the breakpoint. More on
-this further below.
+program. Unless the @code{-hide-locations} option is specified, for
+each code breakpoint and tracepoint (i.e., not for watchpoints, nor
+catchpoints), @value{GDBN} prints a header entry, and then one entry
+for each resolved location of the breakpoint. More on this further
+below.
For each breakpoint and breakpoint location, the following columns are
printed:
@@ -4585,7 +4586,10 @@ any location yet, this field will contain @samp{<PENDING>}. Such
breakpoint won't fire until its location spec is resolved to an actual
location, such as e.g., when a shared library that has the symbol or
line that matches the location specification is loaded. See below for
-details.
+details. This column is not printed if you specify the
+@code{-hide-locations} option, nor if you disable printing of
+addresses with @code{set print address off} (@pxref{set print
+address}).
@item What
For a breakpoint header row, the original location specification
passed to the breakpoint command. For a breakpoint location row,
@@ -4662,7 +4666,7 @@ C@t{++} template function, and a newly loaded shared library has an
instantiation of that template, a new location is added to the list of
locations for the breakpoint. More on this further below.
-Regulars code breakpoints and tracepoints are displayed in the
+Regulars code breakpoints and tracepoints are normally displayed in the
breakpoint table using several rows---one header row per breakpoint,
followed by one row for each of the breakpoint's locations. The
header row has an empty address column, and the original location
@@ -4675,12 +4679,29 @@ indented by one space to the right for grouping, and is of the form
For example:
@smallexample
+(@value{GDBP}) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y foo
stop only if i==1
breakpoint already hit 1 time
1.1 y 0x080486a2 in void foo<int>() at t.cc:8
1.2 y 0x080486ca in void foo<double>() at t.cc:8
+2 breakpoint keep y bar
+ 2.1 y 0x080646a8 in void bar() at t.cc:20
+@end smallexample
+
+When the @code{-hide-locations} flag is used, @value{GDBN} hides the
+breakpoint locations, and doesn't print the address column, since only
+breakpoint locations have addresses. For example:
+
+@smallexample
+
+Num Type Disp Enb What
+(@value{GDBP}) info breakpoints -hide-locations
+1 breakpoint keep y foo
+ stop only if i==1
+ breakpoint already hit 1 time
+2 breakpoint keep y bar
@end smallexample
You cannot delete the individual locations from a breakpoint. However,
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 248b847cfbc..d70cc4651f1 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -12133,7 +12133,7 @@ struct ada_catchpoint : public base_breakpoint
void re_set () override;
void check_status (struct bpstat *bs) override;
enum print_stop_action print_it (const bpstat *bs) const override;
- bool print_one (bp_location **) const override;
+ bool print_one (bp_location **, bool) const override;
void print_mention () const override;
void print_recreate (struct ui_file *fp) const override;
@@ -12385,14 +12385,11 @@ ada_catchpoint::print_it (const bpstat *bs) const
catchpoint kinds. */
bool
-ada_catchpoint::print_one (bp_location **last_loc) const
+ada_catchpoint::print_one (bp_location **last_loc, bool print_address_col) const
{
struct ui_out *uiout = current_uiout;
- struct value_print_options opts;
- get_user_print_options (&opts);
-
- if (opts.addressprint)
+ if (print_address_col)
uiout->field_skip ("addr");
annotate_field (5);
diff --git a/gdb/break-catch-exec.c b/gdb/break-catch-exec.c
index 07417ee499c..d0fb38cf768 100644
--- a/gdb/break-catch-exec.c
+++ b/gdb/break-catch-exec.c
@@ -49,7 +49,7 @@ struct exec_catchpoint : public catchpoint
CORE_ADDR bp_addr,
const target_waitstatus &ws) override;
enum print_stop_action print_it (const bpstat *bs) const override;
- bool print_one (bp_location **) const override;
+ bool print_one (bp_location **, bool) const override;
void print_mention () const override;
void print_recreate (struct ui_file *fp) const override;
@@ -110,17 +110,15 @@ exec_catchpoint::print_it (const bpstat *bs) const
}
bool
-exec_catchpoint::print_one (bp_location **last_loc) const
+exec_catchpoint::print_one (bp_location **last_loc,
+ bool print_address_col) const
{
- struct value_print_options opts;
struct ui_out *uiout = current_uiout;
- get_user_print_options (&opts);
-
/* Field 4, the address, is omitted (which makes the columns
not line up too nicely with the headers, but the effect
is relatively readable). */
- if (opts.addressprint)
+ if (print_address_col)
uiout->field_skip ("addr");
annotate_field (5);
uiout->text ("exec");
diff --git a/gdb/break-catch-fork.c b/gdb/break-catch-fork.c
index 1f8deec6a62..9848d7a22cf 100644
--- a/gdb/break-catch-fork.c
+++ b/gdb/break-catch-fork.c
@@ -49,7 +49,7 @@ struct fork_catchpoint : public catchpoint
CORE_ADDR bp_addr,
const target_waitstatus &ws) override;
enum print_stop_action print_it (const bpstat *bs) const override;
- bool print_one (bp_location **) const override;
+ bool print_one (bp_location **, bool print_address_col) const override;
void print_mention () const override;
void print_recreate (struct ui_file *fp) const override;
@@ -136,17 +136,15 @@ fork_catchpoint::print_it (const bpstat *bs) const
/* Implement the "print_one" method for fork catchpoints. */
bool
-fork_catchpoint::print_one (bp_location **last_loc) const
+fork_catchpoint::print_one (bp_location **last_loc,
+ bool print_address_col) const
{
- struct value_print_options opts;
struct ui_out *uiout = current_uiout;
- get_user_print_options (&opts);
-
/* Field 4, the address, is omitted (which makes the columns not
line up too nicely with the headers, but the effect is relatively
readable). */
- if (opts.addressprint)
+ if (print_address_col)
uiout->field_skip ("addr");
annotate_field (5);
const char *name = is_vfork ? "vfork" : "fork";
diff --git a/gdb/break-catch-load.c b/gdb/break-catch-load.c
index 617ee2b694d..9a91fd264d8 100644
--- a/gdb/break-catch-load.c
+++ b/gdb/break-catch-load.c
@@ -56,7 +56,7 @@ struct solib_catchpoint : public catchpoint
const target_waitstatus &ws) override;
void check_status (struct bpstat *bs) override;
enum print_stop_action print_it (const bpstat *bs) const override;
- bool print_one (bp_location **) const override;
+ bool print_one (bp_location **, bool) const override;
void print_mention () const override;
void print_recreate (struct ui_file *fp) const override;
@@ -159,16 +159,14 @@ solib_catchpoint::print_it (const bpstat *bs) const
}
bool
-solib_catchpoint::print_one (bp_location **locs) const
+solib_catchpoint::print_one (bp_location **locs, bool print_address_col) const
{
- struct value_print_options opts;
struct ui_out *uiout = current_uiout;
- get_user_print_options (&opts);
/* Field 4, the address, is omitted (which makes the columns not
line up too nicely with the headers, but the effect is relatively
readable). */
- if (opts.addressprint)
+ if (print_address_col)
{
annotate_field (4);
uiout->field_skip ("addr");
diff --git a/gdb/break-catch-sig.c b/gdb/break-catch-sig.c
index 57a6255dc33..29276a4cf23 100644
--- a/gdb/break-catch-sig.c
+++ b/gdb/break-catch-sig.c
@@ -57,7 +57,7 @@ struct signal_catchpoint : public catchpoint
CORE_ADDR bp_addr,
const target_waitstatus &ws) override;
enum print_stop_action print_it (const bpstat *bs) const override;
- bool print_one (bp_location **) const override;
+ bool print_one (bp_location **, bool) const override;
void print_mention () const override;
void print_recreate (struct ui_file *fp) const override;
bool explains_signal (enum gdb_signal) override;
@@ -213,17 +213,15 @@ signal_catchpoint::print_it (const bpstat *bs) const
/* Implement the "print_one" method for signal catchpoints. */
bool
-signal_catchpoint::print_one (bp_location **last_loc) const
+signal_catchpoint::print_one (bp_location **last_loc,
+ bool print_address_col) const
{
- struct value_print_options opts;
struct ui_out *uiout = current_uiout;
- get_user_print_options (&opts);
-
/* Field 4, the address, is omitted (which makes the columns
not line up too nicely with the headers, but the effect
is relatively readable). */
- if (opts.addressprint)
+ if (print_address_col)
uiout->field_skip ("addr");
annotate_field (5);
diff --git a/gdb/break-catch-syscall.c b/gdb/break-catch-syscall.c
index af55ecb1b5c..b57401b9aaa 100644
--- a/gdb/break-catch-syscall.c
+++ b/gdb/break-catch-syscall.c
@@ -52,7 +52,7 @@ struct syscall_catchpoint : public catchpoint
CORE_ADDR bp_addr,
const target_waitstatus &ws) override;
enum print_stop_action print_it (const bpstat *bs) const override;
- bool print_one (bp_location **) const override;
+ bool print_one (bp_location **, bool) const override;
void print_mention () const override;
void print_recreate (struct ui_file *fp) const override;
@@ -238,17 +238,16 @@ syscall_catchpoint::print_it (const bpstat *bs) const
/* Implement the "print_one" method for syscall catchpoints. */
bool
-syscall_catchpoint::print_one (bp_location **last_loc) const
+syscall_catchpoint::print_one (bp_location **last_loc,
+ bool print_address_col) const
{
- struct value_print_options opts;
struct ui_out *uiout = current_uiout;
struct gdbarch *gdbarch = loc->gdbarch;
- get_user_print_options (&opts);
/* Field 4, the address, is omitted (which makes the columns not
line up too nicely with the headers, but the effect is relatively
readable). */
- if (opts.addressprint)
+ if (print_address_col)
uiout->field_skip ("addr");
annotate_field (5);
diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c
index f15fa531519..251fffdf21f 100644
--- a/gdb/break-catch-throw.c
+++ b/gdb/break-catch-throw.c
@@ -83,7 +83,7 @@ struct exception_catchpoint : public catchpoint
void re_set () override;
enum print_stop_action print_it (const bpstat *bs) const override;
- bool print_one (bp_location **) const override;
+ bool print_one (bp_location **, bool) const override;
void print_mention () const override;
void print_recreate (struct ui_file *fp) const override;
void print_one_detail (struct ui_out *) const override;
@@ -267,14 +267,12 @@ exception_catchpoint::print_it (const bpstat *bs) const
}
bool
-exception_catchpoint::print_one (bp_location **last_loc) const
+exception_catchpoint::print_one (bp_location **last_loc,
+ bool print_address_col) const
{
- struct value_print_options opts;
struct ui_out *uiout = current_uiout;
- get_user_print_options (&opts);
-
- if (opts.addressprint)
+ if (print_address_col)
uiout->field_skip ("addr");
annotate_field (5);
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 724bc778a9b..653dd83ba92 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -316,7 +316,7 @@ struct ranged_breakpoint : public ordinary_breakpoint
const target_waitstatus &ws) override;
int resources_needed (const struct bp_location *) override;
enum print_stop_action print_it (const bpstat *bs) const override;
- bool print_one (bp_location **) const override;
+ bool print_one (bp_location **, bool) const override;
void print_one_detail (struct ui_out *) const override;
void print_mention () const override;
void print_recreate (struct ui_file *fp) const override;
@@ -6161,6 +6161,34 @@ output_thread_groups (struct ui_out *uiout,
}
}
+/* The options for the "info breakpoints" command. */
+
+struct info_breakpoints_opts
+{
+ /* For "-hide-locations". */
+ bool 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. If RAW_LOC, print raw breakpoint locations
instead of going via breakpoint_ops::print_one. This makes "maint
info breakpoints" show the software breakpoint locations of
@@ -6173,15 +6201,13 @@ print_one_breakpoint_location (struct breakpoint *b,
struct bp_location *loc,
int loc_number,
struct bp_location **last_loc,
- int allflag, bool raw_loc)
+ int allflag, bool raw_loc,
+ bool print_address_col)
{
struct command_line *l;
static char bpenables[] = "nynny";
struct ui_out *uiout = current_uiout;
- struct value_print_options opts;
-
- get_user_print_options (&opts);
gdb_assert (!loc || loc_number != 0);
@@ -6227,7 +6253,7 @@ print_one_breakpoint_location (struct breakpoint *b,
/* 5 and 6 */
bool result = false;
- if (!raw_loc && b->print_one (last_loc))
+ if (!raw_loc && b->print_one (last_loc, print_address_col))
result = true;
else
{
@@ -6238,7 +6264,7 @@ print_one_breakpoint_location (struct breakpoint *b,
/* Field 4, the address, is omitted (which makes the columns
not line up too nicely with the headers, but the effect
is relatively readable). */
- if (opts.addressprint)
+ if (print_address_col)
uiout->field_skip ("addr");
annotate_field (5);
uiout->field_string ("what", w->exp_string.get ());
@@ -6246,7 +6272,7 @@ print_one_breakpoint_location (struct breakpoint *b,
else if (!is_catchpoint (b) || is_exception_catchpoint (b)
|| is_ada_exception_catchpoint (b))
{
- if (opts.addressprint)
+ if (print_address_col)
{
annotate_field (4);
if (loc == nullptr)
@@ -6485,6 +6511,18 @@ print_one_breakpoint_location (struct breakpoint *b,
return result;
}
+/* Return whether to print the "Address" column. The "Address" column
+ is suppressed with either "set print address off", or "info
+ breakpoints -hide-locations". */
+
+static bool
+should_print_address_col (const info_breakpoints_opts &ib_opts)
+{
+ struct value_print_options opts;
+ get_user_print_options (&opts);
+ return opts.addressprint && !ib_opts.hide_locations;
+}
+
/* See breakpoint.h. */
bool fix_multi_location_breakpoint_output_globally = false;
@@ -6492,16 +6530,19 @@ 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
= (uiout->test_flags (fix_multi_location_breakpoint_output)
|| fix_multi_location_breakpoint_output_globally);
+ bool print_address_col = should_print_address_col (ib_opts);
+
gdb::optional<ui_out_emit_tuple> bkpt_tuple_emitter (gdb::in_place, uiout, "bkpt");
bool printed = print_one_breakpoint_location (b, NULL, 0, last_loc,
- allflag, false);
+ allflag, false, print_address_col);
/* The mi2 broken format: the main breakpoint tuple ends here, the locations
are outside. */
@@ -6511,7 +6552,7 @@ print_one_breakpoint (struct breakpoint *b,
/* If this breakpoint has custom print function, it's already
printed. Otherwise, print individual locations, if any, and if
not explicitly disabled by the user. */
- if (!printed || allflag)
+ if (!ib_opts.hide_locations && (!printed || allflag))
{
/* Note that while hardware watchpoints have several locations
internally, that's not a property exposed to users.
@@ -6539,7 +6580,8 @@ print_one_breakpoint (struct breakpoint *b,
{
ui_out_emit_tuple loc_tuple_emitter (uiout, NULL);
print_one_breakpoint_location (b, loc, n, last_loc,
- allflag, allflag);
+ allflag, allflag,
+ print_address_col);
n++;
}
}
@@ -6573,8 +6615,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
@@ -6596,9 +6639,9 @@ pending_breakpoint_p (struct breakpoint *b)
/* Print information on breakpoints (including watchpoints and tracepoints).
- If non-NULL, BP_NUM_LIST is a list of numbers and number ranges as
- understood by number_or_range_parser. Only breakpoints included in this
- list are then printed.
+ If non-NULL, ARGS possibly contains options, followed by a list of
+ numbers and number ranges as understood by number_or_range_parser.
+ Only breakpoints included in this list are then printed.
If SHOW_INTERNAL is true, print internal breakpoints.
@@ -6608,18 +6651,30 @@ pending_breakpoint_p (struct breakpoint *b)
Return the total number of breakpoints listed. */
static int
-breakpoint_1 (const char *bp_num_list, bool show_internal,
+breakpoint_1 (const char *args, bool show_internal,
bool (*filter) (const struct breakpoint *))
{
struct bp_location *last_loc = NULL;
int nr_printable_breakpoints;
- struct value_print_options opts;
int print_address_bits = 0;
int print_type_col_width = 14;
struct ui_out *uiout = current_uiout;
bool has_disabled_by_cond_location = false;
- 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] == '-' && (!show_internal || !isdigit (args[1])))
+ gdb::option::error_unrecognized_option_at (args);
+
+ const char *bp_num_list = args;
+
+ bool print_address_col = should_print_address_col (ib_opts);
/* Compute the number of rows in the table, as well as the size
required for address fields. */
@@ -6658,7 +6713,7 @@ breakpoint_1 (const char *bp_num_list, bool show_internal,
{
ui_out_emit_table table_emitter (uiout,
- opts.addressprint ? 6 : 5,
+ print_address_col ? 6 : 5,
nr_printable_breakpoints,
"BreakpointTable");
@@ -6676,7 +6731,7 @@ breakpoint_1 (const char *bp_num_list, bool show_internal,
if (nr_printable_breakpoints > 0)
annotate_field (3);
uiout->table_header (3, ui_left, "enabled", "Enb"); /* 4 */
- if (opts.addressprint)
+ if (print_address_col)
{
if (nr_printable_breakpoints > 0)
annotate_field (4);
@@ -6719,7 +6774,7 @@ breakpoint_1 (const char *bp_num_list, bool show_internal,
show_internal is set. */
if (show_internal || user_breakpoint_p (b))
{
- print_one_breakpoint (b, &last_loc, show_internal);
+ print_one_breakpoint (b, &last_loc, show_internal, ib_opts);
for (bp_location *loc : b->locations ())
if (loc->disabled_by_cond)
has_disabled_by_cond_location = true;
@@ -6757,6 +6812,29 @@ breakpoint_1 (const char *bp_num_list, bool show_internal,
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. */
@@ -9228,7 +9306,7 @@ ranged_breakpoint::print_it (const bpstat *bs) const
/* Implement the "print_one" method for ranged breakpoints. */
bool
-ranged_breakpoint::print_one (bp_location **last_loc) const
+ranged_breakpoint::print_one (bp_location **last_loc, bool print_address_col) const
{
struct bp_location *bl = loc;
struct value_print_options opts;
@@ -9239,7 +9317,7 @@ ranged_breakpoint::print_one (bp_location **last_loc) const
get_user_print_options (&opts);
- if (opts.addressprint)
+ if (print_address_col)
/* We don't print the address range here, it will be printed later
by print_one_detail_ranged_breakpoint. */
uiout->field_skip ("addr");
@@ -14494,10 +14572,13 @@ 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 (info_breakpoints_cmd,
+ info_breakpoints_command_completer);
add_info_alias ("b", info_breakpoints_cmd, 1);
- add_cmd ("breakpoints", class_maintenance, maintenance_info_breakpoints, _("\
+ cmd_list_element *breakpoints_cmd
+ = 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\
@@ -14517,6 +14598,8 @@ is prefixed with \"server \".\n\n\
Convenience variable \"$bpnum\" contains the number of the last\n\
breakpoint set."),
&maintenanceinfolist);
+ set_cmd_completer_handle_brkchars (breakpoints_cmd,
+ info_breakpoints_command_completer);
add_basic_prefix_cmd ("catch", class_breakpoint, _("\
Set catchpoints to catch events."),
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index ac738fd7c2d..93b521c1694 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -685,8 +685,9 @@ struct breakpoint
/* Display information about this breakpoint, for "info
breakpoints". Returns false if this method should use the
- default behavior. */
- virtual bool print_one (bp_location **) const
+ default behavior. PRINT_ADDRESS_COL specifies whether the "addr"
+ column should be printed. */
+ virtual bool print_one (bp_location **, bool print_address_col) const
{
return false;
}
diff --git a/gdb/cli/cli-option.c b/gdb/cli/cli-option.c
index b1794ad4b17..69a2418f7ae 100644
--- a/gdb/cli/cli-option.c
+++ b/gdb/cli/cli-option.c
@@ -174,6 +174,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. */
@@ -189,7 +197,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, "--"))
@@ -235,7 +243,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 26a8da3a5a4..42385979774 100644
--- a/gdb/cli/cli-option.h
+++ b/gdb/cli/cli-option.h
@@ -335,6 +335,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.36.0
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 0/2] info breakpoints improvements
2022-05-19 21:55 [PATCH 0/2] info breakpoints improvements Pedro Alves
2022-05-19 21:55 ` [PATCH 1/2] Always show locations for breakpoints & show canonical location spec Pedro Alves
2022-05-19 21:55 ` [PATCH 2/2] Introduce "info breakpoints -hide-locations" Pedro Alves
@ 2022-05-20 5:57 ` Eli Zaretskii
2022-05-23 17:06 ` Pedro Alves
2022-05-24 8:38 ` Luis Machado
3 siblings, 1 reply; 39+ messages in thread
From: Eli Zaretskii @ 2022-05-20 5:57 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> From: Pedro Alves <pedro@palves.net>
> Date: Thu, 19 May 2022 22:55:50 +0100
>
> we get:
>
> (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 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 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 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
> 4 breakpoint keep y gdb.c:27
> 4.1 y 0x000055555564107b in main(int, char**) at src/gdb/gdb.c:28
> (top-gdb)
I must confess that the new display is much more cluttered, and
includes redundant information, so it's harder to read. It also makes
the important stuff harder to find. Why exactly is this deemed as
improvement, and in particular, why would we want this behavior as the
default? (I won't mind to have this as opt-in behavior, if someone
finds this useful in some situations.)
> Patch #2 introduces an "info breakpoints -hide-locations" option.
> With that, you get just the breakpoint header rows, showing the
> canonical location spec originally used to set the breakpoint, but not
> what the spec expanded to:
>
> (top-gdb) i b -h
> Num Type Disp Enb What
> 1 breakpoint keep y internal_error
> 2 breakpoint keep y -qualified internal_error
> 3 breakpoint keep y errors.c:54
If we want a concise display that only shows the important parts, I'd
lose the "Disp" column.
Thanks.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/2] Always show locations for breakpoints & show canonical location spec
2022-05-19 21:55 ` [PATCH 1/2] Always show locations for breakpoints & show canonical location spec Pedro Alves
@ 2022-05-20 6:45 ` Eli Zaretskii
2022-05-23 17:04 ` [PATCH v2 " Pedro Alves
2022-05-20 7:45 ` [PATCH " Metzger, Markus T
1 sibling, 1 reply; 39+ messages in thread
From: Eli Zaretskii @ 2022-05-20 6:45 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> From: Pedro Alves <pedro@palves.net>
> Date: Thu, 19 May 2022 22:55:51 +0100
>
> Another interesting case where this commit helps is when the user sets
> a breakpoint by line number, and the line corresponds to a comment or
> empty line, or to code that was optimized away. E.g., again when
> debugging GDB:
>
> (top-gdb) b 27
> Breakpoint 4 at 0x469aa6: file src/gdb/gdb.c, line 28.
>
> Note I asked for line 27 but got line 28.
>
> "info breakpoints" currently says:
>
> (top-gdb) info breakpoints 4
> Num Type Disp Enb Address What
> 4 breakpoint keep y 0x0000000000469aa6 in main(int, char**) at src/gdb/gdb.c:28
>
> Again here we lost the info about the original location spec, line 27.
> While after this commit, we get:
>
> (top-gdb) info breakpoints 4
> Num Type Disp Enb Address What
> 4 breakpoint keep y src/gdb/gdb.c:27
> 4.1 y 0x0000000000469aa6 in main(int, char**) at src/gdb/gdb.c:28
Wow! that is soooo confusing. If we want to have this feature, we've
got to put some explanation for the difference in line numbers (which
could be much more than one line), as partr of the display itself.
Something like "(comment line)" after gdb.c:27, for example. I would
even find it confusing to see 4.1 location in this case, since it's
clear that only a single location is required. I realize now that
every breakpoint will from now on have its N.1 line displayed, which
is also a change to the worse, IMO, given the multiple-location
breakpoints we are/were used to until now.
> +@item break @var{location_spec}
> +Set a breakpoint at all the locations the given @var{location_spec}
> +resolves to. @var{location_spec} can specify a function name, a line
> +number, an address of an instruction, and more. @xref{Specify
> +Location}, for a list of all the possible ways to specify a
> +@var{location}. The breakpoint will stop your program just before it
> +executes any of the instructions at the resolved locations' addresses.
> +
> +When using source languages that permit overloading of symbols, such
> +as C@t{++}, a function name may refer to more than one symbol, and
> +thus more than one place to break. @xref{Ambiguous
> +Expressions,,Ambiguous Expressions}, for a discussion of that
> +situation.
> +
> +It is possible that a given function is instantiated more than once in
> +the program, resulting in a breakpoint with multiple resolved
> +locations. This can happen e.g., with inlined functions, or template
> +functions.
This text was written when we showed N.x locations only when needed,
so the text means to explain when a breakpoint will have several
locations instead of just one. But with this changeset, we will be
displaying multiple locations _always_, so this text is no longer
enough. We should augment it with the background for multiple shown
locations even in the cases where the underlying breakpoint is
inserted only at a single address, such as your example of setting a
breakpoint at a location that doesn't have any corresponding code
address.
> +A breakpoint location specification (@pxref{Specify Location}) may be
> +resolved to several locations in your program. E.g., @code{break
> +func} will find a location for each function named @code{func} in the
> +program.
Again, this "e.g." is incomplete and misses the frequent case, where
there will be always 2 "locations", possibly identical, shown in the
table.
> For each code breakpoint and tracepoint (not for
> +watchpoints, nor catchpoints), @value{GDBN} prints a header entry, and
> +then one entry for each resolved location of the breakpoint.
Likewise: "one entry" will be misleading with the new display, because
we will always show at least 2.
> +Enabled breakpoints and breakpoint locations are marked with @samp{y}.
What does "enabled breakpoint location" mean? One cannot enable a
location. We should find some different terminology here for the N.x
breakpoints, maybe something like "breakpoint" for the N.x and
"breakpoint group" for the "parent" breakpoint? You seem to use
"breakpoint header" below, but that term is not explained and not used
consistently. Nor is it a "header", strictly speaking: it's
conceptually an item that represents a group of actual breakpoints.
> +For a breakpoint whose location specification hasn't been resolved to
> +any location yet, this field will contain @samp{<PENDING>}. Such
> +breakpoint won't fire until its location spec is resolved to an actual
> +location, such as e.g., when a shared library that has the symbol or
^^^^^^^^
This should be "address", to avoid even more confusion wrt "location".
(After reading more of your text, I realize that this is the tip of a
very large iceberg, see below.)
> +For a breakpoint header row, the original location specification
> +passed to the breakpoint command. For a breakpoint location row,
> +where the breakpoint location is in the source for your program, as a
> +file and line number.
This should explain the crucial difference between "the original
location specification" and "breakpoint location" used elsewhere in
the text. It is entirely not trivial to grasp the importance of that
distinction. (Another tip of the iceberg.)
> +@value{GDBN} may add new locations to existing breakpoints when new
> +symbols are loaded. For example, if you have a breakpoint in a
> +C@t{++} template function, and a newly loaded shared library has an
> +instantiation of that template, a new location is added to the list of
> +locations for the breakpoint. More on this further below.
I fail to see why this paragraph is important to have. What it says
is trivial, and having it here makes a complex description harder to
understand, because it interrupts the description with unimportant
details.
> +Regulars code breakpoints and tracepoints are displayed in the
^^^^^^^^
Typo.
> +breakpoint table using several rows---one header row per breakpoint,
> +followed by one row for each of the breakpoint's locations. The
> +header row has an empty address column, and the original location
> +specification in the what column. The rows for individual locations
> +contain the addresses for the locations, and show the functions to
> +which those addresses belong. The number column for a location is
> +indented by one space to the right for grouping, and is of the form
> @var{breakpoint-number}.@var{location-number}.
IMO, something like this text should precede the description of the
table contents, because it both provides a high-level overview of how
each breakpoint is displayed, and defines terminology like "header
row", "breakpoint location row", etc. Having it here is "too late".
> set a breakpoint in a shared library at the beginning of your
> debugging session, when the library is not loaded, and when the
> symbols from the library are not available. When you try to set
> -breakpoint, @value{GDBN} will ask you if you want to set
> -a so called @dfn{pending breakpoint}---breakpoint whose address
> -is not yet resolved.
> +a breakpoint, @value{GDBN} will ask you for confirmation of whether
> +you want to set a so called @dfn{pending breakpoint}---a breakpoint with
> +no resolved locations yet.
I'm not sure it is a good idea to talk about "resolved locations",
here and elsewhere. Why not "resolved addresses", as the original
text did? Using "location" here introduces a conceptual ambiguity,
whereby a "location" could be in the source code and also in the
executable code, and that overloading of concepts makes this complex
issue even harder to understand. My suggestion is to use "location"
for source-level location specs, and for nothing else.
It sounds like you want to use the term "breakpoint location" to refer
to those N.x "instances" of the breakpoints, and that is the root
cause of the above terminology overloading. If that is the case,
let's find a better term for "breakpoint location" (which in itself is
a vague and problematic term, since there are actually 2 locations
involved: the source-level location and the address in the code).
> After the program is run, whenever a new shared library is loaded,
> -@value{GDBN} reevaluates all the breakpoints. When a newly loaded
> -shared library contains the symbol or line referred to by some
> -pending breakpoint, that breakpoint is resolved and becomes an
> -ordinary breakpoint. When a library is unloaded, all breakpoints
> -that refer to its symbols or source lines become pending again.
> -
> -This logic works for breakpoints with multiple locations, too. For
> -example, if you have a breakpoint in a C@t{++} template function, and
> -a newly loaded shared library has an instantiation of that template,
> -a new location is added to the list of locations for the breakpoint.
> -
> -Except for having unresolved address, pending breakpoints do not
> +@value{GDBN} reevaluates all the breakpoints' location specs. When a
> +newly loaded shared library contains the symbol or line referred to by
> +some pending breakpoint, a location for the breakpoint is found and
> +the breakpoint becomes an ordinary breakpoint. When a library is
> +unloaded, breakpoint locations that refer to its symbols or source
> +lines are not deleted -- they are instead displayed with a
> +@code{<PENDING>} address. This is so that if the shared library is
> +loaded again, the location's enabled/disabled state is preserved.
This text has the same terminology difficulties I mentioned above.
> @value{GDBN} provides some additional commands for controlling what
> -happens when the @samp{break} command cannot resolve breakpoint
> -address specification to an address:
> +happens when the @samp{break} command cannot resolve its breakpoint
> +location specification to any location:
The "resolve breakpoint location specification to any location" part
uses "location" in two very different senses in the same sentence --
do you see how confusing and hard to understand this is?
> + When @value{GDBN} cannot find any
> +location for the location specification, it queries you whether a
Likewise here.
> +This indicates that when @value{GDBN} cannot resolve any location for
> +the location specification, it should create a pending breakpoint
And here. And elsewhere -- if we don't find a good solution for this
problematic terminology, we will bump into such problems and confuse
the heck out of our users from here to eternity.
> +This indicates that pending breakpoints are not to be created. If
> +@value{GDBN} cannot resolve any location for the location
> +specification, @value{GDBN} throws an error.
I wouldn't use "throws an error" in describing user-facing features.
"Displays an error message", perhaps?
> +breakpoint; This field will not be present if no address can be
^^^
Typo(s).
> +This field is present for ordinary breakpoints and tracepoints.
What is an "ordinary" breakpoint, and how is it different from the
other kinds? (There are other places in the new text that use this
unexplained terminology.)
Thanks.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/2] Introduce "info breakpoints -hide-locations"
2022-05-19 21:55 ` [PATCH 2/2] Introduce "info breakpoints -hide-locations" Pedro Alves
@ 2022-05-20 6:48 ` Eli Zaretskii
0 siblings, 0 replies; 39+ messages in thread
From: Eli Zaretskii @ 2022-05-20 6:48 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> From: Pedro Alves <pedro@palves.net>
> Date: Thu, 19 May 2022 22:55:52 +0100
>
> +Regulars code breakpoints and tracepoints are normally displayed in the
^^^^^^^^
Typo.
Thanks.
^ permalink raw reply [flat|nested] 39+ messages in thread
* RE: [PATCH 1/2] Always show locations for breakpoints & show canonical location spec
2022-05-19 21:55 ` [PATCH 1/2] Always show locations for breakpoints & show canonical location spec Pedro Alves
2022-05-20 6:45 ` Eli Zaretskii
@ 2022-05-20 7:45 ` Metzger, Markus T
2022-05-23 17:05 ` Lancelot SIX
1 sibling, 1 reply; 39+ messages in thread
From: Metzger, Markus T @ 2022-05-20 7:45 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Hello Pedro,
> (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 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 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 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
This change looks very nice to me. It will be very helpful once GDB starts
sliding breakpoints per function and we end up with different source lines
for different instances.
I wouldn't indent the breakpoint instances, though. There is already visual
feedback in the longer breakpoint numbers and the fact that 'type' and 'disp'
are omitted to separate the groups.
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] 39+ messages in thread
* [PATCH v2 1/2] Always show locations for breakpoints & show canonical location spec
2022-05-20 6:45 ` Eli Zaretskii
@ 2022-05-23 17:04 ` Pedro Alves
2022-05-24 13:06 ` Eli Zaretskii
0 siblings, 1 reply; 39+ messages in thread
From: Pedro Alves @ 2022-05-23 17:04 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
Hi Eli,
On 2022-05-20 07:45, Eli Zaretskii wrote:
>
>> +@item break @var{location_spec}
>> +Set a breakpoint at all the locations the given @var{location_spec}
>> +resolves to. @var{location_spec} can specify a function name, a line
>> +number, an address of an instruction, and more. @xref{Specify
>> +Location}, for a list of all the possible ways to specify a
>> +@var{location}. The breakpoint will stop your program just before it
>> +executes any of the instructions at the resolved locations' addresses.
>> +
>> +When using source languages that permit overloading of symbols, such
>> +as C@t{++}, a function name may refer to more than one symbol, and
>> +thus more than one place to break. @xref{Ambiguous
>> +Expressions,,Ambiguous Expressions}, for a discussion of that
>> +situation.
>> +
>> +It is possible that a given function is instantiated more than once in
>> +the program, resulting in a breakpoint with multiple resolved
>> +locations. This can happen e.g., with inlined functions, or template
>> +functions.
>
> This text was written when we showed N.x locations only when needed,
> so the text means to explain when a breakpoint will have several
> locations instead of just one. But with this changeset, we will be
> displaying multiple locations _always_, so this text is no longer
> enough. We should augment it with the background for multiple shown
> locations even in the cases where the underlying breakpoint is
> inserted only at a single address, such as your example of setting a
> breakpoint at a location that doesn't have any corresponding code
> address.
I don't understand what you're actually suggesting. This is the
documentation of the "break" command, not "info breakpoints".
But read on, maybe I clarified it already.
>
>> +A breakpoint location specification (@pxref{Specify Location}) may be
>> +resolved to several locations in your program. E.g., @code{break
>> +func} will find a location for each function named @code{func} in the
>> +program.
>
> Again, this "e.g." is incomplete and misses the frequent case, where
> there will be always 2 "locations", possibly identical, shown in the
> table.
I am making a distinction between a "location specification", and
a "resolved location". So here:
(top-gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y internal_error
1.1 y 0x0000555555f813be in internal_error(char const*, int, char const*, ...) at /home/pedro/gdb/binutils-gdb/src/gdbsupport/errors.cc:51
(top-gdb)
The "What" column of the breakpoint row, which just says "internal_error"
is the location specification, the string the user had input (or rather, a
canonicalized form of it) and gdb uses to match actual locations.
The "What" column of the "1.1" line is the actual found location that matches
the input string.
The "will find a location for each function named func", is talking about more
than info breakpoints. Since this is the part describing the "break" command,
you can already see it here:
(top-gdb) b main
Breakpoint 3 at 0xed06c: main. (24 locations)
^^^^^^^^^^^^
The user typed "main", and gdb found 24 locations. I.e., the single
location specification "main" was resolved to 24 locations in your program.
>
>> For each code breakpoint and tracepoint (not for
>> +watchpoints, nor catchpoints), @value{GDBN} prints a header entry, and
>> +then one entry for each resolved location of the breakpoint.
>
> Likewise: "one entry" will be misleading with the new display, because
> we will always show at least 2.
It says "prints a header entry", so that's 1 entry, "and then one entry FOR EACH
resolved location". I'm not sure what you find confusing here. If the breakpoint
has no locations (a pending breakpoint), that results in one entry for the breakpoint,
and then 0 for its zero locations, so it's obviously incorrect that we will always show
at least 2. If the breakpoint has 1 location, then it results in one entry for the breakpoint,
and 1 entry for its locations; If the breakpoint has 5 location, then it results in one entry
for the breakpoint, and 5 entry for its locations; etc. I think the text covers all that.
This is based on existing text actually, from the text after the table describing the
columns, where we describe multiple locations currently:
"A breakpoint with multiple locations is displayed in the breakpoint table using several
rows—one header row, followed by one row for each breakpoint location. "
I just reused that text before the table so that I could talk about
breakpoint locations in the description of the columns. I just didn't use the
word "row". I've reworded the new text a bit now, using "row" too. Hopefully it reads
clearer that way.
>
>> +Enabled breakpoints and breakpoint locations are marked with @samp{y}.
>
> What does "enabled breakpoint location" mean? One cannot enable a
> location.
Sure we can.
(top-gdb) info breakpoints 3
3 breakpoint keep y main
3.1 y 0x00000000000ed06c in main(int, char**) at /home/pedro/gdb/binutils-gdb/src/gdb/gdb.c:25
3.2 y 0x0000000000866436 in selftests::string_view::capacity_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/capacity/1.cc:167
3.3 y 0x0000000000866855 in selftests::string_view::cons_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/cons/char/1.cc:62
...
(top-gdb) disable 3.1 3.2
(top-gdb) info breakpoints 3
Num Type Disp Enb Address What
3 breakpoint keep y main
3.1 n 0x00000000000ed06c in main(int, char**) at /home/pedro/gdb/binutils-gdb/src/gdb/gdb.c:25
3.2 n 0x0000000000866436 in selftests::string_view::capacity_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/capacity/1.cc:167
3.3 y 0x0000000000866855 in selftests::string_view::cons_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/cons/char/1.cc:62
...
(top-gdb) enable 3.2
(top-gdb) info breakpoints 3
Num Type Disp Enb Address What
3 breakpoint keep y main
3.1 n 0x00000000000ed06c in main(int, char**) at /home/pedro/gdb/binutils-gdb/src/gdb/gdb.c:25
3.2 y 0x0000000000866436 in selftests::string_view::capacity_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/capacity/1.cc:167
3.3 y 0x0000000000866855 in selftests::string_view::cons_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/cons/char/1.cc:62
This is documented in the "Set Breaks" node already, here:
"You cannot delete the individual locations from a breakpoint. However, each location can be individually enabled or
disabled by passing breakpoint-number.location-number as argument to the enable and disable commands."
Note that, as I explained in the commit log of the patch in question, with current master, if the
breakpoint's enabled state is different from the location's enabled state, then GDB is already
presenting a single-location breakpoint in "multi-location" mode. E.g., with current master
as top gdb, debugging another gdb:
(top-gdb) info breakpoints 1
Num Type Disp Enb Address What
1 breakpoint keep y 0x0000000000a2d362 in internal_error(char const*, int, char const*, ...) at /home/pedro/gdb/binutils-gdb/src/gdbsupport/errors.cc:51
(top-gdb) disable 1.1
(top-gdb) info breakpoints 1
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
1.1 n 0x0000000000a2d362 in internal_error(char const*, int, char const*, ...) at /home/pedro/gdb/binutils-gdb/src/gdbsupport/errors.cc:51
(top-gdb)
> We should find some different terminology here for the N.x
> breakpoints, maybe something like "breakpoint" for the N.x and
> "breakpoint group" for the "parent" breakpoint? You seem to use
> "breakpoint header" below, but that term is not explained and not used
> consistently. Nor is it a "header", strictly speaking: it's
> conceptually an item that represents a group of actual breakpoints.
We already have terms for this and we make use of them pervasively. A breakpoint
can have 0 locations (we call this a pending breakpoint), one location, or more than one
locations. IOW, a breakpoint has a list of locations, and that location list can
have 0, 1, N elements. This is already explained in the manual, both for CLI users,
and for MI, and understood by users. I see no point in coming up with new terminology.
This is the current text, you can see it pre-rendered here:
https://sourceware.org/gdb/onlinedocs/gdb/Set-Breaks.html#index-breakpoints_002c-multiple-locations
"A breakpoint with multiple locations is displayed in the breakpoint table using several rows—one header row, followed
by one row for each breakpoint location. The header row has ‘<MULTIPLE>’ in the address column. The rows for
individual locations contain the actual addresses for locations, and show the functions to which those
locations belong. The number column for a location is of the form breakpoint-number.location-number."
>
>> +For a breakpoint whose location specification hasn't been resolved to
>> +any location yet, this field will contain @samp{<PENDING>}. Such
>> +breakpoint won't fire until its location spec is resolved to an actual
>> +location, such as e.g., when a shared library that has the symbol or
> ^^^^^^^^
> This should be "address", to avoid even more confusion wrt "location".
> (After reading more of your text, I realize that this is the tip of a
> very large iceberg, see below.)
>
>> +For a breakpoint header row, the original location specification
>> +passed to the breakpoint command. For a breakpoint location row,
>> +where the breakpoint location is in the source for your program, as a
>> +file and line number.
>
> This should explain the crucial difference between "the original
> location specification" and "breakpoint location" used elsewhere in
> the text. It is entirely not trivial to grasp the importance of that
> distinction. (Another tip of the iceberg.)
OK, I can agree with that. Do note the current manual is already
ambiguous here, for it calls both things "location", and that's it.
So I'm actually proposing to improve things by calling the user-input
text as "location specification", distinct from the actual locations
the specification matches or resolves to.
Even GDB's source code uses "location" for both -- struct bp_location
for the breakpoint locations, the "1.1", "1.2" things, stored in each
struct breakpoint in the breakpoint::loc field, as a linked list:
/* Location(s) associated with this high-level breakpoint. */
bp_location *loc = NULL;
We then have breakpoint::insert_location to physically insert a
location, etc.
And then, we have a different field called "breakpoint::location" for the
breakpoint's location specification:
/* Location we used to set the breakpoint. */
event_location_up location;
This is where the linespec, is stored.
If this patch goes through, I'd like to rename this event_location_up
field to location_spec accordingly.
So here's what I changed in the manual compared to the previous version
of the patch:
- renamed the "Specify Location" node to "Location Specifications".
- In the (now named) "Location Specifications" node, explained
what a location specification is, describing it as like a blueprint,
and how it can match more than one actual location. I moved the pre-existing
examples that were in the "Set Breaks" chapter there. We have a ton
of cross references to this chapter throughout, so having them here centralized
should help.
- added more "info breakpoints" examples, better explaining the
difference between what the "What" column shows for the breakpoint's
row vs the breakpoint's locations rows.
- added an example about "b -qualified", and another "b file.c:line". I think
these will help a lot.
- adjusted a few other "info breakpoints" examples in the manual that I had
missed before.
- several misc minor edits here and there inspired by your comments.
>
>> +@value{GDBN} may add new locations to existing breakpoints when new
>> +symbols are loaded. For example, if you have a breakpoint in a
>> +C@t{++} template function, and a newly loaded shared library has an
>> +instantiation of that template, a new location is added to the list of
>> +locations for the breakpoint. More on this further below.
>
> I fail to see why this paragraph is important to have. What it says
> is trivial, and having it here makes a complex description harder to
> understand, because it interrupts the description with unimportant
> details.
Note this is a rewording of preexisting text. Currently we say:
"This logic works for breakpoints with multiple locations, too. For example, if you have
a breakpoint in a C++ template function, and a newly loaded shared library has an
instantiation of that template, a new location is added to the list of locations
for the breakpoint."
... in the part explaining pending breakpoints. But the fact that we add breakpoint
locations when shared libs are loaded is not specific to pending breakpoints, so
I moved it up right after we say that breakpoints can have multiple locations. If
we don't explain this about new symbols resulting in more locations, then users may
assume that breakpoints only end up with locations that exist at the time the breakpoint
was first set. So I think it's an important thing to say explicitly.
>
>> +Regulars code breakpoints and tracepoints are displayed in the
> ^^^^^^^^
> Typo.
Fixed.
>
>> +breakpoint table using several rows---one header row per breakpoint,
>> +followed by one row for each of the breakpoint's locations. The
>> +header row has an empty address column, and the original location
>> +specification in the what column. The rows for individual locations
>> +contain the addresses for the locations, and show the functions to
>> +which those addresses belong. The number column for a location is
>> +indented by one space to the right for grouping, and is of the form
>> @var{breakpoint-number}.@var{location-number}.
>
> IMO, something like this text should precede the description of the
> table contents, because it both provides a high-level overview of how
> each breakpoint is displayed, and defines terminology like "header
> row", "breakpoint location row", etc. Having it here is "too late".
I actually already did that, that's why I added the paragraph saying:
"For each code breakpoint and tracepoint (not for watchpoints, nor catchpoints),
@value{GDBN} prints a header entry, and then one entry for each resolved
location of the breakpoint.", and finished it with "More on this further below.".
I don't think we should move this text (not the one I added, the one you're
pointing at), because it is talking about the "info breakpoint" columns, after they
are introduced. It's a cyclical dependency, which I had broken with that
short paragraph I just mentioned.
>
>> set a breakpoint in a shared library at the beginning of your
>> debugging session, when the library is not loaded, and when the
>> symbols from the library are not available. When you try to set
>> -breakpoint, @value{GDBN} will ask you if you want to set
>> -a so called @dfn{pending breakpoint}---breakpoint whose address
>> -is not yet resolved.
>> +a breakpoint, @value{GDBN} will ask you for confirmation of whether
>> +you want to set a so called @dfn{pending breakpoint}---a breakpoint with
>> +no resolved locations yet.
>
> I'm not sure it is a good idea to talk about "resolved locations",
> here and elsewhere. Why not "resolved addresses", as the original
> text did? Using "location" here introduces a conceptual ambiguity,
> whereby a "location" could be in the source code and also in the
> executable code, and that overloading of concepts makes this complex
> issue even harder to understand. My suggestion is to use "location"
> for source-level location specs, and for nothing else.
I disagree with that, because that's not how GDB behaves. GDB's user interface
uses "location" for the resolved locations. We call something like "1.3" the
location number. We have that whole section talking about breakpoints with
multiple locations, and it's very clearly talking about those. We have error
messages like:
(gdb) disable 2.200
Bad breakpoint location number '200'
etc. etc. etc.
It's the string that we pass as WHERE, as in "break WHERE" that should get a
different name, IMO, and I propose calling that a "location spec" / " location
specification", just like we already have the term "linespec" too, originally
meaning "line specification" obviously, which then GDB resolves to an actual
line. IOW, I see a location spec as a superset of a linespec (as linespec
is one of the 3 ways you can specify a location).
>
> It sounds like you want to use the term "breakpoint location" to refer
> to those N.x "instances" of the breakpoints, and that is the root
> cause of the above terminology overloading.
That's exactly what the term means, and I'm not inventing that now!
It has always been like that ever since multi-locations support was added
to GDB, at least, and the manual describes exactly that. E.g., from
the current master's manual:
"You cannot delete the individual locations from a breakpoint. However, each location can be individually
enabled or disabled by passing breakpoint-number.location-number as argument to the
enable and disable commands."
"A breakpoint with multiple locations is displayed in the breakpoint table using several rows—one header row,
followed by one row for each breakpoint location. The header row has ‘<MULTIPLE>’ in the address column.
The rows for individual locations contain the actual addresses for locations, and show the functions to which
those locations belong. The number column for a location is of the form breakpoint-number.location-number."
> If that is the case,
> let's find a better term for "breakpoint location" (which in itself is
> a vague and problematic term, since there are actually 2 locations
> involved: the source-level location and the address in the code).
>
>> After the program is run, whenever a new shared library is loaded,
>> -@value{GDBN} reevaluates all the breakpoints. When a newly loaded
>> -shared library contains the symbol or line referred to by some
>> -pending breakpoint, that breakpoint is resolved and becomes an
>> -ordinary breakpoint. When a library is unloaded, all breakpoints
>> -that refer to its symbols or source lines become pending again.
>> -
>> -This logic works for breakpoints with multiple locations, too. For
>> -example, if you have a breakpoint in a C@t{++} template function, and
>> -a newly loaded shared library has an instantiation of that template,
>> -a new location is added to the list of locations for the breakpoint.
>> -
>> -Except for having unresolved address, pending breakpoints do not
>> +@value{GDBN} reevaluates all the breakpoints' location specs. When a
>> +newly loaded shared library contains the symbol or line referred to by
>> +some pending breakpoint, a location for the breakpoint is found and
>> +the breakpoint becomes an ordinary breakpoint. When a library is
>> +unloaded, breakpoint locations that refer to its symbols or source
>> +lines are not deleted -- they are instead displayed with a
>> +@code{<PENDING>} address. This is so that if the shared library is
>> +loaded again, the location's enabled/disabled state is preserved.
>
> This text has the same terminology difficulties I mentioned above.
>
>> @value{GDBN} provides some additional commands for controlling what
>> -happens when the @samp{break} command cannot resolve breakpoint
>> -address specification to an address:
>> +happens when the @samp{break} command cannot resolve its breakpoint
>> +location specification to any location:
>
> The "resolve breakpoint location specification to any location" part
> uses "location" in two very different senses in the same sentence --
> do you see how confusing and hard to understand this is?
I honestly don't... One says "specification", the other one doesn't.
This to me is like saying "resolve an airplane blueprint to an actual airplane."
I used "airplane" twice, but it is not ambiguous.
I clarified this in the now-renamed "Location Specifications" section, please
take a look at the new version.
>
>> + When @value{GDBN} cannot find any
>> +location for the location specification, it queries you whether a
>
> Likewise here.
>
>> +This indicates that when @value{GDBN} cannot resolve any location for
>> +the location specification, it should create a pending breakpoint
>
> And here. And elsewhere -- if we don't find a good solution for this
> problematic terminology, we will bump into such problems and confuse
> the heck out of our users from here to eternity.
>
>> +This indicates that pending breakpoints are not to be created. If
>> +@value{GDBN} cannot resolve any location for the location
>> +specification, @value{GDBN} throws an error.
>
> I wouldn't use "throws an error" in describing user-facing features.
> "Displays an error message", perhaps?
OK. I actually had looked to see if "throw an error" is used in the manual,
as I also wasn't sure whether it would be good, but found it one other
spot, so assumed yes. Saying "throws an error" is more accurate in the
sense that an error also stops command processing. Just prints an error
or displays an error as we typically say throughout doesn't convey that.
Maybe "aborts the command with an error" would be a better way to convey this?
I notice now -- some spots say also "error raised". But I think the
command abortion is more informative here, like so:
@item set breakpoint pending off
This indicates that pending breakpoints are not to be created. If
@value{GDBN} cannot resolve any location for the location
specification, it aborts the breakpoint creation with an error. This
setting does not affect any pending breakpoints previously created.
>
>> +breakpoint; This field will not be present if no address can be
> ^^^
> Typo(s).
>
>> +This field is present for ordinary breakpoints and tracepoints.
>
> What is an "ordinary" breakpoint, and how is it different from the
> other kinds? (There are other places in the new text that use this
> unexplained terminology.)
I've just using the terminology already used in this table. See:
type
The type of the breakpoint. For ordinary breakpoints this will be ‘breakpoint’, but many values are possible.
catch-type
If the type of the breakpoint is ‘catchpoint’, then this indicates the exact type of catchpoint.
Since "info breakpoints" lists all of breakpoints, watchpoints, catchpoint,
tracepoints, "delete" deletes all of these, etc., you can think of all of these
being different types of breakpoints, and indeed that's how things are implemented,
and what the "type" column above suggests, as well as the catch-type's column's description,
seen above.
Thus "ordinary" here refers to actual code breakpoints, as opposed to the generic "breakpoint"
term meaning all of code breakpoints, watchpoints, tracepoints, catchpoints.
Below is the updated patch. Only the documentation changed.
From 49e1502a9c6c1344f6cc449e87ac39d4cef77caf Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Tue, 17 May 2022 13:12:04 +0100
Subject: [PATCH] Always show locations for breakpoints & show canonical
location spec
I've thought for a few years that "info breakpoints" should show BOTH
the canonical location spec behind each breakpoint, and the actual
resolved location(s) where the breakpoint is inserted. It currently
only shows the latter.
Here are for example 3 breakpoints that I set while debugging gdb:
(top-gdb) info breakpoints
Num Type Disp Enb Address What
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. Breakpoints 1 and 2 look like
the same from that output, but really aren't. I'll have forgotten
after a while which was which though. And what's with the third
breakpoint, showing seemingly unrelated functions in its locations?
GDB of course knows all those breakpoints were set differently. It
needs to remember the location specs 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 breakpoints had been created:
(top-gdb) shell cat bpts.cmd
break internal_error
break -qualified internal_error
break errors.c:54
(top-gdb)
Ah, the "-qualified" information for breakpoint 2 was lost from "info
breakpoints" output. And now it's obvious why we have two locations
for breakpoint 3 -- that breakpoint was set by FILE:LINE, and GDB
resolved that to two locations in unrelated functions that happen to
be implemented in files with the same name.
I propose that we show this info in "info breakpoints" too, with a
conceptualy simple change that just removes special cases. To get
there, let's observe a fact -- there are a couple cases where "info
breakpoints" prints the breakpoint's locations in their own rows:
#1 - when the breakpoint has multiple locations:
(gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
1.1 y 0x0000000000a2d874 in func(int) at func.c:51
1.2 y 0x0000000000b2d640 in func(int) at func2.c:51
#2 - when the breakpoint only has one location, but it is disabled,
while the breakpoint itself is enabled. E.g.:
(gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y 0x0000000000a2d874 in func(int) at func.c:51
(gdb) disable 1.1
(gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
1.1 n 0x0000000000a2d874 in func(int) at func.c:51
(the last example shows <MULTIPLE> when there's only one location, but
let's ignore that...)
Note that when we print the locations in a separate row, the "What"
column for the breakpoint header row is empty. That seems to me to be
the perfect spot to put the breakpoint's location spec.
Then, if we make it such that breakpoints always print their locations
in separate rows, even if they only have one location, that space is
always there to use. The result for the confusing example at the top
would be this:
(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 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 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 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
(Note: this looks better on a terminal with color.)
Breakpoints would no longer move from single-location to
multiple-location "display modes" as locations were added/removed
(e.g., when a shared library is loaded), they'd always be in
multi-location display mode. And we'd get rid of the "breakpoint has
a single location that is disabled" special case.
Now, after using this for a while, I came to wish that the output
would give a better visual clue on each group of "breakpoint + its
locations". This commit implements that by simply indenting the
location number by one space, like so:
(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 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 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 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
The need for grouping is clearer if you have breakpoints with dozens
of locations, such as when you do "b main" when debugging GDB.
Another interesting case where this commit helps is when the user sets
a breakpoint by line number, and the line corresponds to a comment or
empty line, or to code that was optimized away. E.g., again when
debugging GDB:
(top-gdb) b 27
Breakpoint 4 at 0x469aa6: file src/gdb/gdb.c, line 28.
Note I asked for line 27 but got line 28.
"info breakpoints" currently says:
(top-gdb) info breakpoints 4
Num Type Disp Enb Address What
4 breakpoint keep y 0x0000000000469aa6 in main(int, char**) at src/gdb/gdb.c:28
Again here we lost the info about the original location spec, line 27.
While after this commit, we get:
(top-gdb) info breakpoints 4
Num Type Disp Enb Address What
4 breakpoint keep y src/gdb/gdb.c:27
4.1 y 0x0000000000469aa6 in main(int, char**) at src/gdb/gdb.c:28
Lancelot is working on a GDB change that will make GDB slide each
breakpoint location's line independently, so we could end up with
something like this, even:
(top-gdb) info breakpoints 4
Num Type Disp Enb Address What
4 breakpoint keep y func.cc:27
4.1 y 0x0000000000469aa6 in func<float>(float) at func.cc:28
4.2 y 0x000000000047abb8 in func<int>(int) at func.cc:30
The output for watchpoints and catchpoints is unmodified, they will
continue being printed like before, as we never print locations for
those:
Num Type Disp Enb Address What
2 catchpoint keep y syscall "<any syscall>"
3 hw watchpoint keep y *main
Before this commit can go in, I'll need to address testsuite fallout,
of course. I expect this will be a significant effort, so I'm hoping
we can come to agreement on whether the GDB change is OK before
wasting time adjusting the testsuite.
Documentation adjustments included. They also clean up a few tangent
things in the area of the docs I was adjusting to better match
reality.
Change-Id: Ic42ad8565e79ca67bfebb22cbb4794ea816fd08b
---
gdb/doc/gdb.texinfo | 384 +++++++++++++++++++++++++++-----------------
gdb/doc/guile.texi | 2 +-
gdb/doc/python.texi | 5 +-
gdb/breakpoint.c | 89 +++++-----
4 files changed, 278 insertions(+), 202 deletions(-)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 90c307a7dd2..f988d9d6bd1 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -2024,7 +2024,7 @@ func<int>() func<float>()
(@value{GDBP}) p 'func<
@end smallexample
-When setting breakpoints however (@pxref{Specify Location}), you don't
+When setting breakpoints however (@pxref{Location Specifications}), you don't
usually need to type a quote before the function name, because
@value{GDBN} understands that you want to set a breakpoint on a
function:
@@ -4344,16 +4344,24 @@ convenience variables.
@table @code
@item break @var{location}
-Set a breakpoint at the given @var{location}, which can specify a
-function name, a line number, or an address of an instruction.
-(@xref{Specify Location}, for a list of all the possible ways to
-specify a @var{location}.) The breakpoint will stop your program just
-before it executes any of the code in the specified @var{location}.
+Set a breakpoint at all the locations the given location specification
+matches. @var{location} can specify a function name, a line number,
+an address of an instruction, and more. @xref{Location
+Specifications}, for the various forms of @var{location}.
-When using source languages that permit overloading of symbols, such as
-C@t{++}, a function name may refer to more than one possible place to break.
-@xref{Ambiguous Expressions,,Ambiguous Expressions}, for a discussion of
-that situation.
+When using source languages that permit overloading of symbols, such
+as C@t{++}, a function name may refer to more than one symbol, and
+thus more than one place to break. @xref{Ambiguous
+Expressions,,Ambiguous Expressions}, for a discussion of that
+situation.
+
+It is possible that a given function is instantiated more than once in
+the program, resulting in a breakpoint with multiple addresses, and
+thus multiple locations. This can happen e.g., with inlined
+functions, or template functions.
+
+The breakpoint will stop your program just before it executes any of
+the instructions at its locations' addresses.
It is also possible to insert a breakpoint that will stop the program
only if a specific thread (@pxref{Thread-Specific Breakpoints})
@@ -4407,11 +4415,11 @@ command:
@smallexample
(@value{GDBP}) info breakpoints
Num Type Disp Enb Address What
-1 breakpoint keep y <MULTIPLE>
+1 breakpoint keep y func
stop only if a == 10
-1.1 N* 0x00000000000011b6 in ...
-1.2 y 0x00000000000011c2 in ...
-1.3 N* 0x00000000000011ce in ...
+ 1.1 N* 0x00000000000011b6 in ...
+ 1.2 y 0x00000000000011c2 in ...
+ 1.3 N* 0x00000000000011ce in ...
(*): Breakpoint condition is invalid at this location.
@end smallexample
@@ -4545,32 +4553,42 @@ optionally be surrounded by spaces.
@cindex @code{$_} and @code{info breakpoints}
@item info breakpoints @r{[}@var{list}@dots{}@r{]}
@itemx info break @r{[}@var{list}@dots{}@r{]}
-Print a table of all breakpoints, watchpoints, and catchpoints set and
-not deleted. Optional argument @var{n} means print information only
-about the specified breakpoint(s) (or watchpoint(s) or catchpoint(s)).
-For each breakpoint, following columns are printed:
+Print a table of all breakpoints, watchpoints, tracepoints, and
+catchpoints set and not deleted. The optional argument @var{n} means
+print information only about the specified breakpoint(s) (or
+watchpoint(s) or tracepoint(s) or catchpoint(s)).
+
+For each code breakpoint and tracepoint (i.e., not for watchpoints,
+nor catchpoints), after printing a row for the breakpoint itself,
+@value{GDBN} prints one row for each of the breakpoint's locations.
+More on this further below.
+
+For each breakpoint (and watchpoint, tracepoint, catchpoint) and
+breakpoint location, the following columns are printed:
@table @emph
@item Breakpoint Numbers
@item Type
-Breakpoint, watchpoint, or catchpoint.
+Breakpoint, watchpoint, tracepoint, or catchpoint. This is empty for
+breakpoint location rows.
@item Disposition
Whether the breakpoint is marked to be disabled or deleted when hit.
+This is empty for breakpoint location rows.
@item Enabled or Disabled
-Enabled breakpoints are marked with @samp{y}. @samp{n} marks breakpoints
-that are not enabled.
+Enabled breakpoints and breakpoint locations are marked with @samp{y}.
+@samp{n} marks breakpoints and breakpoint locations that are not
+enabled. Note individual locations can be enabled/disabled
+independently of their parent breakpoint.
@item Address
-Where the breakpoint is in your program, as a memory address. For a
-pending breakpoint whose address is not yet known, this field will
-contain @samp{<PENDING>}. Such breakpoint won't fire until a shared
-library that has the symbol or line referred by breakpoint is loaded.
-See below for details. A breakpoint with several locations will
-have @samp{<MULTIPLE>} in this field---see below for details.
+Where the breakpoint location is in your program, as a memory address.
+For a breakpoint with no locations, this field will contain
+@samp{<PENDING>}.
@item What
-Where the breakpoint is in the source for your program, as a file and
-line number. For a pending breakpoint, the original string passed to
-the breakpoint command will be listed as it cannot be resolved until
-the appropriate shared library is loaded in the future.
+For a breakpoint header row, this shows the breakpoint's location
+specification, derived from the string passed to the breakpoint
+command. For a breakpoint location row, this shows where the
+breakpoint location is in the source for your program, as a file and
+line number.
@end table
@noindent
@@ -4583,8 +4601,7 @@ its condition evaluation mode in between parentheses.
Breakpoint commands, if any, are listed after that. A pending breakpoint is
allowed to have a condition specified for it. The condition is not parsed for
-validity until a shared library is loaded that allows the pending
-breakpoint to resolve to a valid location.
+validity until the pending breakpoint is resolved to a valid location.
@noindent
@code{info break} with a breakpoint
@@ -4614,46 +4631,85 @@ the breakpoints are conditional, this is even useful
@cindex multiple locations, breakpoints
@cindex breakpoints, multiple locations
-It is possible that a breakpoint corresponds to several locations
-in your program. Examples of this situation are:
+It is possible that a breakpoint corresponds to several locations in
+your program. @value{GDBN} will insert a breakpoint at all the
+locations its location specification resolves to.
+@xref{Location Specifications}.
+
+@value{GDBN} may add new locations to existing breakpoints when new
+symbols are loaded. For example, if you have a breakpoint in a
+C@t{++} template function, and a newly loaded shared library has an
+instantiation of that template, a new location is added to the list of
+locations for the breakpoint. More on this further below.
+
+As mentioned above, regular code breakpoints and tracepoints are
+displayed in the breakpoint table using several rows---one header row
+per breakpoint, followed by one row for each of the breakpoint's
+locations. The header row has an empty address column, and the
+breakpoint's location specification in the what column. The rows for
+individual locations contain the addresses for the locations, and show
+the functions to which those addresses belong. The number column for
+a location is indented by one space to the right for grouping, and is
+of the form @var{breakpoint-number}.@var{location-number}.
-@itemize @bullet
-@item
-Multiple functions in the program may have the same name.
+For example:
-@item
-For a C@t{++} constructor, the @value{NGCC} compiler generates several
-instances of the function body, used in different cases.
+@smallexample
+Num Type Disp Enb Address What
+1 breakpoint keep y foo
+ stop only if i==1
+ breakpoint already hit 1 time
+ 1.1 y 0x080486a2 in void foo<int>() at t.cc:8
+ 1.2 y 0x080486ca in void foo<double>() at t.cc:8
+@end smallexample
-@item
-For a C@t{++} template function, a given line in the function can
-correspond to any number of instantiations.
+Above you can tell from the what column that the breakpoint had been
+created with @code{break foo}, and that @value{GDBN} found two actual
+locations that matched that location specification, each of them a
+different instanciation of the foo function template.
-@item
-For an inlined function, a given source line can correspond to
-several places where that function is inlined.
-@end itemize
+Breakpoints created with the @code{-qualified} option show that fact
+in the breakpoint row's what column, as part of the breakpoint's
+location specification. For example:
+
+@smallexample
+Num Type Disp Enb Address What
+1 breakpoint keep y -qualified func
+ 1.1 y 0x080486a2 in void func() at func.cc:30
+2 breakpoint keep y func
+ 2.1 y 0x080486a2 in void func() at func.cc:30
+@end smallexample
-In all those cases, @value{GDBN} will insert a breakpoint at all
-the relevant locations.
+Note how above both breakpoints resolved to the same location, while
+they show different location specifications. If the program
+subsequently loads a shared library that defines a C@t{++} method
+named @code{A::func()}, then breakpoint 2 will gain a new location,
+but not breakpoint 1, like so:
-A breakpoint with multiple locations is displayed in the breakpoint
-table using several rows---one header row, followed by one row for
-each breakpoint location. The header row has @samp{<MULTIPLE>} in the
-address column. The rows for individual locations contain the actual
-addresses for locations, and show the functions to which those
-locations belong. The number column for a location is of the form
-@var{breakpoint-number}.@var{location-number}.
+@smallexample
+Num Type Disp Enb Address What
+1 breakpoint keep y -qualified func
+ 1.1 y 0x0000080486a2 in void func() at func.cc:30
+2 breakpoint keep y func
+ 2.1 y 0x0000080486a2 in void func() at func.cc:30
+ 2.2 y 0x7ffff6caa000 in int A::func(int) at shlib.cc:102
+@end smallexample
-For example:
+If you set a breakpoint by line number, @value{GDBN} may determine
+that there is no actual code at the given line number, and arm the
+breakpoint at the next line number with code instead. This can happen
+for example if the line number you specify corresponds to a source
+location that only contains comments, or contains source code that was
+completely optimized out. You can identify such a situation happened
+in @code{info breakpoints} output, as the location specification for
+the breakpoint as shown in the what column gives you the line you
+requested, while the breakpoint's location row shows the line the
+breakpoint was actually planted at:
@smallexample
Num Type Disp Enb Address What
-1 breakpoint keep y <MULTIPLE>
- stop only if i==1
- breakpoint already hit 1 time
-1.1 y 0x080486a2 in void foo<int>() at t.cc:8
-1.2 y 0x080486ca in void foo<double>() at t.cc:8
+1 breakpoint keep y file.c:18
+ 1.1 y 0x08028832 in void func() at path/to/file.c:20
@end smallexample
You cannot delete the individual locations from a breakpoint. However,
@@ -4677,53 +4733,54 @@ any shared library is loaded or unloaded. Typically, you would
set a breakpoint in a shared library at the beginning of your
debugging session, when the library is not loaded, and when the
symbols from the library are not available. When you try to set
-breakpoint, @value{GDBN} will ask you if you want to set
-a so called @dfn{pending breakpoint}---breakpoint whose address
-is not yet resolved.
+a breakpoint, @value{GDBN} will ask you for confirmation of whether
+you want to set a so called @dfn{pending breakpoint}---a breakpoint with
+no resolved locations yet.
After the program is run, whenever a new shared library is loaded,
-@value{GDBN} reevaluates all the breakpoints. When a newly loaded
-shared library contains the symbol or line referred to by some
-pending breakpoint, that breakpoint is resolved and becomes an
-ordinary breakpoint. When a library is unloaded, all breakpoints
-that refer to its symbols or source lines become pending again.
-
-This logic works for breakpoints with multiple locations, too. For
-example, if you have a breakpoint in a C@t{++} template function, and
-a newly loaded shared library has an instantiation of that template,
-a new location is added to the list of locations for the breakpoint.
-
-Except for having unresolved address, pending breakpoints do not
+@value{GDBN} reevaluates all the breakpoints' location specs. When a
+newly loaded shared library contains the symbol or line referred to by
+some pending breakpoint, a location for the breakpoint is found and
+the breakpoint becomes an ordinary breakpoint. When a library is
+unloaded, breakpoint locations that refer to its symbols or source
+lines are not deleted -- they are instead displayed with a
+@code{<PENDING>} address. This is so that if the shared library is
+loaded again, the location's enabled/disabled state is preserved.
+
+Except for having no resolved locations, pending breakpoints do not
differ from regular breakpoints. You can set conditions or commands,
enable and disable them and perform other breakpoint operations.
@value{GDBN} provides some additional commands for controlling what
-happens when the @samp{break} command cannot resolve breakpoint
-address specification to an address:
+happens when the @samp{break} command cannot resolve its breakpoint
+location specification to any location:
@kindex set breakpoint pending
@kindex show breakpoint pending
@table @code
@item set breakpoint pending auto
-This is the default behavior. When @value{GDBN} cannot find the breakpoint
-location, it queries you whether a pending breakpoint should be created.
+This is the default behavior. When @value{GDBN} cannot find any
+location for the location specification, it queries you whether a
+pending breakpoint should be created.
@item set breakpoint pending on
-This indicates that an unrecognized breakpoint location should automatically
-result in a pending breakpoint being created.
+This indicates that when @value{GDBN} cannot resolve any location for
+the location specification, it should create a pending breakpoint
+without asking for confirmation.
@item set breakpoint pending off
-This indicates that pending breakpoints are not to be created. Any
-unrecognized breakpoint location results in an error. This setting does
-not affect any pending breakpoints previously created.
+This indicates that pending breakpoints are not to be created. If
+@value{GDBN} cannot resolve any location for the location
+specification, it aborts the breakpoint creation with an error. This
+setting does not affect any pending breakpoints previously created.
@item show breakpoint pending
Show the current behavior setting for creating pending breakpoints.
@end table
The settings above only affect the @code{break} command and its
-variants. Once breakpoint is set, it will be automatically updated
-as shared libraries are loaded and unloaded.
+variants. Once a breakpoint is created, it will be automatically
+updated as shared libraries are loaded and unloaded.
@cindex automatic hardware breakpoints
For some targets, @value{GDBN} can automatically decide if hardware or
@@ -5444,8 +5501,8 @@ breakpoint where your program just stopped.
@item clear @var{location}
Delete any breakpoints set at the specified @var{location}.
-@xref{Specify Location}, for the various forms of @var{location}; the
-most useful ones are listed below:
+@xref{Location Specifications}, for the various forms of
+@var{location}; the most useful ones are listed below:
@table @code
@item clear @var{function}
@@ -5852,12 +5909,14 @@ you could do the following:
(gdb) set dprintf-style call
(gdb) set dprintf-function fprintf
(gdb) set dprintf-channel mylog
-(gdb) dprintf 25,"at line 25, glob=%d\n",glob
+(gdb) dprintf main.c:25,"at line 25, glob=%d\n",glob
Dprintf 1 at 0x123456: file main.c, line 25.
(gdb) info break
-1 dprintf keep y 0x00123456 in main at main.c:25
+Num Type Disp Enb Address What
+1 dprintf keep y main.c:25
call (void) fprintf (mylog,"at line 25, glob=%d\n",glob)
continue
+ 1.1 y 0x00123456 in main at main.c:25
(gdb)
@end example
@@ -6304,7 +6363,7 @@ argument.
@itemx u @var{location}
Continue running your program until either the specified @var{location} is
reached, or the current stack frame returns. The location is any of
-the forms described in @ref{Specify Location}.
+the forms described in @ref{Location Specifications}.
This form of the command uses temporary breakpoints, and
hence is quicker than @code{until} without an argument. The specified
location is actually reached only if it is in the current frame. This
@@ -6329,7 +6388,7 @@ invocations have returned.
@item advance @var{location}
Continue running the program up to the given @var{location}. An argument is
required, which should be of one of the forms described in
-@ref{Specify Location}.
+@ref{Location Specifications}.
Execution will also stop upon exit from the current stack
frame. This command is similar to @code{until}, but @code{advance} will
not skip over recursive function calls, and the target location doesn't
@@ -6429,7 +6488,7 @@ A more flexible solution is to execute @kbd{skip boring}. This instructs
@code{foo}.
Functions may be skipped by providing either a function name, linespec
-(@pxref{Specify Location}), regular expression that matches the function's
+(@pxref{Location Specifications}), regular expression that matches the function's
name, file name or a @code{glob}-style pattern that matches the file name.
On Posix systems the form of the regular expression is
@@ -6466,7 +6525,7 @@ over when stepping.
@itemx -fu @var{linespec}
Functions named by @var{linespec} or the function containing the line
named by @var{linespec} will be skipped over when stepping.
-@xref{Specify Location}.
+@xref{Location Specifications}.
@item -rfunction @var{regexp}
@itemx -rfu @var{regexp}
@@ -6499,7 +6558,7 @@ will be skipped.
@item skip function @r{[}@var{linespec}@r{]}
After running this command, the function named by @var{linespec} or the
function containing the line named by @var{linespec} will be skipped over when
-stepping. @xref{Specify Location}.
+stepping. @xref{Location Specifications}.
If you do not specify @var{linespec}, the function you're currently debugging
will be skipped.
@@ -7130,9 +7189,8 @@ breakpoints on all threads, or on a particular thread.
@kindex break @dots{} thread @var{thread-id}
@item break @var{location} thread @var{thread-id}
@itemx break @var{location} thread @var{thread-id} if @dots{}
-@var{location} specifies source lines; there are several ways of
-writing them (@pxref{Specify Location}), but the effect is always to
-specify some source line.
+@var{location} specifies a location or locations in your program's
+code. @xref{Location Specifications}, for details.
Use the qualifier @samp{thread @var{thread-id}} with a breakpoint command
to specify that you only want @value{GDBN} to stop the program when a
@@ -8932,7 +8990,7 @@ prefer to use Emacs facilities to view source; see @ref{Emacs, ,Using
@menu
* List:: Printing source lines
-* Specify Location:: How to specify code locations
+* Location Specifications:: How to specify code locations
* Edit:: Editing source files
* Search:: Searching source files
* Source Path:: Specifying source directories
@@ -8948,7 +9006,7 @@ prefer to use Emacs facilities to view source; see @ref{Emacs, ,Using
To print lines from a source file, use the @code{list} command
(abbreviated @code{l}). By default, ten lines are printed.
There are several ways to specify what part of the file you want to
-print; see @ref{Specify Location}, for the full list.
+print; see @ref{Location Specifications}, for the full list.
Here are the forms of the @code{list} command most commonly used:
@@ -8997,7 +9055,7 @@ each repetition moves up in the source file.
In general, the @code{list} command expects you to supply zero, one or two
@dfn{locations}. Locations specify source lines; there are several ways
-of writing them (@pxref{Specify Location}), but the effect is always
+of writing them (@pxref{Location Specifications}), but the effect is always
to specify some source line.
Here is a complete description of the possible arguments for @code{list}:
@@ -9028,17 +9086,41 @@ Print lines just before the lines last printed.
As described in the preceding table.
@end table
-@node Specify Location
-@section Specifying a Location
+@node Location Specifications
+@section Location Specifications
@cindex specifying location
@cindex location
@cindex source location
Several @value{GDBN} commands accept arguments that specify a location
of your program's code. Since @value{GDBN} is a source-level
-debugger, a location usually specifies some line in the source code.
-Locations may be specified using three different formats:
-linespec locations, explicit locations, or address locations.
+debugger, a location specification usually indicates some line in the
+source code, but it can also indicate a function name, an address, and
+more.
+
+A location specification serves as a blueprint, and it may match more
+than one actual location in your program. Examples of this situation
+are:
+
+@itemize @bullet
+@item
+Multiple functions in the program may have the same name.
+
+@item
+For a C@t{++} constructor, the @value{NGCC} compiler generates several
+instances of the function body, used in different cases.
+
+@item
+For a C@t{++} template function, a given line in the function can
+correspond to any number of instantiations.
+
+@item
+For an inlined function, a given source line can correspond to several
+places where that function is inlined.
+@end itemize
+
+Locations may be specified using three different formats: linespec
+locations, explicit locations, or address locations.
@menu
* Linespec Locations:: Linespec locations
@@ -9260,9 +9342,9 @@ want to print if you want to see other parts of the program:
@item edit @var{location}
Edit the source file specified by @code{location}. Editing starts at
that @var{location}, e.g., at the specified source line of the
-specified file. @xref{Specify Location}, for all the possible forms
-of the @var{location} argument; here are the forms of the @code{edit}
-command most commonly used:
+specified file. @xref{Location Specifications}, for all the possible
+forms of the @var{location} argument; here are the forms of the
+@code{edit} command most commonly used:
@table @code
@item edit @var{number}
@@ -9645,8 +9727,8 @@ well as hex.
@itemx info line @var{location}
Print the starting and ending addresses of the compiled code for
source line @var{location}. You can specify source lines in any of
-the ways documented in @ref{Specify Location}. With no @var{location}
-information about the current source line is printed.
+the ways documented in @ref{Location Specifications}. With no
+@var{location} information about the current source line is printed.
@end table
For example, we can use @code{info line} to discover the location of
@@ -9870,10 +9952,10 @@ Dump of assembler code from 0x400281 to 0x40028b:
End of assembler dump.
@end smallexample
-Addresses cannot be specified as a location (@pxref{Specify Location}).
-So, for example, if you want to disassemble function @code{bar}
-in file @file{foo.c}, you must type @samp{disassemble 'foo.c'::bar}
-and not @samp{disassemble foo.c:bar}.
+Addresses cannot be specified as a location (@pxref{Location
+Specifications}). So, for example, if you want to disassemble
+function @code{bar} in file @file{foo.c}, you must type
+@samp{disassemble 'foo.c'::bar} and not @samp{disassemble foo.c:bar}.
Some architectures have more than one commonly-used set of instruction
mnemonics or other syntax.
@@ -14466,8 +14548,8 @@ conditions and actions.
@kindex trace
@item trace @var{location}
The @code{trace} command is very similar to the @code{break} command.
-Its argument @var{location} can be any valid location.
-@xref{Specify Location}. The @code{trace} command defines a tracepoint,
+Its argument @var{location} can be any valid location specification.
+@xref{Location Specifications}. The @code{trace} command defines a tracepoint,
which is a point in the target program where the debugger will briefly stop,
collect some data, and then allow the program to continue. Setting a tracepoint
or changing its actions takes effect immediately if the remote stub
@@ -14986,21 +15068,22 @@ the state about installed on target of each location
@smallexample
(@value{GDBP}) @b{info trace}
Num Type Disp Enb Address What
-1 tracepoint keep y 0x0804ab57 in foo() at main.cxx:7
+1 tracepoint keep y foo1
while-stepping 20
collect globfoo, $regs
end
collect globfoo2
end
pass count 1200
-2 tracepoint keep y <MULTIPLE>
+ 1.1 y 0x0804ab57 in foo1() at main.cxx:7
+2 tracepoint keep y func4
collect $eip
-2.1 y 0x0804859c in func4 at change-loc.h:35
+ 2.1 y 0x0804859c in func4 at change-loc.h:35
installed on target
-2.2 y 0xb7ffc480 in func4 at change-loc.h:35
+ 2.2 y 0xb7ffc480 in func4 at change-loc.h:35
installed on target
-2.3 y <PENDING> set_tracepoint
-3 tracepoint keep y 0x080485b1 in foo at change-loc.c:29
+3 tracepoint keep y foo2
+ 3.1 y 0x080485b1 in foo2 at change-loc.c:29
not installed on target
(@value{GDBP})
@end smallexample
@@ -16946,7 +17029,8 @@ tag. For example:
Breakpoint 2 at 0x40060d: file main.cc, line 10.
(gdb) info breakpoints
Num Type Disp Enb Address What
-1 breakpoint keep y 0x0040060d in function[abi:cxx11](int)
+1 breakpoint keep y function(int)
+ 1.1 y 0x0040060d in function[abi:cxx11](int)
at main.cc:10
@end smallexample
@@ -17426,9 +17510,9 @@ peculiarities and holes to be aware of.
@itemize @bullet
@item
-Linespecs (@pxref{Specify Location}) are never relative to the current
-crate. Instead, they act as if there were a global namespace of
-crates, somewhat similar to the way @code{extern crate} behaves.
+Linespecs (@pxref{Location Specifications}) are never relative to the
+current crate. Instead, they act as if there were a global namespace
+of crates, somewhat similar to the way @code{extern crate} behaves.
That is, if @value{GDBN} is stopped at a breakpoint in a function in
crate @samp{A}, module @samp{B}, then @code{break B::f} will attempt
@@ -18757,7 +18841,7 @@ Flags @code{-c} and @code{-s} cannot be used together.
These commands are like the @code{break @dots{} thread @dots{}}
command (@pxref{Thread Stops}). The
@var{location} argument specifies source lines, as described
-in @ref{Specify Location}.
+in @ref{Location Specifications}.
Use the qualifier @samp{task @var{taskno}} with a breakpoint command
to specify that you only want @value{GDBN} to stop the program when a
@@ -19520,10 +19604,11 @@ These commands can be used to enable or disable type printers.
@cindex local variables
@item info scope @var{location}
List all the variables local to a particular scope. This command
-accepts a @var{location} argument---a function name, a source line, or
-an address preceded by a @samp{*}, and prints all the variables local
-to the scope defined by that location. (@xref{Specify Location}, for
-details about supported forms of @var{location}.) For example:
+accepts a location specification argument---a function name, a source
+line, or an address preceded by a @samp{*}, and prints all the
+variables local to the scope defined by that location specification.
+(@xref{Location Specifications}, for details about supported forms of
+@var{location}.) For example:
@smallexample
(@value{GDBP}) @b{info scope command_line_handler}
@@ -20114,8 +20199,8 @@ an address of your own choosing, with the following commands:
@item jump @var{location}
@itemx j @var{location}
Resume execution at @var{location}. Execution stops again immediately
-if there is a breakpoint there. @xref{Specify Location}, for a description
-of the different forms of @var{location}. It is common
+if there is a breakpoint there. @xref{Location Specifications}, for a
+description of the different forms of @var{location}. It is common
practice to use the @code{tbreak} command in conjunction with
@code{jump}. @xref{Set Breaks, ,Setting Breakpoints}.
@@ -25356,7 +25441,7 @@ use the @code{break-range} command.
Set a breakpoint for an address range given by
@var{start-location} and @var{end-location}, which can specify a function name,
a line number, an offset of lines from the current line or from the start
-location, or an address of an instruction (see @ref{Specify Location},
+location, or an address of an instruction (see @ref{Location Specifications},
for a list of all the possible ways to specify a @var{location}.)
The breakpoint will stop execution of the inferior whenever it
executes an instruction at any address within the specified range,
@@ -30544,11 +30629,10 @@ value is @samp{y}, or disabled, in which case the value is @samp{n}.
Note that this is not the same as the field @code{enable}.
@item addr
-The address of the breakpoint. This may be a hexidecimal number,
+The address of the breakpoint. This may be a hexadecimal number,
giving the address; or the string @samp{<PENDING>}, for a pending
-breakpoint; or the string @samp{<MULTIPLE>}, for a breakpoint with
-multiple locations. This field will not be present if no address can
-be determined. For example, a watchpoint does not have an address.
+breakpoint; This field will not be present if no address can be
+determined. For example, a watchpoint does not have an address.
@item addr_flags
Optional field containing any flags related to the address. These flags are
@@ -30629,16 +30713,14 @@ is not.
Some extra data, the exact contents of which are type-dependent.
@item locations
-This field is present if the breakpoint has multiple locations. It is also
-exceptionally present if the breakpoint is enabled and has a single, disabled
-location.
+This field is present for ordinary breakpoints and tracepoints.
The value is a list of locations. The format of a location is described below.
@end table
-A location in a multi-location breakpoint is represented as a tuple with the
-following fields:
+Each location in an ordinary breakpoint or tracepoint is represented
+as a tuple with the following fields:
@table @code
@@ -30660,7 +30742,7 @@ at this location.
@end table
@item addr
-The address of this location as an hexidecimal number.
+The address of this location as an hexadecimal number.
@item addr_flags
Optional field containing any flags related to the address. These flags are
@@ -32542,8 +32624,8 @@ fullname="/home/foo/bar/try.c",line="13",arch="i386:x86_64"@}
@end smallexample
Resumes execution of the inferior program at the location specified by
-parameter. @xref{Specify Location}, for a description of the
-different forms of @var{location}.
+the parameter. @xref{Location Specifications}, for a description of
+the different forms of @var{location}.
@subsubheading @value{GDBN} Command
@@ -34925,7 +35007,7 @@ next trace frame that corresponds to a tracepoint at an address outside
the specified range. Both bounds are considered to be inside the range.
@item line
-Line specification is required as parameter. @xref{Specify Location}.
+Line specification is required as parameter. @xref{Location Specifications}.
Finds next trace frame that corresponds to a tracepoint at
the specified location.
diff --git a/gdb/doc/guile.texi b/gdb/doc/guile.texi
index 3c517230929..63916eed181 100644
--- a/gdb/doc/guile.texi
+++ b/gdb/doc/guile.texi
@@ -1965,7 +1965,7 @@ This constant means that filename completion should be performed.
@item COMPLETE_LOCATION
This constant means that location completion should be done.
-@xref{Specify Location}.
+@xref{Location Specifications}.
@item COMPLETE_COMMAND
This constant means that completion should examine @value{GDBN}
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index cb5283e03c0..f933c7d30c9 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -577,7 +577,8 @@ either @code{None} or another tuple that contains all the locations
that match the expression represented as @code{gdb.Symtab_and_line}
objects (@pxref{Symbol Tables In Python}). If @var{expression} is
provided, it is decoded the way that @value{GDBN}'s inbuilt
-@code{break} or @code{edit} commands do (@pxref{Specify Location}).
+@code{break} or @code{edit} commands do (@pxref{Location
+Specifications}).
@end defun
@defun gdb.prompt_hook (current_prompt)
@@ -4186,7 +4187,7 @@ This constant means that filename completion should be performed.
@vindex COMPLETE_LOCATION
@item gdb.COMPLETE_LOCATION
This constant means that location completion should be done.
-@xref{Specify Location}.
+@xref{Location Specifications}.
@vindex COMPLETE_COMMAND
@item gdb.COMPLETE_COMMAND
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 9abc1443d96..60c34c4d2f8 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -6179,42 +6179,36 @@ print_one_breakpoint_location (struct breakpoint *b,
static char bpenables[] = "nynny";
struct ui_out *uiout = current_uiout;
- int header_of_multiple = 0;
- int part_of_multiple = (loc != NULL);
struct value_print_options opts;
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 || b->loc->disabled_by_cond)))
- header_of_multiple = 1;
- if (loc == NULL)
- loc = b->loc;
annotate_record ();
/* 1 */
annotate_field (0);
- if (part_of_multiple)
- uiout->field_fmt ("number", "%d.%d", b->number, loc_number);
+ if (loc != nullptr)
+ {
+ if (uiout->is_mi_like_p ())
+ uiout->field_fmt ("number", "%d.%d", b->number, loc_number);
+ else
+ uiout->field_fmt ("number", " %d.%d", b->number, loc_number);
+ }
else
uiout->field_signed ("number", b->number);
/* 2 */
annotate_field (1);
- if (part_of_multiple)
+ if (loc != nullptr)
uiout->field_skip ("type");
else
uiout->field_string ("type", bptype_string (b->type));
/* 3 */
annotate_field (2);
- if (part_of_multiple)
+ if (loc != nullptr)
uiout->field_skip ("disp");
else
uiout->field_string ("disp", bpdisp_text (b->disposition));
@@ -6225,7 +6219,7 @@ print_one_breakpoint_location (struct breakpoint *b,
display "N*" on CLI, where "*" refers to a footnote below the
table. For MI, simply display a "N" without a footnote. */
const char *N = (uiout->is_mi_like_p ()) ? "N" : "N*";
- if (part_of_multiple)
+ if (loc != nullptr)
uiout->field_string ("enabled", (loc->disabled_by_cond ? N
: (loc->enabled ? "y" : "n")));
else
@@ -6255,9 +6249,8 @@ print_one_breakpoint_location (struct breakpoint *b,
if (opts.addressprint)
{
annotate_field (4);
- if (header_of_multiple)
- uiout->field_string ("addr", "<MULTIPLE>",
- metadata_style.style ());
+ if (loc == nullptr)
+ uiout->field_skip ("addr");
else if (b->loc == NULL || loc->shlib_disabled)
uiout->field_string ("addr", "<PENDING>",
metadata_style.style ());
@@ -6266,14 +6259,22 @@ print_one_breakpoint_location (struct breakpoint *b,
loc->gdbarch, loc->address);
}
annotate_field (5);
- if (!header_of_multiple)
+ if (loc == nullptr)
+ {
+ if (b->location != nullptr)
+ uiout->field_string
+ ("what", event_location_to_string (b->location.get ()));
+ else
+ uiout->field_skip ("what");
+ }
+ else
print_breakpoint_location (b, loc);
if (b->loc)
*last_loc = b->loc;
}
}
- if (loc != NULL && !header_of_multiple)
+ if (loc != nullptr)
{
std::vector<int> inf_nums;
int mi_only = 1;
@@ -6296,8 +6297,7 @@ print_one_breakpoint_location (struct breakpoint *b,
mi_only = 0;
output_thread_groups (uiout, "thread-groups", inf_nums, mi_only);
}
-
- if (!part_of_multiple)
+ else
{
if (b->thread != -1)
{
@@ -6315,10 +6315,10 @@ print_one_breakpoint_location (struct breakpoint *b,
uiout->text ("\n");
- if (!part_of_multiple)
+ if (loc == nullptr)
b->print_one_detail (uiout);
- if (part_of_multiple && frame_id_p (b->frame_id))
+ if (loc != nullptr && frame_id_p (b->frame_id))
{
annotate_field (6);
uiout->text ("\tstop only in stack frame at ");
@@ -6329,7 +6329,7 @@ print_one_breakpoint_location (struct breakpoint *b,
uiout->text ("\n");
}
- if (!part_of_multiple && b->cond_string)
+ if (loc == nullptr && b->cond_string)
{
annotate_field (7);
if (is_tracepoint (b))
@@ -6351,7 +6351,7 @@ print_one_breakpoint_location (struct breakpoint *b,
uiout->text ("\n");
}
- if (!part_of_multiple && b->thread != -1)
+ if (loc == nullptr && b->thread != -1)
{
/* FIXME should make an annotation for this. */
uiout->text ("\tstop only in thread ");
@@ -6366,7 +6366,7 @@ print_one_breakpoint_location (struct breakpoint *b,
uiout->text ("\n");
}
- if (!part_of_multiple)
+ if (loc == nullptr)
{
if (b->hit_count)
{
@@ -6392,7 +6392,7 @@ print_one_breakpoint_location (struct breakpoint *b,
}
}
- if (!part_of_multiple && b->ignore_count)
+ if (loc == nullptr && b->ignore_count)
{
annotate_field (8);
uiout->message ("\tignore next %pF hits\n",
@@ -6402,7 +6402,7 @@ print_one_breakpoint_location (struct breakpoint *b,
/* Note that an enable count of 1 corresponds to "enable once"
behavior, which is reported by the combination of enablement and
disposition, so we don't need to mention it here. */
- if (!part_of_multiple && b->enable_count > 1)
+ if (loc == nullptr && b->enable_count > 1)
{
annotate_field (8);
uiout->text ("\tdisable after ");
@@ -6416,7 +6416,7 @@ print_one_breakpoint_location (struct breakpoint *b,
uiout->text (" hits\n");
}
- if (!part_of_multiple && is_tracepoint (b))
+ if (loc == nullptr && is_tracepoint (b))
{
struct tracepoint *tp = (struct tracepoint *) b;
@@ -6429,7 +6429,7 @@ print_one_breakpoint_location (struct breakpoint *b,
}
l = b->commands ? b->commands.get () : NULL;
- if (!part_of_multiple && l)
+ if (loc == nullptr && l)
{
annotate_field (9);
ui_out_emit_tuple tuple_emitter (uiout, "script");
@@ -6440,7 +6440,7 @@ print_one_breakpoint_location (struct breakpoint *b,
{
struct tracepoint *t = (struct tracepoint *) b;
- if (!part_of_multiple && t->pass_count)
+ if (loc == nullptr && t->pass_count)
{
annotate_field (10);
uiout->text ("\tpass count ");
@@ -6450,7 +6450,7 @@ print_one_breakpoint_location (struct breakpoint *b,
/* Don't display it when tracepoint or tracepoint location is
pending. */
- if (!header_of_multiple && loc != NULL && !loc->shlib_disabled)
+ if (loc != nullptr && !loc->shlib_disabled)
{
annotate_field (11);
@@ -6468,7 +6468,7 @@ print_one_breakpoint_location (struct breakpoint *b,
}
}
- if (uiout->is_mi_like_p () && !part_of_multiple)
+ if (uiout->is_mi_like_p () && loc == nullptr)
{
if (is_watchpoint (b))
{
@@ -6513,25 +6513,18 @@ print_one_breakpoint (struct breakpoint *b,
locations, if any. */
if (!printed || allflag)
{
- /* 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
+ /* Note that while hardware watchpoints have several locations
internally, that's not a property exposed to users.
Likewise, while catchpoints may be implemented with
breakpoints (e.g., catch throw), that's not a property
exposed to users. We do however display the internal
breakpoint locations with "maint info breakpoints". */
- if (!is_hardware_watchpoint (b)
- && (!is_catchpoint (b) || is_exception_catchpoint (b)
- || is_ada_exception_catchpoint (b))
- && (allflag
- || (b->loc && (b->loc->next
- || !b->loc->enabled
- || b->loc->disabled_by_cond))))
+ if (!is_watchpoint (b)
+ && (!is_catchpoint (b)
+ || ((is_exception_catchpoint (b)
+ || is_ada_exception_catchpoint (b))
+ && allflag)))
{
gdb::optional<ui_out_emit_list> locations_list;
base-commit: ee3272d472e864bcbe86d6c647d7b48df715c72b
--
2.36.0
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/2] Always show locations for breakpoints & show canonical location spec
2022-05-20 7:45 ` [PATCH " Metzger, Markus T
@ 2022-05-23 17:05 ` Lancelot SIX
0 siblings, 0 replies; 39+ messages in thread
From: Lancelot SIX @ 2022-05-23 17:05 UTC (permalink / raw)
To: Metzger, Markus T; +Cc: Pedro Alves, gdb-patches
On Fri, May 20, 2022 at 07:45:09AM +0000, Metzger, Markus T via Gdb-patches wrote:
> Hello Pedro,
>
> > (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 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 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 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
>
> This change looks very nice to me. It will be very helpful once GDB starts
> sliding breakpoints per function and we end up with different source lines
> for different instances.
>
> I wouldn't indent the breakpoint instances, though. There is already visual
> feedback in the longer breakpoint numbers and the fact that 'type' and 'disp'
> are omitted to separate the groups.
Hi,
I also find it usefull to have some information about the breakpoint
location as entered by the user. I think the motivating examples Pedro
gave give good examples that similar breakpoint locations can come from
quite different user intentions.
As for the indentation, I have to admit I do not really have a strong
opinion. I am used to the way the things currently are, but this is
probably not the greatest justifications.
Just to experiment with other options, I have a prototype usign a
`tree`-like indentation:
(gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y errors.cc:50
└─ 1.1 y 0x000000000224fd06 in internal_error(char const*, int, char const*, ...) at ../../gdbsupport/errors.cc:51
2 breakpoint keep y internal_error
└─ 2.1 y 0x000000000224fd06 in internal_error(char const*, int, char const*, ...) at ../../gdbsupport/errors.cc:51
3 breakpoint keep y -qualified internal_error
└─ 3.1 y 0x000000000224fd06 in internal_error(char const*, int, char const*, ...) at ../../gdbsupport/errors.cc:51
4 breakpoint keep y error
├─ 4.1 y 0x00000000010b30fe in gcc_c_plugin::error(char const*) const at ../../gdb/../include/gcc-c-fe.def:198
├─ 4.2 y 0x00000000010c6290 in gcc_cp_plugin::error(char const*) const at ../../gdb/../include/gcc-cp-fe.def:983
└─ 4.3 y 0x000000000224fbe1 in error(char const*, ...) at ../../gdbsupport/errors.cc:39
I am not sure this is a clear improvement, but I can push the patches
somewhere if someone wants to give it a try.
Best,
Lancelot.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 0/2] info breakpoints improvements
2022-05-20 5:57 ` [PATCH 0/2] info breakpoints improvements Eli Zaretskii
@ 2022-05-23 17:06 ` Pedro Alves
2022-05-24 13:14 ` Eli Zaretskii
0 siblings, 1 reply; 39+ messages in thread
From: Pedro Alves @ 2022-05-23 17:06 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
On 2022-05-20 06:57, Eli Zaretskii wrote:
>> From: Pedro Alves <pedro@palves.net>
>> Date: Thu, 19 May 2022 22:55:50 +0100
>>
>> we get:
>>
>> (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 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 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 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
>> 4 breakpoint keep y gdb.c:27
>> 4.1 y 0x000055555564107b in main(int, char**) at src/gdb/gdb.c:28
>> (top-gdb)
>
> I must confess that the new display is much more cluttered, and
> includes redundant information, so it's harder to read. It also makes
> the important stuff harder to find. Why exactly is this deemed as
> improvement, and in particular, why would we want this behavior as the
> default? (I won't mind to have this as opt-in behavior, if someone
> finds this useful in some situations.)
None of the information is actually redundant. The "Enb" column shows
a different "y", as you can disable a whole breakpoint, or its locations
independently. The "What" column is showing different things for the breakpoint
and its locations. I've explained in a lot of detail why I think we want this
in the commit log of patch #1, and now in the reply to your review of patch #1 too.
>
>> Patch #2 introduces an "info breakpoints -hide-locations" option.
>> With that, you get just the breakpoint header rows, showing the
>> canonical location spec originally used to set the breakpoint, but not
>> what the spec expanded to:
>>
>> (top-gdb) i b -h
>> Num Type Disp Enb What
>> 1 breakpoint keep y internal_error
>> 2 breakpoint keep y -qualified internal_error
>> 3 breakpoint keep y errors.c:54
>
> If we want a concise display that only shows the important parts, I'd
> lose the "Disp" column.
I'm not sure how you're determining "important". That'd make it impossible to
distinguish a "break" from a "tbreak". I'm only after hiding the breakpoint locations.
I.e., I want to still see all information about each of the breakpoints, just not
its resolved locations. I only made -hide-locations skip the "Address" column because it
would always be empty otherwise. For example, here I set a tbreak, and I have a
silent breakpoint:
(top-gdb) info breakpoints -h
Num Type Disp Enb What
1 breakpoint keep y internal_error
2 breakpoint keep y info_command
silent
return
3 breakpoint keep y main
breakpoint already hit 1 time
4 breakpoint del y get_selected_frame
stop only if debug_infrun > 0
So, a more concise display may be interesting, but I'd like to think of it as
a separate feature.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 0/2] info breakpoints improvements
2022-05-19 21:55 [PATCH 0/2] info breakpoints improvements Pedro Alves
` (2 preceding siblings ...)
2022-05-20 5:57 ` [PATCH 0/2] info breakpoints improvements Eli Zaretskii
@ 2022-05-24 8:38 ` Luis Machado
2022-05-24 10:02 ` Pedro Alves
3 siblings, 1 reply; 39+ messages in thread
From: Luis Machado @ 2022-05-24 8:38 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 5/19/22 22:55, Pedro Alves wrote:
> The subject of breakpoint line number sliding came up again in an
> internal discussion, which reminded me of this WIP patch that I
> originally posted here:
> https://sourceware.org/pipermail/gdb-patches/2019-June/158638.html
>
> Here's an updated version that is close to ready for merging. It
> needs agreement, and, testsuite adjustments. Oh, and I realize now,
> NEWS entries. Both patches already contain documentation changes. I
> went through the whole "Set Breaks" chapter fixing up things to better
> match current reality as I was documenting the new features.
>
> Patch #1 changes GDB such that instead of:
>
> (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
> 4 breakpoint keep y 0x000055555564107b in main(int, char**) at src/gdb/gdb.c:28
> (top-gdb)
>
> we get:
>
> (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 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 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 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
> 4 breakpoint keep y gdb.c:27
> 4.1 y 0x000055555564107b in main(int, char**) at src/gdb/gdb.c:28
> (top-gdb)
>
The additional information is nice to have, though I don't use it myself and I'm happy with <MULTIPLE> and the
fact GDB adjusts line numbers (it's just natural). Maybe others not familiar with DWARF and line tables will find it
useful. Maybe even IDE's dealing with GDB.
I could make a point that the patch makes things slightly more verbose, but the breakpoint display is already quite verbose. :-)
So overall it looks like a good direction to go.
I anticipate a lot of tedious changes to the testsuite due to changing patterns.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 0/2] info breakpoints improvements
2022-05-24 8:38 ` Luis Machado
@ 2022-05-24 10:02 ` Pedro Alves
2022-05-24 13:20 ` Eli Zaretskii
0 siblings, 1 reply; 39+ messages in thread
From: Pedro Alves @ 2022-05-24 10:02 UTC (permalink / raw)
To: Luis Machado, gdb-patches
On 2022-05-24 09:38, Luis Machado wrote:
> I could make a point that the patch makes things slightly more verbose, but the breakpoint display is already quite verbose. :-)
Yeah. It's already common to see multi-location breakpoints anyhow, at least when debugging C++ code.
I was playing a bit with this today, and noticed that there is something that could be improved, and this applies
to current master as well.
Notice below, debugging gdb with current master gdb:
(top-gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y 0x0000000000a30006 in internal_error(char const*, int, char const*, ...) at /home/pedro/gdb/binutils-gdb/src/gdbsupport/errors.cc:51
2 breakpoint keep y 0x0000000000261340 in info_command(char const*, int) at /home/pedro/gdb/binutils-gdb/src/gdb/cli/cli-cmds.c:216
silent
return
3 breakpoint keep y <MULTIPLE>
3.1 y 0x00000000002a3452 in gcc_c_plugin::error(char const*) const at /home/pedro/gdb/binutils-gdb/src/gdb/../include/gcc-c-fe.def:198
3.2 y 0x00000000002ac504 in gcc_cp_plugin::error(char const*) const at /home/pedro/gdb/binutils-gdb/src/gdb/../include/gcc-cp-fe.def:983
3.3 y 0x0000000000a2ff51 in error(char const*, ...) at /home/pedro/gdb/binutils-gdb/src/gdbsupport/errors.cc:39
4 breakpoint keep n <MULTIPLE>
4.1 y 0x00000000000ed059 in main(int, char**) at /home/pedro/gdb/binutils-gdb/src/gdb/gdb.c:25
4.2 y 0x00000000008686ca in selftests::string_view::capacity_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/capacity/1.cc:166
4.3 y 0x0000000000868ae9 in selftests::string_view::cons_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/cons/char/1.cc:61
4.4 y 0x0000000000868bda in selftests::string_view::cons_2::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/cons/char/2.cc:40
4.5 y 0x0000000000868c40 in selftests::string_view::cons_3::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/cons/char/3.cc:33
4.6 y 0x0000000000868e11 in selftests::string_view::element_access_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/element_access/char/1.cc:65
4.7 y 0x0000000000868e25 in selftests::string_view::element_access_empty::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/element_access/char/empty.cc:25
4.8 y 0x0000000000869037 in selftests::string_view::element_access_front_back::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/element_access/char/front_back.cc:37
4.9 y 0x0000000000869463 in selftests::string_view::inserters_2::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/inserters/char/2.cc:83
4.10 y 0x00000000008695cf in selftests::string_view::modifiers_remove_prefix::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/modifiers/remove_prefix/char/1.cc:57
4.11 y 0x000000000086971b in selftests::string_view::modifiers_remove_suffix::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/modifiers/remove_suffix/char/1.cc:57
4.12 y 0x0000000000869ede in selftests::string_view::operations_compare_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/compare/char/1.cc:126
4.13 y 0x000000000086a021 in selftests::string_view::operations_compare_13650::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/compare/char/13650.cc:44
4.14 y 0x000000000086a110 in selftests::string_view::operations_copy_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/copy/char/1.cc:40
4.15 y 0x000000000086a1e1 in selftests::string_view::operations_data_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/data/char/1.cc:38
4.16 y 0x000000000086a7a4 in selftests::string_view::operations_find_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/find/char/1.cc:159
4.17 y 0x000000000086ad06 in selftests::string_view::operations_find_2::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/find/char/2.cc:157
4.18 y 0x000000000086b367 in selftests::string_view::operations_find_3::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/find/char/3.cc:157
4.19 y 0x000000000086b412 in selftests::string_view::operations_find_4::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/find/char/4.cc:39
4.20 y 0x000000000086b9e5 in selftests::string_view::operations_rfind_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/rfind/char/1.cc:89
4.21 y 0x000000000086bc3e in selftests::string_view::operations_rfind_2::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/rfind/char/2.cc:47
4.22 y 0x000000000086bff2 in selftests::string_view::operations_rfind_3::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/rfind/char/3.cc:62
4.23 y 0x000000000086c32f in selftests::string_view::operations_substr_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/substr/char/1.cc:73
4.24 y 0x000000000086e181 in selftests::string_view::operators_2::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operators/char/2.cc:365
Note how breakpoint 4 is disabled, but since all the locations are enabled, the "n" doesn't stand out all that much.
With my patch it's the same thing, and potentially slightly worse, since now for each disabled breakpoint you will see
one "n" and one "y" entry. Note I only indented the "Num" column:
(top-gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep n internal_error
1.1 y 0x0000000000a30062 in internal_error(char const*, int, char const*, ...) at /home/pedro/gdb/binutils-gdb/src/gdbsupport/errors.cc:51
2 breakpoint keep y info_command
silent
return
2.1 y 0x0000000000261353 in info_command(char const*, int) at /home/pedro/gdb/binutils-gdb/src/gdb/cli/cli-cmds.c:217
3 breakpoint keep n main
3.1 y 0x00000000000ed06c in main(int, char**) at /home/pedro/gdb/binutils-gdb/src/gdb/gdb.c:25
3.2 y 0x00000000008686d2 in selftests::string_view::capacity_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/capacity/1.cc:167
3.3 y 0x0000000000868af1 in selftests::string_view::cons_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/cons/char/1.cc:62
3.4 y 0x0000000000868be2 in selftests::string_view::cons_2::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/cons/char/2.cc:41
3.5 y 0x0000000000868c48 in selftests::string_view::cons_3::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/cons/char/3.cc:34
3.6 y 0x0000000000868e19 in selftests::string_view::element_access_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/element_access/char/1.cc:66
3.7 y 0x0000000000868e31 in selftests::string_view::element_access_empty::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/element_access/char/empty.cc:25
3.8 y 0x000000000086903f in selftests::string_view::element_access_front_back::main()
at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/element_access/char/front_back.cc:38
3.9 y 0x000000000086946b in selftests::string_view::inserters_2::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/inserters/char/2.cc:84
3.10 y 0x00000000008695d7 in selftests::string_view::modifiers_remove_prefix::main()
at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/modifiers/remove_prefix/char/1.cc:58
3.11 y 0x0000000000869723 in selftests::string_view::modifiers_remove_suffix::main()
at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/modifiers/remove_suffix/char/1.cc:58
3.12 y 0x0000000000869ee6 in selftests::string_view::operations_compare_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/compare/char/1.cc:127
3.13 y 0x000000000086a029 in selftests::string_view::operations_compare_13650::main()
at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/compare/char/13650.cc:45
3.14 y 0x000000000086a118 in selftests::string_view::operations_copy_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/copy/char/1.cc:41
3.15 y 0x000000000086a1e9 in selftests::string_view::operations_data_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/data/char/1.cc:39
3.16 y 0x000000000086a7ac in selftests::string_view::operations_find_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/find/char/1.cc:160
3.17 y 0x000000000086ad0e in selftests::string_view::operations_find_2::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/find/char/2.cc:158
3.18 y 0x000000000086b36f in selftests::string_view::operations_find_3::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/find/char/3.cc:158
3.19 y 0x000000000086b41a in selftests::string_view::operations_find_4::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/find/char/4.cc:40
3.20 y 0x000000000086b9ed in selftests::string_view::operations_rfind_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/rfind/char/1.cc:90
3.21 y 0x000000000086bc46 in selftests::string_view::operations_rfind_2::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/rfind/char/2.cc:48
3.22 y 0x000000000086bffa in selftests::string_view::operations_rfind_3::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/rfind/char/3.cc:63
3.23 y 0x000000000086c337 in selftests::string_view::operations_substr_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/substr/char/1.cc:74
3.24 y 0x000000000086e189 in selftests::string_view::operators_2::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operators/char/2.cc:366
but see what happens if we indent the "Enb" and "What" columns as well below. Now it's much easier to distinguish disabled breakpoint vs
disabled location. And by indenting the "What" column as well, we can spot the hierarchy by focusing on that column as well.
Note that the addresses in the "Address" column aren't indented so they align with "Address" in the table header. Here's what
it looks like:
(top-gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep n internal_error
1.1 y 0x0000000000a300ea in internal_error(char const*, int, char const*, ...) at /home/pedro/gdb/binutils-gdb/src/gdbsupport/errors.cc:51
2 breakpoint keep y info_command
silent
return
2.1 y 0x00000000002613db in info_command(char const*, int) at /home/pedro/gdb/binutils-gdb/src/gdb/cli/cli-cmds.c:217
3 breakpoint keep n main
3.1 y 0x00000000000ed06c in main(int, char**) at /home/pedro/gdb/binutils-gdb/src/gdb/gdb.c:25
3.2 y 0x000000000086875a in selftests::string_view::capacity_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/capacity/1.cc:167
3.3 y 0x0000000000868b79 in selftests::string_view::cons_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/cons/char/1.cc:62
3.4 y 0x0000000000868c6a in selftests::string_view::cons_2::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/cons/char/2.cc:41
3.5 y 0x0000000000868cd0 in selftests::string_view::cons_3::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/cons/char/3.cc:34
3.6 y 0x0000000000868ea1 in selftests::string_view::element_access_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/element_access/char/1.cc:66
3.7 y 0x0000000000868eb9 in selftests::string_view::element_access_empty::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/element_access/char/empty.cc:25
3.8 y 0x00000000008690c7 in selftests::string_view::element_access_front_back::main()
at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/element_access/char/front_back.cc:38
3.9 y 0x00000000008694f3 in selftests::string_view::inserters_2::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/inserters/char/2.cc:84
3.10 y 0x000000000086965f in selftests::string_view::modifiers_remove_prefix::main()
at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/modifiers/remove_prefix/char/1.cc:58
3.11 y 0x00000000008697ab in selftests::string_view::modifiers_remove_suffix::main()
at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/modifiers/remove_suffix/char/1.cc:58
3.12 y 0x0000000000869f6e in selftests::string_view::operations_compare_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/compare/char/1.cc:127
3.13 y 0x000000000086a0b1 in selftests::string_view::operations_compare_13650::main()
at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/compare/char/13650.cc:45
3.14 y 0x000000000086a1a0 in selftests::string_view::operations_copy_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/copy/char/1.cc:41
3.15 y 0x000000000086a271 in selftests::string_view::operations_data_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/data/char/1.cc:39
3.16 y 0x000000000086a834 in selftests::string_view::operations_find_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/find/char/1.cc:160
3.17 y 0x000000000086ad96 in selftests::string_view::operations_find_2::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/find/char/2.cc:158
3.18 y 0x000000000086b3f7 in selftests::string_view::operations_find_3::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/find/char/3.cc:158
3.19 y 0x000000000086b4a2 in selftests::string_view::operations_find_4::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/find/char/4.cc:40
3.20 y 0x000000000086ba75 in selftests::string_view::operations_rfind_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/rfind/char/1.cc:90
3.21 y 0x000000000086bcce in selftests::string_view::operations_rfind_2::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/rfind/char/2.cc:48
3.22 y 0x000000000086c082 in selftests::string_view::operations_rfind_3::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/rfind/char/3.cc:63
3.23 y 0x000000000086c3bf in selftests::string_view::operations_substr_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/substr/char/1.cc:74
3.24 y 0x000000000086e211 in selftests::string_view::operators_2::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operators/char/2.cc:366
I'm liking this a lot. I think I want to keep it.
There's one extra easy thing we could do, which is to print one empty line after each breakpoint, like so:
(top-gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep n internal_error
1.1 y 0x0000000000a30126 in internal_error(char const*, int, char const*, ...) at /home/pedro/gdb/binutils-gdb/src/gdbsupport/errors.cc:51
2 breakpoint keep y info_command
silent
return
2.1 y 0x0000000000261417 in info_command(char const*, int) at /home/pedro/gdb/binutils-gdb/src/gdb/cli/cli-cmds.c:217
3 breakpoint keep n main
3.1 y 0x00000000000ed06c in main(int, char**) at /home/pedro/gdb/binutils-gdb/src/gdb/gdb.c:25
3.2 y 0x0000000000868796 in selftests::string_view::capacity_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/capacity/1.cc:167
3.3 y 0x0000000000868bb5 in selftests::string_view::cons_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/cons/char/1.cc:62
3.4 y 0x0000000000868ca6 in selftests::string_view::cons_2::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/cons/char/2.cc:41
3.5 y 0x0000000000868d0c in selftests::string_view::cons_3::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/cons/char/3.cc:34
3.6 y 0x0000000000868edd in selftests::string_view::element_access_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/element_access/char/1.cc:66
3.7 y 0x0000000000868ef5 in selftests::string_view::element_access_empty::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/element_access/char/empty.cc:25
3.8 y 0x0000000000869103 in selftests::string_view::element_access_front_back::main()
at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/element_access/char/front_back.cc:38
3.9 y 0x000000000086952f in selftests::string_view::inserters_2::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/inserters/char/2.cc:84
3.10 y 0x000000000086969b in selftests::string_view::modifiers_remove_prefix::main()
at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/modifiers/remove_prefix/char/1.cc:58
3.11 y 0x00000000008697e7 in selftests::string_view::modifiers_remove_suffix::main()
at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/modifiers/remove_suffix/char/1.cc:58
3.12 y 0x0000000000869faa in selftests::string_view::operations_compare_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/compare/char/1.cc:127
3.13 y 0x000000000086a0ed in selftests::string_view::operations_compare_13650::main()
at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/compare/char/13650.cc:45
3.14 y 0x000000000086a1dc in selftests::string_view::operations_copy_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/copy/char/1.cc:41
3.15 y 0x000000000086a2ad in selftests::string_view::operations_data_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/data/char/1.cc:39
3.16 y 0x000000000086a870 in selftests::string_view::operations_find_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/find/char/1.cc:160
3.17 y 0x000000000086add2 in selftests::string_view::operations_find_2::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/find/char/2.cc:158
3.18 y 0x000000000086b433 in selftests::string_view::operations_find_3::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/find/char/3.cc:158
3.19 y 0x000000000086b4de in selftests::string_view::operations_find_4::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/find/char/4.cc:40
3.20 y 0x000000000086bab1 in selftests::string_view::operations_rfind_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/rfind/char/1.cc:90
3.21 y 0x000000000086bd0a in selftests::string_view::operations_rfind_2::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/rfind/char/2.cc:48
3.22 y 0x000000000086c0be in selftests::string_view::operations_rfind_3::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/rfind/char/3.cc:63
3.23 y 0x000000000086c3fb in selftests::string_view::operations_substr_1::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operations/substr/char/1.cc:74
3.24 y 0x000000000086e24d in selftests::string_view::operators_2::main() at /home/pedro/gdb/binutils-gdb/src/gdb/unittests/basic_string_view/operators/char/2.cc:366
4 breakpoint keep y error
4.1 y 0x00000000002a352a in gcc_c_plugin::error(char const*) const at /home/pedro/gdb/binutils-gdb/src/gdb/../include/gcc-c-fe.def:198
4.2 y 0x00000000002ac5dc in gcc_cp_plugin::error(char const*) const at /home/pedro/gdb/binutils-gdb/src/gdb/../include/gcc-cp-fe.def:983
4.3 y 0x0000000000a30072 in error(char const*, ...) at /home/pedro/gdb/binutils-gdb/src/gdbsupport/errors.cc:39
(top-gdb)
Not so sure about this one.
Here's what these all look like in a color terminal, btw:
https://i.imgur.com/nMUgejh.png (no extra indent, no extra line)
https://i.imgur.com/sjVmEhe.png (extra indent, no extra line)
https://i.imgur.com/zK2H1Bh.png (extra indent, extra line)
I've pushed the series with an extra patch on top for the extra indentation, to the
users/palves/info_breakpoints_improvements branch, in case anyone wants to play with it.
>
> So overall it looks like a good direction to go.
>
> I anticipate a lot of tedious changes to the testsuite due to changing patterns.
>
Yeah... I think the pain is worth it though. Maybe I'll end up adding or using
some preexisting shared procedures to parse info breakpoints output throughout,
so the next time output changes it's easier... We'll see.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v2 1/2] Always show locations for breakpoints & show canonical location spec
2022-05-23 17:04 ` [PATCH v2 " Pedro Alves
@ 2022-05-24 13:06 ` Eli Zaretskii
2022-05-25 19:32 ` Pedro Alves
0 siblings, 1 reply; 39+ messages in thread
From: Eli Zaretskii @ 2022-05-24 13:06 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> Date: Mon, 23 May 2022 18:04:18 +0100
> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <pedro@palves.net>
>
> >> +@item break @var{location_spec}
> >> +Set a breakpoint at all the locations the given @var{location_spec}
> >> +resolves to. @var{location_spec} can specify a function name, a line
> >> +number, an address of an instruction, and more. @xref{Specify
> >> +Location}, for a list of all the possible ways to specify a
> >> +@var{location}. The breakpoint will stop your program just before it
> >> +executes any of the instructions at the resolved locations' addresses.
> >> +
> >> +When using source languages that permit overloading of symbols, such
> >> +as C@t{++}, a function name may refer to more than one symbol, and
> >> +thus more than one place to break. @xref{Ambiguous
> >> +Expressions,,Ambiguous Expressions}, for a discussion of that
> >> +situation.
> >> +
> >> +It is possible that a given function is instantiated more than once in
> >> +the program, resulting in a breakpoint with multiple resolved
> >> +locations. This can happen e.g., with inlined functions, or template
> >> +functions.
> >
> > This text was written when we showed N.x locations only when needed,
> > so the text means to explain when a breakpoint will have several
> > locations instead of just one. But with this changeset, we will be
> > displaying multiple locations _always_, so this text is no longer
> > enough. We should augment it with the background for multiple shown
> > locations even in the cases where the underlying breakpoint is
> > inserted only at a single address, such as your example of setting a
> > breakpoint at a location that doesn't have any corresponding code
> > address.
>
> I don't understand what you're actually suggesting. This is the
> documentation of the "break" command, not "info breakpoints".
My point was that "breakpoint with multiple location" will from now on
happen in much more frequent situations than function overloading. In
particular, it will happen in programs that don't use any OOP
techniques and not even written in OO programming languages.
IOW, the text above describes a use case that is not a very important
one, under the proposed changes, while keeping silence about a much
more important use case.
> >> +A breakpoint location specification (@pxref{Specify Location}) may be
> >> +resolved to several locations in your program. E.g., @code{break
> >> +func} will find a location for each function named @code{func} in the
> >> +program.
> >
> > Again, this "e.g." is incomplete and misses the frequent case, where
> > there will be always 2 "locations", possibly identical, shown in the
> > table.
>
> I am making a distinction between a "location specification", and
> a "resolved location". So here:
>
> (top-gdb) info breakpoints
> Num Type Disp Enb Address What
> 1 breakpoint keep y internal_error
> 1.1 y 0x0000555555f813be in internal_error(char const*, int, char const*, ...) at /home/pedro/gdb/binutils-gdb/src/gdbsupport/errors.cc:51
> (top-gdb)
I understand that this is what you had in mind. What I had in mind is
this other example of yours:
> 4 breakpoint keep y gdb.c:27
> 4.1 y 0x000055555564107b in main(int, char**) at src/gdb/gdb.c:28
As you see, here we also have "2 locations, possibly identical",
without having 2 functions called 'main'. Specifically, the
"gdb.c:27" vs "gdb.c:28" parts, which in many cases, but not always,
will be identical. I'm saying that this is no less important than
multiple functions by the same name, and should be part of the above
text.
> The "will find a location for each function named func", is talking about more
> than info breakpoints. Since this is the part describing the "break" command,
> you can already see it here:
>
> (top-gdb) b main
> Breakpoint 3 at 0xed06c: main. (24 locations)
> ^^^^^^^^^^^^
>
> The user typed "main", and gdb found 24 locations. I.e., the single
> location specification "main" was resolved to 24 locations in your program.
I'm saying that from the user POV, at least this user, the above two
situations are very similar. I realize that internally they are quite
different, but the manner in which both breakpoints will be presented
by "info breakpoints" has the same traits relevant to the discussion
of "locations": there could be more than one "location" for a
breakpoint in either case.
We should keep in mind that the notion a GDB user has about
breakpoints is based on what GDB shows, not on the internal
implementation details. So similar displays lead to similar mental
models, regardless of the actual implementation.
> >> For each code breakpoint and tracepoint (not for
> >> +watchpoints, nor catchpoints), @value{GDBN} prints a header entry, and
> >> +then one entry for each resolved location of the breakpoint.
> >
> > Likewise: "one entry" will be misleading with the new display, because
> > we will always show at least 2.
>
> It says "prints a header entry", so that's 1 entry, "and then one entry FOR EACH
> resolved location". I'm not sure what you find confusing here.
I guess the confusing part is the "header entry". AFAIR, it was never
explicitly described under that name before it is used. Also,
"header" is too general a word, not necessarily related to
breakpoints, so it doesn't explain itself well enough, given its wide
use and its higher importance under the new behavior, where there will
be _always_ a header entry. "Header" is much more natural for
describing the table header, i.e. this part:
>> Num Type Disp Enb Address What
> I just reused that text before the table so that I could talk about
> breakpoint locations in the description of the columns. I just didn't use the
> word "row". I've reworded the new text a bit now, using "row" too. Hopefully it reads
> clearer that way.
I can only say that it confused me, when I compared the text with the
"pictures": the examples of the new display.
> >> +Enabled breakpoints and breakpoint locations are marked with @samp{y}.
> >
> > What does "enabled breakpoint location" mean? One cannot enable a
> > location.
>
> Sure we can.
We are mis-communicating. "Location" is a frequently-used word that
has certain natural meaning in many contexts. That natural meaning
doesn't support the notion of "enabling". You use "location" in a
very special (and IMO problematic, as I tried to explain later in my
comments) meaning, and in that meaning a "location" indeed _can_ be
enabled. But when reading this text, that meaning was not yet clear
to me, if it ever becomes clear later, and thus this text made me
raise a brow. I described the surprise which I felt while reading the
text as a feedback in the hope that we will be able to make the manual
more clear and less confusing upon first, linear, top-to-bottom
reading, and avoid the need for the reader to repeatedly read the same
text before its meaning dawns on the reader.
> This is documented in the "Set Breaks" node already, here:
>
> "You cannot delete the individual locations from a breakpoint. However, each location can be individually enabled or
> disabled by passing breakpoint-number.location-number as argument to the enable and disable commands."
>
> Note that, as I explained in the commit log of the patch in question, with current master, if the
> breakpoint's enabled state is different from the location's enabled state, then GDB is already
> presenting a single-location breakpoint in "multi-location" mode. E.g., with current master
> as top gdb, debugging another gdb:
This completely misses the point. I'm not accusing you in doing
something inconsistent, I'm just saying that reading that text, in its
immediate context, caused me to wonder what is all this about. So
there's no need to defend what you did by pointing to some other text
elsewhere. What I hope is that by pointing out the parts that caused
confusion and brow-raising, I will help in making the manual more
self-explanatory and unequivocal. If my comments aren't helpful, you
can disregard them, but you cannot convince me that what I felt when
I've read the text was not confusion. If we want to do something
about that confusion, let's discuss ways of changing the text to avoid
such confusion.
> > This should explain the crucial difference between "the original
> > location specification" and "breakpoint location" used elsewhere in
> > the text. It is entirely not trivial to grasp the importance of that
> > distinction. (Another tip of the iceberg.)
>
> OK, I can agree with that. Do note the current manual is already
> ambiguous here, for it calls both things "location", and that's it.
Under the current GDB behavior, this is less important, because
multi-location display is not as pervasive as you intend to make it.
Thus, in the current manual I could completely ignore this terminology
issue if I don't bump frequently enough into overloaded functions.
Not so under the new behavior.
> So I'm actually proposing to improve things by calling the user-input
> text as "location specification", distinct from the actual locations
> the specification matches or resolves to.
My suggestion is to use "location" (with or without additional
qualifiers) for only one of these two, if only because we tend to
shorthand that by using just "location". We could use it for what you
now call "location specification", which would allow us to use just
"location" as it shorthand. With your proposed text, "location" is
ambiguous, because it could allude to any of these two. The other
meaning of "location" I propose to call "address in the code" or
"address" for short -- because that's what it conceptually is. (If
you are bothered by the case of unloaded shared library and similar
situations, we could say "unresolved address" in those cases -- which
are rather marginal, so once explained, they can be ignored in the
rest of the text. By contrast, these two "locations" are pervasive:
we need to refer to them all over the manual. So it's much more
important IMO to make that part of our terminology clear and
unequivocal.)
Can you tel concretely why you object to changing the "location"
terminology along these lines? Even if you think the current
terminology is okay, what will we lose by switching the terminology I
propose above?
> Even GDB's source code uses "location" for both -- struct bp_location
> for the breakpoint locations, the "1.1", "1.2" things, stored in each
> struct breakpoint in the breakpoint::loc field, as a linked list:
I know. But I hope we can avoid terminology ambiguities even though
they are present in the code. The reader of the GDB manual will not
necessarily read the GDB source at the same time, and is not required
to do so if all they want is to learn how to use GDB.
> >> +@value{GDBN} may add new locations to existing breakpoints when new
> >> +symbols are loaded. For example, if you have a breakpoint in a
> >> +C@t{++} template function, and a newly loaded shared library has an
> >> +instantiation of that template, a new location is added to the list of
> >> +locations for the breakpoint. More on this further below.
> >
> > I fail to see why this paragraph is important to have. What it says
> > is trivial, and having it here makes a complex description harder to
> > understand, because it interrupts the description with unimportant
> > details.
>
> Note this is a rewording of preexisting text.
Preexisting text is not sacred. It's okay to modify or even delete
parts of the old text where they are sub-optimal or aren't
contributing to the manual enough to justify their existence.
Also, please understand that when presented with a large rewrite of
the manual, I normally cannot go back to the old text and refrain from
commenting on some parts that were more or less verbatim copied from
the old version. Instead, I read the new text as a whole and comment
on that, since that is what will the reader see after the changes are
installed.
> >> +breakpoint table using several rows---one header row per breakpoint,
> >> +followed by one row for each of the breakpoint's locations. The
> >> +header row has an empty address column, and the original location
> >> +specification in the what column. The rows for individual locations
> >> +contain the addresses for the locations, and show the functions to
> >> +which those addresses belong. The number column for a location is
> >> +indented by one space to the right for grouping, and is of the form
> >> @var{breakpoint-number}.@var{location-number}.
> >
> > IMO, something like this text should precede the description of the
> > table contents, because it both provides a high-level overview of how
> > each breakpoint is displayed, and defines terminology like "header
> > row", "breakpoint location row", etc. Having it here is "too late".
>
> I actually already did that, that's why I added the paragraph saying:
> [...]
> >> set a breakpoint in a shared library at the beginning of your
> >> debugging session, when the library is not loaded, and when the
> >> symbols from the library are not available. When you try to set
> >> -breakpoint, @value{GDBN} will ask you if you want to set
> >> -a so called @dfn{pending breakpoint}---breakpoint whose address
> >> -is not yet resolved.
> >> +a breakpoint, @value{GDBN} will ask you for confirmation of whether
> >> +you want to set a so called @dfn{pending breakpoint}---a breakpoint with
> >> +no resolved locations yet.
> >
> > I'm not sure it is a good idea to talk about "resolved locations",
> > here and elsewhere. Why not "resolved addresses", as the original
> > text did? Using "location" here introduces a conceptual ambiguity,
> > whereby a "location" could be in the source code and also in the
> > executable code, and that overloading of concepts makes this complex
> > issue even harder to understand. My suggestion is to use "location"
> > for source-level location specs, and for nothing else.
>
> I disagree with that, because that's not how GDB behaves.
> [...]
I'm disappointed by such a wholesale rejection of these parts of my
comments. The terminology issue is very important, at least IMO, and
the rest of the review circles around it and its various aspects. If
you disagree with it so strongly, almost all the non-trivial parts of
my review can be just ignored, because they are based on this
difficulty in the terminology.
I'm not sure I know how to proceed, given that we have such a basic
disagreement on these aspects of the changes for the manual.
> >> +This field is present for ordinary breakpoints and tracepoints.
> >
> > What is an "ordinary" breakpoint, and how is it different from the
> > other kinds? (There are other places in the new text that use this
> > unexplained terminology.)
>
> I've just using the terminology already used in this table. See:
When I say "not explained" I mean there's no text which says something
like "an ordinary breakpoint is ...", or some text that explains what
are the "not ordinary" breakpoints, and by doing that explains
implicitly what are the "ordinary" ones. If the old text didn't have
that, it means that old text was also sub-optimal. (And my reading of
these changes to the manual led me to believe that this distinction
will be more important once the behavior changes, because you modified
the original text even where the old one was not invalidated by the
changes in behavior.) Thus the comment.
And this part of the patch:
> -A location in a multi-location breakpoint is represented as a tuple with the
> -following fields:
> +Each location in an ordinary breakpoint or tracepoint is represented
> +as a tuple with the following fields:
led me to believe that "ordinary" somehow is related to
"multi-location".
> Thus "ordinary" here refers to actual code breakpoints, as opposed to the generic "breakpoint"
> term meaning all of code breakpoints, watchpoints, tracepoints, catchpoints.
How about saying that explicitly, before we start relying on this
terminology?
> Below is the updated patch. Only the documentation changed.
Given that you rejected most of the important parts of my comments,
I'm not sure how to proceed with reviewing this new version. The main
part of my review is the suggestion to find better, more intuitive
terminology for "breakpoints" vs "header rows" in the "info break"
display, and also better terminology for the two types of "locations".
If this is left as it was in the original patch, what should I look at
in the revised patch?
Please understand, like I explained above, that I read the new text
after the patch as a whole, and base my review on what it tells me and
how it describes the GDB features. It's not just diffs to me, it's a
more-or-less complete document, and it must make sense as a whole.
Currently, it doesn't, due to those terminology problems.
If we keep this terminology, the relevant sections of the manual will
keep confusing me. The result is that I will keep mentioning this in
my future reviews, and that makes little sense to me.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 0/2] info breakpoints improvements
2022-05-23 17:06 ` Pedro Alves
@ 2022-05-24 13:14 ` Eli Zaretskii
2022-05-24 13:45 ` Pedro Alves
0 siblings, 1 reply; 39+ messages in thread
From: Eli Zaretskii @ 2022-05-24 13:14 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> Date: Mon, 23 May 2022 18:06:22 +0100
> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <pedro@palves.net>
>
> >> (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 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 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 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
> >> 4 breakpoint keep y gdb.c:27
> >> 4.1 y 0x000055555564107b in main(int, char**) at src/gdb/gdb.c:28
> >> (top-gdb)
> >
> > I must confess that the new display is much more cluttered, and
> > includes redundant information, so it's harder to read. It also makes
> > the important stuff harder to find. Why exactly is this deemed as
> > improvement, and in particular, why would we want this behavior as the
> > default? (I won't mind to have this as opt-in behavior, if someone
> > finds this useful in some situations.)
>
> None of the information is actually redundant. The "Enb" column shows
> a different "y", as you can disable a whole breakpoint, or its locations
> independently.
In the "usual" case, such as the one below:
> >> 4 breakpoint keep y gdb.c:127
> >> 4.1 y 0x000055555564107b in main(int, char**) at src/gdb/gdb.c:127
one of the two "y"s is redundant, as well as one of the two
"gdb.c:127"s. And in this case:
>> 1 breakpoint keep y internal_error
>> 1.1 y 0x00000000005755a5 in internal_error(char const*, int, char const*, ...) at src/gdb/common/errors.c:54
the "internal_error" part of the "header row" is also redundant.
> The "What" column is showing different things for the breakpoint
> and its locations.
Different, but in many cases quite similar, with many common parts.
> >> Num Type Disp Enb What
> >> 1 breakpoint keep y internal_error
> >> 2 breakpoint keep y -qualified internal_error
> >> 3 breakpoint keep y errors.c:54
> >
> > If we want a concise display that only shows the important parts, I'd
> > lose the "Disp" column.
>
> I'm not sure how you're determining "important". That'd make it impossible to
> distinguish a "break" from a "tbreak". I'm only after hiding the breakpoint locations.
IME, the "tbreak" vs "break" difference is much less important than
the rest.
> So, a more concise display may be interesting, but I'd like to think of it as
> a separate feature.
It's okay to disagree about user perspectives on what a "concise"
display should and should not have. I thought that providing the
perspective of one (heavy) user of GDB on this will help make the
discussion more objective and balanced. Apologies if that is not
useful.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 0/2] info breakpoints improvements
2022-05-24 10:02 ` Pedro Alves
@ 2022-05-24 13:20 ` Eli Zaretskii
2022-05-24 13:29 ` Pedro Alves
0 siblings, 1 reply; 39+ messages in thread
From: Eli Zaretskii @ 2022-05-24 13:20 UTC (permalink / raw)
To: Pedro Alves; +Cc: luis.machado, gdb-patches
> Date: Tue, 24 May 2022 11:02:07 +0100
> From: Pedro Alves <pedro@palves.net>
>
> Note how breakpoint 4 is disabled, but since all the locations are enabled, the "n" doesn't stand out all that much.
I'm confused: what is the meaning of having a breakpoint disabled,
while all of its locations are enabled? Under which conditions will
such a breakpoint break?
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 0/2] info breakpoints improvements
2022-05-24 13:20 ` Eli Zaretskii
@ 2022-05-24 13:29 ` Pedro Alves
2022-05-24 13:43 ` Eli Zaretskii
0 siblings, 1 reply; 39+ messages in thread
From: Pedro Alves @ 2022-05-24 13:29 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: luis.machado, gdb-patches
On 2022-05-24 14:20, Eli Zaretskii wrote:
>> Date: Tue, 24 May 2022 11:02:07 +0100
>> From: Pedro Alves <pedro@palves.net>
>>
>> Note how breakpoint 4 is disabled, but since all the locations are enabled, the "n" doesn't stand out all that much.
>
> I'm confused: what is the meaning of having a breakpoint disabled,
> while all of its locations are enabled? Under which conditions will
> such a breakpoint break?
>
A location only breaks if it is enabled, _and_ its parent is enabled, so never.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 0/2] info breakpoints improvements
2022-05-24 13:29 ` Pedro Alves
@ 2022-05-24 13:43 ` Eli Zaretskii
2022-05-24 13:50 ` Pedro Alves
0 siblings, 1 reply; 39+ messages in thread
From: Eli Zaretskii @ 2022-05-24 13:43 UTC (permalink / raw)
To: Pedro Alves; +Cc: luis.machado, gdb-patches
> Date: Tue, 24 May 2022 14:29:27 +0100
> Cc: luis.machado@arm.com, gdb-patches@sourceware.org
> From: Pedro Alves <pedro@palves.net>
>
> On 2022-05-24 14:20, Eli Zaretskii wrote:
> >> Date: Tue, 24 May 2022 11:02:07 +0100
> >> From: Pedro Alves <pedro@palves.net>
> >>
> >> Note how breakpoint 4 is disabled, but since all the locations are enabled, the "n" doesn't stand out all that much.
> >
> > I'm confused: what is the meaning of having a breakpoint disabled,
> > while all of its locations are enabled? Under which conditions will
> > such a breakpoint break?
> >
>
> A location only breaks if it is enabled, _and_ its parent is enabled, so never.
That's what I thought. But then why not "propagate" the "n" of the
disabled breakpoint to all of its locations? That way
(gdb) info break 4.10
will show the truth, no?
I also wonder whether we should display something special on the
"header row" instead of "y" or "n" when some of the locations are
enabled and others aren't. How about "p" (for "partial") or maybe
"y/n"?
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 0/2] info breakpoints improvements
2022-05-24 13:14 ` Eli Zaretskii
@ 2022-05-24 13:45 ` Pedro Alves
0 siblings, 0 replies; 39+ messages in thread
From: Pedro Alves @ 2022-05-24 13:45 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
On 2022-05-24 14:14, Eli Zaretskii wrote:
>> Date: Mon, 23 May 2022 18:06:22 +0100
>> Cc: gdb-patches@sourceware.org
>> From: Pedro Alves <pedro@palves.net>
>>
>>>> (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 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 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 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
>>>> 4 breakpoint keep y gdb.c:27
>>>> 4.1 y 0x000055555564107b in main(int, char**) at src/gdb/gdb.c:28
>>>> (top-gdb)
>>>
>>> I must confess that the new display is much more cluttered, and
>>> includes redundant information, so it's harder to read. It also makes
>>> the important stuff harder to find. Why exactly is this deemed as
>>> improvement, and in particular, why would we want this behavior as the
>>> default? (I won't mind to have this as opt-in behavior, if someone
>>> finds this useful in some situations.)
>>
>> None of the information is actually redundant. The "Enb" column shows
>> a different "y", as you can disable a whole breakpoint, or its locations
>> independently.
>
> In the "usual" case, such as the one below:
>
>>>> 4 breakpoint keep y gdb.c:127
>>>> 4.1 y 0x000055555564107b in main(int, char**) at src/gdb/gdb.c:127
>
> one of the two "y"s is redundant, as well as one of the two
> "gdb.c:127"s. And in this case:
>
>>> 1 breakpoint keep y internal_error
>>> 1.1 y 0x00000000005755a5 in internal_error(char const*, int, char const*, ...) at src/gdb/common/errors.c:54
>
> the "internal_error" part of the "header row" is also redundant.
>
Well, with that view, this is very hard to discuss. The example quoted
at the top shows that breakpoints 1 through 3 all have the same breakpoint
location in internal_error at src/gdb/common/errors.c:54 , yet, they were all
specified differently. There is no way to distinguish them in the CLI today.
"internal_error" alone is not a source location, it's just a way to tell
GDB to find the locations where that function exists. It's just a function name.
The source location is the address, filename and line number tuple.
>> The "What" column is showing different things for the breakpoint
>> and its locations.
>
> Different, but in many cases quite similar, with many common parts.
Sure. But not always, and thus I find it important to show the missing
info, so we can tell which is which.
>> So, a more concise display may be interesting, but I'd like to think of it as
>> a separate feature.
>
> It's okay to disagree about user perspectives on what a "concise"
> display should and should not have. I thought that providing the
> perspective of one (heavy) user of GDB on this will help make the
> discussion more objective and balanced. Apologies if that is not
> useful.
I accept your feedback, and concede that it may be interesting to have
a mode that prints a more concise display. But I find it a different feature.
Nothing prevents you or someone else from adding some "info breakpoints -concise"
option. It would be up to debate what exactly does "concise" show, e.g., whether that
would mean we print the breakpoint's commands too, for example. For me, I would like
a flag that only suppresses the locations and nothing else, and that is very easy
to specify (there's no scope for divergence on what "hiding locations" means)
so that's what I am proposing.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 0/2] info breakpoints improvements
2022-05-24 13:43 ` Eli Zaretskii
@ 2022-05-24 13:50 ` Pedro Alves
2022-05-24 14:03 ` Eli Zaretskii
0 siblings, 1 reply; 39+ messages in thread
From: Pedro Alves @ 2022-05-24 13:50 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: luis.machado, gdb-patches
On 2022-05-24 14:43, Eli Zaretskii wrote:
>> Date: Tue, 24 May 2022 14:29:27 +0100
>> Cc: luis.machado@arm.com, gdb-patches@sourceware.org
>> From: Pedro Alves <pedro@palves.net>
>>
>> On 2022-05-24 14:20, Eli Zaretskii wrote:
>>>> Date: Tue, 24 May 2022 11:02:07 +0100
>>>> From: Pedro Alves <pedro@palves.net>
>>>>
>>>> Note how breakpoint 4 is disabled, but since all the locations are enabled, the "n" doesn't stand out all that much.
>>>
>>> I'm confused: what is the meaning of having a breakpoint disabled,
>>> while all of its locations are enabled? Under which conditions will
>>> such a breakpoint break?
>>>
>>
>> A location only breaks if it is enabled, _and_ its parent is enabled, so never.
>
> That's what I thought. But then why not "propagate" the "n" of the
> disabled breakpoint to all of its locations?
Because then when you re-enable the parent breakpoint, you'd have lost the enabled/disabled
state of the individual locations. It's like, if you turn off the mains switch in your
home, the individual light switches in each room stay in their positions. And you can
flip on/off the individual switches before turning on the mains too.
That way
>
> (gdb) info break 4.10
>
> will show the truth, no?
You can't "info break" an individual location.
>
> I also wonder whether we should display something special on the
> "header row" instead of "y" or "n" when some of the locations are
> enabled and others aren't. How about "p" (for "partial") or maybe
> "y/n"?
>
That may be useful, yes.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 0/2] info breakpoints improvements
2022-05-24 13:50 ` Pedro Alves
@ 2022-05-24 14:03 ` Eli Zaretskii
2022-05-24 14:09 ` Pedro Alves
2022-05-24 14:11 ` Andreas Schwab
0 siblings, 2 replies; 39+ messages in thread
From: Eli Zaretskii @ 2022-05-24 14:03 UTC (permalink / raw)
To: Pedro Alves; +Cc: luis.machado, gdb-patches
> Date: Tue, 24 May 2022 14:50:01 +0100
> Cc: luis.machado@arm.com, gdb-patches@sourceware.org
> From: Pedro Alves <pedro@palves.net>
>
> >> A location only breaks if it is enabled, _and_ its parent is enabled, so never.
> >
> > That's what I thought. But then why not "propagate" the "n" of the
> > disabled breakpoint to all of its locations?
>
> Because then when you re-enable the parent breakpoint, you'd have lost the enabled/disabled
> state of the individual locations.
I don't understand why would that be lost. I'm not proposing to
actually disable each location, I propose to _display_ them as
disabled in that case.
> > (gdb) info break 4.10
> >
> > will show the truth, no?
>
> You can't "info break" an individual location.
And you are certain we never will support that?
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 0/2] info breakpoints improvements
2022-05-24 14:03 ` Eli Zaretskii
@ 2022-05-24 14:09 ` Pedro Alves
2022-05-24 14:25 ` Eli Zaretskii
2022-05-24 14:11 ` Andreas Schwab
1 sibling, 1 reply; 39+ messages in thread
From: Pedro Alves @ 2022-05-24 14:09 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: luis.machado, gdb-patches
On 2022-05-24 15:03, Eli Zaretskii wrote:
>> Date: Tue, 24 May 2022 14:50:01 +0100
>> Cc: luis.machado@arm.com, gdb-patches@sourceware.org
>> From: Pedro Alves <pedro@palves.net>
>>
>>>> A location only breaks if it is enabled, _and_ its parent is enabled, so never.
>>>
>>> That's what I thought. But then why not "propagate" the "n" of the
>>> disabled breakpoint to all of its locations?
>>
>> Because then when you re-enable the parent breakpoint, you'd have lost the enabled/disabled
>> state of the individual locations.
>
> I don't understand why would that be lost. I'm not proposing to
> actually disable each location, I propose to _display_ them as
> disabled in that case.
If you do that, then you're hiding information for no good reason, IMO. It makes
it confusing, one would no longer be able to tell which locations would be enabled
once you re-enable the parent breakpoint. You'd make "enable 3.2" seem to have
no effect on a disabled breakpoint, as "info break" would still show the location
as disabled.
>
>>> (gdb) info break 4.10
>>>
>>> will show the truth, no?
>>
>> You can't "info break" an individual location.
>
> And you are certain we never will support that?
Of course not, but as usual, we can cross that bridge when to come to it. I suspect
that if we did support that, we would still want to print the breakpoint row for
breakpoint 4 and then only location 10 (skipping all other locations), so you'd still
see the breakpoint's enable state, and other info.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 0/2] info breakpoints improvements
2022-05-24 14:03 ` Eli Zaretskii
2022-05-24 14:09 ` Pedro Alves
@ 2022-05-24 14:11 ` Andreas Schwab
2022-05-24 14:17 ` Pedro Alves
2022-05-24 14:26 ` [PATCH 0/2] info breakpoints improvements Eli Zaretskii
1 sibling, 2 replies; 39+ messages in thread
From: Andreas Schwab @ 2022-05-24 14:11 UTC (permalink / raw)
To: Eli Zaretskii via Gdb-patches; +Cc: Pedro Alves, Eli Zaretskii
On Mai 24 2022, Eli Zaretskii via Gdb-patches wrote:
>> Date: Tue, 24 May 2022 14:50:01 +0100
>> Cc: luis.machado@arm.com, gdb-patches@sourceware.org
>> From: Pedro Alves <pedro@palves.net>
>>
>> >> A location only breaks if it is enabled, _and_ its parent is enabled, so never.
>> >
>> > That's what I thought. But then why not "propagate" the "n" of the
>> > disabled breakpoint to all of its locations?
>>
>> Because then when you re-enable the parent breakpoint, you'd have lost the enabled/disabled
>> state of the individual locations.
>
> I don't understand why would that be lost. I'm not proposing to
> actually disable each location, I propose to _display_ them as
> disabled in that case.
I think it would be better in that case to omit the column or display it
as "-", to indicate that it is ignored right now.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 0/2] info breakpoints improvements
2022-05-24 14:11 ` Andreas Schwab
@ 2022-05-24 14:17 ` Pedro Alves
2022-05-24 19:49 ` [PATCH] Show enabled locations with disabled breakpoint parent as "y-" (Re: [PATCH 0/2] info breakpoints improvements) Pedro Alves
2022-05-24 14:26 ` [PATCH 0/2] info breakpoints improvements Eli Zaretskii
1 sibling, 1 reply; 39+ messages in thread
From: Pedro Alves @ 2022-05-24 14:17 UTC (permalink / raw)
To: Andreas Schwab, Eli Zaretskii via Gdb-patches
On 2022-05-24 15:11, Andreas Schwab wrote:
> On Mai 24 2022, Eli Zaretskii via Gdb-patches wrote:
>
>>> Date: Tue, 24 May 2022 14:50:01 +0100
>>> Cc: luis.machado@arm.com, gdb-patches@sourceware.org
>>> From: Pedro Alves <pedro@palves.net>
>>>
>>>>> A location only breaks if it is enabled, _and_ its parent is enabled, so never.
>>>>
>>>> That's what I thought. But then why not "propagate" the "n" of the
>>>> disabled breakpoint to all of its locations?
>>>
>>> Because then when you re-enable the parent breakpoint, you'd have lost the enabled/disabled
>>> state of the individual locations.
>>
>> I don't understand why would that be lost. I'm not proposing to
>> actually disable each location, I propose to _display_ them as
>> disabled in that case.
>
> I think it would be better in that case to omit the column or display it
> as "-", to indicate that it is ignored right now.
>
Yes, we could print them differently, preserving the user-specified enable/disable
state. We print them in uppercase N/Y when the location is disabled due to an invalid
condition already, for example.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 0/2] info breakpoints improvements
2022-05-24 14:09 ` Pedro Alves
@ 2022-05-24 14:25 ` Eli Zaretskii
2022-05-24 14:33 ` Pedro Alves
0 siblings, 1 reply; 39+ messages in thread
From: Eli Zaretskii @ 2022-05-24 14:25 UTC (permalink / raw)
To: Pedro Alves; +Cc: luis.machado, gdb-patches
> Date: Tue, 24 May 2022 15:09:30 +0100
> Cc: luis.machado@arm.com, gdb-patches@sourceware.org
> From: Pedro Alves <pedro@palves.net>
>
> > I don't understand why would that be lost. I'm not proposing to
> > actually disable each location, I propose to _display_ them as
> > disabled in that case.
>
> If you do that, then you're hiding information for no good reason, IMO.
From my POV, it's the other way around: we will be showing the user
the actual current behavior of that location.
> It makes
> it confusing, one would no longer be able to tell which locations would be enabled
> once you re-enable the parent breakpoint. You'd make "enable 3.2" seem to have
> no effect on a disabled breakpoint, as "info break" would still show the location
> as disabled.
Why is this an important use case? It sounds much more useful and
frequent to first enable the whole breakpoint, and only then enable or
disable some of its particular locations. As long as the breakpoint
is disabled, the internal status of enabled/disabled of its location
is not important at all.
This is a user-level command, not a "maint" command. So what is of
utmost importance is how this will actually behave, not what the
software's internal state is.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 0/2] info breakpoints improvements
2022-05-24 14:11 ` Andreas Schwab
2022-05-24 14:17 ` Pedro Alves
@ 2022-05-24 14:26 ` Eli Zaretskii
1 sibling, 0 replies; 39+ messages in thread
From: Eli Zaretskii @ 2022-05-24 14:26 UTC (permalink / raw)
To: Andreas Schwab; +Cc: gdb-patches, pedro
> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: Pedro Alves <pedro@palves.net>, Eli Zaretskii <eliz@gnu.org>
> Date: Tue, 24 May 2022 16:11:15 +0200
>
> On Mai 24 2022, Eli Zaretskii via Gdb-patches wrote:
>
> >> Date: Tue, 24 May 2022 14:50:01 +0100
> >> Cc: luis.machado@arm.com, gdb-patches@sourceware.org
> >> From: Pedro Alves <pedro@palves.net>
> >>
> >> >> A location only breaks if it is enabled, _and_ its parent is enabled, so never.
> >> >
> >> > That's what I thought. But then why not "propagate" the "n" of the
> >> > disabled breakpoint to all of its locations?
> >>
> >> Because then when you re-enable the parent breakpoint, you'd have lost the enabled/disabled
> >> state of the individual locations.
> >
> > I don't understand why would that be lost. I'm not proposing to
> > actually disable each location, I propose to _display_ them as
> > disabled in that case.
>
> I think it would be better in that case to omit the column or display it
> as "-", to indicate that it is ignored right now.
That'd be also fine with me, thanks. My point was only that showing
"y" there is misleading.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 0/2] info breakpoints improvements
2022-05-24 14:25 ` Eli Zaretskii
@ 2022-05-24 14:33 ` Pedro Alves
0 siblings, 0 replies; 39+ messages in thread
From: Pedro Alves @ 2022-05-24 14:33 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: luis.machado, gdb-patches
On 2022-05-24 15:25, Eli Zaretskii wrote:
>> Date: Tue, 24 May 2022 15:09:30 +0100
>> Cc: luis.machado@arm.com, gdb-patches@sourceware.org
>> From: Pedro Alves <pedro@palves.net>
>>
>>> I don't understand why would that be lost. I'm not proposing to
>>> actually disable each location, I propose to _display_ them as
>>> disabled in that case.
>>
>> If you do that, then you're hiding information for no good reason, IMO.
>
> From my POV, it's the other way around: we will be showing the user
> the actual current behavior of that location.
>
>> It makes
>> it confusing, one would no longer be able to tell which locations would be enabled
>> once you re-enable the parent breakpoint. You'd make "enable 3.2" seem to have
>> no effect on a disabled breakpoint, as "info break" would still show the location
>> as disabled.
>
> Why is this an important use case?
> It sounds much more useful and
> frequent to first enable the whole breakpoint, and only then enable or
> disable some of its particular locations. As long as the breakpoint
> is disabled, the internal status of enabled/disabled of its location
> is not important at all.
It is important to be able to look at the breakpoint list and understand that
some locations will be re-enabled once you enable the breakpoint, and which
those are. We may want to present the enabled locations slightly differently
when the parent breakpoint is disabled, but I disagree with showing them as
disabled in the same way you would show them if the user had disabled them
explicitly.
I think we're going in circles now.
>
> This is a user-level command, not a "maint" command. So what is of
> utmost importance is how this will actually behave, not what the
> software's internal state is.
A locations enable/disable state is not internal state, it is user-visible
state that the user can control. I don't understand why you are bringing
up such an odd "maint" command argument.
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Show enabled locations with disabled breakpoint parent as "y-" (Re: [PATCH 0/2] info breakpoints improvements)
2022-05-24 14:17 ` Pedro Alves
@ 2022-05-24 19:49 ` Pedro Alves
2022-05-25 13:57 ` Eli Zaretskii
0 siblings, 1 reply; 39+ messages in thread
From: Pedro Alves @ 2022-05-24 19:49 UTC (permalink / raw)
To: Andreas Schwab, Eli Zaretskii via Gdb-patches
On 2022-05-24 15:17, Pedro Alves wrote:
> On 2022-05-24 15:11, Andreas Schwab wrote:
>> On Mai 24 2022, Eli Zaretskii via Gdb-patches wrote:
>>
>>>> Date: Tue, 24 May 2022 14:50:01 +0100
>>>> Cc: luis.machado@arm.com, gdb-patches@sourceware.org
>>>> From: Pedro Alves <pedro@palves.net>
>>>>
>>>>>> A location only breaks if it is enabled, _and_ its parent is enabled, so never.
>>>>>
>>>>> That's what I thought. But then why not "propagate" the "n" of the
>>>>> disabled breakpoint to all of its locations?
>>>>
>>>> Because then when you re-enable the parent breakpoint, you'd have lost the enabled/disabled
>>>> state of the individual locations.
>>>
>>> I don't understand why would that be lost. I'm not proposing to
>>> actually disable each location, I propose to _display_ them as
>>> disabled in that case.
>>
>> I think it would be better in that case to omit the column or display it
>> as "-", to indicate that it is ignored right now.
>>
>
> Yes, we could print them differently, preserving the user-specified enable/disable
> state. We print them in uppercase N/Y when the location is disabled due to an invalid
> condition already, for example.
Here's a patch. Code, manual, NEWS, testcase, all included.
From 9675cd4b4948c6ca8f62d8c2cbadc42d4f94a1f9 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Tue, 24 May 2022 19:30:10 +0100
Subject: [PATCH] Show enabled locations with disabled breakpoint parent as
"y-"
Currently, breakpoint locations that are enabled while their parent
breakpoint is disabled are displayed with "y" in the Enb colum of
"info breakpoints":
(gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep n <MULTIPLE>
1.1 y 0x00000000000011b6 in ...
1.2 y 0x00000000000011c2 in ...
1.3 n 0x00000000000011ce in ...
Such locations won't trigger a break, so to avoid confusion, show "y-"
instead. For example:
(gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep n <MULTIPLE>
1.1 y- 0x00000000000011b6 in ...
1.2 y- 0x00000000000011c2 in ...
1.3 n 0x00000000000011ce in ...
The "-" sign is inspired on how the TUI represents breakpoints on the
left side of the source window, with "b-" for a disabled breakpoint.
Change-Id: I9952313743c51bf21b4b380c72360ef7d4396a09
---
gdb/doc/gdb.texinfo | 13 ++++++++
gdb/NEWS | 10 ++++++
gdb/breakpoint.c | 37 +++++++++++++++++++----
gdb/testsuite/gdb.cp/ena-dis-br-range.exp | 28 ++++++++---------
4 files changed, 68 insertions(+), 20 deletions(-)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index e5c1ee33aac..68679982919 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -4668,6 +4668,19 @@ in which case @value{GDBN} acts on all the locations in the range (inclusive).
Disabling or enabling the parent breakpoint (@pxref{Disabling}) affects
all of the locations that belong to that breakpoint.
+Locations that are enabled while their parent breakpoint is disabled
+won't trigger a break, and are denoted by @code{y-} in the @code{Enb}
+column. For example:
+
+@smallexample
+(@value{GDBP}) info breakpoints
+Num Type Disp Enb Address What
+1 breakpoint keep n <MULTIPLE>
+1.1 y- 0x00000000000011b6 in ...
+1.2 y- 0x00000000000011c2 in ...
+1.3 n 0x00000000000011ce in ...
+@end smallexample
+
@cindex pending breakpoints
It's quite common to have a breakpoint inside a shared library.
Shared libraries can be loaded and unloaded explicitly,
diff --git a/gdb/NEWS b/gdb/NEWS
index a72fee81550..52ffdc4c83a 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,16 @@
*** Changes since GDB 12
+* "info breakpoints" now displays enabled breakpoint locations of
+ disabled breakpoints as in the "y-" state. For example:
+
+ (gdb) info breakpoints
+ Num Type Disp Enb Address What
+ 1 breakpoint keep n <MULTIPLE>
+ 1.1 y- 0x00000000000011b6 in ...
+ 1.2 y- 0x00000000000011c2 in ...
+ 1.3 n 0x00000000000011ce in ...
+
* Support for Thread Local Storage (TLS) variables on FreeBSD arm and
aarch64 architectures.
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index eac6410a578..ed932a19ed7 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -6234,13 +6234,38 @@ print_one_breakpoint_location (struct breakpoint *b,
/* 4 */
annotate_field (3);
- /* For locations that are disabled because of an invalid condition,
- display "N*" on CLI, where "*" refers to a footnote below the
- table. For MI, simply display a "N" without a footnote. */
- const char *N = (uiout->is_mi_like_p ()) ? "N" : "N*";
if (part_of_multiple)
- uiout->field_string ("enabled", (loc->disabled_by_cond ? N
- : (loc->enabled ? "y" : "n")));
+ {
+ /* For locations that are disabled because of an invalid
+ condition, display "N*" on the CLI, where "*" refers to a
+ footnote below the table. For MI, simply display a "N"
+ without a footnote. On the CLI, for enabled locations whose
+ breakpoint is disabled, display "y-". */
+ auto get_enable_state = [uiout, loc] () -> const char *
+ {
+ if (uiout->is_mi_like_p ())
+ {
+ if (loc->disabled_by_cond)
+ return "N";
+ else if (!loc->enabled)
+ return "n";
+ else
+ return "y";
+ }
+ else
+ {
+ if (loc->disabled_by_cond)
+ return "N*";
+ else if (!loc->enabled)
+ return "n";
+ else if (!breakpoint_enabled (loc->owner))
+ return "y-";
+ else
+ return "y";
+ }
+ };
+ uiout->field_string ("enabled", get_enable_state ());
+ }
else
uiout->field_fmt ("enabled", "%c", bpenables[(int) b->enable_state]);
diff --git a/gdb/testsuite/gdb.cp/ena-dis-br-range.exp b/gdb/testsuite/gdb.cp/ena-dis-br-range.exp
index 782756e9ab1..83c9838aa42 100644
--- a/gdb/testsuite/gdb.cp/ena-dis-br-range.exp
+++ b/gdb/testsuite/gdb.cp/ena-dis-br-range.exp
@@ -44,10 +44,10 @@ proc make_info_breakpoint_reply_re {b1 b2 b21 b22 b23 b24} {
"1${ws}breakpoint keep ${b1}${ws}.* in marker\\(\\) at .*" \
"${ws}breakpoint already hit 1 time.*" \
"2${ws}breakpoint${ws}keep${ws}${b2}${ws}<MULTIPLE>.*" \
- "2.1${ws}${b21}.*" \
- "2.2${ws}${b22}.*" \
- "2.3${ws}${b23}.*" \
- "2.4${ws}${b24}.*" \
+ "2.1${ws}${b21}${ws}.*" \
+ "2.2${ws}${b22}${ws}.*" \
+ "2.3${ws}${b23}${ws}.*" \
+ "2.4${ws}${b24}${ws}.*" \
]
}
@@ -74,18 +74,18 @@ proc test_enable_disable {cmd b1 b2 b21 b22 b23 b24} {
test_enable_disable "disable 1" n y y y y y
test_enable_disable "enable 1" y y y y y y
-# Check that we can disable/disable a breakpoint with multiple
+# Check that we can disable/enable a breakpoint with multiple
# locations.
-test_enable_disable "disable 2" y n y y y y
-test_enable_disable "enable 2" y y y y y y
+test_enable_disable "disable 2" y n y- y- y- y-
+test_enable_disable "enable 2" y y y y y y
-# Check that we can disable/disable a range of breakpoints.
-test_enable_disable "disable 1-2" n n y y y y
-test_enable_disable "enable 1-2" y y y y y y
+# Check that we can disable/enable a range of breakpoints.
+test_enable_disable "disable 1-2" n n y- y- y- y-
+test_enable_disable "enable 1-2" y y y y y y
-# Check that we can disable/disable a list of breakpoints.
-test_enable_disable "disable 1 2" n n y y y y
-test_enable_disable "enable 1 2" y y y y y y
+# Check that we can disable/enable a list of breakpoints.
+test_enable_disable "disable 1 2" n n y- y- y- y-
+test_enable_disable "enable 1 2" y y y y y y
# Check that we can disable/enable a single location breakpoint.
test_enable_disable "disable 2.2" y y y n y y
@@ -100,7 +100,7 @@ test_enable_disable "enable 2.2-3" y y y y y y
test_enable_disable "disable 2.2-2" y y y n y y
test_enable_disable "enable 2.2-2" y y y y y y
-# Check that we can disable/disable a list of breakpoints that
+# Check that we can disable/enable a list of breakpoints that
# includes some elements with location ranges and others without.
test_enable_disable "disable 1 2.1 2.3-4" n y n y n n
test_enable_disable "enable 1 2.1 2.3-4" y y y y y y
base-commit: e8123c847f61c7458200b349615c47e9df17a0ed
--
2.36.0
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Show enabled locations with disabled breakpoint parent as "y-" (Re: [PATCH 0/2] info breakpoints improvements)
2022-05-24 19:49 ` [PATCH] Show enabled locations with disabled breakpoint parent as "y-" (Re: [PATCH 0/2] info breakpoints improvements) Pedro Alves
@ 2022-05-25 13:57 ` Eli Zaretskii
2022-05-25 19:19 ` Pedro Alves
0 siblings, 1 reply; 39+ messages in thread
From: Eli Zaretskii @ 2022-05-25 13:57 UTC (permalink / raw)
To: Pedro Alves; +Cc: schwab, gdb-patches
> Date: Tue, 24 May 2022 20:49:36 +0100
> From: Pedro Alves <pedro@palves.net>
> Cc: Eli Zaretskii <eliz@gnu.org>
>
> >> I think it would be better in that case to omit the column or display it
> >> as "-", to indicate that it is ignored right now.
> >>
> >
> > Yes, we could print them differently, preserving the user-specified enable/disable
> > state. We print them in uppercase N/Y when the location is disabled due to an invalid
> > condition already, for example.
>
> Here's a patch. Code, manual, NEWS, testcase, all included.
Thanks, the documentation parts are okay.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Show enabled locations with disabled breakpoint parent as "y-" (Re: [PATCH 0/2] info breakpoints improvements)
2022-05-25 13:57 ` Eli Zaretskii
@ 2022-05-25 19:19 ` Pedro Alves
0 siblings, 0 replies; 39+ messages in thread
From: Pedro Alves @ 2022-05-25 19:19 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: schwab, gdb-patches
On 2022-05-25 14:57, Eli Zaretskii wrote:
>> Date: Tue, 24 May 2022 20:49:36 +0100
>> From: Pedro Alves <pedro@palves.net>
>> Cc: Eli Zaretskii <eliz@gnu.org>
>>
>>>> I think it would be better in that case to omit the column or display it
>>>> as "-", to indicate that it is ignored right now.
>>>>
>>>
>>> Yes, we could print them differently, preserving the user-specified enable/disable
>>> state. We print them in uppercase N/Y when the location is disabled due to an invalid
>>> condition already, for example.
>>
>> Here's a patch. Code, manual, NEWS, testcase, all included.
>
> Thanks, the documentation parts are okay.
Thanks, I've merged it.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v2 1/2] Always show locations for breakpoints & show canonical location spec
2022-05-24 13:06 ` Eli Zaretskii
@ 2022-05-25 19:32 ` Pedro Alves
2022-05-26 12:48 ` Eli Zaretskii
0 siblings, 1 reply; 39+ messages in thread
From: Pedro Alves @ 2022-05-25 19:32 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
On 2022-05-24 14:06, Eli Zaretskii wrote:
>
>>>> +Enabled breakpoints and breakpoint locations are marked with @samp{y}.
>>>
>>> What does "enabled breakpoint location" mean? One cannot enable a
>>> location.
>>
>> Sure we can.
>
> We are mis-communicating. "Location" is a frequently-used word that
> has certain natural meaning in many contexts. That natural meaning
> doesn't support the notion of "enabling".
Well, yeah, but above the text is very clearly talking about "breakpoint
locations", i.e., the locations in the program where the breakpoint is armed.
Enabling/disabling one of these locations is really about enabling/disabling
the arming of a breakpoint at such a location. There is no need to be
concerned about the general term here, the context doesn't leave
scope for ambiguity. Even though, the term does make sense here.
> You use "location" in a
> very special (and IMO problematic, as I tried to explain later in my
> comments) meaning, and in that meaning a "location" indeed _can_ be
> enabled. But when reading this text, that meaning was not yet clear
> to me, if it ever becomes clear later, and thus this text made me
> raise a brow. I described the surprise which I felt while reading the
> text as a feedback in the hope that we will be able to make the manual
> more clear and less confusing upon first, linear, top-to-bottom
> reading, and avoid the need for the reader to repeatedly read the same
> text before its meaning dawns on the reader.
I think we can get rid of any confusion by naming the input location
spec, the arguments you pass to commands, differently.
I've just sent a separate patch about this, which cleans up the manual in
that direction, and you'll notice that it isn't really about "info breakpoints".
In fact, it barely touches that section. It's a lot more general. Please take a
fresh look at that one. I'll then get back to these "info breakpoints" changes on
top of it, and I think we will have a much better foundation.
Here is the URL to the patch in question:
[PATCH] gdb/manual: Introduce locspecs
https://sourceware.org/pipermail/gdb-patches/2022-May/189417.html
>
>> So I'm actually proposing to improve things by calling the user-input
>> text as "location specification", distinct from the actual locations
>> the specification matches or resolves to.
>
> My suggestion is to use "location" (with or without additional
> qualifiers) for only one of these two, if only because we tend to
> shorthand that by using just "location". We could use it for what you
> now call "location specification", which would allow us to use just
> "location" as it shorthand. With your proposed text, "location" is
> ambiguous, because it could allude to any of these two. The other
> meaning of "location" I propose to call "address in the code" or
> "address" for short -- because that's what it conceptually is. (If
> you are bothered by the case of unloaded shared library and similar
> situations, we could say "unresolved address" in those cases -- which
> are rather marginal, so once explained, they can be ignored in the
> rest of the text. By contrast, these two "locations" are pervasive:
> we need to refer to them all over the manual. So it's much more
> important IMO to make that part of our terminology clear and
> unequivocal.)
I really think that this is the wrong direction, and working on
the patch that I pointed at above really convinced me so even more.
I believe that that patch will convince you as well. Please take a look
there.
As another data point, after writing that other patch, i.e., just now,
I went to look what other debuggers call similar things, and here's what I saw:
- Delve, the Go debugger, calls what I'm suggesting to call
"Location Specification" + shorthand "locspec", as "Location Specifier".
See:
https://github.com/go-delve/delve/blob/master/Documentation/cli/locspec.md
That's very close -- specification vs specifier. I could go with "Specifier" too
instead of "Specification" if people prefer it.
Note how their "break" command is documented as:
"break [name] [locspec]"
https://github.com/go-delve/delve/tree/master/Documentation/cli#break
That's very close to what I proposed in the patch I just sent, as it uses
the same "locspec" shorthand. Nice coincidence.
They also have wording using "matching", as in
"If called with the locspec argument it will delete all the breakpoints matching the locspec.
If locspec is omitted all breakpoints are deleted."
... which is the wording I used in that patch as well. I swear I didn't find this until
after I wrote my patch. It should give some credence at this being a good candidate
for a term of art.
- LLDB calls "locations" to what GDB calls breakpoint locations too:
See here:
https://lldb.llvm.org/use/tutorial.html#setting-breakpoints
They use the "resolve to one or more locations" wording too. See:
"Note that setting a breakpoint creates a logical breakpoint, which could resolve to one or more
locations. For instance, break by selector would set a breakpoint on all the methods that implement
that selector in the classes in your program. Similarly, a file and line breakpoint might result in multiple
locations if that file and line were inlined in different places in your code."
Here, lldb debugging gdb, note how the user interface uses "location" just like GDB.
Setting a breakpoint:
(lldb) b main
Breakpoint 1: 24 locations.
^^^^^^^^^^^^
Listing breakpoints:
(lldb) break list
Current breakpoints:
1: name = 'main', locations = 24
^^^^^^^^^^^^^^
1.1: where = gdb`main + 34 at gdb.c:28:10, address = gdb[0x00000000000ed07b], unresolved, hit count = 0
1.2: where = gdb`main + 8 at 13650.cc:45:9, address = gdb[0x0000000000869cf3], unresolved, hit count = 0
1.3: where = gdb`main + 8 at 1.cc:39:9, address = gdb[0x0000000000869eb3], unresolved, hit count = 0
1.4: where = gdb`main + 8 at 1.cc:160:9, address = gdb[0x000000000086a476], unresolved, hit count = 0
1.5: where = gdb`main + 8 at 2.cc:158:9, address = gdb[0x000000000086a9d8], unresolved, hit count = 0
1.6: where = gdb`main + 8 at 3.cc:158:9, address = gdb[0x000000000086b039], unresolved, hit count = 0
1.7: where = gdb`main + 8 at 4.cc:40:9, address = gdb[0x000000000086b0e4], unresolved, hit count = 0
1.8: where = gdb`main + 8 at 1.cc:90:9, address = gdb[0x000000000086b6b7], unresolved, hit count = 0
1.9: where = gdb`main + 8 at 2.cc:48:9, address = gdb[0x000000000086b910], unresolved, hit count = 0
1.10: where = gdb`main + 8 at 3.cc:63:9, address = gdb[0x000000000086bcc4], unresolved, hit count = 0
1.11: where = gdb`main + 8 at 1.cc:74:9, address = gdb[0x000000000086c001], unresolved, hit count = 0
1.12: where = gdb`main + 8 at 2.cc:366:9, address = gdb[0x000000000086de53], unresolved, hit count = 0
1.13: where = gdb`main + 8 at 1.cc:41:9, address = gdb[0x0000000000869de2], unresolved, hit count = 0
1.14: where = gdb`main + 8 at 1.cc:127:9, address = gdb[0x0000000000869bb0], unresolved, hit count = 0
1.15: where = gdb`main + 8 at 1.cc:58:9, address = gdb[0x00000000008693ed], unresolved, hit count = 0
1.16: where = gdb`main + 8 at 1.cc:58:9, address = gdb[0x00000000008692a1], unresolved, hit count = 0
1.17: where = gdb`main + 8 at 2.cc:84:9, address = gdb[0x0000000000869135], unresolved, hit count = 0
1.18: where = gdb`main + 8 at front_back.cc:38:9, address = gdb[0x0000000000868d09], unresolved, hit count = 0
1.19: where = gdb`main + 27 at empty.cc:27:22, address = gdb[0x0000000000868b0a], unresolved, hit count = 0
1.20: where = gdb`main + 8 at 1.cc:66:9, address = gdb[0x0000000000868ae3], unresolved, hit count = 0
1.21: where = gdb`main + 8 at 3.cc:34:9, address = gdb[0x0000000000868912], unresolved, hit count = 0
1.22: where = gdb`main + 8 at 2.cc:41:9, address = gdb[0x00000000008688ac], unresolved, hit count = 0
1.23: where = gdb`main + 8 at 1.cc:62:9, address = gdb[0x00000000008687bb], unresolved, hit count = 0
1.24: where = gdb`main + 8 at 1.cc:167:9, address = gdb[0x000000000086839c], unresolved, hit count = 0
2: name = 'handle_inferior_event', locations = 1
2.1: where = gdb`::handle_inferior_event(execution_control_state *) + 44 at infrun.c:5335:21, address = gdb[0x00000000004ea21d], unresolved, hit count = 0
(lldb)
Disabling a location:
(lldb) break disable 1.50
error: '1.50' is not a currently valid breakpoint/location id.
^^^^^^^^^^^^^^^^^^^
Curiously, a single-location breakpoint is shown just like multi-location breakpoints too, there's no
distinction. See breakpoint 2 above. And they show the original breakpoint spec in the "name = ..." field,
it's not lost for them like it is in gdb's "info breakpoints".
OOC, here's what they show for a disabled breakpoint with enabled locations (except location 1.3, I disabled that one):
(lldb) break list
Current breakpoints:
1: name = 'main', locations = 24 Options: disabled
1.1: where = gdb`main + 34 at gdb.c:28:10, address = gdb[0x00000000000ed07b], unresolved, hit count = 0
1.2: where = gdb`main + 8 at 13650.cc:45:9, address = gdb[0x0000000000869cf3], unresolved, hit count = 0
1.3: where = gdb`main + 8 at 1.cc:39:9, address = gdb[0x0000000000869eb3], unresolved, hit count = 0 Options: disabled
1.4: where = gdb`main + 8 at 1.cc:160:9, address = gdb[0x000000000086a476], unresolved, hit count = 0
1.5: where = gdb`main + 8 at 2.cc:158:9, address = gdb[0x000000000086a9d8], unresolved, hit count = 0
1.6: where = gdb`main + 8 at 3.cc:158:9, address = gdb[0x000000000086b039], unresolved, hit count = 0
...
>
> Can you tel concretely why you object to changing the "location"
> terminology along these lines? Even if you think the current
> terminology is okay, what will we lose by switching the terminology I
> propose above?
You lose the fact that it is the right name for it, and the fact that everyone
understands what it means today, so you force everyone to learn something new
for no good reason. Also, the list of breakpoint locations in MI is exposed in the
field called ... "locations". And there is a proposal pending on the list to
expose the breakpoint locations to Python, via a "locations" attribute...
>
>> Even GDB's source code uses "location" for both -- struct bp_location
>> for the breakpoint locations, the "1.1", "1.2" things, stored in each
>> struct breakpoint in the breakpoint::loc field, as a linked list:
>
> I know. But I hope we can avoid terminology ambiguities even though
> they are present in the code. The reader of the GDB manual will not
> necessarily read the GDB source at the same time, and is not required
> to do so if all they want is to learn how to use GDB.
My point is that the ambiguity already exists, is old, but I plan to
do something about, including fixing the code, and was am telling you
what my plan for the source code was.
> Preexisting text is not sacred. It's okay to modify or even delete
> parts of the old text where they are sub-optimal or aren't
> contributing to the manual enough to justify their existence.
Certainly. But it is also the case that you should expect that a contributor
will assume that the terms used in current text will have already been vetted
when the text was originally added, so reusing them or moving text around shouldn't
require rewriting everything.
>
> Also, please understand that when presented with a large rewrite of
> the manual, I normally cannot go back to the old text and refrain from
> commenting on some parts that were more or less verbatim copied from
> the old version. Instead, I read the new text as a whole and comment
> on that, since that is what will the reader see after the changes are
> installed.
I somewhat understand that (don't understand the "cannot", you could do
that for terms you find suspicious, if not for the whole text), though I also
believe that can lead to people feeling like they're getting unfair
requirements.
>>> I'm not sure it is a good idea to talk about "resolved locations",
>>> here and elsewhere. Why not "resolved addresses", as the original
>>> text did? Using "location" here introduces a conceptual ambiguity,
>>> whereby a "location" could be in the source code and also in the
>>> executable code, and that overloading of concepts makes this complex
>>> issue even harder to understand. My suggestion is to use "location"
>>> for source-level location specs, and for nothing else.
>>
>> I disagree with that, because that's not how GDB behaves.
>> [...]
>
> I'm disappointed by such a wholesale rejection of these parts of my
> comments. The terminology issue is very important, at least IMO, and
> the rest of the review circles around it and its various aspects. If
> you disagree with it so strongly, almost all the non-trivial parts of
> my review can be just ignored, because they are based on this
> difficulty in the terminology.
>
> I'm not sure I know how to proceed, given that we have such a basic
> disagreement on these aspects of the changes for the manual.
It should be OK to have disagreements, I hope. I tried to explain
why I disagreed. You can disagree back and explain why. At some point
we should reach somewhere in the middle. But let me try again.
When you set a breakpoint, you pass the "break" command some arguments, which
can be a function name, a file name, a line number, a stap probe,
an address, and more. There are three formats you can use to
specify where you want the breakpoints. Linespecs, explicit locations,
and address locations. Once you pass this input to GDB, gdb will
find the matching locations, and will tell you how many it found.
For example:
(top-gdb) b main
Breakpoint 3 at 0xed06c: main. (24 locations)
It is not correct to think of the locations as just "addresses",
because they are more than that. For example here are three breakpoints
set at different functions, one of them inlined. Note that both breakpoints'
locations have the same address, though different function names and line numbers:
(gdb) b func_extern_caller
Breakpoint 1 at 0x5555555552d3: file /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.opt/inline-break.c, line 208.
(gdb) b func_inline_caller
Note: breakpoint 1 also set at pc 0x5555555552d3.
Breakpoint 2 at 0x5555555552d3: file /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.opt/inline-break.c, line 199.
(gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y 0x00005555555552d3 in func_extern_caller at /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.opt/inline-break.c:208
2 breakpoint keep y 0x00005555555552d3 in func_inline_caller at /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.opt/inline-break.c:199
^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^ ^^^
same! different... different...
So they should be considered different locations. Running to each of the breakpoints will make GDB
present the stop differently:
(gdb) disable
(gdb) enable 1
(gdb) r
Starting program: /home/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.opt/inline-break/inline-break
Breakpoint 1, func_extern_caller (x=1) at /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.opt/inline-break.c:208
208 return func_inline_caller (x);
(gdb) disable
(gdb) enable 2
(gdb) r
The program being debugged has been started already.
Start it from the beginning? (y or n) y
Starting program: /home/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.opt/inline-break/inline-break
Breakpoint 2, func_inline_caller (x=1) at /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.opt/inline-break.c:199
199 return func_inline_callee (x);
Also, the local variables you can access in each of the breakpoints's conditions are
different, the breakpoints's commands will see a different context, etc., as the breakpoints
are set in different functions, even though they have the same addresses.
So calling breakpoint locations "addresses" instead if just incorrect.
>
>>>> +This field is present for ordinary breakpoints and tracepoints.
>>>
>>> What is an "ordinary" breakpoint, and how is it different from the
>>> other kinds? (There are other places in the new text that use this
>>> unexplained terminology.)
>>
>> I've just using the terminology already used in this table. See:
>
> When I say "not explained" I mean there's no text which says something
> like "an ordinary breakpoint is ...", or some text that explains what
> are the "not ordinary" breakpoints, and by doing that explains
> implicitly what are the "ordinary" ones. If the old text didn't have
> that, it means that old text was also sub-optimal. (And my reading of
> these changes to the manual led me to believe that this distinction
> will be more important once the behavior changes, because you modified
> the original text even where the old one was not invalidated by the
> changes in behavior.) Thus the comment.
>
OK, but you have to understand that if a contributor just copies a term from
the surrounding text, they'll think that it should be OK to use it
again, otherwise how did the term get into the manual in the first place?
Requiring that contributions that just add more instances of the same clean up
the prior mess isn't fair, because adding one more use of the same term will not
make any difference to the user's understanding, nor to the work necessary to clarify
elsewhere in some intro text what the term means. That can be done as an
orthogonal change.
> And this part of the patch:
>
>> -A location in a multi-location breakpoint is represented as a tuple with the
>> -following fields:
>> +Each location in an ordinary breakpoint or tracepoint is represented
>> +as a tuple with the following fields:
>
> led me to believe that "ordinary" somehow is related to
> "multi-location".
>
>> Thus "ordinary" here refers to actual code breakpoints, as opposed to the generic "breakpoint"
>> term meaning all of code breakpoints, watchpoints, tracepoints, catchpoints.
>
> How about saying that explicitly, before we start relying on this
> terminology?
The text I'm changing _already_ relies on that terminology, so "before we start"
doesn't apply. I agree it would be nice to clarify this, but I don't see why it has
to be in this patch. I actually think it should be a separate patch. There are
references to "regular breakpoints" too, to mean the same thing. And I'm starting to
use "code breakpoints" more myself, as I think it has more meaning (contrast
with "data breakpoint", which is a term many are familiar as alternative to
our "watchpoint").
>
>> Below is the updated patch. Only the documentation changed.
>
> Given that you rejected most of the important parts of my comments,
> I'm not sure how to proceed with reviewing this new version. The main
> part of my review is the suggestion to find better, more intuitive
> terminology for "breakpoints" vs "header rows" in the "info break"
> display, and also better terminology for the two types of "locations".
> If this is left as it was in the original patch, what should I look at
> in the revised patch?
It wasn't left as it was, the v2 patch is different, as I had explained
with some bullet points where I detailed the difference, which you've skipped.
But nevermind, let's focus on discussing the patch I pointed at at the
top of this email.
The "info breakpoints" improvements changes should be much smaller once
I rebase them on that patch (after it goes in).
>
> Please understand, like I explained above, that I read the new text
> after the patch as a whole, and base my review on what it tells me and
> how it describes the GDB features. It's not just diffs to me, it's a
> more-or-less complete document, and it must make sense as a whole.
> Currently, it doesn't, due to those terminology problems.
Please understand that when writing the patch I too look at
the manual as a whole. I constantly build the html of the manual
when I'm editing it, and re-read the whole section or sections I am changing,
trying to make sure everything fits together. I sometimes spend hours tweaking
small details here and there until I have something that I think is good. The
changes I proposed made sense to me from that angle.
In the end, we all want the manual to improve.
>
> If we keep this terminology, the relevant sections of the manual will
> keep confusing me. The result is that I will keep mentioning this in
> my future reviews, and that makes little sense to me.
I think the patch I posted to introduce locspecs will fix that. Please take
a look at it.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v2 1/2] Always show locations for breakpoints & show canonical location spec
2022-05-25 19:32 ` Pedro Alves
@ 2022-05-26 12:48 ` Eli Zaretskii
2022-05-26 14:04 ` Pedro Alves
0 siblings, 1 reply; 39+ messages in thread
From: Eli Zaretskii @ 2022-05-26 12:48 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> Date: Wed, 25 May 2022 20:32:24 +0100
> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <pedro@palves.net>
>
> On 2022-05-24 14:06, Eli Zaretskii wrote:
>
> >
> >>>> +Enabled breakpoints and breakpoint locations are marked with @samp{y}.
> >>>
> >>> What does "enabled breakpoint location" mean? One cannot enable a
> >>> location.
> >>
> >> Sure we can.
> >
> > We are mis-communicating. "Location" is a frequently-used word that
> > has certain natural meaning in many contexts. That natural meaning
> > doesn't support the notion of "enabling".
>
> Well, yeah, but above the text is very clearly talking about "breakpoint
> locations", i.e., the locations in the program where the breakpoint is armed.
> Enabling/disabling one of these locations is really about enabling/disabling
> the arming of a breakpoint at such a location.
Yes, and that's the point I tried to make: you can enable or disable a
breakpoint at some location, not enable/disable the location itself,
which is what the text was saying.
> > My suggestion is to use "location" (with or without additional
> > qualifiers) for only one of these two, if only because we tend to
> > shorthand that by using just "location". We could use it for what you
> > now call "location specification", which would allow us to use just
> > "location" as it shorthand. With your proposed text, "location" is
> > ambiguous, because it could allude to any of these two. The other
> > meaning of "location" I propose to call "address in the code" or
> > "address" for short -- because that's what it conceptually is. (If
> > you are bothered by the case of unloaded shared library and similar
> > situations, we could say "unresolved address" in those cases -- which
> > are rather marginal, so once explained, they can be ignored in the
> > rest of the text. By contrast, these two "locations" are pervasive:
> > we need to refer to them all over the manual. So it's much more
> > important IMO to make that part of our terminology clear and
> > unequivocal.)
>
> I really think that this is the wrong direction, and working on
> the patch that I pointed at above really convinced me so even more.
> I believe that that patch will convince you as well. Please take a look
> there.
I did, and it didn't. It actually convinced me even more that we
cannot use "location" in both contexts without creating confusion.
> As another data point, after writing that other patch, i.e., just now,
> I went to look what other debuggers call similar things, and here's what I saw:
Other debuggers could mean other things, or they could be also wrong
in this aspect. Our manual should make sense to us, even if other
debuggers decide to use different terminology. Likewise when adopting
terminology used by others.
Moreover, half if not more of the evidence you provide is about things
I'm not objected to: I'm okay with calling "location spec" what was
formerly known as "linespec". My problem is with the other use of
"location".
> > Can you tel concretely why you object to changing the "location"
> > terminology along these lines? Even if you think the current
> > terminology is okay, what will we lose by switching the terminology I
> > propose above?
>
> You lose the fact that it is the right name for it, and the fact that everyone
> understands what it means today, so you force everyone to learn something new
> for no good reason.
"Breakpoint address" is nothing new, either. We use it all the time,
so I see no need for everyone to relearn.
> >> Even GDB's source code uses "location" for both -- struct bp_location
> >> for the breakpoint locations, the "1.1", "1.2" things, stored in each
> >> struct breakpoint in the breakpoint::loc field, as a linked list:
> >
> > I know. But I hope we can avoid terminology ambiguities even though
> > they are present in the code. The reader of the GDB manual will not
> > necessarily read the GDB source at the same time, and is not required
> > to do so if all they want is to learn how to use GDB.
>
> My point is that the ambiguity already exists, is old, but I plan to
> do something about, including fixing the code, and was am telling you
> what my plan for the source code was.
We agree about the general goal, but disagree about some minor point,
such as what to call "the place where we insert the breakpoint".
> > Preexisting text is not sacred. It's okay to modify or even delete
> > parts of the old text where they are sub-optimal or aren't
> > contributing to the manual enough to justify their existence.
>
> Certainly. But it is also the case that you should expect that a contributor
> will assume that the terms used in current text will have already been vetted
> when the text was originally added, so reusing them or moving text around shouldn't
> require rewriting everything.
I think it is reasonable to improve text borrowed from previous
versions while we are doing significant changes in the area. Such
changes are a good opportunity to improve the clarity of the manual.
Minor cleanups as part of more significant changes, both in code and
in the documentation, and are routinely done in many projects,
including in GDB. So I don't think my requests to change what was
copied from the preexisting text are unreasonable.
> It is not correct to think of the locations as just "addresses",
> because they are more than that. For example here are three breakpoints
> set at different functions, one of them inlined. Note that both breakpoints'
> locations have the same address, though different function names and line numbers:
>
> (gdb) b func_extern_caller
> Breakpoint 1 at 0x5555555552d3: file /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.opt/inline-break.c, line 208.
> (gdb) b func_inline_caller
> Note: breakpoint 1 also set at pc 0x5555555552d3.
> Breakpoint 2 at 0x5555555552d3: file /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.opt/inline-break.c, line 199.
> (gdb) info breakpoints
> Num Type Disp Enb Address What
> 1 breakpoint keep y 0x00005555555552d3 in func_extern_caller at /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.opt/inline-break.c:208
> 2 breakpoint keep y 0x00005555555552d3 in func_inline_caller at /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.opt/inline-break.c:199
> ^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^ ^^^
> same! different... different...
>
I'm afraid I don't see the problem. Sure, the addresses are
identical, but these are two different breakpoints: they have
different numbers. I never said that if the addresses are identical,
the corresponding breakpoints are also equal: that is definitely not
true. A breakpoint has many attributes besides the address where it
breaks: conditions, commands, etc.
> So they should be considered different locations.
> Also, the local variables you can access in each of the breakpoints's conditions are
> different, the breakpoints's commands will see a different context, etc., as the breakpoints
> are set in different functions, even though they have the same addresses.
Well, of course. As I said, a breakpoint has many attributes, not
just the code address.
But now I'm beginning to wonder what is your definition of "location"
in this context? What does it entail if two different "locations" can
be resolved to the same code address? And where is that meaning of
"location" described? The text you proposed uses "location(s) that
match(es) the location spec", which doesn't seem to imply any
attributes except the "place" where the breakpoint will be set, since
"location spec" is just source-level description of one or more such
"places". How does "location" imply anything about local variables,
commands, etc.?
> So calling breakpoint locations "addresses" instead if just incorrect.
You seem to use "location" in some generalized sense, to mean many
things in addition to the place in the program, which I don't yet
fully understand. But if so, why is it okay to use "location" in such
a generalized sense, but it is not okay to make a similar
generalization of "breakpoint address"?
I'm not wedded to "address", btw, I just don't want us to use
"location" for that. If there's some alternative term, we could
consider it. Although "address" is already being used for a very
similar, if not the same, thing, and thus is a natural candidate,
because people won't need to learn a new term.
> > When I say "not explained" I mean there's no text which says something
> > like "an ordinary breakpoint is ...", or some text that explains what
> > are the "not ordinary" breakpoints, and by doing that explains
> > implicitly what are the "ordinary" ones. If the old text didn't have
> > that, it means that old text was also sub-optimal. (And my reading of
> > these changes to the manual led me to believe that this distinction
> > will be more important once the behavior changes, because you modified
> > the original text even where the old one was not invalidated by the
> > changes in behavior.) Thus the comment.
> >
>
> OK, but you have to understand that if a contributor just copies a term from
> the surrounding text, they'll think that it should be OK to use it
> again, otherwise how did the term get into the manual in the first place?
> Requiring that contributions that just add more instances of the same clean up
> the prior mess isn't fair, because adding one more use of the same term will not
> make any difference to the user's understanding, nor to the work necessary to clarify
> elsewhere in some intro text what the term means. That can be done as an
> orthogonal change.
I don't see why we would need to wait for a separate patch. What is
the advantage of that? If the problem is that it's a lot of work for
you, you need just to ask, and I will write the explanation myself, so
you could use it in your patch.
> >> Thus "ordinary" here refers to actual code breakpoints, as opposed to the generic "breakpoint"
> >> term meaning all of code breakpoints, watchpoints, tracepoints, catchpoints.
> >
> > How about saying that explicitly, before we start relying on this
> > terminology?
>
> The text I'm changing _already_ relies on that terminology, so "before we start"
> doesn't apply.
I meant "before" in the reading order of the text, not in
chronological sense of when this text was originally written.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v2 1/2] Always show locations for breakpoints & show canonical location spec
2022-05-26 12:48 ` Eli Zaretskii
@ 2022-05-26 14:04 ` Pedro Alves
2022-05-26 15:03 ` Eli Zaretskii
0 siblings, 1 reply; 39+ messages in thread
From: Pedro Alves @ 2022-05-26 14:04 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
On 2022-05-26 13:48, Eli Zaretskii wrote:
>> Date: Wed, 25 May 2022 20:32:24 +0100
>> Cc: gdb-patches@sourceware.org
>> From: Pedro Alves <pedro@palves.net>
>>
>> On 2022-05-24 14:06, Eli Zaretskii wrote:
>>
>>>
>>>>>> +Enabled breakpoints and breakpoint locations are marked with @samp{y}.
>>>>>
>>>>> What does "enabled breakpoint location" mean? One cannot enable a
>>>>> location.
>>>>
>>>> Sure we can.
>>>
>>> We are mis-communicating. "Location" is a frequently-used word that
>>> has certain natural meaning in many contexts. That natural meaning
>>> doesn't support the notion of "enabling".
>>
>> Well, yeah, but above the text is very clearly talking about "breakpoint
>> locations", i.e., the locations in the program where the breakpoint is armed.
>> Enabling/disabling one of these locations is really about enabling/disabling
>> the arming of a breakpoint at such a location.
>
> Yes, and that's the point I tried to make: you can enable or disable a
> breakpoint at some location, not enable/disable the location itself,
> which is what the text was saying.
"enable or disable a breakpoint at some location" is slightly ambiguous because
it doesn't distinguish between the whole breakpoint vs one individual location
the breakpoint is set at. There is one logical breakpoint, and then one or
more individual locations that the single/singular breakpoint is set at.
See the difference?
So we can say instead:
you can enable/disable each individual location the breakpoint is set at.
and that is more accurate.
>
>>> My suggestion is to use "location" (with or without additional
>>> qualifiers) for only one of these two, if only because we tend to
>>> shorthand that by using just "location". We could use it for what you
>>> now call "location specification", which would allow us to use just
>>> "location" as it shorthand. With your proposed text, "location" is
>>> ambiguous, because it could allude to any of these two. The other
>>> meaning of "location" I propose to call "address in the code" or
>>> "address" for short -- because that's what it conceptually is. (If
>>> you are bothered by the case of unloaded shared library and similar
>>> situations, we could say "unresolved address" in those cases -- which
>>> are rather marginal, so once explained, they can be ignored in the
>>> rest of the text. By contrast, these two "locations" are pervasive:
>>> we need to refer to them all over the manual. So it's much more
>>> important IMO to make that part of our terminology clear and
>>> unequivocal.)
>>
>> I really think that this is the wrong direction, and working on
>> the patch that I pointed at above really convinced me so even more.
>> I believe that that patch will convince you as well. Please take a look
>> there.
>
> I did, and it didn't. It actually convinced me even more that we
> cannot use "location" in both contexts without creating confusion.
What a mystery. The current text uses location throughout and it
isn't a problem.
I would like to hear from anyone who is confused, what misunderstanding
said confusion actually caused.
>
>> As another data point, after writing that other patch, i.e., just now,
>> I went to look what other debuggers call similar things, and here's what I saw:
>
> Other debuggers could mean other things, or they could be also wrong
> in this aspect. Our manual should make sense to us, even if other
> debuggers decide to use different terminology. Likewise when adopting
> terminology used by others.
>
> Moreover, half if not more of the evidence you provide is about things
> I'm not objected to: I'm okay with calling "location spec" what was
> formerly known as "linespec". My problem is with the other use of
> "location".
GDB has been calling the actual locations a breakpoint is set at,
as "locations", since forever. Other debuggers do the same. And now,
OOTB, you decide that the term is confusing. Who exactly are you protecting?
Nobody is confused by this.
> We agree about the general goal, but disagree about some minor point,
> such as what to call "the place where we insert the breakpoint".
It is not a minor point. You are suggesting to change the naming of
something that is very core to how breakpoints are displayed to users
and they are manipulated by gdb users, both CLI and user interfaces (MI).
> But now I'm beginning to wonder what is your definition of "location"
> in this context? What does it entail if two different "locations" can
> be resolved to the same code address? And where is that meaning of
> "location" described? The text you proposed uses "location(s) that
> match(es) the location spec", which doesn't seem to imply any
> attributes except the "place" where the breakpoint will be set, since
> "location spec" is just source-level description of one or more such
> "places". How does "location" imply anything about local variables,
> commands, etc.?
A location is some actual place in the program, identifiable by
some "coordinates". Some source line in the program can be a location
in the program, if you have its coordinates. Line number alone isn't sufficient.
A source location is fully defined by the path to its file and the line number, the
full prototype of its function, and the address of the first instruction that the line was
compiled down to. Addresses are per-inferior, so add inferior to the coordinate system. If
you have the same code loaded in multiple inferiors, GDB considers the same code at each
address its own location. E.g. here's a breakpoint set at handle_inferior_event, with two inferior gdbs loaded:
3 breakpoint keep y <MULTIPLE>
3.1 y 0x00000000004ea2ac in handle_inferior_event(execution_control_state*) at /home/pedro/gdb/binutils-gdb/src/gdb/infrun.c:5331 inf 1
3.2 y 0x00000000004ea2ac in handle_inferior_event(execution_control_state*) at /home/pedro/gdb/binutils-gdb/src/gdb/infrun.c:5331 inf 2
Note "inf 1" and "inf 2" on the right hand side. So you have two locations for the same code, in two inferiors.
You may not have debug info for all locations in the program. So a location
may be missing some coordinates, like the file/line. It won't be a source location,
but it is still some actual program location. Or you may lack the full path
to the filename. Etc. But the tuple of the coordinates identify some actual spot in
the actual program. Address alone isn't sufficient to identify source location,
given inlining. Of course, if you don't have debug info, address is all you got.
OTOH, a location specification is a way to find or refer to these actual locations in the program.
A location spec may leave out some parts of the "coordinates". For example,
it can just give a relative filename, or a filename with no directory
components, or even not specify a filename at all, just a line number.
Or, it can specify a function name without specifying the full prototype, like the
function arguments, or the namespace and class the function/method belongs to,
in C++, for example.
The point is that the spec may be incomplete, and GDB will do its best to fill in the
missing bits from context, and find all the locations in the program that match the
incomplete specification.
So for example, with this toy C++ program:
$ cat foo.cc
void func (int) {}
void func (long) {}
namespace A { void func (long) {} }
int main () {}
"func" would be a location specification, and the matching locations that GDB
finds would be the actual locations that exist in the program. Like:
(gdb) b func
Breakpoint 1 at 0x40110d: func. (3 locations)
(gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
1.1 y 0x000000000040110d in func(int) at foo.cc:1
1.2 y 0x0000000000401118 in func(long) at foo.cc:2
1.3 y 0x0000000000401123 in A::func(long) at foo.cc:3
(gdb)
So would a location spec like "-line 10". That one doesn't identify a location
by itself, but GDB fills in the missing bits.
This is the problem with the current manual. It uses "location" to refer to the
potentially incomplete location specification. Switching to using "location spec"
gets rid of the ambiguity.
Same thing for all the other examples in the manual that I added to the "Address Specifications"
section.
This idea of "actual locations" is used the same way throughout multiple commands,
not just breakpoints. It just happens that with breakpoint, GDB records the list
of locations it found, so it is easier to discuss using breakpoints. But e.g.
the "list" command does exactly the same "spec to actual locations in the program"
matching.
I would like to point out the fact that we've been referring to "location spec"
vs "actual location" in these emails (not the patches), and no one ends up confused.
Please consider that.
> I'm not wedded to "address", btw, I just don't want us to use
> "location" for that. If there's some alternative term, we could
> consider it. Although "address" is already being used for a very
> similar, if not the same, thing, and thus is a natural candidate,
> because people won't need to learn a new term.
Can we please just focus on documenting how GDB works? That's what
the manual should be doing. GDB calls these things breakpoint locations,
and it is not going to change because of the manual. That'd backwards.
We don't even have to do anything, just keep using the same name we've
always used. Please!
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v2 1/2] Always show locations for breakpoints & show canonical location spec
2022-05-26 14:04 ` Pedro Alves
@ 2022-05-26 15:03 ` Eli Zaretskii
2022-05-26 15:10 ` Pedro Alves
2022-05-26 19:29 ` Pedro Alves
0 siblings, 2 replies; 39+ messages in thread
From: Eli Zaretskii @ 2022-05-26 15:03 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> Date: Thu, 26 May 2022 15:04:21 +0100
> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <pedro@palves.net>
>
> > Yes, and that's the point I tried to make: you can enable or disable a
> > breakpoint at some location, not enable/disable the location itself,
> > which is what the text was saying.
>
> "enable or disable a breakpoint at some location" is slightly ambiguous because
> it doesn't distinguish between the whole breakpoint vs one individual location
> the breakpoint is set at.
How so? it says "disable a breakpoint AT SOME LOCATION". IOW, the
breakpoint is disabled/enabled only at some location.
> >> I really think that this is the wrong direction, and working on
> >> the patch that I pointed at above really convinced me so even more.
> >> I believe that that patch will convince you as well. Please take a look
> >> there.
> >
> > I did, and it didn't. It actually convinced me even more that we
> > cannot use "location" in both contexts without creating confusion.
>
> What a mystery. The current text uses location throughout and it
> isn't a problem.
I explained why in another message: we were using "location" in just
one sense, now we are supposed to be using it in two senses: one for
"specifying" a location, the other for the location itself. In many
situations in life the thing and its specification are one and the
same, or tightly coupled. E.g., we frequently say in the
documentation of a program stuff like
This function deletes the specified FILE...
which doesn't distinguish between FILE and its specification.
We are now supposed to distinguish between a "location" and its
"specification". This is not easy to do, and I think it risks
creating a confusion. This is based both on experience with other
things that are defined through some sort of "specification", and on
actual reading the text of your patches.
> > We agree about the general goal, but disagree about some minor point,
> > such as what to call "the place where we insert the breakpoint".
>
> It is not a minor point. You are suggesting to change the naming of
> something that is very core to how breakpoints are displayed to users
> and they are manipulated by gdb users, both CLI and user interfaces (MI).
So do you: the new term "location specification" changes how we name
the argument passed to "break" and other commands. I don't consider
such changes in terminology to be such a big deal, but if you do, why
aren't you concerned about a similar change in your proposal?
> > But now I'm beginning to wonder what is your definition of "location"
> > in this context? What does it entail if two different "locations" can
> > be resolved to the same code address? And where is that meaning of
> > "location" described? The text you proposed uses "location(s) that
> > match(es) the location spec", which doesn't seem to imply any
> > attributes except the "place" where the breakpoint will be set, since
> > "location spec" is just source-level description of one or more such
> > "places". How does "location" imply anything about local variables,
> > commands, etc.?
>
> A location is some actual place in the program, identifiable by
> some "coordinates". Some source line in the program can be a location
> in the program, if you have its coordinates. Line number alone isn't sufficient.
> A source location is fully defined by the path to its file and the line number, the
> full prototype of its function, and the address of the first instruction that the line was
> compiled down to. Addresses are per-inferior, so add inferior to the coordinate system. If
> you have the same code loaded in multiple inferiors, GDB considers the same code at each
> address its own location. E.g. here's a breakpoint set at handle_inferior_event, with two inferior gdbs loaded:
>
> 3 breakpoint keep y <MULTIPLE>
> 3.1 y 0x00000000004ea2ac in handle_inferior_event(execution_control_state*) at /home/pedro/gdb/binutils-gdb/src/gdb/infrun.c:5331 inf 1
> 3.2 y 0x00000000004ea2ac in handle_inferior_event(execution_control_state*) at /home/pedro/gdb/binutils-gdb/src/gdb/infrun.c:5331 inf 2
>
> Note "inf 1" and "inf 2" on the right hand side. So you have two locations for the same code, in two inferiors.
I don't think we have this described on this level of detail anywhere,
do we? I think if we agree to use "source location" for this, we
should have all of the above in the description of the term.
> You may not have debug info for all locations in the program. So a location
> may be missing some coordinates, like the file/line. It won't be a source location,
> but it is still some actual program location. Or you may lack the full path
> to the filename. Etc. But the tuple of the coordinates identify some actual spot in
> the actual program. Address alone isn't sufficient to identify source location,
> given inlining. Of course, if you don't have debug info, address is all you got.
So we can also have "incomplete" source locations, right? And the
source location has some minimum set of attributes that _must_ be
resolved in order for the source location to be valid, right? We
should then document that as well, including the possibility of its
being incomplete.
> OTOH, a location specification is a way to find or refer to these actual locations in the program.
My impression is that a location specification is a higher-level way
of defining a set of one or more actual source locations. GDB then
resolves the spec to the actual source locations by using the source
and debug information. Right?
> A location spec may leave out some parts of the "coordinates".
I think "coordinates", i.e. attributes of the source location, are not
really relevant to location specifications. Location specifications
are conceptually "descriptors" of the source locations, and the fact
that they happen to share some attributes is completely unimportant
in the context of this discussion.
> I would like to point out the fact that we've been referring to "location spec"
> vs "actual location" in these emails (not the patches), and no one ends up confused.
> Please consider that.
We are not confused because we've exchanged any number of messages to
figure out what is meant by these terms. This and the sibling thread
amassed 2 dozens of messages, and only now you described in full what
you mean by "location", so now I understand what you mean by that.
Readers of the GDB manual need to be able to understand this stuff the
first time they read the text, and we need to use wording and
terminology that don't run the risk of being misinterpreted by someone
who, unlike you and me, isn't so familiar with GDB and its internal
workings.
> > I'm not wedded to "address", btw, I just don't want us to use
> > "location" for that. If there's some alternative term, we could
> > consider it. Although "address" is already being used for a very
> > similar, if not the same, thing, and thus is a natural candidate,
> > because people won't need to learn a new term.
>
> Can we please just focus on documenting how GDB works? That's what
> the manual should be doing.
AFAIU, that's what we were doing all the time. We are trying to
document that in a way that is as clear and as understandable as
possible.
> GDB calls these things breakpoint locations
Where does it call them that? in the GDB source code? That's not very
relevant for the manual: we don't use variable names in the manual,
and readers of the manual don't necessarily read the GDB source code.
So we need to find good terminology for the manual.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v2 1/2] Always show locations for breakpoints & show canonical location spec
2022-05-26 15:03 ` Eli Zaretskii
@ 2022-05-26 15:10 ` Pedro Alves
2022-05-26 15:33 ` Eli Zaretskii
2022-05-26 19:29 ` Pedro Alves
1 sibling, 1 reply; 39+ messages in thread
From: Pedro Alves @ 2022-05-26 15:10 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
On 2022-05-26 16:03, Eli Zaretskii wrote:
> Where does it call them that? in the GDB source code? That's not very
> relevant for the manual: we don't use variable names in the manual,
> and readers of the manual don't necessarily read the GDB source code.
> So we need to find good terminology for the manual.
No, in error messages, in online help, and, importantly, the manual has
been very clearly calling them breakpoint locations, so every user that refers
to the "1.1", "1.2", etc., things calls them the breakpoint locations.
(top-gdb) disable 2.3
Bad breakpoint location number '3'
Anyhow, I can't reply to your message in detail right now, but I would like
to say that I will try to document locations vs locations specs better
and then send a v4, when I have a chance.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v2 1/2] Always show locations for breakpoints & show canonical location spec
2022-05-26 15:10 ` Pedro Alves
@ 2022-05-26 15:33 ` Eli Zaretskii
0 siblings, 0 replies; 39+ messages in thread
From: Eli Zaretskii @ 2022-05-26 15:33 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> Date: Thu, 26 May 2022 16:10:58 +0100
> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <pedro@palves.net>
>
> (top-gdb) disable 2.3
> Bad breakpoint location number '3'
If we agree to use "source location" in the manual, the above will not
need to change.
> Anyhow, I can't reply to your message in detail right now, but I would like
> to say that I will try to document locations vs locations specs better
> and then send a v4, when I have a chance.
If you agree to "source location", please use that in v4, and then I
hope we will be close to the final version, if not at it.
Thanks.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v2 1/2] Always show locations for breakpoints & show canonical location spec
2022-05-26 15:03 ` Eli Zaretskii
2022-05-26 15:10 ` Pedro Alves
@ 2022-05-26 19:29 ` Pedro Alves
2022-05-26 19:55 ` Eli Zaretskii
2023-04-10 15:07 ` Andrew Burgess
1 sibling, 2 replies; 39+ messages in thread
From: Pedro Alves @ 2022-05-26 19:29 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
On 2022-05-26 16:03, Eli Zaretskii wrote:
>> Date: Thu, 26 May 2022 15:04:21 +0100
>> Cc: gdb-patches@sourceware.org
>> From: Pedro Alves <pedro@palves.net>
>>
>>> Yes, and that's the point I tried to make: you can enable or disable a
>>> breakpoint at some location, not enable/disable the location itself,
>>> which is what the text was saying.
>>
>> "enable or disable a breakpoint at some location" is slightly ambiguous because
>> it doesn't distinguish between the whole breakpoint vs one individual location
>> the breakpoint is set at.
>
> How so? it says "disable a breakpoint AT SOME LOCATION". IOW, the
> breakpoint is disabled/enabled only at some location.
>
>>>> I really think that this is the wrong direction, and working on
>>>> the patch that I pointed at above really convinced me so even more.
>>>> I believe that that patch will convince you as well. Please take a look
>>>> there.
>>>
>>> I did, and it didn't. It actually convinced me even more that we
>>> cannot use "location" in both contexts without creating confusion.
>>
>> What a mystery. The current text uses location throughout and it
>> isn't a problem.
>
> I explained why in another message: we were using "location" in just
> one sense, now we are supposed to be using it in two senses: one for
> "specifying" a location, the other for the location itself. In many
> situations in life the thing and its specification are one and the
> same, or tightly coupled. E.g., we frequently say in the
> documentation of a program stuff like
>
> This function deletes the specified FILE...
>
> which doesn't distinguish between FILE and its specification.
>
A better example would be:
This function deletes all files that match GLOB...
and then what is a GLOB is described somewhere, and a glob is very
obviously different from the actual files the glob matches.
> We are now supposed to distinguish between a "location" and its
> "specification". This is not easy to do, and I think it risks
> creating a confusion. This is based both on experience with other
> things that are defined through some sort of "specification", and on
> actual reading the text of your patches.
>
I don't think it is, difficult. A spec looks like one of these strings:
func
source:line
*expression
-source src -line l
-function func -label lab
etc. You can't confuse this with an actual concrete location. A spec
resolves to locations. But it isn't itself a location. The documentation
for each command doesn't say that currently, just uses @var{location}, but
if you follow the xfer to the "Specify locations" node, it is clear there.
But we'll make it clearer, regardless.
>
>>> But now I'm beginning to wonder what is your definition of "location"
>>> in this context? What does it entail if two different "locations" can
>>> be resolved to the same code address? And where is that meaning of
>>> "location" described? The text you proposed uses "location(s) that
>>> match(es) the location spec", which doesn't seem to imply any
>>> attributes except the "place" where the breakpoint will be set, since
>>> "location spec" is just source-level description of one or more such
>>> "places". How does "location" imply anything about local variables,
>>> commands, etc.?
>>
>> A location is some actual place in the program, identifiable by
>> some "coordinates". Some source line in the program can be a location
>> in the program, if you have its coordinates. Line number alone isn't sufficient.
>> A source location is fully defined by the path to its file and the line number, the
>> full prototype of its function, and the address of the first instruction that the line was
>> compiled down to. Addresses are per-inferior, so add inferior to the coordinate system. If
>> you have the same code loaded in multiple inferiors, GDB considers the same code at each
>> address its own location. E.g. here's a breakpoint set at handle_inferior_event, with two inferior gdbs loaded:
>>
>> 3 breakpoint keep y <MULTIPLE>
>> 3.1 y 0x00000000004ea2ac in handle_inferior_event(execution_control_state*) at /home/pedro/gdb/binutils-gdb/src/gdb/infrun.c:5331 inf 1
>> 3.2 y 0x00000000004ea2ac in handle_inferior_event(execution_control_state*) at /home/pedro/gdb/binutils-gdb/src/gdb/infrun.c:5331 inf 2
>>
>> Note "inf 1" and "inf 2" on the right hand side. So you have two locations for the same code, in two inferiors.
>
> I don't think we have this described on this level of detail anywhere,
> do we? I think if we agree to use "source location" for this, we
> should have all of the above in the description of the term.
>
>> You may not have debug info for all locations in the program. So a location
>> may be missing some coordinates, like the file/line. It won't be a source location,
>> but it is still some actual program location. Or you may lack the full path
>> to the filename. Etc. But the tuple of the coordinates identify some actual spot in
>> the actual program. Address alone isn't sufficient to identify source location,
>> given inlining. Of course, if you don't have debug info, address is all you got.
>
> So we can also have "incomplete" source locations, right? And the
> source location has some minimum set of attributes that _must_ be
> resolved in order for the source location to be valid, right? We
> should then document that as well, including the possibility of its
> being incomplete.
>
I documented all this in the Location Specifications section. I'll send v4 in a bit.
I did not go for "source locations", however. I swear I am not trying to be
difficult... The issue is that as I tried to describe things, "source location" read
awkwardly, and kind of a lie-ish. Because, you can have usable resolved locations without
sources, even if they are incomplete. It depends on command if they are usable. Instead, I
noticed something.
Here:
* List:: Printing source lines
-* Specify Location:: How to specify code locations
+* Location Specifications:: How to specify code locations
^^^^^^^^^^^^^^
And note what the intro paragraph of that node currently says:
"Several GDB commands accept arguments that specify a location or locations of your program’s code."
Clearly the arguments specify something, and that something is ... "locations of your program’s code."
So I went with "code locations" instead. I actually first started by using "program location"
throughout, or "location in your program", but then as I polished, and polished, and
polished, I ended up using "code location" everywhere. I think it reads really nicely.
Code location is nice for being a superset of "resolved complete source location" and "resolved location
that is just some address and maybe function name" (because we don't have debug info). It fits both.
I also went through all the command's documentation again, looked at the code for each,
checking what exactly GDB looks for in the resolved locations, and switched the manual
to refer to resolved lines or resolved addresses etc. Very similar to your earlier comments,
except that by switching to use "resolved to" instead of "matches", it now looks better
to me too this way. For example:
+@itemx info line @var{locspec}
Print the starting and ending addresses of the compiled code for
-source line @var{location}. You can specify source lines in any of
-the ways documented in @ref{Specify Location}. With no @var{location}
-information about the current source line is printed.
+source lines that @var{locspec} resolves to. @xref{Location
+Specifications}, for the various forms of @var{locspec}.
+With no @var{locspec}, information about the current source line is
+printed.
@kindex info macros
-@item info macros @var{location}
-Show all macro definitions that are in effect at the location specified
-by @var{location}, and describe the source location or compiler
-command-line where those definitions were established.
+@item info macros @var{locspec}
+Show all macro definitions that are in effect at the source line
+@var{locspec} resolves to, and describe the source location
+or compiler command-line where those definitions were established.
etc.
For the case where you wanted to add @var{addr}, I removed
the parenthesis from the sentence, like:
-We can also inquire (using @code{*@var{addr}} as the form for
-@var{location}) what source line covers a particular address:
+We can also inquire, using @code{*@var{addr}} as the form for
+@var{locspec}, what source line covers a particular address
+@var{addr}:
Oh, and for the "source lines" ambiguity in the "list command", I tweaked it
by saying something else other than "ambiguous locations". Like so:
+ If either
+@var{first} or @var{last} resolve to more than one source line in the
+program, then the list command will show the list of resolved source
+lines and does not proceed with the source code listing.
The trick is to say "source code listing" instead of "print source lines".
I did a lot of small changes here and there throughout...
Anyhow, you'll see in v4 in a bit.
I hope you will be happy with this one.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v2 1/2] Always show locations for breakpoints & show canonical location spec
2022-05-26 19:29 ` Pedro Alves
@ 2022-05-26 19:55 ` Eli Zaretskii
2022-05-26 20:40 ` Pedro Alves
2023-04-10 15:07 ` Andrew Burgess
1 sibling, 1 reply; 39+ messages in thread
From: Eli Zaretskii @ 2022-05-26 19:55 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> Date: Thu, 26 May 2022 20:29:50 +0100
> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <pedro@palves.net>
>
> I documented all this in the Location Specifications section. I'll send v4 in a bit.
>
> I did not go for "source locations", however. I swear I am not trying to be
> difficult... The issue is that as I tried to describe things, "source location" read
> awkwardly, and kind of a lie-ish. Because, you can have usable resolved locations without
> sources, even if they are incomplete. It depends on command if they are usable. Instead, I
> noticed something.
>
> Here:
>
> * List:: Printing source lines
> -* Specify Location:: How to specify code locations
> +* Location Specifications:: How to specify code locations
> ^^^^^^^^^^^^^^
>
> And note what the intro paragraph of that node currently says:
>
> "Several GDB commands accept arguments that specify a location or locations of your program’s code."
>
> Clearly the arguments specify something, and that something is ... "locations of your program’s code."
>
> So I went with "code locations" instead.
I could agree with this, but note that you are contradicting yourself:
"code" can and is sometimes interpreted as "machine code", and thus
"code location" can be interpreted as "address", something you didn't
want. By contrast, "source location" is unequivocally a source-level
concept, and reflects the fact that it refers to a certain line of
source code in a certain file. Moreover, it follows the example of
that C++ page you yourself used as an argument. Why now you deviate
from all that is a mystery for me.
But if you don't care about all these inconsistencies, "code location"
is fine with me, as it qualifies the overly-general "location" enough
to solve the potential ambiguity, which was what bothered me.
> Anyhow, you'll see in v4 in a bit.
>
> I hope you will be happy with this one.
Thanks.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v2 1/2] Always show locations for breakpoints & show canonical location spec
2022-05-26 19:55 ` Eli Zaretskii
@ 2022-05-26 20:40 ` Pedro Alves
0 siblings, 0 replies; 39+ messages in thread
From: Pedro Alves @ 2022-05-26 20:40 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
On 2022-05-26 20:55, Eli Zaretskii wrote:
>> Date: Thu, 26 May 2022 20:29:50 +0100
>> Cc: gdb-patches@sourceware.org
>> From: Pedro Alves <pedro@palves.net>
>>
>> I documented all this in the Location Specifications section. I'll send v4 in a bit.
>>
>> I did not go for "source locations", however. I swear I am not trying to be
>> difficult... The issue is that as I tried to describe things, "source location" read
>> awkwardly, and kind of a lie-ish. Because, you can have usable resolved locations without
>> sources, even if they are incomplete. It depends on command if they are usable. Instead, I
>> noticed something.
>>
>> Here:
>>
>> * List:: Printing source lines
>> -* Specify Location:: How to specify code locations
>> +* Location Specifications:: How to specify code locations
>> ^^^^^^^^^^^^^^
>>
>> And note what the intro paragraph of that node currently says:
>>
>> "Several GDB commands accept arguments that specify a location or locations of your program’s code."
>>
>> Clearly the arguments specify something, and that something is ... "locations of your program’s code."
>>
>> So I went with "code locations" instead.
>
> I could agree with this, but note that you are contradicting yourself:
> "code" can and is sometimes interpreted as "machine code", and thus
> "code location" can be interpreted as "address", something you didn't
> want.
I didn't want "address" specifically, because it is either incorrect or
misleading. E.g., "info macro" doesn't work with addresses, but you had
suggested to say "address" for this one at some point. I was also worried
with "addresses that match locspec", as it is sort of ambiguous with address
locations. Breakpoints are set with the complete location coordinate, just
the address is not enough to identify the location properly, given inlining.
Explaining what "code location" means in the location specification chapter
and talking about locspecs resolved to addresses, works, but just plain
address doesn't.
> By contrast, "source location" is unequivocally a source-level
> concept, and reflects the fact that it refers to a certain line of
> source code in a certain file. Moreover, it follows the example of
> that C++ page you yourself used as an argument. Why now you deviate
> from all that is a mystery for me.
I pointed at the C++ page as an argument for the word "location", which
you so strongly wanted to replace by something else, like "place" or "address"
or some other unknown term. I had never understood that you would be fine with
a qualifier on top of location. I wish I had known earlier.
It's great we managed to meet in the middle somewhere.
>
> But if you don't care about all these inconsistencies, "code location"
> is fine with me, as it qualifies the overly-general "location" enough
> to solve the potential ambiguity, which was what bothered me.
Great, let's go with it.
>
>> Anyhow, you'll see in v4 in a bit.
>>
>> I hope you will be happy with this one.
>
> Thanks.
Looking forward for a review.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v2 1/2] Always show locations for breakpoints & show canonical location spec
2022-05-26 19:29 ` Pedro Alves
2022-05-26 19:55 ` Eli Zaretskii
@ 2023-04-10 15:07 ` Andrew Burgess
1 sibling, 0 replies; 39+ messages in thread
From: Andrew Burgess @ 2023-04-10 15:07 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Eli Zaretskii
Pedro Alves <pedro@palves.net> writes:
> On 2022-05-26 16:03, Eli Zaretskii wrote:
>>> Date: Thu, 26 May 2022 15:04:21 +0100
>>> Cc: gdb-patches@sourceware.org
>>> From: Pedro Alves <pedro@palves.net>
>>>
>>>> Yes, and that's the point I tried to make: you can enable or disable a
>>>> breakpoint at some location, not enable/disable the location itself,
>>>> which is what the text was saying.
>>>
>>> "enable or disable a breakpoint at some location" is slightly ambiguous because
>>> it doesn't distinguish between the whole breakpoint vs one individual location
>>> the breakpoint is set at.
>>
>> How so? it says "disable a breakpoint AT SOME LOCATION". IOW, the
>> breakpoint is disabled/enabled only at some location.
>>
>>>>> I really think that this is the wrong direction, and working on
>>>>> the patch that I pointed at above really convinced me so even more.
>>>>> I believe that that patch will convince you as well. Please take a look
>>>>> there.
>>>>
>>>> I did, and it didn't. It actually convinced me even more that we
>>>> cannot use "location" in both contexts without creating confusion.
>>>
>>> What a mystery. The current text uses location throughout and it
>>> isn't a problem.
>>
>> I explained why in another message: we were using "location" in just
>> one sense, now we are supposed to be using it in two senses: one for
>> "specifying" a location, the other for the location itself. In many
>> situations in life the thing and its specification are one and the
>> same, or tightly coupled. E.g., we frequently say in the
>> documentation of a program stuff like
>>
>> This function deletes the specified FILE...
>>
>> which doesn't distinguish between FILE and its specification.
>>
>
> A better example would be:
>
> This function deletes all files that match GLOB...
>
> and then what is a GLOB is described somewhere, and a glob is very
> obviously different from the actual files the glob matches.
>
>
>> We are now supposed to distinguish between a "location" and its
>> "specification". This is not easy to do, and I think it risks
>> creating a confusion. This is based both on experience with other
>> things that are defined through some sort of "specification", and on
>> actual reading the text of your patches.
>>
>
> I don't think it is, difficult. A spec looks like one of these strings:
>
> func
> source:line
> *expression
> -source src -line l
> -function func -label lab
>
> etc. You can't confuse this with an actual concrete location. A spec
> resolves to locations. But it isn't itself a location. The documentation
> for each command doesn't say that currently, just uses @var{location}, but
> if you follow the xfer to the "Specify locations" node, it is clear there.
>
> But we'll make it clearer, regardless.
>
>>
>>>> But now I'm beginning to wonder what is your definition of "location"
>>>> in this context? What does it entail if two different "locations" can
>>>> be resolved to the same code address? And where is that meaning of
>>>> "location" described? The text you proposed uses "location(s) that
>>>> match(es) the location spec", which doesn't seem to imply any
>>>> attributes except the "place" where the breakpoint will be set, since
>>>> "location spec" is just source-level description of one or more such
>>>> "places". How does "location" imply anything about local variables,
>>>> commands, etc.?
>>>
>>> A location is some actual place in the program, identifiable by
>>> some "coordinates". Some source line in the program can be a location
>>> in the program, if you have its coordinates. Line number alone isn't sufficient.
>>> A source location is fully defined by the path to its file and the line number, the
>>> full prototype of its function, and the address of the first instruction that the line was
>>> compiled down to. Addresses are per-inferior, so add inferior to the coordinate system. If
>>> you have the same code loaded in multiple inferiors, GDB considers the same code at each
>>> address its own location. E.g. here's a breakpoint set at handle_inferior_event, with two inferior gdbs loaded:
>>>
>>> 3 breakpoint keep y <MULTIPLE>
>>> 3.1 y 0x00000000004ea2ac in handle_inferior_event(execution_control_state*) at /home/pedro/gdb/binutils-gdb/src/gdb/infrun.c:5331 inf 1
>>> 3.2 y 0x00000000004ea2ac in handle_inferior_event(execution_control_state*) at /home/pedro/gdb/binutils-gdb/src/gdb/infrun.c:5331 inf 2
>>>
>>> Note "inf 1" and "inf 2" on the right hand side. So you have two locations for the same code, in two inferiors.
>>
>> I don't think we have this described on this level of detail anywhere,
>> do we? I think if we agree to use "source location" for this, we
>> should have all of the above in the description of the term.
>>
>>> You may not have debug info for all locations in the program. So a location
>>> may be missing some coordinates, like the file/line. It won't be a source location,
>>> but it is still some actual program location. Or you may lack the full path
>>> to the filename. Etc. But the tuple of the coordinates identify some actual spot in
>>> the actual program. Address alone isn't sufficient to identify source location,
>>> given inlining. Of course, if you don't have debug info, address is all you got.
>>
>> So we can also have "incomplete" source locations, right? And the
>> source location has some minimum set of attributes that _must_ be
>> resolved in order for the source location to be valid, right? We
>> should then document that as well, including the possibility of its
>> being incomplete.
>>
>
> I documented all this in the Location Specifications section. I'll send v4 in a bit.
>
> I did not go for "source locations", however. I swear I am not trying to be
> difficult... The issue is that as I tried to describe things, "source location" read
> awkwardly, and kind of a lie-ish. Because, you can have usable resolved locations without
> sources, even if they are incomplete. It depends on command if they are usable. Instead, I
> noticed something.
>
> Here:
>
> * List:: Printing source lines
> -* Specify Location:: How to specify code locations
> +* Location Specifications:: How to specify code locations
> ^^^^^^^^^^^^^^
>
> And note what the intro paragraph of that node currently says:
>
> "Several GDB commands accept arguments that specify a location or locations of your program’s code."
>
> Clearly the arguments specify something, and that something is ... "locations of your program’s code."
>
> So I went with "code locations" instead. I actually first started by using "program location"
> throughout, or "location in your program", but then as I polished, and polished, and
> polished, I ended up using "code location" everywhere. I think it reads really nicely.
> Code location is nice for being a superset of "resolved complete source location" and "resolved location
> that is just some address and maybe function name" (because we don't have debug info). It fits both.
>
> I also went through all the command's documentation again, looked at the code for each,
> checking what exactly GDB looks for in the resolved locations, and switched the manual
> to refer to resolved lines or resolved addresses etc. Very similar to your earlier comments,
> except that by switching to use "resolved to" instead of "matches", it now looks better
> to me too this way. For example:
>
> +@itemx info line @var{locspec}
> Print the starting and ending addresses of the compiled code for
> -source line @var{location}. You can specify source lines in any of
> -the ways documented in @ref{Specify Location}. With no @var{location}
> -information about the current source line is printed.
> +source lines that @var{locspec} resolves to. @xref{Location
> +Specifications}, for the various forms of @var{locspec}.
> +With no @var{locspec}, information about the current source line is
> +printed.
>
> @kindex info macros
> -@item info macros @var{location}
> -Show all macro definitions that are in effect at the location specified
> -by @var{location}, and describe the source location or compiler
> -command-line where those definitions were established.
> +@item info macros @var{locspec}
> +Show all macro definitions that are in effect at the source line
> +@var{locspec} resolves to, and describe the source location
> +or compiler command-line where those definitions were established.
>
> etc.
>
> For the case where you wanted to add @var{addr}, I removed
> the parenthesis from the sentence, like:
>
> -We can also inquire (using @code{*@var{addr}} as the form for
> -@var{location}) what source line covers a particular address:
> +We can also inquire, using @code{*@var{addr}} as the form for
> +@var{locspec}, what source line covers a particular address
> +@var{addr}:
>
> Oh, and for the "source lines" ambiguity in the "list command", I tweaked it
> by saying something else other than "ambiguous locations". Like so:
>
> + If either
> +@var{first} or @var{last} resolve to more than one source line in the
> +program, then the list command will show the list of resolved source
> +lines and does not proceed with the source code listing.
>
> The trick is to say "source code listing" instead of "print source lines".
>
>
> I did a lot of small changes here and there throughout...
>
> Anyhow, you'll see in v4 in a bit.
Hi Pedro,
I was doing some work in the 'info breakpoints' area, and remembered
this patch. Was this something you're still working on?
Thanks,
Andrew
^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2023-04-10 15:07 UTC | newest]
Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-19 21:55 [PATCH 0/2] info breakpoints improvements Pedro Alves
2022-05-19 21:55 ` [PATCH 1/2] Always show locations for breakpoints & show canonical location spec Pedro Alves
2022-05-20 6:45 ` Eli Zaretskii
2022-05-23 17:04 ` [PATCH v2 " Pedro Alves
2022-05-24 13:06 ` Eli Zaretskii
2022-05-25 19:32 ` Pedro Alves
2022-05-26 12:48 ` Eli Zaretskii
2022-05-26 14:04 ` Pedro Alves
2022-05-26 15:03 ` Eli Zaretskii
2022-05-26 15:10 ` Pedro Alves
2022-05-26 15:33 ` Eli Zaretskii
2022-05-26 19:29 ` Pedro Alves
2022-05-26 19:55 ` Eli Zaretskii
2022-05-26 20:40 ` Pedro Alves
2023-04-10 15:07 ` Andrew Burgess
2022-05-20 7:45 ` [PATCH " Metzger, Markus T
2022-05-23 17:05 ` Lancelot SIX
2022-05-19 21:55 ` [PATCH 2/2] Introduce "info breakpoints -hide-locations" Pedro Alves
2022-05-20 6:48 ` Eli Zaretskii
2022-05-20 5:57 ` [PATCH 0/2] info breakpoints improvements Eli Zaretskii
2022-05-23 17:06 ` Pedro Alves
2022-05-24 13:14 ` Eli Zaretskii
2022-05-24 13:45 ` Pedro Alves
2022-05-24 8:38 ` Luis Machado
2022-05-24 10:02 ` Pedro Alves
2022-05-24 13:20 ` Eli Zaretskii
2022-05-24 13:29 ` Pedro Alves
2022-05-24 13:43 ` Eli Zaretskii
2022-05-24 13:50 ` Pedro Alves
2022-05-24 14:03 ` Eli Zaretskii
2022-05-24 14:09 ` Pedro Alves
2022-05-24 14:25 ` Eli Zaretskii
2022-05-24 14:33 ` Pedro Alves
2022-05-24 14:11 ` Andreas Schwab
2022-05-24 14:17 ` Pedro Alves
2022-05-24 19:49 ` [PATCH] Show enabled locations with disabled breakpoint parent as "y-" (Re: [PATCH 0/2] info breakpoints improvements) Pedro Alves
2022-05-25 13:57 ` Eli Zaretskii
2022-05-25 19:19 ` Pedro Alves
2022-05-24 14:26 ` [PATCH 0/2] info breakpoints improvements Eli Zaretskii
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).