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

[Philippe Waroquiers <philippe.waroquiers@skynet.be>]

On Tue, 2018-07-17 at 16:57 +0100, Andrew Burgess wrote:
> Now that the 'frame apply' command has been merged to master, this is
> a rebase of the v3 patch.  Merge conflicts have been resolved, but the
> content is largely unchanged.
FWIW, I like the idea of the patch, as it is too easy to
(too silently) get a wrong frame with 'frame <a wrong frame level/number>'.

Find below some comments to discuss the terminology 'frame level'
versus 'frame number'.

Philippe

> 
> Thanks,
> Andrew
> 
> ---
> 
> 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 GDB allows the user to view a stack
> frame that is not part of the current backtrace.  Internally, a new
> frame is created with the given stack and pc addresses, and this is
> shown to the user.
> 
> 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 viewed
> 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.  By
> making the default behaviour of 'frame' (and friends) be to select a
> stack frame by level number, it is hoped that enough
> backwards-compatibility is maintained that users will not be overly
> inconvenienced.
> 
> The 'frame', 'select-frame', and 'info frame' commands now all take a
> frame specification string as an argument, this string can be any of the
> following:
> 
>   (1) An integer.  This is treated as a frame number.  If a frame for
>   that number does not exist then the user gets an error.
> 
>   (2) A string like 'level <NUMBER>', where <NUMBER> is a frame number
>   as in option (1) above.

I did a trial to scan the documentation for the usage of 'frame number',
in order to replace it by 'frame level'.
Note that Eli has some reserve about this idea/replacement,
see thread https://sourceware.org/ml/gdb-patches/2018-07/msg00365.html.

IMO, the sentences (1) and (2) above should preferably
use consistently 'frame level' 'level <LEVEL>' : at the moment,
we seem to semi-randomly use level or number, explaining at some places
that level is a frame number. 

It would be good to clarify exactly what naming convention should
be used, and apply it (more) systematically (in the existing
documentation and/or in this patch).



>   (3) A string like 'address <ADDRESS>', where <ADDRESS> is a
>   stack-frame address.  If there is no frame for this address then the
>   user gets an error.
Below, (5) and (6) are using STACK-ADDRESS instead of ADDRESS.
I am not sure to understand if there is (or not) a difference of concept
between this ADDRESS and the below STACK-ADDRESS.
I am wondering which address we are speaking about.
When doing 'info frame', I get something like:
  (gdb) info frame
  Stack level 1, frame at 0x7fffffffdf80:
   rip = 0x555555554f5e in sleeper_or_burner (sleepers.c:86); saved rip = 0x55555555549d
   called by frame at 0x7fffffffe040, caller of frame at 0x7fffffffdf40
   source language c.
   Arglist at 0x7fffffffdf70, args: v=0x7fffffffdf90
   Locals at 0x7fffffffdf70, Previous frame's sp is 0x7fffffffdf80
   Saved registers:
    rbp at 0x7fffffffdf70, rip at 0x7fffffffdf78
  (gdb) 
Maybe it would be good to reference in the doc the field given
in the above output ?


Note that the above command output uses 'Stack level 1',
and not 'Stack number 1' :).


> 
>   (4) A string like 'function <NAME>', where <NAME> is a function name,
>   the inner most frame for function <NAME> is selected.  If there is no
>   frame for function <NAME> then the user gets an error.
> 
>   (5) A string like 'view <STACK-ADDRESS>', this views a new frame
>   with stack address <STACK-ADDRESS>.
> 
>   (6) A string like 'view <STACK-ADDRESS> <PC-ADDRESS>', this views
>   a new frame with stack address <STACK-ADDRESS> an the pc <PC-ADDRESS>.
> 
> This change assumes that the most common use of the commands like
> 'frame' is to select a frame by frame level number, it is for this
> reason that this is the behaviour that is kept for backwards
> compatibility.  Any of the alternative behaviours, which are assumed to
> be less used, now require a change in user behaviour.
> 
> The MI command '-stack-select-frame' has also changed in the same way.
> The default behaviour is to select a frame by level number, but the
> other selection methods are also available.

In the rest of the patch, assuming there is an agreement about
using frame level instead of frame number, there are some other
occurrences of number to replace/clarify.

Philippe