This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Eli,

Sorry for the delay in replying, this dropped off my radar for a while
with other things...

* Eli Zaretskii <eliz@gnu.org> [2018-05-21 19:32:33 +0300]:

> > Date: Mon, 21 May 2018 12:53:57 +0100
> > From: Andrew Burgess <andrew.burgess@embecosm.com>
> > Cc: gdb-patches@sourceware.org
> > 
> > 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.
> 
> The old text was this:
> 
> > > > -@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.
> 
> This seems to imply that if the frame #3 D existed, but the unwind
> failed to find it, the user could say "frame STACK-ADDR PC-ADDR" to
> refer to that frame.
> 
> By contrast, your new text:
> 
> > > > +@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.
> 
> forces the use of "create", which is confusing, since the frame does
> exist on the stack, albeit unbeknownst to GDB.
> 
> IOW, I always thought of stack frames as existing or not independently
> of whether the GDB unwinder succeeded to find them, so it's strange
> for me to talk about "creating" a frame in this use case.

Sure. I think our mental models for stack frames and GDB are pretty
much in sync.  Like I previously tried to explain the "create" here is
only "creating" within GDB, not within the inferior....

>                                                            For that
> matter, I don't understand why we need to force the user to type
> "create" explicitly.

Because, whether we call it "creating" or "viewing an existing frame
that is in the inferior, but not in the backtrace", I do think that
there are two _very similar_ but different actions here.

One action is that the user wants to select a frame that GDB is aware
of, and is part of the backtrace.

The second action is that the user wants to use a $sp and $pc value to
visualise a stack frame that is not part of GDB's backtrace.

My assumption going into this work is that the first case (selecting
by number) is what most users do, most of the time, and that the
second case is much rarer.

By forcing the second case to live behind a new keyword we prevent the
user from accidentally visualising a new stack frame in the case where
they miss-type the stack frame number.  Under the new model,
miss-typing a frame number would give an error.

> 
> > 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".
> 
> Why do we need an extra keyword, again?
> 
> If we do need a keyword, how about "frame add"?

Personally, I think 'add' is worse than 'create' - what's the frame
being added too?  But I do acknowledge that 'create' is not ideal
either.

I wonder if 'new' is better than 'create', maybe implies less "making
something in the inferior"?  Or how about, 'for' instead, like this:

  (gdb) frame for STACK-ADDR PC-ADDR

Alternatively, could this problem be solved by a better description in
the manual, maybe something like this:

    Create within GDB a representation of a stack frame that is not
    part of the current backtrace.  The newly created frame has a
    stack address @var{stack-addr}, and an optional program counter
    address @var{pc-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.

Thanks for all your help reviewing this patch.

Andrew


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]