|
Jan Kratochvil |
a972d47 |
http://sourceware.org/ml/gdb-patches/2014-07/msg00530.html
|
|
Jan Kratochvil |
a972d47 |
Subject: [read_frame_arg patch] Handle partially optimized out values similarly to unavailable values (Re: [patchv2] Fix crash on optimized-out entry data values)
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
--V88s5gaDVPzZ0KCq
|
|
Jan Kratochvil |
a972d47 |
Content-Type: text/plain; charset=us-ascii
|
|
Jan Kratochvil |
a972d47 |
Content-Disposition: inline
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
On Thu, 17 Jul 2014 14:23:06 +0200, Pedro Alves wrote:
|
|
Jan Kratochvil |
a972d47 |
> On 07/16/2014 10:58 PM, Jan Kratochvil wrote:
|
|
Jan Kratochvil |
a972d47 |
> > This patch is apparently not suitable for gdb-7.8 which is I guess often
|
|
Jan Kratochvil |
a972d47 |
> > crashing on -O2 -g entry values so there could be some rather minimal crash
|
|
Jan Kratochvil |
a972d47 |
> > avoiding patch instead.
|
|
Jan Kratochvil |
a972d47 |
>
|
|
Jan Kratochvil |
a972d47 |
> Yeah.
|
|
Jan Kratochvil |
a972d47 |
>
|
|
Jan Kratochvil |
a972d47 |
> So this was originally "caused" (more exposed) by 4f14910f:
|
|
Jan Kratochvil |
a972d47 |
>
|
|
Jan Kratochvil |
a972d47 |
> gdb/ChangeLog
|
|
Jan Kratochvil |
a972d47 |
> 2013-11-26 Andrew Burgess <aburgess@broadcom.com>
|
|
Jan Kratochvil |
a972d47 |
>
|
|
Jan Kratochvil |
a972d47 |
> * value.c (allocate_optimized_out_value): Mark value as non-lazy.
|
|
Jan Kratochvil |
a972d47 |
>
|
|
Jan Kratochvil |
a972d47 |
> I tried a few approaches in value_available_contents_eq
|
|
Jan Kratochvil |
a972d47 |
> today, and ended up thinking that the simplest should be to
|
|
Jan Kratochvil |
a972d47 |
> just revert that patch until we have the fuller fix in place.
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
OK, that seems as the best solution for 7.8 to me.
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
> While doing just that fixes the crash, it surprisingly causes
|
|
Jan Kratochvil |
a972d47 |
> one of your new tests to FAIL:
|
|
Jan Kratochvil |
a972d47 |
>
|
|
Jan Kratochvil |
a972d47 |
> (gdb) frame
|
|
Jan Kratochvil |
a972d47 |
> #0 bar (ref=ref@entry=@0x7fffffffd184: 10) at gdb.arch/amd64-entry-value-paramref.cc:23
|
|
Jan Kratochvil |
a972d47 |
> 23 vv++; /* break-here */
|
|
Jan Kratochvil |
a972d47 |
> (gdb) FAIL: gdb.arch/amd64-entry-value-paramref.exp: frame
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
There is a bug in that entry value code of mine, fix attached.
|
|
Jan Kratochvil |
a972d47 |
The testcase then PASSes with the reverted optimization by Andrew Burgess.
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
For the attached fix - if you nitpick the missing conditional case:
|
|
Jan Kratochvil |
a972d47 |
value_optimized_out (val_deref) && value_optimized_out (entryval_deref)
|
|
Jan Kratochvil |
a972d47 |
It is not detected there but that IMO does not matter much as
|
|
Jan Kratochvil |
a972d47 |
* It is for 7.8 only, for trunk it will get compared correctly thanks to the
|
|
Jan Kratochvil |
a972d47 |
new implementation of value_available_contents_eq()
|
|
Jan Kratochvil |
a972d47 |
called value_contents_eq().
|
|
Jan Kratochvil |
a972d47 |
* If the conditional
|
|
Jan Kratochvil |
a972d47 |
if (val != val_deref
|
|
Jan Kratochvil |
a972d47 |
&& !value_optimized_out (val_deref)
|
|
Jan Kratochvil |
a972d47 |
&& !value_optimized_out (entryval_deref)
|
|
Jan Kratochvil |
a972d47 |
&& value_available_contents_eq (val_deref, 0,
|
|
Jan Kratochvil |
a972d47 |
entryval_deref, 0,
|
|
Jan Kratochvil |
a972d47 |
TYPE_LENGTH (type_deref)))
|
|
Jan Kratochvil |
a972d47 |
val_equal = 1;
|
|
Jan Kratochvil |
a972d47 |
fails it may just print
|
|
Jan Kratochvil |
a972d47 |
bar (ref=@0x7fffffffd904: <optimized out>, ref@entry=@0x7fffffffd904: <optimized out>)
|
|
Jan Kratochvil |
a972d47 |
(or some variant with some partially optimized-out/unavailable parts)
|
|
Jan Kratochvil |
a972d47 |
instead of the more correct
|
|
Jan Kratochvil |
a972d47 |
bar (ref=ref@entry=@0x7fffffffd904: <optimized out>)
|
|
Jan Kratochvil |
a972d47 |
which is not much a bug.
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
The attached fix no longe makes sense after the new implementation
|
|
Jan Kratochvil |
a972d47 |
of value_available_contents_eq() called value_contents_eq() gets applied as it
|
|
Jan Kratochvil |
a972d47 |
handles all the optimized-out/unavailable values on its own, therefore the
|
|
Jan Kratochvil |
a972d47 |
attached patch is really only for 7.8.
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
> Turns out it's the code disabled in value_of_dwarf_reg_entry:
|
|
Jan Kratochvil |
a972d47 |
>
|
|
Jan Kratochvil |
a972d47 |
> target_val = dwarf_entry_parameter_to_value (parameter,
|
|
Jan Kratochvil |
a972d47 |
> TYPE_LENGTH (target_type),
|
|
Jan Kratochvil |
a972d47 |
> target_type, caller_frame,
|
|
Jan Kratochvil |
a972d47 |
> caller_per_cu);
|
|
Jan Kratochvil |
a972d47 |
>
|
|
Jan Kratochvil |
a972d47 |
> /* value_as_address dereferences TYPE_CODE_REF. */
|
|
Jan Kratochvil |
a972d47 |
> addr = extract_typed_address (value_contents (outer_val), checked_type);
|
|
Jan Kratochvil |
a972d47 |
>
|
|
Jan Kratochvil |
a972d47 |
> /* The target entry value has artificial address of the entry value
|
|
Jan Kratochvil |
a972d47 |
> reference. */
|
|
Jan Kratochvil |
a972d47 |
> VALUE_LVAL (target_val) = lval_memory;
|
|
Jan Kratochvil |
a972d47 |
> set_value_address (target_val, addr);
|
|
Jan Kratochvil |
a972d47 |
>
|
|
Jan Kratochvil |
a972d47 |
> It looks quite wrong to me to just change a value's lval like that.
|
|
Jan Kratochvil |
a972d47 |
>
|
|
Jan Kratochvil |
a972d47 |
> I ran the testsuite with that code disabled (like in the patch below),
|
|
Jan Kratochvil |
a972d47 |
> and that caused no regressions. I can't say I really understand the
|
|
Jan Kratochvil |
a972d47 |
> intention here though. What would we be missing if we removed that code?
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
I cannot reproduce any wrong case having the code above #if 0-ed.
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
I just do not find it correct to have it disabled. But at the same time I do
|
|
Jan Kratochvil |
a972d47 |
like much / I do not find correct the code myself. It is a bit problematic to
|
|
Jan Kratochvil |
a972d47 |
have struct value describing a memory content which is no longer present
|
|
Jan Kratochvil |
a972d47 |
there.
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
What happens there:
|
|
Jan Kratochvil |
a972d47 |
------------------------------------------------------------------------------
|
|
Jan Kratochvil |
a972d47 |
volatile int vv;
|
|
Jan Kratochvil |
a972d47 |
static __attribute__((noinline)) int
|
|
Jan Kratochvil |
a972d47 |
bar (int &ref) {
|
|
Jan Kratochvil |
a972d47 |
ref = 20;
|
|
Jan Kratochvil |
a972d47 |
vv++; /* break-here */
|
|
Jan Kratochvil |
a972d47 |
return ref;
|
|
Jan Kratochvil |
a972d47 |
}
|
|
Jan Kratochvil |
a972d47 |
int main (void) {
|
|
Jan Kratochvil |
a972d47 |
int var = 10;
|
|
Jan Kratochvil |
a972d47 |
return bar (var);
|
|
Jan Kratochvil |
a972d47 |
}
|
|
Jan Kratochvil |
a972d47 |
------------------------------------------------------------------------------
|
|
Jan Kratochvil |
a972d47 |
<4><c7>: Abbrev Number: 13 (DW_TAG_GNU_call_site_parameter)
|
|
Jan Kratochvil |
a972d47 |
<c8> DW_AT_location : 1 byte block: 55 (DW_OP_reg5 (rdi))
|
|
Jan Kratochvil |
a972d47 |
<ca> DW_AT_GNU_call_site_value: 2 byte block: 91 74 (DW_OP_fbreg: -12)
|
|
Jan Kratochvil |
a972d47 |
<cd> DW_AT_GNU_call_site_data_value: 1 byte block: 3a (DW_OP_lit10)
|
|
Jan Kratochvil |
a972d47 |
------------------------------------------------------------------------------
|
|
Jan Kratochvil |
a972d47 |
gdb -ex 'b value_addr' -ex r --args ../gdb ./1 -ex 'watch vv' -ex r -ex 'p &ref@entry'
|
|
Jan Kratochvil |
a972d47 |
->
|
|
Jan Kratochvil |
a972d47 |
6 return ref;
|
|
Jan Kratochvil |
a972d47 |
bar (ref=@0x7fffffffd944: 20, ref@entry=@0x7fffffffd944: 10) at 1.C:25
|
|
Jan Kratochvil |
a972d47 |
------------------------------------------------------------------------------
|
|
Jan Kratochvil |
a972d47 |
At /* break-here */ struct value variable 'ref' is TYPE_CODE_REF.
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
With FSF GDB HEAD:
|
|
Jan Kratochvil |
a972d47 |
(gdb) x/gx arg1.contents
|
|
Jan Kratochvil |
a972d47 |
0x6004000a4ad0: 0x00007fffffffd944
|
|
Jan Kratochvil |
a972d47 |
(gdb) p ((struct value *)arg1.location.computed.closure).lval
|
|
Jan Kratochvil |
a972d47 |
$1 = lval_memory
|
|
Jan Kratochvil |
a972d47 |
(gdb) p/x ((struct value *)arg1.location.computed.closure).location.address
|
|
Jan Kratochvil |
a972d47 |
$3 = 0x7fffffffd944
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
With your #if0-ed code:
|
|
Jan Kratochvil |
a972d47 |
(gdb) x/gx arg1.contents
|
|
Jan Kratochvil |
a972d47 |
0x6004000a4ad0: 0x00007fffffffd944
|
|
Jan Kratochvil |
a972d47 |
(gdb) p ((struct value *)arg1.location.computed.closure).lval
|
|
Jan Kratochvil |
a972d47 |
$8 = not_lval
|
|
Jan Kratochvil |
a972d47 |
(gdb) p/x ((struct value *)arg1.location.computed.closure).location.address
|
|
Jan Kratochvil |
a972d47 |
$9 = 0x0
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
I do not see how to access
|
|
Jan Kratochvil |
a972d47 |
((struct value *)arg1.location.computed.closure).location.address
|
|
Jan Kratochvil |
a972d47 |
from GDB CLI. Trying
|
|
Jan Kratochvil |
a972d47 |
(gdb) p &ref@entry
|
|
Jan Kratochvil |
a972d47 |
will invoke value_addr()'s:
|
|
Jan Kratochvil |
a972d47 |
if (TYPE_CODE (type) == TYPE_CODE_REF)
|
|
Jan Kratochvil |
a972d47 |
/* Copy the value, but change the type from (T&) to (T*). We
|
|
Jan Kratochvil |
a972d47 |
keep the same location information, which is efficient, and
|
|
Jan Kratochvil |
a972d47 |
allows &(&X) to get the location containing the reference. */
|
|
Jan Kratochvil |
a972d47 |
and therefore the address gets fetched already from
|
|
Jan Kratochvil |
a972d47 |
arg1.contents
|
|
Jan Kratochvil |
a972d47 |
and not from
|
|
Jan Kratochvil |
a972d47 |
((struct value *)arg1.location.computed.closure).location.address
|
|
Jan Kratochvil |
a972d47 |
.
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
And for any other type than TYPE_CODE_REF this code you #if 0-ed does not get
|
|
Jan Kratochvil |
a972d47 |
executed at all. This DW_AT_GNU_call_site_data_value DWARF was meant
|
|
Jan Kratochvil |
a972d47 |
primarily for Fortran but with -O0 entry values do not get produced
|
|
Jan Kratochvil |
a972d47 |
and with -Og and higher Fortran always optimizes out the passing by reference.
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
If you do not like the #if 0 code there I am OK with removing it as I do not
|
|
Jan Kratochvil |
a972d47 |
know how to make it's use reproducible for user anyway. In the worst case
|
|
Jan Kratochvil |
a972d47 |
- if there really is some way how to exploit it - one should just get
|
|
Jan Kratochvil |
a972d47 |
Attempt to take address of value not located in memory.
|
|
Jan Kratochvil |
a972d47 |
instead of some wrong value and it may be easy to fix then.
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
Thanks for the analysis,
|
|
Jan Kratochvil |
a972d47 |
Jan
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
--V88s5gaDVPzZ0KCq
|
|
Jan Kratochvil |
a972d47 |
Content-Type: text/plain; charset=us-ascii
|
|
Jan Kratochvil |
a972d47 |
Content-Disposition: inline; filename=1
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
gdb/
|
|
Jan Kratochvil |
a972d47 |
2014-07-20 Jan Kratochvil <jan.kratochvil@redhat.com>
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
* stack.c (read_frame_arg): Verify value_optimized_out before calling
|
|
Jan Kratochvil |
a972d47 |
value_available_contents_eq.
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
diff --git a/gdb/stack.c b/gdb/stack.c
|
|
Jan Kratochvil |
a972d47 |
index 0d6d8e7..4db5df5 100644
|
|
Jan Kratochvil |
a972d47 |
--- a/gdb/stack.c
|
|
Jan Kratochvil |
a972d47 |
+++ b/gdb/stack.c
|
|
Jan Kratochvil |
a972d47 |
@@ -413,6 +413,8 @@ read_frame_arg (struct symbol *sym, struct frame_info *frame,
|
|
Jan Kratochvil |
a972d47 |
/* If the reference addresses match but dereferenced
|
|
Jan Kratochvil |
a972d47 |
content does not match print them. */
|
|
Jan Kratochvil |
a972d47 |
if (val != val_deref
|
|
Jan Kratochvil |
a972d47 |
+ && !value_optimized_out (val_deref)
|
|
Jan Kratochvil |
a972d47 |
+ && !value_optimized_out (entryval_deref)
|
|
Jan Kratochvil |
a972d47 |
&& value_available_contents_eq (val_deref, 0,
|
|
Jan Kratochvil |
a972d47 |
entryval_deref, 0,
|
|
Jan Kratochvil |
a972d47 |
TYPE_LENGTH (type_deref)))
|
|
Jan Kratochvil |
a972d47 |
|
|
Jan Kratochvil |
a972d47 |
--V88s5gaDVPzZ0KCq--
|
|
Jan Kratochvil |
a972d47 |
|