Re: [PATCH] Consistent display of "<optimized out>"

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

On 12/08/2013 9:01 PM, Mark Kettenis wrote:
>> Date: Mon, 12 Aug 2013 14:55:04 +0100
>> From: Pedro Alves <palves@redhat.com>
>>
>> On 08/12/2013 02:31 PM, Andrew Burgess wrote:
>>> On 06/08/2013 7:39 PM, Pedro Alves wrote:
>>>> On 08/06/2013 04:41 PM, Mark Kettenis wrote:
>>>>>> Date: Tue, 6 Aug 2013 14:49:03 +0100
>>>>>> From: "Andrew Burgess" <aburgess@broadcom.com>
>>>>
>>>>>> 3. My understanding was that values lost due to the ABI of a call site
>>>>>> were recorded as optimized out.  For evidence I would present
>>>>>> dwarf2_frame_prev_register, and how DWARF2_FRAME_REG_UNDEFINED is handled.
>>>>>>
>>>>>> For these reasons I believe my patch should still be considered, what do
>>>>>> you think?
>>>>>
>>>>> I think that registers are either available or unavailble.  A register
>>>>> being unavailble implies that a variable that is supposed to live in
>>>>> such a register may have been optimized out.  Whether GDB's pseudo
>>>>> variables that respresent registers are considered unavailable or
>>>>> optimized out in that case is arguable.
>>>>
>>>> I think improving consistency as in Andrew's patch is good.
>>>
>>> Given almost a week has passed with no further feedback I plan to
>>> commit this patch tomorrow unless there's any further discussion to be had.
>>
>> TBC, note my opinion doesn't get to overrule Mark's.  Consensus
>> works much better, and Mark does have deep knowledge of all
>> ABI/pseudo registers/etc. gdb things.
>> That said, Mark, if you still disagree, please counter argue,
>> otherwise, we'll just have to assume you do agree with the
>> rationales and clarifications.
> 
> Can't say I agree.  It simply doesn't make sense for registers to be
> "optimized out".  

For the sake of this discussion lets say I agree with you, what I hope
to show is that it doesn't actually matter if I agree or not as this
patch has /nothing/ to do with solving the issue that is bothering you.

>                    I guess there are two reasons why GDB can't display
> the contents of a register in a frame:
> 
> 1. The register contents aren't made available by the debugging
>    interface, i.e. ptrace(2) or the remote stub doesn't tell us.
> 
> 2. The register wasn't saved before calling another function.

Ok, so lets look at how gdb handles these cases before my patch,

CASE 1: This results in a value object marked "unavailable", when
printed using "p $reg" we get "<unavailable>" and when printed using
"info registers reg" we get "*value not available*".

CASE 2: This currently /does/ result in a value marked "optimized-out",
when printed using "p $reg" we get "<optimized-out>" and when printed
using "info register reg" we get "*value not available*".

My patch is based on the following idea: When printing a value we should
always display the same result no matter how the value is printed.[1]

As I understand your point you believe CASE 2 should print (something
like) "<not-saved>". OK, so, how could we achieve that?  Here are a few
possible options,

(#0) Leave things alone.
This makes me unhappy as it breaks my rule about printing the same value
in different ways.  Also, I assume this does not make you happy as we
can't tell the difference between unavailable values and not-saved values.

(#1) Change the string "*value not available*" to be "<not-saved>".
This is bad because CASE 1 above would also print "<not-saved>" and I
think we agree that for CASE 1 we should print "<unavailable>".

(#2) Apply my patch, but for values marked optimized out print
"<not-saved>".
Well, this handles CASE 1 correctly[2], but now we print
"<optimized-out>" when we using "p $reg" and "<not-saved>" for "info
registers reg".  This makes me unhappy as it breaks my rule about
displaying the save value in the same way.

(#3) Apply my patch, but for value marked optimized out print
"<not-saved>", AND special case other places we print "<optimized-out>"
to print "<not-saved>".
So in theory this gets you what you want.  We don't think registers
should ever be "optimized-out" so we don't miss our inability to print
optimized out registers, but there are two problems, first, we litter
the printing code with special cases to print either "<optimized-out>"
or "<not-saved>", and second, I suspect given that your presented with a
value marked optimized out, deciding whether it should be printed as
optimized out or not saved will be hard (and unreliable).

(#4) Replace every print of "<optimized-out>" with "<not-saved>".
I mention this just for completeness, I hope we both agree this is just
silly, obviously DWARF can mark variables as optimized out, so we do
have to support values that are optimized out, and print them as
"<optimized-out>".

(#5) Introduce a new value type "not-saved".
This is the only way I can see that you will get what you want.  Values
that are in registers that are lost due to ABI would be marked as
"not-saved", and printed as "<not-saved>".  It turns out that I've just
submitted a patch set[3] which would make supporting a new reason really
easy, however this is /not/ what this patch is about.


So, what is this patch about.  This patch is about printing what a value
/is/, if a value /is/ unavailable print "<unavailable>", if a value /is/
optimized out, print "<optimized-out>" ... in some future world if a
value /is/ not-saved, then print "<not-saved>".

The problem is that you want a not-saved state, and currently that
doesn't exist in gdb.

If once you have introduce the not-saved state then you could, if you
like change the code:
   if (optimized_out (value))
     print "<optimized-out>"
to be:
   gdb_assert (!optimized_out (value));

and sure, that would be a reverting part of this patch, but it doesn't
feel like a huge amount of work, and given that right now register
values /are/ optimized out, surely we should print them correctly?

Thanks,
Andrew

[1] Obviously here I'm talking about different ways to ask for
effectively the same view of the value, so "p $reg" and "info register reg".

[2] Or will once my follow-up patch is applied:
    http://sourceware.org/ml/gdb-patches/2013-08/msg00171.html

[3] http://sourceware.org/ml/gdb-patches/2013-08/msg00300.html

> 
> I guess after Andrew's chnages 1) would be shown as <unavailable> and
> 2) would become <optimized out>.  But in the latter case something
> like <not saved> would make more sense.
> 
> That said, Pedro, you're pretty much the expert for this area of GDB.
> So If you think Andrew should go ahead with this, feel free to ignore
> me.
> 
> 
>