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

[Joel Brobecker <brobecker at adacore dot com>]

> 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.

I have pushed this patch.  There was a tiny merge conflict to resolve,
so attached is the patch I ended up pushing.  Re-tested on x86_64-linux.

-- 
Joel

>From 8d76bcdaa04cdee397afeaf08a2ec7c2669971a2 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Tue, 8 Jul 2014 08:15:35 -0700
Subject: [PATCH] Handle variable-sized fields in the interior of structure
 type

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.
---
 gdb/gdbtypes.c                           | 54 ++++++++++++++++++--------------
 gdb/testsuite/gdb.base/vla-datatypes.c   |  1 +
 gdb/testsuite/gdb.base/vla-datatypes.exp |  8 +++--
 3 files changed, 37 insertions(+), 26 deletions(-)

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index e99a2f3..97d94f2 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1790,7 +1790,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);
@@ -1804,37 +1804,45 @@ 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_internal (TYPE_FIELD_TYPE (resolved_type, i),
+      TYPE_FIELD_TYPE (resolved_type, i)
+	= resolve_dynamic_type_internal (TYPE_FIELD_TYPE (resolved_type, i),
 					 addr, 0);
 
-      /* 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;
+      /* 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