Re: [PATCH] accept all C integer types in function parameters referenced by alloc_align (PR 88363)

[Martin Sebor <msebor at gmail dot com>]

On 12/11/18 12:17 AM, Jakub Jelinek wrote:
> On Mon, Dec 10, 2018 at 04:30:11PM -0700, Martin Sebor wrote:
> > Some of my testing exposed a minor problem in GCC 9's validation
> > of the type of function parameters referred to by attribute
> > positional arguments.  Whereas GCC 8 accepts all C integer types,
> > including enumerated types, such as:
> >
> >    enum AllocAlign { Align16 = 16, Align32 = 32 };
> >
> >    __attribute__ ((alloc_align (1))) void*
> >    alloc (size_t, enum AllocAlign)
> >
> > GCC 9 only accepts signed and unsigned integer types.  This change
> > (introduced by myself) was unintentional, and a fix for it is in
> > the attached trivial patch.  I plan to commit it without approval
> > in the next day or so unless any concerns or suggestions come up.

Attached is an updated version of the patch that restores
the original behavior for the positional argument validation
(i.e., prior to r266195) for integral types except bool as
discussed.

Martin

Index: gcc/c-family/c-attribs.c
===================================================================
--- gcc/c-family/c-attribs.c	(revision 266966)
+++ gcc/c-family/c-attribs.c	(working copy)
@@ -498,10 +498,11 @@ attribute_takes_identifier_p (const_tree attr_id)
 
 /* Verify that argument value POS at position ARGNO to attribute NAME
    applied to function TYPE refers to a function parameter at position
-   POS and the expected type CODE.  If so, return POS after default
-   conversions, if any.  Otherwise, issue appropriate warnings and
-   return null.  A non-zero 1-based ARGNO should be passed ib by
-   callers only for attributes with more than one argument.  */
+   POS and the expected type CODE.  Treat CODE == INTEGER_TYPE as
+   matching all C integral types except bool.  If successful, return
+   POS after default conversions, if any.  Otherwise, issue appropriate
+   warnings and return null.  A non-zero 1-based ARGNO should be passed
+   in by callers only for attributes with more than one argument.  */
 
 tree
 positional_argument (const_tree fntype, const_tree atname, tree pos,
@@ -630,11 +631,11 @@ positional_argument (const_tree fntype, const_tree
 	  return NULL_TREE;
 	}
 
-      /* Where the expected code is STRING_CST accept any pointer
-	 to a narrow character type, qualified or otherwise.  */
       bool type_match;
       if (code == STRING_CST && POINTER_TYPE_P (argtype))
 	{
+	  /* Where the expected code is STRING_CST accept any pointer
+	     to a narrow character type, qualified or otherwise.  */
 	  tree type = TREE_TYPE (argtype);
 	  type = TYPE_MAIN_VARIANT (type);
 	  type_match = (type == char_type_node
@@ -641,6 +642,11 @@ positional_argument (const_tree fntype, const_tree
 			|| type == signed_char_type_node
 			|| type == unsigned_char_type_node);
 	}
+      else if (code == INTEGER_TYPE)
+	/* For integers, accept enums, wide characters and other types
+	   that match INTEGRAL_TYPE_P except for bool.  */
+	type_match = (INTEGRAL_TYPE_P (argtype)
+		      && TREE_CODE (argtype) != BOOLEAN_TYPE);
       else
 	type_match = TREE_CODE (argtype) == code;
 
Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 266966)
+++ gcc/doc/extend.texi	(working copy)
@@ -2471,7 +2471,9 @@ The @code{aligned} attribute can also be used for
 @item alloc_align (@var{position})
 @cindex @code{alloc_align} function attribute
 The @code{alloc_align} attribute may be applied to a function that
-returns a pointer and takes at least one argument of an integer type.
+returns a pointer and takes at least one argument of an integer or
+enumerated type, but not @code{bool}.  Arguments of other types are
+diagnosed.
 It indicates that the returned pointer is aligned on a boundary given
 by the function argument at @var{position}.  Meaningful alignments are
 powers of 2 greater than one.  GCC uses this information to improve
@@ -2495,7 +2497,9 @@ given by parameter 1.
 @itemx alloc_size (@var{position-1}, @var{position-2})
 @cindex @code{alloc_size} function attribute
 The @code{alloc_size} attribute may be applied to a function that
-returns a pointer and takes at least one argument of an integer type.
+returns a pointer and takes at least one argument of an integer or
+enumerated type, but not @code{bool}.  Arguments of other types are
+diagnosed.
 It indicates that the returned pointer points to memory whose size is
 given by the function argument at @var{position-1}, or by the product
 of the arguments at @var{position-1} and @var{position-2}.  Meaningful
Index: gcc/testsuite/c-c++-common/attributes-4.c
===================================================================
--- gcc/testsuite/c-c++-common/attributes-4.c	(nonexistent)
+++ gcc/testsuite/c-c++-common/attributes-4.c	(working copy)
@@ -0,0 +1,47 @@
+/* PR c/88363 - alloc_align attribute doesn't accept enumerated arguments
+   Verify that attribute positional arguments can refer to all C integer
+   types in both C and C++.
+   { dg-do compile }
+   { dg-options "-Wall" }
+   { dg-options "-Wall -Wno-c++-compat" { target c } } */
+
+#define ATTR(...) __attribute__ ((__VA_ARGS__))
+
+#if __cplusplus == 199711L
+typedef __CHAR16_TYPE__ char16_t;
+typedef __CHAR32_TYPE__ char32_t;
+#elif !__cplusplus
+typedef _Bool           bool;
+typedef __CHAR16_TYPE__ char16_t;
+typedef __CHAR32_TYPE__ char32_t;
+typedef __WCHAR_TYPE__  wchar_t;
+#endif
+
+enum A { A0 };
+
+ATTR (alloc_align (1)) void* falloc_align_char (char);
+ATTR (alloc_align (1)) void* falloc_align_char16 (char16_t);
+ATTR (alloc_align (1)) void* falloc_align_char32 (char32_t);
+ATTR (alloc_align (1)) void* falloc_align_wchar (wchar_t);
+/* Using an enum might make sense in an API that limits the alignments
+   it accepts to just the set of the defined enumerators.   */
+ATTR (alloc_align (1)) void* falloc_align_enum (enum A);
+ATTR (alloc_align (1)) void* falloc_align_int128 (__int128_t);
+
+
+ATTR (alloc_align (1)) void* falloc_size_char (char);
+ATTR (alloc_size (1)) void* falloc_size_char16 (char16_t);
+ATTR (alloc_size (1)) void* falloc_size_char32 (char32_t);
+ATTR (alloc_size (1)) void* falloc_size_wchar (wchar_t);
+ATTR (alloc_size (1)) void* falloc_size_enum (enum A);
+ATTR (alloc_align (1)) void* falloc_size_int128 (__int128_t);
+
+
+typedef struct { int i; } S;
+
+/* Using bool is most likely a bug and so diagnosed even though
+   it could be accepted.  None of the other types makes sense.  */
+ATTR (alloc_align (1)) void* falloc_align_bool (bool);      /* { dg-warning "attribute argument value .1. refers to parameter type .\(_Bool|bool\)" } */
+ATTR (alloc_align (1)) void* falloc_align_float (float);    /* { dg-warning "attribute argument value .1. refers to parameter type .float" } */
+ATTR (alloc_align (1)) void* falloc_align_voidp (void*);    /* { dg-warning "attribute argument value .1. refers to parameter type .void ?\\\*" } */
+ATTR (alloc_align (1)) void* falloc_align_struct (S);       /* { dg-warning "attribute argument value .1. refers to parameter type .S" } */