Add support for variable field offsets

In Ada, a field can have a variable offset.  This patch adds support
for this case to gdb, using the existing dynamic type resolution code.

Doing just this, though, would break C++ virtual base handling.

It turns out that virtual base handling only worked by the ugliest of
hacks.  In particular, the DWARF reader would call decode_locdesc for
a virtual base location.  Here's an example of such an expression from
gdb's m-static test case:

    <241>   DW_AT_data_member_location: 6 byte block: 12 6 48 1c 6 22 	(DW_OP_dup; DW_OP_deref; DW_OP_lit24; DW_OP_minus; DW_OP_deref; DW_OP_plus)

When examining this, decode_locdesc would treat DW_OP_deref as a no-op
and compute some answer (here, -24).  This would be stored as the
offset.

Later, in gnu-v3-abi.c, the real offset would be computed by digging
around in the vtable.

This patch cleans up this area.  In particular, it now evaluates the
location expression on demand.

Note there is a new FIXME in gnu-v3-abi.c.  I think some of the
callers are incorrect here, and have only worked because this member
is unused.  I will file a bug for this.  I didn't fix this problem in
this series because I felt it was already too complex.

gdb/ChangeLog
2020-04-24  Tom Tromey  <tromey@adacore.com>

	* dwarf2/read.c (handle_data_member_location): New overload.
	(dwarf2_add_field): Use it.
	(decode_locdesc): Add "computed" parameter.  Update comment.
	* gdbtypes.c (is_dynamic_type_internal): Also look for
	FIELD_LOC_KIND_DWARF_BLOCK.
	(resolve_dynamic_struct): Handle FIELD_LOC_KIND_DWARF_BLOCK.
	* gdbtypes.c (is_dynamic_type_internal): Add special case for C++
	virtual base classes.
	* gnu-v3-abi.c (gnuv3_baseclass_offset): Handle
	FIELD_LOC_KIND_DWARF_BLOCK.

gdb/testsuite/ChangeLog
2020-04-24  Tom Tromey  <tromey@adacore.com>

	* gdb.ada/variant.exp: Add dynamic field offset tests.
	* gdb.ada/variant/pck.ads (Nested_And_Variable): New type.
	* gdb.ada/variant/pkg.adb: Add new variables.

8 files changed, 218 insertions, 45 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 29e9a47..1f89444 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,18 @@
 2020-04-24  Tom Tromey  <tromey@adacore.com>
 
+	* dwarf2/read.c (handle_data_member_location): New overload.
+	(dwarf2_add_field): Use it.
+	(decode_locdesc): Add "computed" parameter.  Update comment.
+	* gdbtypes.c (is_dynamic_type_internal): Also look for
+	FIELD_LOC_KIND_DWARF_BLOCK.
+	(resolve_dynamic_struct): Handle FIELD_LOC_KIND_DWARF_BLOCK.
+	* gdbtypes.c (is_dynamic_type_internal): Add special case for C++
+	virtual base classes.
+	* gnu-v3-abi.c (gnuv3_baseclass_offset): Handle
+	FIELD_LOC_KIND_DWARF_BLOCK.
+
+2020-04-24  Tom Tromey  <tromey@adacore.com>
+
 	* dwarf2/read.c (read_structure_type): Handle dynamic length.
 	* gdbtypes.c (is_dynamic_type_internal): Check
 	TYPE_HAS_DYNAMIC_LENGTH.
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index f6d0624..14d53a2 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1435,7 +1435,8 @@ static const char *namespace_name (struct die_info *die,
 
 static void process_enumeration_scope (struct die_info *, struct dwarf2_cu *);
 
-static CORE_ADDR decode_locdesc (struct dwarf_block *, struct dwarf2_cu *);
+static CORE_ADDR decode_locdesc (struct dwarf_block *, struct dwarf2_cu *,
+				 bool * = nullptr);
 
 static enum dwarf_array_dim_ordering read_array_order (struct die_info *,
 						       struct dwarf2_cu *);
@@ -14212,6 +14213,53 @@ handle_data_member_location (struct die_info *die, struct dwarf2_cu *cu,
   return 0;
 }
 
+/* Look for DW_AT_data_member_location and store the results in FIELD.  */
+
+static void
+handle_data_member_location (struct die_info *die, struct dwarf2_cu *cu,
+			     struct field *field)
+{
+  struct attribute *attr;
+
+  attr = dwarf2_attr (die, DW_AT_data_member_location, cu);
+  if (attr != NULL)
+    {
+      if (attr->form_is_constant ())
+	{
+	  LONGEST offset = attr->constant_value (0);
+	  SET_FIELD_BITPOS (*field, offset * bits_per_byte);
+	}
+      else if (attr->form_is_section_offset ())
+	dwarf2_complex_location_expr_complaint ();
+      else if (attr->form_is_block ())
+	{
+	  bool handled;
+	  CORE_ADDR offset = decode_locdesc (DW_BLOCK (attr), cu, &handled);
+	  if (handled)
+	    SET_FIELD_BITPOS (*field, offset * bits_per_byte);
+	  else
+	    {
+	      struct objfile *objfile
+		= cu->per_cu->dwarf2_per_objfile->objfile;
+	      struct dwarf2_locexpr_baton *dlbaton
+		= XOBNEW (&objfile->objfile_obstack,
+			  struct dwarf2_locexpr_baton);
+	      dlbaton->data = DW_BLOCK (attr)->data;
+	      dlbaton->size = DW_BLOCK (attr)->size;
+	      /* When using this baton, we want to compute the address
+		 of the field, not the value.  This is why
+		 is_reference is set to false here.  */
+	      dlbaton->is_reference = false;
+	      dlbaton->per_cu = cu->per_cu;
+
+	      SET_FIELD_DWARF_BLOCK (*field, dlbaton);
+	    }
+	}
+      else
+	dwarf2_complex_location_expr_complaint ();
+    }
+}
+
 /* Add an aggregate field to the field list.  */
 
 static void
@@ -14256,8 +14304,6 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
 
   if (die->tag == DW_TAG_member && ! die_is_declaration (die, cu))
     {
-      LONGEST offset;
-
       /* Data member other than a C++ static data member.  */
 
       /* Get type of field.  */
@@ -14277,8 +14323,7 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
 	}
 
       /* Get bit offset of field.  */
-      if (handle_data_member_location (die, cu, &offset))
-	SET_FIELD_BITPOS (*fp, offset * bits_per_byte);
+      handle_data_member_location (die, cu, fp);
       attr = dwarf2_attr (die, DW_AT_bit_offset, cu);
       if (attr != nullptr)
 	{
@@ -14387,11 +14432,8 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
     }
   else if (die->tag == DW_TAG_inheritance)
     {
-      LONGEST offset;
-
       /* C++ base class field.  */
-      if (handle_data_member_location (die, cu, &offset))
-	SET_FIELD_BITPOS (*fp, offset * bits_per_byte);
+      handle_data_member_location (die, cu, fp);
       FIELD_BITSIZE (*fp) = 0;
       FIELD_TYPE (*fp) = die_type (die, cu);
       FIELD_NAME (*fp) = TYPE_NAME (fp->type);
@@ -22657,27 +22699,13 @@ read_signatured_type (struct signatured_type *sig_type)
 
 /* Decode simple location descriptions.
    Given a pointer to a dwarf block that defines a location, compute
-   the location and return the value.
-
-   NOTE drow/2003-11-18: This function is called in two situations
-   now: for the address of static or global variables (partial symbols
-   only) and for offsets into structures which are expected to be
-   (more or less) constant.  The partial symbol case should go away,
-   and only the constant case should remain.  That will let this
-   function complain more accurately.  A few special modes are allowed
-   without complaint for global variables (for instance, global
-   register values and thread-local values).
-
-   A location description containing no operations indicates that the
-   object is optimized out.  The return value is 0 for that case.
-   FIXME drow/2003-11-16: No callers check for this case any more; soon all
-   callers will only want a very basic result and this can become a
-   complaint.
-
-   Note that stack[0] is unused except as a default error return.  */
+   the location and return the value.  If COMPUTED is non-null, it is
+   set to true to indicate that decoding was successful, and false
+   otherwise.  If COMPUTED is null, then this function may emit a
+   complaint.  */
 
 static CORE_ADDR
-decode_locdesc (struct dwarf_block *blk, struct dwarf2_cu *cu)
+decode_locdesc (struct dwarf_block *blk, struct dwarf2_cu *cu, bool *computed)
 {
   struct objfile *objfile = cu->per_cu->dwarf2_per_objfile->objfile;
   size_t i;
@@ -22688,6 +22716,9 @@ decode_locdesc (struct dwarf_block *blk, struct dwarf2_cu *cu)
   unsigned int bytes_read, unsnd;
   gdb_byte op;
 
+  if (computed != nullptr)
+    *computed = false;
+
   i = 0;
   stacki = 0;
   stack[stacki] = 0;
@@ -22767,7 +22798,12 @@ decode_locdesc (struct dwarf_block *blk, struct dwarf2_cu *cu)
 	case DW_OP_reg31:
 	  stack[++stacki] = op - DW_OP_reg0;
 	  if (i < size)
-	    dwarf2_complex_location_expr_complaint ();
+	    {
+	      if (computed == nullptr)
+		dwarf2_complex_location_expr_complaint ();
+	      else
+		return 0;
+	    }
 	  break;
 
 	case DW_OP_regx:
@@ -22775,7 +22811,12 @@ decode_locdesc (struct dwarf_block *blk, struct dwarf2_cu *cu)
 	  i += bytes_read;
 	  stack[++stacki] = unsnd;
 	  if (i < size)
-	    dwarf2_complex_location_expr_complaint ();
+	    {
+	      if (computed == nullptr)
+		dwarf2_complex_location_expr_complaint ();
+	      else
+		return 0;
+	    }
 	  break;
 
 	case DW_OP_addr:
@@ -22857,7 +22898,12 @@ decode_locdesc (struct dwarf_block *blk, struct dwarf2_cu *cu)
 	     global symbols, although the variable's address will be bogus
 	     in the psymtab.  */
 	  if (i < size)
-	    dwarf2_complex_location_expr_complaint ();
+	    {
+	      if (computed == nullptr)
+		dwarf2_complex_location_expr_complaint ();
+	      else
+		return 0;
+	    }
 	  break;
 
         case DW_OP_GNU_push_tls_address:
@@ -22871,11 +22917,18 @@ decode_locdesc (struct dwarf_block *blk, struct dwarf2_cu *cu)
 	     non-zero to not look as a variable garbage collected by linker
 	     which have DW_OP_addr 0.  */
 	  if (i < size)
-	    dwarf2_complex_location_expr_complaint ();
+	    {
+	      if (computed == nullptr)
+		dwarf2_complex_location_expr_complaint ();
+	      else
+		return 0;
+	    }
 	  stack[stacki]++;
           break;
 
 	case DW_OP_GNU_uninit:
+	  if (computed != nullptr)
+	    return 0;
 	  break;
 
 	case DW_OP_addrx:
@@ -22887,16 +22940,17 @@ decode_locdesc (struct dwarf_block *blk, struct dwarf2_cu *cu)
 	  break;
 
 	default:
-	  {
-	    const char *name = get_DW_OP_name (op);
+	  if (computed == nullptr)
+	    {
+	      const char *name = get_DW_OP_name (op);
 
-	    if (name)
-	      complaint (_("unsupported stack op: '%s'"),
-			 name);
-	    else
-	      complaint (_("unsupported stack op: '%02x'"),
-			 op);
-	  }
+	      if (name)
+		complaint (_("unsupported stack op: '%s'"),
+			   name);
+	      else
+		complaint (_("unsupported stack op: '%02x'"),
+			   op);
+	    }
 
 	  return (stack[stacki]);
 	}
@@ -22905,16 +22959,21 @@ decode_locdesc (struct dwarf_block *blk, struct dwarf2_cu *cu)
          outside of the allocated space.  Also enforce minimum>0.  */
       if (stacki >= ARRAY_SIZE (stack) - 1)
 	{
-	  complaint (_("location description stack overflow"));
+	  if (computed == nullptr)
+	    complaint (_("location description stack overflow"));
 	  return 0;
 	}
 
       if (stacki <= 0)
 	{
-	  complaint (_("location description stack underflow"));
+	  if (computed == nullptr)
+	    complaint (_("location description stack underflow"));
 	  return 0;
 	}
     }
+
+  if (computed != nullptr)
+    *computed = true;
   return (stack[stacki]);
 }
 
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 6d755e9..7398435 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2015,10 +2015,27 @@ is_dynamic_type_internal (struct type *type, int top_level)
       {
 	int i;
 
+	bool is_cplus = HAVE_CPLUS_STRUCT (type);
+
 	for (i = 0; i < TYPE_NFIELDS (type); ++i)
-	  if (!field_is_static (&TYPE_FIELD (type, i))
-	      && is_dynamic_type_internal (TYPE_FIELD_TYPE (type, i), 0))
+	  {
+	    /* Static fields can be ignored here.  */
+	    if (field_is_static (&TYPE_FIELD (type, i)))
+	      continue;
+	    /* If the field has dynamic type, then so does TYPE.  */
+	    if (is_dynamic_type_internal (TYPE_FIELD_TYPE (type, i), 0))
+	      return 1;
+	    /* If the field is at a fixed offset, then it is not
+	       dynamic.  */
+	    if (TYPE_FIELD_LOC_KIND (type, i) != FIELD_LOC_KIND_DWARF_BLOCK)
+	      continue;
+	    /* Do not consider C++ virtual base types to be dynamic
+	       due to the field's offset being dynamic; these are
+	       handled via other means.  */
+	    if (is_cplus && BASETYPE_VIA_VIRTUAL (type, i))
+	      continue;
 	    return 1;
+	  }
       }
       break;
     }
@@ -2430,6 +2447,24 @@ resolve_dynamic_struct (struct type *type,
       if (field_is_static (&TYPE_FIELD (resolved_type, i)))
 	continue;
 
+      if (TYPE_FIELD_LOC_KIND (resolved_type, i) == FIELD_LOC_KIND_DWARF_BLOCK)
+	{
+	  struct dwarf2_property_baton baton;
+	  baton.property_type
+	    = lookup_pointer_type (TYPE_FIELD_TYPE (resolved_type, i));
+	  baton.locexpr = *TYPE_FIELD_DWARF_BLOCK (resolved_type, i);
+
+	  struct dynamic_prop prop;
+	  prop.kind = PROP_LOCEXPR;
+	  prop.data.baton = &baton;
+
+	  CORE_ADDR addr;
+	  if (dwarf2_evaluate_property (&prop, nullptr, addr_stack, &addr,
+					true))
+	    SET_FIELD_BITPOS (TYPE_FIELD (resolved_type, i),
+			      TARGET_CHAR_BIT * (addr - addr_stack->addr));
+	}
+
       /* As we know this field is not a static field, the field's
 	 field_loc_kind should be FIELD_LOC_KIND_BITPOS.  Verify
 	 this is the case, but only trigger a simple error rather
diff --git a/gdb/gnu-v3-abi.c b/gdb/gnu-v3-abi.c
index 89574ec..7055843 100644
--- a/gdb/gnu-v3-abi.c
+++ b/gdb/gnu-v3-abi.c
@@ -30,6 +30,7 @@
 #include "typeprint.h"
 #include <algorithm>
 #include "cli/cli-style.h"
+#include "dwarf2/loc.h"
 
 static struct cp_abi_ops gnu_v3_abi_ops;
 
@@ -461,6 +462,31 @@ gnuv3_baseclass_offset (struct type *type, int index,
   if (!BASETYPE_VIA_VIRTUAL (type, index))
     return TYPE_BASECLASS_BITPOS (type, index) / 8;
 
+  /* If we have a DWARF expression for the offset, evaluate it.  */
+  if (TYPE_FIELD_LOC_KIND (type, index) == FIELD_LOC_KIND_DWARF_BLOCK)
+    {
+      struct dwarf2_property_baton baton;
+      baton.property_type
+	= lookup_pointer_type (TYPE_FIELD_TYPE (type, index));
+      baton.locexpr = *TYPE_FIELD_DWARF_BLOCK (type, index);
+
+      struct dynamic_prop prop;
+      prop.kind = PROP_LOCEXPR;
+      prop.data.baton = &baton;
+
+      struct property_addr_info addr_stack;
+      addr_stack.type = type;
+      /* Note that we don't set "valaddr" here.  Doing so causes
+	 regressions.  FIXME.  */
+      addr_stack.addr = address + embedded_offset;
+      addr_stack.next = nullptr;
+
+      CORE_ADDR result;
+      if (dwarf2_evaluate_property (&prop, nullptr, &addr_stack, &result,
+				    true))
+	return (int) (result - addr_stack.addr);
+    }
+
   /* To access a virtual base, we need to use the vbase offset stored in
      our vtable.  Recent GCC versions provide this information.  If it isn't
      available, we could get what we needed from RTTI, or from drawing the
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 6664700..4e7dfac 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,11 @@
 2020-04-24  Tom Tromey  <tromey@adacore.com>
 
+	* gdb.ada/variant.exp: Add dynamic field offset tests.
+	* gdb.ada/variant/pck.ads (Nested_And_Variable): New type.
+	* gdb.ada/variant/pkg.adb: Add new variables.
+
+2020-04-24  Tom Tromey  <tromey@adacore.com>
+
 	* gdb.ada/variant.exp: New file
 	* gdb.ada/variant/pkg.adb: New file
 	* gdb.ada/variant/pck.adb: New file
diff --git a/gdb/testsuite/gdb.ada/variant.exp b/gdb/testsuite/gdb.ada/variant.exp
index b68bf60..490956a 100644
--- a/gdb/testsuite/gdb.ada/variant.exp
+++ b/gdb/testsuite/gdb.ada/variant.exp
@@ -37,4 +37,10 @@ foreach_with_prefix scenario {none all minimal} {
 
     gdb_test "print st1" " = \\(i => -4, one => 1, x => 2\\)"
     gdb_test "print st2" " = \\(i => 99, one => 1, y => 77\\)"
+
+    gdb_test "print nav1" " = \\(one => 0, two => 93, str => \"\"\\)"
+    gdb_test "print nav2" \
+	" = \\(one => 3, two => 0, str => \"zzz\", onevalue => 33, str2 => \"\"\\)"
+    gdb_test "print nav3" \
+	" = \\(one => 3, two => 7, str => \"zzz\", onevalue => 33, str2 => \"qqqqqqq\", twovalue => 88\\)"
 }
diff --git a/gdb/testsuite/gdb.ada/variant/pck.ads b/gdb/testsuite/gdb.ada/variant/pck.ads
index 41b6efd..3895b9c 100644
--- a/gdb/testsuite/gdb.ada/variant/pck.ads
+++ b/gdb/testsuite/gdb.ada/variant/pck.ads
@@ -34,4 +34,21 @@ package Pck is
 	   Y : Integer;
       end case;
    end record;
+
+   type Nested_And_Variable (One, Two: Integer) is record
+       Str : String (1 .. One);
+       case One is
+          when 0 =>
+	     null;
+          when others =>
+	     OneValue : Integer;
+             Str2 : String (1 .. Two);
+             case Two is
+	        when 0 =>
+		   null;
+		when others =>
+		   TwoValue : Integer;
+             end case;
+       end case;
+   end record;
 end Pck;
diff --git a/gdb/testsuite/gdb.ada/variant/pkg.adb b/gdb/testsuite/gdb.ada/variant/pkg.adb
index 0cc38f5..91cf080 100644
--- a/gdb/testsuite/gdb.ada/variant/pkg.adb
+++ b/gdb/testsuite/gdb.ada/variant/pkg.adb
@@ -22,6 +22,17 @@ procedure Pkg is
    ST1 : constant Second_Type := (I => -4, One => 1, X => 2);
    ST2 : constant Second_Type := (I => 99, One => 1, Y => 77);
 
+   NAV1 : constant Nested_And_Variable := (One => 0, Two => 93,
+                                           Str => (others => 'z'));
+   NAV2 : constant Nested_And_Variable := (One => 3, OneValue => 33,
+                                           Str => (others => 'z'),
+                                           Str2 => (others => 'q'),
+                                           Two => 0);
+   NAV3 : constant Nested_And_Variable := (One => 3, OneValue => 33,
+                                           Str => (others => 'z'),
+                                           Str2 => (others => 'q'),
+                                           Two => 7, TwoValue => 88);
+
 begin
    R := (C => 'd');
    Q := (C => Character'First, X_First => 27);