From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 118274 invoked by alias); 23 Jul 2018 20:46:03 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 118256 invoked by uid 89); 23 Jul 2018 20:46:02 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-7.6 required=5.0 tests=BAYES_00,GIT_PATCH_2,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy=views, trial, Andrew, andrew X-HELO: mailsec118.isp.belgacom.be Received: from mailsec118.isp.belgacom.be (HELO mailsec118.isp.belgacom.be) (195.238.20.114) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 23 Jul 2018 20:45:58 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=skynet.be; i=@skynet.be; q=dns/txt; s=securemail; t=1532378758; x=1563914758; h=message-id:subject:from:to:date:in-reply-to:references: mime-version:content-transfer-encoding; bh=ijpcNA/PQjaiynjx01R/Rd987MkVKoIryViTEwMi0NU=; b=HXA2in9cM07j6ocCJO3/+GPgwiy1nJAXXaZzwZLgVuftOt+s4W8JLn3T lDFlBuNnhOUeMUaoo0BPr5iVE70GHg==; Received: from 14.172-134-109.adsl-dyn.isp.belgacom.be (HELO md) ([109.134.172.14]) by relay.skynet.be with ESMTP/TLS/AES256-GCM-SHA384; 23 Jul 2018 22:45:55 +0200 Message-ID: <1532378755.1467.1.camel@skynet.be> Subject: Re: [PATCHv4] gdb: Change how frames are selected for 'frame' and 'info frame'. From: Philippe Waroquiers To: Andrew Burgess , gdb-patches@sourceware.org Date: Mon, 23 Jul 2018 20:46:00 -0000 In-Reply-To: <20180717155751.30898-1-andrew.burgess@embecosm.com> References: <20180607161915.9425-1-andrew.burgess@embecosm.com> <20180717155751.30898-1-andrew.burgess@embecosm.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2018-07/txt/msg00670.txt.bz2 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 '. 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 ', where 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 ' : 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
', where
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 ', where is a function name, > the inner most frame for function is selected. If there is no > frame for function then the user gets an error. > > (5) A string like 'view ', this views a new frame > with stack address . > > (6) A string like 'view ', this views > a new frame with stack address an the pc . > > 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