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: [PATCH] Always print call-clobbered registers in outer frames.


On 19/09/2013 8:15 PM, Pedro Alves wrote:
> On 09/19/2013 05:58 PM, Pedro Alves wrote:
> 
>> Now, GDB itself could just assume that call-clobbered registers
>> are always <not saved> in frames other than the innermost, but
>> that'll be not very user friendly, I think.
>>
>> Perhaps... we should completely toss out this patch, and go
>> the other way around.  When printing registers of an outer
>> frame, ignore DW_CFA_undefined, and read the registers
>> from the inner frame anyway...  IOW, define the values of
>> call-clobbered registers in outer frames as "the values the
>> registers would have if the inner frame returned _now_".
>> Mark, what do you think?
>> (I didn't try implementing that, but I think that'll just
>> mean ignoring the optimized_out flag when dumping registers
>> (but not when printing variables marked as living in "optimized
>> out" registers).
>>
> 
> Like this...
> 
> --------------
> Subject: [PATCH] Always print call-clobbered registers in outer frames.
> 
> With older GCCs, when some variable lived on a call-clobbered register
> just before a call, GCC would mark such register as undefined across
> the function call, with DW_CFA_undefined.  This is interpreted by the
> debugger as meaning the variable is optimized out at that point.  That
> is, if the user does "up", and tries to print the variable.
> 
> Newer GCCs stopped doing that.  They now just don't emit a location
> for the variable, resulting in GDB printing "<optimized out>" all the
> same.  (See <https://sourceware.org/ml/gdb-patches/2012-08/msg00787.html>.)
> 
> The difference is that with binaries produced by those older GCCs, GDB
> will also print the registers themselves (info registers / p $reg) as
> "<optimized out>".  This is confusing.  

I agree with you 100% on this, however, I feel we're merging two
arguments together here.  The reason gdb prints <optimised out> is
really because values only support 3 states: available, unavailable, and
optimized-out, we use optimized-out for the DW_CFA_undefined state.

What we really need is a new state, lets call it not-saved, we don't
necessarily need to have extra state inside the value object to model
this, we might (as in your original patch) be able to derive this state,
but this is a state that a register can be in.

> This patch makes GDB always follow this rule (which is what the
> heuristic unwinders usually do by default):
> 
>  The values of call-clobbered registers in the outer frame, if not
>  saved by the caller, are defined as being the values the registers
>  would have if the inner frame was to return immediately.

You're right that most targets seem to follow this rule, which seems odd
to me, however I think that the rs600, s390, sh, and tic6x targets
don't, they set the call-clobbered registers to DW_CFA_undefined in
their dwarf2_frame_set_init_reg functions, this seems much more sensible
to me, assuming that call-clobbered registers have not been used seems
.... odd.

> 
> The documentation is updated to more clearly explain this.
> 
> IOW, ignore DW_CFA_undefined _when printing frame registers_, but
> not when printing variables.  This translates to, if value of a frame
> register, comes out as optimized out (that's what "not saved"
> lval_register values end up as), fetch it from the next frame.

I really think this is going to confuse users, we're basically saying
that call-clobbered registers are assumed to never be clobbered .... we
might get away with this from very shallow call-stacks, but for very
deep stacks this seems very unlikely, in which case, a user switches to
some outer stack and does "info registers", how does he know which
registers gdb has carefully retrieved and are correct, and which are
just random values fetched from some inner frame?

My question then is, what makes you believe the inner, possibly
call-clobbered value is right?

> Which is what you'd get with a newer GCC.  

I think this statement is only true for platforms that don't define any
call-clobbered registers in their dwarf2_frame_set_init_reg function.
Like I said before, this feels like a bug for those platforms to me,
probably one that has only become apparent since gcc stopped marking
values as DW_CFA_undefined; I guess this has not been an issue as gcc at
the same time stopped leaving variables in these registers.

>                                            And, it's exactly what you
> get today if your force a return from frame #1, even with such an old
> binary, and _unpatched_ GDB:

I agree with this, but don't think it's relevant, your talking about
altering the control flow of the inferior, this is not what "info
registers" means to me, I'm expecting to see the value of those
registers as they were, in that frame, at the time of the call, if gdb
can't figure that out then I'd much rather gdb just says so... but maybe
I'm expecting the wrong thing...

> 
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 60d2877..23227af 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -10031,10 +10031,22 @@ were exited and their saved registers restored.  In order to see the
>  true contents of hardware registers, you must select the innermost
>  frame (with @samp{frame 0}).
>  
> -However, @value{GDBN} must deduce where registers are saved, from the machine
> -code generated by your compiler.  If some registers are not saved, or if
> -@value{GDBN} is unable to locate the saved registers, the selected stack
> -frame makes no difference.
> +Most ABIs reserve some registers as ``scratch'' registers that don't
> +need to be saved by the caller (a.k.a. caller-saved or call-clobbered
> +registers).  It may therefore not be possible for @value{GDBN} to know

I think here you should say "Most ABIs reserve some registers as
``scratch'' registers that don't need to be saved by the callee"
(callee, not caller).

> +the value a register had before the call (in other words, in the outer
> +frame), if the register value has since been changed by the callee.
> +@value{GDBN} tries to deduce where registers are saved, from the debug
> +info, unwind info, or the machine code generated by your compiler.  

I don't think this is correct either, we try to figure out where callee
saved registers are stored, but these are not the scratch registers, if
the scratch registers are needed by the caller then the caller will save
them, but the callee will just go ahead and use them.  Similarly the
DWARF only tells us about callee saved registers.  The caller saved
registers are now not mentioned in the DWARF as gcc has made sure that
no variables are live in these registers.

>                                                                       If
> +some register is not saved, or if @value{GDBN} is unable to locate the
> +saved register, @value{GDBN} will assume the register in the outer
> +frame had the same location and value it has in the inner frame. In
> +other words, the values of call-clobbered registers in the outer
> +frame, if not saved by the caller, are defined as being the values the
> +registers would have if the inner frame was to return immediately.
> +This is usually harmless, but note however that if you change a
> +register in the outer frame, you may also be affecting the inner
> +frame.

I'd just like to pick up on the "harmless" here, I agree that it would
be "harmless" in the sense that if we did a return in gdb it would be
harmless; the register is callee clobbered, the caller either does not
care what is in the register after the call, or has code to restore the
value that it does care about.  From a debugging point of few though, I
suspect showing the wrong value might be far from harmless, and if this
patch is merged we need to draw attention to the fact that the more
"outer" the frame it you're looking at, the more likely call-clobbered
registers are to be wrong.




Thanks,
Andrew


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