public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] AArch64: Prevent infinite recursion when checking AAPCS types
@ 2018-12-12  9:59 Alan Hayward
  2019-01-09 16:21 ` [PING][PATCH] " Alan Hayward
  2019-01-09 19:01 ` [PATCH] " Pedro Alves
  0 siblings, 2 replies; 6+ messages in thread
From: Alan Hayward @ 2018-12-12  9:59 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd, Alan Hayward

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)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PING][PATCH] AArch64: Prevent infinite recursion when checking AAPCS types
  2018-12-12  9:59 [PATCH] AArch64: Prevent infinite recursion when checking AAPCS types Alan Hayward
@ 2019-01-09 16:21 ` Alan Hayward
  2019-01-09 19:01 ` [PATCH] " Pedro Alves
  1 sibling, 0 replies; 6+ messages in thread
From: Alan Hayward @ 2019-01-09 16:21 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd

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)
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] AArch64: Prevent infinite recursion when checking AAPCS types
  2018-12-12  9:59 [PATCH] AArch64: Prevent infinite recursion when checking AAPCS types Alan Hayward
  2019-01-09 16:21 ` [PING][PATCH] " Alan Hayward
@ 2019-01-09 19:01 ` Pedro Alves
  2019-01-10 13:44   ` Alan Hayward
  1 sibling, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2019-01-09 19:01 UTC (permalink / raw)
  To: Alan Hayward, gdb-patches; +Cc: nd

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.  */
>  
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] AArch64: Prevent infinite recursion when checking AAPCS types
  2019-01-09 19:01 ` [PATCH] " Pedro Alves
@ 2019-01-10 13:44   ` Alan Hayward
  2019-01-10 16:07     ` Pedro Alves
  0 siblings, 1 reply; 6+ messages in thread
From: Alan Hayward @ 2019-01-10 13:44 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, nd



> 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.  */
>> 
>> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] AArch64: Prevent infinite recursion when checking AAPCS types
  2019-01-10 13:44   ` Alan Hayward
@ 2019-01-10 16:07     ` Pedro Alves
  2019-01-15 10:30       ` Alan Hayward
  0 siblings, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2019-01-10 16:07 UTC (permalink / raw)
  To: Alan Hayward; +Cc: gdb-patches, nd

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.  */
>>>
>>>
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] AArch64: Prevent infinite recursion when checking AAPCS types
  2019-01-10 16:07     ` Pedro Alves
@ 2019-01-15 10:30       ` Alan Hayward
  0 siblings, 0 replies; 6+ messages in thread
From: Alan Hayward @ 2019-01-15 10:30 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, nd



> 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.  */


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-01-15 10:30 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-12  9:59 [PATCH] AArch64: Prevent infinite recursion when checking AAPCS types Alan Hayward
2019-01-09 16:21 ` [PING][PATCH] " Alan Hayward
2019-01-09 19:01 ` [PATCH] " Pedro Alves
2019-01-10 13:44   ` Alan Hayward
2019-01-10 16:07     ` Pedro Alves
2019-01-15 10:30       ` Alan Hayward

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).