public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Pedro Alves <pedro@palves.net>
To: Eli Zaretskii <eliz@gnu.org>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH v2 1/2] Always show locations for breakpoints & show canonical location spec
Date: Thu, 26 May 2022 15:04:21 +0100	[thread overview]
Message-ID: <cbacae25-68e6-6f20-008b-ec505804562c@palves.net> (raw)
In-Reply-To: <8335gwpiih.fsf@gnu.org>

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!

  reply	other threads:[~2022-05-26 14:04 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cbacae25-68e6-6f20-008b-ec505804562c@palves.net \
    --to=pedro@palves.net \
    --cc=eliz@gnu.org \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).