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

[Pedro Alves <palves at redhat dot com>]

On 09/24/2013 04:18 PM, Andrew Burgess wrote:
> 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.

No, no, no, it's correct.  That's what the "respecting relevant
ABI rules" meant -- IOW, as if the inner frame actually ran its
epilogue.

Note that <not-saved> definition isn't correct for at least the PC!
(as the program ran through the outer frame, the PC pointed to the actual
call instruction just before the call, and then it switched to point to
the the call destination, well, just right after the call.  But, the PC _never_
pointed to the call's return address in the outer frame.  Yet that's
what we present in info reg / p $pc in outer frames...  IOW, we present
the PC in the same way I was suggesting we present the other
registers.  == more consistency.)
And, in some ports, that definition is not correct for the stack
pointer either.

> 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.

Only if you choose to look at it that way.  Maybe I'm not being clear
enough.  Another way to say it would be, perhaps, that
the value of a register in the outer frame is defined as being
the value that the inner function would leave in it, were it to
immediately run its epilogue and return.

> 
> 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).  

So what's your opinion on how we present the PC register in outer frames?
Because it's not what you're suggesting.

> 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.

Did you see the patch I sent to Mark, that does just that?

> 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.

Right.

> 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.  

It also says:

 "The default rule for all columns before interpretation of the initial
 instructions is the undefined rule. However, an ABI authoring body or
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 a compilation system authoring body may specify an alternate default
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 value for any or all columns."

And see:
 http://gcc.gnu.org/ml/gcc/2006-12/msg00293.html

And:
 http://gcc.gnu.org/ml/gcc/2006-12/msg00312.html

AFAICS, that patch is in the tree.

> 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 think calling it "lack" or not that depends on what gcc, being
the "compilation system authoring body" is considering to be
the default.

> 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.  

Yes.  If the variable doesn't actually exist, the compiler doesn't
actually emit that "this register is not saved" info at all, IIUC.
The only reason the compiler emitted that was to say that the
"variable lived in this register, but the register is gone".  IOW,
it's info meant to be used at the C/higher-level language/variable
level.  IIUC, there was never a time when the compiler emitted info
saying "all these registers x,y,z are undefined (because they're
ABI call-clobbered)" in all functions.

> 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.

Well, yes and no.  If we followed my definition of outer frame registers,
the registers value of <not-saved> wouldn't ever need to exist, by
definition, but, we would instead show "[not saved]"
or same such other auxiliary, out-of-band info in "info registers"
output -- and that info would be extracted from GDB's own knowledge
of the ABI and from the debug/unwind info.

Note that the "no patch required" above isn't strictly true, as your
paragraph actually argues _against_ my AMD64 patch at:
<https://sourceware.org/ml/gdb-patches/2013-09/msg00848.html>,
and, likewise argues that rs6000, s390, sh and tic6x are actually
wrong in defaulting some registers to DWARF2_FRAME_REG_UNDEFINED.

> 
>>                   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.

Yeah.  But what does "forgive" actually mean?  I'm currently inclined
to never make the prologue scanners assume not-saved registers are
clobbered (as in, make them default to same_value, like x86 scanners
do currently).

>>> 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.)

(I wonder if what we should have is some kind of switch or knob that
allows getting _both_ behaviors from GDB...)

-- 
Pedro Alves