[Bug c/81824] Warn for missing attributes with function aliases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81824 --- Comment #18 from Alexandre Oliva --- Author: aoliva Date: Thu Jul 18 00:38:45 2019 New Revision: 273563 URL: https://gcc.gnu.org/viewcvs?rev=273563=gcc=rev Log: -Wmissing-attributes: check that we avoid duplicates and false positives The initial patch for PR 81824 fixed various possibilities of -Wmissing-attributes reporting duplicates and false positives. The test that avoided them was a little obscure, though, so this patch rewrites it into a more self-evident form. The patch also adds a testcase that already passed, but that explicitly covers some of the possibilities of reporting duplicates and false positives that preexisting tests did not cover. for gcc/ChangeLog PR middle-end/81824 * attribs.c (decls_mismatched_attributes): Simplify the logic that avoids duplicates and false positives. for gcc/testsuite/ChangeLog PR middle-end/81824 * g++.dg/Wmissing-attributes-1.C: New. Some of its fragments are from Martin Sebor. Added: trunk/gcc/testsuite/g++.dg/Wmissing-attributes-1.C Modified: trunk/gcc/ChangeLog trunk/gcc/attribs.c trunk/gcc/testsuite/ChangeLog
[Bug c/81824] Warn for missing attributes with function aliases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81824 --- Comment #17 from Martin Sebor --- Making -Wattribute-alias a synonym for -Wattribute-alias=1 makes sense. I'll take care of it.
[Bug c/81824] Warn for missing attributes with function aliases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81824 Khem Raj changed: What|Removed |Added CC||raj.khem at gmail dot com --- Comment #16 from Khem Raj --- (In reply to Martin Sebor from comment #10) > Author: msebor > Date: Fri Nov 9 17:32:52 2018 > New Revision: 265980 > > URL: https://gcc.gnu.org/viewcvs?rev=265980=gcc=rev > Log: > PR middle-end/81824 - Warn for missing attributes with function aliases > > gcc/c-family/ChangeLog: > > PR middle-end/81824 > * c-attribs.c (handle_copy_attribute): New function. > > gcc/cp/ChangeLog: > > PR middle-end/81824 > * pt.c (warn_spec_missing_attributes): Move code to attribs.c. > Call decls_mismatched_attributes. > > gcc/ChangeLog: > > PR middle-end/81824 > * attribs.c (has_attribute): New helper function. > (decls_mismatched_attributes, maybe_diag_alias_attributes): Same. > * attribs.h (decls_mismatched_attributes): Declare. > * cgraphunit.c (handle_alias_pairs): Call maybe_diag_alias_attributes. > (maybe_diag_incompatible_alias): Use OPT_Wattribute_alias_. > * common.opt (-Wattribute-alias): Take an argument. > (-Wno-attribute-alias): New option. > * doc/extend.texi (Common Function Attributes): Document copy. > (Common Variable Attributes): Same. > * doc/invoke.texi (-Wmissing-attributes): Document enhancement. > (-Wattribute-alias): Document new option argument. > > gcc/testsuite/ChangeLog: > > PR middle-end/81824 > * gcc.dg/Wattribute-alias.c: New test. > * gcc.dg/Wmissing-attributes.c: New test. > * gcc.dg/attr-copy.c: New test. > * gcc.dg/attr-copy-2.c: New test. > * gcc.dg/attr-copy-3.c: New test. > * gcc.dg/attr-copy-4.c: New test. > > > Added: > trunk/gcc/testsuite/gcc.dg/Wattribute-alias.c > trunk/gcc/testsuite/gcc.dg/Wmissing-attributes.c > trunk/gcc/testsuite/gcc.dg/attr-copy-2.c > trunk/gcc/testsuite/gcc.dg/attr-copy-3.c > trunk/gcc/testsuite/gcc.dg/attr-copy-4.c > trunk/gcc/testsuite/gcc.dg/attr-copy.c > Modified: > trunk/gcc/ChangeLog > trunk/gcc/attribs.c > trunk/gcc/attribs.h > trunk/gcc/c-family/ChangeLog > trunk/gcc/c-family/c-attribs.c > trunk/gcc/cgraphunit.c > trunk/gcc/common.opt > trunk/gcc/cp/ChangeLog > trunk/gcc/cp/pt.c > trunk/gcc/doc/extend.texi > trunk/gcc/doc/invoke.texi > trunk/gcc/testsuite/ChangeLog > trunk/libgomp/libgomp.h the new behavior is to not accept -Wattribute-alias and mutate this option into -Wattribute-alias=, this conflicts with gcc8, is this planned move or do we want to attach default level to -Wattribute-alias if no argument is used ?
[Bug c/81824] Warn for missing attributes with function aliases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81824 --- Comment #15 from Martin Sebor --- Author: msebor Date: Tue Nov 13 19:57:51 2018 New Revision: 266084 URL: https://gcc.gnu.org/viewcvs?rev=266084=gcc=rev Log: PR middle-end/81824 - Warn for missing attributes with function aliases gcc/c-family/ChangeLog: * c-attribs.c (handle_copy_attribute): Exclude inlining attributes. (handle_tls_model_attribute): Improve diagnostics. gcc/testsuite/ChangeLog: * gcc.dg/attr-copy-5.c: New test. * gcc.dg/tls/diag-6.c: Adjust expected diagnostics. Added: trunk/gcc/testsuite/gcc.dg/attr-copy-5.c Modified: trunk/gcc/c-family/ChangeLog trunk/gcc/c-family/c-attribs.c trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.dg/tls/diag-6.c
[Bug c/81824] Warn for missing attributes with function aliases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81824 --- Comment #13 from Martin Sebor --- Author: msebor Date: Mon Nov 12 18:02:41 2018 New Revision: 266034 URL: https://gcc.gnu.org/viewcvs?rev=266034=gcc=rev Log: PR c/81824 - Warn for missing attributes with function aliases gcc/testsuite/ChangeLog: * gcc.dg/Wattribute-alias.c: Require ifunc support. Modified: trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.dg/Wattribute-alias.c
[Bug c/81824] Warn for missing attributes with function aliases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81824 --- Comment #14 from Martin Sebor --- Thanks. The test needed dg-require-ifunc.
[Bug c/81824] Warn for missing attributes with function aliases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81824 Christophe Lyon changed: What|Removed |Added CC||clyon at gcc dot gnu.org --- Comment #12 from Christophe Lyon --- The new test gcc.dg/Wattribute-alias.c fails on bare-metal toolchains (eg arm-eabi, aarch64-elf): /gcc/testsuite/gcc.dg/Wattribute-alias.c:49:39: error: ifunc is not supported on this target /gcc/testsuite/gcc.dg/Wattribute-alias.c:48:44: error: ifunc is not supported on this target /gcc/testsuite/gcc.dg/Wattribute-alias.c:47:37: error: ifunc is not supported on this target I don't know if there is an effective-target for this?
[Bug c/81824] Warn for missing attributes with function aliases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81824 Martin Sebor changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED Target Milestone|--- |9.0 Severity|normal |enhancement --- Comment #11 from Martin Sebor --- Committed in r265980.
[Bug c/81824] Warn for missing attributes with function aliases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81824 --- Comment #10 from Martin Sebor --- Author: msebor Date: Fri Nov 9 17:32:52 2018 New Revision: 265980 URL: https://gcc.gnu.org/viewcvs?rev=265980=gcc=rev Log: PR middle-end/81824 - Warn for missing attributes with function aliases gcc/c-family/ChangeLog: PR middle-end/81824 * c-attribs.c (handle_copy_attribute): New function. gcc/cp/ChangeLog: PR middle-end/81824 * pt.c (warn_spec_missing_attributes): Move code to attribs.c. Call decls_mismatched_attributes. gcc/ChangeLog: PR middle-end/81824 * attribs.c (has_attribute): New helper function. (decls_mismatched_attributes, maybe_diag_alias_attributes): Same. * attribs.h (decls_mismatched_attributes): Declare. * cgraphunit.c (handle_alias_pairs): Call maybe_diag_alias_attributes. (maybe_diag_incompatible_alias): Use OPT_Wattribute_alias_. * common.opt (-Wattribute-alias): Take an argument. (-Wno-attribute-alias): New option. * doc/extend.texi (Common Function Attributes): Document copy. (Common Variable Attributes): Same. * doc/invoke.texi (-Wmissing-attributes): Document enhancement. (-Wattribute-alias): Document new option argument. gcc/testsuite/ChangeLog: PR middle-end/81824 * gcc.dg/Wattribute-alias.c: New test. * gcc.dg/Wmissing-attributes.c: New test. * gcc.dg/attr-copy.c: New test. * gcc.dg/attr-copy-2.c: New test. * gcc.dg/attr-copy-3.c: New test. * gcc.dg/attr-copy-4.c: New test. Added: trunk/gcc/testsuite/gcc.dg/Wattribute-alias.c trunk/gcc/testsuite/gcc.dg/Wmissing-attributes.c trunk/gcc/testsuite/gcc.dg/attr-copy-2.c trunk/gcc/testsuite/gcc.dg/attr-copy-3.c trunk/gcc/testsuite/gcc.dg/attr-copy-4.c trunk/gcc/testsuite/gcc.dg/attr-copy.c Modified: trunk/gcc/ChangeLog trunk/gcc/attribs.c trunk/gcc/attribs.h trunk/gcc/c-family/ChangeLog trunk/gcc/c-family/c-attribs.c trunk/gcc/cgraphunit.c trunk/gcc/common.opt trunk/gcc/cp/ChangeLog trunk/gcc/cp/pt.c trunk/gcc/doc/extend.texi trunk/gcc/doc/invoke.texi trunk/gcc/testsuite/ChangeLog trunk/libgomp/libgomp.h
[Bug c/81824] Warn for missing attributes with function aliases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81824 Martin Sebor changed: What|Removed |Added Keywords||patch Assignee|unassigned at gcc dot gnu.org |msebor at gcc dot gnu.org --- Comment #9 from Martin Sebor --- Patch: https://gcc.gnu.org/ml/gcc-patches/2018-10/msg00047.html
[Bug c/81824] Warn for missing attributes with function aliases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81824
--- Comment #8 from Martin Sebor ---
I see. I think you are specifically talking about the case when the three
attributes are used together, as in:
void __attribute__ ((weak, alias ("__foo"), visibility ("..."))) foo ();
void __foo () { ... }
and not warning that __foo isn't declared weak, or with alias, or with
visibility. Yes, warning for that wouldn't be very helpful.
[Bug c/81824] Warn for missing attributes with function aliases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81824 --- Comment #7 from joseph at codesourcery dot com --- On Thu, 8 Feb 2018, msebor at gcc dot gnu.org wrote: > Okay, that would make sense. But then what do you mean by "weak, alias, > visibility attributes are expected to differ between different names and > shouldn't be diagnosed." Weak is a property of a symbol, not of a function - it's correct for a function to have both strong and weak names.
[Bug c/81824] Warn for missing attributes with function aliases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81824
--- Comment #6 from Martin Sebor ---
Okay, that would make sense. But then what do you mean by "weak, alias,
visibility attributes are expected to differ between different names and
shouldn't be diagnosed."
The example in comment #0 shows a potential bug: declaring an alias to be more
restrictive ("nothrow") than its target makes it possible for the target to
quietly violate the guarantee provided by the alias to its callers. So I would
expect it to be diagnosed for the reasons of correctness. (I realize that for
nothrow this isn't applicable to Glibc because nothing in Glibc throws. But it
is applicable to C++ code bases.)
It's not a potential bug for an alias were less restrictive ("might throw")
than its targets. It just means that uses of the alias may lead to suboptimal
code compared to the target, and so warning for the mismatch will help point
that out.
So the warning would be helpful in both cases, just for different reasons. (In
my view, this is analogous to the const/pure situation we discussed.)
I think the same argument applies to weak.
I haven't thought about visibility too much but it seems different. I can't
think of a problem with defining an alias with a different visibility than its
target.
[Bug c/81824] Warn for missing attributes with function aliases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81824 --- Comment #5 from joseph at codesourcery dot com --- In the case most likely to appear in glibc, foo would be declared with the nothrow attribute and __foo would be missing it. I see no reason not to diagnose the other case as well, I just think it's less likely to be applicable to glibc.
[Bug c/81824] Warn for missing attributes with function aliases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81824 --- Comment #4 from Martin Sebor --- (In reply to Joseph S. Myers from comment #0) > Note: this warning is only for attributes relating to the function itself, > not to those relating to a particular name for the function. For example, > weak, alias, visibility attributes are expected to differ between different > names and shouldn't be diagnosed. It would be necessary to work out which > existing function attributes fall in which category. Joseph, to make sure I understand: foo in the example is an alias for __foo and you want a warning for __foo. What you're saying above is that if __foo were nothrow, you wouldn't want to see a warning for foo pointing out that it's missing the attribute. Yes?
[Bug c/81824] Warn for missing attributes with function aliases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81824 Martin Sebor changed: What|Removed |Added Status|NEW |ASSIGNED --- Comment #3 from Martin Sebor --- I'm adding a new warning to do something similar for the C++ front end per Jason's suggestion (https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00197.html). I don't expect to also resolve this request for GCC 8 but I might as well assign it to myself and finish it in GCC 9.
[Bug c/81824] Warn for missing attributes with function aliases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81824
Martin Sebor changed:
What|Removed |Added
See Also||https://gcc.gnu.org/bugzill
||a/show_bug.cgi?id=81854
--- Comment #2 from Martin Sebor ---
The challenge here is that the alias target may not be in scope at the point of
the alias declaration, so while the test case in comment #0 could be detected
by enhancing the handle_alias_attribute function in c-family/c-attribs.c, one
where the declaration order is reversed as in the following could not be:
void foo (void) __attribute__ ((nothrow, weak, alias ("__foo")));
void __foo (void) { }
Bug 81854 suggests that there is a way to detect this in handle_alias_pairs()
in cgraphunit.c.
[Bug c/81824] Warn for missing attributes with function aliases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81824
Martin Sebor changed:
What|Removed |Added
Keywords||diagnostic
Status|UNCONFIRMED |NEW
Last reconfirmed||2017-08-11
CC||msebor at gcc dot gnu.org
Ever confirmed|0 |1
--- Comment #1 from Martin Sebor ---
Confirmed with the C++ test case below. I'll look into the warning but I
wonder if in addition it would be worth to add some intrinsics to query and
manipulate attributes. For instance, if the built-in __builtin_attributes__(X)
extracted all attributes from a declaration or a type it could be used to
easily declare __foo with all the same attributes as foo:
void foo (void) __attribute__ ((nothrow));
__typeof__ (foo) __foo __attribute ((__builtin_attributes__ ((foo;
Attributes could be removed by using another built-in, say
__builtin_remove_attribute.
$ cat z.c && gcc -O2 -S -Wall -Wextra -Wpedantic -Wunused
-fdump-tree-optimized=/dev/stdout -xc++ z.c
extern "C" {
void foo (void) __attribute__ ((nothrow));
__typeof__ (foo) __foo;
void bar (void) __attribute__ ((nothrow));
__typeof__ (bar) __bar __attribute__ ((nothrow));
#if DEF
void __foo (void) { }
void __bar (void) { }
void foo (void) __attribute__ ((weak, alias ("__foo")));
void bar (void) __attribute__ ((weak, alias ("__bar")));
#else
void call_foo (void)
{
try {
__foo ();
}
catch (...) {
__builtin_abort ();
}
}
void call_bar (void)
{
try {
__bar ();
}
catch (...) {
__builtin_abort ();
}
}
#endif
}
;; Function void call_foo() (call_foo, funcdef_no=0, decl_uid=2282,
cgraph_uid=0, symbol_order=0)
void call_foo() ()
{
void * _1;
[100.00%] [count: INV]:
__foo ();
[100.00%] [count: INV]:
return;
[0.00%] [count: INV]:
:
_1 = __builtin_eh_pointer (1);
__cxa_begin_catch (_1);
__builtin_abort ();
}
;; Function void call_bar() (call_bar, funcdef_no=1, decl_uid=2286,
cgraph_uid=1, symbol_order=1)
void call_bar() ()
{
void * _1;
[100.00%] [count: INV]:
__bar (); [tail call]
return;
}
