Re: [PATCHv2 2/2] gdb: Change how frames are selected for 'frame' and 'info frame'.

[Andrew Burgess <andrew dot burgess at embecosm dot com>]

Eli,

Thanks for your feedback.  I've addressed two of your comments, but
wanted to better understand your third point so that I could address
is correctly...

* Eli Zaretskii <eliz@gnu.org> [2018-05-11 16:46:30 +0300]:

> > 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?

I don't really understand what it is you're asking here, but I think
it might be related to overloading of the word "frame", and possibly
the keyword "create" not being a good choice.

Sure, within the inferior there are a set of frames, some of these
form the current stack, and some might exist on some alternative
stack.  Those frames exist regardless of GDB's ability to visualise
them.

However, there's a second use of the word frame, which we use for
GDB's representation of a stack-frame.  Under this second meaning the
only frames that exist are those GDB can derive from the current
machine state.  So, if the user asks for a backtrace and is told about
#0 A, #1 B, #2 C, then GDB only knows about those 3 frames.

If the user knows of #3 D that called C (but the unwind failed for
some reason) then they can use: 'frame create STACK-ADDR PC-ADDR' to
"create" a suitable frame and examine the machine state.  The frame
being "created" here is really a GDB frame, that is, a representation
of a preexisting inferior frame.

Similarly, if there's a second stack the user can "create" frames to
represent the frames on the second stack.

What we /can't/ do at the moment is backtrace out of these newly
created frames, which is a bit of a shame.  I do have a plan to work
on this, but that's for another day...

Anyway, I'm happy to rework the text if you can suggest some
improvements.  Alternatively, maybe it was my choice of "create" as
the keyword that was confusing... again, if you have any better
suggestions I'm happy to change it, I'm not tied to "create".

Thanks,

Andrew