* [PATCH] reject conflicting attributes before calling handlers (PR 86453)
@ 2018-07-11 22:04 Martin Sebor
2018-07-12 8:46 ` Richard Biener
2018-07-13 8:53 ` Christophe Lyon
0 siblings, 2 replies; 5+ messages in thread
From: Martin Sebor @ 2018-07-11 22:04 UTC (permalink / raw)
To: Richard Biener, Gcc Patch List
[-- Attachment #1: Type: text/plain, Size: 525 bytes --]
The attached change set adjusts the attribute exclusion code
to detect and reject incompatible attributes before attribute
handlers are called to have a chance to make changes despite
the exclusions. The handlers are not run when a conflict is
found.
Tested on x86_64-linux. I expected the fallout to be bigger
but only a handful of tests needed adjusting and the changes
all look like clear improvements. I.e., conflicting attributes
that diagnosed as being ignored really are being ignored as one
would expect.
Martin
[-- Attachment #2: gcc-86453.diff --]
[-- Type: text/x-patch, Size: 10177 bytes --]
PR c/86453 - error: type variant differs by TYPE_PACKED in free_lang_data since r255469
gcc/ChangeLog:
PR c/86453
* attribs.c (decl_attributes): Reject conflicting attributes before
calling attribute handlers.
gcc/testsuite/ChangeLog:
PR c/86453
* c-c++-common/Wattributes.c: Adjust.
* gcc.dg/Wattributes-10.c: New test.
* g++.dg/Wattributes-3.C: Adjust.
* g++.dg/lto/pr86453_0.C: New test.
* gcc.dg/Wattributes-6.c: Adjust.
* gcc.dg/pr18079.c: Adjust.
* gcc.dg/torture/pr42363.c: Adjust.
Index: gcc/attribs.c
===================================================================
--- gcc/attribs.c (revision 262542)
+++ gcc/attribs.c (working copy)
@@ -672,6 +672,35 @@ decl_attributes (tree *node, tree attributes, int
bool no_add_attrs = false;
+ /* Check for exclusions with other attributes on the current
+ declation as well as the last declaration of the same
+ symbol already processed (if one exists). Detect and
+ reject incompatible attributes. */
+ bool built_in = flags & ATTR_FLAG_BUILT_IN;
+ if (spec->exclude
+ && (flag_checking || !built_in))
+ {
+ /* Always check attributes on user-defined functions.
+ Check them on built-ins only when -fchecking is set.
+ Ignore __builtin_unreachable -- it's both const and
+ noreturn. */
+
+ if (!built_in
+ || !DECL_P (*anode)
+ || (DECL_FUNCTION_CODE (*anode) != BUILT_IN_UNREACHABLE
+ && (DECL_FUNCTION_CODE (*anode)
+ != BUILT_IN_UBSAN_HANDLE_BUILTIN_UNREACHABLE)))
+ {
+ bool no_add = diag_attr_exclusions (last_decl, *anode, name, spec);
+ if (!no_add && anode != node)
+ no_add = diag_attr_exclusions (last_decl, *node, name, spec);
+ no_add_attrs |= no_add;
+ }
+ }
+
+ if (no_add_attrs)
+ continue;
+
if (spec->handler != NULL)
{
int cxx11_flag =
@@ -695,33 +724,6 @@ decl_attributes (tree *node, tree attributes, int
returned_attrs = chainon (ret, returned_attrs);
}
- /* If the attribute was successfully handled on its own and is
- about to be added check for exclusions with other attributes
- on the current declation as well as the last declaration of
- the same symbol already processed (if one exists). */
- bool built_in = flags & ATTR_FLAG_BUILT_IN;
- if (spec->exclude
- && !no_add_attrs
- && (flag_checking || !built_in))
- {
- /* Always check attributes on user-defined functions.
- Check them on built-ins only when -fchecking is set.
- Ignore __builtin_unreachable -- it's both const and
- noreturn. */
-
- if (!built_in
- || !DECL_P (*anode)
- || (DECL_FUNCTION_CODE (*anode) != BUILT_IN_UNREACHABLE
- && (DECL_FUNCTION_CODE (*anode)
- != BUILT_IN_UBSAN_HANDLE_BUILTIN_UNREACHABLE)))
- {
- bool no_add = diag_attr_exclusions (last_decl, *anode, name, spec);
- if (!no_add && anode != node)
- no_add = diag_attr_exclusions (last_decl, *node, name, spec);
- no_add_attrs |= no_add;
- }
- }
-
/* Layout the decl in case anything changed. */
if (spec->type_required && DECL_P (*node)
&& (VAR_P (*node)
Index: gcc/testsuite/c-c++-common/Wattributes.c
===================================================================
--- gcc/testsuite/c-c++-common/Wattributes.c (revision 262542)
+++ gcc/testsuite/c-c++-common/Wattributes.c (working copy)
@@ -39,13 +39,13 @@ PackedPacked { int i; };
aligned and packed on a function declaration. */
void ATTR ((aligned (8), packed))
-faligned8_1 (void); /* { dg-warning ".packed. attribute ignored" } */
+faligned8_1 (void); /* { dg-warning "ignoring attribute .packed. because it conflicts with attribute .aligned." } */
void ATTR ((aligned (8)))
-faligned8_2 (void); /* { dg-message "previous declaration here" "" { xfail *-*-* } } */
+faligned8_2 (void); /* { dg-message "previous declaration here" } */
void ATTR ((packed))
-faligned8_2 (void); /* { dg-warning ".packed. attribute ignored" } */
+faligned8_2 (void); /* { dg-warning "ignoring attribute .packed. because it conflicts with attribute .aligned." } */
/* Exercise the handling of the mutually exclusive attributes
always_inline and noinline (in that order). */
Index: gcc/testsuite/g++.dg/Wattributes-3.C
===================================================================
--- gcc/testsuite/g++.dg/Wattributes-3.C (revision 262542)
+++ gcc/testsuite/g++.dg/Wattributes-3.C (working copy)
@@ -26,6 +26,8 @@ B::operator char () const { return 0; }
ATTR ((__noinline__))
B::operator int () const // { dg-warning "ignoring attribute .noinline. because it conflicts with attribute .always_inline." }
+// { dg-warning "function might not be inlinable" "" { target *-*-* } .-1 }
+
{
return 0;
}
@@ -43,6 +45,7 @@ C::operator char () { return 0; }
ATTR ((__noinline__))
C::operator short () // { dg-warning "ignoring attribute .noinline. because it conflicts with attribute .always_inline." }
+// { dg-warning "function might not be inlinable" "" { target *-*-* } .-1 }
{ return 0; }
inline ATTR ((__noinline__))
Index: gcc/testsuite/g++.dg/lto/pr86453_0.C
===================================================================
--- gcc/testsuite/g++.dg/lto/pr86453_0.C (nonexistent)
+++ gcc/testsuite/g++.dg/lto/pr86453_0.C (working copy)
@@ -0,0 +1,11 @@
+/* PR 86453/middle-end - error: type variant differs by TYPE_PACKED
+ in free_lang_data
+ { dg-lto-do link }
+ { dg-require-effective-target shared }
+ { dg-require-effective-target fpic }
+ { dg-lto-options {{-fPIC -shared -flto -w}} } */
+
+struct
+{
+ int *__attribute__((aligned, packed)) a;
+} b;
Index: gcc/testsuite/gcc.dg/Wattributes-10.c
===================================================================
--- gcc/testsuite/gcc.dg/Wattributes-10.c (nonexistent)
+++ gcc/testsuite/gcc.dg/Wattributes-10.c (working copy)
@@ -0,0 +1,26 @@
+/* PR middle-end/86453 - error: type variant differs by TYPE_PACKED in
+ free_lang_data since r255469
+ { dg-do compile }
+ { dg-options "-Wall" } */
+
+#define A(expr) do { int a[1 - 2 * !(expr)]; (void)&a; } while (0)
+
+struct S
+{
+ int* __attribute__ ((aligned (16))) paligned;
+ int* __attribute__ ((packed)) ppacked;
+
+ int* __attribute__ ((aligned (16), packed)) qaligned; /* { dg-warning "ignoring attribute .packed. because it conflicts with attribute .aligned." } */
+ int* __attribute__ ((packed, aligned (16))) qpacked; /* { dg-warning "ignoring attribute .aligned. because it conflicts with attribute .packed." } */
+} s;
+
+void test (void)
+{
+ /* Verify that attributes reported ignored really are ignored
+ and not applied. */
+
+ A (__alignof__ (s.paligned) == 16);
+ A (__alignof__ (s.ppacked) < 16);
+ A (__alignof__ (s.qaligned) == 16);
+ A (__alignof__ (s.qpacked) == __alignof__ (s.ppacked));
+}
Index: gcc/testsuite/gcc.dg/Wattributes-6.c
===================================================================
--- gcc/testsuite/gcc.dg/Wattributes-6.c (revision 262542)
+++ gcc/testsuite/gcc.dg/Wattributes-6.c (working copy)
@@ -39,13 +39,13 @@ PackedPacked { int i; };
aligned and packed on a function declaration. */
void ATTR ((aligned (8), packed))
-faligned8_1 (void); /* { dg-warning ".packed. attribute ignored" } */
+faligned8_1 (void); /* { dg-warning "ignoring attribute .packed. because it conflicts with attribute .aligned." } */
void ATTR ((aligned (8)))
-faligned8_2 (void); /* { dg-message "previous declaration here" "" { xfail *-*-* } } */
+faligned8_2 (void); /* { dg-message "previous declaration here" } */
void ATTR ((packed))
-faligned8_2 (void); /* { dg-warning ".packed. attribute ignored" } */
+faligned8_2 (void); /* { dg-warning "ignoring attribute .packed. because it conflicts with attribute .aligned." } */
/* Exercise the handling of the mutually exclusive attributes
always_inline and noinline (in that order). */
Index: gcc/testsuite/gcc.dg/pr18079.c
===================================================================
--- gcc/testsuite/gcc.dg/pr18079.c (revision 262542)
+++ gcc/testsuite/gcc.dg/pr18079.c (working copy)
@@ -6,7 +6,7 @@ __attribute__ ((noinline))
__attribute__ ((always_inline))
int
fn1 (int r)
-{ /* { dg-warning "attribute ignored due to conflict" } */
+{ /* { dg-warning "ignoring attribute .always_inline. because it conflicts with attribute .noinline." } */
return r & 4;
}
@@ -13,7 +13,7 @@ fn1 (int r)
__attribute__ ((noinline, always_inline))
int
fn2 (int r)
-{ /* { dg-warning "attribute ignored due to conflict" } */
+{ /* { dg-warning "ignoring attribute .always_inline. because it conflicts with attribute .noinline." } */
return r & 4;
}
@@ -21,7 +21,7 @@ __attribute__ ((always_inline))
__attribute__ ((noinline))
inline int
fn3 (int r)
-{ /* { dg-warning "attribute ignored due to conflict" } */
+{ /* { dg-warning "ignoring attribute .noinline. because it conflicts with attribute .always_inline." } */
return r & 8;
}
@@ -28,6 +28,6 @@ fn3 (int r)
__attribute__ ((always_inline, noinline))
inline int
fn4 (int r)
-{ /* { dg-warning "attribute ignored due to conflict" } */
+{ /* { dg-warning "ignoring attribute .noinline. because it conflicts with attribute .always_inline." } */
return r & 8;
}
Index: gcc/testsuite/gcc.dg/torture/pr42363.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/pr42363.c (revision 262542)
+++ gcc/testsuite/gcc.dg/torture/pr42363.c (working copy)
@@ -54,8 +54,12 @@ static int __attribute__ ((pure, const, noreturn))
barf (void) {
/* { dg-warning "ignoring attribute .const." "const" { target *-*-* } .-1 } */
/* { dg-warning "ignoring attribute .noreturn." "noreturn" { target *-*-* } .-2 } */
-} /* { dg-warning "does return" } */
+ /* The noreturn attribute is ignored so verify there is no warning
+ for returning from the function:
+ { dg-bogus "does return" } */
+}
+
static int __attribute__ ((pure, const))
bark (void) { /* { dg-warning "ignoring attribute .const." } */
barf ();
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] reject conflicting attributes before calling handlers (PR 86453)
2018-07-11 22:04 [PATCH] reject conflicting attributes before calling handlers (PR 86453) Martin Sebor
@ 2018-07-12 8:46 ` Richard Biener
2018-07-13 8:53 ` Christophe Lyon
1 sibling, 0 replies; 5+ messages in thread
From: Richard Biener @ 2018-07-12 8:46 UTC (permalink / raw)
To: Martin Sebor; +Cc: Gcc Patch List
On Wed, 11 Jul 2018, Martin Sebor wrote:
> The attached change set adjusts the attribute exclusion code
> to detect and reject incompatible attributes before attribute
> handlers are called to have a chance to make changes despite
> the exclusions. The handlers are not run when a conflict is
> found.
>
> Tested on x86_64-linux. I expected the fallout to be bigger
> but only a handful of tests needed adjusting and the changes
> all look like clear improvements. I.e., conflicting attributes
> that diagnosed as being ignored really are being ignored as one
> would expect.
OK.
Note the LTO testcase is now redundant after my commit so you can
omit it.
Thanks,
Richard.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] reject conflicting attributes before calling handlers (PR 86453)
2018-07-11 22:04 [PATCH] reject conflicting attributes before calling handlers (PR 86453) Martin Sebor
2018-07-12 8:46 ` Richard Biener
@ 2018-07-13 8:53 ` Christophe Lyon
2018-07-13 15:10 ` Martin Sebor
1 sibling, 1 reply; 5+ messages in thread
From: Christophe Lyon @ 2018-07-13 8:53 UTC (permalink / raw)
To: Martin Sebor; +Cc: Richard Biener, gcc Patches
Hi,
On Thu, 12 Jul 2018 at 00:04, Martin Sebor <msebor@gmail.com> wrote:
>
> The attached change set adjusts the attribute exclusion code
> to detect and reject incompatible attributes before attribute
> handlers are called to have a chance to make changes despite
> the exclusions. The handlers are not run when a conflict is
> found.
>
> Tested on x86_64-linux. I expected the fallout to be bigger
> but only a handful of tests needed adjusting and the changes
> all look like clear improvements. I.e., conflicting attributes
> that diagnosed as being ignored really are being ignored as one
> would expect.
>
Since you committed this patch (r262596), I've noticed regressions on
aarch64/arm:
g++.dg/warn/pr86453.C -std=c++11 (test for warnings, line 4)
g++.dg/warn/pr86453.C -std=c++11 (test for excess errors)
g++.dg/warn/pr86453.C -std=c++14 (test for warnings, line 4)
g++.dg/warn/pr86453.C -std=c++14 (test for excess errors)
g++.dg/warn/pr86453.C -std=c++98 (test for warnings, line 4)
g++.dg/warn/pr86453.C -std=c++98 (test for excess errors)
The log says:
Excess errors:
/gcc/testsuite/g++.dg/warn/pr86453.C:4:44: warning: ignoring attribute
'packed' because it conflicts with attribute 'aligned' [-Wattributes]
Isn't there the same message on x86_64?
> Martin
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] reject conflicting attributes before calling handlers (PR 86453)
2018-07-13 8:53 ` Christophe Lyon
@ 2018-07-13 15:10 ` Martin Sebor
2018-07-13 15:46 ` Christophe Lyon
0 siblings, 1 reply; 5+ messages in thread
From: Martin Sebor @ 2018-07-13 15:10 UTC (permalink / raw)
To: Christophe Lyon; +Cc: Richard Biener, gcc Patches
On 07/13/2018 02:53 AM, Christophe Lyon wrote:
> Hi,
>
> On Thu, 12 Jul 2018 at 00:04, Martin Sebor <msebor@gmail.com> wrote:
>>
>> The attached change set adjusts the attribute exclusion code
>> to detect and reject incompatible attributes before attribute
>> handlers are called to have a chance to make changes despite
>> the exclusions. The handlers are not run when a conflict is
>> found.
>>
>> Tested on x86_64-linux. I expected the fallout to be bigger
>> but only a handful of tests needed adjusting and the changes
>> all look like clear improvements. I.e., conflicting attributes
>> that diagnosed as being ignored really are being ignored as one
>> would expect.
>>
>
> Since you committed this patch (r262596), I've noticed regressions on
> aarch64/arm:
> g++.dg/warn/pr86453.C -std=c++11 (test for warnings, line 4)
> g++.dg/warn/pr86453.C -std=c++11 (test for excess errors)
> g++.dg/warn/pr86453.C -std=c++14 (test for warnings, line 4)
> g++.dg/warn/pr86453.C -std=c++14 (test for excess errors)
> g++.dg/warn/pr86453.C -std=c++98 (test for warnings, line 4)
> g++.dg/warn/pr86453.C -std=c++98 (test for excess errors)
>
> The log says:
> Excess errors:
> /gcc/testsuite/g++.dg/warn/pr86453.C:4:44: warning: ignoring attribute
> 'packed' because it conflicts with attribute 'aligned' [-Wattributes]
>
> Isn't there the same message on x86_64?
There was. The test above was added between the time I tested
my patch and the time I committed it. I adjusted it yesterday
via r262609 so the failure should be gone.
Martin
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] reject conflicting attributes before calling handlers (PR 86453)
2018-07-13 15:10 ` Martin Sebor
@ 2018-07-13 15:46 ` Christophe Lyon
0 siblings, 0 replies; 5+ messages in thread
From: Christophe Lyon @ 2018-07-13 15:46 UTC (permalink / raw)
To: Martin Sebor; +Cc: Richard Biener, gcc Patches
On Fri, 13 Jul 2018 at 17:10, Martin Sebor <msebor@gmail.com> wrote:
>
> On 07/13/2018 02:53 AM, Christophe Lyon wrote:
> > Hi,
> >
> > On Thu, 12 Jul 2018 at 00:04, Martin Sebor <msebor@gmail.com> wrote:
> >>
> >> The attached change set adjusts the attribute exclusion code
> >> to detect and reject incompatible attributes before attribute
> >> handlers are called to have a chance to make changes despite
> >> the exclusions. The handlers are not run when a conflict is
> >> found.
> >>
> >> Tested on x86_64-linux. I expected the fallout to be bigger
> >> but only a handful of tests needed adjusting and the changes
> >> all look like clear improvements. I.e., conflicting attributes
> >> that diagnosed as being ignored really are being ignored as one
> >> would expect.
> >>
> >
> > Since you committed this patch (r262596), I've noticed regressions on
> > aarch64/arm:
> > g++.dg/warn/pr86453.C -std=c++11 (test for warnings, line 4)
> > g++.dg/warn/pr86453.C -std=c++11 (test for excess errors)
> > g++.dg/warn/pr86453.C -std=c++14 (test for warnings, line 4)
> > g++.dg/warn/pr86453.C -std=c++14 (test for excess errors)
> > g++.dg/warn/pr86453.C -std=c++98 (test for warnings, line 4)
> > g++.dg/warn/pr86453.C -std=c++98 (test for excess errors)
> >
> > The log says:
> > Excess errors:
> > /gcc/testsuite/g++.dg/warn/pr86453.C:4:44: warning: ignoring attribute
> > 'packed' because it conflicts with attribute 'aligned' [-Wattributes]
> >
> > Isn't there the same message on x86_64?
>
> There was. The test above was added between the time I tested
> my patch and the time I committed it. I adjusted it yesterday
> via r262609 so the failure should be gone.
>
Indeed, thanks!
I reported the regression because I didn't see any comment about it on
gcc-patches.
Christophe
> Martin
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-07-13 15:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-11 22:04 [PATCH] reject conflicting attributes before calling handlers (PR 86453) Martin Sebor
2018-07-12 8:46 ` Richard Biener
2018-07-13 8:53 ` Christophe Lyon
2018-07-13 15:10 ` Martin Sebor
2018-07-13 15:46 ` Christophe Lyon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).