Re: [PATCH] Always print call-clobbered registers in outer frames.

["Andrew Burgess" <aburgess at broadcom dot com>]

On 24/09/2013 2:43 PM, Pedro Alves wrote:
> On 09/24/2013 01:56 PM, Andrew Burgess wrote:
> 
>>>> My question then is, what makes you believe the inner, possibly
>>>> call-clobbered value is right?
>>>
>>> Nothing, of course.  It's just a choice -- either assume it's right,
>>> and somehow warn the user it may not be right through some other means;
>>> or, assume it's not right, and hide the value from the user.  If
>>> GDB is wrong, the user can still fetch the value, though it'll take
>>> more steps (up+p $reg+down, etc.).  It might still be a good idea to
>>> provide an easier way to "flatten" the not-saved registers as
>>> convenience for that, not sure).
>>
>> I guess the thing I'm struggling with is why would we /ever/ assume the
>> value in an inner frame is correct?  
> 
> "correct" depends on the definition.  If we defined registers in
> frame > 0 the way I suggested in response to Mark, then they always are.

I've included the definition here for reference:

  "The values of registers in the outer frame, are defined as
   being the values the registers would have if the inner frame
   was to return immediately, respecting relevant ABI rules."

I dislike that definition.  I'd prefer:

  "The values of registers in the outer frame, are defined as
   being the value that was in that register at the time of the
   call to the inner frame.  Sometimes this value is not available
   in which case the value will be displayed as <not-saved>."

My real concern with your definition is that I don't believe it's
correct!  If you did an "immediate" return then even those register that
are callee saved would not be restored.  I think you need to expand your
definition, otherwise it would sound like we don't do register unwinding
at all.  Assuming that we do perform register unwinding and you expand
your definition, we would then have a split register set, one half
looking backwards, one half looking forwards; we have the registers that
we unwind, these display the value at the time of the call, and the set
looking forward, these hold the value from an inner frame.

My expectation of walking back up the stack is that I'm walking back
into history, not walking into the future; I want to see the stack frame
as it _was_ at the time of the call, not how it might be at the time of
a return (or if we force a return, how it will be).  But this is just my
opinion, others might use gdb differently and have different expectations.

> Doing otherwise means assuming we know the ABI the outer and inner
> frames agreed on.

I agree ... and disagree with this statement :)

Right now on x86-64 we don't define a dwarf2_frame_set_init_reg
function, nor in the prologue scanning amd64_frame_prev_register
function do we ever return frame_unwind_got_optimized.  gdb basically
has no idea which registers are callee saved, and which are callee
clobbered.

I claim that right now, on x86-64 the behaviour we have is the behaviour
as defined by your definition of a register in an outer frame.  I'm
claiming that you'll not see <optimized-out> (or what would be
<not-saved>) for a register right now UNLESS you specifically craft some
dwarf to mark a register as undefined.

The problem is that (by my reading of the DWARF standard) the compiler
should be marking callee clobbered registers as undefined (this is based
on appendix D.6 Call Frame Information Example from DWARF4), however,
gcc, at least the versions I have to hand, don't do this.  To work
around this, on some targets, we use the dwarf2_frame_set_init_reg
function to teach gdb about the ABI, we now /do/ see registers as not-saved.

So, back to your statement that to mark registers as not-saved requires
gdb to know the ABI, you are correct that some targets do have the ABI
baked in, but this is only to work around a lack of DWARF from the compiler.

I guess, looking the this issue again, I'd suggest my argument is this,
if the DWARF or the prologue scanner has gone to the effort of saying
"this register is not saved" then we should display it as such, your
patch seems (to me) to be ignoring this and printing some other value
instead.  If a particular target (x86-64 say) wants the behaviour you've
defined then, hey, we already have that behaviour, we just don't define
a default DWARF state, and don't have the prologue scanner mark any
registers as not-saved.  No patch required.  If we do apply your (value
from inner frame) patch then it feels like we're taking away the option
for other targets to support a not-saved state for registers.

>                   There are a number of ways to get that wrong, like
> with assembly code, or even probably with gcc attributes or some JITs,
> meaning GDB will hide the value from the user behind <not saved>, when
> it should not.  Granted, not the _usual_ scenario, but I bet it's real.
> Yeah, can always be fixed in the unwind/debug itself.

I agree that it would be nice if the DWARF code didn't need to have the
ABI wired in; but that's something to take up with the gcc folk I guess.
 The prologue scanner code is harder, there's always going to be some
aspect of ABI awareness within that code, but generally that should be
the mechanism of last resort so I think we can forgive that a more than
other code.

> 
>> Sure, for very shallow stacks on
>> machines with a non-small register set there's a reasonable chance the
>> value is correct, but in any other case I seriously don't see how taking
>> the value from an inner frame is any better than picking a random number.
> 
> Again, it's a matter of definition -- "taking a value from an
> inner frame" just meant "if the unwind info doesn't say the register was
> saved, then assume that when the inner function returned, the register
> would still hold that value".
> 
>> I think that if you print it "they" (users) will use it, assuming it's
>> correct.  Given that I don't think it's correct I think we're likely to
>> be giving users miss-information.
> 
> And here is where I'm wondering what sort of use do you think they'll
> be giving them.  :-)  GDB will still print <optimized out> for variables
> that happened to be live in such registers at the time of the call.  If the
> functions involved are C or even higher-level language code, nothing
> changes, because indeed if the register is supposedly call-clobbered
> nothing in the caller will be expecting to find the old register value
> still in the register.  But, when you're peeking into machine register values
> in the debugger you're going deeper than that.  You're probably looking into
> mapping them to a disassembly of the machine code.  In my mind, at this
> level, it's not strictly correct for GDB to assume any particular ABI -- best
> be driven by unwind info, and if the unwind info doesn't specify where the
> register was saved, at least give the user some useful value, rather than
> forcing her to go hunt for it in inner frames.

Given I'm expecting a view of the frame as it was, the value is, in
general not going to be correct (for me), and so is not useful.  I'd
rather not see it.

> (Again, I think both perspectives are reasonable.  But if the
> always-print-registers route is harder to explain to other GDB developers,
> then that does hint that users wouldn't understand it either, which does
> favor the <not saved> route.)