Re: [PATCH v2 1/2] Always show locations for breakpoints & show canonical location spec

[Eli Zaretskii <eliz@gnu.org>]

> 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.