[commit] Remove setting value address for reference entry value target data value

[Jan Kratochvil <jan.kratochvil@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 207 bytes --]

On Tue, 22 Jul 2014 21:19:22 +0200, Pedro Alves wrote:
> Thanks Jan.  Indeed I'd much prefer removing it.
> It's fine with me to still leave it in 7.8 in case we missed
> something.

Removed on trunk.


Jan

[-- Attachment #2: 1 --]
[-- Type: text/plain, Size: 5404 bytes --]

commit 45326f6fbe28ef5bac22dac447a4181c44cb945a
Author: Jan Kratochvil <jan.kratochvil@redhat.com>
Date:   Tue Jul 22 22:15:27 2014 +0200

    Remove setting value address for reference entry value target data value.
    
    I cannot reproduce any wrong case having the code removed.
    
    I just do not find it correct to have it disabled.  But at the same time I do
    like much / I do not find correct the code myself.  It is a bit problematic to
    have struct value describing a memory content which is no longer present
    there.
    
    What happens there:
    ------------------------------------------------------------------------------
    volatile int vv;
    static __attribute__((noinline)) int
    bar (int &ref) {
      ref = 20;
      vv++; /* break-here */
      return ref;
    }
    int main (void) {
      int var = 10;
      return bar (var);
    }
    ------------------------------------------------------------------------------
     <4><c7>: Abbrev Number: 13 (DW_TAG_GNU_call_site_parameter)
        <c8>   DW_AT_location    : 1 byte block: 55         (DW_OP_reg5 (rdi))
        <ca>   DW_AT_GNU_call_site_value: 2 byte block: 91 74       (DW_OP_fbreg: -12)
        <cd>   DW_AT_GNU_call_site_data_value: 1 byte block: 3a     (DW_OP_lit10)
    ------------------------------------------------------------------------------
    gdb -ex 'b value_addr' -ex r --args ../gdb ./1 -ex 'watch vv' -ex r -ex 'p &ref@entry'
    ->
    6    return ref;
    bar (ref=@0x7fffffffd944: 20, ref@entry=@0x7fffffffd944: 10) at 1.C:25
    ------------------------------------------------------------------------------
    At /* break-here */ struct value variable 'ref' is TYPE_CODE_REF.
    
    With FSF GDB HEAD:
    (gdb) x/gx arg1.contents
    0x6004000a4ad0: 0x00007fffffffd944
    (gdb) p ((struct value *)arg1.location.computed.closure).lval
    $1 = lval_memory
    (gdb) p/x ((struct value *)arg1.location.computed.closure).location.address
    $3 = 0x7fffffffd944
    
    With your #if0-ed code:
    (gdb) x/gx arg1.contents
    0x6004000a4ad0: 0x00007fffffffd944
    (gdb) p ((struct value *)arg1.location.computed.closure).lval
    $8 = not_lval
    (gdb) p/x ((struct value *)arg1.location.computed.closure).location.address
    $9 = 0x0
    
    I do not see how to access
            ((struct value *)arg1.location.computed.closure).location.address
    from GDB CLI.  Trying
    (gdb) p &ref@entry
    will invoke value_addr()'s:
      if (TYPE_CODE (type) == TYPE_CODE_REF)
          /* Copy the value, but change the type from (T&) to (T*).  We
             keep the same location information, which is efficient, and
             allows &(&X) to get the location containing the reference.  */
    and therefore the address gets fetched already from
      arg1.contents
    and not from
      ((struct value *)arg1.location.computed.closure).location.address
    .
    
    And for any other type than TYPE_CODE_REF this code you removed does not get
    executed at all.  This DW_AT_GNU_call_site_data_value DWARF was meant
    primarily for Fortran but with -O0 entry values do not get produced
    and with -Og and higher Fortran always optimizes out the passing by reference.
    
    If you do not like the removed code there I am OK with removing it as I do not
    know how to make it's use reproducible for user anyway.  In the worst case
    - if there really is some way how to exploit it - one should just get
      Attempt to take address of value not located in memory.
    instead of some wrong value and it may be easy to fix then.
    
    gdb/
    2014-07-22  Jan Kratochvil  <jan.kratochvil@redhat.com>
    
    	* dwarf2loc.c (value_of_dwarf_reg_entry): Remove setting value address
    	for reference entry value target data value.
    
    Message-ID: <20140720150727.GA18488@host2.jankratochvil.net>

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 73a13d3..687e2fe 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2014-07-22  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
+	* dwarf2loc.c (value_of_dwarf_reg_entry): Remove setting value address
+	for reference entry value target data value.
+
+2014-07-22  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
 	* stack.c (read_frame_arg): Verify value_optimized_out before calling
 	value_available_contents_eq.
 
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index fcab9b9..b1c7ee1 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1312,7 +1312,6 @@ value_of_dwarf_reg_entry (struct type *type, struct frame_info *frame,
   struct value *outer_val, *target_val, *val;
   struct call_site_parameter *parameter;
   struct dwarf2_per_cu_data *caller_per_cu;
-  CORE_ADDR addr;
 
   parameter = dwarf_expr_reg_to_entry_parameter (frame, kind, kind_u,
 						 &caller_per_cu);
@@ -1335,14 +1334,6 @@ value_of_dwarf_reg_entry (struct type *type, struct frame_info *frame,
 					       target_type, caller_frame,
 					       caller_per_cu);
 
-  /* value_as_address dereferences TYPE_CODE_REF.  */
-  addr = extract_typed_address (value_contents (outer_val), checked_type);
-
-  /* The target entry value has artificial address of the entry value
-     reference.  */
-  VALUE_LVAL (target_val) = lval_memory;
-  set_value_address (target_val, addr);
-
   release_value (target_val);
   val = allocate_computed_value (type, &entry_data_value_funcs,
 				 target_val /* closure */);