[2/2] handle VLA in a struct or union
Commit Message
>>>>> "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.
Comments
Hey Tom,
> 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.
Great minds think alike. I was offering alternatives in case people
don't want to keep stuff that is not necessarily specific to Ada
but only used by Ada in the common area. But that would usually be
nonsense to me, since that encourages proliferation of the same code
at multiple locations. But sometimes it's only a bandaid and we don't
want to encourage the use of that code, so keeping it close the few
spots where it is still necessary might make better sense.
I can't remember. Is there a dash in "overthinking"? :-)
> Here's the updated patch, which I think addresses all review comments.
I'll try to review the patch again, but you should feel free to
push it anytime you're ready. I am fairly busy at the moment :-(.
Thanks, Tom!
Hi 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.
I decided to take the time to look at it today (or else I fear I'd never
take it ;-)). The patch looks good to me, and you should feel free to
go ahead and push.
I have one small comment (but that's not a request for change):
> +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;
Ada allows dynamic fields being in the middle. For instance:
type Really_Dyn (S1, S2 : Integer) is record
T1 : Table (1 .. S1);
T2 : Table (1 .. S2);
V : Integer;
end Really_Dyn;
We currently are having trouble describing bounds referencing a field
of the containing record like in this case, but our intent is to
eventually produce the correct debugging information both for array
bounds as well as field location.
We don't need to worry about this today. It's just a heads-up that
I will like fix this area up as soon as we have a compiler that
produces this kind of info. In the meantime, it's really great that
you're generating an error, it's going to be make this limitation
really easy to notice when it needs to be lifted.
>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:
Joel> We don't need to worry about this today. It's just a heads-up that
Joel> I will like fix this area up as soon as we have a compiler that
Joel> produces this kind of info. In the meantime, it's really great that
Joel> you're generating an error, it's going to be make this limitation
Joel> really easy to notice when it needs to be lifted.
FWIW there's also a bug in GCC bugzilla about the bad debuginfo
generated here for C. If that's even fixed, I will fix up gdb.
Tom
>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:
Joel> I decided to take the time to look at it today (or else I fear I'd never
Joel> take it ;-)). The patch looks good to me, and you should feel free to
Joel> go ahead and push.
I'm pushing this series now. Thanks, Joel.
Tom
@@ -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
@@ -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;
@@ -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 */
}
@@ -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"
@@ -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 *
@@ -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);