Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Andrew Burgess" <aburgess@broadcom.com>
To: "Mark Kettenis" <mark.kettenis@xs4all.nl>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH] Consistent display of "<optimized out>"
Date: Tue, 06 Aug 2013 16:02:00 -0000	[thread overview]
Message-ID: <52011DF8.3050102@broadcom.com> (raw)
In-Reply-To: <201308061541.r76FfYQN022875@glazunov.sibelius.xs4all.nl>

On 06/08/2013 4:41 PM, Mark Kettenis wrote:
>> Date: Tue, 6 Aug 2013 14:49:03 +0100
>> From: "Andrew Burgess" <aburgess@broadcom.com>
>>
>> On 06/08/2013 2:18 PM, Mark Kettenis wrote:
>>>> Date: Tue, 6 Aug 2013 14:08:46 +0100
>>>> From: "Andrew Burgess" <aburgess@broadcom.com>
>>>>
>>>> In some cases we report optimized out registers as "*value not available*"
>>>> rather than "<optimized out>", the patch below makes this more consistent
>>>> in the case I've spotted.
>>>>
>>>> Here's an example:
>>>>
>>>> (gdb) up
>>>> #1  0x0000000000400800 in first_frame () at dw2-reg-undefined.c:27
>>>> 27      in dw2-reg-undefined.c
>>>> (gdb) info registers rax
>>>> rax            *value not available*
>>>> (gdb) p/x $rax
>>>> $1 = <optimized out>
>>>>
>>>> After the patch the behaviour is now:
>>>>
>>>> (gdb) up
>>>> #1  0x0000000000400800 in first_frame () at dw2-reg-undefined.c:27
>>>> 27      in dw2-reg-undefined.c
>>>> (gdb) info registers rax
>>>> rax            <optimized out>
>>>> (gdb) p/x $rax
>>>> $1 = <optimized out>
>>>>
>>>> The behaviour for values that are unavailable is currently unchanged,
>>>> though I have a follow up patch for this too.
>>>>
>>>> OK to apply?
>>>
>>> I'd say no.  There is a difference between "unavailable" and
>>> "optimized out".  Registers will be unavailable even if you compile
>>> without any optimization, because the ABI specifies that their
>>> contents are not saved across function calls.  So "optimized out"
>>> makes very little sense for registers.
>>
>> I disagree for 3 reasons.
>>
>> 1. This patch is about consistency, having "print <reg>" report one
>> thing and "info register <reg>" report another seems like a bad thing to
>> me.
>>
>> 2. We previously fetched the register by calling
>> deprecated_frame_register_read, this eventually gets a value object by
>> called frame_unwind_register_value, we then extract the unavailable and
>> optimized-out attributes from the value object and for no good reason I
>> can see create a new value object and mark it as unavailable.  My patch
>> side-steps this middle ground of calling
>> deprecated_frame_register_read, and instead works with the value we get
>> back by reading the register, this is already marked optimized out.  My
>> interpretation of your concern would be that you don't think it /should/
>> be marked as optimized out, I believe however, that this is not really
>> an issue for this patch, if this changes later then we'd revert back to
>> printing unavailable rather than optimized out, my patch doesn't prevent
>> that, it just prints the real state of the value object.
>>
>> 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.

If I understand correctly you're talking here not about how gdb
represents registers, but registers in a "real" sense.  I'm not sure I
agree with your sentence, I guess it depends on your exact definition of
"unavailable", clearly optimized out could be a sub-set of unavailable.

>                                                             A register
> being unavailble implies that a variable that is supposed to live in
> such a register may have been optimized out.  

I agree with this.

>                                               Whether GDB's pseudo
> variables that respresent registers are considered unavailable or
> optimized out in that case is arguable.

I don't really understand what "that case" is referring too, nor what
"pseudo variables" are.  We can consider different cases, for example, a
request for a program variable that has been stored in a register that
is not preserved over an ABI call, this will create a gdb value object
with a location of the specified register, and attempt to fetch the
register will then mark the value as (in the example I've already cited)
optimized out.  Similarly, a direct request to gdb for the specified
register would result in a value that is marked optimiszed out.

I've only given as an example the DWARF unwinder, I guess other
unwinders could handle registers lost over a call differently and mark
them as unavailable, however, I'm not aware of any that do this, though
I've not looked that hard, can you give an example?

Also I thought, though I could be wrong on this, that the gdb value
"unavailable" was currently only used for value content that had not
been collected through tracing, though I believe there are a few places,
such as the one I'm patching here, where the value "unavailable" state
is being set incorrectly.  Do you have any examples of how the gdb value
"unavailable" state is intentionally used for something other than
non-collected trace content?

I think you're suggesting that the use of optimized out as a way to mark
registers lost due to the call ABI is wrong, and though I can see how
having a separate state for this, or maybe using unavailable, might be a
good thing it's certainly not what this patch is about, nor do I think
this patch is a step away from that, if that is indeed a goal for gdb,
even after my patch, if a value is marked unavailable rather than
optimized out, it will be displayed as unavailable.

Thanks,
Andrew









  reply	other threads:[~2013-08-06 16:02 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-06 13:09 Andrew Burgess
2013-08-06 13:18 ` Mark Kettenis
2013-08-06 13:49   ` Andrew Burgess
2013-08-06 15:41     ` Mark Kettenis
2013-08-06 16:02       ` Andrew Burgess [this message]
2013-08-06 18:39       ` Pedro Alves
2013-08-12 13:32         ` Andrew Burgess
2013-08-12 13:55           ` Pedro Alves
2013-08-12 14:01             ` Andrew Burgess
2013-08-12 20:01             ` Mark Kettenis
2013-08-13  8:27               ` Andrew Burgess
2013-08-16 18:41               ` Pedro Alves
2013-08-16 20:28                 ` Pedro Alves
2013-08-19 10:25                 ` Andrew Burgess
2013-09-05 16:29                   ` [PATCH] Print registers not saved in the frame as "<not saved>", instead of "<optimized out>". (was: Re: [PATCH] Consistent display of "<optimized out>") Pedro Alves
2013-09-05 16:35                     ` [PATCH] Print registers not saved in the frame as "<not saved>", instead of "<optimized out>" Andrew Burgess
2013-09-16 19:05                       ` Pedro Alves
2013-09-18 14:04                         ` Andrew Burgess
2013-09-18 15:54                           ` [PATCH+DOC] " Pedro Alves
2013-09-18 16:30                             ` Eli Zaretskii
2013-09-18 17:35                               ` Pedro Alves
2013-09-18 19:35                                 ` Eli Zaretskii
2013-09-18 20:47                               ` Mark Kettenis
2013-09-19  7:53                                 ` Eli Zaretskii
2013-09-19 16:58                                   ` Pedro Alves
2013-09-19 19:15                                     ` [PATCH] Always print call-clobbered registers in outer frames. (was: Re: [PATCH+DOC] Print registers not saved in the frame as "<not saved>", instead of "<optimized out>".) Pedro Alves
2013-09-19 19:35                                       ` Eli Zaretskii
2013-09-19 23:13                                       ` Doug Evans
2013-09-19 23:22                                       ` Doug Evans
2013-09-20 11:04                                       ` [PATCH] Always print call-clobbered registers in outer frames Andrew Burgess
2013-09-24 12:07                                         ` Pedro Alves
2013-09-24 12:56                                           ` Andrew Burgess
2013-09-24 13:43                                             ` Pedro Alves
2013-09-24 15:18                                               ` Andrew Burgess
2013-09-24 19:36                                                 ` Pedro Alves
2013-09-24 23:22                                                   ` Andrew Burgess
2013-10-02 16:05                                                     ` Pedro Alves
2013-10-02 19:07                                                       ` Doug Evans
2013-09-20 12:28                                       ` [PATCH] Always print call-clobbered registers in outer frames. (was: Re: [PATCH+DOC] Print registers not saved in the frame as "<not saved>", instead of "<optimized out>".) Mark Kettenis
2013-09-24 12:06                                         ` [PATCH] Always print call-clobbered registers in outer frames Pedro Alves
2013-10-02 16:17                                     ` [PATCH+DOC] Print registers not saved in the frame as "<not saved>", instead of "<optimized out>" Pedro Alves
2013-09-18 16:30                             ` Andreas Schwab
2013-09-18 17:36                               ` Pedro Alves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52011DF8.3050102@broadcom.com \
    --to=aburgess@broadcom.com \
    --cc=gdb-patches@sourceware.org \
    --cc=mark.kettenis@xs4all.nl \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox