[RFA] Handle variable-sized fields in the interior of structure type

[Joel Brobecker <brobecker at adacore dot com>]

Hello,

In Ada, variable-sized field can be located at any position of
a structure. Consider for instance the following declarations:

   Dyn_Size : Integer := 1;

   type Table is array (Positive range <>) of Integer;

   type Inner is record
      T1 : Table (1 .. Dyn_Size) := (others => 1);
      T2 : Table (1 .. Dyn_Size) := (others => 2);
   end record;

   type Inner_Array is array (1 .. 2) of Inner;

   type Outer is
      record
         I0 : Integer := 0;
         A1 : Inner_Array;
         Marker : Integer := 16#01020304#;
      end record;

   Rt : Outer;

What this does is declare a variable "Rt" of type Outer, which
contains 3 fields where the second (A1) is of type Inner_Array.
type Inner_Array is an array with 2 elements of type Inner.
Because type Inner contains two arrays whose upper bound depend
on a variable, the size of the array, and therefore the size of
type Inner is dynamic, thus making field A1 a dynamically-size
field.

When trying to print the value of Rt, we hit the following limitation:

    (gdb) print rt
    Attempt to resolve a variably-sized type which appears in the interior of
    a structure type

The limitation was somewhat making sense in C, but needs to be lifted
for Ada. This patch mostly lifts that limitation. As a result of this
patch, the type length computation had to be reworked a little bit.

gdb/ChangeLog:

        * gdbtypes.c (resolve_dynamic_struct): Do not generate an error
        if detecting a variable-sized field that is not the last field.
        Fix struct type length computation.

gdb/testsuite/ChangeLog:

        * gdb.base/vla-datatypes.c (vla_factory): Add new variable
        inner_vla_struct_object_size.
        * gdb.base/vla-datatypes.exp: Adjust last test, and mark it
        as xfail.

Tested on x86_64-linux. This patch transforms one borderline PASS
into an XFAIL.  I can also add an Ada testcase for this based on
the sources above, but the C test already exercises the issue and
I think FSF GNAT is missing some recent patches we've made internally
to improve the quality of the debugging info for Ada (we working on
submitting them, however).

OK to apply?

Thanks!
-- 
Joel

---
 gdb/gdbtypes.c                           | 56 ++++++++++++++++++--------------
 gdb/testsuite/gdb.base/vla-datatypes.c   |  1 +
 gdb/testsuite/gdb.base/vla-datatypes.exp |  8 +++--
 3 files changed, 38 insertions(+), 27 deletions(-)

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index d0c002f..2fd32e2 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1776,7 +1776,7 @@ resolve_dynamic_struct (struct type *type, CORE_ADDR addr)
 {
   struct type *resolved_type;
   int i;
-  int vla_field = TYPE_NFIELDS (type) - 1;
+  unsigned resolved_type_bit_length = 0;
 
   gdb_assert (TYPE_CODE (type) == TYPE_CODE_STRUCT);
   gdb_assert (TYPE_NFIELDS (type) > 0);
@@ -1790,36 +1790,44 @@ resolve_dynamic_struct (struct type *type, CORE_ADDR addr)
 	  TYPE_NFIELDS (resolved_type) * sizeof (struct field));
   for (i = 0; i < TYPE_NFIELDS (resolved_type); ++i)
     {
-      struct type *t;
+      unsigned new_bit_length;
 
       if (field_is_static (&TYPE_FIELD (type, i)))
 	continue;
 
-      t = resolve_dynamic_type (TYPE_FIELD_TYPE (resolved_type, i), addr);
-
-      /* This is a bit odd.  We do not support a VLA in any position
-	 of a struct except for the last.  GCC does have an extension
-	 that allows a VLA in the middle of a structure, but the DWARF
-	 it emits is relatively useless to us, so we can't represent
-	 such a type properly -- and even if we could, we do not have
-	 enough information to redo structure layout anyway.
-	 Nevertheless, we check all the fields in case something odd
-	 slips through, since it's better to see an error than
-	 incorrect results.  */
-      if (t != TYPE_FIELD_TYPE (resolved_type, i)
-	  && i != vla_field)
-	error (_("Attempt to resolve a variably-sized type which appears "
-		 "in the interior of a structure type"));
-
-      TYPE_FIELD_TYPE (resolved_type, i) = t;
+      TYPE_FIELD_TYPE (resolved_type, i)
+	= resolve_dynamic_type (TYPE_FIELD_TYPE (resolved_type, i), 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
+	 than an internal error if that fails.  While failing
+	 that verification indicates a bug in our code, the error
+	 is not severe enough to suggest to the user he stops
+	 his debugging session because of it.  */
+      if (TYPE_FIELD_LOC_KIND (resolved_type, i) != FIELD_LOC_KIND_BITPOS)
+	error (_("Cannot determine struct field location"
+		 " (invalid location kind)"));
+      new_bit_length = TYPE_FIELD_BITPOS (resolved_type, i);
+      if (TYPE_FIELD_BITSIZE (resolved_type, i) != 0)
+	new_bit_length += TYPE_FIELD_BITSIZE (resolved_type, i);
+      else
+	new_bit_length += (TYPE_LENGTH (TYPE_FIELD_TYPE (resolved_type, i))
+			   * TARGET_CHAR_BIT);
+
+      /* Normally, we would use the position and size of the last field
+	 to determine the size of the enclosing structure.  But GCC seems
+	 to be encoding the position of some fields incorrectly when
+	 the struct contains a dynamic field that is not placed last.
+	 So we compute the struct size based on the field that has
+	 the highest position + size - probably the best we can do.  */
+      if (new_bit_length > resolved_type_bit_length)
+	resolved_type_bit_length = new_bit_length;
     }
 
-  /* Due to the above restrictions we can successfully compute
-     the size of the resulting structure here, as the offset of
-     the final field plus its size.  */
   TYPE_LENGTH (resolved_type)
-    = (TYPE_FIELD_BITPOS (resolved_type, vla_field) / TARGET_CHAR_BIT
-       + TYPE_LENGTH (TYPE_FIELD_TYPE (resolved_type, vla_field)));
+    = (resolved_type_bit_length + TARGET_CHAR_BIT - 1) / TARGET_CHAR_BIT;
+
   return resolved_type;
 }
 
diff --git a/gdb/testsuite/gdb.base/vla-datatypes.c b/gdb/testsuite/gdb.base/vla-datatypes.c
index 1ef30a5..41f3fd3 100644
--- a/gdb/testsuite/gdb.base/vla-datatypes.c
+++ b/gdb/testsuite/gdb.base/vla-datatypes.c
@@ -100,6 +100,7 @@ vla_factory (int n)
   size_t bar_size        = sizeof(bar_vla);
   size_t vla_struct_object_size = sizeof(vla_struct_object);
   size_t vla_union_object_size = sizeof(vla_union_object);
+  size_t inner_vla_struct_object_size = sizeof(inner_vla_struct_object);
 
   return;                                 /* break_end_of_vla_factory */
 }
diff --git a/gdb/testsuite/gdb.base/vla-datatypes.exp b/gdb/testsuite/gdb.base/vla-datatypes.exp
index 0e56bd7..15135a3 100644
--- a/gdb/testsuite/gdb.base/vla-datatypes.exp
+++ b/gdb/testsuite/gdb.base/vla-datatypes.exp
@@ -152,6 +152,8 @@ gdb_test "whatis ++int_vla\[0\]" "type = int" "whatis ++int_vla\[0\]"
 gdb_test "print int_vla\[0\]" " = 42" \
          "print int_vla\[0\] - whatis no side effects"
 
-# This gives an error for now.
-gdb_test "print sizeof(inner_vla_struct_object)" \
-    "appears in the interior of a structure type"
+# Fails due to incorrect debugging information generated by GCC.
+setup_xfail "*-*-*"
+gdb_test \
+    "print inner_vla_struct_object_size == sizeof(inner_vla_struct_object)" \
+    " = 1" "size of inner_vla_struct_object"
-- 
1.9.1