Re: [PATCH 2/2] handle VLA in a struct or union

Tom Tromey
2014-05-21T17:28:10Z

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> Perhaps we might think of having a new value_from_contents_and_address
Joel> that produces the value without resolving the type? If Ada is the only
Joel> user, as it would be, we could possibly put implement that in ada-lang,
Joel> I think. Kind of ugly, but this would be medium-term only, since we are
Joel> slowly trying to get rid of the GNAT encodings.

I put it in value.c, since I generally like to keep the whole module
together, having been bitten before by code living in the wrong spot.
I don't really care much though, in case you do.

Here's the updated patch, which I think addresses all review comments.

Tom

b/gdb/ChangeLog:
2014-05-21 Tom Tromey <tromey@redhat.com>

* ada-lang.c (ada_template_to_fixed_record_type_1): Use
value_from_contents_and_address_unresolved.
(ada_template_to_fixed_record_type_1): Likewise.
(ada_which_variant_applies): Likewise.
* value.h (value_from_contents_and_address_unresolved): Declare.
* value.c (value_from_contents_and_address_unresolved): New
function.
* gdbtypes.c (is_dynamic_type, resolve_dynamic_type)
<TYPE_CODE_STRUCT, TYPE_CODE_UNION>: New cases.
(resolve_dynamic_struct, resolve_dynamic_union): New functions.

b/gdb/testsuite/ChangeLog:
2014-05-21 Tom Tromey <tromey@redhat.com>

* gdb.base/vla-datatypes.exp: Add tests for VLA-in-structure and
VLA-in-union.
* gdb.base/vla-datatypes.c (vla_factory): Add vla_struct,
inner_vla_struct, vla_union types. Initialize objects of those
types and compute their sizes.

commit 81f06c4113ba4c263057000a3f3e576b0b36681c
Author: Tom Tromey <tromey@redhat.com>
Date: Thu May 8 11:26:44 2014 -0600

handle VLA in a struct or union

It is valid in GNU C to have a VLA in a struct or union type, but gdb
did not handle this.

This patch adds support for these cases in the obvious way.

Built and regtested on x86-64 Fedora 20.
New tests included.

2014-05-21 Tom Tromey <tromey@redhat.com>

* ada-lang.c (ada_template_to_fixed_record_type_1): Use
value_from_contents_and_address_unresolved.
(ada_template_to_fixed_record_type_1): Likewise.
(ada_which_variant_applies): Likewise.
* value.h (value_from_contents_and_address_unresolved): Declare.
* value.c (value_from_contents_and_address_unresolved): New
function.
* gdbtypes.c (is_dynamic_type, resolve_dynamic_type)
<TYPE_CODE_STRUCT, TYPE_CODE_UNION>: New cases.
(resolve_dynamic_struct, resolve_dynamic_union): New functions.

2014-05-21 Tom Tromey <tromey@redhat.com>

* gdb.base/vla-datatypes.exp: Add tests for VLA-in-structure and
VLA-in-union.
* gdb.base/vla-datatypes.c (vla_factory): Add vla_struct,
inner_vla_struct, vla_union types. Initialize objects of those
types and compute their sizes.

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 38972c6..c12fbb8 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -7385,7 +7385,11 @@ ada_which_variant_applies (struct type *var_type, struct type *outer_type,
struct value *discrim;
LONGEST discrim_val;

- outer = value_from_contents_and_address (outer_type, outer_valaddr, 0);
+ /* Using plain value_from_contents_and_address here causes problems
+ because we will end up trying to resolve a type that is currently
+ being constructed. */
+ outer = value_from_contents_and_address_unresolved (outer_type,
+ outer_valaddr, 0);
discrim = ada_value_struct_elt (outer, discrim_name, 1);
if (discrim == NULL)
return -1;
@@ -7925,7 +7929,13 @@ ada_template_to_fixed_record_type_1 (struct type *type,
GDB may fail to allocate a value for it. So check the
size first before creating the value. */
check_size (rtype);
- dval = value_from_contents_and_address (rtype, valaddr, address);
+ /* Using plain value_from_contents_and_address here
+ causes problems because we will end up trying to
+ resolve a type that is currently being
+ constructed. */
+ dval = value_from_contents_and_address_unresolved (rtype,
+ valaddr,
+ address);
rtype = value_type (dval);
}
else
@@ -8030,7 +8040,11 @@ ada_template_to_fixed_record_type_1 (struct type *type,

if (dval0 == NULL)
{
- dval = value_from_contents_and_address (rtype, valaddr, address);
+ /* Using plain value_from_contents_and_address here causes
+ problems because we will end up trying to resolve a type
+ that is currently being constructed. */
+ dval = value_from_contents_and_address_unresolved (rtype, valaddr,
+ address);
rtype = value_type (dval);
}
else
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 94d6fe9..0035d36 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1636,6 +1636,18 @@ is_dynamic_type (struct type *type)
return 1;
return is_dynamic_type (TYPE_TARGET_TYPE (type));
}
+
+ case TYPE_CODE_STRUCT:
+ case TYPE_CODE_UNION:
+ {
+ int i;
+
+ for (i = 0; i < TYPE_NFIELDS (type); ++i)
+ if (!field_is_static (&TYPE_FIELD (type, i))
+ && is_dynamic_type (TYPE_FIELD_TYPE (type, i)))
+ return 1;
+ }
+ break;
}

return 0;
@@ -1717,6 +1729,97 @@ resolve_dynamic_array (struct type *type)
range_type);
}

+/* Resolve dynamic bounds of members of the union TYPE to static
+ bounds. */
+
+static struct type *
+resolve_dynamic_union (struct type *type, CORE_ADDR addr)
+{
+ struct type *resolved_type;
+ int i;
+ unsigned int max_len = 0;
+
+ gdb_assert (TYPE_CODE (type) == TYPE_CODE_UNION);
+
+ resolved_type = copy_type (type);
+ TYPE_FIELDS (resolved_type)
+ = TYPE_ALLOC (resolved_type,
+ TYPE_NFIELDS (resolved_type) * sizeof (struct field));
+ memcpy (TYPE_FIELDS (resolved_type),
+ TYPE_FIELDS (type),
+ TYPE_NFIELDS (resolved_type) * sizeof (struct field));
+ for (i = 0; i < TYPE_NFIELDS (resolved_type); ++i)
+ {
+ struct type *t;
+
+ if (field_is_static (&TYPE_FIELD (type, i)))
+ continue;
+
+ t = resolve_dynamic_type (TYPE_FIELD_TYPE (resolved_type, i), addr);
+ TYPE_FIELD_TYPE (resolved_type, i) = t;
+ if (TYPE_LENGTH (t) > max_len)
+ max_len = TYPE_LENGTH (t);
+ }
+
+ TYPE_LENGTH (resolved_type) = max_len;
+ return resolved_type;
+}
+
+/* Resolve dynamic bounds of members of the struct TYPE to static
+ bounds. */
+
+static struct type *
+resolve_dynamic_struct (struct type *type, CORE_ADDR addr)
+{
+ struct type *resolved_type;
+ int i;
+ int vla_field = TYPE_NFIELDS (type) - 1;
+
+ gdb_assert (TYPE_CODE (type) == TYPE_CODE_STRUCT);
+ gdb_assert (TYPE_NFIELDS (type) > 0);
+
+ resolved_type = copy_type (type);
+ TYPE_FIELDS (resolved_type)
+ = TYPE_ALLOC (resolved_type,
+ TYPE_NFIELDS (resolved_type) * sizeof (struct field));
+ memcpy (TYPE_FIELDS (resolved_type),
+ TYPE_FIELDS (type),
+ TYPE_NFIELDS (resolved_type) * sizeof (struct field));
+ for (i = 0; i < TYPE_NFIELDS (resolved_type); ++i)
+ {
+ struct type *t;
+
+ 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;
+ }
+
+ /* 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)));
+ return resolved_type;
+}
+
/* See gdbtypes.h */

struct type *
@@ -1753,6 +1856,14 @@ resolve_dynamic_type (struct type *type, CORE_ADDR addr)
case TYPE_CODE_RANGE:
resolved_type = resolve_dynamic_range (type);
break;
+
+ case TYPE_CODE_UNION:
+ resolved_type = resolve_dynamic_union (type, addr);
+ break;
+
+ case TYPE_CODE_STRUCT:
+ resolved_type = resolve_dynamic_struct (type, addr);
+ break;
}

return resolved_type;
diff --git a/gdb/testsuite/gdb.base/vla-datatypes.c b/gdb/testsuite/gdb.base/vla-datatypes.c
index 51e342e..1ef30a5 100644
--- a/gdb/testsuite/gdb.base/vla-datatypes.c
+++ b/gdb/testsuite/gdb.base/vla-datatypes.c
@@ -46,6 +46,27 @@ vla_factory (int n)
BAR bar_vla[n];
int i;

+ struct vla_struct
+ {
+ int something;
+ int vla_field[n];
+ } vla_struct_object;
+
+ struct inner_vla_struct
+ {
+ int something;
+ int vla_field[n];
+ int after;
+ } inner_vla_struct_object;
+
+ union vla_union
+ {
+ int vla_field[n];
+ } vla_union_object;
+
+ vla_struct_object.something = n;
+ inner_vla_struct_object.something = n;
+ inner_vla_struct_object.after = n;
for (i = 0; i < n; i++)
{
int_vla[i] = i*2;
@@ -61,6 +82,9 @@ vla_factory (int n)
foo_vla[i].a = i*2;
bar_vla[i].x = i*2;
bar_vla[i].y.a = i*2;
+ vla_struct_object.vla_field[i] = i*2;
+ vla_union_object.vla_field[i] = i*2;
+ inner_vla_struct_object.vla_field[i] = i*2;
}

size_t int_size = sizeof(int_vla); /* vlas_filled */
@@ -74,6 +98,8 @@ vla_factory (int n)
size_t uchar_size = sizeof(unsigned_char_vla);
size_t foo_size = sizeof(foo_vla);
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);

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 8247658..0e56bd7 100644
--- a/gdb/testsuite/gdb.base/vla-datatypes.exp
+++ b/gdb/testsuite/gdb.base/vla-datatypes.exp
@@ -53,6 +53,10 @@ gdb_test "print foo_vla" \
gdb_test "print bar_vla" \
"\\\{\\\{x = 0, y = \\\{a = 0\\\}\\\}, \\\{x = 2, y = \\\{a = 2\\\}\\\}, \\\{x = 4, y = \\\{a = 4\\\}\\\}, \\\{x = 6, y = \\\{a = 6\\\}\\\}, \\\{x = 8, y = \\\{a = 8\\\}\\\}\\\}" \
"print bar_vla"
+gdb_test "print vla_struct_object" \
+ "\\\{something = 5, vla_field = \\\{0, 2, 4, 6, 8\\\}\\\}"
+gdb_test "print vla_union_object" \
+ "\\\{vla_field = \\\{0, 2, 4, 6, 8\\\}\\\}"

# Check whatis of VLA's.
gdb_test "whatis int_vla" "type = int \\\[5\\\]" "whatis int_vla"
@@ -74,6 +78,8 @@ gdb_test "whatis unsigned_char_vla" "type = unsigned char \\\[5\\\]" \
"whatis unsigned_char_vla"
gdb_test "whatis foo_vla" "type = struct foo \\\[5\\\]" "whatis foo_vla"
gdb_test "whatis bar_vla" "type = BAR \\\[5\\\]" "whatis bar_vla"
+gdb_test "whatis vla_struct_object" "type = struct vla_struct"
+gdb_test "whatis vla_union_object" "type = union vla_union"

# Check ptype of VLA's.
gdb_test "ptype int_vla" "type = int \\\[5\\\]" "ptype int_vla"
@@ -96,6 +102,10 @@ gdb_test "ptype foo_vla" "type = struct foo {\r\n\\s+int a;\r\n} \\\[5\\\]" \
gdb_test "ptype bar_vla" \
"type = struct bar {\r\n\\s+int x;\r\n\\s+struct foo y;\r\n} \\\[5\\\]" \
"ptype bar_vla"
+gdb_test "ptype vla_struct_object" \
+ "type = struct vla_struct {\r\n\\s+int something;\r\n\\s+int vla_field\\\[5\\\];\r\n}"
+gdb_test "ptype vla_union_object" \
+ "type = union vla_union {\r\n\\s+int vla_field\\\[5\\\];\r\n}"

# Check the size of the VLA's.
gdb_breakpoint [gdb_get_line_number "break_end_of_vla_factory"]
@@ -119,6 +129,10 @@ gdb_test "print uchar_size == sizeof(unsigned_char_vla)" " = 1" \
"size of unsigned_char_vla"
gdb_test "print foo_size == sizeof(foo_vla)" " = 1" "size of foo_vla"
gdb_test "print bar_size == sizeof(bar_vla)" " = 1" "size of bar_vla"
+gdb_test "print vla_struct_object_size == sizeof(vla_struct_object)" \
+ " = 1" "size of vla_struct_object"
+gdb_test "print vla_union_object_size == sizeof(vla_union_object)" \
+ " = 1" "size of vla_union_object"

# Check side effects for sizeof argument.
set sizeof_int [get_sizeof "int" 4]
@@ -137,3 +151,7 @@ gdb_test "print int_vla\[0\]" " = 42" \
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"
diff --git a/gdb/value.c b/gdb/value.c
index d125a09..1fa72df 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3338,6 +3338,29 @@ value_from_pointer (struct type *type, CORE_ADDR addr)
/* Create a value of type TYPE whose contents come from VALADDR, if it
is non-null, and whose memory address (in the inferior) is
ADDRESS. The type of the created value may differ from the passed
+ type TYPE. Make sure to retrieve values new type after this call.
+ Note that TYPE is not passed through resolve_dynamic_type; this is
+ a special API intended for use only by Ada. */
+
+struct value *
+value_from_contents_and_address_unresolved (struct type *type,
+ const gdb_byte *valaddr,
+ CORE_ADDR address)
+{
+ struct value *v;
+
+ if (valaddr == NULL)
+ v = allocate_value_lazy (type);
+ else
+ v = value_from_contents (type, valaddr);
+ set_value_address (v, address);
+ VALUE_LVAL (v) = lval_memory;
+ return v;
+}
+
+/* Create a value of type TYPE whose contents come from VALADDR, if it
+ is non-null, and whose memory address (in the inferior) is
+ ADDRESS. The type of the created value may differ from the passed
type TYPE. Make sure to retrieve values new type after this call. */

struct value *
diff --git a/gdb/value.h b/gdb/value.h
index 144e182..ce82376 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -574,6 +574,8 @@ extern struct value *value_from_history_ref (char *, char **);
extern struct value *value_at (struct type *type, CORE_ADDR addr);
extern struct value *value_at_lazy (struct type *type, CORE_ADDR addr);

+extern struct value *value_from_contents_and_address_unresolved
+ (struct type *, const gdb_byte *, CORE_ADDR);
extern struct value *value_from_contents_and_address (struct type *,
const gdb_byte *,
CORE_ADDR);