[PATCH] avoid warning for members declared both aligned and packed (PR 84108)

[PATCH] avoid warning for members declared both aligned and packed (PR 84108)

[Martin Sebor]

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

The design of the attribute exclusion framework includes
support for different exclusions applying to different
kinds of declarations (functions, types, and variables
or fields), but the support is incomplete -- the logic
to consider these differences is missing.  This is
because the differences are apparently rare.  However,
as the bug below points out, they do exist.

PR middle-end/84108 - incorrect -Wattributes warning for
packed/aligned conflict on struct members, shows that while
declaring a non-member variable aligned is enough to reduce
the its alignment and declaring it both aligned and packed
triggers a -Wattributes warning:

   int a __attribute__((packed, aligned (2)));   // -Wattributes

a struct member must be declared both aligned and packed in
order to have its alignment reduced.  (Declaring a member
just aligned has no effect and doesn't cause a warning).

   struct S {
     int b __attribute__((packed, aligned (2)));
     int c __attribute__((aligned (2)));           // no effect
   };

As a result of the incomplete logic GCC 8 issues a -Wattributes
for the declaration of b in the struct.

By adding the missing logic the attached patch lets GCC avoid
the spurious warning.

I considered adding support for detecting the ineffective
attribute aligned on the declaration of the member c at
the same time but since that's not a regression I decided
to defer that until GCC 9.  I opened bug 84185 to track it.

Tested on x86_64-linux with no regressions.

Martin

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

PR middle-end/84108 - incorrect -Wattributes warning for packed/aligned conflict on struct members

gcc/ChangeLog:

	PR c/84108
	* attribs.c (diag_attr_exclusions): Consider the exclusion(s)
	that correspond to the kind of a declaration.

gcc/testsuite/ChangeLog:

	PR c/84108
	* gcc.dg/Wattributes-8.c: New test.

diff --git a/gcc/attribs.c b/gcc/attribs.c
index 2cac9c4..140863b 100644
--- a/gcc/attribs.c
+++ b/gcc/attribs.c
@@ -410,6 +410,22 @@ diag_attr_exclusions (tree last_decl, tree node, tree attrname,
 	  if (!lookup_attribute (excl->name, attrs[i]))
 	    continue;
 
+	  /* An exclusion may apply either to a function declaration,
+	     type declaration, or a field/variable declaration, or
+	     any subset of the three.  */
+	  if (TREE_CODE (node) == FUNCTION_DECL
+	      && !excl->function)
+	    continue;
+
+	  if (TREE_CODE (node) == TYPE_DECL
+	      && !excl->type)
+	    continue;
+
+	  if ((TREE_CODE (node) == FIELD_DECL
+	       || TREE_CODE (node) == VAR_DECL)
+	      && !excl->variable)
+	    continue;
+
 	  found = true;
 
 	  /* Print a note?  */
diff --git a/gcc/testsuite/gcc.dg/Wattributes-8.c b/gcc/testsuite/gcc.dg/Wattributes-8.c
new file mode 100644
index 0000000..a4b4c00
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wattributes-8.c
@@ -0,0 +1,38 @@
+/* PR middle-end/84108 - incorrect -Wattributes warning for packed/aligned
+   conflict on struct members
+   { dg-do compile }
+   { dg-options "-Wall -Wattributes" } */
+
+#define ATTR(list) __attribute__ (list)
+#define ASSERT(e) _Static_assert (e, #e)
+
+/* GCC is inconsistent in how it treats attribute aligned between
+   variable and member declarations.  Attribute aligned alone is
+   sufficient to reduce a variable's alignment requirement but
+   the attribute must be paired with packed to have the same
+   effect on a member.  Worse, declaring a variable both aligned
+   and packed emits a warning.  */
+
+/* Avoid exercising this since emitting a warning for these given
+   the requirement for members seems like a misfeature:
+   int a ATTR ((packed, aligned (2)));   // -Wattributes
+   int b ATTR ((aligned (2), packed));   // -Wattributes
+   ASSERT (_Alignof (a) == 2);
+   ASSERT (_Alignof (b) == 2);  */
+
+int c ATTR ((aligned (2)));           // okay (reduces alignment)
+ASSERT (_Alignof (c) == 2);
+
+struct {
+  int a ATTR ((packed, aligned (2)));   /* { dg-bogus "\\\[-Wattributes" } */
+  int b ATTR ((aligned (2), packed));   /* { dg-bogus "\\\[-Wattributes" } */
+
+  /* Avoid exercising this since the attribute has no effect yet
+     there is no warning.
+     int c ATTR ((aligned (2)));           // missing warning?  */
+} s;
+
+ASSERT (_Alignof (s.a) == 2);
+ASSERT (_Alignof (s.b) == 2);
+
+/* ASSERT (_Alignof (s.c) == 4); */

Re: [PATCH] avoid warning for members declared both aligned and packed (PR 84108)

[Martin Sebor]

Ping: https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00125.html

On 02/02/2018 11:58 AM, Martin Sebor wrote:
> The design of the attribute exclusion framework includes
> support for different exclusions applying to different
> kinds of declarations (functions, types, and variables
> or fields), but the support is incomplete -- the logic
> to consider these differences is missing.  This is
> because the differences are apparently rare.  However,
> as the bug below points out, they do exist.
>
> PR middle-end/84108 - incorrect -Wattributes warning for
> packed/aligned conflict on struct members, shows that while
> declaring a non-member variable aligned is enough to reduce
> the its alignment and declaring it both aligned and packed
> triggers a -Wattributes warning:
>
>   int a __attribute__((packed, aligned (2)));   // -Wattributes
>
> a struct member must be declared both aligned and packed in
> order to have its alignment reduced.  (Declaring a member
> just aligned has no effect and doesn't cause a warning).
>
>   struct S {
>     int b __attribute__((packed, aligned (2)));
>     int c __attribute__((aligned (2)));           // no effect
>   };
>
> As a result of the incomplete logic GCC 8 issues a -Wattributes
> for the declaration of b in the struct.
>
> By adding the missing logic the attached patch lets GCC avoid
> the spurious warning.
>
> I considered adding support for detecting the ineffective
> attribute aligned on the declaration of the member c at
> the same time but since that's not a regression I decided
> to defer that until GCC 9.  I opened bug 84185 to track it.
>
> Tested on x86_64-linux with no regressions.
>
> Martin

Re: [PATCH] avoid warning for members declared both aligned and packed (PR 84108)

[Jeff Law]

On 02/02/2018 11:58 AM, Martin Sebor wrote:
> The design of the attribute exclusion framework includes
> support for different exclusions applying to different
> kinds of declarations (functions, types, and variables
> or fields), but the support is incomplete -- the logic
> to consider these differences is missing.  This is
> because the differences are apparently rare.  However,
> as the bug below points out, they do exist.
> 
> PR middle-end/84108 - incorrect -Wattributes warning for
> packed/aligned conflict on struct members, shows that while
> declaring a non-member variable aligned is enough to reduce
> the its alignment and declaring it both aligned and packed
> triggers a -Wattributes warning:
> 
>   int a __attribute__((packed, aligned (2)));   // -Wattributes
> 
> a struct member must be declared both aligned and packed in
> order to have its alignment reduced.  (Declaring a member
> just aligned has no effect and doesn't cause a warning).
> 
>   struct S {
>     int b __attribute__((packed, aligned (2)));
>     int c __attribute__((aligned (2)));           // no effect
>   };
> 
> As a result of the incomplete logic GCC 8 issues a -Wattributes
> for the declaration of b in the struct.
> 
> By adding the missing logic the attached patch lets GCC avoid
> the spurious warning.
> 
> I considered adding support for detecting the ineffective
> attribute aligned on the declaration of the member c at
> the same time but since that's not a regression I decided
> to defer that until GCC 9.  I opened bug 84185 to track it.
> 
> Tested on x86_64-linux with no regressions.
> 
> Martin
> 
> gcc-84108.diff
> 
> 
> PR middle-end/84108 - incorrect -Wattributes warning for packed/aligned conflict on struct members
> 
> gcc/ChangeLog:
> 
> 	PR c/84108
> 	* attribs.c (diag_attr_exclusions): Consider the exclusion(s)
> 	that correspond to the kind of a declaration.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c/84108
> 	* gcc.dg/Wattributes-8.c: New test.
OK.  Sorry for the wait.

Thanks.

Jeff