[RFC,gdb/exp] Handle DW_OP_GNU_variable_value refs to abstract dies

Message ID 982919b5-93ba-3840-5839-6accc904b459@suse.de
State New, archived
Headers

Commit Message

Tom de Vries Aug. 24, 2018, 1:09 p.m. UTC
  [ was: Re: [PATCH 1/3] Add support for DW_OP_GNU_variable_value ]

On 08/23/2018 11:12 PM, Kevin Buettner wrote:
> On Wed, 22 Aug 2018 17:35:23 +0200
> Tom de Vries <tdevries@suse.de> wrote:
> 
>> On 08/18/2018 10:31 PM, Kevin Buettner wrote:
>>>     This patch adds support for DW_OP_GNU_variable_value to GDB.
>>>     
>>>     Jakub Jelinek provides a fairly expansive discussion of this DWARF
>>>     expression opcode in his GCC patch...
>>>     
>>>         https://gcc.gnu.org/ml/gcc-patches/2017-02/msg01499.html
>>>     
>>>     It has also been proposed for addition to the DWARF Standard:
>>>     
>>>         http://www.dwarfstd.org/ShowIssue.php?issue=161109.2  
>>
>> Hi,
>>
>> AFAIU from the discussion here (
>> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01351.html ) if:
>> - a DW_OP_GNU_variable_value refers to a die 'a', and
>> - there's a die 'b' with abstract_origin 'a' that does have a
>>   DW_AT_location, and
>> - die 'b' is 'in scope' in an evaluation context,
>> then the evaluation of DW_OP_GNU_variable_value 'a' should return the
>> value found at the DW_AT_location of die 'b'.
>>
>> I've written a gcc demonstrator patch to generate code like this for
>> VLAs, and found that gdb master (containing this patch series) does not
>> support this.
>>
>> Is this further support of DW_OP_GNU_variable_value something you're
>> currently working on, or plan to work on?
> 
> Tom and I discussed this briefly on IRC.  Tom says that he'll take
> a look at it...
> 

Hi,

sofar I've written:
- a hack that fixes the test-case I have, without regressing anything,
  and
- the rationale for the change,
hoping that this should sufficiently explain what we're trying to implement.

Hints on how to implement are welcome.

Thanks,
- Tom
  

Comments

Richard Biener Aug. 24, 2018, 1:18 p.m. UTC | #1
On Fri, 24 Aug 2018, Tom de Vries wrote:

> [ was: Re: [PATCH 1/3] Add support for DW_OP_GNU_variable_value ]
> 
> On 08/23/2018 11:12 PM, Kevin Buettner wrote:
> > On Wed, 22 Aug 2018 17:35:23 +0200
> > Tom de Vries <tdevries@suse.de> wrote:
> > 
> >> On 08/18/2018 10:31 PM, Kevin Buettner wrote:
> >>>     This patch adds support for DW_OP_GNU_variable_value to GDB.
> >>>     
> >>>     Jakub Jelinek provides a fairly expansive discussion of this DWARF
> >>>     expression opcode in his GCC patch...
> >>>     
> >>>         https://gcc.gnu.org/ml/gcc-patches/2017-02/msg01499.html
> >>>     
> >>>     It has also been proposed for addition to the DWARF Standard:
> >>>     
> >>>         http://www.dwarfstd.org/ShowIssue.php?issue=161109.2  
> >>
> >> Hi,
> >>
> >> AFAIU from the discussion here (
> >> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01351.html ) if:
> >> - a DW_OP_GNU_variable_value refers to a die 'a', and
> >> - there's a die 'b' with abstract_origin 'a' that does have a
> >>   DW_AT_location, and
> >> - die 'b' is 'in scope' in an evaluation context,
> >> then the evaluation of DW_OP_GNU_variable_value 'a' should return the
> >> value found at the DW_AT_location of die 'b'.
> >>
> >> I've written a gcc demonstrator patch to generate code like this for
> >> VLAs, and found that gdb master (containing this patch series) does not
> >> support this.
> >>
> >> Is this further support of DW_OP_GNU_variable_value something you're
> >> currently working on, or plan to work on?
> > 
> > Tom and I discussed this briefly on IRC.  Tom says that he'll take
> > a look at it...
> > 
> 
> Hi,
> 
> sofar I've written:
> - a hack that fixes the test-case I have, without regressing anything,
>   and
> - the rationale for the change,
> hoping that this should sufficiently explain what we're trying to implement.
> 
> Hints on how to implement are welcome.

I wonder if there isn't already support to lookup 'd' when the
DIE of the concrete instance just has the location and refers to
'd' via an abstract origin.  So the idea is to do this kind of
lookup from the original context of the conrete instance we came
from when evaluating DW_OP_GNU_variable_value on the 
abstract instance DIE.

Btw, apart from the use with early debug / LTO this would make it
possible to remove repeating VLA types in inline instances.  If you
build the following with -O -g you get three instances of int[n]
DIEs, one in the abstract copy and two generated for the
DW_TAG_inlined_subroutine in bar.  Ideally we could elide those,
having the type of a fully specified in the abstract instance
and the concrete instances providing locations for n.

static inline void foo (int n) { int a[n]; }
int bar(int n1, int n2)
{
  foo (n1);
  foo (n2);
}

Richard.
  

Patch

[gdb/exp] Handle DW_OP_GNU_variable_value refs to abstract dies

Consider a vla variable 'a' in function f1:
...
 <2><1a7>: Abbrev Number: 11 (DW_TAG_variable)
    <1a8>   DW_AT_description : a
    <1aa>   DW_AT_abstract_origin: <0x311>
...
with abstract origin 'a':
...
 <2><311>: Abbrev Number: 3 (DW_TAG_variable)
    <312>   DW_AT_name        : a
    <317>   DW_AT_type        : <0x325>
...
and inherited abstract vla type:
...
 <1><325>: Abbrev Number: 9 (DW_TAG_array_type)
    <326>   DW_AT_type        : <0x33a>
 <2><32e>: Abbrev Number: 10 (DW_TAG_subrange_type)
    <32f>   DW_AT_type        : <0x2ea>
    <333>   DW_AT_upper_bound : 5 byte block: fd 1b 3 0 0
                                (DW_OP_GNU_variable_value: <0x31b>)
...
where the upper bound refers to this artificial variable D.1922 without location
attribute:
...
 <2><31b>: Abbrev Number: 8 (DW_TAG_variable)
    <31c>   DW_AT_description : (indirect string, offset: 0x39a): D.1922
    <320>   DW_AT_type        : <0x2ea>
    <324>   DW_AT_artificial  : 1
...

Currently, when we execute "p sizeof (a)" in f1, the upper bound is calculated
by evaluating the DW_OP_GNU_variable_value expression referring to D.1922, but
since that die doesn't have a location attribute, we get:
...
value has been optimized out
...

However, there's also artificial variable D.4283 that is sibling of vla
variable 'a', has artificial variable D.1922 as abstract origin, and has a
location attribute:
...
 <2><1ae>: Abbrev Number: 12 (DW_TAG_variable)
    <1af>   DW_AT_description : (indirect string, offset: 0x1f8): D.4283
    <1b3>   DW_AT_abstract_origin: <0x31b>
    <1b7>   DW_AT_location    : 11 byte block: 75 1 8 20 24 8 20 26 31 1c 9f
                                (DW_OP_breg5 (rdi):1; DW_OP_const1u: 32;
				 DW_OP_shl; DW_OP_const1u: 32; DW_OP_shra;
				 DW_OP_lit1; DW_OP_minus; DW_OP_stack_value)
...

The intended behaviour for DW_OP_GNU_variable_value is to find a die that
refers to D.1922 as abstract origin, has a location attribute and is
'in scope', so the expected behaviour is:
...
$1 = 6
...

The 'in scope' concept can be thought of as variable D.1922 having name
attribute "D.1922", and variable D.4283 inheriting that attribute, resulting
in D.4283 being declared with name "D.1922" alongside vla a in f1, and when we
lookup "DW_OP_GNU_variable_value D.1922", it should work as if we try to find
the value of a variable named "D.1922" on the gdb command line using
"p D.1922", and we should return the value of D.4283.

This patch contains a hack that handles the case described above.

Build and reg-tested on x86_64-linux.

---
 gdb/dwarf2loc.c  | 10 ++++++----
 gdb/dwarf2loc.h  |  2 +-
 gdb/dwarf2read.c | 28 ++++++++++++++++++++++++++--
 3 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 200fa03f46..a75eb158c7 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -65,7 +65,7 @@  static struct value *indirect_synthetic_pointer
   (sect_offset die, LONGEST byte_offset,
    struct dwarf2_per_cu_data *per_cu,
    struct frame_info *frame,
-   struct type *type);
+   struct type *type, bool = false);
 
 /* Until these have formal names, we define these here.
    ref: http://gcc.gnu.org/wiki/DebugFission
@@ -573,7 +573,7 @@  sect_variable_value (struct dwarf_expr_context *ctx, sect_offset sect_off,
 
   struct type *type = lookup_pointer_type (die_type);
   struct frame_info *frame = get_selected_frame (_("No frame selected."));
-  return indirect_synthetic_pointer (sect_off, 0, per_cu, frame, type);
+  return indirect_synthetic_pointer (sect_off, 0, per_cu, frame, type, true);
 }
 
 class dwarf_evaluate_loc_desc : public dwarf_expr_context
@@ -2181,12 +2181,14 @@  fetch_const_value_from_synthetic_pointer (sect_offset die, LONGEST byte_offset,
 static struct value *
 indirect_synthetic_pointer (sect_offset die, LONGEST byte_offset,
 			    struct dwarf2_per_cu_data *per_cu,
-			    struct frame_info *frame, struct type *type)
+			    struct frame_info *frame, struct type *type,
+			    bool resolve_abstract_p)
 {
   /* Fetch the location expression of the DIE we're pointing to.  */
   struct dwarf2_locexpr_baton baton
     = dwarf2_fetch_die_loc_sect_off (die, per_cu,
-				     get_frame_address_in_block_wrapper, frame);
+				     get_frame_address_in_block_wrapper, frame,
+				     resolve_abstract_p);
 
   /* Get type of pointed-to DIE.  */
   struct type *orig_type = dwarf2_fetch_die_type_sect_off (die, per_cu);
diff --git a/gdb/dwarf2loc.h b/gdb/dwarf2loc.h
index f82e7b2d11..08120651b6 100644
--- a/gdb/dwarf2loc.h
+++ b/gdb/dwarf2loc.h
@@ -67,7 +67,7 @@  const gdb_byte *dwarf2_find_location_expression
 struct dwarf2_locexpr_baton dwarf2_fetch_die_loc_sect_off
   (sect_offset offset_in_cu, struct dwarf2_per_cu_data *per_cu,
    CORE_ADDR (*get_frame_pc) (void *baton),
-   void *baton);
+   void *baton, bool = false);
 
 struct dwarf2_locexpr_baton dwarf2_fetch_die_loc_cu_off
   (cu_offset offset_in_cu, struct dwarf2_per_cu_data *per_cu,
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 8834d08a1c..3d214b7cee 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -92,6 +92,8 @@ 
 #include "rust-lang.h"
 #include "common/pathstuff.h"
 
+std::unordered_map<struct die_info *,struct die_info *> abstract_to_concrete;
+
 /* When == 1, print basic high level tracing messages.
    When > 1, be more verbose.
    This is in contrast to the low level DIE reading of dwarf_die_debug.  */
@@ -14283,7 +14285,23 @@  read_variable (struct die_info *die, struct dwarf2_cu *cu)
 	}
     }
 
-  new_symbol (die, NULL, cu, storage);
+  struct symbol *res = new_symbol (die, NULL, cu, storage);
+  if (res == NULL && dwarf2_attr (die, DW_AT_location, cu))
+    {
+      struct attribute *attr = dwarf2_attr (die, DW_AT_abstract_origin, cu);
+      if (attr)
+	{
+	  struct dwarf2_cu *origin_cu = cu;
+	  struct die_info *origin_die = follow_die_ref (die, attr, &origin_cu);
+	  /* We have a variable without a name, but with a location and an
+	     abstract origin.  This may be a concrete instance of an abstract
+	     variable referenced from an DW_OP_GNU_variable_value, so save it to
+	     find it back later.  Hack: assume 1-on-1 relationship.  Of course,
+	     in reality one abstract variable may have many concrete instances.
+	     TODO: Fix this hack by storing into a 1-to-many data structure.  */
+	  abstract_to_concrete[origin_die] = die;
+	}
+    }
 }
 
 /* Call CALLBACK from DW_AT_ranges attribute value OFFSET
@@ -22998,7 +23016,7 @@  struct dwarf2_locexpr_baton
 dwarf2_fetch_die_loc_sect_off (sect_offset sect_off,
 			       struct dwarf2_per_cu_data *per_cu,
 			       CORE_ADDR (*get_frame_pc) (void *baton),
-			       void *baton)
+			       void *baton, bool resolve_abstract_p)
 {
   struct dwarf2_cu *cu;
   struct die_info *die;
@@ -23023,6 +23041,12 @@  dwarf2_fetch_die_loc_sect_off (sect_offset sect_off,
     error (_("Dwarf Error: Cannot find DIE at %s referenced in module %s"),
 	   sect_offset_str (sect_off), objfile_name (objfile));
 
+  if (resolve_abstract_p
+      && abstract_to_concrete.find (die) != abstract_to_concrete.end ())
+    /* TODO: Rewrite abstract_to_concrete into a 1-to-many data structure, and
+       find the appropriate concrete instance.  */
+    die = abstract_to_concrete[die];
+
   attr = dwarf2_attr (die, DW_AT_location, cu);
   if (!attr)
     {