public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Pedro Alves <pedro@palves.net>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH v4] gdb/manual: Introduce location specs
Date: Sat, 28 May 2022 10:39:48 +0300	[thread overview]
Message-ID: <838rqmm7gb.fsf@gnu.org> (raw)
In-Reply-To: <20220526194250.2310460-1-pedro@palves.net> (message from Pedro Alves on Thu, 26 May 2022 20:42:50 +0100)

> From: Pedro Alves <pedro@palves.net>
> Date: Thu, 26 May 2022 20:42:50 +0100
> 
>  gdb/doc/gdb.texinfo | 419 +++++++++++++++++++++++++-------------------
>  gdb/doc/guile.texi  |   2 +-
>  gdb/doc/python.texi |   5 +-
>  3 files changed, 246 insertions(+), 180 deletions(-)

Thanks, my comments are below.

> +@item break @var{locspec}
> +Set a breakpoint at all the code locations in your program the given
> +@var{locspec} resolves to.

I find the style that puts "resolves to" last awkward and hard to
read.  I suggest a slight rewording:

  Set a breakpoint at all the code locations in your program that are
  the result of resolving the given @var{locspec}.

> +a line number, an address of an instruction, and more.  @xref{Location
> +Specifications}, for the various forms of @var{locspec}.  The
> +breakpoint will stop your program just before it executes the
> +instruction at the address of any of the breakpoint's locations.
                                            ^^^^^^^^^^^^^^^^^^^^^^
"breakpoint's code locations"

>  A breakpoint with multiple locations is displayed in the breakpoint
                              ^^^^^^^^^
"code locations"

>  table using several rows---one header row, followed by one row for
>  each breakpoint location.  The header row has @samp{<MULTIPLE>} in the
        ^^^^^^^^^^^^^^^^^^^
"code location of the breakpoint"

> +address column.  Each breakpoint location row contains the actual
                         ^^^^^^^^^^^^^^^^^^^
"code location"

> +address, source file, source line and function of its code location.
> +The number column for a breakpoint location is of the form
                           ^^^^^^^^^^^^^^^^^^^
"code 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 resolve the
> +location spec, it should create a pending breakpoint without
> +confirmation.
>  
>  @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 the location
> +spec, it should create a pending breakpoint without confirmation.

These two option's values seem to have an identical description.  Is
that intentional?  I think the first of the two is a copy/paste error.

Btw, is the "cannot resolve the location spec" situation and its
reasons described somewhere?  The text in this section seems to
indicate this is described in "Location Specifications", but I don't
think I see it there.

>  The settings above only affect the @code{break} command and its
> -variants.  Once breakpoint is set, it will be automatically updated
> +variants.  Once a breakpoint is set, it will be automatically updated
>  as shared libraries are loaded and unloaded.

Does this mean the resolution process of an unresolved location spec
is retried each time a shared library is loaded?  If so, I think we
should say it here explicitly.

> -@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:
> +@item clear @var{locspec}
> +Delete breakpoints with code locations that match @var{locspec}.
                                          ^^^^^^^^^^^^^^^^^^^^^^^^
"that are the result of resolving @var{locspec}"

> +@item dprintf @var{locspec},@var{template},@var{expression}[,@var{expression}@dots{}]
> +Whenever execution reaches a code location in your program that
> +matches @var{locspec}, print the values of one or more

Likewise here (and elsewhere): I don't think we should talk about
"matching a location spec", but about "resolution of a location spec".
"Matching" implies something that we don't really do here.

>  Print lines from @var{first} to @var{last}.  Both arguments are
> -locations.  When a @code{list} command has two locations, and the
> -source file of the second location is omitted, this refers to
> -the same source file as the first location.
> +location specs.  When a @code{list} command has two location specs,
> +and the source file of the second location spec is omitted, this
> +refers to the same source file as the first location spec.  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.
             ^^^^^^^^^^^^^^^^
"will not proceed", to be consistent with "will show".

> +Likewise, if @var{last} resolves to more than one source line in the
> +program, then the list command will print the list of resolved source
> +lines and does not proceed with the source code listing.

Same here.

> +
>  @item list @var{first},
>  Print lines starting with @var{first}.
>  
> @@ -9041,17 +9038,75 @@ 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 location spec
> +@cindex locspec
>  @cindex source location
> +@cindex code 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.
> +or locations of your program's code.  Since @value{GDBN} is a
> +source-level debugger, a location specification usually indicates some
> +line in the source code, but it can also indicate a function name, an
> +address, a label, and more.
> +
> +A concrete code location in your program is uniquely identifiable by a
> +set of logical attributes.  Typically, a line number, the source file
> +the line belongs to, the fully-qualified and prototyped function it is
> +defined in, and an instruction address.  Because each inferior has its
> +own address space, also an inferior number.
> +
> +On the other hand, a @dfn{location specification} (a.k.a.@: location
> +spec)

"location spec" should be in @dfn here the first time it is used.

> +Here are examples of typical situations that result in a location spec
> +matching multiple concrete code locations in your program:

Should we also enumerate some situations where a location spec cannot
be completely resolved?  The text talks about that possibility, but
only in passing, with no details or practical examples.

> +Addresses cannot be specified with a location spec (@pxref{Location
> +Specifications}).

I think they can, but only if using one particular form of a location
spec, the *ADDR one.  Or am I missing something?

> +register other than the program counter.  If @var{locspec} resolves to
> +an address in a different function from the one currently executing, the
> +results may be bizarre if the two functions expect different patterns
> +of arguments or of local variables.  For this reason, the @code{jump}
> +command requests confirmation if the specified line is not in the
> +function currently executing.                  ^^^^

"line" or "code location"?

> +@item break-range @var{start-locspec}, @var{end-locspec}
> +Set a breakpoint for an address range given by @var{start-locspec} and
> +@var{end-locspec}, which are location specs.  @xref{Location
> +Specifications}, for a list of all the possible forms of location
> +specs.  If either @var{start-locspec} or @var{end-locspec} resolve to
> +multiple addresses in the program, then the command aborts with an
> +error without creating a breakpoint.  The breakpoint will stop
> +execution of the inferior whenever it executes an instruction at any
> +address within the specified range, including @var{start-locspec} and
> +@var{end-locspec}.

This deviates from the usual practice elsewhere, and talks about
location specs resolving into _addresses_ and not code locations.  is
that intentional and necessary?

> -If @var{location} cannot be parsed (for example if it
> +If @var{locspec} cannot be parsed (for example if it
>  refers to unknown files or functions), create a pending
>  breakpoint. Without this flag, @value{GDBN} will report
             ^^
Two spaces between sentences.  (Yes, a mistake in the original text.)

>  @smallexample
> - -exec-jump @var{location}
> + -exec-jump @var{locspec}
>  @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
   ^^^^^^^^^^^^^
I think this should be "@var{locspec}".

> +Executes the inferior until the address @var{locspec} resolves to.  If

This is highly ambiguous: "until" here could be interpreted as
referring to "resolves" instead of to "address".  Suggest to rephrase:

  Executes the inferior until it reaches the address to which
  @var{locspec} resolves.

> +there is no argument, the inferior executes until a source line
> +greater than the current one is reached.

Suggest to get rid of the passive tense with

  "...until it reaches a source line greater than the current line."


  parent reply	other threads:[~2022-05-28  7:39 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-26 19:42 Pedro Alves
2022-05-27 14:16 ` Eli Zaretskii
2022-05-27 15:04   ` Pedro Alves
2022-05-27 15:52     ` Eli Zaretskii
2022-05-27 17:11       ` Pedro Alves
2022-05-27 17:31         ` Eli Zaretskii
2022-05-27 17:51           ` Pedro Alves
2022-05-27 18:23             ` Eli Zaretskii
2022-05-27 18:42               ` Pedro Alves
2022-05-27 18:50                 ` Pedro Alves
2022-05-27 19:00                   ` Eli Zaretskii
2022-05-27 19:30                     ` Pedro Alves
2022-05-28  7:43                       ` Eli Zaretskii
2022-05-30 14:38                         ` Pedro Alves
2022-05-27 18:55                 ` Eli Zaretskii
2022-05-27 19:05                   ` Pedro Alves
2022-05-27 19:14                     ` Eli Zaretskii
2022-05-27 19:17                       ` Pedro Alves
2022-05-27 19:34                         ` Eli Zaretskii
2022-05-27 19:38                           ` Pedro Alves
2022-05-28  7:39 ` Eli Zaretskii [this message]
2022-05-30 14:44   ` [pushed v5] " Pedro Alves
2022-05-30 16:15     ` Eli Zaretskii
2022-05-31 11:05       ` [PATCH] Improve clear command's documentation Pedro Alves
2022-05-31 12:36         ` Eli Zaretskii
2022-05-31 13:04           ` Pedro Alves
2022-05-31 13:42             ` Eli Zaretskii
2022-05-31 14:47               ` Pedro Alves
2022-05-31 16:06                 ` Eli Zaretskii
2022-05-31 11:13       ` [PATCH] Explicitly mention yet-unloaded shared libraries in location spec examples Pedro Alves
2022-05-31 11:47         ` Eli Zaretskii
2022-05-31 11:17       ` [pushed v5] gdb/manual: Introduce location specs Pedro Alves
2022-05-31 11:31       ` [PATCH] Improve break-range's documentation Pedro Alves
2022-05-31 11:55         ` Eli Zaretskii
2022-05-31 12:03           ` Pedro Alves
2022-05-31 12:09             ` Eli Zaretskii
2022-06-01 17:17     ` RTe: Location Specs (Was: [pushed v5] gdb/manual: Introduce location specs) Eli Zaretskii
2022-06-02 11:10       ` Pedro Alves
2022-06-02 11:49         ` Eli Zaretskii
2022-06-02 12:40           ` Pedro Alves
2022-06-02 12:56             ` Eli Zaretskii
2022-06-02 13:44               ` Pedro Alves
2022-06-02 16:37                 ` 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=838rqmm7gb.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=gdb-patches@sourceware.org \
    --cc=pedro@palves.net \
    /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).