[3/3] diagnose attribute aligned conflicts (PR 81566)

Submitted by Martin Sebor on Aug. 8, 2017, 4:13 p.m.

Details

Message ID 184eaaaa-bea6-482f-1579-c3e926d130fb@gmail.com
State New
Headers show

Commit Message

Martin Sebor Aug. 8, 2017, 4:13 p.m.
Patch 3 in the series restores the diagnostics for conflicting
attribute aligned on the same function (this regressed in r192199
in GCC 4.9), and also makes use of the enhanced infrastructure to
enhance the detection of the same conflicts on distinct declarations
of the same function.

Rather than issuing an error for the confluct as was done in GCC 4.8
I made it a -Wattributes warning.  That seems more in line with how
otherwise syntactically valid attributes are handled elsewhere, and
it also lets code developed since the regression was introduced to
temporarily suppress the warning until the problem is conflict is
corrected.

Martin

Patch hide | download patch | download mbox

PR c/81566 - invalid attribute aligned accepted on functions

gcc/c-family/ChangeLog:

	PR c/81566
	c-attribs.c (handle_aligned_attribute): Diagnose conflicting
	attribute specifications.

gcc/testsuite/ChangeLog:

	PR c/8166
	* c-c++-common/Wattributes-2.c: New test.

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index ac704bc..5a37dc2 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -1769,14 +1769,18 @@  check_cxx_fundamental_alignment_constraints (tree node,
    struct attribute_spec.handler.  */
 
 static tree
-handle_aligned_attribute (tree *node, tree ARG_UNUSED (name), tree args,
+handle_aligned_attribute (tree *node, tree name, tree args,
 			  int flags, bool *no_add_attrs)
 {
   tree decl = NULL_TREE;
   tree *type = NULL;
-  int is_type = 0;
+  bool is_type = false;
   tree align_expr;
-  int i;
+
+  /* The last (already pushed) declaration with all validated attributes
+     merged in or the current about-to-be-pushed one if one hassn't been
+     yet.  */
+  tree last_decl = node[1] ? node[1] : *node;
 
   if (args)
     {
@@ -1795,10 +1799,21 @@  handle_aligned_attribute (tree *node, tree ARG_UNUSED (name), tree args,
       is_type = TREE_CODE (*node) == TYPE_DECL;
     }
   else if (TYPE_P (*node))
-    type = node, is_type = 1;
+    type = node, is_type = true;
+
+  /* Log2 of specified alignment.  */
+  int pow2align = check_user_alignment (align_expr, true);
+
+  /* The alignment in bits corresponding to the specified alignment.  */
+  unsigned bitalign = (1U << pow2align) * BITS_PER_UNIT;
 
-  if ((i = check_user_alignment (align_expr, true)) == -1
-      || !check_cxx_fundamental_alignment_constraints (*node, i, flags))
+  /* The alignment of the current declaration and that of the last
+     pushed declaration, determined on demand below.  */
+  unsigned curalign = 0;
+  unsigned lastalign = 0;
+
+  if (pow2align == -1
+      || !check_cxx_fundamental_alignment_constraints (*node, pow2align, flags))
     *no_add_attrs = true;
   else if (is_type)
     {
@@ -1819,7 +1834,7 @@  handle_aligned_attribute (tree *node, tree ARG_UNUSED (name), tree args,
       else
 	*type = build_variant_type_copy (*type);
 
-      SET_TYPE_ALIGN (*type, (1U << i) * BITS_PER_UNIT);
+      SET_TYPE_ALIGN (*type, bitalign);
       TYPE_USER_ALIGN (*type) = 1;
     }
   else if (! VAR_OR_FUNCTION_DECL_P (decl)
@@ -1828,8 +1843,34 @@  handle_aligned_attribute (tree *node, tree ARG_UNUSED (name), tree args,
       error ("alignment may not be specified for %q+D", decl);
       *no_add_attrs = true;
     }
+  else if (TREE_CODE (decl) == FUNCTION_DECL
+	   && ((curalign = DECL_ALIGN (decl)) > bitalign
+	       || ((lastalign = DECL_ALIGN (last_decl)) > bitalign)))
+    {
+      /* Either a prior attribute on the same declaration or one
+	 on a prior declaration of the same function specifies
+	 stricter alignment than this attribute.  */
+      bool note = lastalign != 0;
+      if (lastalign)
+	curalign = lastalign;
+
+      curalign /= BITS_PER_UNIT;
+      bitalign /= BITS_PER_UNIT;
+
+      if (DECL_USER_ALIGN (decl) || DECL_USER_ALIGN (last_decl))
+	warning (OPT_Wattributes,
+		 "ignoring attribute %<%E (%u)%> because it conflicts with "
+		 "attribute %<%E (%u)%>", name, bitalign, name, curalign);
+      else
+	error ("alignment for %q+D must be at least %d", decl, curalign);
+
+      if (note)
+	inform (DECL_SOURCE_LOCATION (last_decl), "previous declaration here");
+
+      *no_add_attrs = true;
+    }
   else if (DECL_USER_ALIGN (decl)
-	   && DECL_ALIGN (decl) > (1U << i) * BITS_PER_UNIT)
+	   && DECL_ALIGN (decl) > bitalign)
     /* C++-11 [dcl.align/4]:
 
 	   When multiple alignment-specifiers are specified for an
@@ -1839,21 +1880,9 @@  handle_aligned_attribute (tree *node, tree ARG_UNUSED (name), tree args,
       This formally comes from the c++11 specification but we are
       doing it for the GNU attribute syntax as well.  */
     *no_add_attrs = true;
-  else if (TREE_CODE (decl) == FUNCTION_DECL
-	   && DECL_ALIGN (decl) > (1U << i) * BITS_PER_UNIT)
-    {
-      if (DECL_USER_ALIGN (decl))
-	error ("alignment for %q+D was previously specified as %d "
-	       "and may not be decreased", decl,
-	       DECL_ALIGN (decl) / BITS_PER_UNIT);
-      else
-	error ("alignment for %q+D must be at least %d", decl,
-	       DECL_ALIGN (decl) / BITS_PER_UNIT);
-      *no_add_attrs = true;
-    }
   else
     {
-      SET_DECL_ALIGN (decl, (1U << i) * BITS_PER_UNIT);
+      SET_DECL_ALIGN (decl, bitalign);
       DECL_USER_ALIGN (decl) = 1;
     }
 
diff --git a/gcc/testsuite/c-c++-common/Wattributes-2.c b/gcc/testsuite/c-c++-common/Wattributes-2.c
new file mode 100644
index 0000000..0904742
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wattributes-2.c
@@ -0,0 +1,74 @@ 
+/* PR c/81566 - invalid attribute aligned accepted on functions
+   { dg-do compile }
+   { dg-options "-Wall -Wattributes -ftrack-macro-expansion=0" } */
+
+#define ATTR(list) __attribute__ (list)
+#define ALIGN(n)   ATTR ((aligned (n)))
+
+/* It's okay to increase the alignment of a function.  */
+
+void ALIGN (16) ALIGN (32)
+falign32_1 (void);
+
+void ALIGN (16) falign32_2 (void);
+void ALIGN (32) falign32_2 (void);
+
+void falign32_2 (void) { }
+
+void ALIGN (32) falign32_2 (void);
+
+/* It's not okay to decrease it.  */
+
+void ALIGN (32) ALIGN (16)
+falign64_3 (void);            /* { dg-warning "ignoring attribute .aligned \\(16\\). because it conflicts with attribute .aligned \\(32\\)." } */
+
+void ALIGN (32)
+falign64_3 (void);
+
+void falign64_3 (void);
+
+void falign64_3 (void) { }
+
+
+void ALIGN (32)
+falign64_4 (void);            /* { dg-message "previous declaration here" } */
+
+void ALIGN (16)
+falign64_4 (void);            /* { dg-warning "ignoring attribute .aligned \\(16\\). because it conflicts with attribute .aligned \\(32\\)." } */
+
+void ALIGN (32)
+falign64_4 (void);            /* { dg-message "previous declaration here" } */
+
+void ALIGN (16)
+falign64_4 (void);            /* { dg-warning "ignoring attribute .aligned \\(16\\). because it conflicts with attribute .aligned \\(32\\)." } */
+
+void ALIGN (64)
+falign64_4 (void);
+
+void ALIGN (32)
+falign64_4 (void);            /* { dg-warning "ignoring attribute .aligned \\(32\\). because it conflicts with attribute .aligned \\(64\\)." } */
+
+void falign64_4 (void);
+
+void ALIGN (64)
+falign64_4 (void) { }
+
+void falign64_4 (void);
+
+void ALIGN (64)
+falign64_4 (void);
+
+
+void ATTR ((aligned (16), aligned (32)))
+falign64_5 (void);
+
+void ATTR ((aligned (32), aligned (64)))
+falign64_5 (void);
+
+void ATTR ((aligned (16), aligned (32), aligned (64)))
+falign64_5 (void);            /* { dg-warning "ignoring attribute .aligned \\(16\\). because it conflicts with attribute .aligned \\(64\\)." } */
+                              /* { dg-warning "ignoring attribute .aligned \\(32\\). because it conflicts with attribute .aligned \\(64\\)." "" { target *-*-* } .-1 } */
+
+
+void ATTR ((aligned (16), aligned (32), aligned (16)))
+falign64_6 (void);            /* { dg-warning "ignoring attribute .aligned \\(16\\). because it conflicts with attribute .aligned \\(32\\)." } */