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

[Andrew Burgess <aburgess@redhat.com>]

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