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

[Pedro Alves <pedro@palves.net>]

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.