From patchwork Wed May 21 17:28:03 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 1057 Return-Path: X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx20.g.dreamhost.com (mx2.sub5.homie.mail.dreamhost.com [208.113.200.128]) by wilcox.dreamhost.com (Postfix) with ESMTP id EA1C4360073 for ; Wed, 21 May 2014 10:28:12 -0700 (PDT) Received: by homiemail-mx20.g.dreamhost.com (Postfix, from userid 14314964) id 9FD3C41D72171; Wed, 21 May 2014 10:28:12 -0700 (PDT) X-Original-To: gdb@patchwork.siddhesh.in Delivered-To: x14314964@homiemail-mx20.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx20.g.dreamhost.com (Postfix) with ESMTPS id 6E3D741CB450A for ; Wed, 21 May 2014 10:28:12 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:cc:subject:references:date:in-reply-to :message-id:mime-version:content-type; q=dns; s=default; b=bhz7g 9rqLu0tsNGDmjoDv90bA8Nc8m3kpnsxaUTt2H2Cs/R1LXTL31lTG03y5ks+kaWzf mR2yOBzlgNaSZAZKZqZZ+cxhc/PWONC6uH5weZF6lm3eQUe+QsAubNW6kpOt2Sti +madFYj/vfG5S0eyYSwgENWkBQ6bEd7k79tLWg= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:cc:subject:references:date:in-reply-to :message-id:mime-version:content-type; s=default; bh=TbAwfvNdQmx BjbhN1Yk/EYxw1uk=; b=U4bSY0F0hdboavDdqKk56vX7H3trXZKpaj/M+dB7FBY L1esIrPC55kH+Q7gr+0AWO8AaKwOzi5FJ4AlfvCYkM+JJVuLweZ01hUKY9p2/HX7 39acIUjh/oHVyWaFKBQk6q+rRoqRDwRW/lRUTcT1B6iGYa4hyPfOTVN0b8A3mvyE = Received: (qmail 22521 invoked by alias); 21 May 2014 17:28:10 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 22511 invoked by uid 89); 21 May 2014 17:28:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 21 May 2014 17:28:07 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s4LHS522002343 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 21 May 2014 13:28:05 -0400 Received: from barimba (ovpn-113-182.phx2.redhat.com [10.3.113.182]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s4LHS3U3014557 (version=TLSv1/SSLv3 cipher=AES128-GCM-SHA256 bits=128 verify=NO); Wed, 21 May 2014 13:28:04 -0400 From: Tom Tromey To: Joel Brobecker Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 2/2] handle VLA in a struct or union References: <1399574816-12845-1-git-send-email-tromey@redhat.com> <1399574816-12845-3-git-send-email-tromey@redhat.com> <20140508210914.GE4063@adacore.com> <87lhucuf98.fsf@fleche.redhat.com> <20140508223853.GF4063@adacore.com> <20140509155726.GG4063@adacore.com> Date: Wed, 21 May 2014 11:28:03 -0600 In-Reply-To: <20140509155726.GG4063@adacore.com> (Joel Brobecker's message of "Fri, 9 May 2014 08:57:26 -0700") Message-ID: <87fvk3kpng.fsf@fleche.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-DH-Original-To: gdb@patchwork.siddhesh.in >>>>> "Joel" == Joel Brobecker 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 * 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) : New cases. (resolve_dynamic_struct, resolve_dynamic_union): New functions. b/gdb/testsuite/ChangeLog: 2014-05-21 Tom Tromey * 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 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 * 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) : New cases. (resolve_dynamic_struct, resolve_dynamic_union): New functions. 2014-05-21 Tom Tromey * 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);