From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 122838 invoked by alias); 21 May 2018 11:54:06 -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 122824 invoked by uid 89); 21 May 2018 11:54:05 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=damaged, Those, convey, shame X-HELO: mail-wm0-f43.google.com Received: from mail-wm0-f43.google.com (HELO mail-wm0-f43.google.com) (74.125.82.43) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 21 May 2018 11:54:03 +0000 Received: by mail-wm0-f43.google.com with SMTP id n10-v6so26052815wmc.1 for ; Mon, 21 May 2018 04:54:02 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=rI3k0fDHXRwzKnnRiOsVF2HmpW043zgScDJP6Ppa92g=; b=HDMIRzflQ8mHKOvG9J5d49iFXGchIj7wycwmm0RYZFplTrWui2lf2/WvO4UToty2wH g/St2zPZ/AKuAlAagd+DVDe+35P+KOk27xQ7Bahos8PaYnrsvA4tH1/XEI0ji0FPKezl UoBKzxp9OZBrzYBjFOSTpVEEgbDsfRwLiyoSWVwVX5GqA7ysEa3fzuGzMIOqQN7Mra2O 8Vgfqk8FekYi9TmbFkrykpbXzpPuRrbjLWLtTUfYNGTJ5VvGT46amECtS4E5m+ZlpTV8 kHz7K/feQKyXh4pSCOJQcCncUgyhMaJ0hqD9WS/Pu32/ekAra0iJ6jdYnaUZcMMsuQLx MlCQ== X-Gm-Message-State: ALKqPwe+YEA2bHV80LtXBDLLR6s0qHvwwdRwOYEHgG9Ll9SMJEYjiz4V K5K0xvIjD69e8bW/vnkgRS/V/rg+ X-Google-Smtp-Source: AB8JxZqDLhk2Vg4eXsbokW19y3dTd+G+WpirATUBFcT+kyKU7NGe5Ja/jqPlQiOxTCcumV1bViUa/w== X-Received: by 2002:a1c:7a0b:: with SMTP id v11-v6mr10985395wmc.58.1526903641126; Mon, 21 May 2018 04:54:01 -0700 (PDT) Received: from localhost ([78.40.158.50]) by smtp.gmail.com with ESMTPSA id 16-v6sm29840406wrt.20.2018.05.21.04.53.58 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 21 May 2018 04:54:00 -0700 (PDT) Date: Mon, 21 May 2018 12:16:00 -0000 From: Andrew Burgess To: Eli Zaretskii Cc: gdb-patches@sourceware.org Subject: Re: [PATCHv2 2/2] gdb: Change how frames are selected for 'frame' and 'info frame'. Message-ID: <20180521115357.GS3797@embecosm.com> References: <63020671a997f926cd747677cd4e614e51e81f8d.1525797846.git.andrew.burgess@embecosm.com> <83k1sanw3t.fsf@gnu.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <83k1sanw3t.fsf@gnu.org> X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes X-SW-Source: 2018-05/txt/msg00473.txt.bz2 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 [2018-05-11 16:46:30 +0300]: > > From: Andrew Burgess > > Cc: Andrew Burgess > > 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 > > + - frame level > > + Both of these select frame at level . > > + - frame address > > + Select frame for . > > + - frame function > > + Select inner most frame for function . > > + - frame create > > + Create a frame for . > > + - frame create > > + Create a frame for . > > + 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 > > + -stack-select-frame level > > + -stack-select-frame address > > + -stack-select-frame function > > + -stack-select-frame create > > + -stack-select-frame create > > 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