From patchwork Tue Jul 8 17:20:53 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joel Brobecker X-Patchwork-Id: 1951 Received: (qmail 18917 invoked by alias); 8 Jul 2014 17:21:17 -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 18853 invoked by uid 89); 8 Jul 2014 17:21:10 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Tue, 08 Jul 2014 17:21:05 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id E89BB116165; Tue, 8 Jul 2014 13:21:03 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id p5uiHuw4JBz8; Tue, 8 Jul 2014 13:21:03 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id BF065116154; Tue, 8 Jul 2014 13:21:03 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 16B7640F62; Tue, 8 Jul 2014 10:21:03 -0700 (PDT) From: Joel Brobecker To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [RFA] Handle variable-sized fields in the interior of structure type Date: Tue, 8 Jul 2014 10:20:53 -0700 Message-Id: <1404840053-24084-1-git-send-email-brobecker@adacore.com> Hello, 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. Tested on x86_64-linux. This patch transforms one borderline PASS into an XFAIL. I can also add an Ada testcase for this based on the sources above, but the C test already exercises the issue and I think FSF GNAT is missing some recent patches we've made internally to improve the quality of the debugging info for Ada (we working on submitting them, however). OK to apply? Thanks! diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c index d0c002f..2fd32e2 100644 --- a/gdb/gdbtypes.c +++ b/gdb/gdbtypes.c @@ -1776,7 +1776,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); @@ -1790,36 +1790,44 @@ 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 (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; + TYPE_FIELD_TYPE (resolved_type, i) + = resolve_dynamic_type (TYPE_FIELD_TYPE (resolved_type, i), addr); + + /* 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"