public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Andrew Burgess <andrew.burgess@embecosm.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCHv2 2/2] gdb: Change how frames are selected for 'frame' and 'info frame'.
Date: Fri, 11 May 2018 15:44:00 -0000	[thread overview]
Message-ID: <83k1sanw3t.fsf@gnu.org> (raw)
In-Reply-To: <63020671a997f926cd747677cd4e614e51e81f8d.1525797846.git.andrew.burgess@embecosm.com>	(message from Andrew Burgess on Tue, 8 May 2018 17:58:45 +0100)

> From: Andrew Burgess <andrew.burgess@embecosm.com>
> Cc: Andrew Burgess <andrew.burgess@embecosm.com>
> Date: Tue,  8 May 2018 17:58:45 +0100
> 
> The 'frame' command, and thanks to code reuse the 'info frame' and
> 'select-frame' commands, currently have an overloaded mechanism for
> selecting a frame.
> 
> These commands take one or two parameters, if it's one parameter then
> we first try to use the parameter as an integer to select a frame by
> level (or depth in the stack).  If that fails then we treat the
> parameter as an address and try to select a stack frame by
> stack-address.  If we still have not selected a stack frame, or we
> initially had two parameters, then we create a new stack frame and
> select that.
> 
> The result of this is that a typo by the user, entering the wrong stack
> frame level for example, can result in a brand new frame being created
> rather than an error.
> 
> The purpose of this commit is to remove this overloading, while still
> offering the same functionality through some new sub-commands.

An alternative could be to request confirmation before creating a new
frame.

> diff --git a/gdb/NEWS b/gdb/NEWS
> index 46f6635dda0..fe91887cde4 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -12,6 +12,35 @@
>  * C expressions can now use _Alignof, and C++ expressions can now use
>    alignof.
>  
> +* Changes to the "frame", "select-frame", and "info frame" CLI
> +  commands, as well as the "-stack-select-frame" MI command.
> +  Selecting frames by number remains unchanged, however, selecting
> +  frames by stack-address, or creating new frames now requires the use
> +  of a sub-command.  Various sub-commands now exist for the various
> +  methods of selecting a frame; level, address, function, and create.
> +  As an example, here are the variants of "frame" that are now available:
> +  - frame <number>
> +  - frame level <number>
> +    Both of these select frame at level <number>.
> +  - frame address <stack-address>
> +    Select frame for <stack-address>.
> +  - frame function <name>
> +    Select inner most frame for function <name>.
> +  - frame create <stack-address>
> +    Create a frame for <stack-address>.
> +  - frame create <stack-address> <pc-address>
> +    Create a frame for <stack-address> <pc-address>.
> +  There are similar variants for the "select-frame" and "info frame"
> +  commands.  The MI command "-stack-select-frame" places the
> +  sub-command name after the command name as a keyword, the available
> +  variants are:
> +  -stack-select-frame <number>
> +  -stack-select-frame level <number>
> +  -stack-select-frame address <stack-address>
> +  -stack-select-frame function <name>
> +  -stack-select-frame create <stack-address>
> +  -stack-select-frame create <stack-address> <pc-address>

This is IMO too wordy for NEWS.  I think it's enough to mention that
new sub-command and tell that creation of new frames now requires that
sub-command.

> -@item frame @var{stack-addr} [ @var{pc-addr} ]
> -@itemx f @var{stack-addr} [ @var{pc-addr} ]
> -Select the frame at address @var{stack-addr}.  This is useful mainly if the
> -chaining of stack frames has been damaged by a bug, making it
> -impossible for @value{GDBN} to assign numbers properly to all frames.  In
> -addition, this can be useful when your program has multiple stacks and
> -switches between them.  The optional @var{pc-addr} can also be given to
> -specify the value of PC for the stack frame.
> +@kindex frame address
> +@item address @var{stack-address}
> +Select the frame with stack address @var{stack-address}.  Recall that
> +each stack frame has a stack frame address, which roughly corresponds
> +to the location on the stack where the stack frame local variables are
> +stored.

The text in the "Recall" sentence IMO doesn't convey anything useful,
it just says that each frame has a stack address which can be used to
select it, i.e. repeats what the previous sentence already said.

> +@kindex frame create
> +@item create @var{stack-address} @r{[} @var{pc-addr} @r{]}
> +Create and then select a new frame at stack address @var{stack-addr}.
> +This is useful mainly if the chaining of stack frames has been damaged
> +by a bug, making it impossible for @value{GDBN} to assign numbers
> +properly to all frames.  In addition, this can be useful when your
> +program has multiple stacks and switches between them.  The optional
> +@var{pc-addr} can also be given to specify the value of PC for the
> +stack frame.

I'm surprised: is it true that PC-ADDR is used only for creating new
stack frames?  The description refers to programs with multiple
stacks, in which case I'd expect to be able to refer to existing stack
frames on another stack.  The original text seemed to allow that, at
least implicitly.  Also, is it true that damaged stack is only
relevant when creating new frames?

Thanks.

  reply	other threads:[~2018-05-11 14:47 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-08 16:58 [PATCHv2 0/2] Changes to frame selection Andrew Burgess
2018-05-08 16:58 ` [PATCHv2 1/2] gdb: Split func_command into two parts Andrew Burgess
2018-05-18 19:57   ` Pedro Alves
2018-05-21 15:52     ` Andrew Burgess
2018-05-21 16:06       ` Pedro Alves
2018-05-08 16:59 ` [PATCHv2 2/2] gdb: Change how frames are selected for 'frame' and 'info frame' Andrew Burgess
2018-05-11 15:44   ` Eli Zaretskii [this message]
2018-05-21 12:16     ` Andrew Burgess
2018-05-21 17:46       ` Eli Zaretskii
2018-06-05 18:53         ` Andrew Burgess
2018-06-05 21:16           ` Philippe Waroquiers
2018-06-06  8:22             ` Andrew Burgess
2018-06-06 14:56               ` Eli Zaretskii
2018-06-07 16:19   ` [PATCHv3] " Andrew Burgess
2018-06-29 12:23     ` Andrew Burgess
2018-07-17 15:58     ` [PATCHv4] " Andrew Burgess
2018-07-23 20:46       ` Philippe Waroquiers
2018-07-25 18:14         ` Andrew Burgess
2018-08-13 22:20           ` [PATCHv5_A 1/2] " Andrew Burgess
     [not found]           ` <cover.1534197765.git.andrew.burgess@embecosm.com>
2018-08-13 22:20             ` [PATCHv5_B 2/2] " Andrew Burgess
2018-08-13 22:20           ` [PATCHv5 0/2] " Andrew Burgess
2018-08-14 10:31             ` Philippe Waroquiers
2018-08-21 13:10               ` Joel Brobecker
2018-08-27 11:04             ` Andrew Burgess
2018-08-27 15:23               ` Eli Zaretskii
2018-08-28  8:43                 ` Andrew Burgess
2018-08-28  9:08                   ` Eli Zaretskii
2018-08-28 18:03                     ` [PATCHv6] " Andrew Burgess
2018-08-28 18:20                       ` Eli Zaretskii
2018-09-05  7:46                       ` PING: " Andrew Burgess
2018-09-13 18:02                       ` Pedro Alves
2018-09-18 23:01                         ` Andrew Burgess
2018-09-19 16:26                           ` Pedro Alves
2018-09-26 23:06                             ` Andrew Burgess
2018-09-27 20:58                               ` Pedro Alves

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=83k1sanw3t.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=andrew.burgess@embecosm.com \
    --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).