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

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

[Andrew Burgess]

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?

Thanks,
Andrew

gdb/ChangeLog

2013-08-06  Andrew Burgess  <aburgess@broadcom.com>

	* infcmd.c (default_print_one_register_info): Add detection of
	optimized out values.
	(default_print_registers_info): Switch to using
	get_frame_register_value.

gdb/testsuite/ChangeLog

2013-08-06  Andrew Burgess  <aburgess@broadcom.com>

	* gdb.dwarf2/dw2-reg-undefined.exp: Change pattern for info
	register to "<optimized out>", and also print the registers.

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 154cde2..f6a5290 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2033,6 +2033,12 @@ default_print_one_register_info (struct ui_file *file,
       fprintf_filtered (file, "*value not available*\n");
       return;
     }
+  else if (value_optimized_out (val))
+    {
+      val_print_optimized_out (file);
+      fprintf_filtered (file, "\n");
+      return;
+    }
 
   /* If virtual format is floating, print it that way, and in raw
      hex.  */
@@ -2107,9 +2113,6 @@ default_print_registers_info (struct gdbarch *gdbarch,
 
   for (i = 0; i < numregs; i++)
     {
-      struct type *regtype;
-      struct value *val;
-
       /* Decide between printing all regs, non-float / vector regs, or
          specific reg.  */
       if (regnum == -1)
@@ -2137,16 +2140,9 @@ default_print_registers_info (struct gdbarch *gdbarch,
 	  || *(gdbarch_register_name (gdbarch, i)) == '\0')
 	continue;
 
-      regtype = register_type (gdbarch, i);
-      val = allocate_value (regtype);
-
-      /* Get the data in raw format.  */
-      if (! deprecated_frame_register_read (frame, i, value_contents_raw (val)))
-	mark_value_bytes_unavailable (val, 0, TYPE_LENGTH (value_type (val)));
-
       default_print_one_register_info (file,
 				       gdbarch_register_name (gdbarch, i),
-				       val);
+				       get_frame_register_value (frame, i));
     }
 }
 
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
index 7b7b4d1..4686648 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
@@ -38,22 +38,34 @@ if ![runto stop_frame] {
 gdb_test "bt" "#0  (0x\[0-9a-f\]+ in )?stop_frame \[^\r\n\]*\r\n#1  \[^\r\n\]*first_frame \[^\r\n\]*\r\n#2  \[^\r\n\]*main\[^\r\n\]*" \
     "backtrace from stop_frame"
 
-set value_pattern "0x\[0-9a-f\]+\\s+\[0-9\]+"
-set opt_out_pattern "\\*value not available\\*"
-
 for {set f 0} {$f < 3} {incr f} {
     if {${f} == 0} {
-	set pattern_rax_rbx_rcx ${value_pattern}
-	set pattern_r8_r9 ${value_pattern}
+	set pattern_rax_rbx_rcx_print "$hex"
+	set pattern_rax_rbx_rcx_info "$hex\\s+$decimal"
+	set pattern_r8_r9_print "$hex"
+	set pattern_r8_r9_info "$hex\\s+$decimal"
     } else {
-	set pattern_rax_rbx_rcx ${opt_out_pattern}
-	set pattern_r8_r9 ${value_pattern}
+	set pattern_rax_rbx_rcx_print "<optimized out>"
+	set pattern_rax_rbx_rcx_info "<optimized out>"
+	set pattern_r8_r9_print "$hex"
+	set pattern_r8_r9_info "$hex\\s+$decimal"
     }
 
     # Select frame.
     gdb_test "frame ${f}" "#${f}.*" "Switch to frame ${f}"
 
+    gdb_test "p/x \$rax" ".*$pattern_rax_rbx_rcx_print.*" \
+	"print \$rax in frame ${f}"
+    gdb_test "p/x \$rbx" "$pattern_rax_rbx_rcx_print" \
+	"print \$rbx in frame ${f}"
+    gdb_test "p/x \$rcx" "$pattern_rax_rbx_rcx_print" \
+	"print \$rcx in frame ${f}"
+
+    gdb_test "p/x \$r8" "$pattern_r8_r9_print" "print \$r8 in frame ${f}"
+    gdb_test "p/x \$r9" "$pattern_r8_r9_print" "print \$r9 in frame ${f}"
+
+
     # Display register values.
-    gdb_test "info registers rax rbx rcx r8 r9" "rax\\s+${pattern_rax_rbx_rcx}\\s*\r\nrbx\\s+${pattern_rax_rbx_rcx}\\s*\r\nrcx\\s+${pattern_rax_rbx_rcx}\\s*\r\nr8\\s+${pattern_r8_r9}\\s*\r\nr9\\s+${pattern_r8_r9}\\s*" \
+    gdb_test "info registers rax rbx rcx r8 r9" "rax\\s+${pattern_rax_rbx_rcx_info}\\s*\r\nrbx\\s+${pattern_rax_rbx_rcx_info}\\s*\r\nrcx\\s+${pattern_rax_rbx_rcx_info}\\s*\r\nr8\\s+${pattern_r8_r9_info}\\s*\r\nr9\\s+${pattern_r8_r9_info}\\s*" \
 	"Check values of rax, rbx, rcx, r8, r9 in frame ${f}"
 }

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

[Mark Kettenis]

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

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

[Andrew Burgess]

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?

Thanks,
Andrew

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

[Mark Kettenis]

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

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

[Andrew Burgess]

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

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

[Pedro Alves]

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.

When <unavailable> values were introduced, this particular
"info registers"/"*value not available*" string already existed,
and it was never adjusted.

The distinction between <unavailable> and <optimized out> is:

 - <optimized out> means that the object no longer exists in the
   compiled binary, because the compiler got rid of it (the debug
   info tells us there's no way to retrieve it).

 - <unavailable> means that the object was not optimized out, and actually
   exists in the program (and the debug info describes how to get at it), but
   we don't have access to the bits necessary to extract/compute the object
   from -- e.g., a core file section has been truncated, a tracepoint did
   not collect the necessary registers or memory, etc..

I favor normalizing GDB's output in this direction.

-- 
Pedro Alves

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

[Andrew Burgess]

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.

Thanks,
Andrew

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

[Pedro Alves]

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.  In any case, Andrew, please wait
until someone gives the patch an OK.  I did not look at the patch at
all in any detail, and/or whether it actually follows the guidelines
I presented.

-- 
Pedro Alves

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

[Andrew Burgess]

On 12/08/2013 2:55 PM, Pedro Alves wrote:
> 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.  In any case, Andrew, please wait
> until someone gives the patch an OK.  I did not look at the patch at
> all in any detail, and/or whether it actually follows the guidelines
> I presented.

Thanks for the reply.  This is why I posted before committing, I wasn't
sure quite what state this patch was in.  I'm more than happy to wait.

Cheers,
Andrew

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

[Mark Kettenis]

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

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.

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

[Andrew Burgess]

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

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

[Pedro Alves]

On 08/12/2013 09: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".  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.
> 
> 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.

This is a tough call.  I do agree that "optimized out" for registers
is a bit confusing.  However, we already do print "<optimized out>" in
other places, such as when printing expressions, and consistency
is good.  If we did add a distinction, I agree with Andrew that it should
be done in a more systematic way.  However, I'm not really sure we need
much machinery.  Wouldn't something like:

void
val_print_optimized_out (const struct value *val, struct ui_file *stream)
{
  if (value_lval_const (val) == lval_register)
    fprintf_filtered (stream, _("<not saved>"));
  else
    fprintf_filtered (stream, _("<optimized out>"));
}

work?  What could be the register value cases that would print
"not saved" that we'd still want to print "optimized out" ?

...

Hmm, I spent a bit playing with this, and here's what I got.  At
first glance, the changes in output all look reasonable to me.

frame_unwind_got_optimized needed a tweak to return an
lval_register value; it's returning not_lval presently.

Pushed here (along with Andrew's patch) for convenience:
 https://github.com/palves/gdb/commits/register_not_saved
 git@github.com:palves/gdb.git register_not_saved

Here's the (trimmed for brevity) diff of the resulting gdb.log between
after Andrew's path and after my patch, of a run of 
gdb.dwarf2/dw2-op-out-param.exp gdb.dwarf2/dw2-reg-undefined.exp
gdb.mi/mi-reg-undefined.exp, the only tests affected.

(Code patch further below.)

~~~~

 (gdb) PASS: gdb.dwarf2/dw2-reg-undefined.exp: Switch to frame 1
 p/x $rax
-$6 = <optimized out>
+$6 = <not saved>
 (gdb) PASS: gdb.dwarf2/dw2-reg-undefined.exp: print $rax in frame 1
 p/x $rbx
-$7 = <optimized out>
+$7 = <not saved>
 (gdb) PASS: gdb.dwarf2/dw2-reg-undefined.exp: print $rbx in frame 1
 p/x $rcx
-$8 = <optimized out>
+$8 = <not saved>
 (gdb) PASS: gdb.dwarf2/dw2-reg-undefined.exp: print $rcx in frame 1
 p/x $r8
 $9 = 0x323d7b1f40
@@ -109,9 +109,9 @@ p/x $r9
 $10 = 0x323d00f310
 (gdb) PASS: gdb.dwarf2/dw2-reg-undefined.exp: print $r9 in frame 1
 info registers rax rbx rcx r8 r9
-rax            <optimized out>
-rbx            <optimized out>
-rcx            <optimized out>
+rax            <not saved>
+rbx            <not saved>
+rcx            <not saved>
 r8             0x323d7b1f40    215779843904
 r9             0x323d00f310    215771837200
 (gdb) PASS: gdb.dwarf2/dw2-reg-undefined.exp: Check values of rax, rbx, rcx, r8, r9 in frame 1
@@ -120,13 +120,13 @@ frame 2
 33       first_frame ();
 (gdb) PASS: gdb.dwarf2/dw2-reg-undefined.exp: Switch to frame 2
 p/x $rax
-$11 = <optimized out>
+$11 = <not saved>
 (gdb) PASS: gdb.dwarf2/dw2-reg-undefined.exp: print $rax in frame 2
 p/x $rbx
-$12 = <optimized out>
+$12 = <not saved>
 (gdb) PASS: gdb.dwarf2/dw2-reg-undefined.exp: print $rbx in frame 2
 p/x $rcx
-$13 = <optimized out>
+$13 = <not saved>
 (gdb) PASS: gdb.dwarf2/dw2-reg-undefined.exp: print $rcx in frame 2
 p/x $r8
 $14 = 0x323d7b1f40
@@ -135,13 +135,13 @@ p/x $r9
 $15 = 0x323d00f310
 (gdb) PASS: gdb.dwarf2/dw2-reg-undefined.exp: print $r9 in frame 2
 info registers rax rbx rcx r8 r9
-rax            <optimized out>
-rbx            <optimized out>
-rcx            <optimized out>
+rax            <not saved>
+rbx            <not saved>
+rcx            <not saved>
 r8             0x323d7b1f40    215779843904
 r9             0x323d00f310    215771837200
 (gdb) PASS: gdb.dwarf2/dw2-reg-undefined.exp: Check values of rax, rbx, rcx, r8, r9 in frame 2


 
 PASS: gdb.mi/mi-reg-undefined.exp: mi runto stop_frame
@@ -247,35 +247,35 @@ Expecting: ^(330-data-list-register-valu
 (gdb) 
 PASS: gdb.mi/mi-reg-undefined.exp: register values, format r, frame 0
 Expecting: ^(221-data-list-register-values --thread 1 --frame 1 x  0 1 2 8 9[
-]+)?(221\^done,register-values=\[{number="0",value="<optimized out>"},{number="1",value="<optimized out>"},{number="2",value="<optimized out>"},{number="8",value="0x[0-9A-Fa-f]+"},{number="9",value="0x[0-9A-Fa-f]+"}\][
+]+)?(221\^done,register-values=\[{number="0",value="<not saved>"},{number="1",value="<not saved>"},{number="2",value="<not saved>"},{number="8",value="0x[0-9A-Fa-f]+"},{number="9",value="0x[0-9A-Fa-f]+"}\][
 ]+[(]gdb[)] 
 [ ]*)
 221-data-list-register-values --thread 1 --frame 1 x  0 1 2 8 9
-221^done,register-values=[{number="0",value="<optimized out>"},{number="1",value="<optimized out>"},{number="2",value="<optimized out>"},{number="8",value="0x323d7b1f40"},{number="9",value="0x323d00f310"}]
+221^done,register-values=[{number="0",value="<not saved>"},{number="1",value="<not saved>"},{number="2",value="<not saved>"},{number="8",value="0x323d7b1f40"},{number="9",value="0x323d00f310"}]
 (gdb) 
 PASS: gdb.mi/mi-reg-undefined.exp: register values, format x, frame 1
 Expecting: ^(331-data-list-register-values --thread 1 --frame 1 r 0 1 2 8 9[
-]+)?(331\^done,register-values=\[{number="0",value="<optimized out>"},{number="1",value="<optimized out>"},{number="2",value="<optimized out>"},{number="8",value="0x[0-9A-Fa-f]+"},{number="9",value="0x[0-9A-Fa-f]+"}\][
+]+)?(331\^done,register-values=\[{number="0",value="<not saved>"},{number="1",value="<not saved>"},{number="2",value="<not saved>"},{number="8",value="0x[0-9A-Fa-f]+"},{number="9",value="0x[0-9A-Fa-f]+"}\][
 ]+[(]gdb[)] 
 [ ]*)
 331-data-list-register-values --thread 1 --frame 1 r 0 1 2 8 9
-331^done,register-values=[{number="0",value="<optimized out>"},{number="1",value="<optimized out>"},{number="2",value="<optimized out>"},{number="8",value="0x000000323d7b1f40"},{number="9",value="0x000000323d00f310"}]
+331^done,register-values=[{number="0",value="<not saved>"},{number="1",value="<not saved>"},{number="2",value="<not saved>"},{number="8",value="0x000000323d7b1f40"},{number="9",value="0x000000323d00f310"}]
 (gdb) 
 PASS: gdb.mi/mi-reg-undefined.exp: register values, format r, frame 1
 Expecting: ^(222-data-list-register-values --thread 1 --frame 2 x  0 1 2 8 9[
-]+)?(222\^done,register-values=\[{number="0",value="<optimized out>"},{number="1",value="<optimized out>"},{number="2",value="<optimized out>"},{number="8",value="0x[0-9A-Fa-f]+"},{number="9",value="0x[0-9A-Fa-f]+"}\][
+]+)?(222\^done,register-values=\[{number="0",value="<not saved>"},{number="1",value="<not saved>"},{number="2",value="<not saved>"},{number="8",value="0x[0-9A-Fa-f]+"},{number="9",value="0x[0-9A-Fa-f]+"}\][
 ]+[(]gdb[)] 
 [ ]*)
 222-data-list-register-values --thread 1 --frame 2 x  0 1 2 8 9
-222^done,register-values=[{number="0",value="<optimized out>"},{number="1",value="<optimized out>"},{number="2",value="<optimized out>"},{number="8",value="0x323d7b1f40"},{number="9",value="0x323d00f310"}]
+222^done,register-values=[{number="0",value="<not saved>"},{number="1",value="<not saved>"},{number="2",value="<not saved>"},{number="8",value="0x323d7b1f40"},{number="9",value="0x323d00f310"}]
 (gdb) 
 PASS: gdb.mi/mi-reg-undefined.exp: register values, format x, frame 2
 Expecting: ^(332-data-list-register-values --thread 1 --frame 2 r 0 1 2 8 9[
-]+)?(332\^done,register-values=\[{number="0",value="<optimized out>"},{number="1",value="<optimized out>"},{number="2",value="<optimized out>"},{number="8",value="0x[0-9A-Fa-f]+"},{number="9",value="0x[0-9A-Fa-f]+"}\][
+]+)?(332\^done,register-values=\[{number="0",value="<not saved>"},{number="1",value="<not saved>"},{number="2",value="<not saved>"},{number="8",value="0x[0-9A-Fa-f]+"},{number="9",value="0x[0-9A-Fa-f]+"}\][
 ]+[(]gdb[)] 
 [ ]*)
 332-data-list-register-values --thread 1 --frame 2 r 0 1 2 8 9
-332^done,register-values=[{number="0",value="<optimized out>"},{number="1",value="<optimized out>"},{number="2",value="<optimized out>"},{number="8",value="0x000000323d7b1f40"},{number="9",value="0x000000323d00f310"}]
+332^done,register-values=[{number="0",value="<not saved>"},{number="1",value="<not saved>"},{number="2",value="<not saved>"},{number="8",value="0x000000323d7b1f40"},{number="9",value="0x000000323d00f310"}]
 (gdb) 
 PASS: gdb.mi/mi-reg-undefined.exp: register values, format r, frame 2
 testcase ../../../src/gdb/testsuite/gdb.mi/mi-reg-undefined.exp completed in 0 seconds
@@ -338,7 +338,7 @@ Breakpoint 2, 0x000000000040058f in brea
 (gdb) PASS: gdb.dwarf2/dw2-op-out-param.exp: continue to breakpoint: Stop in breakpt for test int_param_single_reg_loc
 bt
 #0  0x000000000040058f in breakpt ()
-#1  0x00000000004005a2 in int_param_single_reg_loc (operand0=<optimized out>, operand1=0xdeadbe00deadbe01, operand2=<optimized out>)
+#1  0x00000000004005a2 in int_param_single_reg_loc (operand0=<not saved>, operand1=0xdeadbe00deadbe01, operand2=<not saved>)
 #2  0x0000000000400577 in main ()
 (gdb) PASS: gdb.dwarf2/dw2-op-out-param.exp: Backtrace for test int_param_single_reg_loc
 continue
@@ -348,7 +348,7 @@ Breakpoint 2, 0x000000000040058f in brea
 (gdb) PASS: gdb.dwarf2/dw2-op-out-param.exp: continue to breakpoint: Stop in breakpt for struct_param_single_reg_loc
 bt
 #0  0x000000000040058f in breakpt ()
-#1  0x00000000004005b2 in struct_param_single_reg_loc (operand0=<optimized out>, operand1=<optimized out>, operand2=<optimized out>)
+#1  0x00000000004005b2 in struct_param_single_reg_loc (operand0=<not saved>, operand1=<not saved>, operand2=<not saved>)
 #2  0x000000000040057d in main ()
 (gdb) KFAIL: gdb.dwarf2/dw2-op-out-param.exp: Backtrace for test struct_param_single_reg_loc (PRMS: symtab/14604)
 continue
@@ -371,7 +371,7 @@ bt
 #1  0x00000000004005d2 in int_param_two_reg_pieces (operand0=0xdeadbe05, operand1=0xdeadbe0100000000, operand2=0x0)
 #2  0x0000000000400589 in main ()
 (gdb) KFAIL: gdb.dwarf2/dw2-op-out-param.exp: Backtrace for test int_param_two_reg_pieces (PRMS: symtab/14605)
~~~~

-------
From 2e7a45e5752bbdd3344b540dd27fafe0faf4e627 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 16 Aug 2013 11:31:13 +0100
Subject: [PATCH] WIP: print "optimized out" register values as "<not saved>"
 instead of "optimized out".

---
 gdb/cp-valprint.c                              |  4 ++--
 gdb/frame-unwind.c                             |  2 +-
 gdb/infcmd.c                                   |  2 +-
 gdb/jv-valprint.c                              |  4 ++--
 gdb/p-valprint.c                               |  4 ++--
 gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp  | 10 +++++-----
 gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp |  4 ++--
 gdb/testsuite/gdb.mi/mi-reg-undefined.exp      |  4 ++--
 gdb/valprint.c                                 | 13 ++++++++-----
 gdb/valprint.h                                 |  3 ++-
 gdb/value.c                                    | 23 ++++++++++++++++++++++-
 gdb/value.h                                    |  4 ++++
 12 files changed, 53 insertions(+), 24 deletions(-)

diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
index e83d979..1d7147c 100644
--- a/gdb/cp-valprint.c
+++ b/gdb/cp-valprint.c
@@ -298,7 +298,7 @@ cp_print_value_fields (struct type *type, struct type *real_type,
 					  TYPE_FIELD_BITPOS (type, i),
 					  TYPE_FIELD_BITSIZE (type, i)))
 		{
-		  val_print_optimized_out (stream);
+		  val_print_optimized_out (val, stream);
 		}
 	      else
 		{
@@ -334,7 +334,7 @@ cp_print_value_fields (struct type *type, struct type *real_type,
 				      _("<error reading variable: %s>"),
 				      ex.message);
 		  else if (v == NULL)
-		    val_print_optimized_out (stream);
+		    val_print_optimized_out (NULL, stream);
 		  else
 		    cp_print_static_field (TYPE_FIELD_TYPE (type, i),
 					   v, stream, recurse + 1,
diff --git a/gdb/frame-unwind.c b/gdb/frame-unwind.c
index ce2f6da..69c977e 100644
--- a/gdb/frame-unwind.c
+++ b/gdb/frame-unwind.c
@@ -164,7 +164,7 @@ frame_unwind_got_optimized (struct frame_info *frame, int regnum)
   struct gdbarch *gdbarch = frame_unwind_arch (frame);
   struct type *reg_type = register_type (gdbarch, regnum);
 
-  return allocate_optimized_out_value (reg_type);
+  return allocate_not_saved_value (reg_type, frame, regnum);
 }
 
 /* Return a value which indicates that FRAME copied REGNUM into
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index f6a5290..76e84c9 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2035,7 +2035,7 @@ default_print_one_register_info (struct ui_file *file,
     }
   else if (value_optimized_out (val))
     {
-      val_print_optimized_out (file);
+      val_print_optimized_out (val, file);
       fprintf_filtered (file, "\n");
       return;
     }
diff --git a/gdb/jv-valprint.c b/gdb/jv-valprint.c
index 3b90e54..961a8e8 100644
--- a/gdb/jv-valprint.c
+++ b/gdb/jv-valprint.c
@@ -395,7 +395,7 @@ java_print_value_fields (struct type *type, const gdb_byte *valaddr,
 	      else if (!value_bits_valid (val, TYPE_FIELD_BITPOS (type, i),
 					  TYPE_FIELD_BITSIZE (type, i)))
 		{
-		  val_print_optimized_out (stream);
+		  val_print_optimized_out (val, stream);
 		}
 	      else
 		{
@@ -420,7 +420,7 @@ java_print_value_fields (struct type *type, const gdb_byte *valaddr,
 		  struct value *v = value_static_field (type, i);
 
 		  if (v == NULL)
-		    val_print_optimized_out (stream);
+		    val_print_optimized_out (v, stream);
 		  else
 		    {
 		      struct value_print_options opts;
diff --git a/gdb/p-valprint.c b/gdb/p-valprint.c
index 05d4c6f..6e8a408 100644
--- a/gdb/p-valprint.c
+++ b/gdb/p-valprint.c
@@ -629,7 +629,7 @@ pascal_object_print_value_fields (struct type *type, const gdb_byte *valaddr,
 	      else if (!value_bits_valid (val, TYPE_FIELD_BITPOS (type, i),
 					  TYPE_FIELD_BITSIZE (type, i)))
 		{
-		  val_print_optimized_out (stream);
+		  val_print_optimized_out (val, stream);
 		}
 	      else
 		{
@@ -657,7 +657,7 @@ pascal_object_print_value_fields (struct type *type, const gdb_byte *valaddr,
 		  v = value_field_bitfield (type, i, valaddr, offset, val);
 
 		  if (v == NULL)
-		    val_print_optimized_out (stream);
+		    val_print_optimized_out (v, stream);
 		  else
 		    pascal_object_print_static_field (v, stream, recurse + 1,
 						      options);
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
index 5e4ca01..fecb0df 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
@@ -46,16 +46,16 @@ gdb_test_no_output "set print frame-arguments all"
 
 # (1) int_param_single_reg_loc
 gdb_continue_to_breakpoint "Stop in breakpt for test int_param_single_reg_loc"
-gdb_test "bt" "#0  ($hex in )?breakpt \\(\\)\r\n#1  ($hex in )?int_param_single_reg_loc \\(operand0=<optimized out>, operand1=0xdeadbe00deadbe01, operand2=<optimized out>\\)\r\n#2  ($hex in )?main \\(\\)" "Backtrace for test int_param_single_reg_loc"
+gdb_test "bt" "#0  ($hex in )?breakpt \\(\\)\r\n#1  ($hex in )?int_param_single_reg_loc \\(operand0=<not saved>, operand1=0xdeadbe00deadbe01, operand2=<not saved>\\)\r\n#2  ($hex in )?main \\(\\)" "Backtrace for test int_param_single_reg_loc"
 
 # (2) struct_param_single_reg_loc
 gdb_continue_to_breakpoint "Stop in breakpt for struct_param_single_reg_loc"
 set test "Backtrace for test struct_param_single_reg_loc"
 gdb_test_multiple "bt" "$test" {
-    -re "#0  ($hex in )?breakpt \\(\\)\r\n#1  ($hex in )?struct_param_single_reg_loc \\(operand0={a = 0xdeadbe00deadbe01, b = <optimized out>}, operand1={a = <optimized out>, b = 0xdeadbe04deadbe05}, operand2=<optimized out>\\)\r\n#2  ($hex in )?main \\(\\)\r\n$gdb_prompt $" {
+    -re "#0  ($hex in )?breakpt \\(\\)\r\n#1  ($hex in )?struct_param_single_reg_loc \\(operand0={a = 0xdeadbe00deadbe01, b = <not saved>}, operand1={a = <not saved>, b = 0xdeadbe04deadbe05}, operand2=<not saved>\\)\r\n#2  ($hex in )?main \\(\\)\r\n$gdb_prompt $" {
 	xpass $test
     }
-    -re "#0  ($hex in )?breakpt \\(\\)\r\n#1  ($hex in )?struct_param_single_reg_loc \\(operand0=<optimized out>, operand1=<optimized out>, operand2=<optimized out>\\)\r\n#2  ($hex in )?main \\(\\)\r\n$gdb_prompt $" {
+    -re "#0  ($hex in )?breakpt \\(\\)\r\n#1  ($hex in )?struct_param_single_reg_loc \\(operand0=<not saved>, operand1=<not saved>, operand2=<not saved>\\)\r\n#2  ($hex in )?main \\(\\)\r\n$gdb_prompt $" {
 	kfail "symtab/14604" $test
     }
 }
@@ -64,7 +64,7 @@ gdb_test_multiple "bt" "$test" {
 gdb_continue_to_breakpoint "Stop in breakpt for struct_param_two_reg_pieces"
 set test "Backtrace for test struct_param_two_reg_pieces"
 gdb_test_multiple "bt" "$test" {
-    -re "#0  ($hex in )?breakpt \\(\\)\r\n#1  ($hex in )?struct_param_two_reg_pieces \\(operand0={a = 0xdeadbe04deadbe05, b = <optimized out>}, operand1={a = <optimized out>, b = 0xdeadbe00deadbe01}, operand2=<optimized out>\\)\r\n#2  ($hex in )?main \\(\\)\r\n$gdb_prompt $" {
+    -re "#0  ($hex in )?breakpt \\(\\)\r\n#1  ($hex in )?struct_param_two_reg_pieces \\(operand0={a = 0xdeadbe04deadbe05, b = <not saved>}, operand1={a = <not saved>, b = 0xdeadbe00deadbe01}, operand2=<not saved>\\)\r\n#2  ($hex in )?main \\(\\)\r\n$gdb_prompt $" {
 	xpass $test
     }
     -re "#0  ($hex in )?breakpt \\(\\)\r\n#1  ($hex in )?struct_param_two_reg_pieces \\(operand0=.*, operand1=.*, operand2=.*\\)\r\n#2  ($hex in )?main \\(\\)\r\n$gdb_prompt $" {
@@ -76,7 +76,7 @@ gdb_test_multiple "bt" "$test" {
 gdb_continue_to_breakpoint "Stop in breakpt for int_param_two_reg_pieces"
 set test "Backtrace for test int_param_two_reg_pieces"
 gdb_test_multiple "bt" "$test" {
-    -re "#0  ($hex in )?breakpt \\(\\)\r\n#1  ($hex in )?int_param_two_reg_pieces \\(operand0=<optimized out>, operand1=<optimized out>, operand2=<optimized out>\\)\r\n#2  ($hex in )?main \\(\\)\r\n$gdb_prompt $" {
+    -re "#0  ($hex in )?breakpt \\(\\)\r\n#1  ($hex in )?int_param_two_reg_pieces \\(operand0=<not saved>, operand1=<not saved>, operand2=<not saved>\\)\r\n#2  ($hex in )?main \\(\\)\r\n$gdb_prompt $" {
 	xpass $test
     }
     -re "#0  ($hex in )?breakpt \\(\\)\r\n#1  ($hex in )?int_param_two_reg_pieces \\(operand0=.*, operand1=.*, operand2=.*\\)\r\n#2  ($hex in )?main \\(\\)\r\n$gdb_prompt $" {
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
index 4686648..44bf8e9 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
@@ -45,8 +45,8 @@ for {set f 0} {$f < 3} {incr f} {
 	set pattern_r8_r9_print "$hex"
 	set pattern_r8_r9_info "$hex\\s+$decimal"
     } else {
-	set pattern_rax_rbx_rcx_print "<optimized out>"
-	set pattern_rax_rbx_rcx_info "<optimized out>"
+	set pattern_rax_rbx_rcx_print "<not saved>"
+	set pattern_rax_rbx_rcx_info "<not saved>"
 	set pattern_r8_r9_print "$hex"
 	set pattern_r8_r9_info "$hex\\s+$decimal"
     }
diff --git a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
index 8bcbb21..cb3a716 100644
--- a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
+++ b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
@@ -52,13 +52,13 @@ mi_gdb_test "111-stack-list-frames" \
     "111\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"stop_frame\",.*\},frame=\{level=\"1\",addr=\"$hex\",func=\"first_frame\",.*\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",.*\}\\\]" \
     "stack frame listing"
 
-set opt_out_pattern "<optimized out>"
+set not_saved_pattern "<not saved>"
 
 for {set f 0} {$f < 3} {incr f} {
     if {${f} == 0} {
 	set pattern_0_1_2 ${hex}
     } else {
-	set pattern_0_1_2 ${opt_out_pattern}
+	set pattern_0_1_2 ${not_saved_pattern}
     }
 
     mi_gdb_test "22${f}-data-list-register-values --thread 1 --frame ${f} x  0 1 2 8 9" \
diff --git a/gdb/valprint.c b/gdb/valprint.c
index 753ae34..285a7fe 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -314,7 +314,7 @@ valprint_check_validity (struct ui_file *stream,
       if (!value_bits_valid (val, TARGET_CHAR_BIT * embedded_offset,
 			     TARGET_CHAR_BIT * TYPE_LENGTH (type)))
 	{
-	  val_print_optimized_out (stream);
+	  val_print_optimized_out (val, stream);
 	  return 0;
 	}
 
@@ -336,9 +336,12 @@ valprint_check_validity (struct ui_file *stream,
 }
 
 void
-val_print_optimized_out (struct ui_file *stream)
+val_print_optimized_out (const struct value *val, struct ui_file *stream)
 {
-  fprintf_filtered (stream, _("<optimized out>"));
+  if (val != NULL && value_lval_const (val) == lval_register)
+    fprintf_filtered (stream, _("<not saved>"));
+  else
+    fprintf_filtered (stream, _("<optimized out>"));
 }
 
 void
@@ -805,7 +808,7 @@ value_check_printable (struct value *val, struct ui_file *stream,
       if (options->summary && !scalar_type_p (value_type (val)))
 	fprintf_filtered (stream, "...");
       else
-	val_print_optimized_out (stream);
+	val_print_optimized_out (val, stream);
       return 0;
     }
 
@@ -966,7 +969,7 @@ val_print_scalar_formatted (struct type *type,
      printed, because all bits contribute to its representation.  */
   if (!value_bits_valid (val, TARGET_CHAR_BIT * embedded_offset,
 			      TARGET_CHAR_BIT * TYPE_LENGTH (type)))
-    val_print_optimized_out (stream);
+    val_print_optimized_out (val, stream);
   else if (!value_bytes_available (val, embedded_offset, TYPE_LENGTH (type)))
     val_print_unavailable (stream);
   else
diff --git a/gdb/valprint.h b/gdb/valprint.h
index 2959098..40e2ebb 100644
--- a/gdb/valprint.h
+++ b/gdb/valprint.h
@@ -160,7 +160,8 @@ extern int read_string (CORE_ADDR addr, int len, int width,
 			enum bfd_endian byte_order, gdb_byte **buffer,
 			int *bytes_read);
 
-extern void val_print_optimized_out (struct ui_file *stream);
+extern void val_print_optimized_out (const struct value *val,
+				     struct ui_file *stream);
 
 extern void val_print_unavailable (struct ui_file *stream);
 
diff --git a/gdb/value.c b/gdb/value.c
index 09ab1bb..6032267 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -747,6 +747,22 @@ allocate_optimized_out_value (struct type *type)
   return retval;
 }
 
+/* Allocate a "not saved" value for type TYPE.  */
+
+struct value *
+allocate_not_saved_value (struct type *type,
+			  struct frame_info *frame, int regnum)
+{
+  struct value *retval = allocate_value (type);
+
+  set_value_optimized_out (retval, 1);
+  VALUE_LVAL (retval) = lval_register;
+  VALUE_FRAME_ID (retval) = get_frame_id (frame);
+  VALUE_REGNUM (retval) = regnum;
+
+  return retval;
+}
+
 /* Accessor methods.  */
 
 struct value *
@@ -886,7 +902,12 @@ static void
 require_not_optimized_out (const struct value *value)
 {
   if (value->optimized_out)
-    error (_("value has been optimized out"));
+    {
+      if (value->lval == lval_register)
+	error (_("register has not been saved in frame"));
+      else
+	error (_("value has been optimized out"));
+    }
 }
 
 static void
diff --git a/gdb/value.h b/gdb/value.h
index bef193c..d4463bf 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -243,6 +243,10 @@ extern int valprint_check_validity (struct ui_file *stream, struct type *type,
 
 extern struct value *allocate_optimized_out_value (struct type *type);
 
+extern struct value *allocate_not_saved_value (struct type *type,
+					       struct frame_info *frame,
+					       int regnum);
+
 /* If VALUE is lval_computed, return its lval_funcs structure.  */
 
 extern const struct lval_funcs *value_computed_funcs (const struct value *);
-- 
1.7.11.7

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

[Pedro Alves]

On 08/16/2013 07:41 PM, Pedro Alves wrote:

> +/* Allocate a "not saved" value for type TYPE.  */
> +
> +struct value *
> +allocate_not_saved_value (struct type *type,
> +			  struct frame_info *frame, int regnum)
> +{
> +  struct value *retval = allocate_value (type);
> +
> +  set_value_optimized_out (retval, 1);
> +  VALUE_LVAL (retval) = lval_register;
> +  VALUE_FRAME_ID (retval) = get_frame_id (frame);

I noticed that this should be the previous frame's id, not FRAME's.
I've fixed it in the branch, and did some additional cleaning up.

-- 
Pedro Alves

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

[Andrew Burgess]

On 16/08/2013 7:41 PM, Pedro Alves wrote:
> On 08/12/2013 09: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".  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.
>>
>> 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.
> 
> This is a tough call.  I do agree that "optimized out" for registers
> is a bit confusing.  However, we already do print "<optimized out>" in
> other places, such as when printing expressions, and consistency
> is good.  If we did add a distinction, I agree with Andrew that it should
> be done in a more systematic way.  However, I'm not really sure we need
> much machinery.  Wouldn't something like:
> 
> void
> val_print_optimized_out (const struct value *val, struct ui_file *stream)
> {
>   if (value_lval_const (val) == lval_register)
>     fprintf_filtered (stream, _("<not saved>"));
>   else
>     fprintf_filtered (stream, _("<optimized out>"));
> }
> 
> work?  What could be the register value cases that would print
> "not saved" that we'd still want to print "optimized out" ?

The only case I can immediately think of where this would cause a
problem would be for computed locations, (lval_computed).  The easy
answer would be (in that case) the blame the compiler - why say the
location is in a register if that register is volatile - but sadly I see
this way too often.

However, exchanging what I see as the current larger inconsistency, for
this much smaller one seems like a good deal to me, especially if it
gets this patch unblocked...

Thanks,
Andrew

[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]

Hi guys,

Getting back to this, trying to make progress.

On 08/19/2013 11:24 AM, Andrew Burgess wrote:
> On 16/08/2013 7:41 PM, Pedro Alves wrote:
>> On 08/12/2013 09: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".  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.
>>>
>>> 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.
>>
>> This is a tough call.  I do agree that "optimized out" for registers
>> is a bit confusing.  However, we already do print "<optimized out>" in
>> other places, such as when printing expressions, and consistency
>> is good.  If we did add a distinction, I agree with Andrew that it should
>> be done in a more systematic way.  However, I'm not really sure we need
>> much machinery.  Wouldn't something like:
>>
>> void
>> val_print_optimized_out (const struct value *val, struct ui_file *stream)
>> {
>>   if (value_lval_const (val) == lval_register)
>>     fprintf_filtered (stream, _("<not saved>"));
>>   else
>>     fprintf_filtered (stream, _("<optimized out>"));
>> }
>>
>> work?  What could be the register value cases that would print
>> "not saved" that we'd still want to print "optimized out" ?
> 
> The only case I can immediately think of where this would cause a
> problem would be for computed locations, (lval_computed).  The easy
> answer would be (in that case) the blame the compiler - why say the
> location is in a register if that register is volatile - but sadly I see
> this way too often.

Hmm, OK, but then lval_computed values with that change won't
ever show "<not saved>", due to the lval_register check.  IOW,
we'd have to do something else in addition to lval_computed values
to make them print something other than the current <optimized out>.

However, I've come to think there's a really simple rule to
follow here -- We should only ever print <not saved> for values
that represent machine/pseudo registers.  IOW, $pc, $rax, etc.
If the debug info happens to describe a variable as being located
in some optimized out register, we should still print
<optimized out>.  The previous version of the patch failed that:

 (gdb) PASS: gdb.dwarf2/dw2-op-out-param.exp: continue to breakpoint: Stop in breakpt for test int_param_single_reg_loc
  bt
  #0  0x000000000040058f in breakpt ()
 -#1  0x00000000004005a2 in int_param_single_reg_loc (operand0=<optimized out>, operand1=0xdeadbe00deadbe01, operand2=<optimized out>)
 +#1  0x00000000004005a2 in int_param_single_reg_loc (operand0=<not saved>, operand1=0xdeadbe00deadbe01, operand2=<not saved>)
  #2  0x0000000000400577 in main ()

It didn't really make a lot of sense.  This new version doesn't have
that change anymore.

That simple rule suggests that whatever the internal representation,
we should be easily able to have a single central point where to tag
such values.  In fact, I think that already exists in value_of_register.

> However, exchanging what I see as the current larger inconsistency, for
> this much smaller one seems like a good deal to me, especially if it
> gets this patch unblocked...

Alright, what do you (all) think of of this (supposedly finished) patch
on top of yours (Andrew's) then?

Force-pushed here (along with Andrew's patch) for convenience:
 https://github.com/palves/gdb/commits/register_not_saved
 git@github.com:palves/gdb.git register_not_saved

--------------
Subject: [PATCH] Print registers not saved in the frame as "<not saved>"
 instead of "<optimized out>".

Currently we print not saved registers as <optimized out>.  That's
confusing, because what this really means is that the register was not
saved in the frame before calling another function.

Before:

 (gdb) p/x $rax $1 = <optimized out>
 (gdb) info registers rax
 rax            <optimized out>

After:

 (gdb) p/x $rax
 $1 = <not saved>
 (gdb) info registers rax
 rax            <not saved>

However, if for some reason the debug info describes a variable as being
in such a register, we still want to print <optimized out>.  IOW, <not
saved> is reserved for inspecting registers at the machine level.
The patch uses lval_register+optimized_out to encode the latter.

frame_unwind_got_optimized creates not_lval optimized out registers,
so by default, in most cases, we'll see <optimized out>.

value_of_register is the function eval.c uses for evaluating
OP_REGISTER (again, $pc, etc.), and related bits.  It isn't used for
anything else.  This function makes sure to return lval_register
values.  The patch makes "info registers" and the MI equivalent use it
too.  I think it just makes a lot of sense, as this makes it so that
when printing machine registers ($pc, etc.), we go through a central
function.

We're likely to need a different encoding at some point, if/when we
support partially saved registers.  Even then, I think
value_of_register will still be the spot to tag the intention to print
machine register values differently.

value_from_register however may also return optimized out
lval_register values, so at a couple places where we're computing a
variable's location from a dwarf expression, we convert the resulting
value away from lval_register to a regular optimized out value.

Tested on x86_64 Fedora 17

gdb/
2013-09-05  Pedro Alves  <palves@redhat.com>

	* cp-valprint.c (cp_print_value_fields): Adjust calls to
	val_print_optimized_out.
	* jv-valprint.c (java_print_value_fields): Likewise.
	* p-valprint.c (pascal_object_print_value_fields): Likewise.
	* dwarf2loc.c (dwarf2_evaluate_loc_desc_full) <xxxxxx>: If the register
	was not saved, return a new optimized out value.
	* findvar.c (address_from_register): Likewise.
	* frame.c (put_frame_register): Tweak error string to say the
	register was not saved, rather than optimized out.
	* infcmd.c (default_print_one_register_info): Adjust call to
	val_print_optimized_out.  Use value_of_register instead of
	get_frame_register_value.
	* mi/mi-main.c (output_register): Use value_of_register instead of
	get_frame_register_value.
	* valprint.c (valprint_check_validity): Likewise.
	(val_print_optimized_out): New value parameter.  If the value is
	lval_register, print <not saved> instead.
	(value_check_printable, val_print_scalar_formatted): Adjust calls
	to val_print_optimized_out.
	* valprint.h (val_print_optimized_out): New value parameter.
	* value.c (struct value) <optimized_out>: Extend comment.
	(error_value_optimized_out): New function.
	(require_not_optimized_out): Use it.  Use a different string for
	lval_register values.
	* value.h (error_value_optimized_out): New declaration.

gdb/testsuite/
2013-09-05  Pedro Alves  <palves@redhat.com>

	* gdb.mi/mi-reg-undefined.exp (opt_out_pattern): Delete.
	(not_saved_pattern): New.
	Replace uses for the former with the latter.
---
 gdb/cp-valprint.c                              |  4 ++--
 gdb/dwarf2loc.c                                | 16 +++++++++++++---
 gdb/findvar.c                                  |  9 +++++++++
 gdb/frame.c                                    |  2 +-
 gdb/infcmd.c                                   |  4 ++--
 gdb/jv-valprint.c                              |  4 ++--
 gdb/mi/mi-main.c                               |  2 +-
 gdb/p-valprint.c                               |  4 ++--
 gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp |  4 ++--
 gdb/testsuite/gdb.mi/mi-reg-undefined.exp      |  4 ++--
 gdb/valprint.c                                 | 13 ++++++++-----
 gdb/valprint.h                                 |  3 ++-
 gdb/value.c                                    | 22 +++++++++++++++++++---
 gdb/value.h                                    |  5 +++++
 14 files changed, 70 insertions(+), 26 deletions(-)

diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
index e83d979..1d7147c 100644
--- a/gdb/cp-valprint.c
+++ b/gdb/cp-valprint.c
@@ -298,7 +298,7 @@ cp_print_value_fields (struct type *type, struct type *real_type,
 					  TYPE_FIELD_BITPOS (type, i),
 					  TYPE_FIELD_BITSIZE (type, i)))
 		{
-		  val_print_optimized_out (stream);
+		  val_print_optimized_out (val, stream);
 		}
 	      else
 		{
@@ -334,7 +334,7 @@ cp_print_value_fields (struct type *type, struct type *real_type,
 				      _("<error reading variable: %s>"),
 				      ex.message);
 		  else if (v == NULL)
-		    val_print_optimized_out (stream);
+		    val_print_optimized_out (NULL, stream);
 		  else
 		    cp_print_static_field (TYPE_FIELD_TYPE (type, i),
 					   v, stream, recurse + 1,
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 55d43f1..1313e17 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -2290,11 +2290,21 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
 	    if (byte_offset != 0)
 	      error (_("cannot use offset on synthetic pointer to register"));
 	    do_cleanups (value_chain);
-	    if (gdb_regnum != -1)
-	      retval = value_from_register (type, gdb_regnum, frame);
-	    else
+	   if (gdb_regnum == -1)
 	      error (_("Unable to access DWARF register number %d"),
 		     dwarf_regnum);
+	   retval = value_from_register (type, gdb_regnum, frame);
+	   if (value_optimized_out (retval))
+	     {
+	       /* This means the register has undefined value / was
+		  not saved.  As we're computing the location of some
+		  variable etc. in the program, not a value for
+		  inspecting a register ($pc, $sp, etc.), return a
+		  generic optimized out value instead, so that we show
+		  <optimized out> instead of <not saved>.  */
+	       do_cleanups (value_chain);
+	       retval = allocate_optimized_out_value (type);
+	     }
 	  }
 	  break;
 
diff --git a/gdb/findvar.c b/gdb/findvar.c
index 25242be..c3550b4 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -757,6 +757,15 @@ address_from_register (struct type *type, int regnum, struct frame_info *frame)
   value = value_from_register (type, regnum, frame);
   gdb_assert (value);
 
+  if (value_optimized_out (value))
+    {
+      /* This function is used while computing a location expression.
+	 Complain about the value being optimized out, rather than
+	 letting value_as_address complain about some random register
+	 the expression depends on not being saved.  */
+      error_value_optimized_out ();
+    }
+
   result = value_as_address (value);
   release_value (value);
   value_free (value);
diff --git a/gdb/frame.c b/gdb/frame.c
index d52c26a..be1a1b1 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1140,7 +1140,7 @@ put_frame_register (struct frame_info *frame, int regnum,
   frame_register (frame, regnum, &optim, &unavail,
 		  &lval, &addr, &realnum, NULL);
   if (optim)
-    error (_("Attempt to assign to a value that was optimized out."));
+    error (_("Attempt to assign to a register that was not saved."));
   switch (lval)
     {
     case lval_memory:
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index f6a5290..2bba49e 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2035,7 +2035,7 @@ default_print_one_register_info (struct ui_file *file,
     }
   else if (value_optimized_out (val))
     {
-      val_print_optimized_out (file);
+      val_print_optimized_out (val, file);
       fprintf_filtered (file, "\n");
       return;
     }
@@ -2142,7 +2142,7 @@ default_print_registers_info (struct gdbarch *gdbarch,
 
       default_print_one_register_info (file,
 				       gdbarch_register_name (gdbarch, i),
-				       get_frame_register_value (frame, i));
+				       value_of_register (i, frame));
     }
 }
 
diff --git a/gdb/jv-valprint.c b/gdb/jv-valprint.c
index 3b90e54..cb89a85 100644
--- a/gdb/jv-valprint.c
+++ b/gdb/jv-valprint.c
@@ -395,7 +395,7 @@ java_print_value_fields (struct type *type, const gdb_byte *valaddr,
 	      else if (!value_bits_valid (val, TYPE_FIELD_BITPOS (type, i),
 					  TYPE_FIELD_BITSIZE (type, i)))
 		{
-		  val_print_optimized_out (stream);
+		  val_print_optimized_out (val, stream);
 		}
 	      else
 		{
@@ -420,7 +420,7 @@ java_print_value_fields (struct type *type, const gdb_byte *valaddr,
 		  struct value *v = value_static_field (type, i);
 
 		  if (v == NULL)
-		    val_print_optimized_out (stream);
+		    val_print_optimized_out (NULL, stream);
 		  else
 		    {
 		      struct value_print_options opts;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index e6e98b6..503b7bf 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1161,7 +1161,7 @@ output_register (struct frame_info *frame, int regnum, int format,
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
   struct ui_out *uiout = current_uiout;
-  struct value *val = get_frame_register_value (frame, regnum);
+  struct value *val = value_of_register (regnum, frame);
   struct cleanup *tuple_cleanup;
   struct value_print_options opts;
   struct ui_file *stb;
diff --git a/gdb/p-valprint.c b/gdb/p-valprint.c
index 05d4c6f..e6d4b91 100644
--- a/gdb/p-valprint.c
+++ b/gdb/p-valprint.c
@@ -629,7 +629,7 @@ pascal_object_print_value_fields (struct type *type, const gdb_byte *valaddr,
 	      else if (!value_bits_valid (val, TYPE_FIELD_BITPOS (type, i),
 					  TYPE_FIELD_BITSIZE (type, i)))
 		{
-		  val_print_optimized_out (stream);
+		  val_print_optimized_out (val, stream);
 		}
 	      else
 		{
@@ -657,7 +657,7 @@ pascal_object_print_value_fields (struct type *type, const gdb_byte *valaddr,
 		  v = value_field_bitfield (type, i, valaddr, offset, val);
 
 		  if (v == NULL)
-		    val_print_optimized_out (stream);
+		    val_print_optimized_out (NULL, stream);
 		  else
 		    pascal_object_print_static_field (v, stream, recurse + 1,
 						      options);
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
index 4686648..44bf8e9 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
@@ -45,8 +45,8 @@ for {set f 0} {$f < 3} {incr f} {
 	set pattern_r8_r9_print "$hex"
 	set pattern_r8_r9_info "$hex\\s+$decimal"
     } else {
-	set pattern_rax_rbx_rcx_print "<optimized out>"
-	set pattern_rax_rbx_rcx_info "<optimized out>"
+	set pattern_rax_rbx_rcx_print "<not saved>"
+	set pattern_rax_rbx_rcx_info "<not saved>"
 	set pattern_r8_r9_print "$hex"
 	set pattern_r8_r9_info "$hex\\s+$decimal"
     }
diff --git a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
index 8bcbb21..cb3a716 100644
--- a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
+++ b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
@@ -52,13 +52,13 @@ mi_gdb_test "111-stack-list-frames" \
     "111\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"stop_frame\",.*\},frame=\{level=\"1\",addr=\"$hex\",func=\"first_frame\",.*\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",.*\}\\\]" \
     "stack frame listing"
 
-set opt_out_pattern "<optimized out>"
+set not_saved_pattern "<not saved>"
 
 for {set f 0} {$f < 3} {incr f} {
     if {${f} == 0} {
 	set pattern_0_1_2 ${hex}
     } else {
-	set pattern_0_1_2 ${opt_out_pattern}
+	set pattern_0_1_2 ${not_saved_pattern}
     }
 
     mi_gdb_test "22${f}-data-list-register-values --thread 1 --frame ${f} x  0 1 2 8 9" \
diff --git a/gdb/valprint.c b/gdb/valprint.c
index 0f6d65e..61492cc 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -314,7 +314,7 @@ valprint_check_validity (struct ui_file *stream,
       if (!value_bits_valid (val, TARGET_CHAR_BIT * embedded_offset,
 			     TARGET_CHAR_BIT * TYPE_LENGTH (type)))
 	{
-	  val_print_optimized_out (stream);
+	  val_print_optimized_out (val, stream);
 	  return 0;
 	}
 
@@ -336,9 +336,12 @@ valprint_check_validity (struct ui_file *stream,
 }
 
 void
-val_print_optimized_out (struct ui_file *stream)
+val_print_optimized_out (const struct value *val, struct ui_file *stream)
 {
-  fprintf_filtered (stream, _("<optimized out>"));
+  if (val != NULL && value_lval_const (val) == lval_register)
+    fprintf_filtered (stream, _("<not saved>"));
+  else
+    fprintf_filtered (stream, _("<optimized out>"));
 }
 
 void
@@ -805,7 +808,7 @@ value_check_printable (struct value *val, struct ui_file *stream,
       if (options->summary && !val_print_scalar_type_p (value_type (val)))
 	fprintf_filtered (stream, "...");
       else
-	val_print_optimized_out (stream);
+	val_print_optimized_out (val, stream);
       return 0;
     }
 
@@ -966,7 +969,7 @@ val_print_scalar_formatted (struct type *type,
      printed, because all bits contribute to its representation.  */
   if (!value_bits_valid (val, TARGET_CHAR_BIT * embedded_offset,
 			      TARGET_CHAR_BIT * TYPE_LENGTH (type)))
-    val_print_optimized_out (stream);
+    val_print_optimized_out (val, stream);
   else if (!value_bytes_available (val, embedded_offset, TYPE_LENGTH (type)))
     val_print_unavailable (stream);
   else
diff --git a/gdb/valprint.h b/gdb/valprint.h
index e7073b6..1d86ed7 100644
--- a/gdb/valprint.h
+++ b/gdb/valprint.h
@@ -160,7 +160,8 @@ extern int read_string (CORE_ADDR addr, int len, int width,
 			enum bfd_endian byte_order, gdb_byte **buffer,
 			int *bytes_read);
 
-extern void val_print_optimized_out (struct ui_file *stream);
+extern void val_print_optimized_out (const struct value *val,
+				     struct ui_file *stream);
 
 extern void val_print_unavailable (struct ui_file *stream);
 
diff --git a/gdb/value.c b/gdb/value.c
index 42a8d2f..871b57c 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -197,8 +197,13 @@ struct value
      reset, be sure to consider this use as well!  */
   unsigned int lazy : 1;
 
-  /* If nonzero, this is the value of a variable which does not
-     actually exist in the program.  */
+  /* If nonzero, this is the value of a variable that does not
+     actually exist in the program.  If nonzero, and LVAL is
+     lval_register, this is a register ($pc, $sp, etc., never a
+     program variable) that has not been saved in the frame.  All
+     optimized-out values are treated pretty much the same, except
+     registers have a different string representation and related
+     error strings.  */
   unsigned int optimized_out : 1;
 
   /* If value is a variable, is it initialized or not.  */
@@ -902,11 +907,22 @@ value_actual_type (struct value *value, int resolve_simple_types,
   return result;
 }
 
+void
+error_value_optimized_out (void)
+{
+  error (_("value has been optimized out"));
+}
+
 static void
 require_not_optimized_out (const struct value *value)
 {
   if (value->optimized_out)
-    error (_("value has been optimized out"));
+    {
+      if (value->lval == lval_register)
+	error (_("register has not been saved in frame"));
+      else
+	error_value_optimized_out ();
+    }
 }
 
 static void
diff --git a/gdb/value.h b/gdb/value.h
index 98dbadf..db964e3 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -273,6 +273,11 @@ extern void set_value_lazy (struct value *value, int val);
 extern int value_stack (struct value *);
 extern void set_value_stack (struct value *value, int val);
 
+/* Throw an error complaining that the value has been optimized
+   out.  */
+
+extern void error_value_optimized_out (void);
+
 /* value_contents() and value_contents_raw() both return the address
    of the gdb buffer used to hold a copy of the contents of the lval.
    value_contents() is used when the contents of the buffer are needed
-- 
1.7.11.7

Re: [PATCH] Print registers not saved in the frame as "<not saved>", instead of "<optimized out>".

[Andrew Burgess]

On 05/09/2013 5:29 PM, Pedro Alves wrote:
> Hi guys,
> 
> Getting back to this, trying to make progress.
> 
> On 08/19/2013 11:24 AM, Andrew Burgess wrote:
>> On 16/08/2013 7:41 PM, Pedro Alves wrote:
>>> On 08/12/2013 09: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".  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.
>>>>
>>>> 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.
>>>
>>> This is a tough call.  I do agree that "optimized out" for registers
>>> is a bit confusing.  However, we already do print "<optimized out>" in
>>> other places, such as when printing expressions, and consistency
>>> is good.  If we did add a distinction, I agree with Andrew that it should
>>> be done in a more systematic way.  However, I'm not really sure we need
>>> much machinery.  Wouldn't something like:
>>>
>>> void
>>> val_print_optimized_out (const struct value *val, struct ui_file *stream)
>>> {
>>>   if (value_lval_const (val) == lval_register)
>>>     fprintf_filtered (stream, _("<not saved>"));
>>>   else
>>>     fprintf_filtered (stream, _("<optimized out>"));
>>> }
>>>
>>> work?  What could be the register value cases that would print
>>> "not saved" that we'd still want to print "optimized out" ?
>>
>> The only case I can immediately think of where this would cause a
>> problem would be for computed locations, (lval_computed).  The easy
>> answer would be (in that case) the blame the compiler - why say the
>> location is in a register if that register is volatile - but sadly I see
>> this way too often.
> 
> Hmm, OK, but then lval_computed values with that change won't
> ever show "<not saved>", due to the lval_register check.  IOW,
> we'd have to do something else in addition to lval_computed values
> to make them print something other than the current <optimized out>.
> 
> However, I've come to think there's a really simple rule to
> follow here -- We should only ever print <not saved> for values
> that represent machine/pseudo registers.  IOW, $pc, $rax, etc.
> If the debug info happens to describe a variable as being located
> in some optimized out register, we should still print
> <optimized out>.  The previous version of the patch failed that:
> 
>  (gdb) PASS: gdb.dwarf2/dw2-op-out-param.exp: continue to breakpoint: Stop in breakpt for test int_param_single_reg_loc
>   bt
>   #0  0x000000000040058f in breakpt ()
>  -#1  0x00000000004005a2 in int_param_single_reg_loc (operand0=<optimized out>, operand1=0xdeadbe00deadbe01, operand2=<optimized out>)
>  +#1  0x00000000004005a2 in int_param_single_reg_loc (operand0=<not saved>, operand1=0xdeadbe00deadbe01, operand2=<not saved>)
>   #2  0x0000000000400577 in main ()
> 
> It didn't really make a lot of sense.  This new version doesn't have
> that change anymore.
> 
> That simple rule suggests that whatever the internal representation,
> we should be easily able to have a single central point where to tag
> such values.  In fact, I think that already exists in value_of_register.
> 
>> However, exchanging what I see as the current larger inconsistency, for
>> this much smaller one seems like a good deal to me, especially if it
>> gets this patch unblocked...
> 
> Alright, what do you (all) think of of this (supposedly finished) patch
> on top of yours (Andrew's) then?


Looks good to me.  Thanks for this.

Andrew

Re: [PATCH] Print registers not saved in the frame as "<not saved>", instead of "<optimized out>".

[Pedro Alves]

On 09/05/2013 05:35 PM, Andrew Burgess wrote:
> On 05/09/2013 5:29 PM, Pedro Alves wrote:
>> Hi guys,
>>
>> Getting back to this, trying to make progress.
>>
>> On 08/19/2013 11:24 AM, Andrew Burgess wrote:
>>> On 16/08/2013 7:41 PM, Pedro Alves wrote:
>>>> On 08/12/2013 09: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".  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.
>>>>>
>>>>> 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.
>>>>
>>>> This is a tough call.  I do agree that "optimized out" for registers
>>>> is a bit confusing.  However, we already do print "<optimized out>" in
>>>> other places, such as when printing expressions, and consistency
>>>> is good.  If we did add a distinction, I agree with Andrew that it should
>>>> be done in a more systematic way.  However, I'm not really sure we need
>>>> much machinery.  Wouldn't something like:
>>>>
>>>> void
>>>> val_print_optimized_out (const struct value *val, struct ui_file *stream)
>>>> {
>>>>   if (value_lval_const (val) == lval_register)
>>>>     fprintf_filtered (stream, _("<not saved>"));
>>>>   else
>>>>     fprintf_filtered (stream, _("<optimized out>"));
>>>> }
>>>>
>>>> work?  What could be the register value cases that would print
>>>> "not saved" that we'd still want to print "optimized out" ?
>>>
>>> The only case I can immediately think of where this would cause a
>>> problem would be for computed locations, (lval_computed).  The easy
>>> answer would be (in that case) the blame the compiler - why say the
>>> location is in a register if that register is volatile - but sadly I see
>>> this way too often.
>>
>> Hmm, OK, but then lval_computed values with that change won't
>> ever show "<not saved>", due to the lval_register check.  IOW,
>> we'd have to do something else in addition to lval_computed values
>> to make them print something other than the current <optimized out>.
>>
>> However, I've come to think there's a really simple rule to
>> follow here -- We should only ever print <not saved> for values
>> that represent machine/pseudo registers.  IOW, $pc, $rax, etc.
>> If the debug info happens to describe a variable as being located
>> in some optimized out register, we should still print
>> <optimized out>.  The previous version of the patch failed that:
>>
>>  (gdb) PASS: gdb.dwarf2/dw2-op-out-param.exp: continue to breakpoint: Stop in breakpt for test int_param_single_reg_loc
>>   bt
>>   #0  0x000000000040058f in breakpt ()
>>  -#1  0x00000000004005a2 in int_param_single_reg_loc (operand0=<optimized out>, operand1=0xdeadbe00deadbe01, operand2=<optimized out>)
>>  +#1  0x00000000004005a2 in int_param_single_reg_loc (operand0=<not saved>, operand1=0xdeadbe00deadbe01, operand2=<not saved>)
>>   #2  0x0000000000400577 in main ()
>>
>> It didn't really make a lot of sense.  This new version doesn't have
>> that change anymore.
>>
>> That simple rule suggests that whatever the internal representation,
>> we should be easily able to have a single central point where to tag
>> such values.  In fact, I think that already exists in value_of_register.
>>
>>> However, exchanging what I see as the current larger inconsistency, for
>>> this much smaller one seems like a good deal to me, especially if it
>>> gets this patch unblocked...
>>
>> Alright, what do you (all) think of of this (supposedly finished) patch
>> on top of yours (Andrew's) then?
> 
> 
> Looks good to me.  Thanks for this.

Thanks.  Could you apply your patch then?

-- 
Pedro Alves

Re: [PATCH] Print registers not saved in the frame as "<not saved>", instead of "<optimized out>".

[Andrew Burgess]

On 16/09/2013 8:05 PM, Pedro Alves wrote:
> On 09/05/2013 05:35 PM, Andrew Burgess wrote:
>> On 05/09/2013 5:29 PM, Pedro Alves wrote:
>>> Hi guys,
>>>
>>> Getting back to this, trying to make progress.
>>>
>>> On 08/19/2013 11:24 AM, Andrew Burgess wrote:
>>>> On 16/08/2013 7:41 PM, Pedro Alves wrote:
>>>>> On 08/12/2013 09: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".  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.
>>>>>>
>>>>>> 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.
>>>>>
>>>>> This is a tough call.  I do agree that "optimized out" for registers
>>>>> is a bit confusing.  However, we already do print "<optimized out>" in
>>>>> other places, such as when printing expressions, and consistency
>>>>> is good.  If we did add a distinction, I agree with Andrew that it should
>>>>> be done in a more systematic way.  However, I'm not really sure we need
>>>>> much machinery.  Wouldn't something like:
>>>>>
>>>>> void
>>>>> val_print_optimized_out (const struct value *val, struct ui_file *stream)
>>>>> {
>>>>>   if (value_lval_const (val) == lval_register)
>>>>>     fprintf_filtered (stream, _("<not saved>"));
>>>>>   else
>>>>>     fprintf_filtered (stream, _("<optimized out>"));
>>>>> }
>>>>>
>>>>> work?  What could be the register value cases that would print
>>>>> "not saved" that we'd still want to print "optimized out" ?
>>>>
>>>> The only case I can immediately think of where this would cause a
>>>> problem would be for computed locations, (lval_computed).  The easy
>>>> answer would be (in that case) the blame the compiler - why say the
>>>> location is in a register if that register is volatile - but sadly I see
>>>> this way too often.
>>>
>>> Hmm, OK, but then lval_computed values with that change won't
>>> ever show "<not saved>", due to the lval_register check.  IOW,
>>> we'd have to do something else in addition to lval_computed values
>>> to make them print something other than the current <optimized out>.
>>>
>>> However, I've come to think there's a really simple rule to
>>> follow here -- We should only ever print <not saved> for values
>>> that represent machine/pseudo registers.  IOW, $pc, $rax, etc.
>>> If the debug info happens to describe a variable as being located
>>> in some optimized out register, we should still print
>>> <optimized out>.  The previous version of the patch failed that:
>>>
>>>  (gdb) PASS: gdb.dwarf2/dw2-op-out-param.exp: continue to breakpoint: Stop in breakpt for test int_param_single_reg_loc
>>>   bt
>>>   #0  0x000000000040058f in breakpt ()
>>>  -#1  0x00000000004005a2 in int_param_single_reg_loc (operand0=<optimized out>, operand1=0xdeadbe00deadbe01, operand2=<optimized out>)
>>>  +#1  0x00000000004005a2 in int_param_single_reg_loc (operand0=<not saved>, operand1=0xdeadbe00deadbe01, operand2=<not saved>)
>>>   #2  0x0000000000400577 in main ()
>>>
>>> It didn't really make a lot of sense.  This new version doesn't have
>>> that change anymore.
>>>
>>> That simple rule suggests that whatever the internal representation,
>>> we should be easily able to have a single central point where to tag
>>> such values.  In fact, I think that already exists in value_of_register.
>>>
>>>> However, exchanging what I see as the current larger inconsistency, for
>>>> this much smaller one seems like a good deal to me, especially if it
>>>> gets this patch unblocked...
>>>
>>> Alright, what do you (all) think of of this (supposedly finished) patch
>>> on top of yours (Andrew's) then?
>>
>>
>> Looks good to me.  Thanks for this.
> 
> Thanks.  Could you apply your patch then?
> 

Committed. Thanks for your time.

Andrew

[PATCH+DOC] Print registers not saved in the frame as "<not saved>", instead of "<optimized out>".

[Pedro Alves]

Eli, I've added NEWS and documentation changes to this patch.
Are those OK?

---------
Currently we print not saved registers as <optimized out>.  That's
confusing, because what this really means is that the register was not
saved in the frame before calling another function.

Before:

 (gdb) p/x $rax $1 = <optimized out>
 (gdb) info registers rax
 rax            <optimized out>

After:

 (gdb) p/x $rax
 $1 = <not saved>
 (gdb) info registers rax
 rax            <not saved>

However, if for some reason the debug info describes a variable as being
in such a register, we still want to print <optimized out>.  IOW, <not
saved> is reserved for inspecting registers at the machine level.
The patch uses lval_register+optimized_out to encode the latter.

frame_unwind_got_optimized creates not_lval optimized out registers,
so by default, in most cases, we'll see <optimized out>.

value_of_register is the function eval.c uses for evaluating
OP_REGISTER (again, $pc, etc.), and related bits.  It isn't used for
anything else.  This function makes sure to return lval_register
values.  The patch makes "info registers" and the MI equivalent use it
too.  I think it just makes a lot of sense, as this makes it so that
when printing machine registers ($pc, etc.), we go through a central
function.

We're likely to need a different encoding at some point, if/when we
support partially saved registers.  Even then, I think
value_of_register will still be the spot to tag the intention to print
machine register values differently.

value_from_register however may also return optimized out
lval_register values, so at a couple places where we're computing a
variable's location from a dwarf expression, we convert the resulting
value away from lval_register to a regular optimized out value.

Tested on x86_64 Fedora 17

gdb/
2013-09-18  Pedro Alves  <palves@redhat.com>

	* cp-valprint.c (cp_print_value_fields): Adjust calls to
	val_print_optimized_out.
	* jv-valprint.c (java_print_value_fields): Likewise.
	* p-valprint.c (pascal_object_print_value_fields): Likewise.
	* dwarf2loc.c (dwarf2_evaluate_loc_desc_full) <xxxxxx>: If the register
	was not saved, return a new optimized out value.
	* findvar.c (address_from_register): Likewise.
	* frame.c (put_frame_register): Tweak error string to say the
	register was not saved, rather than optimized out.
	* infcmd.c (default_print_one_register_info): Adjust call to
	val_print_optimized_out.  Use value_of_register instead of
	get_frame_register_value.
	* mi/mi-main.c (output_register): Use value_of_register instead of
	get_frame_register_value.
	* valprint.c (valprint_check_validity): Likewise.
	(val_print_optimized_out): New value parameter.  If the value is
	lval_register, print <not saved> instead.
	(value_check_printable, val_print_scalar_formatted): Adjust calls
	to val_print_optimized_out.
	* valprint.h (val_print_optimized_out): New value parameter.
	* value.c (struct value) <optimized_out>: Extend comment.
	(error_value_optimized_out): New function.
	(require_not_optimized_out): Use it.  Use a different string for
	lval_register values.
	* value.h (error_value_optimized_out): New declaration.
	* NEWS: Mention <not saved>.

gdb/testsuite/
2013-09-18  Pedro Alves  <palves@redhat.com>

	* gdb.mi/mi-reg-undefined.exp (opt_out_pattern): Delete.
	(not_saved_pattern): New.
	Replace uses for the former with the latter.

gdb/doc/
2013-09-18  Pedro Alves  <palves@redhat.com>

	* gdb.texinfo (Registers): Explain <not saved>.
---
 gdb/NEWS                                       |  9 +++++++++
 gdb/cp-valprint.c                              |  4 ++--
 gdb/doc/gdb.texinfo                            | 15 +++++++++++----
 gdb/dwarf2loc.c                                | 16 +++++++++++++---
 gdb/findvar.c                                  |  9 +++++++++
 gdb/frame.c                                    |  2 +-
 gdb/infcmd.c                                   |  4 ++--
 gdb/jv-valprint.c                              |  4 ++--
 gdb/mi/mi-main.c                               |  2 +-
 gdb/p-valprint.c                               |  4 ++--
 gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp |  4 ++--
 gdb/testsuite/gdb.mi/mi-reg-undefined.exp      |  4 ++--
 gdb/valprint.c                                 | 13 ++++++++-----
 gdb/valprint.h                                 |  3 ++-
 gdb/value.c                                    | 22 +++++++++++++++++++---
 gdb/value.h                                    |  5 +++++
 16 files changed, 90 insertions(+), 30 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index af06a21..b06f90c 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -15,6 +15,15 @@
 
 * The "catch syscall" command now works on arm*-linux* targets.
 
+* GDB now shows "<not saved>" when printing values of registers that
+  have not been saved in the frame:
+
+    (gdb) p $rax $1 = <not saved>
+    (gdb) info registers rax rax <not saved>
+
+  Before, the former would print "<optimized out>", and the latter
+  "*value not available*".
+
 * Python scripting
 
   ** Frame filters and frame decorators have been added.
diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
index e83d979..1d7147c 100644
--- a/gdb/cp-valprint.c
+++ b/gdb/cp-valprint.c
@@ -298,7 +298,7 @@ cp_print_value_fields (struct type *type, struct type *real_type,
 					  TYPE_FIELD_BITPOS (type, i),
 					  TYPE_FIELD_BITSIZE (type, i)))
 		{
-		  val_print_optimized_out (stream);
+		  val_print_optimized_out (val, stream);
 		}
 	      else
 		{
@@ -334,7 +334,7 @@ cp_print_value_fields (struct type *type, struct type *real_type,
 				      _("<error reading variable: %s>"),
 				      ex.message);
 		  else if (v == NULL)
-		    val_print_optimized_out (stream);
+		    val_print_optimized_out (NULL, stream);
 		  else
 		    cp_print_static_field (TYPE_FIELD_TYPE (type, i),
 					   v, stream, recurse + 1,
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 60d2877..80b6648 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -10031,10 +10031,17 @@ 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.
+In some ABIs, some registers may not be preserved, or saved, across
+function calls.  It may therefore not be possible for @value{GDBN} to
+know the value a register had before the call (in other words, in a
+outer frame).  Values of registers that @value{GDBN} can tell were not
+saved in their stack frames are shown as @w{@samp{<not saved>}}.
+
+However, if debug or unwind information is missing, @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.
 
 @node Floating Point Hardware
 @section Floating Point Hardware
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 55d43f1..1313e17 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -2290,11 +2290,21 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
 	    if (byte_offset != 0)
 	      error (_("cannot use offset on synthetic pointer to register"));
 	    do_cleanups (value_chain);
-	    if (gdb_regnum != -1)
-	      retval = value_from_register (type, gdb_regnum, frame);
-	    else
+	   if (gdb_regnum == -1)
 	      error (_("Unable to access DWARF register number %d"),
 		     dwarf_regnum);
+	   retval = value_from_register (type, gdb_regnum, frame);
+	   if (value_optimized_out (retval))
+	     {
+	       /* This means the register has undefined value / was
+		  not saved.  As we're computing the location of some
+		  variable etc. in the program, not a value for
+		  inspecting a register ($pc, $sp, etc.), return a
+		  generic optimized out value instead, so that we show
+		  <optimized out> instead of <not saved>.  */
+	       do_cleanups (value_chain);
+	       retval = allocate_optimized_out_value (type);
+	     }
 	  }
 	  break;
 
diff --git a/gdb/findvar.c b/gdb/findvar.c
index 25242be..c3550b4 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -757,6 +757,15 @@ address_from_register (struct type *type, int regnum, struct frame_info *frame)
   value = value_from_register (type, regnum, frame);
   gdb_assert (value);
 
+  if (value_optimized_out (value))
+    {
+      /* This function is used while computing a location expression.
+	 Complain about the value being optimized out, rather than
+	 letting value_as_address complain about some random register
+	 the expression depends on not being saved.  */
+      error_value_optimized_out ();
+    }
+
   result = value_as_address (value);
   release_value (value);
   value_free (value);
diff --git a/gdb/frame.c b/gdb/frame.c
index d52c26a..be1a1b1 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1140,7 +1140,7 @@ put_frame_register (struct frame_info *frame, int regnum,
   frame_register (frame, regnum, &optim, &unavail,
 		  &lval, &addr, &realnum, NULL);
   if (optim)
-    error (_("Attempt to assign to a value that was optimized out."));
+    error (_("Attempt to assign to a register that was not saved."));
   switch (lval)
     {
     case lval_memory:
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index e29dcde..46102a5 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2035,7 +2035,7 @@ default_print_one_register_info (struct ui_file *file,
     }
   else if (value_optimized_out (val))
     {
-      val_print_optimized_out (file);
+      val_print_optimized_out (val, file);
       fprintf_filtered (file, "\n");
       return;
     }
@@ -2142,7 +2142,7 @@ default_print_registers_info (struct gdbarch *gdbarch,
 
       default_print_one_register_info (file,
 				       gdbarch_register_name (gdbarch, i),
-				       get_frame_register_value (frame, i));
+				       value_of_register (i, frame));
     }
 }
 
diff --git a/gdb/jv-valprint.c b/gdb/jv-valprint.c
index 3b90e54..cb89a85 100644
--- a/gdb/jv-valprint.c
+++ b/gdb/jv-valprint.c
@@ -395,7 +395,7 @@ java_print_value_fields (struct type *type, const gdb_byte *valaddr,
 	      else if (!value_bits_valid (val, TYPE_FIELD_BITPOS (type, i),
 					  TYPE_FIELD_BITSIZE (type, i)))
 		{
-		  val_print_optimized_out (stream);
+		  val_print_optimized_out (val, stream);
 		}
 	      else
 		{
@@ -420,7 +420,7 @@ java_print_value_fields (struct type *type, const gdb_byte *valaddr,
 		  struct value *v = value_static_field (type, i);
 
 		  if (v == NULL)
-		    val_print_optimized_out (stream);
+		    val_print_optimized_out (NULL, stream);
 		  else
 		    {
 		      struct value_print_options opts;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index e8c4744..5cde1ae 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1161,7 +1161,7 @@ output_register (struct frame_info *frame, int regnum, int format,
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
   struct ui_out *uiout = current_uiout;
-  struct value *val = get_frame_register_value (frame, regnum);
+  struct value *val = value_of_register (regnum, frame);
   struct cleanup *tuple_cleanup;
   struct value_print_options opts;
   struct ui_file *stb;
diff --git a/gdb/p-valprint.c b/gdb/p-valprint.c
index 05d4c6f..e6d4b91 100644
--- a/gdb/p-valprint.c
+++ b/gdb/p-valprint.c
@@ -629,7 +629,7 @@ pascal_object_print_value_fields (struct type *type, const gdb_byte *valaddr,
 	      else if (!value_bits_valid (val, TYPE_FIELD_BITPOS (type, i),
 					  TYPE_FIELD_BITSIZE (type, i)))
 		{
-		  val_print_optimized_out (stream);
+		  val_print_optimized_out (val, stream);
 		}
 	      else
 		{
@@ -657,7 +657,7 @@ pascal_object_print_value_fields (struct type *type, const gdb_byte *valaddr,
 		  v = value_field_bitfield (type, i, valaddr, offset, val);
 
 		  if (v == NULL)
-		    val_print_optimized_out (stream);
+		    val_print_optimized_out (NULL, stream);
 		  else
 		    pascal_object_print_static_field (v, stream, recurse + 1,
 						      options);
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
index 4686648..44bf8e9 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
@@ -45,8 +45,8 @@ for {set f 0} {$f < 3} {incr f} {
 	set pattern_r8_r9_print "$hex"
 	set pattern_r8_r9_info "$hex\\s+$decimal"
     } else {
-	set pattern_rax_rbx_rcx_print "<optimized out>"
-	set pattern_rax_rbx_rcx_info "<optimized out>"
+	set pattern_rax_rbx_rcx_print "<not saved>"
+	set pattern_rax_rbx_rcx_info "<not saved>"
 	set pattern_r8_r9_print "$hex"
 	set pattern_r8_r9_info "$hex\\s+$decimal"
     }
diff --git a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
index 8bcbb21..cb3a716 100644
--- a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
+++ b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
@@ -52,13 +52,13 @@ mi_gdb_test "111-stack-list-frames" \
     "111\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"stop_frame\",.*\},frame=\{level=\"1\",addr=\"$hex\",func=\"first_frame\",.*\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",.*\}\\\]" \
     "stack frame listing"
 
-set opt_out_pattern "<optimized out>"
+set not_saved_pattern "<not saved>"
 
 for {set f 0} {$f < 3} {incr f} {
     if {${f} == 0} {
 	set pattern_0_1_2 ${hex}
     } else {
-	set pattern_0_1_2 ${opt_out_pattern}
+	set pattern_0_1_2 ${not_saved_pattern}
     }
 
     mi_gdb_test "22${f}-data-list-register-values --thread 1 --frame ${f} x  0 1 2 8 9" \
diff --git a/gdb/valprint.c b/gdb/valprint.c
index 0f6d65e..61492cc 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -314,7 +314,7 @@ valprint_check_validity (struct ui_file *stream,
       if (!value_bits_valid (val, TARGET_CHAR_BIT * embedded_offset,
 			     TARGET_CHAR_BIT * TYPE_LENGTH (type)))
 	{
-	  val_print_optimized_out (stream);
+	  val_print_optimized_out (val, stream);
 	  return 0;
 	}
 
@@ -336,9 +336,12 @@ valprint_check_validity (struct ui_file *stream,
 }
 
 void
-val_print_optimized_out (struct ui_file *stream)
+val_print_optimized_out (const struct value *val, struct ui_file *stream)
 {
-  fprintf_filtered (stream, _("<optimized out>"));
+  if (val != NULL && value_lval_const (val) == lval_register)
+    fprintf_filtered (stream, _("<not saved>"));
+  else
+    fprintf_filtered (stream, _("<optimized out>"));
 }
 
 void
@@ -805,7 +808,7 @@ value_check_printable (struct value *val, struct ui_file *stream,
       if (options->summary && !val_print_scalar_type_p (value_type (val)))
 	fprintf_filtered (stream, "...");
       else
-	val_print_optimized_out (stream);
+	val_print_optimized_out (val, stream);
       return 0;
     }
 
@@ -966,7 +969,7 @@ val_print_scalar_formatted (struct type *type,
      printed, because all bits contribute to its representation.  */
   if (!value_bits_valid (val, TARGET_CHAR_BIT * embedded_offset,
 			      TARGET_CHAR_BIT * TYPE_LENGTH (type)))
-    val_print_optimized_out (stream);
+    val_print_optimized_out (val, stream);
   else if (!value_bytes_available (val, embedded_offset, TYPE_LENGTH (type)))
     val_print_unavailable (stream);
   else
diff --git a/gdb/valprint.h b/gdb/valprint.h
index e7073b6..1d86ed7 100644
--- a/gdb/valprint.h
+++ b/gdb/valprint.h
@@ -160,7 +160,8 @@ extern int read_string (CORE_ADDR addr, int len, int width,
 			enum bfd_endian byte_order, gdb_byte **buffer,
 			int *bytes_read);
 
-extern void val_print_optimized_out (struct ui_file *stream);
+extern void val_print_optimized_out (const struct value *val,
+				     struct ui_file *stream);
 
 extern void val_print_unavailable (struct ui_file *stream);
 
diff --git a/gdb/value.c b/gdb/value.c
index bc1239d..d96d285 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -197,8 +197,13 @@ struct value
      reset, be sure to consider this use as well!  */
   unsigned int lazy : 1;
 
-  /* If nonzero, this is the value of a variable which does not
-     actually exist in the program.  */
+  /* If nonzero, this is the value of a variable that does not
+     actually exist in the program.  If nonzero, and LVAL is
+     lval_register, this is a register ($pc, $sp, etc., never a
+     program variable) that has not been saved in the frame.  All
+     optimized-out values are treated pretty much the same, except
+     registers have a different string representation and related
+     error strings.  */
   unsigned int optimized_out : 1;
 
   /* If value is a variable, is it initialized or not.  */
@@ -902,11 +907,22 @@ value_actual_type (struct value *value, int resolve_simple_types,
   return result;
 }
 
+void
+error_value_optimized_out (void)
+{
+  error (_("value has been optimized out"));
+}
+
 static void
 require_not_optimized_out (const struct value *value)
 {
   if (value->optimized_out)
-    error (_("value has been optimized out"));
+    {
+      if (value->lval == lval_register)
+	error (_("register has not been saved in frame"));
+      else
+	error_value_optimized_out ();
+    }
 }
 
 static void
diff --git a/gdb/value.h b/gdb/value.h
index 98dbadf..db964e3 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -273,6 +273,11 @@ extern void set_value_lazy (struct value *value, int val);
 extern int value_stack (struct value *);
 extern void set_value_stack (struct value *value, int val);
 
+/* Throw an error complaining that the value has been optimized
+   out.  */
+
+extern void error_value_optimized_out (void);
+
 /* value_contents() and value_contents_raw() both return the address
    of the gdb buffer used to hold a copy of the contents of the lval.
    value_contents() is used when the contents of the buffer are needed
-- 
1.7.11.7

Re: [PATCH+DOC] Print registers not saved in the frame as "<not saved>", instead of "<optimized out>".

[Eli Zaretskii]

> Date: Wed, 18 Sep 2013 16:54:27 +0100
> From: Pedro Alves <palves@redhat.com>
> CC: gdb-patches@sourceware.org, Mark Kettenis <mark.kettenis@xs4all.nl>
> 
> Eli, I've added NEWS and documentation changes to this patch.
> Are those OK?

Almost.

> +* GDB now shows "<not saved>" when printing values of registers that
> +  have not been saved in the frame:
> +
> +    (gdb) p $rax $1 = <not saved>
> +    (gdb) info registers rax rax <not saved>

Wrong formatting of what GDB prints.

> +In some ABIs, some registers may not be preserved, or saved, across
> +function calls.  It may therefore not be possible for @value{GDBN} to
> +know the value a register had before the call (in other words, in a
                                                                     ^
"the"

> +outer frame).  Values of registers that @value{GDBN} can tell were not
> +saved in their stack frames are shown as @w{@samp{<not saved>}}.
> +
> +However, if debug or unwind information is missing, @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.

I don't understand the significance of the last paragraph.

Also, shouldn't we mention optimizations as (the main) reason for
registers being unavailable?

Thanks.

Re: [PATCH+DOC] Print registers not saved in the frame as "<not saved>", instead of "<optimized out>".

[Pedro Alves]

On 09/18/2013 05:30 PM, Eli Zaretskii wrote:
>> Date: Wed, 18 Sep 2013 16:54:27 +0100
>> From: Pedro Alves <palves@redhat.com>
>> CC: gdb-patches@sourceware.org, Mark Kettenis <mark.kettenis@xs4all.nl>
>>
>> Eli, I've added NEWS and documentation changes to this patch.
>> Are those OK?
> 
> Almost.
> 
>> +* GDB now shows "<not saved>" when printing values of registers that
>> +  have not been saved in the frame:
>> +
>> +    (gdb) p $rax $1 = <not saved>
>> +    (gdb) info registers rax rax <not saved>
> 
> Wrong formatting of what GDB prints.

Blah.  Bad copy/paste.  Thanks.

> 
>> +In some ABIs, some registers may not be preserved, or saved, across
>> +function calls.  It may therefore not be possible for @value{GDBN} to
>> +know the value a register had before the call (in other words, in a
>                                                                      ^
> "the"

Fixed.

> 
>> +outer frame).  Values of registers that @value{GDBN} can tell were not
>> +saved in their stack frames are shown as @w{@samp{<not saved>}}.
>> +
>> +However, if debug or unwind information is missing, @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.
> 
> I don't understand the significance of the last paragraph.

It's preexisting actually.  I just added the "debug or unwind info" bit.
But yeah, it's confusing.  I _think_ I know what it's talking about.
I think "makes no difference" refers to GDB assuming $reg in an outer
frame is found at the same location as in the inner frame (that is,
assuming the call clobbered register hasn't been clobbered yet).

I've rewritten all this text now.  What do you think?

> Also, shouldn't we mention optimizations as (the main) reason for
> registers being unavailable?

I'm not actually sure how to say that.  Not sure you actually
always need optimization to see this in the debugger.  But maybe
we don't need to in this new version.  :-)

diff --git c/gdb/NEWS w/gdb/NEWS
index af06a21..7c10cbb 100644
--- c/gdb/NEWS
+++ w/gdb/NEWS
@@ -15,6 +15,19 @@

 * The "catch syscall" command now works on arm*-linux* targets.

+* GDB now shows "<not saved>" when printing values of registers the
+  debug info indicates have not been saved in the frame and there's
+  nowhere to retrieve them from:
+
+    (gdb) p $rax
+    $1 = <not saved>
+
+    (gdb) info registers rax
+    rax            <not saved>
+
+  Before, the former would print "<optimized out>", and the latter
+  "*value not available*".
+
 * Python scripting

   ** Frame filters and frame decorators have been added.
diff --git c/gdb/doc/gdb.texinfo w/gdb/doc/gdb.texinfo
index 60d2877..d122874 100644
--- c/gdb/doc/gdb.texinfo
+++ w/gdb/doc/gdb.texinfo
@@ -10031,10 +10031,21 @@ 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.
+In some ABIs, some registers may not be preserved, or saved, across
+function calls.  It may therefore not be possible for @value{GDBN} to
+know the value a register had before the call (in other words, in the
+outer frame), if the register has since been clobbered 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.  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.  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.  Values of
+registers that @value{GDBN} can definitely tell from the debug/unwind
+info were not saved in their stack frames and there's nowhere to
+retrieve the original value from anymore are shown as
+@w{@samp{<not saved>}}.

 @node Floating Point Hardware
 @section Floating Point Hardware

Re: [PATCH+DOC] Print registers not saved in the frame as "<not saved>", instead of "<optimized out>".

[Eli Zaretskii]

> Date: Wed, 18 Sep 2013 18:35:07 +0100
> From: Pedro Alves <palves@redhat.com>
> CC: aburgess@broadcom.com, gdb-patches@sourceware.org, mark.kettenis@xs4all.nl
> 
> > I don't understand the significance of the last paragraph.
> 
> It's preexisting actually.  I just added the "debug or unwind info" bit.
> But yeah, it's confusing.  I _think_ I know what it's talking about.
> I think "makes no difference" refers to GDB assuming $reg in an outer
> frame is found at the same location as in the inner frame (that is,
> assuming the call clobbered register hasn't been clobbered yet).
> 
> I've rewritten all this text now.  What do you think?

We are almost there, just a few minor comments below.

> > Also, shouldn't we mention optimizations as (the main) reason for
> > registers being unavailable?
> 
> I'm not actually sure how to say that.  Not sure you actually
> always need optimization to see this in the debugger.  But maybe
> we don't need to in this new version.  :-)

One of the comments below addresses that.

> +In some ABIs, some registers may not be preserved, or saved, across
> +function calls.

It's enough to mention optimizations here, as in

  In some ABIs, or in optimized code, some registers may not be preserved

> +the outer frame, you may also be affecting the inner frame.  Values of
> +registers that @value{GDBN} can definitely tell from the debug/unwind
> +info were not saved in their stack frames and there's nowhere to
> +retrieve the original value from anymore are shown as
> +@w{@samp{<not saved>}}.

The last sentence is unduly hard to read, because it uses passive
tense too aggressively.  I would rephrase:

  If @value{GDBN} can definitely tell from the debug/unwind info that
  the value of a register was not saved in its stack frame, and
  there's no other place for it from which to retrieve that register's
  original value, @value{GDBN} will show the value of such a register
  as @w{@samp{<not saved>}}.

OK with those changes.

Thanks.

Re: [PATCH+DOC] Print registers not saved in the frame as "<not saved>", instead of "<optimized out>".

[Mark Kettenis]

> Date: Wed, 18 Sep 2013 19:30:07 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> Also, shouldn't we mention optimizations as (the main) reason for
> registers being unavailable?

I don't consider optimisations as an important cause of registers not
being saved.  It is primarily a consequence of the fact that most ABIs
reserve some registers as "scratch" registers that don't need to be
saved.

Re: [PATCH+DOC] Print registers not saved in the frame as "<not saved>", instead of "<optimized out>".

[Eli Zaretskii]

> Date: Wed, 18 Sep 2013 22:47:24 +0200 (CEST)
> From: Mark Kettenis <mark.kettenis@xs4all.nl>
> CC: palves@redhat.com, aburgess@broadcom.com, gdb-patches@sourceware.org,
>         mark.kettenis@xs4all.nl
> 
> > Date: Wed, 18 Sep 2013 19:30:07 +0300
> > From: Eli Zaretskii <eliz@gnu.org>
> > 
> > Also, shouldn't we mention optimizations as (the main) reason for
> > registers being unavailable?
> 
> I don't consider optimisations as an important cause of registers not
> being saved.

The question is: does it happen in practice?  If it does, I think we
should mention that, because it's important to let the users know that
recompiling without optimizations might remove this nuisance.

Re: [PATCH+DOC] Print registers not saved in the frame as "<not saved>", instead of "<optimized out>".

[Pedro Alves]

On 09/19/2013 08:53 AM, Eli Zaretskii wrote:
>> Date: Wed, 18 Sep 2013 22:47:24 +0200 (CEST)
>> From: Mark Kettenis <mark.kettenis@xs4all.nl>
>> CC: palves@redhat.com, aburgess@broadcom.com, gdb-patches@sourceware.org,
>>         mark.kettenis@xs4all.nl
>>
>>> Date: Wed, 18 Sep 2013 19:30:07 +0300
>>> From: Eli Zaretskii <eliz@gnu.org>
>>>
>>> Also, shouldn't we mention optimizations as (the main) reason for
>>> registers being unavailable?
>>
>> I don't consider optimisations as an important cause of registers not
>> being saved.
> 
> The question is: does it happen in practice?  If it does, I think we
> should mention that, because it's important to let the users know that
> recompiling without optimizations might remove this nuisance.

I think it's really independent of optimization.

Jan's comments at <https://sourceware.org/ml/gdb-patches/2012-08/msg00787.html>
help understand this.   Current/recent enough GCC doesn't mark variables/arguments
as being in call-clobbered registers in the ranges corresponding to function calls,
while older GCCs did.  Newer GCCs will just not say where the variable is, so GDB
will end up realizing the variable is optimized out.  In fact, if I build
an -O2 version of GDB, and do

 $ readelf --debug-dump=frames ./gdb | grep DW_CFA_undefined

nothing comes out.

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

I wondered what would the "return" command do if forcing
a return to a function where we current show registers
as "<optimized out>/<not saved>", to see if that
would through a "value optimized out" error or some such.
It actually doesn't:

Breakpoint 2, stop_frame () at dw2-reg-undefined.c:22
22      in dw2-reg-undefined.c
(gdb) info registers
rax            0x0      0
rbx            0x0      0
rcx            0x400552 4195666
rdx            0x7fffffffdb18   140737488345880
rsi            0x7fffffffdb08   140737488345864
rdi            0x1      1
rbp            0x7fffffffda00   0x7fffffffda00
rsp            0x7fffffffda00   0x7fffffffda00
r8             0x323d7b1f40     215779843904
r9             0x323d00f310     215771837200
r10            0x7fffffffd880   140737488345216
r11            0x323d421640     215776106048
r12            0x400430 4195376
r13            0x7fffffffdb00   140737488345856
r14            0x0      0
r15            0x0      0
rip            0x400540 0x400540 <stop_frame+4>
eflags         0x246    [ PF ZF IF ]
cs             0x33     51
ss             0x2b     43
ds             0x0      0
es             0x0      0
fs             0x0      0
gs             0x0      0
(gdb) up
#1  0x0000000000400550 in first_frame () at dw2-reg-undefined.c:27
27      in dw2-reg-undefined.c
(gdb) info registers
rax            <optimized out>
rbx            <optimized out>
rcx            <optimized out>
rdx            <optimized out>
rsi            <optimized out>
rdi            <optimized out>
rbp            0x7fffffffda10   0x7fffffffda10
rsp            <optimized out>
r8             0x323d7b1f40     215779843904
r9             0x323d00f310     215771837200
r10            0x7fffffffd880   140737488345216
r11            0x323d421640     215776106048
r12            0x400430 4195376
r13            0x7fffffffdb00   140737488345856
r14            0x0      0
r15            0x0      0
rip            0x400550 0x400550 <first_frame+14>
eflags         0x246    [ PF ZF IF ]
cs             0x33     51
ss             0x2b     43
ds             0x0      0
es             0x0      0
fs             0x0      0
gs             0x0      0
(gdb) down
#0  stop_frame () at dw2-reg-undefined.c:22
22      in dw2-reg-undefined.c
(gdb) return
Make stop_frame return now? (y or n) y
#0  first_frame () at dw2-reg-undefined.c:28
28      in dw2-reg-undefined.c
(gdb) info registers
rax            0x0      0
rbx            0x0      0
rcx            0x400552 4195666
rdx            0x7fffffffdb18   140737488345880
rsi            0x7fffffffdb08   140737488345864
rdi            0x1      1
rbp            0x7fffffffda10   0x7fffffffda10
rsp            0x7fffffffda00   0x7fffffffda00
r8             0x323d7b1f40     215779843904
r9             0x323d00f310     215771837200
r10            0x7fffffffd880   140737488345216
r11            0x323d421640     215776106048
r12            0x400430 4195376
r13            0x7fffffffdb00   140737488345856
r14            0x0      0
r15            0x0      0
rip            0x400550 0x400550 <first_frame+14>
eflags         0x246    [ PF ZF IF ]
cs             0x33     51
ss             0x2b     43
ds             0x0      0
es             0x0      0
fs             0x0      0
gs             0x0      0
(gdb)

In case that isn't clear, GDB did fetch the values of
call-clobbered registers from the inner frame.  I didn't
check where/how that happens.

-- 
Pedro Alves

[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]

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.  See with the gdb.dwarf/dw2-op-out-param.exp
test:

Breakpoint 2, 0x000000000040058f in breakpt ()
(gdb) up
#1  0x00000000004005a2 in int_param_single_reg_loc (operand0=<optimized out>, operand1=-2401054115373400575, operand2=<optimized out>)
(gdb) info registers
rax            0x323d7b35b0     215779849648
rbx            0xdeadbe00deadbe01       -2401054115373400575
rcx            <optimized out>
rdx            0xdeadbe04deadbe05       -2401054098193531387
rsi            <optimized out>
rdi            <optimized out>
rbp            0x0      0x0
rsp            0x7fffffffda20   0x7fffffffda20
r8             0x323d7b1f40     215779843904
r9             0x323d00f310     215771837200
r10            0x7fffffffd890   140737488345232
r11            0x323d421640     215776106048
r12            0x400430 4195376
r13            0x7fffffffdb10   140737488345872
r14            0x0      0
r15            0x0      0
rip            0x4005a2 0x4005a2 <int_param_single_reg_loc+10>
eflags         0x202    [ IF ]
cs             0x33     51
ss             0x2b     43
ds             0x0      0
es             0x0      0
fs             0x0      0
gs             0x0      0

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.

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.

After the patch, we get:

#0  0x000000000040058f in breakpt ()
(gdb) info registers
rax            0x323d7b35b0     215779849648
rbx            0xdeadbe00deadbe01       -2401054115373400575
rcx            0xdeadbe02deadbe03       -2401054106783465981
rdx            0xdeadbe04deadbe05       -2401054098193531387
rsi            0xdeadbe06deadbe07       -2401054089603596793
rdi            0xdeadbe08deadbe09       -2401054081013662199
rbp            0x0      0x0
rsp            0x7fffffffda18   0x7fffffffda18
r8             0x323d7b1f40     215779843904
r9             0x323d00f310     215771837200
r10            0x7fffffffd890   140737488345232
r11            0x323d421640     215776106048
r12            0x400430 4195376
r13            0x7fffffffdb10   140737488345872
r14            0x0      0
r15            0x0      0
rip            0x40058f 0x40058f <breakpt>
eflags         0x202    [ IF ]
cs             0x33     51
ss             0x2b     43
ds             0x0      0
es             0x0      0
fs             0x0      0
gs             0x0      0
(gdb) up
#1  0x00000000004005a2 in int_param_single_reg_loc (operand0=<optimized out>, operand1=-2401054115373400575, operand2=<optimized out>)
(gdb) info registers
rax            0x323d7b35b0     215779849648
rbx            0xdeadbe00deadbe01       -2401054115373400575
rcx            0xdeadbe02deadbe03       -2401054106783465981
rdx            0xdeadbe04deadbe05       -2401054098193531387
rsi            0xdeadbe06deadbe07       -2401054089603596793
rdi            0xdeadbe08deadbe09       -2401054081013662199
rbp            0x0      0x0
rsp            0x7fffffffda20   0x7fffffffda20
r8             0x323d7b1f40     215779843904
r9             0x323d00f310     215771837200
r10            0x7fffffffd890   140737488345232
r11            0x323d421640     215776106048
r12            0x400430 4195376
r13            0x7fffffffdb10   140737488345872
r14            0x0      0
r15            0x0      0
rip            0x4005a2 0x4005a2 <int_param_single_reg_loc+10>
eflags         0x202    [ IF ]
cs             0x33     51
ss             0x2b     43
ds             0x0      0
es             0x0      0
fs             0x0      0
gs             0x0      0
(gdb)

Which is what you'd get with a newer GCC.  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:

(gdb) return
Make breakpt return now? (y or n) y
#0  0x00000000004005a2 in int_param_single_reg_loc (operand0=-2401054106783465981, operand1=-2401054115373400575,
    operand2=-2401054089603596793)
(gdb) info registers
rax            0x323d7b35b0     215779849648
rbx            0xdeadbe00deadbe01       -2401054115373400575
rcx            0xdeadbe02deadbe03       -2401054106783465981
rdx            0xdeadbe04deadbe05       -2401054098193531387
rsi            0xdeadbe06deadbe07       -2401054089603596793
rdi            0xdeadbe08deadbe09       -2401054081013662199
rbp            0x0      0x0
rsp            0x7fffffffda20   0x7fffffffda20
r8             0x323d7b1f40     215779843904
r9             0x323d00f310     215771837200
r10            0x7fffffffd890   140737488345232
r11            0x323d421640     215776106048
r12            0x400430 4195376
r13            0x7fffffffdb10   140737488345872
r14            0x0      0
r15            0x0      0
rip            0x4005a2 0x4005a2 <int_param_single_reg_loc+10>
eflags         0x202    [ IF ]
cs             0x33     51
ss             0x2b     43
ds             0x0      0
es             0x0      0
fs             0x0      0
gs             0x0      0
(gdb)

This is rule is applied in value_of_register.  This is the function
eval.c uses for evaluating OP_REGISTER (again, $reg, $pc, etc.), and
related bits.  It isn't used for anything else.  The patch makes "info
registers" and the MI equivalent use it too.  I think it just makes a
lot of sense, as this makes it so that when printing machine registers
($pc, etc.), we go through a central function.

Note how gdb.mi/mi-reg-undefined.exp and
gdb.dwarf2/dw2-reg-undefined.exp tests needed adjustment, but not
gdb.dwarf2/dw2-op-out-param.exp, as that prints an optimized out
variable, not register.

Tested on x86_64 Fedora 17

gdb/
2013-09-18  Pedro Alves  <palves@redhat.com>

	* findvar.c (value_of_register): Rename to ...
	(value_of_register_helper): ... this.
	(value_of_register): Reimplement.
	* infcmd.c (default_print_one_register_info): Use
	value_of_register instead of get_frame_register_value.
	* mi/mi-main.c (output_register): Use value_of_register instead of
	get_frame_register_value.

gdb/testsuite/
2013-09-18  Pedro Alves  <palves@redhat.com>

	* gdb.dwarf2/dw2-reg-undefined.exp: Expect hex numbers instead of
	"<optimized out>" for all frames.
	* gdb.mi/mi-reg-undefined.exp (opt_out_pattern): Delete.
	(top level): Expect hex numbers instead of "<optimized out>" for
	all frames.

gdb/doc/
2013-09-18  Pedro Alves  <palves@redhat.com>

	* gdb.texinfo (Registers): Expand explanation of not-saved
	call-clobbered registers in frames other than the innermost.
---
 gdb/doc/gdb.texinfo                            | 20 ++++++++++++----
 gdb/findvar.c                                  | 32 ++++++++++++++++++++++----
 gdb/infcmd.c                                   |  2 +-
 gdb/mi/mi-main.c                               |  2 +-
 gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp | 17 +++++---------
 gdb/testsuite/gdb.mi/mi-reg-undefined.exp      | 14 ++++-------
 6 files changed, 55 insertions(+), 32 deletions(-)

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
+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.  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.
 
 @node Floating Point Hardware
 @section Floating Point Hardware
diff --git a/gdb/findvar.c b/gdb/findvar.c
index 25242be..5cbbc83 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -254,12 +254,11 @@ store_typed_address (gdb_byte *buf, struct type *type, CORE_ADDR addr)
 
 
 
-/* Return a `value' with the contents of (virtual or cooked) register
-   REGNUM as found in the specified FRAME.  The register's type is
-   determined by register_type().  */
+/* Helper for value_of_register; see value_of_register for description
+   of parameters/return.  */
 
-struct value *
-value_of_register (int regnum, struct frame_info *frame)
+static struct value *
+value_of_register_helper (int regnum, struct frame_info *frame)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
   struct value *reg_val;
@@ -277,6 +276,29 @@ value_of_register (int regnum, struct frame_info *frame)
 
 /* Return a `value' with the contents of (virtual or cooked) register
    REGNUM as found in the specified FRAME.  The register's type is
+   determined by register_type().  */
+
+struct value *
+value_of_register (int regnum, struct frame_info *frame)
+{
+  while (1)
+    {
+      struct value *reg_val = value_of_register_helper (regnum, frame);
+
+      if (!value_optimized_out (reg_val))
+	return reg_val;
+      /* If a register has been marked as "optimized out"/"not saved"
+	 in the unwind/debug info for this frame, fetch it from the
+	 next/inner frame.  The innermost frame fetches registers from
+	 the regcache, so should never return an optimized out
+	 register.  */
+      gdb_assert (frame_relative_level (frame) > 0);
+      frame = get_next_frame (frame);
+    }
+}
+
+/* Return a `value' with the contents of (virtual or cooked) register
+   REGNUM as found in the specified FRAME.  The register's type is
    determined by register_type().  The value is not fetched.  */
 
 struct value *
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index e29dcde..e9cd255 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2142,7 +2142,7 @@ default_print_registers_info (struct gdbarch *gdbarch,
 
       default_print_one_register_info (file,
 				       gdbarch_register_name (gdbarch, i),
-				       get_frame_register_value (frame, i));
+				       value_of_register (i, frame));
     }
 }
 
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index e8c4744..5cde1ae 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1161,7 +1161,7 @@ output_register (struct frame_info *frame, int regnum, int format,
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
   struct ui_out *uiout = current_uiout;
-  struct value *val = get_frame_register_value (frame, regnum);
+  struct value *val = value_of_register (regnum, frame);
   struct cleanup *tuple_cleanup;
   struct value_print_options opts;
   struct ui_file *stb;
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
index 4686648..58b50d5 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
@@ -39,17 +39,12 @@ gdb_test "bt" "#0  (0x\[0-9a-f\]+ in )?stop_frame \[^\r\n\]*\r\n#1  \[^\r\n\]*fi
     "backtrace from stop_frame"
 
 for {set f 0} {$f < 3} {incr f} {
-    if {${f} == 0} {
-	set pattern_rax_rbx_rcx_print "$hex"
-	set pattern_rax_rbx_rcx_info "$hex\\s+$decimal"
-	set pattern_r8_r9_print "$hex"
-	set pattern_r8_r9_info "$hex\\s+$decimal"
-    } else {
-	set pattern_rax_rbx_rcx_print "<optimized out>"
-	set pattern_rax_rbx_rcx_info "<optimized out>"
-	set pattern_r8_r9_print "$hex"
-	set pattern_r8_r9_info "$hex\\s+$decimal"
-    }
+    # Even though registers 0->6 are marked as being undefined in
+    # frames > 0, GDB should retrieve their values from inner frames.
+    set pattern_rax_rbx_rcx_print "$hex"
+    set pattern_rax_rbx_rcx_info "$hex\\s+$decimal"
+    set pattern_r8_r9_print "$hex"
+    set pattern_r8_r9_info "$hex\\s+$decimal"
 
     # Select frame.
     gdb_test "frame ${f}" "#${f}.*" "Switch to frame ${f}"
diff --git a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
index 8bcbb21..d0f275c 100644
--- a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
+++ b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
@@ -52,20 +52,14 @@ mi_gdb_test "111-stack-list-frames" \
     "111\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"stop_frame\",.*\},frame=\{level=\"1\",addr=\"$hex\",func=\"first_frame\",.*\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",.*\}\\\]" \
     "stack frame listing"
 
-set opt_out_pattern "<optimized out>"
-
 for {set f 0} {$f < 3} {incr f} {
-    if {${f} == 0} {
-	set pattern_0_1_2 ${hex}
-    } else {
-	set pattern_0_1_2 ${opt_out_pattern}
-    }
-
+    # Even though registers 0->6 are marked as being undefined in
+    # frames > 0, GDB should retrieve their values from inner frames.
     mi_gdb_test "22${f}-data-list-register-values --thread 1 --frame ${f} x  0 1 2 8 9" \
-	"22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
+	"22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${hex}\"\},\{number=\"1\",value=\"${hex}\"\},\{number=\"2\",value=\"${hex}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
 	"register values, format x, frame ${f}"
 
     mi_gdb_test "33${f}-data-list-register-values --thread 1 --frame ${f} r 0 1 2 8 9" \
-	"33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
+	"33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${hex}\"\},\{number=\"1\",value=\"${hex}\"\},\{number=\"2\",value=\"${hex}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
 	"register values, format r, frame ${f}"
 }
-- 
1.7.11.7

Re: [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>".)

[Eli Zaretskii]

> Date: Thu, 19 Sep 2013 20:15:20 +0100
> From: Pedro Alves <palves@redhat.com>
> CC: Eli Zaretskii <eliz@gnu.org>, Mark Kettenis <mark.kettenis@xs4all.nl>,
>         aburgess@broadcom.com
> 
> +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

"@:" after "a.k.a."

Also, caller-saved and call-clobbered should probably be in ``..''
quotes.

Otherwise, OK for the documentation part.

Thanks.

Re: [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>".)

[Doug Evans]

Pedro Alves writes:
 > gdb/
 > 2013-09-18  Pedro Alves  <palves@redhat.com>
 > 
 > 	* findvar.c (value_of_register): Rename to ...
 > 	(value_of_register_helper): ... this.
 > 	(value_of_register): Reimplement.
 > 	* infcmd.c (default_print_one_register_info): Use
 > 	value_of_register instead of get_frame_register_value.
 > 	* mi/mi-main.c (output_register): Use value_of_register instead of
 > 	get_frame_register_value.
 > 
 > gdb/testsuite/
 > 2013-09-18  Pedro Alves  <palves@redhat.com>
 > 
 > 	* gdb.dwarf2/dw2-reg-undefined.exp: Expect hex numbers instead of
 > 	"<optimized out>" for all frames.
 > 	* gdb.mi/mi-reg-undefined.exp (opt_out_pattern): Delete.
 > 	(top level): Expect hex numbers instead of "<optimized out>" for
 > 	all frames.
 > 
 > gdb/doc/
 > 2013-09-18  Pedro Alves  <palves@redhat.com>
 > 
 > 	* gdb.texinfo (Registers): Expand explanation of not-saved
 > 	call-clobbered registers in frames other than the innermost.

"works for me"

Re: [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>".)

[Doug Evans]

Pedro Alves writes:
 > gdb/
 > 2013-09-18  Pedro Alves  <palves@redhat.com>
 > 
 > 	* findvar.c (value_of_register): Rename to ...
 > 	(value_of_register_helper): ... this.
 > 	(value_of_register): Reimplement.
 > 	* infcmd.c (default_print_one_register_info): Use
 > 	value_of_register instead of get_frame_register_value.
 > 	* mi/mi-main.c (output_register): Use value_of_register instead of
 > 	get_frame_register_value.
 > 
 > gdb/testsuite/
 > 2013-09-18  Pedro Alves  <palves@redhat.com>
 > 
 > 	* gdb.dwarf2/dw2-reg-undefined.exp: Expect hex numbers instead of
 > 	"<optimized out>" for all frames.
 > 	* gdb.mi/mi-reg-undefined.exp (opt_out_pattern): Delete.
 > 	(top level): Expect hex numbers instead of "<optimized out>" for
 > 	all frames.
 > 
 > gdb/doc/
 > 2013-09-18  Pedro Alves  <palves@redhat.com>
 > 
 > 	* gdb.texinfo (Registers): Expand explanation of not-saved
 > 	call-clobbered registers in frames other than the innermost.

Sorry for the followup.
This doesn't have to be part of this patch of course, but IWBN if "i reg"
included a note next to register values obtained via heuristics.
[such values could have a flag set]

It could even be just a general warning printed by "i reg" in
non-inner-most frames, but it feels like the above shouldn't be too hard.

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

[Andrew Burgess]

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

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

[Pedro Alves]

On 09/20/2013 12:03 PM, Andrew Burgess wrote:
> On 19/09/2013 8:15 PM, Pedro Alves wrote:
>> 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.

Luckily, I already have a patch that does that.  :-)

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

Okay.  Reading more code, and investigating the history behind it
convinced me that that is indeed the direction GDB was currently
heading.

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

I had toyed with something like Doug suggested.  See my
other reply to Mark.

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

(skipping answering some of the other points as I believe I've answered
them in my other reply to Mark).

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

Indeed.

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

Hmm.  You're right, but only if you see a sequence/connection between
the sentences.  They're different paragraphs -- the second sentence is
mostly preexisting, and I didn't mean to imply the GDB tries to figure
out where caller-saved/scratch registers are.  I'll rephrase it (in
this or the <not saved> patch, whatever we end up with.  Thanks.

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

Right, that's what I was thinking.  It'd be better to write that out
explicitly.

> From a debugging point of few though, I
> suspect showing the wrong value might be far from harmless, 

No sure why.

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

Blah, what a rat hole.  :-P :-)

Please guys, re-confirm to me the direction you prefer seeing
GDB follow, even after my reasoning further explained (I'm okay
with either approach), and I'll update the corresponding patch's
documentation bits with the suggestions you've sent.

-- 
Pedro Alves

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

[Andrew Burgess]

On 24/09/2013 1:06 PM, Pedro Alves wrote:
> On 09/20/2013 12:03 PM, Andrew Burgess wrote:
>> On 19/09/2013 8:15 PM, Pedro Alves wrote:
>>> 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.
> 
> Luckily, I already have a patch that does that.  :-)
> 
>>> 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.
> 
> Okay.  Reading more code, and investigating the history behind it
> convinced me that that is indeed the direction GDB was currently
> heading.
> 
>>
>>>
>>> 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?
> 
> I had toyed with something like Doug suggested.  See my
> other reply to Mark.




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

> 
> (skipping answering some of the other points as I believe I've answered
> them in my other reply to Mark).
> 
>>> -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).
> 
> Indeed.
> 
>>
>>> +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.
> 
> Hmm.  You're right, but only if you see a sequence/connection between
> the sentences.  They're different paragraphs -- the second sentence is
> mostly preexisting, and I didn't mean to imply the GDB tries to figure
> out where caller-saved/scratch registers are.  I'll rephrase it (in
> this or the <not saved> patch, whatever we end up with.  Thanks.
> 
>>
>>>                                                                       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. 
> 
> Right, that's what I was thinking.  It'd be better to write that out
> explicitly.
> 
>> From a debugging point of few though, I
>> suspect showing the wrong value might be far from harmless, 
> 
> No sure why.

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.

Are there any other examples within gdb that you can think of (I can't)
where we print information that we know is very likely wrong? (Obviously
I mean deliberately here rather than being mislead by invalid debug
information).

> 
>> 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.
> 
> Blah, what a rat hole.  :-P :-)
> 
> Please guys, re-confirm to me the direction you prefer seeing
> GDB follow, even after my reasoning further explained (I'm okay
> with either approach), and I'll update the corresponding patch's
> documentation bits with the suggestions you've sent.

I think the original "<not-saved>" patch was great and a good
improvement on where we are right now, I'd much rather see that merged
than the "values-from-inner-frame" patch.

Thanks for your continued effort :)

Andrew

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

[Pedro Alves]

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.
Doing otherwise means assuming we know the ABI the outer and inner
frames agreed on.  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.

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

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

-- 
Pedro Alves

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

[Andrew Burgess]

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

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

[Pedro Alves]

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

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

[Andrew Burgess]

On 24/09/2013 8:36 PM, Pedro Alves wrote:
> 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.)

OK, I accept the point you make here.  But I still don't think this is
the right patch to apply ....

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

No, I do understand what you're suggesting I just think it's the wrong
thing to do... but if I've not convinced you then I guess my argument
must be pretty weak, and your point about the PC is a good example of
where I'm clearly wrong, still....

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

Honestly, I've always thought it would be nice if the unwound PC pointed
at the call instruction ... I've always just assumed that was "hard" and
lived with what we have.  I didn't think about that when I made my
previous statements otherwise I'd have pre-emptively added an exception
for the PC, doing so now would seem childish :)

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

Indeed.  Looked good to me, except ... see my comments below.

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

OK, so that would seem to suggest that the patch you posted for Mark is
correct and should go in, at least, when debugging a gcc binary, except,
see comments below....

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

You're right "lack" was a bad choice of words and indicated a "lack" of
understanding on my part.  Apologies.

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

I wasn't arguing the rs6000, etc are wrong, rather that different
targets might prefer different behaviours, applying the inner-value
patch I think takes away that choice.

If what we want for x86-64 is the inner-frame behaviour then could we
not just leave things as they are?  The only thing we loose is the
additional "not saved" out-of-band information in "info registers", but
we maintain the behaviour of the dwarf undefined state for any targets
that do want to make use of it.

[ ASIDE: The "not-saved" information could easily be added back in by
specialising the "info register" code for x86-64, a lot of other targets
already specialise. ]

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

By "forgive" I simply meant that the prologue scanners are one of the
places where ABI specific knowledge is most likely to be written into
gdb, and this is probably one of the most acceptable places.

I think the behaviour of the scanners should really be what makes most
sense to each target, to me marking callee clobbered registers undefined
makes more sense... but if the inner-value patch is applied it would
make little difference if we use same-value or undefined as they'll have
the same result ... right?

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

That might indeed be the only way to make us both happy :)

Cheers,
Andrew

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

[Pedro Alves]

OK, I've given this whole thing some time for comments
from others.

As I'm not hearing any support for the point of view I've
presented in the follow up discussions of this patch, I'll
just forget about it, and I'll go apply the original <not saved>
one instead.  That one doesn't really change GDB's behavior,
other than making output a little clearer.
We can always reconsider the other idea if we want to.

-- 
Pedro Alves

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

[Doug Evans]

Pedro Alves writes:
 > OK, I've given this whole thing some time for comments
 > from others.
 > 
 > As I'm not hearing any support for the point of view I've
 > presented in the follow up discussions of this patch, I'll
 > just forget about it, and I'll go apply the original <not saved>
 > one instead.  That one doesn't really change GDB's behavior,
 > other than making output a little clearer.
 > We can always reconsider the other idea if we want to.

Ok by me.

Re: [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]

> Date: Thu, 19 Sep 2013 20:15:20 +0100
> From: Pedro Alves <palves@redhat.com>
> 
> 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.  See with the gdb.dwarf/dw2-op-out-param.exp
> test:
> 
> Breakpoint 2, 0x000000000040058f in breakpt ()
> (gdb) up
> #1  0x00000000004005a2 in int_param_single_reg_loc (operand0=<optimized out>, operand1=-2401054115373400575, operand2=<optimized out>)
> (gdb) info registers
> rax            0x323d7b35b0     215779849648
> rbx            0xdeadbe00deadbe01       -2401054115373400575
> rcx            <optimized out>
> rdx            0xdeadbe04deadbe05       -2401054098193531387
> rsi            <optimized out>
> rdi            <optimized out>
> rbp            0x0      0x0
> rsp            0x7fffffffda20   0x7fffffffda20
> r8             0x323d7b1f40     215779843904
> r9             0x323d00f310     215771837200
> r10            0x7fffffffd890   140737488345232
> r11            0x323d421640     215776106048
> r12            0x400430 4195376
> r13            0x7fffffffdb10   140737488345872
> r14            0x0      0
> r15            0x0      0
> rip            0x4005a2 0x4005a2 <int_param_single_reg_loc+10>
> eflags         0x202    [ IF ]
> cs             0x33     51
> ss             0x2b     43
> ds             0x0      0
> es             0x0      0
> fs             0x0      0
> gs             0x0      0
> 
> 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.
> 
> 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.
> 
> After the patch, we get:
> 
> #0  0x000000000040058f in breakpt ()
> (gdb) info registers
> rax            0x323d7b35b0     215779849648
> rbx            0xdeadbe00deadbe01       -2401054115373400575
> rcx            0xdeadbe02deadbe03       -2401054106783465981
> rdx            0xdeadbe04deadbe05       -2401054098193531387
> rsi            0xdeadbe06deadbe07       -2401054089603596793
> rdi            0xdeadbe08deadbe09       -2401054081013662199
> rbp            0x0      0x0
> rsp            0x7fffffffda18   0x7fffffffda18
> r8             0x323d7b1f40     215779843904
> r9             0x323d00f310     215771837200
> r10            0x7fffffffd890   140737488345232
> r11            0x323d421640     215776106048
> r12            0x400430 4195376
> r13            0x7fffffffdb10   140737488345872
> r14            0x0      0
> r15            0x0      0
> rip            0x40058f 0x40058f <breakpt>
> eflags         0x202    [ IF ]
> cs             0x33     51
> ss             0x2b     43
> ds             0x0      0
> es             0x0      0
> fs             0x0      0
> gs             0x0      0
> (gdb) up
> #1  0x00000000004005a2 in int_param_single_reg_loc (operand0=<optimized out>, operand1=-2401054115373400575, operand2=<optimized out>)
> (gdb) info registers
> rax            0x323d7b35b0     215779849648
> rbx            0xdeadbe00deadbe01       -2401054115373400575
> rcx            0xdeadbe02deadbe03       -2401054106783465981
> rdx            0xdeadbe04deadbe05       -2401054098193531387
> rsi            0xdeadbe06deadbe07       -2401054089603596793
> rdi            0xdeadbe08deadbe09       -2401054081013662199
> rbp            0x0      0x0
> rsp            0x7fffffffda20   0x7fffffffda20
> r8             0x323d7b1f40     215779843904
> r9             0x323d00f310     215771837200
> r10            0x7fffffffd890   140737488345232
> r11            0x323d421640     215776106048
> r12            0x400430 4195376
> r13            0x7fffffffdb10   140737488345872
> r14            0x0      0
> r15            0x0      0
> rip            0x4005a2 0x4005a2 <int_param_single_reg_loc+10>
> eflags         0x202    [ IF ]
> cs             0x33     51
> ss             0x2b     43
> ds             0x0      0
> es             0x0      0
> fs             0x0      0
> gs             0x0      0
> (gdb)
> 
> Which is what you'd get with a newer GCC.  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:
> 
> (gdb) return
> Make breakpt return now? (y or n) y
> #0  0x00000000004005a2 in int_param_single_reg_loc (operand0=-2401054106783465981, operand1=-2401054115373400575,
>     operand2=-2401054089603596793)
> (gdb) info registers
> rax            0x323d7b35b0     215779849648
> rbx            0xdeadbe00deadbe01       -2401054115373400575
> rcx            0xdeadbe02deadbe03       -2401054106783465981
> rdx            0xdeadbe04deadbe05       -2401054098193531387
> rsi            0xdeadbe06deadbe07       -2401054089603596793
> rdi            0xdeadbe08deadbe09       -2401054081013662199
> rbp            0x0      0x0
> rsp            0x7fffffffda20   0x7fffffffda20
> r8             0x323d7b1f40     215779843904
> r9             0x323d00f310     215771837200
> r10            0x7fffffffd890   140737488345232
> r11            0x323d421640     215776106048
> r12            0x400430 4195376
> r13            0x7fffffffdb10   140737488345872
> r14            0x0      0
> r15            0x0      0
> rip            0x4005a2 0x4005a2 <int_param_single_reg_loc+10>
> eflags         0x202    [ IF ]
> cs             0x33     51
> ss             0x2b     43
> ds             0x0      0
> es             0x0      0
> fs             0x0      0
> gs             0x0      0
> (gdb)
> 
> This is rule is applied in value_of_register.  This is the function
> eval.c uses for evaluating OP_REGISTER (again, $reg, $pc, etc.), and
> related bits.  It isn't used for anything else.  The patch makes "info
> registers" and the MI equivalent use it too.  I think it just makes a
> lot of sense, as this makes it so that when printing machine registers
> ($pc, etc.), we go through a central function.
> 
> Note how gdb.mi/mi-reg-undefined.exp and
> gdb.dwarf2/dw2-reg-undefined.exp tests needed adjustment, but not
> gdb.dwarf2/dw2-op-out-param.exp, as that prints an optimized out
> variable, not register.

I strongly disagree with this change.  I think it is useful to show
that rgeisters have not been saved.  If you want to look at their
value anyway, you can always go down (or up) the stack and chase the
value of the register yourself.

I also think it doesn't make sense for the varobj subsystem to
reimplement bits of the unwinder subsystem.

If newer GCC versions are even less explicit about saving or not
saving registers than older versions, we should probably improve the
_dwarf2_frame_init_reg() implementations to indicate properly which
registers are caller-saved and which registers are callee-saved.

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

[Pedro Alves]

On 09/20/2013 01:27 PM, Mark Kettenis wrote:

> I strongly disagree with this change.  

Ack.

My thought process was that it was not that much different from
defining the value of PC in frames other than #0 to be the
return address, not the value the PC had _before_ the call.
If we went further and defined all register values in that
direction, things would all be consistent.  Something like:

  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.

IOW, call-clobbered registers would show whatever value's
current in the inner frame, and callee-saved registers
would show the saved values.

I think _both_ definitions are reasonable, though this
one seemed a little more consistent to me.

> I think it is useful to show that rgeisters have not been saved.

Right, but that could also be done in some other way.  I
toyed with something like Doug suggested.  That is, make
"info reg" indicate whether the register was actually saved
or GDB assumed same_value.  Something like:

(gdb) i reg
rax            0x1 1		[not saved]
rbx            0x0 0
rcx            0x2 2		[not saved]
rdx            0x3 3		[not saved]
rsi            0x4 4		[not saved]
rdi            0x5 5		[not saved]
rbp            0x7fffffffda70   0x7fffffffda70
rsp            0x7fffffffda40   0x7fffffffda40

So GDB would still "flatten" the registers, and "p $rax"
would still print some number, and the user could tell
that the registers' values might no longer be the
values the frame had when it called out the inner frame.

>  If you want to look at their
> value anyway, you can always go down (or up) the stack and chase the
> value of the register yourself.

Of course.  Though not very convenient.  If that's the direction
we're heading, and if GDB gets the saved/not-saved wrong, that
may be a nuisance.  It might be useful to have shorthand for
that (though admittedly the use cases will probably trigger so
seldom that we may never hear requests for it). 

> 
> I also think it doesn't make sense for the varobj subsystem to
> reimplement bits of the unwinder subsystem.

You mean value machinery, I guess.  I wouldn't say reimplement.
The idea was to still leave the unwinder treating not-saved as is,
and only hide that at the user-facing presentation layer, so that
anything that might need to rely on the unwinder machinery getting
info about registers at inner frame call time, wouldn't get possibly
no-longer-correct register values unless it wanted to.  (IOW, we could
move the value_of_register function to stack.c and call it part of
the unwinder machinery as well.)

> If newer GCC versions are even less explicit about saving or not
> saving registers than older versions, we should probably improve the
> _dwarf2_frame_init_reg() implementations to indicate properly which
> registers are caller-saved and which registers are callee-saved.

So...  I tried that out for AMD64, on top of the <not-saved> patch.

(top-gdb) i reg
rax            0x5e60ff 6185215
rbx            0x0      0
rcx            0x323d435bf0     215776189424
rdx            0x7fffffffda50   140737488345680
rsi            0x0      0
rdi            0x7fffffffda50   140737488345680
rbp            0x7fffffffd9a0   0x7fffffffd9a0
rsp            0x7fffffffd860   0x7fffffffd860
r8             0x70     112
r9             0x100000 1048576
r10            0x8      8
r11            0x246    582
r12            0x45aed0 4566736
r13            0x7fffffffdb50   140737488345936
r14            0x0      0
r15            0x0      0
rip            0x5e6114 0x5e6114 <captured_main+21>
eflags         0x206    [ PF IF ]
cs             0x33     51
ss             0x2b     43
ds             0x0      0
es             0x0      0
fs             0x0      0
gs             0x0      0
(top-gdb) up
#1  0x00000000005e3876 in catch_errors (func=0x5e60ff <captured_main>, func_args=0x7fffffffda50, errstring=0x8a6194 "", mask=RETURN_MASK_ALL)
    at ../../src/gdb/exceptions.c:524
524           val = func (func_args);
(top-gdb) i reg
rax            <not saved>
rbx            0x0      0
rcx            <not saved>
rdx            <not saved>
rsi            <not saved>
rdi            <not saved>
rbp            0x7fffffffda10   0x7fffffffda10
rsp            0x7fffffffd9b0   0x7fffffffd9b0
r8             <not saved>
r9             <not saved>
r10            <not saved>
r11            <not saved>
r12            0x45aed0 4566736
r13            0x7fffffffdb50   140737488345936
r14            0x0      0
r15            0x0      0
rip            0x5e3876 0x5e3876 <catch_errors+95>
eflags         <not saved>
cs             <not saved>
ss             <not saved>
ds             <not saved>
es             <not saved>
fs             <not saved>
gs             <not saved>


The patch also teaches the heuristic unwinder about call-clobbered
registers, though I wonder whether we should be assuming much about the ABI
around assembly or no-debug-info functions.  Hmm.

Argh, this thread is getting quite confusing with the back and forth.
Sorry about that.

---
 gdb/amd64-tdep.c                               | 54 +++++++++++++++++++++++++-
 gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp | 31 +++++++++++++--
 gdb/testsuite/gdb.mi/mi-reg-undefined.exp      | 43 ++++++++++++++++----
 3 files changed, 115 insertions(+), 13 deletions(-)

diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 3ab74f0..3ab6058 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -40,6 +40,7 @@
 #include "exceptions.h"
 #include "amd64-tdep.h"
 #include "i387-tdep.h"
+#include "dwarf2-frame.h"
 
 #include "features/i386/amd64.c"
 #include "features/i386/amd64-avx.c"
@@ -1744,6 +1745,28 @@ amd64_alloc_frame_cache (void)
   return cache;
 }
 
+/* Return true if the ABI specifies REGNUM as a callee-saved
+   register.  */
+
+static int
+amd64_register_callee_saved (int regnum)
+{
+  if (regnum == AMD64_RBP_REGNUM
+      || regnum == AMD64_RBX_REGNUM
+      || (AMD64_R12_REGNUM <= regnum && regnum <= AMD64_R15_REGNUM))
+    return 1;
+
+  /* The control bits of the MXCSR register are callee-saved.  */
+  if (regnum == AMD64_MXCSR_REGNUM)
+    return 1;
+
+  /* The x87 control word is callee-saved.  */
+  if (regnum == AMD64_FCTRL_REGNUM)
+    return 1;
+
+  return 0;
+}
+
 /* GCC 4.4 and later, can put code in the prologue to realign the
    stack pointer.  Check whether PC points to such code, and update
    CACHE accordingly.  Return the first instruction after the code
@@ -2423,7 +2446,10 @@ amd64_frame_prev_register (struct frame_info *this_frame, void **this_cache,
     return frame_unwind_got_memory (this_frame, regnum,
 				    cache->saved_regs[regnum]);
 
-  return frame_unwind_got_register (this_frame, regnum, regnum);
+  if (amd64_register_callee_saved (regnum))
+    return frame_unwind_got_register (this_frame, regnum, regnum);
+  else
+    return frame_unwind_got_optimized (this_frame, regnum);
 }
 
 static const struct frame_unwind amd64_frame_unwind =
@@ -2846,6 +2872,30 @@ static const int amd64_record_regmap[] =
   AMD64_DS_REGNUM, AMD64_ES_REGNUM, AMD64_FS_REGNUM, AMD64_GS_REGNUM
 };
 
+/* Implement the "init_reg" dwarf2_frame_ops method.  */
+
+static void
+amd64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
+			     struct dwarf2_frame_state_reg *reg,
+			     struct frame_info *this_frame)
+{
+  /* Mark the PC as the destination for the return address.  */
+  if (regnum == gdbarch_pc_regnum (gdbarch))
+    reg->how = DWARF2_FRAME_REG_RA;
+
+  /* Mark the stack pointer as the call frame address.  */
+  else if (regnum == gdbarch_sp_regnum (gdbarch))
+    reg->how = DWARF2_FRAME_REG_CFA;
+
+  /* The above was taken from the default init_reg in dwarf2-frame.c
+     while the below is AMD64 specific.  */
+
+  else if (amd64_register_callee_saved (regnum))
+    reg->how = DWARF2_FRAME_REG_SAME_VALUE;
+  else
+    reg->how = DWARF2_FRAME_REG_UNDEFINED;
+}
+
 void
 amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -2948,6 +2998,8 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   frame_unwind_append_unwinder (gdbarch, &amd64_frame_unwind);
   frame_base_set_default (gdbarch, &amd64_frame_base);
 
+  dwarf2_frame_set_init_reg (gdbarch, amd64_dwarf2_frame_init_reg);
+
   /* If we have a register mapping, enable the generic core file support.  */
   if (tdep->gregset_reg_offset)
     set_gdbarch_regset_from_core_section (gdbarch,
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
index 44bf8e9..c11b007 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
@@ -44,11 +44,24 @@ for {set f 0} {$f < 3} {incr f} {
 	set pattern_rax_rbx_rcx_info "$hex\\s+$decimal"
 	set pattern_r8_r9_print "$hex"
 	set pattern_r8_r9_info "$hex\\s+$decimal"
+	set pattern_r12_r15_print "$hex"
+	set pattern_r12_r15_info "$hex\\s+$decimal"
     } else {
+	# Even though rbx is callee-saved, these are all explicitly
+	# marked undefined in the debug info.
 	set pattern_rax_rbx_rcx_print "<not saved>"
 	set pattern_rax_rbx_rcx_info "<not saved>"
-	set pattern_r8_r9_print "$hex"
-	set pattern_r8_r9_info "$hex\\s+$decimal"
+
+	# ABI specifies these as caller-saved.  GDB should be
+	# implicitly aware they're not saved, even if the debug info
+	# doesn't explicitly say so (and also doesn't say they were
+	# actually saved).
+	set pattern_r8_r9_print "<not saved>"
+	set pattern_r8_r9_info "<not saved>"
+
+	# ABI specifies these as callee-saved.
+	set pattern_r12_r15_print "$hex"
+	set pattern_r12_r15_info "$hex\\s+$decimal"
     }
 
     # Select frame.
@@ -64,8 +77,18 @@ for {set f 0} {$f < 3} {incr f} {
     gdb_test "p/x \$r8" "$pattern_r8_r9_print" "print \$r8 in frame ${f}"
     gdb_test "p/x \$r9" "$pattern_r8_r9_print" "print \$r9 in frame ${f}"
 
+    gdb_test "p/x \$r12" "$pattern_r12_r15_print" "print \$r12 in frame ${f}"
+    gdb_test "p/x \$r15" "$pattern_r12_r15_print" "print \$r15 in frame ${f}"
 
     # Display register values.
-    gdb_test "info registers rax rbx rcx r8 r9" "rax\\s+${pattern_rax_rbx_rcx_info}\\s*\r\nrbx\\s+${pattern_rax_rbx_rcx_info}\\s*\r\nrcx\\s+${pattern_rax_rbx_rcx_info}\\s*\r\nr8\\s+${pattern_r8_r9_info}\\s*\r\nr9\\s+${pattern_r8_r9_info}\\s*" \
-	"Check values of rax, rbx, rcx, r8, r9 in frame ${f}"
+    send_gdb "info registers rax rbx rcx r8 r9 r12 r15\n"
+    gdb_expect_list "info registers in frame ${f}" "$gdb_prompt $" \
+	[list \
+	     "rax\\s+$pattern_rax_rbx_rcx_info\r\n" \
+	     "rbx\\s+$pattern_rax_rbx_rcx_info\r\n" \
+	     "rcx\\s+$pattern_rax_rbx_rcx_info\r\n" \
+	     "r8\\s+$pattern_r8_r9_info\r\n" \
+	     "r9\\s+$pattern_r8_r9_info\r\n" \
+	     "r12\\s+$pattern_r12_r15_info\r\n" \
+	     "r15\\s+$pattern_r12_r15_info\r\n" ]
 }
diff --git a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
index cb3a716..64fa581 100644
--- a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
+++ b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
@@ -52,20 +52,47 @@ mi_gdb_test "111-stack-list-frames" \
     "111\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"stop_frame\",.*\},frame=\{level=\"1\",addr=\"$hex\",func=\"first_frame\",.*\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",.*\}\\\]" \
     "stack frame listing"
 
-set not_saved_pattern "<not saved>"
-
 for {set f 0} {$f < 3} {incr f} {
     if {${f} == 0} {
-	set pattern_0_1_2 ${hex}
+	set pattern ${hex}
     } else {
-	set pattern_0_1_2 ${not_saved_pattern}
+	set pattern "<not saved>"
     }
 
-    mi_gdb_test "22${f}-data-list-register-values --thread 1 --frame ${f} x  0 1 2 8 9" \
-	"22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
+    # - rax,rbx,rcx are explicitly marked undefined in the debug info,
+    #   so GDB should display them all as not saved, even though rbx
+    #   is callee-saved.
+    #
+    # - r8-r9 are caller-saved.  GDB should be implicitly aware
+    #   they're not saved, even if the debug info doesn't explicitly
+    #   say so (and also doesn't say they were actually saved).
+    #
+    # - r12-r15 are callee-saved.  GDB should be able to find their
+    #   values.
+
+    mi_gdb_test "22${f}-data-list-register-values --thread 1 --frame ${f} x  0 1 2 8 9 12 15" \
+	[join [list \
+		   "22${f}\\^done,register-values=\\\["\
+		   "\{number=\"0\",value=\"${pattern}\"\}," \
+		   "\{number=\"1\",value=\"${pattern}\"\}," \
+		   "\{number=\"2\",value=\"${pattern}\"\}," \
+		   "\{number=\"8\",value=\"${pattern}\"\}," \
+		   "\{number=\"9\",value=\"${pattern}\"\}," \
+		   "\{number=\"12\",value=\"$hex\"\}," \
+		   "\{number=\"15\",value=\"$hex\"\}" \
+		   "\\\]" ] "" ] \
 	"register values, format x, frame ${f}"
 
-    mi_gdb_test "33${f}-data-list-register-values --thread 1 --frame ${f} r 0 1 2 8 9" \
-	"33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
+    mi_gdb_test "33${f}-data-list-register-values --thread 1 --frame ${f} r  0 1 2 8 9 12 15" \
+	[join [list \
+		   "33${f}\\^done,register-values=\\\["\
+		   "\{number=\"0\",value=\"${pattern}\"\}," \
+		   "\{number=\"1\",value=\"${pattern}\"\}," \
+		   "\{number=\"2\",value=\"${pattern}\"\}," \
+		   "\{number=\"8\",value=\"${pattern}\"\}," \
+		   "\{number=\"9\",value=\"${pattern}\"\}," \
+		   "\{number=\"12\",value=\"$hex\"\}," \
+		   "\{number=\"15\",value=\"$hex\"\}" \
+		   "\\\]" ] "" ] \
 	"register values, format r, frame ${f}"
 }
-- 
1.7.11.7

Re: [PATCH+DOC] Print registers not saved in the frame as "<not saved>", instead of "<optimized out>".

[Pedro Alves]

FYI, I've now applied this patch, as below:

-------
Subject: Print registers not saved in the frame as "<not saved>" instead of "<optimized out>".

Currently, in some scenarios, GDB prints <optimized out> when printing
outer frame registers.  An <optimized out> register is a confusing
concept.  What this really means is that the register is
call-clobbered, or IOW, not saved by the callee.  This patch makes GDB
say that instead.

Before patch:

 (gdb) p/x $rax $1 = <optimized out>
 (gdb) info registers rax
 rax            <optimized out>

After patch:

 (gdb) p/x $rax
 $1 = <not saved>
 (gdb) info registers rax
 rax            <not saved>

However, if for some reason the debug info describes a variable as
being in such a register (**), we still want to print <optimized out>
when printing the variable.  IOW, <not saved> is reserved for
inspecting registers at the machine level.  The patch uses
lval_register+optimized_out to encode the not saved registers, and
makes it so that optimized out variables always end up in
!lval_register values.

** See <https://sourceware.org/ml/gdb-patches/2012-08/msg00787.html>.
Current/recent enough GCC doesn't mark variables/arguments as being in
call-clobbered registers in the ranges corresponding to function
calls, while older GCCs did.  Newer GCCs will just not say where the
variable is, so GDB will end up realizing the variable is optimized
out.

frame_unwind_got_optimized creates not_lval optimized out registers,
so by default, in most cases, we'll see <optimized out>.

value_of_register is the function eval.c uses for evaluating
OP_REGISTER (again, $pc, etc.), and related bits.  It isn't used for
anything else.  This function makes sure to return lval_register
values.  The patch makes "info registers" and the MI equivalent use it
too.  I think it just makes a lot of sense, as this makes it so that
when printing machine registers ($pc, etc.), we go through a central
function.

We're likely to need a different encoding at some point, if/when we
support partially saved registers.  Even then, I think
value_of_register will still be the spot to tag the intention to print
machine register values differently.

value_from_register however may also return optimized out
lval_register values, so at a couple places where we're computing a
variable's location from a dwarf expression, we convert the resulting
value away from lval_register to a regular optimized out value.

Tested on x86_64 Fedora 17

gdb/
2013-10-02  Pedro Alves  <palves@redhat.com>

	* cp-valprint.c (cp_print_value_fields): Adjust calls to
	val_print_optimized_out.
	* jv-valprint.c (java_print_value_fields): Likewise.
	* p-valprint.c (pascal_object_print_value_fields): Likewise.
	* dwarf2loc.c (dwarf2_evaluate_loc_desc_full)
	<DWARF_VALUE_REGISTER>: If the register was not saved, return a
	new optimized out value.
	* findvar.c (address_from_register): Likewise.
	* frame.c (put_frame_register): Tweak error string to say the
	register was not saved, rather than optimized out.
	* infcmd.c (default_print_one_register_info): Adjust call to
	val_print_optimized_out.  Use value_of_register instead of
	get_frame_register_value.
	* mi/mi-main.c (output_register): Use value_of_register instead of
	get_frame_register_value.
	* valprint.c (valprint_check_validity): Likewise.
	(val_print_optimized_out): New value parameter.  If the value is
	lval_register, print <not saved> instead.
	(value_check_printable, val_print_scalar_formatted): Adjust calls
	to val_print_optimized_out.
	* valprint.h (val_print_optimized_out): New value parameter.
	* value.c (struct value) <optimized_out>: Extend comment.
	(error_value_optimized_out): New function.
	(require_not_optimized_out): Use it.  Use a different string for
	lval_register values.
	* value.h (error_value_optimized_out): New declaration.
	* NEWS: Mention <not saved>.

gdb/testsuite/
2013-10-02  Pedro Alves  <palves@redhat.com>

	* gdb.dwarf2/dw2-reg-undefined.exp <pattern_rax_rbx_rcx_print,
	pattern_rax_rbx_rcx_info>: Set to "<not saved>".
	* gdb.mi/mi-reg-undefined.exp (opt_out_pattern): Delete.
	(not_saved_pattern): New.
	Replace use of the former with the latter.

gdb/doc/
2013-10-02  Pedro Alves  <palves@redhat.com>

	* gdb.texinfo (Registers): Expand description of saved registers
	in frames.  Explain <not saved>.
---

 gdb/NEWS                                       |   14 +++++++++++
 gdb/cp-valprint.c                              |    4 ++-
 gdb/doc/gdb.texinfo                            |   31 +++++++++++++++++++++---
 gdb/dwarf2loc.c                                |   16 ++++++++++--
 gdb/findvar.c                                  |    9 +++++++
 gdb/frame.c                                    |    2 +-
 gdb/infcmd.c                                   |    4 ++-
 gdb/jv-valprint.c                              |    4 ++-
 gdb/mi/mi-main.c                               |    2 +-
 gdb/p-valprint.c                               |    4 ++-
 gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp |    4 ++-
 gdb/testsuite/gdb.mi/mi-reg-undefined.exp      |    4 ++-
 gdb/valprint.c                                 |   13 ++++++----
 gdb/valprint.h                                 |    3 ++
 gdb/value.c                                    |   22 +++++++++++++++--
 gdb/value.h                                    |    5 ++++
 16 files changed, 111 insertions(+), 30 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index af06a21..c0fe72a 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -15,6 +15,20 @@
 
 * The "catch syscall" command now works on arm*-linux* targets.
 
+* GDB now consistently shows "<not saved>" when printing values of
+  registers the debug info indicates have not been saved in the frame
+  and there's nowhere to retrieve them from
+  (callee-saved/call-clobbered registers):
+
+    (gdb) p $rax
+    $1 = <not saved>
+
+    (gdb) info registers rax
+    rax            <not saved>
+
+  Before, the former would print "<optimized out>", and the latter
+  "*value not available*".
+
 * Python scripting
 
   ** Frame filters and frame decorators have been added.
diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
index e83d979..1d7147c 100644
--- a/gdb/cp-valprint.c
+++ b/gdb/cp-valprint.c
@@ -298,7 +298,7 @@ cp_print_value_fields (struct type *type, struct type *real_type,
 					  TYPE_FIELD_BITPOS (type, i),
 					  TYPE_FIELD_BITSIZE (type, i)))
 		{
-		  val_print_optimized_out (stream);
+		  val_print_optimized_out (val, stream);
 		}
 	      else
 		{
@@ -334,7 +334,7 @@ cp_print_value_fields (struct type *type, struct type *real_type,
 				      _("<error reading variable: %s>"),
 				      ex.message);
 		  else if (v == NULL)
-		    val_print_optimized_out (stream);
+		    val_print_optimized_out (NULL, stream);
 		  else
 		    cp_print_static_field (TYPE_FIELD_TYPE (type, i),
 					   v, stream, recurse + 1,
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 60d2877..ffee6e1 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -10031,10 +10031,33 @@ 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.
+@cindex caller-saved registers
+@cindex call-clobbered registers
+@cindex volatile registers
+@cindex <not saved> values
+Usually ABIs reserve some registers as not needed to be saved by the
+callee (a.k.a.: ``caller-saved'', ``call-clobbered'' or ``volatile''
+registers).  It may therefore not be possible for @value{GDBN} to know
+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 the inner frame saved
+(``callee-saved'') registers, from the debug info, unwind info, or the
+machine code generated by your compiler.  If some register is not
+saved, and @value{GDBN} knows the register is ``caller-saved'' (via
+its own knowledge of the ABI, or because the debug/unwind info
+explicitly says the register's value is undefined), @value{GDBN}
+displays @w{@samp{<not saved>}} as the register's value.  With targets
+that @value{GDBN} has no knowledge of the register saving convention,
+if a register was not saved by the callee, then its value and location
+in the outer frame are assumed to be the same of the inner frame.
+This is usually harmless, because if the register is call-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.  Note,
+however, that if you change such a register in the outer frame, you
+may also be affecting the inner frame.  Also, the more ``outer'' the
+frame is you're looking at, the more likely a call-clobbered
+register's value is to be wrong, in the sense that it doesn't actually
+represent the value the register had just before the call.
 
 @node Floating Point Hardware
 @section Floating Point Hardware
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 55d43f1..1313e17 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -2290,11 +2290,21 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
 	    if (byte_offset != 0)
 	      error (_("cannot use offset on synthetic pointer to register"));
 	    do_cleanups (value_chain);
-	    if (gdb_regnum != -1)
-	      retval = value_from_register (type, gdb_regnum, frame);
-	    else
+	   if (gdb_regnum == -1)
 	      error (_("Unable to access DWARF register number %d"),
 		     dwarf_regnum);
+	   retval = value_from_register (type, gdb_regnum, frame);
+	   if (value_optimized_out (retval))
+	     {
+	       /* This means the register has undefined value / was
+		  not saved.  As we're computing the location of some
+		  variable etc. in the program, not a value for
+		  inspecting a register ($pc, $sp, etc.), return a
+		  generic optimized out value instead, so that we show
+		  <optimized out> instead of <not saved>.  */
+	       do_cleanups (value_chain);
+	       retval = allocate_optimized_out_value (type);
+	     }
 	  }
 	  break;
 
diff --git a/gdb/findvar.c b/gdb/findvar.c
index 25242be..c3550b4 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -757,6 +757,15 @@ address_from_register (struct type *type, int regnum, struct frame_info *frame)
   value = value_from_register (type, regnum, frame);
   gdb_assert (value);
 
+  if (value_optimized_out (value))
+    {
+      /* This function is used while computing a location expression.
+	 Complain about the value being optimized out, rather than
+	 letting value_as_address complain about some random register
+	 the expression depends on not being saved.  */
+      error_value_optimized_out ();
+    }
+
   result = value_as_address (value);
   release_value (value);
   value_free (value);
diff --git a/gdb/frame.c b/gdb/frame.c
index d52c26a..be1a1b1 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1140,7 +1140,7 @@ put_frame_register (struct frame_info *frame, int regnum,
   frame_register (frame, regnum, &optim, &unavail,
 		  &lval, &addr, &realnum, NULL);
   if (optim)
-    error (_("Attempt to assign to a value that was optimized out."));
+    error (_("Attempt to assign to a register that was not saved."));
   switch (lval)
     {
     case lval_memory:
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index e29dcde..46102a5 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2035,7 +2035,7 @@ default_print_one_register_info (struct ui_file *file,
     }
   else if (value_optimized_out (val))
     {
-      val_print_optimized_out (file);
+      val_print_optimized_out (val, file);
       fprintf_filtered (file, "\n");
       return;
     }
@@ -2142,7 +2142,7 @@ default_print_registers_info (struct gdbarch *gdbarch,
 
       default_print_one_register_info (file,
 				       gdbarch_register_name (gdbarch, i),
-				       get_frame_register_value (frame, i));
+				       value_of_register (i, frame));
     }
 }
 
diff --git a/gdb/jv-valprint.c b/gdb/jv-valprint.c
index 3b90e54..cb89a85 100644
--- a/gdb/jv-valprint.c
+++ b/gdb/jv-valprint.c
@@ -395,7 +395,7 @@ java_print_value_fields (struct type *type, const gdb_byte *valaddr,
 	      else if (!value_bits_valid (val, TYPE_FIELD_BITPOS (type, i),
 					  TYPE_FIELD_BITSIZE (type, i)))
 		{
-		  val_print_optimized_out (stream);
+		  val_print_optimized_out (val, stream);
 		}
 	      else
 		{
@@ -420,7 +420,7 @@ java_print_value_fields (struct type *type, const gdb_byte *valaddr,
 		  struct value *v = value_static_field (type, i);
 
 		  if (v == NULL)
-		    val_print_optimized_out (stream);
+		    val_print_optimized_out (NULL, stream);
 		  else
 		    {
 		      struct value_print_options opts;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index e8c4744..5cde1ae 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1161,7 +1161,7 @@ output_register (struct frame_info *frame, int regnum, int format,
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
   struct ui_out *uiout = current_uiout;
-  struct value *val = get_frame_register_value (frame, regnum);
+  struct value *val = value_of_register (regnum, frame);
   struct cleanup *tuple_cleanup;
   struct value_print_options opts;
   struct ui_file *stb;
diff --git a/gdb/p-valprint.c b/gdb/p-valprint.c
index 05d4c6f..e6d4b91 100644
--- a/gdb/p-valprint.c
+++ b/gdb/p-valprint.c
@@ -629,7 +629,7 @@ pascal_object_print_value_fields (struct type *type, const gdb_byte *valaddr,
 	      else if (!value_bits_valid (val, TYPE_FIELD_BITPOS (type, i),
 					  TYPE_FIELD_BITSIZE (type, i)))
 		{
-		  val_print_optimized_out (stream);
+		  val_print_optimized_out (val, stream);
 		}
 	      else
 		{
@@ -657,7 +657,7 @@ pascal_object_print_value_fields (struct type *type, const gdb_byte *valaddr,
 		  v = value_field_bitfield (type, i, valaddr, offset, val);
 
 		  if (v == NULL)
-		    val_print_optimized_out (stream);
+		    val_print_optimized_out (NULL, stream);
 		  else
 		    pascal_object_print_static_field (v, stream, recurse + 1,
 						      options);
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
index 4686648..44bf8e9 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
@@ -45,8 +45,8 @@ for {set f 0} {$f < 3} {incr f} {
 	set pattern_r8_r9_print "$hex"
 	set pattern_r8_r9_info "$hex\\s+$decimal"
     } else {
-	set pattern_rax_rbx_rcx_print "<optimized out>"
-	set pattern_rax_rbx_rcx_info "<optimized out>"
+	set pattern_rax_rbx_rcx_print "<not saved>"
+	set pattern_rax_rbx_rcx_info "<not saved>"
 	set pattern_r8_r9_print "$hex"
 	set pattern_r8_r9_info "$hex\\s+$decimal"
     }
diff --git a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
index 8bcbb21..cb3a716 100644
--- a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
+++ b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
@@ -52,13 +52,13 @@ mi_gdb_test "111-stack-list-frames" \
     "111\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"stop_frame\",.*\},frame=\{level=\"1\",addr=\"$hex\",func=\"first_frame\",.*\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",.*\}\\\]" \
     "stack frame listing"
 
-set opt_out_pattern "<optimized out>"
+set not_saved_pattern "<not saved>"
 
 for {set f 0} {$f < 3} {incr f} {
     if {${f} == 0} {
 	set pattern_0_1_2 ${hex}
     } else {
-	set pattern_0_1_2 ${opt_out_pattern}
+	set pattern_0_1_2 ${not_saved_pattern}
     }
 
     mi_gdb_test "22${f}-data-list-register-values --thread 1 --frame ${f} x  0 1 2 8 9" \
diff --git a/gdb/valprint.c b/gdb/valprint.c
index 0f6d65e..61492cc 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -314,7 +314,7 @@ valprint_check_validity (struct ui_file *stream,
       if (!value_bits_valid (val, TARGET_CHAR_BIT * embedded_offset,
 			     TARGET_CHAR_BIT * TYPE_LENGTH (type)))
 	{
-	  val_print_optimized_out (stream);
+	  val_print_optimized_out (val, stream);
 	  return 0;
 	}
 
@@ -336,9 +336,12 @@ valprint_check_validity (struct ui_file *stream,
 }
 
 void
-val_print_optimized_out (struct ui_file *stream)
+val_print_optimized_out (const struct value *val, struct ui_file *stream)
 {
-  fprintf_filtered (stream, _("<optimized out>"));
+  if (val != NULL && value_lval_const (val) == lval_register)
+    fprintf_filtered (stream, _("<not saved>"));
+  else
+    fprintf_filtered (stream, _("<optimized out>"));
 }
 
 void
@@ -805,7 +808,7 @@ value_check_printable (struct value *val, struct ui_file *stream,
       if (options->summary && !val_print_scalar_type_p (value_type (val)))
 	fprintf_filtered (stream, "...");
       else
-	val_print_optimized_out (stream);
+	val_print_optimized_out (val, stream);
       return 0;
     }
 
@@ -966,7 +969,7 @@ val_print_scalar_formatted (struct type *type,
      printed, because all bits contribute to its representation.  */
   if (!value_bits_valid (val, TARGET_CHAR_BIT * embedded_offset,
 			      TARGET_CHAR_BIT * TYPE_LENGTH (type)))
-    val_print_optimized_out (stream);
+    val_print_optimized_out (val, stream);
   else if (!value_bytes_available (val, embedded_offset, TYPE_LENGTH (type)))
     val_print_unavailable (stream);
   else
diff --git a/gdb/valprint.h b/gdb/valprint.h
index e7073b6..1d86ed7 100644
--- a/gdb/valprint.h
+++ b/gdb/valprint.h
@@ -160,7 +160,8 @@ extern int read_string (CORE_ADDR addr, int len, int width,
 			enum bfd_endian byte_order, gdb_byte **buffer,
 			int *bytes_read);
 
-extern void val_print_optimized_out (struct ui_file *stream);
+extern void val_print_optimized_out (const struct value *val,
+				     struct ui_file *stream);
 
 extern void val_print_unavailable (struct ui_file *stream);
 
diff --git a/gdb/value.c b/gdb/value.c
index bc1239d..d96d285 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -197,8 +197,13 @@ struct value
      reset, be sure to consider this use as well!  */
   unsigned int lazy : 1;
 
-  /* If nonzero, this is the value of a variable which does not
-     actually exist in the program.  */
+  /* If nonzero, this is the value of a variable that does not
+     actually exist in the program.  If nonzero, and LVAL is
+     lval_register, this is a register ($pc, $sp, etc., never a
+     program variable) that has not been saved in the frame.  All
+     optimized-out values are treated pretty much the same, except
+     registers have a different string representation and related
+     error strings.  */
   unsigned int optimized_out : 1;
 
   /* If value is a variable, is it initialized or not.  */
@@ -902,11 +907,22 @@ value_actual_type (struct value *value, int resolve_simple_types,
   return result;
 }
 
+void
+error_value_optimized_out (void)
+{
+  error (_("value has been optimized out"));
+}
+
 static void
 require_not_optimized_out (const struct value *value)
 {
   if (value->optimized_out)
-    error (_("value has been optimized out"));
+    {
+      if (value->lval == lval_register)
+	error (_("register has not been saved in frame"));
+      else
+	error_value_optimized_out ();
+    }
 }
 
 static void
diff --git a/gdb/value.h b/gdb/value.h
index 98dbadf..db964e3 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -273,6 +273,11 @@ extern void set_value_lazy (struct value *value, int val);
 extern int value_stack (struct value *);
 extern void set_value_stack (struct value *value, int val);
 
+/* Throw an error complaining that the value has been optimized
+   out.  */
+
+extern void error_value_optimized_out (void);
+
 /* value_contents() and value_contents_raw() both return the address
    of the gdb buffer used to hold a copy of the contents of the lval.
    value_contents() is used when the contents of the buffer are needed

Re: [PATCH+DOC] Print registers not saved in the frame as "<not saved>", instead of "<optimized out>".

[Andreas Schwab]

Pedro Alves <palves@redhat.com> writes:

> diff --git a/gdb/NEWS b/gdb/NEWS
> index af06a21..b06f90c 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -15,6 +15,15 @@
>  
>  * The "catch syscall" command now works on arm*-linux* targets.
>  
> +* GDB now shows "<not saved>" when printing values of registers that
> +  have not been saved in the frame:
> +
> +    (gdb) p $rax $1 = <not saved>
                   ^s/ /\n/
> +    (gdb) info registers rax rax <not saved>
                               ^s/ /\n/

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH+DOC] Print registers not saved in the frame as "<not saved>", instead of "<optimized out>".

[Pedro Alves]

On 09/18/2013 05:30 PM, Andreas Schwab wrote:
> Pedro Alves <palves@redhat.com> writes:

>> +* GDB now shows "<not saved>" when printing values of registers that
>> +  have not been saved in the frame:
>> +
>> +    (gdb) p $rax $1 = <not saved>
>                    ^s/ /\n/
>> +    (gdb) info registers rax rax <not saved>
>                                ^s/ /\n/
> 

Thanks.  Fixed in the new version.

-- 
Pedro Alves