[PATCH] diagnose bogus assume_aligned attributes (PR 87533)

[PATCH] diagnose bogus assume_aligned attributes (PR 87533)

[Martin Sebor]

[-- Attachment #1: Type: text/plain, Size: 602 bytes --]

While working on tests for an enhancement in the area of
attributes I noticed that the handler for attribute assume_aligned
(among others) does only a superficial job of detecting meaningless
specifications such as using the attribute on a function returning
void or alignments that aren't powers of two, out-of-range offsets,
and so on.  None of the expected warnings in the test case triggers
(Clang diagnoses all of them).

The attached patch improves the detection of these nonsensical
constructs, and brings GCC closer to the more thorough job other
compilers do.  Tested on x86_64-linux.

Martin

[-- Attachment #2: gcc-87533.diff --]
[-- Type: text/x-patch, Size: 4481 bytes --]

PR middle-end/87533 - bogus assume_aligned attribute silently accepted

gcc/c-family/ChangeLog:

	PR middle-end/87533
	* c-attribs.c (handle_assume_aligned_attribute): Diagnose and
	reject invalid attribute specifications.

gcc/testsuite/ChangeLog:

	PR middle-end/87533
	* gcc.dg/attr-assume_aligned-4.c: New test.

Index: gcc/c-family/c-attribs.c
===================================================================
@@ -2451,23 +2454,63 @@ static tree
    struct attribute_spec.handler.  */
 
 static tree
-handle_assume_aligned_attribute (tree *, tree, tree args, int,
+handle_assume_aligned_attribute (tree *node, tree name, tree args, int,
 				 bool *no_add_attrs)
 {
+  tree decl = *node;
+  tree rettype = TREE_TYPE (decl);
+  if (TREE_CODE (rettype) != POINTER_TYPE)
+    {
+      warning (OPT_Wattributes,
+	       "%qE attribute ignored on a function returning %qT",
+	       name, rettype);
+      *no_add_attrs = true;
+      return NULL_TREE;
+    }
+
+  /* The alignment specified by the first argument.  */
+  tree align = NULL_TREE;
+
   for (; args; args = TREE_CHAIN (args))
     {
-      tree position = TREE_VALUE (args);
-      if (position && TREE_CODE (position) != IDENTIFIER_NODE
-	  && TREE_CODE (position) != FUNCTION_DECL)
-	position = default_conversion (position);
+      tree val = TREE_VALUE (args);
+      if (val && TREE_CODE (val) != IDENTIFIER_NODE
+	  && TREE_CODE (val) != FUNCTION_DECL)
+	val = default_conversion (val);
 
-      if (TREE_CODE (position) != INTEGER_CST)
+      if (!tree_fits_shwi_p (val))
 	{
 	  warning (OPT_Wattributes,
-		   "assume_aligned parameter not integer constant");
+		   "%qE attribute argument %E is not an integer constant",
+		   name, val);
 	  *no_add_attrs = true;
 	  return NULL_TREE;
 	}
+
+      if (!align)
+	{
+	  /* Validate and save the alignment.  */
+	  if (!integer_pow2p (val))
+	    {
+	      warning (OPT_Wattributes,
+		       "%qE attribute argument %E is not a power of 2",
+		       name, val);
+	      *no_add_attrs = true;
+	      return NULL_TREE;
+	}
+
+	  align = val;
+	}
+      else if (tree_int_cst_sgn (val) < 0 || tree_int_cst_le (align, val))
+	{
+	  /* The misalignment specified by the second argument
+	     must be non-negative and less than the alignment.  */
+	  warning (OPT_Wattributes,
+		   "%qE attribute argument %E is not in the range [0, %E)",
+		   name, val, align);
+	  *no_add_attrs = true;
+	  return NULL_TREE;
+	}
     }
   return NULL_TREE;
 }
Index: gcc/testsuite/gcc.dg/attr-assume_aligned-4.c
===================================================================
--- gcc/testsuite/gcc.dg/attr-assume_aligned-4.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/attr-assume_aligned-4.c	(working copy)
@@ -0,0 +1,36 @@
+/* PR middle-end/87533 - bogus assume_aligned attribute silently accepted
+   { dg-do compile }
+   { dg-options "-Wall" } */
+
+#define A(...)  __attribute__ ((assume_aligned (__VA_ARGS__)))
+
+A (1) void fv_1 (void);       /* { dg-warning ".assume_aligned. attribute ignored on a function returning .void." } */
+
+A (1) int fi_1 (void);        /* { dg-warning ".assume_aligned. attribute ignored on a function returning .int." } */
+
+A (-1) void* fpv_m1 (void);   /* { dg-warning ".assume_aligned. attribute argument -1 is not a power of 2" } */
+
+A (0) void* fpv_0 (void);     /* { dg-warning ".assume_aligned. attribute argument 0 is not a power of 2" } */
+
+/* Alignment of 1 is fine, it just doesn't offer any benefits.  */
+A (1) void* fpv_1 (void);
+
+A (3) void* fpv_3 (void);     /* { dg-warning ".assume_aligned. attribute argument 3 is not a power of 2" } */
+
+A (16383) void* fpv_16km1 (void);     /* { dg-warning ".assume_aligned. attribute argument 16383 is not a power of 2" } */
+A (16384) void* fpv_16k (void);
+A (16385) void* fpv_16kp1 (void);    /* { dg-warning ".assume_aligned. attribute argument 16385 is not a power of 2" } */
+
+A (32767) void* fpv_32km1 (void);     /* { dg-warning ".assume_aligned. attribute argument 32767 is not a power of 2" } */
+
+A (4, -1) void* fpv_4_m1 (void);      /* { dg-warning ".assume_aligned. attribute argument -1 is not in the range \\\[0, 4\\\)" } */
+
+A (4, 0) void* fpv_4_0 (void);
+A (4, 1) void* fpv_4_1 (void);
+A (4, 2) void* fpv_4_2 (void);
+A (4, 3) void* fpv_4_3 (void);
+
+A (4, 4) void* fpv_4_3 (void);        /* { dg-warning ".assume_aligned. attribute argument 4 is not in the range \\\[0, 4\\\)" } */
+
+A (4) void* gpv_4_3 (void);
+A (2) void* gpv_4_3 (void);

Re: [PATCH] diagnose bogus assume_aligned attributes (PR 87533)

[Jeff Law]

On 10/5/18 4:56 PM, Martin Sebor wrote:
> While working on tests for an enhancement in the area of
> attributes I noticed that the handler for attribute assume_aligned
> (among others) does only a superficial job of detecting meaningless
> specifications such as using the attribute on a function returning
> void or alignments that aren't powers of two, out-of-range offsets,
> and so on.  None of the expected warnings in the test case triggers
> (Clang diagnoses all of them).
> 
> The attached patch improves the detection of these nonsensical
> constructs, and brings GCC closer to the more thorough job other
> compilers do.  Tested on x86_64-linux.
> 
> Martin
> 
> gcc-87533.diff
> 
> PR middle-end/87533 - bogus assume_aligned attribute silently accepted
> 
> gcc/c-family/ChangeLog:
> 
> 	PR middle-end/87533
> 	* c-attribs.c (handle_assume_aligned_attribute): Diagnose and
> 	reject invalid attribute specifications.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR middle-end/87533
> 	* gcc.dg/attr-assume_aligned-4.c: New test.
OK.
jeff