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

pinskia
2014-05-08T19:01:18Z

> On May 8, 2014, at 11:46 AM, Tom Tromey <tromey@redhat.com> wrote:
>
> It is valid in C to have a VLA in a struct or union type, but gdb did
> not handle this.

In GNU C only. It is extension.

Thanks,
Andrew

>
> This patch adds support for these cases in the obvious way.
>
> Built and regtested on x86-64 Fedora 20.
> New tests included.
>
> However, before this goes in, I'd like to understand the oddity
> pointed out by a change in is_dynamic_type. That is, this test:
>
> /* This can happen with Ada for reasons unknown. */
> && TYPE_FIELD_TYPE (type, i) != NULL
>
> is needed to avoid a crash with the Ada "iwide.exp" test. This type:
>
> (top-gdb) p real_type.main_type.name
> $15 = 0x7ffff0354c6d "ada__tags__type_specific_data___XVE"
>
> ... has a seemingly invalid field:
>
> (top-gdb) p real_type.main_type.nfields
> $9 = 13
> [...]
> (top-gdb) p real_type.main_type.flds_bnds.fields[12]
> $12 = {
> loc = {
> bitpos = 576,
> enumval = 576,
> physaddr = 576,
> physname = 0x240 <Address 0x240 out of bounds>,
> dwarf_block = 0x240
> },
> artificial = 0,
> loc_kind = FIELD_LOC_KIND_BITPOS,
> bitsize = 0,
> type = 0x0,
> name = 0x0
> }
>
> Joel, can you comment? Thanks.
>
> 2014-05-08 Tom Tromey <tromey@redhat.com>
>
> * gdbtypes.c (is_dynamic_type, resolve_dynamic_type)
> <TYPE_CODE_STRUCT, TYPE_CODE_UNION>: New cases.
>
> 2014-05-08 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,
> vla_union types. Initialize objects of those types and compute
> their sizes.
> ---
> gdb/ChangeLog | 5 ++
> gdb/gdbtypes.c | 94 ++++++++++++++++++++++++++++++++
> gdb/testsuite/ChangeLog | 8 +++
> gdb/testsuite/gdb.base/vla-datatypes.c | 16 ++++++
> gdb/testsuite/gdb.base/vla-datatypes.exp | 14 +++++
> 5 files changed, 137 insertions(+)
>
> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index 95b861e..59f354a 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -1636,6 +1636,20 @@ 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))
> + /* This can happen with Ada for reasons unknown. */
> + && TYPE_FIELD_TYPE (type, i) != NULL
> + && is_dynamic_type (TYPE_FIELD_TYPE (type, i)))
> + return 1;
> + }
> + break;
> }
>
> return 0;
> @@ -1753,6 +1767,86 @@ resolve_dynamic_type (struct type *type, CORE_ADDR addr)
> case TYPE_CODE_RANGE:
> resolved_type = resolve_dynamic_range (type);
> break;
> +
> + case TYPE_CODE_UNION:
> + {
> + int i;
> + unsigned int max_len;
> +
> + 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;
> + }
> + break;
> +
> + case TYPE_CODE_STRUCT:
> + {
> + int i;
> + int vla_field = TYPE_NFIELDS (type) - 1;
> +
> + 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)));
> + }
> + break;
> }
>
> return resolved_type;
> diff --git a/gdb/testsuite/gdb.base/vla-datatypes.c b/gdb/testsuite/gdb.base/vla-datatypes.c
> index 51e342e..8561a4e 100644
> --- a/gdb/testsuite/gdb.base/vla-datatypes.c
> +++ b/gdb/testsuite/gdb.base/vla-datatypes.c
> @@ -46,6 +46,18 @@ vla_factory (int n)
> BAR bar_vla[n];
> int i;
>
> + struct vla_struct
> + {
> + int something;
> + int vla_field[n];
> + } vla_struct_object;
> +
> + union vla_union
> + {
> + int vla_field[n];
> + } vla_union_object;
> +
> + vla_struct_object.something = n;
> for (i = 0; i < n; i++)
> {
> int_vla[i] = i*2;
> @@ -61,6 +73,8 @@ 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;
> }
>
> size_t int_size = sizeof(int_vla); /* vlas_filled */
> @@ -74,6 +88,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..36af38d 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]
> --
> 1.9.0
>