Message ID | 20181212095854.78440-1-alan.hayward@arm.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 130745 invoked by alias); 12 Dec 2018 09:59:11 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 130724 invoked by uid 89); 12 Dec 2018 09:59:10 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 spammy=Valid X-HELO: EUR02-VE1-obe.outbound.protection.outlook.com Received: from mail-eopbgr20071.outbound.protection.outlook.com (HELO EUR02-VE1-obe.outbound.protection.outlook.com) (40.107.2.71) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 12 Dec 2018 09:59:06 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector1-arm-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=46wTLUdpufX6r8Uyjj40uEgXGfBSjHkWL9vnjGk30Zc=; b=cEkfVsIwg0r0FL6k2ylQWqvMBPG9ffiJMxmXEm9DED5+R0EBA3QlKyg/1euinMszB5y3as6a6bsHzEAWDwfomqW86faxFhmaOKXuHAOHYePAReAP+C21Z6WosEpJK1AE9gGeApgriGXs4XZyP1naRVyqYiIz2IBlV1wb0WbW6K0= Received: from DB6PR0802MB2133.eurprd08.prod.outlook.com (10.172.226.148) by DB6PR0802MB2550.eurprd08.prod.outlook.com (10.172.251.148) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1404.22; Wed, 12 Dec 2018 09:59:02 +0000 Received: from DB6PR0802MB2133.eurprd08.prod.outlook.com ([fe80::6463:a537:145e:704f]) by DB6PR0802MB2133.eurprd08.prod.outlook.com ([fe80::6463:a537:145e:704f%5]) with mapi id 15.20.1425.016; Wed, 12 Dec 2018 09:59:02 +0000 From: Alan Hayward <Alan.Hayward@arm.com> To: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org> CC: nd <nd@arm.com>, Alan Hayward <Alan.Hayward@arm.com> Subject: [PATCH] AArch64: Prevent infinite recursion when checking AAPCS types Date: Wed, 12 Dec 2018 09:59:02 +0000 Message-ID: <20181212095854.78440-1-alan.hayward@arm.com> authentication-results: spf=none (sender IP is ) smtp.mailfrom=Alan.Hayward@arm.com; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-IsSubscribed: yes |
Commit Message
Alan Hayward
Dec. 12, 2018, 9:59 a.m. UTC
gdb.dwarf2/dw2-cp-infcall-ref-static.exp includes a struct which contains itself. Add support to aapcs_is_vfp_call_or_return_candidate to prevent infinite recursion. The simple solution is to pass a counter through the functions and abort once the max limit is reached. However, this does not catch the case where a struct only contains itself and no other members. That can be solved by marking a type as invalid before checking all of it's children. Add a count for the call or ret candidate to main_type, restricted to 3 bits - the value 5 (or above) is used as invalid. This code is testsed by both gdb.base/infcall-nested-structs.exp and gdb.dwarf2/dw2-cp-infcall-ref-static.exp. gdb/ChangeLog: 2018-12-12 Alan Hayward <alan.hayward@arm.com> * aarch64-tdep.c (HA_MAX_NUM_FLDS): Remove define. (aapcs_is_vfp_call_or_return_candidate_1): Remember type. (aapcs_is_vfp_call_or_return_candidate): Likewise. * gdbtypes.h (AAPCS_VFP_CALL_OR_RET_CAND_MAX): Add define. (AAPCS_VFP_CALL_OR_RET_CAND_INVALID): Likewise. (TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE): Likewise. (TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE): Likewise. (struct main_type): Add aapcs_candidate_count. --- gdb/aarch64-tdep.c | 87 ++++++++++++++++++++++++++-------------------- gdb/gdbtypes.h | 23 ++++++++++++ 2 files changed, 72 insertions(+), 38 deletions(-)
Comments
Ping. > On 12 Dec 2018, at 09:59, Alan Hayward <Alan.Hayward@arm.com> wrote: > > gdb.dwarf2/dw2-cp-infcall-ref-static.exp includes a struct which > contains itself. Add support to aapcs_is_vfp_call_or_return_candidate to > prevent infinite recursion. > > The simple solution is to pass a counter through the functions and abort once > the max limit is reached. However, this does not catch the case where a > struct only contains itself and no other members. That can be solved by > marking a type as invalid before checking all of it's children. > > Add a count for the call or ret candidate to main_type, restricted to 3 > bits - the value 5 (or above) is used as invalid. > > This code is testsed by both gdb.base/infcall-nested-structs.exp > and gdb.dwarf2/dw2-cp-infcall-ref-static.exp. > > gdb/ChangeLog: > > 2018-12-12 Alan Hayward <alan.hayward@arm.com> > > * aarch64-tdep.c (HA_MAX_NUM_FLDS): Remove define. > (aapcs_is_vfp_call_or_return_candidate_1): Remember type. > (aapcs_is_vfp_call_or_return_candidate): Likewise. > * gdbtypes.h (AAPCS_VFP_CALL_OR_RET_CAND_MAX): Add define. > (AAPCS_VFP_CALL_OR_RET_CAND_INVALID): Likewise. > (TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE): Likewise. > (TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE): Likewise. > (struct main_type): Add aapcs_candidate_count. > --- > gdb/aarch64-tdep.c | 87 ++++++++++++++++++++++++++-------------------- > gdb/gdbtypes.h | 23 ++++++++++++ > 2 files changed, 72 insertions(+), 38 deletions(-) > > diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c > index ae56c9ca34..3fc58308d6 100644 > --- a/gdb/aarch64-tdep.c > +++ b/gdb/aarch64-tdep.c > @@ -64,10 +64,6 @@ > #define bit(obj,st) (((obj) >> (st)) & 1) > #define bits(obj,st,fn) (((obj) >> (st)) & submask ((fn) - (st))) > > -/* A Homogeneous Floating-Point or Short-Vector Aggregate may have at most > - four members. */ > -#define HA_MAX_NUM_FLDS 4 > - > /* All possible aarch64 target descriptors. */ > struct target_desc *tdesc_aarch64_list[AARCH64_MAX_SVE_VQ + 1]; > > @@ -1146,46 +1142,58 @@ aarch64_type_align (struct type *t) > > /* Worker function for aapcs_is_vfp_call_or_return_candidate. > > - Return the number of register required, or -1 on failure. > + Set TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE to the number of registers required, > + or AAPCS_VFP_CALL_OR_RET_CAND_INVALID on failure. > > When encountering a base element, if FUNDAMENTAL_TYPE is not set then set it > to the element, else fail if the type of this element does not match the > existing value. */ > > -static int > +static void > aapcs_is_vfp_call_or_return_candidate_1 (struct type *type, > struct type **fundamental_type) > { > if (type == nullptr) > - return -1; > + return; > + > + /* Check if we have already evaluated this TYPE. */ > + if (TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type) > + >= AAPCS_VFP_CALL_OR_RET_CAND_INVALID) > + return; > + > + /* Set TYPE to invalid to prevent infinite recursion. */ > + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, > + AAPCS_VFP_CALL_OR_RET_CAND_INVALID); > > switch (TYPE_CODE (type)) > { > case TYPE_CODE_FLT: > if (TYPE_LENGTH (type) > 16) > - return -1; > + return; > > if (*fundamental_type == nullptr) > *fundamental_type = type; > else if (TYPE_LENGTH (type) != TYPE_LENGTH (*fundamental_type) > || TYPE_CODE (type) != TYPE_CODE (*fundamental_type)) > - return -1; > + return; > > - return 1; > + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, 1); > + return; > > case TYPE_CODE_COMPLEX: > { > struct type *target_type = check_typedef (TYPE_TARGET_TYPE (type)); > if (TYPE_LENGTH (target_type) > 16) > - return -1; > + return; > > if (*fundamental_type == nullptr) > *fundamental_type = target_type; > else if (TYPE_LENGTH (target_type) != TYPE_LENGTH (*fundamental_type) > || TYPE_CODE (target_type) != TYPE_CODE (*fundamental_type)) > - return -1; > + return; > > - return 2; > + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, 2); > + return; > } > > case TYPE_CODE_ARRAY: > @@ -1193,28 +1201,33 @@ aapcs_is_vfp_call_or_return_candidate_1 (struct type *type, > if (TYPE_VECTOR (type)) > { > if (TYPE_LENGTH (type) != 8 && TYPE_LENGTH (type) != 16) > - return -1; > + return; > > if (*fundamental_type == nullptr) > *fundamental_type = type; > else if (TYPE_LENGTH (type) != TYPE_LENGTH (*fundamental_type) > || TYPE_CODE (type) != TYPE_CODE (*fundamental_type)) > - return -1; > + return; > > - return 1; > + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, 1); > } > else > { > + /* Calculate if type of an element in the array is valid. */ > struct type *target_type = TYPE_TARGET_TYPE (type); > - int count = aapcs_is_vfp_call_or_return_candidate_1 > - (target_type, fundamental_type); > + aapcs_is_vfp_call_or_return_candidate_1 (target_type, > + fundamental_type); > > - if (count == -1) > - return count; > + int count = TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (target_type); > + if (count >= AAPCS_VFP_CALL_OR_RET_CAND_INVALID) > + return; > + > + /* Expand to cover the whole array. The set handles overflow. */ > > count *= (TYPE_LENGTH (type) / TYPE_LENGTH (target_type)); > - return count; > + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, count); > } > + return; > } > > case TYPE_CODE_STRUCT: > @@ -1226,20 +1239,23 @@ aapcs_is_vfp_call_or_return_candidate_1 (struct type *type, > { > struct type *member = check_typedef (TYPE_FIELD_TYPE (type, i)); > > - int sub_count = aapcs_is_vfp_call_or_return_candidate_1 > - (member, fundamental_type); > - if (sub_count == -1) > - return -1; > + aapcs_is_vfp_call_or_return_candidate_1 (member, fundamental_type); > + > + int sub_count = TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (member); > + > + if (sub_count + count > AAPCS_VFP_CALL_OR_RET_CAND_MAX) > + return; > + > count += sub_count; > } > - return count; > + > + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, count); > + return; > } > > default: > - break; > + return; > } > - > - return -1; > } > > /* Return true if an argument, whose type is described by TYPE, can be passed or > @@ -1269,16 +1285,11 @@ aapcs_is_vfp_call_or_return_candidate (struct type *type, int *count, > > *fundamental_type = nullptr; > > - int ag_count = aapcs_is_vfp_call_or_return_candidate_1 (type, > - fundamental_type); > + aapcs_is_vfp_call_or_return_candidate_1 (type, fundamental_type); > > - if (ag_count > 0 && ag_count <= HA_MAX_NUM_FLDS) > - { > - *count = ag_count; > - return true; > - } > - else > - return false; > + *count = TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type); > + > + return (*count < AAPCS_VFP_CALL_OR_RET_CAND_INVALID); > } > > /* AArch64 function call information structure. */ > diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h > index f0adec7a20..600112abf5 100644 > --- a/gdb/gdbtypes.h > +++ b/gdb/gdbtypes.h > @@ -391,6 +391,23 @@ DEF_ENUM_FLAGS_TYPE (enum type_instance_flag_value, type_instance_flags); > #define TYPE_ADDRESS_CLASS_ALL(t) (TYPE_INSTANCE_FLAGS(t) \ > & TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL) > > + > +/* * Is the given TYPE able to be placed in an Aarch64 VFP when used as a call > + parameter or return parameter, as per the AAPCS64 5.4.2.C. Value can be: > + 0: the type has not been checked. > + 1 to AAPCS_VFP_CALL_OR_RET_CAND_MAX: Valid, and has that many members. > + AAPCS_VFP_CALL_OR_RET_CAND_MAX: Invalid. */ > + > +#define AAPCS_VFP_CALL_OR_RET_CAND_MAX 4 > +#define AAPCS_VFP_CALL_OR_RET_CAND_INVALID (AAPCS_VFP_CALL_OR_RET_CAND_MAX + 1) > + > +#define TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE(thistype) \ > + (TYPE_MAIN_TYPE (thistype)->aapcs_candidate_count) > + > +#define TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE(thistype, val) \ > + TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (thistype) = \ > + (std::min (val, AAPCS_VFP_CALL_OR_RET_CAND_MAX + 1)) > + > /* * Information needed for a discriminated union. A discriminated > union is handled somewhat differently from an ordinary union. > > @@ -716,6 +733,12 @@ struct main_type > > ENUM_BITFIELD(type_specific_kind) type_specific_field : 3; > > + /* * Is the given TYPE able to be placed in an Aarch64 VFP when used as a call > + parameter or return parameter, as per the AAPCS64 5.4.2.C. Required to > + prevent recursive checking. */ > + > + unsigned int aapcs_candidate_count : 3; > + > /* * Number of fields described for this type. This field appears > at this location because it packs nicely here. */ > > -- > 2.17.2 (Apple Git-113) >
On 12/12/2018 09:59 AM, Alan Hayward wrote: > gdb.dwarf2/dw2-cp-infcall-ref-static.exp includes a struct which > contains itself. Add support to aapcs_is_vfp_call_or_return_candidate to > prevent infinite recursion. But the contained struct is a static field, no? How does that affect the calling convention (or whatever is being computed) ? Without looking at any of this in detail, I'd off hand think that this code should be skipping/ignoring static fields? Thanks, Pedro Alves > > The simple solution is to pass a counter through the functions and abort once > the max limit is reached. However, this does not catch the case where a > struct only contains itself and no other members. That can be solved by > marking a type as invalid before checking all of it's children. > > Add a count for the call or ret candidate to main_type, restricted to 3 > bits - the value 5 (or above) is used as invalid. > > This code is testsed by both gdb.base/infcall-nested-structs.exp > and gdb.dwarf2/dw2-cp-infcall-ref-static.exp. > > gdb/ChangeLog: > > 2018-12-12 Alan Hayward <alan.hayward@arm.com> > > * aarch64-tdep.c (HA_MAX_NUM_FLDS): Remove define. > (aapcs_is_vfp_call_or_return_candidate_1): Remember type. > (aapcs_is_vfp_call_or_return_candidate): Likewise. > * gdbtypes.h (AAPCS_VFP_CALL_OR_RET_CAND_MAX): Add define. > (AAPCS_VFP_CALL_OR_RET_CAND_INVALID): Likewise. > (TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE): Likewise. > (TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE): Likewise. > (struct main_type): Add aapcs_candidate_count. > --- > gdb/aarch64-tdep.c | 87 ++++++++++++++++++++++++++-------------------- > gdb/gdbtypes.h | 23 ++++++++++++ > 2 files changed, 72 insertions(+), 38 deletions(-) > > diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c > index ae56c9ca34..3fc58308d6 100644 > --- a/gdb/aarch64-tdep.c > +++ b/gdb/aarch64-tdep.c > @@ -64,10 +64,6 @@ > #define bit(obj,st) (((obj) >> (st)) & 1) > #define bits(obj,st,fn) (((obj) >> (st)) & submask ((fn) - (st))) > > -/* A Homogeneous Floating-Point or Short-Vector Aggregate may have at most > - four members. */ > -#define HA_MAX_NUM_FLDS 4 > - > /* All possible aarch64 target descriptors. */ > struct target_desc *tdesc_aarch64_list[AARCH64_MAX_SVE_VQ + 1]; > > @@ -1146,46 +1142,58 @@ aarch64_type_align (struct type *t) > > /* Worker function for aapcs_is_vfp_call_or_return_candidate. > > - Return the number of register required, or -1 on failure. > + Set TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE to the number of registers required, > + or AAPCS_VFP_CALL_OR_RET_CAND_INVALID on failure. > > When encountering a base element, if FUNDAMENTAL_TYPE is not set then set it > to the element, else fail if the type of this element does not match the > existing value. */ > > -static int > +static void > aapcs_is_vfp_call_or_return_candidate_1 (struct type *type, > struct type **fundamental_type) > { > if (type == nullptr) > - return -1; > + return; > + > + /* Check if we have already evaluated this TYPE. */ > + if (TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type) > + >= AAPCS_VFP_CALL_OR_RET_CAND_INVALID) > + return; > + > + /* Set TYPE to invalid to prevent infinite recursion. */ > + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, > + AAPCS_VFP_CALL_OR_RET_CAND_INVALID); > > switch (TYPE_CODE (type)) > { > case TYPE_CODE_FLT: > if (TYPE_LENGTH (type) > 16) > - return -1; > + return; > > if (*fundamental_type == nullptr) > *fundamental_type = type; > else if (TYPE_LENGTH (type) != TYPE_LENGTH (*fundamental_type) > || TYPE_CODE (type) != TYPE_CODE (*fundamental_type)) > - return -1; > + return; > > - return 1; > + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, 1); > + return; > > case TYPE_CODE_COMPLEX: > { > struct type *target_type = check_typedef (TYPE_TARGET_TYPE (type)); > if (TYPE_LENGTH (target_type) > 16) > - return -1; > + return; > > if (*fundamental_type == nullptr) > *fundamental_type = target_type; > else if (TYPE_LENGTH (target_type) != TYPE_LENGTH (*fundamental_type) > || TYPE_CODE (target_type) != TYPE_CODE (*fundamental_type)) > - return -1; > + return; > > - return 2; > + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, 2); > + return; > } > > case TYPE_CODE_ARRAY: > @@ -1193,28 +1201,33 @@ aapcs_is_vfp_call_or_return_candidate_1 (struct type *type, > if (TYPE_VECTOR (type)) > { > if (TYPE_LENGTH (type) != 8 && TYPE_LENGTH (type) != 16) > - return -1; > + return; > > if (*fundamental_type == nullptr) > *fundamental_type = type; > else if (TYPE_LENGTH (type) != TYPE_LENGTH (*fundamental_type) > || TYPE_CODE (type) != TYPE_CODE (*fundamental_type)) > - return -1; > + return; > > - return 1; > + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, 1); > } > else > { > + /* Calculate if type of an element in the array is valid. */ > struct type *target_type = TYPE_TARGET_TYPE (type); > - int count = aapcs_is_vfp_call_or_return_candidate_1 > - (target_type, fundamental_type); > + aapcs_is_vfp_call_or_return_candidate_1 (target_type, > + fundamental_type); > > - if (count == -1) > - return count; > + int count = TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (target_type); > + if (count >= AAPCS_VFP_CALL_OR_RET_CAND_INVALID) > + return; > + > + /* Expand to cover the whole array. The set handles overflow. */ > > count *= (TYPE_LENGTH (type) / TYPE_LENGTH (target_type)); > - return count; > + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, count); > } > + return; > } > > case TYPE_CODE_STRUCT: > @@ -1226,20 +1239,23 @@ aapcs_is_vfp_call_or_return_candidate_1 (struct type *type, > { > struct type *member = check_typedef (TYPE_FIELD_TYPE (type, i)); > > - int sub_count = aapcs_is_vfp_call_or_return_candidate_1 > - (member, fundamental_type); > - if (sub_count == -1) > - return -1; > + aapcs_is_vfp_call_or_return_candidate_1 (member, fundamental_type); > + > + int sub_count = TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (member); > + > + if (sub_count + count > AAPCS_VFP_CALL_OR_RET_CAND_MAX) > + return; > + > count += sub_count; > } > - return count; > + > + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, count); > + return; > } > > default: > - break; > + return; > } > - > - return -1; > } > > /* Return true if an argument, whose type is described by TYPE, can be passed or > @@ -1269,16 +1285,11 @@ aapcs_is_vfp_call_or_return_candidate (struct type *type, int *count, > > *fundamental_type = nullptr; > > - int ag_count = aapcs_is_vfp_call_or_return_candidate_1 (type, > - fundamental_type); > + aapcs_is_vfp_call_or_return_candidate_1 (type, fundamental_type); > > - if (ag_count > 0 && ag_count <= HA_MAX_NUM_FLDS) > - { > - *count = ag_count; > - return true; > - } > - else > - return false; > + *count = TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type); > + > + return (*count < AAPCS_VFP_CALL_OR_RET_CAND_INVALID); > } > > /* AArch64 function call information structure. */ > diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h > index f0adec7a20..600112abf5 100644 > --- a/gdb/gdbtypes.h > +++ b/gdb/gdbtypes.h > @@ -391,6 +391,23 @@ DEF_ENUM_FLAGS_TYPE (enum type_instance_flag_value, type_instance_flags); > #define TYPE_ADDRESS_CLASS_ALL(t) (TYPE_INSTANCE_FLAGS(t) \ > & TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL) > > + > +/* * Is the given TYPE able to be placed in an Aarch64 VFP when used as a call > + parameter or return parameter, as per the AAPCS64 5.4.2.C. Value can be: > + 0: the type has not been checked. > + 1 to AAPCS_VFP_CALL_OR_RET_CAND_MAX: Valid, and has that many members. > + AAPCS_VFP_CALL_OR_RET_CAND_MAX: Invalid. */ > + > +#define AAPCS_VFP_CALL_OR_RET_CAND_MAX 4 > +#define AAPCS_VFP_CALL_OR_RET_CAND_INVALID (AAPCS_VFP_CALL_OR_RET_CAND_MAX + 1) > + > +#define TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE(thistype) \ > + (TYPE_MAIN_TYPE (thistype)->aapcs_candidate_count) > + > +#define TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE(thistype, val) \ > + TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (thistype) = \ > + (std::min (val, AAPCS_VFP_CALL_OR_RET_CAND_MAX + 1)) > + > /* * Information needed for a discriminated union. A discriminated > union is handled somewhat differently from an ordinary union. > > @@ -716,6 +733,12 @@ struct main_type > > ENUM_BITFIELD(type_specific_kind) type_specific_field : 3; > > + /* * Is the given TYPE able to be placed in an Aarch64 VFP when used as a call > + parameter or return parameter, as per the AAPCS64 5.4.2.C. Required to > + prevent recursive checking. */ > + > + unsigned int aapcs_candidate_count : 3; > + > /* * Number of fields described for this type. This field appears > at this location because it packs nicely here. */ > >
> On 9 Jan 2019, at 19:01, Pedro Alves <palves@redhat.com> wrote: > > On 12/12/2018 09:59 AM, Alan Hayward wrote: >> gdb.dwarf2/dw2-cp-infcall-ref-static.exp includes a struct which >> contains itself. Add support to aapcs_is_vfp_call_or_return_candidate to >> prevent infinite recursion. > > But the contained struct is a static field, no? How does that affect > the calling convention (or whatever is being computed) ? > > Without looking at any of this in detail, I'd off hand think that > this code should be skipping/ignoring static fields? > > Thanks, > Pedro Alves > Hmmm... yes, good spot. Tried this in GCC, and the static members are not passed as args. GCC doesn’t have any explicit code to cover this in the backend - I suspect that by the time it gets there the static members are effectivley already in a special global var. I’ll write up some new test cases to cover the cases with normal structs with static members. And then I’ll see if I generate some tests with non-static infinite structs. Maybe it’ll squash the need for this patch. Thanks, Alan. >> >> The simple solution is to pass a counter through the functions and abort once >> the max limit is reached. However, this does not catch the case where a >> struct only contains itself and no other members. That can be solved by >> marking a type as invalid before checking all of it's children. >> >> Add a count for the call or ret candidate to main_type, restricted to 3 >> bits - the value 5 (or above) is used as invalid. >> >> This code is testsed by both gdb.base/infcall-nested-structs.exp >> and gdb.dwarf2/dw2-cp-infcall-ref-static.exp. >> >> gdb/ChangeLog: >> >> 2018-12-12 Alan Hayward <alan.hayward@arm.com> >> >> * aarch64-tdep.c (HA_MAX_NUM_FLDS): Remove define. >> (aapcs_is_vfp_call_or_return_candidate_1): Remember type. >> (aapcs_is_vfp_call_or_return_candidate): Likewise. >> * gdbtypes.h (AAPCS_VFP_CALL_OR_RET_CAND_MAX): Add define. >> (AAPCS_VFP_CALL_OR_RET_CAND_INVALID): Likewise. >> (TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE): Likewise. >> (TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE): Likewise. >> (struct main_type): Add aapcs_candidate_count. >> --- >> gdb/aarch64-tdep.c | 87 ++++++++++++++++++++++++++-------------------- >> gdb/gdbtypes.h | 23 ++++++++++++ >> 2 files changed, 72 insertions(+), 38 deletions(-) >> >> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c >> index ae56c9ca34..3fc58308d6 100644 >> --- a/gdb/aarch64-tdep.c >> +++ b/gdb/aarch64-tdep.c >> @@ -64,10 +64,6 @@ >> #define bit(obj,st) (((obj) >> (st)) & 1) >> #define bits(obj,st,fn) (((obj) >> (st)) & submask ((fn) - (st))) >> >> -/* A Homogeneous Floating-Point or Short-Vector Aggregate may have at most >> - four members. */ >> -#define HA_MAX_NUM_FLDS 4 >> - >> /* All possible aarch64 target descriptors. */ >> struct target_desc *tdesc_aarch64_list[AARCH64_MAX_SVE_VQ + 1]; >> >> @@ -1146,46 +1142,58 @@ aarch64_type_align (struct type *t) >> >> /* Worker function for aapcs_is_vfp_call_or_return_candidate. >> >> - Return the number of register required, or -1 on failure. >> + Set TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE to the number of registers required, >> + or AAPCS_VFP_CALL_OR_RET_CAND_INVALID on failure. >> >> When encountering a base element, if FUNDAMENTAL_TYPE is not set then set it >> to the element, else fail if the type of this element does not match the >> existing value. */ >> >> -static int >> +static void >> aapcs_is_vfp_call_or_return_candidate_1 (struct type *type, >> struct type **fundamental_type) >> { >> if (type == nullptr) >> - return -1; >> + return; >> + >> + /* Check if we have already evaluated this TYPE. */ >> + if (TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type) >> + >= AAPCS_VFP_CALL_OR_RET_CAND_INVALID) >> + return; >> + >> + /* Set TYPE to invalid to prevent infinite recursion. */ >> + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, >> + AAPCS_VFP_CALL_OR_RET_CAND_INVALID); >> >> switch (TYPE_CODE (type)) >> { >> case TYPE_CODE_FLT: >> if (TYPE_LENGTH (type) > 16) >> - return -1; >> + return; >> >> if (*fundamental_type == nullptr) >> *fundamental_type = type; >> else if (TYPE_LENGTH (type) != TYPE_LENGTH (*fundamental_type) >> || TYPE_CODE (type) != TYPE_CODE (*fundamental_type)) >> - return -1; >> + return; >> >> - return 1; >> + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, 1); >> + return; >> >> case TYPE_CODE_COMPLEX: >> { >> struct type *target_type = check_typedef (TYPE_TARGET_TYPE (type)); >> if (TYPE_LENGTH (target_type) > 16) >> - return -1; >> + return; >> >> if (*fundamental_type == nullptr) >> *fundamental_type = target_type; >> else if (TYPE_LENGTH (target_type) != TYPE_LENGTH (*fundamental_type) >> || TYPE_CODE (target_type) != TYPE_CODE (*fundamental_type)) >> - return -1; >> + return; >> >> - return 2; >> + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, 2); >> + return; >> } >> >> case TYPE_CODE_ARRAY: >> @@ -1193,28 +1201,33 @@ aapcs_is_vfp_call_or_return_candidate_1 (struct type *type, >> if (TYPE_VECTOR (type)) >> { >> if (TYPE_LENGTH (type) != 8 && TYPE_LENGTH (type) != 16) >> - return -1; >> + return; >> >> if (*fundamental_type == nullptr) >> *fundamental_type = type; >> else if (TYPE_LENGTH (type) != TYPE_LENGTH (*fundamental_type) >> || TYPE_CODE (type) != TYPE_CODE (*fundamental_type)) >> - return -1; >> + return; >> >> - return 1; >> + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, 1); >> } >> else >> { >> + /* Calculate if type of an element in the array is valid. */ >> struct type *target_type = TYPE_TARGET_TYPE (type); >> - int count = aapcs_is_vfp_call_or_return_candidate_1 >> - (target_type, fundamental_type); >> + aapcs_is_vfp_call_or_return_candidate_1 (target_type, >> + fundamental_type); >> >> - if (count == -1) >> - return count; >> + int count = TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (target_type); >> + if (count >= AAPCS_VFP_CALL_OR_RET_CAND_INVALID) >> + return; >> + >> + /* Expand to cover the whole array. The set handles overflow. */ >> >> count *= (TYPE_LENGTH (type) / TYPE_LENGTH (target_type)); >> - return count; >> + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, count); >> } >> + return; >> } >> >> case TYPE_CODE_STRUCT: >> @@ -1226,20 +1239,23 @@ aapcs_is_vfp_call_or_return_candidate_1 (struct type *type, >> { >> struct type *member = check_typedef (TYPE_FIELD_TYPE (type, i)); >> >> - int sub_count = aapcs_is_vfp_call_or_return_candidate_1 >> - (member, fundamental_type); >> - if (sub_count == -1) >> - return -1; >> + aapcs_is_vfp_call_or_return_candidate_1 (member, fundamental_type); >> + >> + int sub_count = TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (member); >> + >> + if (sub_count + count > AAPCS_VFP_CALL_OR_RET_CAND_MAX) >> + return; >> + >> count += sub_count; >> } >> - return count; >> + >> + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, count); >> + return; >> } >> >> default: >> - break; >> + return; >> } >> - >> - return -1; >> } >> >> /* Return true if an argument, whose type is described by TYPE, can be passed or >> @@ -1269,16 +1285,11 @@ aapcs_is_vfp_call_or_return_candidate (struct type *type, int *count, >> >> *fundamental_type = nullptr; >> >> - int ag_count = aapcs_is_vfp_call_or_return_candidate_1 (type, >> - fundamental_type); >> + aapcs_is_vfp_call_or_return_candidate_1 (type, fundamental_type); >> >> - if (ag_count > 0 && ag_count <= HA_MAX_NUM_FLDS) >> - { >> - *count = ag_count; >> - return true; >> - } >> - else >> - return false; >> + *count = TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type); >> + >> + return (*count < AAPCS_VFP_CALL_OR_RET_CAND_INVALID); >> } >> >> /* AArch64 function call information structure. */ >> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h >> index f0adec7a20..600112abf5 100644 >> --- a/gdb/gdbtypes.h >> +++ b/gdb/gdbtypes.h >> @@ -391,6 +391,23 @@ DEF_ENUM_FLAGS_TYPE (enum type_instance_flag_value, type_instance_flags); >> #define TYPE_ADDRESS_CLASS_ALL(t) (TYPE_INSTANCE_FLAGS(t) \ >> & TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL) >> >> + >> +/* * Is the given TYPE able to be placed in an Aarch64 VFP when used as a call >> + parameter or return parameter, as per the AAPCS64 5.4.2.C. Value can be: >> + 0: the type has not been checked. >> + 1 to AAPCS_VFP_CALL_OR_RET_CAND_MAX: Valid, and has that many members. >> + AAPCS_VFP_CALL_OR_RET_CAND_MAX: Invalid. */ >> + >> +#define AAPCS_VFP_CALL_OR_RET_CAND_MAX 4 >> +#define AAPCS_VFP_CALL_OR_RET_CAND_INVALID (AAPCS_VFP_CALL_OR_RET_CAND_MAX + 1) >> + >> +#define TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE(thistype) \ >> + (TYPE_MAIN_TYPE (thistype)->aapcs_candidate_count) >> + >> +#define TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE(thistype, val) \ >> + TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (thistype) = \ >> + (std::min (val, AAPCS_VFP_CALL_OR_RET_CAND_MAX + 1)) >> + >> /* * Information needed for a discriminated union. A discriminated >> union is handled somewhat differently from an ordinary union. >> >> @@ -716,6 +733,12 @@ struct main_type >> >> ENUM_BITFIELD(type_specific_kind) type_specific_field : 3; >> >> + /* * Is the given TYPE able to be placed in an Aarch64 VFP when used as a call >> + parameter or return parameter, as per the AAPCS64 5.4.2.C. Required to >> + prevent recursive checking. */ >> + >> + unsigned int aapcs_candidate_count : 3; >> + >> /* * Number of fields described for this type. This field appears >> at this location because it packs nicely here. */ >> >>
On 01/10/2019 01:44 PM, Alan Hayward wrote: > > >> On 9 Jan 2019, at 19:01, Pedro Alves <palves@redhat.com> wrote: >> >> On 12/12/2018 09:59 AM, Alan Hayward wrote: >>> gdb.dwarf2/dw2-cp-infcall-ref-static.exp includes a struct which >>> contains itself. Add support to aapcs_is_vfp_call_or_return_candidate to >>> prevent infinite recursion. >> >> But the contained struct is a static field, no? How does that affect >> the calling convention (or whatever is being computed) ? >> >> Without looking at any of this in detail, I'd off hand think that >> this code should be skipping/ignoring static fields? >> >> > > Hmmm... yes, good spot. Tried this in GCC, and the static members are not > passed as args. Right, C++ static data members aren't passed anywhere, they're just globals with fixed addresses. You can think of: struct A { static A a; }; Basically the same as: struct A { }; A a; The difference is mainly that in the former, the global is called 'A::a', while in the latter it's called 'a'. Not that much different from, say: struct A { }; namespace NS { A a; }; Thanks, Pedro Alves > GCC doesn’t have any explicit code to cover this in the > backend - I suspect that by the time it gets there the static members are > effectivley already in a special global var. > > I’ll write up some new test cases to cover the cases with normal structs with > static members. > And then I’ll see if I generate some tests with non-static infinite structs. > Maybe it’ll squash the need for this patch. > > > Thanks, > Alan. > > >>> >>> The simple solution is to pass a counter through the functions and abort once >>> the max limit is reached. However, this does not catch the case where a >>> struct only contains itself and no other members. That can be solved by >>> marking a type as invalid before checking all of it's children. >>> >>> Add a count for the call or ret candidate to main_type, restricted to 3 >>> bits - the value 5 (or above) is used as invalid. >>> >>> This code is testsed by both gdb.base/infcall-nested-structs.exp >>> and gdb.dwarf2/dw2-cp-infcall-ref-static.exp. >>> >>> gdb/ChangeLog: >>> >>> 2018-12-12 Alan Hayward <alan.hayward@arm.com> >>> >>> * aarch64-tdep.c (HA_MAX_NUM_FLDS): Remove define. >>> (aapcs_is_vfp_call_or_return_candidate_1): Remember type. >>> (aapcs_is_vfp_call_or_return_candidate): Likewise. >>> * gdbtypes.h (AAPCS_VFP_CALL_OR_RET_CAND_MAX): Add define. >>> (AAPCS_VFP_CALL_OR_RET_CAND_INVALID): Likewise. >>> (TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE): Likewise. >>> (TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE): Likewise. >>> (struct main_type): Add aapcs_candidate_count. >>> --- >>> gdb/aarch64-tdep.c | 87 ++++++++++++++++++++++++++-------------------- >>> gdb/gdbtypes.h | 23 ++++++++++++ >>> 2 files changed, 72 insertions(+), 38 deletions(-) >>> >>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c >>> index ae56c9ca34..3fc58308d6 100644 >>> --- a/gdb/aarch64-tdep.c >>> +++ b/gdb/aarch64-tdep.c >>> @@ -64,10 +64,6 @@ >>> #define bit(obj,st) (((obj) >> (st)) & 1) >>> #define bits(obj,st,fn) (((obj) >> (st)) & submask ((fn) - (st))) >>> >>> -/* A Homogeneous Floating-Point or Short-Vector Aggregate may have at most >>> - four members. */ >>> -#define HA_MAX_NUM_FLDS 4 >>> - >>> /* All possible aarch64 target descriptors. */ >>> struct target_desc *tdesc_aarch64_list[AARCH64_MAX_SVE_VQ + 1]; >>> >>> @@ -1146,46 +1142,58 @@ aarch64_type_align (struct type *t) >>> >>> /* Worker function for aapcs_is_vfp_call_or_return_candidate. >>> >>> - Return the number of register required, or -1 on failure. >>> + Set TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE to the number of registers required, >>> + or AAPCS_VFP_CALL_OR_RET_CAND_INVALID on failure. >>> >>> When encountering a base element, if FUNDAMENTAL_TYPE is not set then set it >>> to the element, else fail if the type of this element does not match the >>> existing value. */ >>> >>> -static int >>> +static void >>> aapcs_is_vfp_call_or_return_candidate_1 (struct type *type, >>> struct type **fundamental_type) >>> { >>> if (type == nullptr) >>> - return -1; >>> + return; >>> + >>> + /* Check if we have already evaluated this TYPE. */ >>> + if (TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type) >>> + >= AAPCS_VFP_CALL_OR_RET_CAND_INVALID) >>> + return; >>> + >>> + /* Set TYPE to invalid to prevent infinite recursion. */ >>> + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, >>> + AAPCS_VFP_CALL_OR_RET_CAND_INVALID); >>> >>> switch (TYPE_CODE (type)) >>> { >>> case TYPE_CODE_FLT: >>> if (TYPE_LENGTH (type) > 16) >>> - return -1; >>> + return; >>> >>> if (*fundamental_type == nullptr) >>> *fundamental_type = type; >>> else if (TYPE_LENGTH (type) != TYPE_LENGTH (*fundamental_type) >>> || TYPE_CODE (type) != TYPE_CODE (*fundamental_type)) >>> - return -1; >>> + return; >>> >>> - return 1; >>> + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, 1); >>> + return; >>> >>> case TYPE_CODE_COMPLEX: >>> { >>> struct type *target_type = check_typedef (TYPE_TARGET_TYPE (type)); >>> if (TYPE_LENGTH (target_type) > 16) >>> - return -1; >>> + return; >>> >>> if (*fundamental_type == nullptr) >>> *fundamental_type = target_type; >>> else if (TYPE_LENGTH (target_type) != TYPE_LENGTH (*fundamental_type) >>> || TYPE_CODE (target_type) != TYPE_CODE (*fundamental_type)) >>> - return -1; >>> + return; >>> >>> - return 2; >>> + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, 2); >>> + return; >>> } >>> >>> case TYPE_CODE_ARRAY: >>> @@ -1193,28 +1201,33 @@ aapcs_is_vfp_call_or_return_candidate_1 (struct type *type, >>> if (TYPE_VECTOR (type)) >>> { >>> if (TYPE_LENGTH (type) != 8 && TYPE_LENGTH (type) != 16) >>> - return -1; >>> + return; >>> >>> if (*fundamental_type == nullptr) >>> *fundamental_type = type; >>> else if (TYPE_LENGTH (type) != TYPE_LENGTH (*fundamental_type) >>> || TYPE_CODE (type) != TYPE_CODE (*fundamental_type)) >>> - return -1; >>> + return; >>> >>> - return 1; >>> + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, 1); >>> } >>> else >>> { >>> + /* Calculate if type of an element in the array is valid. */ >>> struct type *target_type = TYPE_TARGET_TYPE (type); >>> - int count = aapcs_is_vfp_call_or_return_candidate_1 >>> - (target_type, fundamental_type); >>> + aapcs_is_vfp_call_or_return_candidate_1 (target_type, >>> + fundamental_type); >>> >>> - if (count == -1) >>> - return count; >>> + int count = TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (target_type); >>> + if (count >= AAPCS_VFP_CALL_OR_RET_CAND_INVALID) >>> + return; >>> + >>> + /* Expand to cover the whole array. The set handles overflow. */ >>> >>> count *= (TYPE_LENGTH (type) / TYPE_LENGTH (target_type)); >>> - return count; >>> + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, count); >>> } >>> + return; >>> } >>> >>> case TYPE_CODE_STRUCT: >>> @@ -1226,20 +1239,23 @@ aapcs_is_vfp_call_or_return_candidate_1 (struct type *type, >>> { >>> struct type *member = check_typedef (TYPE_FIELD_TYPE (type, i)); >>> >>> - int sub_count = aapcs_is_vfp_call_or_return_candidate_1 >>> - (member, fundamental_type); >>> - if (sub_count == -1) >>> - return -1; >>> + aapcs_is_vfp_call_or_return_candidate_1 (member, fundamental_type); >>> + >>> + int sub_count = TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (member); >>> + >>> + if (sub_count + count > AAPCS_VFP_CALL_OR_RET_CAND_MAX) >>> + return; >>> + >>> count += sub_count; >>> } >>> - return count; >>> + >>> + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, count); >>> + return; >>> } >>> >>> default: >>> - break; >>> + return; >>> } >>> - >>> - return -1; >>> } >>> >>> /* Return true if an argument, whose type is described by TYPE, can be passed or >>> @@ -1269,16 +1285,11 @@ aapcs_is_vfp_call_or_return_candidate (struct type *type, int *count, >>> >>> *fundamental_type = nullptr; >>> >>> - int ag_count = aapcs_is_vfp_call_or_return_candidate_1 (type, >>> - fundamental_type); >>> + aapcs_is_vfp_call_or_return_candidate_1 (type, fundamental_type); >>> >>> - if (ag_count > 0 && ag_count <= HA_MAX_NUM_FLDS) >>> - { >>> - *count = ag_count; >>> - return true; >>> - } >>> - else >>> - return false; >>> + *count = TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type); >>> + >>> + return (*count < AAPCS_VFP_CALL_OR_RET_CAND_INVALID); >>> } >>> >>> /* AArch64 function call information structure. */ >>> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h >>> index f0adec7a20..600112abf5 100644 >>> --- a/gdb/gdbtypes.h >>> +++ b/gdb/gdbtypes.h >>> @@ -391,6 +391,23 @@ DEF_ENUM_FLAGS_TYPE (enum type_instance_flag_value, type_instance_flags); >>> #define TYPE_ADDRESS_CLASS_ALL(t) (TYPE_INSTANCE_FLAGS(t) \ >>> & TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL) >>> >>> + >>> +/* * Is the given TYPE able to be placed in an Aarch64 VFP when used as a call >>> + parameter or return parameter, as per the AAPCS64 5.4.2.C. Value can be: >>> + 0: the type has not been checked. >>> + 1 to AAPCS_VFP_CALL_OR_RET_CAND_MAX: Valid, and has that many members. >>> + AAPCS_VFP_CALL_OR_RET_CAND_MAX: Invalid. */ >>> + >>> +#define AAPCS_VFP_CALL_OR_RET_CAND_MAX 4 >>> +#define AAPCS_VFP_CALL_OR_RET_CAND_INVALID (AAPCS_VFP_CALL_OR_RET_CAND_MAX + 1) >>> + >>> +#define TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE(thistype) \ >>> + (TYPE_MAIN_TYPE (thistype)->aapcs_candidate_count) >>> + >>> +#define TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE(thistype, val) \ >>> + TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (thistype) = \ >>> + (std::min (val, AAPCS_VFP_CALL_OR_RET_CAND_MAX + 1)) >>> + >>> /* * Information needed for a discriminated union. A discriminated >>> union is handled somewhat differently from an ordinary union. >>> >>> @@ -716,6 +733,12 @@ struct main_type >>> >>> ENUM_BITFIELD(type_specific_kind) type_specific_field : 3; >>> >>> + /* * Is the given TYPE able to be placed in an Aarch64 VFP when used as a call >>> + parameter or return parameter, as per the AAPCS64 5.4.2.C. Required to >>> + prevent recursive checking. */ >>> + >>> + unsigned int aapcs_candidate_count : 3; >>> + >>> /* * Number of fields described for this type. This field appears >>> at this location because it packs nicely here. */ >>> >>> >
> On 10 Jan 2019, at 16:07, Pedro Alves <palves@redhat.com> wrote: > > On 01/10/2019 01:44 PM, Alan Hayward wrote: >> >> >>> On 9 Jan 2019, at 19:01, Pedro Alves <palves@redhat.com> wrote: >>> >>> On 12/12/2018 09:59 AM, Alan Hayward wrote: >>>> gdb.dwarf2/dw2-cp-infcall-ref-static.exp includes a struct which >>>> contains itself. Add support to aapcs_is_vfp_call_or_return_candidate to >>>> prevent infinite recursion. >>> >>> But the contained struct is a static field, no? How does that affect >>> the calling convention (or whatever is being computed) ? >>> >>> Without looking at any of this in detail, I'd off hand think that >>> this code should be skipping/ignoring static fields? >>> >>> >> >> Hmmm... yes, good spot. Tried this in GCC, and the static members are not >> passed as args. > > Right, C++ static data members aren't passed anywhere, they're just globals > with fixed addresses. You can think of: > > struct A > { > static A a; > }; > > Basically the same as: > > struct A > { > }; > A a; > > The difference is mainly that in the former, the global is > called 'A::a', while in the latter it's called 'a'. Not that > much different from, say: > > struct A > { > }; > namespace NS > { > A a; > }; > Just an fyi, By creating a c++ test case, I’ve also found some issues with empty structs inside structs (c++ classes have a minimum size of 0, causing g++/clang to reject passing it in registers because it’s not aligned properly). Also, looks like it gdb_asserts when running the test on x86 too. This patch will still be needed for infinite recursive structs, but I’ll throw it in as part of a new patch series. Alan. > Thanks, > Pedro Alves > >> GCC doesn’t have any explicit code to cover this in the >> backend - I suspect that by the time it gets there the static members are >> effectivley already in a special global var. > > >> >> I’ll write up some new test cases to cover the cases with normal structs with >> static members. >> And then I’ll see if I generate some tests with non-static infinite structs. >> Maybe it’ll squash the need for this patch. >> >> >> Thanks, >> Alan. >> >> >>>> >>>> The simple solution is to pass a counter through the functions and abort once >>>> the max limit is reached. However, this does not catch the case where a >>>> struct only contains itself and no other members. That can be solved by >>>> marking a type as invalid before checking all of it's children. >>>> >>>> Add a count for the call or ret candidate to main_type, restricted to 3 >>>> bits - the value 5 (or above) is used as invalid. >>>> >>>> This code is testsed by both gdb.base/infcall-nested-structs.exp >>>> and gdb.dwarf2/dw2-cp-infcall-ref-static.exp. >>>> >>>> gdb/ChangeLog: >>>> >>>> 2018-12-12 Alan Hayward <alan.hayward@arm.com> >>>> >>>> * aarch64-tdep.c (HA_MAX_NUM_FLDS): Remove define. >>>> (aapcs_is_vfp_call_or_return_candidate_1): Remember type. >>>> (aapcs_is_vfp_call_or_return_candidate): Likewise. >>>> * gdbtypes.h (AAPCS_VFP_CALL_OR_RET_CAND_MAX): Add define. >>>> (AAPCS_VFP_CALL_OR_RET_CAND_INVALID): Likewise. >>>> (TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE): Likewise. >>>> (TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE): Likewise. >>>> (struct main_type): Add aapcs_candidate_count. >>>> --- >>>> gdb/aarch64-tdep.c | 87 ++++++++++++++++++++++++++-------------------- >>>> gdb/gdbtypes.h | 23 ++++++++++++ >>>> 2 files changed, 72 insertions(+), 38 deletions(-) >>>> >>>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c >>>> index ae56c9ca34..3fc58308d6 100644 >>>> --- a/gdb/aarch64-tdep.c >>>> +++ b/gdb/aarch64-tdep.c >>>> @@ -64,10 +64,6 @@ >>>> #define bit(obj,st) (((obj) >> (st)) & 1) >>>> #define bits(obj,st,fn) (((obj) >> (st)) & submask ((fn) - (st))) >>>> >>>> -/* A Homogeneous Floating-Point or Short-Vector Aggregate may have at most >>>> - four members. */ >>>> -#define HA_MAX_NUM_FLDS 4 >>>> - >>>> /* All possible aarch64 target descriptors. */ >>>> struct target_desc *tdesc_aarch64_list[AARCH64_MAX_SVE_VQ + 1]; >>>> >>>> @@ -1146,46 +1142,58 @@ aarch64_type_align (struct type *t) >>>> >>>> /* Worker function for aapcs_is_vfp_call_or_return_candidate. >>>> >>>> - Return the number of register required, or -1 on failure. >>>> + Set TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE to the number of registers required, >>>> + or AAPCS_VFP_CALL_OR_RET_CAND_INVALID on failure. >>>> >>>> When encountering a base element, if FUNDAMENTAL_TYPE is not set then set it >>>> to the element, else fail if the type of this element does not match the >>>> existing value. */ >>>> >>>> -static int >>>> +static void >>>> aapcs_is_vfp_call_or_return_candidate_1 (struct type *type, >>>> struct type **fundamental_type) >>>> { >>>> if (type == nullptr) >>>> - return -1; >>>> + return; >>>> + >>>> + /* Check if we have already evaluated this TYPE. */ >>>> + if (TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type) >>>> + >= AAPCS_VFP_CALL_OR_RET_CAND_INVALID) >>>> + return; >>>> + >>>> + /* Set TYPE to invalid to prevent infinite recursion. */ >>>> + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, >>>> + AAPCS_VFP_CALL_OR_RET_CAND_INVALID); >>>> >>>> switch (TYPE_CODE (type)) >>>> { >>>> case TYPE_CODE_FLT: >>>> if (TYPE_LENGTH (type) > 16) >>>> - return -1; >>>> + return; >>>> >>>> if (*fundamental_type == nullptr) >>>> *fundamental_type = type; >>>> else if (TYPE_LENGTH (type) != TYPE_LENGTH (*fundamental_type) >>>> || TYPE_CODE (type) != TYPE_CODE (*fundamental_type)) >>>> - return -1; >>>> + return; >>>> >>>> - return 1; >>>> + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, 1); >>>> + return; >>>> >>>> case TYPE_CODE_COMPLEX: >>>> { >>>> struct type *target_type = check_typedef (TYPE_TARGET_TYPE (type)); >>>> if (TYPE_LENGTH (target_type) > 16) >>>> - return -1; >>>> + return; >>>> >>>> if (*fundamental_type == nullptr) >>>> *fundamental_type = target_type; >>>> else if (TYPE_LENGTH (target_type) != TYPE_LENGTH (*fundamental_type) >>>> || TYPE_CODE (target_type) != TYPE_CODE (*fundamental_type)) >>>> - return -1; >>>> + return; >>>> >>>> - return 2; >>>> + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, 2); >>>> + return; >>>> } >>>> >>>> case TYPE_CODE_ARRAY: >>>> @@ -1193,28 +1201,33 @@ aapcs_is_vfp_call_or_return_candidate_1 (struct type *type, >>>> if (TYPE_VECTOR (type)) >>>> { >>>> if (TYPE_LENGTH (type) != 8 && TYPE_LENGTH (type) != 16) >>>> - return -1; >>>> + return; >>>> >>>> if (*fundamental_type == nullptr) >>>> *fundamental_type = type; >>>> else if (TYPE_LENGTH (type) != TYPE_LENGTH (*fundamental_type) >>>> || TYPE_CODE (type) != TYPE_CODE (*fundamental_type)) >>>> - return -1; >>>> + return; >>>> >>>> - return 1; >>>> + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, 1); >>>> } >>>> else >>>> { >>>> + /* Calculate if type of an element in the array is valid. */ >>>> struct type *target_type = TYPE_TARGET_TYPE (type); >>>> - int count = aapcs_is_vfp_call_or_return_candidate_1 >>>> - (target_type, fundamental_type); >>>> + aapcs_is_vfp_call_or_return_candidate_1 (target_type, >>>> + fundamental_type); >>>> >>>> - if (count == -1) >>>> - return count; >>>> + int count = TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (target_type); >>>> + if (count >= AAPCS_VFP_CALL_OR_RET_CAND_INVALID) >>>> + return; >>>> + >>>> + /* Expand to cover the whole array. The set handles overflow. */ >>>> >>>> count *= (TYPE_LENGTH (type) / TYPE_LENGTH (target_type)); >>>> - return count; >>>> + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, count); >>>> } >>>> + return; >>>> } >>>> >>>> case TYPE_CODE_STRUCT: >>>> @@ -1226,20 +1239,23 @@ aapcs_is_vfp_call_or_return_candidate_1 (struct type *type, >>>> { >>>> struct type *member = check_typedef (TYPE_FIELD_TYPE (type, i)); >>>> >>>> - int sub_count = aapcs_is_vfp_call_or_return_candidate_1 >>>> - (member, fundamental_type); >>>> - if (sub_count == -1) >>>> - return -1; >>>> + aapcs_is_vfp_call_or_return_candidate_1 (member, fundamental_type); >>>> + >>>> + int sub_count = TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (member); >>>> + >>>> + if (sub_count + count > AAPCS_VFP_CALL_OR_RET_CAND_MAX) >>>> + return; >>>> + >>>> count += sub_count; >>>> } >>>> - return count; >>>> + >>>> + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, count); >>>> + return; >>>> } >>>> >>>> default: >>>> - break; >>>> + return; >>>> } >>>> - >>>> - return -1; >>>> } >>>> >>>> /* Return true if an argument, whose type is described by TYPE, can be passed or >>>> @@ -1269,16 +1285,11 @@ aapcs_is_vfp_call_or_return_candidate (struct type *type, int *count, >>>> >>>> *fundamental_type = nullptr; >>>> >>>> - int ag_count = aapcs_is_vfp_call_or_return_candidate_1 (type, >>>> - fundamental_type); >>>> + aapcs_is_vfp_call_or_return_candidate_1 (type, fundamental_type); >>>> >>>> - if (ag_count > 0 && ag_count <= HA_MAX_NUM_FLDS) >>>> - { >>>> - *count = ag_count; >>>> - return true; >>>> - } >>>> - else >>>> - return false; >>>> + *count = TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type); >>>> + >>>> + return (*count < AAPCS_VFP_CALL_OR_RET_CAND_INVALID); >>>> } >>>> >>>> /* AArch64 function call information structure. */ >>>> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h >>>> index f0adec7a20..600112abf5 100644 >>>> --- a/gdb/gdbtypes.h >>>> +++ b/gdb/gdbtypes.h >>>> @@ -391,6 +391,23 @@ DEF_ENUM_FLAGS_TYPE (enum type_instance_flag_value, type_instance_flags); >>>> #define TYPE_ADDRESS_CLASS_ALL(t) (TYPE_INSTANCE_FLAGS(t) \ >>>> & TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL) >>>> >>>> + >>>> +/* * Is the given TYPE able to be placed in an Aarch64 VFP when used as a call >>>> + parameter or return parameter, as per the AAPCS64 5.4.2.C. Value can be: >>>> + 0: the type has not been checked. >>>> + 1 to AAPCS_VFP_CALL_OR_RET_CAND_MAX: Valid, and has that many members. >>>> + AAPCS_VFP_CALL_OR_RET_CAND_MAX: Invalid. */ >>>> + >>>> +#define AAPCS_VFP_CALL_OR_RET_CAND_MAX 4 >>>> +#define AAPCS_VFP_CALL_OR_RET_CAND_INVALID (AAPCS_VFP_CALL_OR_RET_CAND_MAX + 1) >>>> + >>>> +#define TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE(thistype) \ >>>> + (TYPE_MAIN_TYPE (thistype)->aapcs_candidate_count) >>>> + >>>> +#define TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE(thistype, val) \ >>>> + TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (thistype) = \ >>>> + (std::min (val, AAPCS_VFP_CALL_OR_RET_CAND_MAX + 1)) >>>> + >>>> /* * Information needed for a discriminated union. A discriminated >>>> union is handled somewhat differently from an ordinary union. >>>> >>>> @@ -716,6 +733,12 @@ struct main_type >>>> >>>> ENUM_BITFIELD(type_specific_kind) type_specific_field : 3; >>>> >>>> + /* * Is the given TYPE able to be placed in an Aarch64 VFP when used as a call >>>> + parameter or return parameter, as per the AAPCS64 5.4.2.C. Required to >>>> + prevent recursive checking. */ >>>> + >>>> + unsigned int aapcs_candidate_count : 3; >>>> + >>>> /* * Number of fields described for this type. This field appears >>>> at this location because it packs nicely here. */
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index ae56c9ca34..3fc58308d6 100644 --- a/gdb/aarch64-tdep.c +++ b/gdb/aarch64-tdep.c @@ -64,10 +64,6 @@ #define bit(obj,st) (((obj) >> (st)) & 1) #define bits(obj,st,fn) (((obj) >> (st)) & submask ((fn) - (st))) -/* A Homogeneous Floating-Point or Short-Vector Aggregate may have at most - four members. */ -#define HA_MAX_NUM_FLDS 4 - /* All possible aarch64 target descriptors. */ struct target_desc *tdesc_aarch64_list[AARCH64_MAX_SVE_VQ + 1]; @@ -1146,46 +1142,58 @@ aarch64_type_align (struct type *t) /* Worker function for aapcs_is_vfp_call_or_return_candidate. - Return the number of register required, or -1 on failure. + Set TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE to the number of registers required, + or AAPCS_VFP_CALL_OR_RET_CAND_INVALID on failure. When encountering a base element, if FUNDAMENTAL_TYPE is not set then set it to the element, else fail if the type of this element does not match the existing value. */ -static int +static void aapcs_is_vfp_call_or_return_candidate_1 (struct type *type, struct type **fundamental_type) { if (type == nullptr) - return -1; + return; + + /* Check if we have already evaluated this TYPE. */ + if (TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type) + >= AAPCS_VFP_CALL_OR_RET_CAND_INVALID) + return; + + /* Set TYPE to invalid to prevent infinite recursion. */ + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, + AAPCS_VFP_CALL_OR_RET_CAND_INVALID); switch (TYPE_CODE (type)) { case TYPE_CODE_FLT: if (TYPE_LENGTH (type) > 16) - return -1; + return; if (*fundamental_type == nullptr) *fundamental_type = type; else if (TYPE_LENGTH (type) != TYPE_LENGTH (*fundamental_type) || TYPE_CODE (type) != TYPE_CODE (*fundamental_type)) - return -1; + return; - return 1; + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, 1); + return; case TYPE_CODE_COMPLEX: { struct type *target_type = check_typedef (TYPE_TARGET_TYPE (type)); if (TYPE_LENGTH (target_type) > 16) - return -1; + return; if (*fundamental_type == nullptr) *fundamental_type = target_type; else if (TYPE_LENGTH (target_type) != TYPE_LENGTH (*fundamental_type) || TYPE_CODE (target_type) != TYPE_CODE (*fundamental_type)) - return -1; + return; - return 2; + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, 2); + return; } case TYPE_CODE_ARRAY: @@ -1193,28 +1201,33 @@ aapcs_is_vfp_call_or_return_candidate_1 (struct type *type, if (TYPE_VECTOR (type)) { if (TYPE_LENGTH (type) != 8 && TYPE_LENGTH (type) != 16) - return -1; + return; if (*fundamental_type == nullptr) *fundamental_type = type; else if (TYPE_LENGTH (type) != TYPE_LENGTH (*fundamental_type) || TYPE_CODE (type) != TYPE_CODE (*fundamental_type)) - return -1; + return; - return 1; + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, 1); } else { + /* Calculate if type of an element in the array is valid. */ struct type *target_type = TYPE_TARGET_TYPE (type); - int count = aapcs_is_vfp_call_or_return_candidate_1 - (target_type, fundamental_type); + aapcs_is_vfp_call_or_return_candidate_1 (target_type, + fundamental_type); - if (count == -1) - return count; + int count = TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (target_type); + if (count >= AAPCS_VFP_CALL_OR_RET_CAND_INVALID) + return; + + /* Expand to cover the whole array. The set handles overflow. */ count *= (TYPE_LENGTH (type) / TYPE_LENGTH (target_type)); - return count; + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, count); } + return; } case TYPE_CODE_STRUCT: @@ -1226,20 +1239,23 @@ aapcs_is_vfp_call_or_return_candidate_1 (struct type *type, { struct type *member = check_typedef (TYPE_FIELD_TYPE (type, i)); - int sub_count = aapcs_is_vfp_call_or_return_candidate_1 - (member, fundamental_type); - if (sub_count == -1) - return -1; + aapcs_is_vfp_call_or_return_candidate_1 (member, fundamental_type); + + int sub_count = TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (member); + + if (sub_count + count > AAPCS_VFP_CALL_OR_RET_CAND_MAX) + return; + count += sub_count; } - return count; + + TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type, count); + return; } default: - break; + return; } - - return -1; } /* Return true if an argument, whose type is described by TYPE, can be passed or @@ -1269,16 +1285,11 @@ aapcs_is_vfp_call_or_return_candidate (struct type *type, int *count, *fundamental_type = nullptr; - int ag_count = aapcs_is_vfp_call_or_return_candidate_1 (type, - fundamental_type); + aapcs_is_vfp_call_or_return_candidate_1 (type, fundamental_type); - if (ag_count > 0 && ag_count <= HA_MAX_NUM_FLDS) - { - *count = ag_count; - return true; - } - else - return false; + *count = TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (type); + + return (*count < AAPCS_VFP_CALL_OR_RET_CAND_INVALID); } /* AArch64 function call information structure. */ diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h index f0adec7a20..600112abf5 100644 --- a/gdb/gdbtypes.h +++ b/gdb/gdbtypes.h @@ -391,6 +391,23 @@ DEF_ENUM_FLAGS_TYPE (enum type_instance_flag_value, type_instance_flags); #define TYPE_ADDRESS_CLASS_ALL(t) (TYPE_INSTANCE_FLAGS(t) \ & TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL) + +/* * Is the given TYPE able to be placed in an Aarch64 VFP when used as a call + parameter or return parameter, as per the AAPCS64 5.4.2.C. Value can be: + 0: the type has not been checked. + 1 to AAPCS_VFP_CALL_OR_RET_CAND_MAX: Valid, and has that many members. + AAPCS_VFP_CALL_OR_RET_CAND_MAX: Invalid. */ + +#define AAPCS_VFP_CALL_OR_RET_CAND_MAX 4 +#define AAPCS_VFP_CALL_OR_RET_CAND_INVALID (AAPCS_VFP_CALL_OR_RET_CAND_MAX + 1) + +#define TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE(thistype) \ + (TYPE_MAIN_TYPE (thistype)->aapcs_candidate_count) + +#define TYPE_SET_AAPCS_VFP_CALL_OR_RET_CANDIDATE(thistype, val) \ + TYPE_AAPCS_VFP_CALL_OR_RET_CANDIDATE (thistype) = \ + (std::min (val, AAPCS_VFP_CALL_OR_RET_CAND_MAX + 1)) + /* * Information needed for a discriminated union. A discriminated union is handled somewhat differently from an ordinary union. @@ -716,6 +733,12 @@ struct main_type ENUM_BITFIELD(type_specific_kind) type_specific_field : 3; + /* * Is the given TYPE able to be placed in an Aarch64 VFP when used as a call + parameter or return parameter, as per the AAPCS64 5.4.2.C. Required to + prevent recursive checking. */ + + unsigned int aapcs_candidate_count : 3; + /* * Number of fields described for this type. This field appears at this location because it packs nicely here. */