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

2018-02-13 Thread 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


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

2018-02-13 Thread 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




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

2018-02-02 Thread Martin Sebor

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
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 000..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); */