| Message ID | c6c50436-d6f8-69ee-3550-432cd3e32bc7@gmail.com |
|---|---|
| State | New |
| Headers | show |
The bulk of this patch touches what I think is considered the middle-end (attribs.c) so let me include its maintainers, Ian, Jeff, and Richard: https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01087.html The high-level description and rationale for the change are here: https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00602.html Thanks Martin On 08/17/2017 10:03 AM, Martin Sebor wrote: > Attached is an updated patch with just the minor editorial > tweaks for the issues pointed out by Marek (stray comments > and spaces), and with the C++ and libstdc++ bits removed > and posted separately. I also added some text to the manual > to clarify the const/pure effects. > > I thought quite a bit more about the const/pure attributes we > discussed and tried the approach of warning only on a const > declaration after one with attribute pure has been used, but > otherwise allowing and silently ignoring pure after const. > In the end I decided against it, for a few reasons (most of > which I already mentioned but just to summarize). > > First, there is the risk that someone will write code based > on the pure declaration even if there's no intervening call > to the function between it and the const one. Code tends to > be sloppy, and it's also not uncommon to declare the same > function more than once, for whatever reason. (The ssa-ccp-2.c > test is an example of the latter.) > > Second, there are cases of attribute conflicts that GCC already > points out that are much more benign in their effects (the ones > I know about are always_inline/noinline and cold/hot). In light > of the risk above it seems only helpful to include const/pure in > the same set. > > Third, I couldn't find another pair of attributes that GCC would > deliberately handle this way (silently accept both but prefer one > over the other), and introducing a special case just for these > two seemed like a wart. > > Finally, compiling Binutils, GDB, Glkibc, and the Linux kernel > with the enhanced warning didn't turn up any code that does this > sort of thing, either intentionally or otherwise. > > With that, I've left the handling unchanged. > > I do still have the question whether diagnosing attribute > conflicts under -Wattributes is right. The manual implies > that -Wattributes is for attributes in the wrong places or > on the wrong entities, and that -Wignored-attributes should > be expected instead when GCC decides to drop one for some > reason. > > It is a little unfortunate that many -Wattributes warnings > print just "attribute ignored" (and have done so for years). > I think they should all be enhanced to also print why the > attribute is ignored (e.g., "'packed' attribute on function > declarations ignored/not valid/not supported" or something > like that). Those that ignore attributes that would > otherwise be valid e.g., because they conflict with other > specifiers of the same attribute but with a different > operand might then be candidate for changing to > -Wignored-attributes. > > Thanks > Martin >
Ping: https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01087.html On 08/24/2017 02:43 PM, Martin Sebor wrote: > The bulk of this patch touches what I think is considered > the middle-end (attribs.c) so let me include its maintainers, > Ian, Jeff, and Richard: > > https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01087.html > > The high-level description and rationale for the change are > here: > > https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00602.html > > Thanks > Martin > > On 08/17/2017 10:03 AM, Martin Sebor wrote: >> Attached is an updated patch with just the minor editorial >> tweaks for the issues pointed out by Marek (stray comments >> and spaces), and with the C++ and libstdc++ bits removed >> and posted separately. I also added some text to the manual >> to clarify the const/pure effects. >> >> I thought quite a bit more about the const/pure attributes we >> discussed and tried the approach of warning only on a const >> declaration after one with attribute pure has been used, but >> otherwise allowing and silently ignoring pure after const. >> In the end I decided against it, for a few reasons (most of >> which I already mentioned but just to summarize). >> >> First, there is the risk that someone will write code based >> on the pure declaration even if there's no intervening call >> to the function between it and the const one. Code tends to >> be sloppy, and it's also not uncommon to declare the same >> function more than once, for whatever reason. (The ssa-ccp-2.c >> test is an example of the latter.) >> >> Second, there are cases of attribute conflicts that GCC already >> points out that are much more benign in their effects (the ones >> I know about are always_inline/noinline and cold/hot). In light >> of the risk above it seems only helpful to include const/pure in >> the same set. >> >> Third, I couldn't find another pair of attributes that GCC would >> deliberately handle this way (silently accept both but prefer one >> over the other), and introducing a special case just for these >> two seemed like a wart. >> >> Finally, compiling Binutils, GDB, Glkibc, and the Linux kernel >> with the enhanced warning didn't turn up any code that does this >> sort of thing, either intentionally or otherwise. >> >> With that, I've left the handling unchanged. >> >> I do still have the question whether diagnosing attribute >> conflicts under -Wattributes is right. The manual implies >> that -Wattributes is for attributes in the wrong places or >> on the wrong entities, and that -Wignored-attributes should >> be expected instead when GCC decides to drop one for some >> reason. >> >> It is a little unfortunate that many -Wattributes warnings >> print just "attribute ignored" (and have done so for years). >> I think they should all be enhanced to also print why the >> attribute is ignored (e.g., "'packed' attribute on function >> declarations ignored/not valid/not supported" or something >> like that). Those that ignore attributes that would >> otherwise be valid e.g., because they conflict with other >> specifiers of the same attribute but with a different >> operand might then be candidate for changing to >> -Wignored-attributes. >> >> Thanks >> Martin >> >
On Thu, 17 Aug 2017, Martin Sebor wrote: > +/* Check LAST_DECL and NODE of the same symbol for attributes that are > + recorded in EXCL to be mutually exclusive with ATTRNAME, diagnose > + them, and return true if any have been found. NODE can be a DECL > + or a TYPE. */ > + > +static bool > +diag_attr_exclusions (tree last_decl, tree node, tree attrname, > + const attribute_spec *spec) EXCL is not an argument to this function, so the comment above it should not refer to EXCL (presumably it should refer to SPEC instead). > + note &= warning (OPT_Wattributes, > + "ignoring attribute %qE in declaration of " > + "a built-in function qD because it conflicts " > + "with attribute %qs", > + attrname, node, excl->name); %qD not qD, presumably. (Generically, warning_at would be preferred to warning, but that may best be kept separate if you don't already have a location available here.) > +static const struct attribute_spec::exclusions attr_gnu_inline_exclusions[] = > +{ > + ATTR_EXCL ("gnu_inline", true, true, true), > + ATTR_EXCL ("noinline", true, true, true), > + ATTR_EXCL (NULL, false, false, false), > +}; This says gnu_inline is incompatible with noinline, and is listed as the EXCL field for the gnu_inline attribute. > +static const struct attribute_spec::exclusions attr_inline_exclusions[] = > +{ > + ATTR_EXCL ("always_inline", true, true, true), > + ATTR_EXCL ("noinline", true, true, true), > + ATTR_EXCL (NULL, false, false, false), > +}; This is listed as the EXCL field for the noinline attribute, but does not mention gnu_inline. Does this mean some asymmetry in when that pair is diagnosed? I don't see tests for that pair added by the patch. (Of course, gnu_inline + always_inline is OK, and attr_inline_exclusions is also used for the always_inline attribute in this patch.) In general, the data structures where you need to ensure manually that if attribute A is listed in EXCL for B, then attribute B is also listed in EXCL for A, seem concerning. I'd expect either data structures that make such asymmetry impossible, or a self-test that verifies that the tables in use are in fact symmetric (unless there is some reason the symmetry is not in fact required and symmetric diagnostics still result from asymmetric tables - in which case the various combinations and orderings of gnu_inline and noinline definitely need tests to show that the diagnostics work). > +both the @code{const} and the @code{pure} attribute is diagnnosed. s/diagnnosed/diagnosed/
On 08/17/2017 10:03 AM, Martin Sebor wrote: > > First, there is the risk that someone will write code based > on the pure declaration even if there's no intervening call > to the function between it and the const one. Code tends to > be sloppy, and it's also not uncommon to declare the same > function more than once, for whatever reason. (The ssa-ccp-2.c > test is an example of the latter.) True. But I think if we get into a state where the semantics of the implementation are incompatible with the listed attributes (and we can reliably detect that) then we should issue a separate and distinct warning. For example a pure function that reads global memory should issue a warning/error. ISTM such a warning/error would be a separate and distinct patch than detection of attribute conflicts. > > Second, there are cases of attribute conflicts that GCC already > points out that are much more benign in their effects (the ones > I know about are always_inline/noinline and cold/hot). In light > of the risk above it seems only helpful to include const/pure in > the same set. always_inline/noinline conflicts can cause build failures in codebases that require certain functions to be inlined. We can argue about whether or not that's a good policy for those codebases, but they certainly exist. I could envision the hot/cold case generating a link error if the linker script shoved them into different memory segments and getting them wrong caused an overflow. But these IMHO are corner cases and should not drive major decisions here. > > Third, I couldn't find another pair of attributes that GCC would > deliberately handle this way (silently accept both but prefer one > over the other), and introducing a special case just for these > two seemed like a wart. Seems reasonable. > I do still have the question whether diagnosing attribute > conflicts under -Wattributes is right. The manual implies > that -Wattributes is for attributes in the wrong places or > on the wrong entities, and that -Wignored-attributes should > be expected instead when GCC decides to drop one for some > reason. This seems like it ought to be a distinct option given the way -Wattributes is documented. Alternately we can tweak the meaning of -Wattributes. I don't have strong opinions here. > > It is a little unfortunate that many -Wattributes warnings > print just "attribute ignored" (and have done so for years). > I think they should all be enhanced to also print why the > attribute is ignored (e.g., "'packed' attribute on function > declarations ignored/not valid/not supported" or something > like that). Those that ignore attributes that would > otherwise be valid e.g., because they conflict with other > specifiers of the same attribute but with a different > operand might then be candidate for changing to > -Wignored-attributes. Seems like a reasonable follow-up if you're interested. > > Thanks > Martin > > > gcc-81544-1.diff > > > PR c/81544 - attribute noreturn and warn_unused_result on the same function accepted > > gcc/c/ChangeLog: > > PR c/81544 > * c-decl.c (c_decl_attributes): Look up existing declaration and > pass it to decl_attributes. > > gcc/c-family/ChangeLog: > > PR c/81544 > * c-attribs.c (attr_aligned_exclusions): New array. > (attr_alloc_exclusions, attr_cold_hot_exclusions): Same. > (attr_common_exclusions, attr_const_pure_exclusions): Same. > (attr_gnu_inline_exclusions, attr_inline_exclusions): Same. > (attr_noreturn_exclusions, attr_returns_twice_exclusions): Same. > (attr_warn_unused_result_exclusions): Same. > (handle_hot_attribute, handle_cold_attribute): Simplify. > (handle_const_attribute): Warn on function returning void. > (handle_pure_attribute): Same. > * c-warn.c (diagnose_mismatched_attributes): Simplify. > > gcc/testsuite/ChangeLog: > > PR c/81544 > * c-c++-common/Wattributes-2.c: New test. > * c-c++-common/Wattributes.c: New test. > * c-c++-common/attributes-3.c: Adjust. > * gcc.dg/attr-noinline.c: Adjust. > * gcc.dg/pr44964.c: Same. > * gcc.dg/torture/pr42363.c: Same. > * gcc.dg/tree-ssa/ssa-ccp-2.c: Same. > > gcc/ChangeLog: > > PR c/81544 > * attribs.c (empty_attribute_table): Initialize new member of > struct attribute_spec. > (decl_attributes): Add argument. Handle mutually exclusive > combinations of attributes. > * attribs.h (decl_attributes): Add default argument. > * tree-core.h (attribute_spec::exclusions, exclude): New type and > member. > * doc/extend.texi (Common Function Attributes): Update const and pure. > diff --git a/gcc/attribs.h b/gcc/attribs.h > index d4a790b..a9eba0a 100644 > --- a/gcc/attribs.h > +++ b/gcc/attribs.h > @@ -31,7 +31,7 @@ extern void init_attributes (void); > from tree.h. Depending on these flags, some attributes may be > returned to be applied at a later stage (for example, to apply > a decl attribute to the declaration rather than to its type). */ > -extern tree decl_attributes (tree *, tree, int); > +extern tree decl_attributes (tree *, tree, int, tree = NULL_TREE); So we do allow default arguments. Though there seems to be some preference for just defining an overload instead. The arguments for using overloads instead of a default argument here aren't real compelling though. Anyway, consider using an overload. If you think the default argument is cleaner, I won't object. > > extern bool cxx11_attribute_p (const_tree); > extern tree get_attribute_name (const_tree); > diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c > index 0d9ab2d..8a157f6 100644 > --- a/gcc/c-family/c-attribs.c > +++ b/gcc/c-family/c-attribs.c > @@ -44,6 +44,7 @@ along with GCC; see the file COPYING3. If not see > #include "tree-iterator.h" > #include "opts.h" > #include "gimplify.h" > +#include "bitmap.h" I don't immediately see where you need bitmap.h in here, but I could have missed it. It's not a problem if you need it, but if not, let's avoid adding it unnecessarily. > @@ -1667,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 s/hassn't/hasn't/ I'd like to echo Joseph's comment WRT the mirroring of exclusions. Ideally setting up the structure so that happens automatically is best, but alternately, testing for symmetry is acceptable. If asymmetry is desirable, it needs to be documented. Largely it looks good. I think the biggest issue is the exclusion tables and how to deal with cases where we need to ensure symmetry vs any cases where asymmetry is expected/desirable. Jeff
On 09/06/2017 05:30 PM, Joseph Myers wrote: > On Thu, 17 Aug 2017, Martin Sebor wrote: > >> +/* Check LAST_DECL and NODE of the same symbol for attributes that are >> + recorded in EXCL to be mutually exclusive with ATTRNAME, diagnose >> + them, and return true if any have been found. NODE can be a DECL >> + or a TYPE. */ >> + >> +static bool >> +diag_attr_exclusions (tree last_decl, tree node, tree attrname, >> + const attribute_spec *spec) > > EXCL is not an argument to this function, so the comment above it should > not refer to EXCL (presumably it should refer to SPEC instead). > >> + note &= warning (OPT_Wattributes, >> + "ignoring attribute %qE in declaration of " >> + "a built-in function qD because it conflicts " >> + "with attribute %qs", >> + attrname, node, excl->name); > > %qD not qD, presumably. Yes, thanks. Fixed. > > (Generically, warning_at would be preferred to warning, but that may best > be kept separate if you don't already have a location available here.) I agree (on both counts). > >> +static const struct attribute_spec::exclusions attr_gnu_inline_exclusions[] = >> +{ >> + ATTR_EXCL ("gnu_inline", true, true, true), >> + ATTR_EXCL ("noinline", true, true, true), >> + ATTR_EXCL (NULL, false, false, false), >> +}; > > This says gnu_inline is incompatible with noinline, and is listed as the > EXCL field for the gnu_inline attribute. > >> +static const struct attribute_spec::exclusions attr_inline_exclusions[] = >> +{ >> + ATTR_EXCL ("always_inline", true, true, true), >> + ATTR_EXCL ("noinline", true, true, true), >> + ATTR_EXCL (NULL, false, false, false), >> +}; > > This is listed as the EXCL field for the noinline attribute, but does not > mention gnu_inline. Does this mean some asymmetry in when that pair is > diagnosed? I don't see tests for that pair added by the patch. Ah, good catch! always_inline and gnu_inline are compatible with one another so what's missing is noinline_exclusions. I've added it, simplified the others, and included a test case. > (Of course, gnu_inline + always_inline is OK, and attr_inline_exclusions > is also used for the always_inline attribute in this patch.) > > In general, the data structures where you need to ensure manually that if > attribute A is listed in EXCL for B, then attribute B is also listed in > EXCL for A, seem concerning. I'd expect either data structures that make > such asymmetry impossible, or a self-test that verifies that the tables in > use are in fact symmetric (unless there is some reason the symmetry is not > in fact required and symmetric diagnostics still result from asymmetric > tables - in which case the various combinations and orderings of > gnu_inline and noinline definitely need tests to show that the diagnostics > work). If I understand correctly what you're concerned about then I don't think there are any such cases in the updated version of the patch. >> +both the @code{const} and the @code{pure} attribute is diagnnosed. > > s/diagnnosed/diagnosed/ Thanks. In the attached update I've corrected this and the other issues you and Jeff pointed out. Martin PR c/81544 - attribute noreturn and warn_unused_result on the same function accepted gcc/c/ChangeLog: PR c/81544 * c-decl.c (c_decl_attributes): Look up existing declaration and pass it to decl_attributes. gcc/c-family/ChangeLog: PR c/81544 * c-attribs.c (attr_aligned_exclusions): New array. (attr_alloc_exclusions, attr_cold_hot_exclusions): Same. (attr_common_exclusions, attr_const_pure_exclusions): Same. (attr_gnu_inline_exclusions, attr_inline_exclusions): Same. (attr_noreturn_exclusions, attr_returns_twice_exclusions): Same. (attr_warn_unused_result_exclusions): Same. (handle_hot_attribute, handle_cold_attribute): Simplify. (handle_const_attribute): Warn on function returning void. (handle_pure_attribute): Same. * c-warn.c (diagnose_mismatched_attributes): Simplify. gcc/ChangeLog: PR c/81544 * attribs.c (empty_attribute_table): Initialize new member of struct attribute_spec. (decl_attributes): Add argument. Handle mutually exclusive combinations of attributes. * attribs.h (decl_attributes): Add default argument. * tree-core.h (attribute_spec::exclusions, exclude): New type and member. * doc/extend.texi (Common Function Attributes): Update const and pure. gcc/testsuite/ChangeLog: PR c/81544 * c-c++-common/Wattributes-2.c: New test. * c-c++-common/Wattributes.c: New test. * c-c++-common/attributes-3.c: Adjust. * gcc.dg/attr-noinline.c: Adjust. * gcc.dg/pr44964.c: Same. * gcc.dg/torture/pr42363.c: Same. * gcc.dg/tree-ssa/ssa-ccp-2.c: Same. diff --git a/gcc/attribs.c b/gcc/attribs.c index 05fa8ef..3f9ae37 100644 --- a/gcc/attribs.c +++ b/gcc/attribs.c @@ -94,7 +94,7 @@ static bool attributes_initialized = false; static const struct attribute_spec empty_attribute_table[] = { - { NULL, 0, 0, false, false, false, NULL, false } + { NULL, 0, 0, false, false, false, NULL, false, NULL } }; /* Return base name of the attribute. Ie '__attr__' is turned into 'attr'. @@ -343,6 +343,97 @@ get_attribute_namespace (const_tree attr) return get_identifier ("gnu"); } +/* Check LAST_DECL and NODE of the same symbol for attributes that are + recorded in SPEC to be mutually exclusive with ATTRNAME, diagnose + them, and return true if any have been found. NODE can be a DECL + or a TYPE. */ + +static bool +diag_attr_exclusions (tree last_decl, tree node, tree attrname, + const attribute_spec *spec) +{ + const attribute_spec::exclusions *excl = spec->exclude; + + tree_code code = TREE_CODE (node); + + if ((code == FUNCTION_DECL && !excl->function + && (!excl->type || !spec->affects_type_identity)) + || (code == VAR_DECL && !excl->variable + && (!excl->type || !spec->affects_type_identity)) + || (((code == TYPE_DECL || RECORD_OR_UNION_TYPE_P (node)) && !excl->type))) + return false; + + /* True if an attribute that's mutually exclusive with ATTRNAME + has been found. */ + bool found = false; + + if (last_decl && last_decl != node && TREE_TYPE (last_decl) != node) + { + /* Check both the last DECL and its type for conflicts with + the attribute being added to the current decl or type. */ + found |= diag_attr_exclusions (last_decl, last_decl, attrname, spec); + tree decl_type = TREE_TYPE (last_decl); + found |= diag_attr_exclusions (last_decl, decl_type, attrname, spec); + } + + /* NODE is either the current DECL to which the attribute is being + applied or its TYPE. For the former, consider the attributes on + both the DECL and its type. */ + tree attrs[2]; + + if (DECL_P (node)) + { + attrs[0] = DECL_ATTRIBUTES (node); + attrs[1] = TYPE_ATTRIBUTES (TREE_TYPE (node)); + } + else + { + attrs[0] = TYPE_ATTRIBUTES (node); + attrs[1] = NULL_TREE; + } + + /* Iterate over the mutually exclusive attribute names and verify + that the symbol doesn't contain it. */ + for (unsigned i = 0; i != sizeof attrs / sizeof *attrs; ++i) + { + if (!attrs[i]) + continue; + + for ( ; excl->name; ++excl) + { + /* Avoid checking the attribute against itself. */ + if (is_attribute_p (excl->name, attrname)) + continue; + + if (!lookup_attribute (excl->name, attrs[i])) + continue; + + found = true; + + /* Print a note? */ + bool note = last_decl != NULL_TREE; + + if (TREE_CODE (node) == FUNCTION_DECL + && DECL_BUILT_IN (node)) + note &= warning (OPT_Wattributes, + "ignoring attribute %qE in declaration of " + "a built-in function %qD because it conflicts " + "with attribute %qs", + attrname, node, excl->name); + else + note &= warning (OPT_Wattributes, + "ignoring attribute %qE because " + "it conflicts with attribute %qs", + attrname, excl->name); + + if (note) + inform (DECL_SOURCE_LOCATION (last_decl), + "previous declaration here"); + } + } + + return found; +} /* Process the attributes listed in ATTRIBUTES and install them in *NODE, which is either a DECL (including a TYPE_DECL) or a TYPE. If a DECL, @@ -354,7 +445,8 @@ get_attribute_namespace (const_tree attr) a decl attribute to the declaration rather than to its type). */ tree -decl_attributes (tree *node, tree attributes, int flags) +decl_attributes (tree *node, tree attributes, int flags, + tree last_decl /* = NULL_TREE */) { tree a; tree returned_attrs = NULL_TREE; @@ -433,6 +525,8 @@ decl_attributes (tree *node, tree attributes, int flags) targetm.insert_attributes (*node, &attributes); + /* Note that attributes on the same declaration are not necessarily + in the same order as in the source. */ for (a = attributes; a; a = TREE_CHAIN (a)) { tree ns = get_attribute_namespace (a); @@ -441,7 +535,6 @@ decl_attributes (tree *node, tree attributes, int flags) tree *anode = node; const struct attribute_spec *spec = lookup_scoped_attribute_spec (ns, name); - bool no_add_attrs = 0; int fn_ptr_quals = 0; tree fn_ptr_tmp = NULL_TREE; @@ -490,7 +583,8 @@ decl_attributes (tree *node, tree attributes, int flags) | (int) ATTR_FLAG_ARRAY_NEXT)) { /* Pass on this attribute to be tried again. */ - returned_attrs = tree_cons (name, args, returned_attrs); + tree attr = tree_cons (name, args, NULL_TREE); + returned_attrs = chainon (returned_attrs, attr); continue; } else @@ -535,7 +629,8 @@ decl_attributes (tree *node, tree attributes, int flags) else if (flags & (int) ATTR_FLAG_FUNCTION_NEXT) { /* Pass on this attribute to be tried again. */ - returned_attrs = tree_cons (name, args, returned_attrs); + tree attr = tree_cons (name, args, NULL_TREE); + returned_attrs = chainon (returned_attrs, attr); continue; } @@ -557,15 +652,56 @@ decl_attributes (tree *node, tree attributes, int flags) continue; } + bool no_add_attrs = false; + if (spec->handler != NULL) { int cxx11_flag = cxx11_attribute_p (a) ? ATTR_FLAG_CXX11 : 0; - returned_attrs = chainon ((*spec->handler) (anode, name, args, - flags|cxx11_flag, - &no_add_attrs), - returned_attrs); + /* Pass in an array of the current declaration followed + by the last pushed/merged declaration if one exists. + If the handler changes CUR_AND_LAST_DECL[0] replace + *ANODE with its value. */ + tree cur_and_last_decl[] = { *anode, last_decl }; + tree ret = (spec->handler) (cur_and_last_decl, name, args, + flags|cxx11_flag, &no_add_attrs); + + *anode = cur_and_last_decl[0]; + if (ret == error_mark_node) + { + warning (OPT_Wattributes, "%qE attribute ignored", name); + no_add_attrs = true; + } + else + 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. */ diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c index 0d9ab2d..17d6993 100644 --- a/gcc/c-family/c-attribs.c +++ b/gcc/c-family/c-attribs.c @@ -146,6 +146,91 @@ static tree handle_fallthrough_attribute (tree *, tree, tree, int, bool *); static tree handle_patchable_function_entry_attribute (tree *, tree, tree, int, bool *); +/* Helper to define attribute exclusions. */ +#define ATTR_EXCL(name, function, type, variable) \ + { name, function, type, variable } + +/* Define attributes that are mutually exclusive with one another. */ +static const struct attribute_spec::exclusions attr_aligned_exclusions[] = +{ + /* Attribute name exclusion applies to: + function, type, variable */ + ATTR_EXCL ("aligned", true, false, false), + ATTR_EXCL ("packed", true, false, false), + ATTR_EXCL (NULL, false, false, false) +}; + +static const struct attribute_spec::exclusions attr_cold_hot_exclusions[] = +{ + ATTR_EXCL ("cold", true, true, true), + ATTR_EXCL ("hot", true, true, true), + ATTR_EXCL (NULL, false, false, false) +}; + +static const struct attribute_spec::exclusions attr_common_exclusions[] = +{ + ATTR_EXCL ("common", true, true, true), + ATTR_EXCL ("nocommon", true, true, true), + ATTR_EXCL (NULL, false, false, false), +}; + +static const struct attribute_spec::exclusions attr_inline_exclusions[] = +{ + ATTR_EXCL ("noinline", true, true, true), + ATTR_EXCL (NULL, false, false, false), +}; + +static const struct attribute_spec::exclusions attr_noinline_exclusions[] = +{ + ATTR_EXCL ("always_inline", true, true, true), + ATTR_EXCL ("gnu_inline", true, true, true), + ATTR_EXCL (NULL, false, false, false), +}; + +static const struct attribute_spec::exclusions attr_noreturn_exclusions[] = +{ + ATTR_EXCL ("noreturn", true, true, true), + ATTR_EXCL ("alloc_align", true, true, true), + ATTR_EXCL ("alloc_size", true, true, true), + ATTR_EXCL ("const", true, true, true), + ATTR_EXCL ("malloc", true, true, true), + ATTR_EXCL ("pure", true, true, true), + ATTR_EXCL ("returns_twice", true, true, true), + ATTR_EXCL ("warn_unused_result", true, true, true), + ATTR_EXCL (NULL, false, false, false), +}; + +static const struct attribute_spec::exclusions +attr_warn_unused_result_exclusions[] = +{ + ATTR_EXCL ("noreturn", true, true, true), + ATTR_EXCL ("warn_unused_result", true, true, true), + ATTR_EXCL (NULL, false, false, false), +}; + +static const struct attribute_spec::exclusions attr_returns_twice_exclusions[] = +{ + ATTR_EXCL ("noreturn", true, true, true), + ATTR_EXCL (NULL, false, false, false), +}; + +/* Exclusions that apply to attribute alloc_align, alloc_size, and malloc. */ +static const struct attribute_spec::exclusions attr_alloc_exclusions[] = +{ + ATTR_EXCL ("const", true, true, true), + ATTR_EXCL ("noreturn", true, true, true), + ATTR_EXCL ("pure", true, true, true), + ATTR_EXCL (NULL, false, false, false), +}; + +static const struct attribute_spec::exclusions attr_const_pure_exclusions[] = +{ + ATTR_EXCL ("const", true, true, true), + ATTR_EXCL ("noreturn", true, true, true), + ATTR_EXCL ("pure", true, true, true), + ATTR_EXCL (NULL, false, false, false) +}; + /* Table of machine-independent attributes common to all C-like languages. All attributes referencing arguments should be additionally processed @@ -157,209 +242,230 @@ const struct attribute_spec c_common_attribute_table[] = /* { name, min_len, max_len, decl_req, type_req, fn_type_req, handler, affects_type_identity } */ { "packed", 0, 0, false, false, false, - handle_packed_attribute , false}, + handle_packed_attribute , false, + attr_aligned_exclusions }, { "nocommon", 0, 0, true, false, false, - handle_nocommon_attribute, false}, + handle_nocommon_attribute, false, + attr_common_exclusions }, { "common", 0, 0, true, false, false, - handle_common_attribute, false }, + handle_common_attribute, false, + attr_common_exclusions }, /* FIXME: logically, noreturn attributes should be listed as "false, true, true" and apply to function types. But implementing this would require all the places in the compiler that use TREE_THIS_VOLATILE on a decl to identify non-returning functions to be located and fixed to check the function type instead. */ { "noreturn", 0, 0, true, false, false, - handle_noreturn_attribute, false }, + handle_noreturn_attribute, false, + attr_noreturn_exclusions }, { "volatile", 0, 0, true, false, false, - handle_noreturn_attribute, false }, + handle_noreturn_attribute, false, NULL }, { "stack_protect", 0, 0, true, false, false, - handle_stack_protect_attribute, false }, + handle_stack_protect_attribute, false, NULL }, { "noinline", 0, 0, true, false, false, - handle_noinline_attribute, false }, + handle_noinline_attribute, false, + attr_noinline_exclusions }, { "noclone", 0, 0, true, false, false, - handle_noclone_attribute, false }, + handle_noclone_attribute, false, NULL }, { "no_icf", 0, 0, true, false, false, - handle_noicf_attribute, false }, + handle_noicf_attribute, false, NULL }, { "noipa", 0, 0, true, false, false, - handle_noipa_attribute, false }, + handle_noipa_attribute, false, NULL }, { "leaf", 0, 0, true, false, false, - handle_leaf_attribute, false }, + handle_leaf_attribute, false, NULL }, { "always_inline", 0, 0, true, false, false, - handle_always_inline_attribute, false }, + handle_always_inline_attribute, false, + attr_inline_exclusions }, { "gnu_inline", 0, 0, true, false, false, - handle_gnu_inline_attribute, false }, + handle_gnu_inline_attribute, false, + attr_inline_exclusions }, { "artificial", 0, 0, true, false, false, - handle_artificial_attribute, false }, + handle_artificial_attribute, false, NULL }, { "flatten", 0, 0, true, false, false, - handle_flatten_attribute, false }, + handle_flatten_attribute, false, NULL }, { "used", 0, 0, true, false, false, - handle_used_attribute, false }, + handle_used_attribute, false, NULL }, { "unused", 0, 0, false, false, false, - handle_unused_attribute, false }, + handle_unused_attribute, false, NULL }, { "externally_visible", 0, 0, true, false, false, - handle_externally_visible_attribute, false }, + handle_externally_visible_attribute, false, + NULL }, { "no_reorder", 0, 0, true, false, false, - handle_no_reorder_attribute, false }, + handle_no_reorder_attribute, false, NULL }, /* The same comments as for noreturn attributes apply to const ones. */ { "const", 0, 0, true, false, false, - handle_const_attribute, false }, + handle_const_attribute, false, + attr_const_pure_exclusions }, { "scalar_storage_order", 1, 1, false, false, false, - handle_scalar_storage_order_attribute, false }, + handle_scalar_storage_order_attribute, false, + NULL }, { "transparent_union", 0, 0, false, false, false, - handle_transparent_union_attribute, false }, + handle_transparent_union_attribute, false, NULL }, { "constructor", 0, 1, true, false, false, - handle_constructor_attribute, false }, + handle_constructor_attribute, false, NULL }, { "destructor", 0, 1, true, false, false, - handle_destructor_attribute, false }, + handle_destructor_attribute, false, NULL }, { "mode", 1, 1, false, true, false, - handle_mode_attribute, false }, + handle_mode_attribute, false, NULL }, { "section", 1, 1, true, false, false, - handle_section_attribute, false }, + handle_section_attribute, false, NULL }, { "aligned", 0, 1, false, false, false, - handle_aligned_attribute, false }, + handle_aligned_attribute, false, + attr_aligned_exclusions }, { "weak", 0, 0, true, false, false, - handle_weak_attribute, false }, + handle_weak_attribute, false, NULL }, { "noplt", 0, 0, true, false, false, - handle_noplt_attribute, false }, + handle_noplt_attribute, false, NULL }, { "ifunc", 1, 1, true, false, false, - handle_ifunc_attribute, false }, + handle_ifunc_attribute, false, NULL }, { "alias", 1, 1, true, false, false, - handle_alias_attribute, false }, + handle_alias_attribute, false, NULL }, { "weakref", 0, 1, true, false, false, - handle_weakref_attribute, false }, + handle_weakref_attribute, false, NULL }, { "no_instrument_function", 0, 0, true, false, false, handle_no_instrument_function_attribute, - false }, + false, NULL }, { "no_profile_instrument_function", 0, 0, true, false, false, handle_no_profile_instrument_function_attribute, - false }, + false, NULL }, { "malloc", 0, 0, true, false, false, - handle_malloc_attribute, false }, + handle_malloc_attribute, false, + attr_alloc_exclusions }, { "returns_twice", 0, 0, true, false, false, - handle_returns_twice_attribute, false }, + handle_returns_twice_attribute, false, + attr_returns_twice_exclusions }, { "no_stack_limit", 0, 0, true, false, false, - handle_no_limit_stack_attribute, false }, + handle_no_limit_stack_attribute, false, NULL }, { "pure", 0, 0, true, false, false, - handle_pure_attribute, false }, + handle_pure_attribute, false, + attr_const_pure_exclusions }, { "transaction_callable", 0, 0, false, true, false, - handle_tm_attribute, false }, + handle_tm_attribute, false, NULL }, { "transaction_unsafe", 0, 0, false, true, false, - handle_tm_attribute, true }, + handle_tm_attribute, true, NULL }, { "transaction_safe", 0, 0, false, true, false, - handle_tm_attribute, true }, + handle_tm_attribute, true, NULL }, { "transaction_safe_dynamic", 0, 0, true, false, false, - handle_tm_attribute, false }, + handle_tm_attribute, false, NULL }, { "transaction_may_cancel_outer", 0, 0, false, true, false, - handle_tm_attribute, false }, + handle_tm_attribute, false, NULL }, /* ??? These two attributes didn't make the transition from the Intel language document to the multi-vendor language document. */ { "transaction_pure", 0, 0, false, true, false, - handle_tm_attribute, false }, + handle_tm_attribute, false, NULL }, { "transaction_wrap", 1, 1, true, false, false, - handle_tm_wrap_attribute, false }, + handle_tm_wrap_attribute, false, NULL }, /* For internal use (marking of builtins) only. The name contains space to prevent its usage in source code. */ { "no vops", 0, 0, true, false, false, - handle_novops_attribute, false }, + handle_novops_attribute, false, NULL }, { "deprecated", 0, 1, false, false, false, - handle_deprecated_attribute, false }, + handle_deprecated_attribute, false, NULL }, { "vector_size", 1, 1, false, true, false, - handle_vector_size_attribute, true }, + handle_vector_size_attribute, true, NULL }, { "visibility", 1, 1, false, false, false, - handle_visibility_attribute, false }, + handle_visibility_attribute, false, NULL }, { "tls_model", 1, 1, true, false, false, - handle_tls_model_attribute, false }, + handle_tls_model_attribute, false, NULL }, { "nonnull", 0, -1, false, true, true, - handle_nonnull_attribute, false }, + handle_nonnull_attribute, false, NULL }, { "nothrow", 0, 0, true, false, false, - handle_nothrow_attribute, false }, - { "may_alias", 0, 0, false, true, false, NULL, false }, + handle_nothrow_attribute, false, NULL }, + { "may_alias", 0, 0, false, true, false, NULL, false, NULL }, { "cleanup", 1, 1, true, false, false, - handle_cleanup_attribute, false }, + handle_cleanup_attribute, false, NULL }, { "warn_unused_result", 0, 0, false, true, true, - handle_warn_unused_result_attribute, false }, + handle_warn_unused_result_attribute, false, + attr_warn_unused_result_exclusions }, { "sentinel", 0, 1, false, true, true, - handle_sentinel_attribute, false }, + handle_sentinel_attribute, false, NULL }, /* For internal use (marking of builtins) only. The name contains space to prevent its usage in source code. */ { "type generic", 0, 0, false, true, true, - handle_type_generic_attribute, false }, + handle_type_generic_attribute, false, NULL }, { "alloc_size", 1, 2, false, true, true, - handle_alloc_size_attribute, false }, + handle_alloc_size_attribute, false, + attr_alloc_exclusions }, { "cold", 0, 0, true, false, false, - handle_cold_attribute, false }, + handle_cold_attribute, false, + attr_cold_hot_exclusions }, { "hot", 0, 0, true, false, false, - handle_hot_attribute, false }, + handle_hot_attribute, false, + attr_cold_hot_exclusions }, { "no_address_safety_analysis", 0, 0, true, false, false, handle_no_address_safety_analysis_attribute, - false }, + false, NULL }, { "no_sanitize", 1, 1, true, false, false, handle_no_sanitize_attribute, - false }, + false, NULL }, { "no_sanitize_address", 0, 0, true, false, false, handle_no_sanitize_address_attribute, - false }, + false, NULL }, { "no_sanitize_thread", 0, 0, true, false, false, handle_no_sanitize_thread_attribute, - false }, + false, NULL }, { "no_sanitize_undefined", 0, 0, true, false, false, handle_no_sanitize_undefined_attribute, - false }, + false, NULL }, { "asan odr indicator", 0, 0, true, false, false, handle_asan_odr_indicator_attribute, - false }, + false, NULL }, { "warning", 1, 1, true, false, false, - handle_error_attribute, false }, + handle_error_attribute, false, NULL }, { "error", 1, 1, true, false, false, - handle_error_attribute, false }, + handle_error_attribute, false, NULL }, { "target", 1, -1, true, false, false, - handle_target_attribute, false }, + handle_target_attribute, false, NULL }, { "target_clones", 1, -1, true, false, false, - handle_target_clones_attribute, false }, + handle_target_clones_attribute, false, NULL }, { "optimize", 1, -1, true, false, false, - handle_optimize_attribute, false }, + handle_optimize_attribute, false, NULL }, /* For internal use only. The leading '*' both prevents its usage in source code and signals that it may be overridden by machine tables. */ { "*tm regparm", 0, 0, false, true, true, - ignore_attribute, false }, + ignore_attribute, false, NULL }, { "no_split_stack", 0, 0, true, false, false, - handle_no_split_stack_attribute, false }, + handle_no_split_stack_attribute, false, NULL }, /* For internal use (marking of builtins and runtime functions) only. The name contains space to prevent its usage in source code. */ { "fn spec", 1, 1, false, true, true, - handle_fnspec_attribute, false }, + handle_fnspec_attribute, false, NULL }, { "warn_unused", 0, 0, false, false, false, - handle_warn_unused_attribute, false }, + handle_warn_unused_attribute, false, NULL }, { "returns_nonnull", 0, 0, false, true, true, - handle_returns_nonnull_attribute, false }, + handle_returns_nonnull_attribute, false, NULL }, { "omp declare simd", 0, -1, true, false, false, - handle_omp_declare_simd_attribute, false }, + handle_omp_declare_simd_attribute, false, NULL }, { "cilk simd function", 0, -1, true, false, false, - handle_omp_declare_simd_attribute, false }, + handle_omp_declare_simd_attribute, false, NULL }, { "simd", 0, 1, true, false, false, - handle_simd_attribute, false }, + handle_simd_attribute, false, NULL }, { "omp declare target", 0, 0, true, false, false, - handle_omp_declare_target_attribute, false }, + handle_omp_declare_target_attribute, false, + NULL }, { "omp declare target link", 0, 0, true, false, false, - handle_omp_declare_target_attribute, false }, + handle_omp_declare_target_attribute, false, + NULL }, { "alloc_align", 1, 1, false, true, true, - handle_alloc_align_attribute, false }, + handle_alloc_align_attribute, false, + attr_alloc_exclusions }, { "assume_aligned", 1, 2, false, true, true, - handle_assume_aligned_attribute, false }, + handle_assume_aligned_attribute, false, NULL }, { "designated_init", 0, 0, false, true, false, - handle_designated_init_attribute, false }, + handle_designated_init_attribute, false, NULL }, { "bnd_variable_size", 0, 0, true, false, false, - handle_bnd_variable_size_attribute, false }, + handle_bnd_variable_size_attribute, false, NULL }, { "bnd_legacy", 0, 0, true, false, false, - handle_bnd_legacy, false }, + handle_bnd_legacy, false, NULL }, { "bnd_instrument", 0, 0, true, false, false, - handle_bnd_instrument, false }, + handle_bnd_instrument, false, NULL }, { "fallthrough", 0, 0, false, false, false, - handle_fallthrough_attribute, false }, + handle_fallthrough_attribute, false, NULL }, { "patchable_function_entry", 1, 2, true, false, false, handle_patchable_function_entry_attribute, - false }, - { NULL, 0, 0, false, false, false, NULL, false } + false, NULL }, + { NULL, 0, 0, false, false, false, NULL, false, NULL } }; /* Give the specifications for the format attributes, used by C and all @@ -374,10 +480,10 @@ const struct attribute_spec c_common_format_attribute_table[] = /* { name, min_len, max_len, decl_req, type_req, fn_type_req, handler, affects_type_identity } */ { "format", 3, 3, false, true, true, - handle_format_attribute, false }, + handle_format_attribute, false, NULL }, { "format_arg", 1, 1, false, true, true, - handle_format_arg_attribute, false }, - { NULL, 0, 0, false, false, false, NULL, false } + handle_format_arg_attribute, false, NULL }, + { NULL, 0, 0, false, false, false, NULL, false, NULL } }; /* Returns TRUE iff the attribute indicated by ATTR_ID takes a plain @@ -515,14 +621,7 @@ handle_hot_attribute (tree *node, tree name, tree ARG_UNUSED (args), if (TREE_CODE (*node) == FUNCTION_DECL || TREE_CODE (*node) == LABEL_DECL) { - if (lookup_attribute ("cold", DECL_ATTRIBUTES (*node)) != NULL) - { - warning (OPT_Wattributes, "%qE attribute ignored due to conflict " - "with attribute %qs", name, "cold"); - *no_add_attrs = true; - } - /* Most of the rest of the hot processing is done later with - lookup_attribute. */ + /* Attribute hot processing is done later with lookup_attribute. */ } else { @@ -543,14 +642,7 @@ handle_cold_attribute (tree *node, tree name, tree ARG_UNUSED (args), if (TREE_CODE (*node) == FUNCTION_DECL || TREE_CODE (*node) == LABEL_DECL) { - if (lookup_attribute ("hot", DECL_ATTRIBUTES (*node)) != NULL) - { - warning (OPT_Wattributes, "%qE attribute ignored due to conflict " - "with attribute %qs", name, "hot"); - *no_add_attrs = true; - } - /* Most of the rest of the cold processing is done later with - lookup_attribute. */ + /* Attribute cold processing is done later with lookup_attribute. */ } else { @@ -1064,7 +1156,7 @@ handle_no_reorder_attribute (tree *pnode, static tree handle_const_attribute (tree *node, tree name, tree ARG_UNUSED (args), - int ARG_UNUSED (flags), bool *no_add_attrs) + int flags, bool *no_add_attrs) { tree type = TREE_TYPE (*node); @@ -1085,6 +1177,14 @@ handle_const_attribute (tree *node, tree name, tree ARG_UNUSED (args), *no_add_attrs = true; } + /* void __builtin_unreachable(void) is const. Accept other such + built-ins but warn on user-defined functions that return void. */ + if (!(flags & ATTR_FLAG_BUILT_IN) + && TREE_CODE (*node) == FUNCTION_DECL + && VOID_TYPE_P (TREE_TYPE (type))) + warning (OPT_Wattributes, "%qE attribute on function " + "returning %<void%>", name); + return NULL_TREE; } @@ -1667,14 +1767,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 hasn't been + yet. */ + tree last_decl = node[1] ? node[1] : *node; if (args) { @@ -1693,10 +1797,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; - if ((i = check_user_alignment (align_expr, true)) == -1 - || !check_cxx_fundamental_alignment_constraints (*node, i, flags)) + /* 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; + + /* 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) { @@ -1717,7 +1832,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) @@ -1726,8 +1841,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 @@ -1737,21 +1878,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; } @@ -2476,8 +2605,15 @@ handle_pure_attribute (tree *node, tree name, tree ARG_UNUSED (args), int ARG_UNUSED (flags), bool *no_add_attrs) { if (TREE_CODE (*node) == FUNCTION_DECL) - DECL_PURE_P (*node) = 1; - /* ??? TODO: Support types. */ + { + tree type = TREE_TYPE (*node); + if (VOID_TYPE_P (TREE_TYPE (type))) + warning (OPT_Wattributes, "%qE attribute on function " + "returning %<void%>", name); + + DECL_PURE_P (*node) = 1; + /* ??? TODO: Support types. */ + } else { warning (OPT_Wattributes, "%qE attribute ignored", name); diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c index e970ab2..f7c8eac 100644 --- a/gcc/c-family/c-warn.c +++ b/gcc/c-family/c-warn.c @@ -2143,36 +2143,19 @@ diagnose_mismatched_attributes (tree olddecl, tree newdecl) newdecl); /* Diagnose inline __attribute__ ((noinline)) which is silly. */ + const char *noinline = "noinline"; + if (DECL_DECLARED_INLINE_P (newdecl) && DECL_UNINLINABLE (olddecl) - && lookup_attribute ("noinline", DECL_ATTRIBUTES (olddecl))) + && lookup_attribute (noinline, DECL_ATTRIBUTES (olddecl))) warned |= warning (OPT_Wattributes, "inline declaration of %qD follows " - "declaration with attribute noinline", newdecl); + "declaration with attribute %qs", newdecl, noinline); else if (DECL_DECLARED_INLINE_P (olddecl) && DECL_UNINLINABLE (newdecl) && lookup_attribute ("noinline", DECL_ATTRIBUTES (newdecl))) warned |= warning (OPT_Wattributes, "declaration of %q+D with attribute " - "noinline follows inline declaration ", newdecl); - else if (lookup_attribute ("noinline", DECL_ATTRIBUTES (newdecl)) - && lookup_attribute ("always_inline", DECL_ATTRIBUTES (olddecl))) - warned |= warning (OPT_Wattributes, "declaration of %q+D with attribute " - "%qs follows declaration with attribute %qs", - newdecl, "noinline", "always_inline"); - else if (lookup_attribute ("always_inline", DECL_ATTRIBUTES (newdecl)) - && lookup_attribute ("noinline", DECL_ATTRIBUTES (olddecl))) - warned |= warning (OPT_Wattributes, "declaration of %q+D with attribute " - "%qs follows declaration with attribute %qs", - newdecl, "always_inline", "noinline"); - else if (lookup_attribute ("cold", DECL_ATTRIBUTES (newdecl)) - && lookup_attribute ("hot", DECL_ATTRIBUTES (olddecl))) - warned |= warning (OPT_Wattributes, "declaration of %q+D with attribute " - "%qs follows declaration with attribute %qs", - newdecl, "cold", "hot"); - else if (lookup_attribute ("hot", DECL_ATTRIBUTES (newdecl)) - && lookup_attribute ("cold", DECL_ATTRIBUTES (olddecl))) - warned |= warning (OPT_Wattributes, "declaration of %q+D with attribute " - "%qs follows declaration with attribute %qs", - newdecl, "hot", "cold"); + "%qs follows inline declaration ", newdecl, noinline); + return warned; } diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c index a54e121..5852c0d 100644 --- a/gcc/c/c-decl.c +++ b/gcc/c/c-decl.c @@ -4589,7 +4589,16 @@ c_decl_attributes (tree *node, tree attributes, int flags) attributes = tree_cons (get_identifier ("omp declare target"), NULL_TREE, attributes); } - return decl_attributes (node, attributes, flags); + + /* Look up the current declaration with all the attributes merged + so far so that attributes on the current declaration that's + about to be pushed that conflict with the former can be detected, + diagnosed, and rejected as appropriate. */ + tree last_decl = lookup_name (DECL_NAME (*node)); + if (!last_decl) + last_decl = lookup_name_in_scope (DECL_NAME (*node), external_scope); + + return decl_attributes (node, attributes, flags, last_decl); } diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index b253ccc..0db600a 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -2490,9 +2490,14 @@ are automatically detected and this attribute is ignored. @cindex @code{const} function attribute @cindex functions that have no side effects Many functions do not examine any values except their arguments, and -have no effects except the return value. Basically this is just slightly -more strict class than the @code{pure} attribute below, since function is not -allowed to read global memory. +have no effects except to return a value. Calls to such functions lend +themselves to optimization such as common subexpression elimination. +The @code{const} attribute imposes greater restrictions on a function's +definition than the similar @code{pure} attribute below because it prohibits +the function from reading global variables. Consequently, the presence of +the attribute on a function declarations allows GCC to emit more efficient +code for some calls to the function. Decorating the same function with +both the @code{const} and the @code{pure} attribute is diagnnosed. @cindex pointer arguments Note that a function that has pointer arguments and examines the data @@ -3144,7 +3149,7 @@ to prevent recursion. @cindex functions that have no side effects Many functions have no effects except the return value and their return value depends only on the parameters and/or global variables. -Such a function can be subject +Calls to such functions can be subject to common subexpression elimination and loop optimization just as an arithmetic operator would be. These functions should be declared with the attribute @code{pure}. For example, @@ -3162,6 +3167,11 @@ Interesting non-pure functions are functions with infinite loops or those depending on volatile memory or other system resource, that may change between two consecutive calls (such as @code{feof} in a multithreading environment). +The @code{pure} attribute imposes similar but looser restrictions on +a function's defintion than the @code{const} attribute: it allows the +function to read global variables. Decorating the same function with +both the @code{pure} and the @code{const} attribute is diagnosed. + @item returns_nonnull @cindex @code{returns_nonnull} function attribute The @code{returns_nonnull} attribute specifies that the function diff --git a/gcc/testsuite/c-c++-common/attributes-3.c b/gcc/testsuite/c-c++-common/attributes-3.c index 821278c..9d3a61c 100644 --- a/gcc/testsuite/c-c++-common/attributes-3.c +++ b/gcc/testsuite/c-c++-common/attributes-3.c @@ -12,16 +12,16 @@ extern __attribute__((noinline)) int fn1 (void); /* { dg-message "previous decla extern inline int fn1 (void); /* { dg-warning "inline declaration of" } */ extern inline int fn2 (void); /* { dg-message "previous declaration" } */ -extern __attribute__((noinline)) int fn2 (void); /* { dg-warning "attribute noinline follows inline declaration" } */ +extern __attribute__((noinline)) int fn2 (void); /* { dg-warning "attribute .noinline. follows inline declaration" } */ extern __attribute__((always_inline)) int fn3 (void); /* { dg-message "previous declaration" } */ -extern __attribute__((noinline)) int fn3 (void); /* { dg-warning "attribute .noinline. follows declaration with attribute .always_inline." } */ +extern __attribute__((noinline)) int fn3 (void); /* { dg-warning "ignoring attribute .noinline. because it conflicts with attribute .always_inline." } */ extern __attribute__((noinline)) int fn4 (void); /* { dg-message "previous declaration" } */ -extern __attribute__((always_inline)) int fn4 (void); /* { dg-warning "attribute .always_inline. follows declaration with attribute .noinline." } */ +extern __attribute__((always_inline)) int fn4 (void); /* { dg-warning "ignoring attribute .always_inline. because it conflicts with attribute .noinline." } */ extern __attribute__((hot)) int fn5 (void); /* { dg-message "previous declaration" } */ -extern __attribute__((cold)) int fn5 (void); /* { dg-warning "attribute .cold. follows declaration with attribute .hot." } */ +extern __attribute__((cold)) int fn5 (void); /* { dg-warning "ignoring attribute .cold. because it conflicts with attribute .hot." } */ extern __attribute__((cold)) int fn6 (void); /* { dg-message "previous declaration" } */ -extern __attribute__((hot)) int fn6 (void); /* { dg-warning "attribute .hot. follows declaration with attribute .cold." } */ +extern __attribute__((hot)) int fn6 (void); /* { dg-warning "ignoring attribute .hot. because it conflicts with attribute .cold." } */ diff --git a/gcc/testsuite/gcc.dg/attr-noinline.c b/gcc/testsuite/gcc.dg/attr-noinline.c index c2a5b1d..13cc660 100644 --- a/gcc/testsuite/gcc.dg/attr-noinline.c +++ b/gcc/testsuite/gcc.dg/attr-noinline.c @@ -17,7 +17,7 @@ static void function_declaration_both_after(void) {t();} static void function_declaration_noinline_before(void) __attribute__((__noinline__)); /* { dg-message "note: previous declaration" } */ -static inline void function_declaration_noinline_before(void) {t();} /* { dg-warning "follows declaration with attribute noinline" } */ +static inline void function_declaration_noinline_before(void) {t();} /* { dg-warning "follows declaration with attribute .noinline." } */ static inline void function_declaration_noinline_after(void) {t();} /* { dg-message "note: previous definition" } */ @@ -41,7 +41,7 @@ static void function_declaration_inline_noinline_after(void) __attribute__((__no static void function_declaration_noinline_inline_before(void) __attribute__((__noinline__)); /* { dg-message "note: previous declaration" } */ -static inline void function_declaration_noinline_inline_before(void); /* { dg-warning "follows declaration with attribute noinline" } */ +static inline void function_declaration_noinline_inline_before(void); /* { dg-warning "follows declaration with attribute .noinline." } */ static void function_declaration_noinline_inline_before(void) {t();} diff --git a/gcc/testsuite/gcc.dg/pr44964.c b/gcc/testsuite/gcc.dg/pr44964.c index 1df1bde..6c252ee 100644 --- a/gcc/testsuite/gcc.dg/pr44964.c +++ b/gcc/testsuite/gcc.dg/pr44964.c @@ -2,8 +2,9 @@ /* { dg-options "-fkeep-inline-functions -O" } */ static inline __attribute__ ((const)) -void baz (int i) +int baz (int i) { + return i; } static __attribute__ ((always_inline)) diff --git a/gcc/testsuite/gcc.dg/torture/pr42363.c b/gcc/testsuite/gcc.dg/torture/pr42363.c index 9c9da13..ad0eac8 100644 --- a/gcc/testsuite/gcc.dg/torture/pr42363.c +++ b/gcc/testsuite/gcc.dg/torture/pr42363.c @@ -46,16 +46,18 @@ int bizr (void) return i + 1; } -/* This might be regarded as pure and folded, rather than inlined. - It's pure evil. */ +/* This might be regarded as pure and folded, rather than inlined, + but because it's pure evil it's diagnosed and the noreturn attribute + is dropped. The const attribute is dropped as well because it's + mutually exclusive with pure. */ static int __attribute__ ((pure, const, noreturn)) -barf (void) -{ +barf (void) { + /* { dg-warning "ignoring attribute .const." "const" { target *-*-* } .-1 } */ + /* { dg-warning "ignoring attribute .noreturn." "noreturn" { target *-*-* } .-2 } */ } /* { dg-warning "does return" } */ static int __attribute__ ((pure, const)) -bark (void) -{ +bark (void) { /* { dg-warning "ignoring attribute .const." } */ barf (); } diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c index 146b76c..b8c5654 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c @@ -113,17 +113,18 @@ int test9 (int *intarr) int test99 (int *intarr) { - extern int foo9 (int) __attribute__ ((pure)); + extern int foo99 (int) __attribute__ ((pure)); int h, v; g9 = 9; - h = foo9 (g9); + h = foo99 (g9); v = g9; if (v != 9) link_error (); return g9; } -extern int foo99 (int); +/* foo9 is const because of its declaration in test9. */ +extern int foo9 (int); int test999 (int *arr) { @@ -134,10 +135,12 @@ int test999 (int *arr) v1 = g9; if (v1 != 9) link_error (); - l = foo99 (l); + l = foo9 (l); return v1 + l; } +/* foo99 is pure because of its declaration in test99. */ +extern int foo9 (int); int test9999 (void) {
On 09/12/2017 11:06 AM, Jeff Law wrote: > On 08/17/2017 10:03 AM, Martin Sebor wrote: >> >> First, there is the risk that someone will write code based >> on the pure declaration even if there's no intervening call >> to the function between it and the const one. Code tends to >> be sloppy, and it's also not uncommon to declare the same >> function more than once, for whatever reason. (The ssa-ccp-2.c >> test is an example of the latter.) > True. But I think if we get into a state where the semantics of the > implementation are incompatible with the listed attributes (and we can > reliably detect that) then we should issue a separate and distinct > warning. For example a pure function that reads global memory should > issue a warning/error. > > ISTM such a warning/error would be a separate and distinct patch than > detection of attribute conflicts. I agree. This warning is for the case when the implementation is not visible and the mismatch between the declaration and its definition cannot be detected. E.g., when the user of a function defined in another translation unit declares it based on a different (e.g., earlier) version of it, or based on a misunderstanding of its guarantees. >> Second, there are cases of attribute conflicts that GCC already >> points out that are much more benign in their effects (the ones >> I know about are always_inline/noinline and cold/hot). In light >> of the risk above it seems only helpful to include const/pure in >> the same set. > always_inline/noinline conflicts can cause build failures in codebases > that require certain functions to be inlined. We can argue about > whether or not that's a good policy for those codebases, but they > certainly exist. I could envision the hot/cold case generating a link > error if the linker script shoved them into different memory segments > and getting them wrong caused an overflow. > > But these IMHO are corner cases and should not drive major decisions here. The consequence of a const/pure conflict can be "wrong" code (i.e., generating code for a call to a function declared const when its definition is, in fact, only pure, will have unexpected effects). When both the definition and the mismatched declaration are visible the mismatch between it and its declaration could be detected (as a future enhancement). But without it, the second best and, IMO, second most helpful thing GCC can do for two declarations of the same function, one const and the other pure, is to issue a warning about the mismatched guarantees the declarations provide to the caller. (The ideal would be to use the pure attribute instead of const and also emit a warning.) >> Third, I couldn't find another pair of attributes that GCC would >> deliberately handle this way (silently accept both but prefer one >> over the other), and introducing a special case just for these >> two seemed like a wart. > Seems reasonable. > >> I do still have the question whether diagnosing attribute >> conflicts under -Wattributes is right. The manual implies >> that -Wattributes is for attributes in the wrong places or >> on the wrong entities, and that -Wignored-attributes should >> be expected instead when GCC decides to drop one for some >> reason. > This seems like it ought to be a distinct option given the way > -Wattributes is documented. Alternately we can tweak the meaning of > -Wattributes. I don't have strong opinions here. > Okay. > >> >> It is a little unfortunate that many -Wattributes warnings >> print just "attribute ignored" (and have done so for years). >> I think they should all be enhanced to also print why the >> attribute is ignored (e.g., "'packed' attribute on function >> declarations ignored/not valid/not supported" or something >> like that). Those that ignore attributes that would >> otherwise be valid e.g., because they conflict with other >> specifiers of the same attribute but with a different >> operand might then be candidate for changing to >> -Wignored-attributes. > Seems like a reasonable follow-up if you're interested. Okay. > > >> >> Thanks >> Martin >> >> >> gcc-81544-1.diff >> >> >> PR c/81544 - attribute noreturn and warn_unused_result on the same function accepted >> >> gcc/c/ChangeLog: >> >> PR c/81544 >> * c-decl.c (c_decl_attributes): Look up existing declaration and >> pass it to decl_attributes. >> >> gcc/c-family/ChangeLog: >> >> PR c/81544 >> * c-attribs.c (attr_aligned_exclusions): New array. >> (attr_alloc_exclusions, attr_cold_hot_exclusions): Same. >> (attr_common_exclusions, attr_const_pure_exclusions): Same. >> (attr_gnu_inline_exclusions, attr_inline_exclusions): Same. >> (attr_noreturn_exclusions, attr_returns_twice_exclusions): Same. >> (attr_warn_unused_result_exclusions): Same. >> (handle_hot_attribute, handle_cold_attribute): Simplify. >> (handle_const_attribute): Warn on function returning void. >> (handle_pure_attribute): Same. >> * c-warn.c (diagnose_mismatched_attributes): Simplify. >> >> gcc/testsuite/ChangeLog: >> >> PR c/81544 >> * c-c++-common/Wattributes-2.c: New test. >> * c-c++-common/Wattributes.c: New test. >> * c-c++-common/attributes-3.c: Adjust. >> * gcc.dg/attr-noinline.c: Adjust. >> * gcc.dg/pr44964.c: Same. >> * gcc.dg/torture/pr42363.c: Same. >> * gcc.dg/tree-ssa/ssa-ccp-2.c: Same. >> >> gcc/ChangeLog: >> >> PR c/81544 >> * attribs.c (empty_attribute_table): Initialize new member of >> struct attribute_spec. >> (decl_attributes): Add argument. Handle mutually exclusive >> combinations of attributes. >> * attribs.h (decl_attributes): Add default argument. >> * tree-core.h (attribute_spec::exclusions, exclude): New type and >> member. >> * doc/extend.texi (Common Function Attributes): Update const and pure. > > > >> diff --git a/gcc/attribs.h b/gcc/attribs.h >> index d4a790b..a9eba0a 100644 >> --- a/gcc/attribs.h >> +++ b/gcc/attribs.h >> @@ -31,7 +31,7 @@ extern void init_attributes (void); >> from tree.h. Depending on these flags, some attributes may be >> returned to be applied at a later stage (for example, to apply >> a decl attribute to the declaration rather than to its type). */ >> -extern tree decl_attributes (tree *, tree, int); >> +extern tree decl_attributes (tree *, tree, int, tree = NULL_TREE); > So we do allow default arguments. Though there seems to be some > preference for just defining an overload instead. The arguments for > using overloads instead of a default argument here aren't real > compelling though. > > Anyway, consider using an overload. If you think the default argument > is cleaner, I won't object. A default argument effectively adds an overload without the overhead of defining one (i.e., a single declaration and definition vs one for each) so I find it preferable (AFAICS, it's also in line with how other similar cases are handled elsewhere in GCC). >> extern bool cxx11_attribute_p (const_tree); >> extern tree get_attribute_name (const_tree); >> diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c >> index 0d9ab2d..8a157f6 100644 >> --- a/gcc/c-family/c-attribs.c >> +++ b/gcc/c-family/c-attribs.c >> @@ -44,6 +44,7 @@ along with GCC; see the file COPYING3. If not see >> #include "tree-iterator.h" >> #include "opts.h" >> #include "gimplify.h" >> +#include "bitmap.h" > I don't immediately see where you need bitmap.h in here, but I could > have missed it. It's not a problem if you need it, but if not, let's > avoid adding it unnecessarily. Thanks. It's not needed so I've removed it. >> @@ -1667,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 > s/hassn't/hasn't/ > > > I'd like to echo Joseph's comment WRT the mirroring of exclusions. > Ideally setting up the structure so that happens automatically is best, > but alternately, testing for symmetry is acceptable. If asymmetry is > desirable, it needs to be documented. > > Largely it looks good. I think the biggest issue is the exclusion > tables and how to deal with cases where we need to ensure symmetry vs > any cases where asymmetry is expected/desirable. I hope my response to Joseph along with the updated patch resolves this concern. If not, let me know. Thanks Martin
On Tue, 19 Sep 2017, Martin Sebor wrote: > > In general, the data structures where you need to ensure manually that if > > attribute A is listed in EXCL for B, then attribute B is also listed in > > EXCL for A, seem concerning. I'd expect either data structures that make > > such asymmetry impossible, or a self-test that verifies that the tables in > > use are in fact symmetric (unless there is some reason the symmetry is not > > in fact required and symmetric diagnostics still result from asymmetric > > tables - in which case the various combinations and orderings of > > gnu_inline and noinline definitely need tests to show that the diagnostics > > work). > > If I understand correctly what you're concerned about then I don't > think there are any such cases in the updated version of the patch. I don't see how you ensure that it's not possible to have such asymmetry. My point wasn't so much "there was a bug in the previous patch version" as "the choice of data structures for defining such exclusions is prone to such bugs". Which can be addressed either by using different data structures (e.g. listing incompatible pairs in a single array) or by a self-test to verify symmetry so a compiler with asymmetry doesn't build.
On 09/19/2017 03:00 PM, Joseph Myers wrote: > On Tue, 19 Sep 2017, Martin Sebor wrote: > >>> In general, the data structures where you need to ensure manually that if >>> attribute A is listed in EXCL for B, then attribute B is also listed in >>> EXCL for A, seem concerning. I'd expect either data structures that make >>> such asymmetry impossible, or a self-test that verifies that the tables in >>> use are in fact symmetric (unless there is some reason the symmetry is not >>> in fact required and symmetric diagnostics still result from asymmetric >>> tables - in which case the various combinations and orderings of >>> gnu_inline and noinline definitely need tests to show that the diagnostics >>> work). >> >> If I understand correctly what you're concerned about then I don't >> think there are any such cases in the updated version of the patch. > > I don't see how you ensure that it's not possible to have such asymmetry. > My point wasn't so much "there was a bug in the previous patch version" as > "the choice of data structures for defining such exclusions is prone to > such bugs". Which can be addressed either by using different data > structures (e.g. listing incompatible pairs in a single array) or by a > self-test to verify symmetry so a compiler with asymmetry doesn't build. Okay, that's a useful thing to add. It exposed a couple of missing attribute exclusions that I had overlooked. Thanks for the suggestion! Attached is an incremental diff with just these changes to make review easier and an updated patch. As an aside, there are a number of other possible logic errors in the attribute specifications that could be detected by self-tests. The one I ran into is misspelled attribute names. The added test detects misspelled names in exclusions, but not in the main specs. Martin diff --git a/gcc/attribs.c b/gcc/attribs.c index 3f9ae37..2f9a692 100644 --- a/gcc/attribs.c +++ b/gcc/attribs.c @@ -28,6 +28,8 @@ along with GCC; see the file COPYING3. If not see #include "stor-layout.h" #include "langhooks.h" #include "plugin.h" +#include "selftest.h" +#include "hash-set.h" /* Table of the tables of attributes (common, language, format, machine) searched. */ @@ -1078,3 +1080,124 @@ is_function_default_version (const tree decl) return (TREE_CODE (attr) == STRING_CST && strcmp (TREE_STRING_POINTER (attr), "default") == 0); } + +#if CHECKING_P + +namespace selftest +{ + +/* Helper types to verify the consistency attribute exclusions. */ + +typedef std::pair<const char *, const char *> excl_pair; + +struct excl_hash_traits: typed_noop_remove<excl_pair> +{ + typedef excl_pair value_type; + typedef value_type compare_type; + + static hashval_t hash (const value_type &x) + { + hashval_t h1 = htab_hash_string (x.first); + hashval_t h2 = htab_hash_string (x.second); + return h1 ^ h2; + } + + static bool equal (const value_type &x, const value_type &y) + { + return !strcmp (x.first, y.first) && !strcmp (x.second, y.second); + } + + static void mark_deleted (value_type &x) + { + x = value_type (NULL, NULL); + } + + static void mark_empty (value_type &x) + { + x = value_type ("", ""); + } + + static bool is_deleted (const value_type &x) + { + return !x.first && !x.second; + } + + static bool is_empty (const value_type &x) + { + return !*x.first && !*x.second; + } +}; + + +/* Self-test to verify that each attribute exclusion is symmetric, + meaning that if attribute A is encoded as incompatible with + attribute B then the opposite relationship is also encoded. + This test also detects most cases of misspelled attribute names + in exclusions. */ + +static void +test_attribute_exclusions () +{ + /* Iterate over the array of attribute tables first (with TI0 as + the index) and over the array of attribute_spec in each table + (with SI0 as the index). */ + const size_t ntables = ARRAY_SIZE (attribute_tables); + + /* Set of pairs of mutually exclusive attributes. */ + typedef hash_set<excl_pair, excl_hash_traits> exclusion_set; + exclusion_set excl_set; + + for (size_t ti0 = 0; ti0 != ntables; ++ti0) + for (size_t s0 = 0; attribute_tables[ti0][s0].name; ++s0) + { + const attribute_spec::exclusions *excl + = attribute_tables[ti0][s0].exclude; + + /* Skip each attribute that doesn't define exclusions. */ + if (!excl) + continue; + + const char *attr_name = attribute_tables[ti0][s0].name; + + /* Iterate over the set of exclusions for every attribute + (with EI0 as the index) adding the exclusions defined + for each to the set. */ + for (size_t ei0 = 0; excl[ei0].name; ++ei0) + { + const char *excl_name = excl[ei0].name; + + if (!strcmp (attr_name, excl_name)) + continue; + + excl_set.add (excl_pair (attr_name, excl_name)); + } + } + + /* Traverse the set of mutually exclusive pairs of attributes + and verify that they are symmetric. */ + for (exclusion_set::iterator it = excl_set.begin (); + it != excl_set.end (); + ++it) + { + if (!excl_set.contains (excl_pair ((*it).second, (*it).first))) + { + /* An exclusion for an attribute has been found that + doesn't have a corresponding exclusion in the opposite + direction. */ + char desc[120]; + sprintf (desc, "'%s' attribute exclusion '%s' must be symmetric", + (*it).first, (*it).second); + fail (SELFTEST_LOCATION, desc); + } + } +} + +void +attribute_c_tests () +{ + test_attribute_exclusions (); +} + +} /* namespace selftest */ + +#endif /* CHECKING_P */ diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c index 17d6993..f1adf64 100644 --- a/gcc/c-family/c-attribs.c +++ b/gcc/c-family/c-attribs.c @@ -189,7 +189,6 @@ static const struct attribute_spec::exclusions attr_noinline_exclusions[] = static const struct attribute_spec::exclusions attr_noreturn_exclusions[] = { - ATTR_EXCL ("noreturn", true, true, true), ATTR_EXCL ("alloc_align", true, true, true), ATTR_EXCL ("alloc_size", true, true, true), ATTR_EXCL ("const", true, true, true), @@ -226,6 +225,9 @@ static const struct attribute_spec::exclusions attr_alloc_exclusions[] = static const struct attribute_spec::exclusions attr_const_pure_exclusions[] = { ATTR_EXCL ("const", true, true, true), + ATTR_EXCL ("alloc_align", true, true, true), + ATTR_EXCL ("alloc_size", true, true, true), + ATTR_EXCL ("malloc", true, true, true), ATTR_EXCL ("noreturn", true, true, true), ATTR_EXCL ("pure", true, true, true), ATTR_EXCL (NULL, false, false, false) diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c index 30e476d..9af84a0 100644 --- a/gcc/selftest-run-tests.c +++ b/gcc/selftest-run-tests.c @@ -25,6 +25,8 @@ along with GCC; see the file COPYING3. If not see #include "target.h" #include "langhooks.h" #include "options.h" +#include "stringpool.h" +#include "attribs.h" /* This function needed to be split out from selftest.c as it references tests from the whole source tree, and so is within @@ -83,6 +85,7 @@ selftest::run_tests () spellcheck_c_tests (); spellcheck_tree_c_tests (); tree_cfg_c_tests (); + attribute_c_tests (); /* This one relies on most of the above. */ function_tests_c_tests (); diff --git a/gcc/selftest.h b/gcc/selftest.h index 0572fef..608f4ee 100644 --- a/gcc/selftest.h +++ b/gcc/selftest.h @@ -170,6 +170,7 @@ extern const char *path_to_selftest_files; /* Declarations for specific families of tests (by source file), in alphabetical order. */ +extern void attribute_c_tests (); extern void bitmap_c_tests (); extern void diagnostic_c_tests (); extern void diagnostic_show_locus_c_tests (); PR c/81544 - attribute noreturn and warn_unused_result on the same function accepted gcc/c/ChangeLog: PR c/81544 * c-decl.c (c_decl_attributes): Look up existing declaration and pass it to decl_attributes. gcc/c-family/ChangeLog: PR c/81544 * c-attribs.c (attr_aligned_exclusions): New array. (attr_alloc_exclusions, attr_cold_hot_exclusions): Same. (attr_common_exclusions, attr_const_pure_exclusions): Same. (attr_gnu_inline_exclusions, attr_inline_exclusions): Same. (attr_noreturn_exclusions, attr_returns_twice_exclusions): Same. (attr_warn_unused_result_exclusions): Same. (handle_hot_attribute, handle_cold_attribute): Simplify. (handle_const_attribute): Warn on function returning void. (handle_pure_attribute): Same. * c-warn.c (diagnose_mismatched_attributes): Simplify. gcc/ChangeLog: PR c/81544 * attribs.c (empty_attribute_table): Initialize new member of struct attribute_spec. (decl_attributes): Add argument. Handle mutually exclusive combinations of attributes. * attribs.h (decl_attributes): Add default argument. * selftest.h (attribute_c_tests): Declare. * selftest-run-tests.c (selftest::run_tests): Call attribute_c_tests. * tree-core.h (attribute_spec::exclusions, exclude): New type and member. * doc/extend.texi (Common Function Attributes): Update const and pure. gcc/testsuite/ChangeLog: PR c/81544 * c-c++-common/Wattributes-2.c: New test. * c-c++-common/Wattributes.c: New test. * c-c++-common/attributes-3.c: Adjust. * gcc.dg/attr-noinline.c: Adjust. * gcc.dg/pr44964.c: Same. * gcc.dg/torture/pr42363.c: Same. * gcc.dg/tree-ssa/ssa-ccp-2.c: Same. diff --git a/gcc/attribs.c b/gcc/attribs.c index 05fa8ef..2f9a692 100644 --- a/gcc/attribs.c +++ b/gcc/attribs.c @@ -28,6 +28,8 @@ along with GCC; see the file COPYING3. If not see #include "stor-layout.h" #include "langhooks.h" #include "plugin.h" +#include "selftest.h" +#include "hash-set.h" /* Table of the tables of attributes (common, language, format, machine) searched. */ @@ -94,7 +96,7 @@ static bool attributes_initialized = false; static const struct attribute_spec empty_attribute_table[] = { - { NULL, 0, 0, false, false, false, NULL, false } + { NULL, 0, 0, false, false, false, NULL, false, NULL } }; /* Return base name of the attribute. Ie '__attr__' is turned into 'attr'. @@ -343,6 +345,97 @@ get_attribute_namespace (const_tree attr) return get_identifier ("gnu"); } +/* Check LAST_DECL and NODE of the same symbol for attributes that are + recorded in SPEC to be mutually exclusive with ATTRNAME, diagnose + them, and return true if any have been found. NODE can be a DECL + or a TYPE. */ + +static bool +diag_attr_exclusions (tree last_decl, tree node, tree attrname, + const attribute_spec *spec) +{ + const attribute_spec::exclusions *excl = spec->exclude; + + tree_code code = TREE_CODE (node); + + if ((code == FUNCTION_DECL && !excl->function + && (!excl->type || !spec->affects_type_identity)) + || (code == VAR_DECL && !excl->variable + && (!excl->type || !spec->affects_type_identity)) + || (((code == TYPE_DECL || RECORD_OR_UNION_TYPE_P (node)) && !excl->type))) + return false; + + /* True if an attribute that's mutually exclusive with ATTRNAME + has been found. */ + bool found = false; + + if (last_decl && last_decl != node && TREE_TYPE (last_decl) != node) + { + /* Check both the last DECL and its type for conflicts with + the attribute being added to the current decl or type. */ + found |= diag_attr_exclusions (last_decl, last_decl, attrname, spec); + tree decl_type = TREE_TYPE (last_decl); + found |= diag_attr_exclusions (last_decl, decl_type, attrname, spec); + } + + /* NODE is either the current DECL to which the attribute is being + applied or its TYPE. For the former, consider the attributes on + both the DECL and its type. */ + tree attrs[2]; + + if (DECL_P (node)) + { + attrs[0] = DECL_ATTRIBUTES (node); + attrs[1] = TYPE_ATTRIBUTES (TREE_TYPE (node)); + } + else + { + attrs[0] = TYPE_ATTRIBUTES (node); + attrs[1] = NULL_TREE; + } + + /* Iterate over the mutually exclusive attribute names and verify + that the symbol doesn't contain it. */ + for (unsigned i = 0; i != sizeof attrs / sizeof *attrs; ++i) + { + if (!attrs[i]) + continue; + + for ( ; excl->name; ++excl) + { + /* Avoid checking the attribute against itself. */ + if (is_attribute_p (excl->name, attrname)) + continue; + + if (!lookup_attribute (excl->name, attrs[i])) + continue; + + found = true; + + /* Print a note? */ + bool note = last_decl != NULL_TREE; + + if (TREE_CODE (node) == FUNCTION_DECL + && DECL_BUILT_IN (node)) + note &= warning (OPT_Wattributes, + "ignoring attribute %qE in declaration of " + "a built-in function %qD because it conflicts " + "with attribute %qs", + attrname, node, excl->name); + else + note &= warning (OPT_Wattributes, + "ignoring attribute %qE because " + "it conflicts with attribute %qs", + attrname, excl->name); + + if (note) + inform (DECL_SOURCE_LOCATION (last_decl), + "previous declaration here"); + } + } + + return found; +} /* Process the attributes listed in ATTRIBUTES and install them in *NODE, which is either a DECL (including a TYPE_DECL) or a TYPE. If a DECL, @@ -354,7 +447,8 @@ get_attribute_namespace (const_tree attr) a decl attribute to the declaration rather than to its type). */ tree -decl_attributes (tree *node, tree attributes, int flags) +decl_attributes (tree *node, tree attributes, int flags, + tree last_decl /* = NULL_TREE */) { tree a; tree returned_attrs = NULL_TREE; @@ -433,6 +527,8 @@ decl_attributes (tree *node, tree attributes, int flags) targetm.insert_attributes (*node, &attributes); + /* Note that attributes on the same declaration are not necessarily + in the same order as in the source. */ for (a = attributes; a; a = TREE_CHAIN (a)) { tree ns = get_attribute_namespace (a); @@ -441,7 +537,6 @@ decl_attributes (tree *node, tree attributes, int flags) tree *anode = node; const struct attribute_spec *spec = lookup_scoped_attribute_spec (ns, name); - bool no_add_attrs = 0; int fn_ptr_quals = 0; tree fn_ptr_tmp = NULL_TREE; @@ -490,7 +585,8 @@ decl_attributes (tree *node, tree attributes, int flags) | (int) ATTR_FLAG_ARRAY_NEXT)) { /* Pass on this attribute to be tried again. */ - returned_attrs = tree_cons (name, args, returned_attrs); + tree attr = tree_cons (name, args, NULL_TREE); + returned_attrs = chainon (returned_attrs, attr); continue; } else @@ -535,7 +631,8 @@ decl_attributes (tree *node, tree attributes, int flags) else if (flags & (int) ATTR_FLAG_FUNCTION_NEXT) { /* Pass on this attribute to be tried again. */ - returned_attrs = tree_cons (name, args, returned_attrs); + tree attr = tree_cons (name, args, NULL_TREE); + returned_attrs = chainon (returned_attrs, attr); continue; } @@ -557,15 +654,56 @@ decl_attributes (tree *node, tree attributes, int flags) continue; } + bool no_add_attrs = false; + if (spec->handler != NULL) { int cxx11_flag = cxx11_attribute_p (a) ? ATTR_FLAG_CXX11 : 0; - returned_attrs = chainon ((*spec->handler) (anode, name, args, - flags|cxx11_flag, - &no_add_attrs), - returned_attrs); + /* Pass in an array of the current declaration followed + by the last pushed/merged declaration if one exists. + If the handler changes CUR_AND_LAST_DECL[0] replace + *ANODE with its value. */ + tree cur_and_last_decl[] = { *anode, last_decl }; + tree ret = (spec->handler) (cur_and_last_decl, name, args, + flags|cxx11_flag, &no_add_attrs); + + *anode = cur_and_last_decl[0]; + if (ret == error_mark_node) + { + warning (OPT_Wattributes, "%qE attribute ignored", name); + no_add_attrs = true; + } + else + 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. */ @@ -942,3 +1080,124 @@ is_function_default_version (const tree decl) return (TREE_CODE (attr) == STRING_CST && strcmp (TREE_STRING_POINTER (attr), "default") == 0); } + +#if CHECKING_P + +namespace selftest +{ + +/* Helper types to verify the consistency attribute exclusions. */ + +typedef std::pair<const char *, const char *> excl_pair; + +struct excl_hash_traits: typed_noop_remove<excl_pair> +{ + typedef excl_pair value_type; + typedef value_type compare_type; + + static hashval_t hash (const value_type &x) + { + hashval_t h1 = htab_hash_string (x.first); + hashval_t h2 = htab_hash_string (x.second); + return h1 ^ h2; + } + + static bool equal (const value_type &x, const value_type &y) + { + return !strcmp (x.first, y.first) && !strcmp (x.second, y.second); + } + + static void mark_deleted (value_type &x) + { + x = value_type (NULL, NULL); + } + + static void mark_empty (value_type &x) + { + x = value_type ("", ""); + } + + static bool is_deleted (const value_type &x) + { + return !x.first && !x.second; + } + + static bool is_empty (const value_type &x) + { + return !*x.first && !*x.second; + } +}; + + +/* Self-test to verify that each attribute exclusion is symmetric, + meaning that if attribute A is encoded as incompatible with + attribute B then the opposite relationship is also encoded. + This test also detects most cases of misspelled attribute names + in exclusions. */ + +static void +test_attribute_exclusions () +{ + /* Iterate over the array of attribute tables first (with TI0 as + the index) and over the array of attribute_spec in each table + (with SI0 as the index). */ + const size_t ntables = ARRAY_SIZE (attribute_tables); + + /* Set of pairs of mutually exclusive attributes. */ + typedef hash_set<excl_pair, excl_hash_traits> exclusion_set; + exclusion_set excl_set; + + for (size_t ti0 = 0; ti0 != ntables; ++ti0) + for (size_t s0 = 0; attribute_tables[ti0][s0].name; ++s0) + { + const attribute_spec::exclusions *excl + = attribute_tables[ti0][s0].exclude; + + /* Skip each attribute that doesn't define exclusions. */ + if (!excl) + continue; + + const char *attr_name = attribute_tables[ti0][s0].name; + + /* Iterate over the set of exclusions for every attribute + (with EI0 as the index) adding the exclusions defined + for each to the set. */ + for (size_t ei0 = 0; excl[ei0].name; ++ei0) + { + const char *excl_name = excl[ei0].name; + + if (!strcmp (attr_name, excl_name)) + continue; + + excl_set.add (excl_pair (attr_name, excl_name)); + } + } + + /* Traverse the set of mutually exclusive pairs of attributes + and verify that they are symmetric. */ + for (exclusion_set::iterator it = excl_set.begin (); + it != excl_set.end (); + ++it) + { + if (!excl_set.contains (excl_pair ((*it).second, (*it).first))) + { + /* An exclusion for an attribute has been found that + doesn't have a corresponding exclusion in the opposite + direction. */ + char desc[120]; + sprintf (desc, "'%s' attribute exclusion '%s' must be symmetric", + (*it).first, (*it).second); + fail (SELFTEST_LOCATION, desc); + } + } +} + +void +attribute_c_tests () +{ + test_attribute_exclusions (); +} + +} /* namespace selftest */ + +#endif /* CHECKING_P */ diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c index 0d9ab2d..3c0e148 100644 --- a/gcc/c-family/c-attribs.c +++ b/gcc/c-family/c-attribs.c @@ -146,6 +146,93 @@ static tree handle_fallthrough_attribute (tree *, tree, tree, int, bool *); static tree handle_patchable_function_entry_attribute (tree *, tree, tree, int, bool *); +/* Helper to define attribute exclusions. */ +#define ATTR_EXCL(name, function, type, variable) \ + { name, function, type, variable } + +/* Define attributes that are mutually exclusive with one another. */ +static const struct attribute_spec::exclusions attr_aligned_exclusions[] = +{ + /* Attribute name exclusion applies to: + function, type, variable */ + ATTR_EXCL ("aligned", true, false, false), + ATTR_EXCL ("packed", true, false, false), + ATTR_EXCL (NULL, false, false, false) +}; + +static const struct attribute_spec::exclusions attr_cold_hot_exclusions[] = +{ + ATTR_EXCL ("cold", true, true, true), + ATTR_EXCL ("hot", true, true, true), + ATTR_EXCL (NULL, false, false, false) +}; + +static const struct attribute_spec::exclusions attr_common_exclusions[] = +{ + ATTR_EXCL ("common", true, true, true), + ATTR_EXCL ("nocommon", true, true, true), + ATTR_EXCL (NULL, false, false, false), +}; + +static const struct attribute_spec::exclusions attr_inline_exclusions[] = +{ + ATTR_EXCL ("noinline", true, true, true), + ATTR_EXCL (NULL, false, false, false), +}; + +static const struct attribute_spec::exclusions attr_noinline_exclusions[] = +{ + ATTR_EXCL ("always_inline", true, true, true), + ATTR_EXCL ("gnu_inline", true, true, true), + ATTR_EXCL (NULL, false, false, false), +}; + +static const struct attribute_spec::exclusions attr_noreturn_exclusions[] = +{ + ATTR_EXCL ("alloc_align", true, true, true), + ATTR_EXCL ("alloc_size", true, true, true), + ATTR_EXCL ("const", true, true, true), + ATTR_EXCL ("malloc", true, true, true), + ATTR_EXCL ("pure", true, true, true), + ATTR_EXCL ("returns_twice", true, true, true), + ATTR_EXCL ("warn_unused_result", true, true, true), + ATTR_EXCL (NULL, false, false, false), +}; + +static const struct attribute_spec::exclusions +attr_warn_unused_result_exclusions[] = +{ + ATTR_EXCL ("noreturn", true, true, true), + ATTR_EXCL ("warn_unused_result", true, true, true), + ATTR_EXCL (NULL, false, false, false), +}; + +static const struct attribute_spec::exclusions attr_returns_twice_exclusions[] = +{ + ATTR_EXCL ("noreturn", true, true, true), + ATTR_EXCL (NULL, false, false, false), +}; + +/* Exclusions that apply to attribute alloc_align, alloc_size, and malloc. */ +static const struct attribute_spec::exclusions attr_alloc_exclusions[] = +{ + ATTR_EXCL ("const", true, true, true), + ATTR_EXCL ("noreturn", true, true, true), + ATTR_EXCL ("pure", true, true, true), + ATTR_EXCL (NULL, false, false, false), +}; + +static const struct attribute_spec::exclusions attr_const_pure_exclusions[] = +{ + ATTR_EXCL ("const", true, true, true), + ATTR_EXCL ("alloc_align", true, true, true), + ATTR_EXCL ("alloc_size", true, true, true), + ATTR_EXCL ("malloc", true, true, true), + ATTR_EXCL ("noreturn", true, true, true), + ATTR_EXCL ("pure", true, true, true), + ATTR_EXCL (NULL, false, false, false) +}; + /* Table of machine-independent attributes common to all C-like languages. All attributes referencing arguments should be additionally processed @@ -157,209 +244,230 @@ const struct attribute_spec c_common_attribute_table[] = /* { name, min_len, max_len, decl_req, type_req, fn_type_req, handler, affects_type_identity } */ { "packed", 0, 0, false, false, false, - handle_packed_attribute , false}, + handle_packed_attribute , false, + attr_aligned_exclusions }, { "nocommon", 0, 0, true, false, false, - handle_nocommon_attribute, false}, + handle_nocommon_attribute, false, + attr_common_exclusions }, { "common", 0, 0, true, false, false, - handle_common_attribute, false }, + handle_common_attribute, false, + attr_common_exclusions }, /* FIXME: logically, noreturn attributes should be listed as "false, true, true" and apply to function types. But implementing this would require all the places in the compiler that use TREE_THIS_VOLATILE on a decl to identify non-returning functions to be located and fixed to check the function type instead. */ { "noreturn", 0, 0, true, false, false, - handle_noreturn_attribute, false }, + handle_noreturn_attribute, false, + attr_noreturn_exclusions }, { "volatile", 0, 0, true, false, false, - handle_noreturn_attribute, false }, + handle_noreturn_attribute, false, NULL }, { "stack_protect", 0, 0, true, false, false, - handle_stack_protect_attribute, false }, + handle_stack_protect_attribute, false, NULL }, { "noinline", 0, 0, true, false, false, - handle_noinline_attribute, false }, + handle_noinline_attribute, false, + attr_noinline_exclusions }, { "noclone", 0, 0, true, false, false, - handle_noclone_attribute, false }, + handle_noclone_attribute, false, NULL }, { "no_icf", 0, 0, true, false, false, - handle_noicf_attribute, false }, + handle_noicf_attribute, false, NULL }, { "noipa", 0, 0, true, false, false, - handle_noipa_attribute, false }, + handle_noipa_attribute, false, NULL }, { "leaf", 0, 0, true, false, false, - handle_leaf_attribute, false }, + handle_leaf_attribute, false, NULL }, { "always_inline", 0, 0, true, false, false, - handle_always_inline_attribute, false }, + handle_always_inline_attribute, false, + attr_inline_exclusions }, { "gnu_inline", 0, 0, true, false, false, - handle_gnu_inline_attribute, false }, + handle_gnu_inline_attribute, false, + attr_inline_exclusions }, { "artificial", 0, 0, true, false, false, - handle_artificial_attribute, false }, + handle_artificial_attribute, false, NULL }, { "flatten", 0, 0, true, false, false, - handle_flatten_attribute, false }, + handle_flatten_attribute, false, NULL }, { "used", 0, 0, true, false, false, - handle_used_attribute, false }, + handle_used_attribute, false, NULL }, { "unused", 0, 0, false, false, false, - handle_unused_attribute, false }, + handle_unused_attribute, false, NULL }, { "externally_visible", 0, 0, true, false, false, - handle_externally_visible_attribute, false }, + handle_externally_visible_attribute, false, + NULL }, { "no_reorder", 0, 0, true, false, false, - handle_no_reorder_attribute, false }, + handle_no_reorder_attribute, false, NULL }, /* The same comments as for noreturn attributes apply to const ones. */ { "const", 0, 0, true, false, false, - handle_const_attribute, false }, + handle_const_attribute, false, + attr_const_pure_exclusions }, { "scalar_storage_order", 1, 1, false, false, false, - handle_scalar_storage_order_attribute, false }, + handle_scalar_storage_order_attribute, false, + NULL }, { "transparent_union", 0, 0, false, false, false, - handle_transparent_union_attribute, false }, + handle_transparent_union_attribute, false, NULL }, { "constructor", 0, 1, true, false, false, - handle_constructor_attribute, false }, + handle_constructor_attribute, false, NULL }, { "destructor", 0, 1, true, false, false, - handle_destructor_attribute, false }, + handle_destructor_attribute, false, NULL }, { "mode", 1, 1, false, true, false, - handle_mode_attribute, false }, + handle_mode_attribute, false, NULL }, { "section", 1, 1, true, false, false, - handle_section_attribute, false }, + handle_section_attribute, false, NULL }, { "aligned", 0, 1, false, false, false, - handle_aligned_attribute, false }, + handle_aligned_attribute, false, + attr_aligned_exclusions }, { "weak", 0, 0, true, false, false, - handle_weak_attribute, false }, + handle_weak_attribute, false, NULL }, { "noplt", 0, 0, true, false, false, - handle_noplt_attribute, false }, + handle_noplt_attribute, false, NULL }, { "ifunc", 1, 1, true, false, false, - handle_ifunc_attribute, false }, + handle_ifunc_attribute, false, NULL }, { "alias", 1, 1, true, false, false, - handle_alias_attribute, false }, + handle_alias_attribute, false, NULL }, { "weakref", 0, 1, true, false, false, - handle_weakref_attribute, false }, + handle_weakref_attribute, false, NULL }, { "no_instrument_function", 0, 0, true, false, false, handle_no_instrument_function_attribute, - false }, + false, NULL }, { "no_profile_instrument_function", 0, 0, true, false, false, handle_no_profile_instrument_function_attribute, - false }, + false, NULL }, { "malloc", 0, 0, true, false, false, - handle_malloc_attribute, false }, + handle_malloc_attribute, false, + attr_alloc_exclusions }, { "returns_twice", 0, 0, true, false, false, - handle_returns_twice_attribute, false }, + handle_returns_twice_attribute, false, + attr_returns_twice_exclusions }, { "no_stack_limit", 0, 0, true, false, false, - handle_no_limit_stack_attribute, false }, + handle_no_limit_stack_attribute, false, NULL }, { "pure", 0, 0, true, false, false, - handle_pure_attribute, false }, + handle_pure_attribute, false, + attr_const_pure_exclusions }, { "transaction_callable", 0, 0, false, true, false, - handle_tm_attribute, false }, + handle_tm_attribute, false, NULL }, { "transaction_unsafe", 0, 0, false, true, false, - handle_tm_attribute, true }, + handle_tm_attribute, true, NULL }, { "transaction_safe", 0, 0, false, true, false, - handle_tm_attribute, true }, + handle_tm_attribute, true, NULL }, { "transaction_safe_dynamic", 0, 0, true, false, false, - handle_tm_attribute, false }, + handle_tm_attribute, false, NULL }, { "transaction_may_cancel_outer", 0, 0, false, true, false, - handle_tm_attribute, false }, + handle_tm_attribute, false, NULL }, /* ??? These two attributes didn't make the transition from the Intel language document to the multi-vendor language document. */ { "transaction_pure", 0, 0, false, true, false, - handle_tm_attribute, false }, + handle_tm_attribute, false, NULL }, { "transaction_wrap", 1, 1, true, false, false, - handle_tm_wrap_attribute, false }, + handle_tm_wrap_attribute, false, NULL }, /* For internal use (marking of builtins) only. The name contains space to prevent its usage in source code. */ { "no vops", 0, 0, true, false, false, - handle_novops_attribute, false }, + handle_novops_attribute, false, NULL }, { "deprecated", 0, 1, false, false, false, - handle_deprecated_attribute, false }, + handle_deprecated_attribute, false, NULL }, { "vector_size", 1, 1, false, true, false, - handle_vector_size_attribute, true }, + handle_vector_size_attribute, true, NULL }, { "visibility", 1, 1, false, false, false, - handle_visibility_attribute, false }, + handle_visibility_attribute, false, NULL }, { "tls_model", 1, 1, true, false, false, - handle_tls_model_attribute, false }, + handle_tls_model_attribute, false, NULL }, { "nonnull", 0, -1, false, true, true, - handle_nonnull_attribute, false }, + handle_nonnull_attribute, false, NULL }, { "nothrow", 0, 0, true, false, false, - handle_nothrow_attribute, false }, - { "may_alias", 0, 0, false, true, false, NULL, false }, + handle_nothrow_attribute, false, NULL }, + { "may_alias", 0, 0, false, true, false, NULL, false, NULL }, { "cleanup", 1, 1, true, false, false, - handle_cleanup_attribute, false }, + handle_cleanup_attribute, false, NULL }, { "warn_unused_result", 0, 0, false, true, true, - handle_warn_unused_result_attribute, false }, + handle_warn_unused_result_attribute, false, + attr_warn_unused_result_exclusions }, { "sentinel", 0, 1, false, true, true, - handle_sentinel_attribute, false }, + handle_sentinel_attribute, false, NULL }, /* For internal use (marking of builtins) only. The name contains space to prevent its usage in source code. */ { "type generic", 0, 0, false, true, true, - handle_type_generic_attribute, false }, + handle_type_generic_attribute, false, NULL }, { "alloc_size", 1, 2, false, true, true, - handle_alloc_size_attribute, false }, + handle_alloc_size_attribute, false, + attr_alloc_exclusions }, { "cold", 0, 0, true, false, false, - handle_cold_attribute, false }, + handle_cold_attribute, false, + attr_cold_hot_exclusions }, { "hot", 0, 0, true, false, false, - handle_hot_attribute, false }, + handle_hot_attribute, false, + attr_cold_hot_exclusions }, { "no_address_safety_analysis", 0, 0, true, false, false, handle_no_address_safety_analysis_attribute, - false }, + false, NULL }, { "no_sanitize", 1, 1, true, false, false, handle_no_sanitize_attribute, - false }, + false, NULL }, { "no_sanitize_address", 0, 0, true, false, false, handle_no_sanitize_address_attribute, - false }, + false, NULL }, { "no_sanitize_thread", 0, 0, true, false, false, handle_no_sanitize_thread_attribute, - false }, + false, NULL }, { "no_sanitize_undefined", 0, 0, true, false, false, handle_no_sanitize_undefined_attribute, - false }, + false, NULL }, { "asan odr indicator", 0, 0, true, false, false, handle_asan_odr_indicator_attribute, - false }, + false, NULL }, { "warning", 1, 1, true, false, false, - handle_error_attribute, false }, + handle_error_attribute, false, NULL }, { "error", 1, 1, true, false, false, - handle_error_attribute, false }, + handle_error_attribute, false, NULL }, { "target", 1, -1, true, false, false, - handle_target_attribute, false }, + handle_target_attribute, false, NULL }, { "target_clones", 1, -1, true, false, false, - handle_target_clones_attribute, false }, + handle_target_clones_attribute, false, NULL }, { "optimize", 1, -1, true, false, false, - handle_optimize_attribute, false }, + handle_optimize_attribute, false, NULL }, /* For internal use only. The leading '*' both prevents its usage in source code and signals that it may be overridden by machine tables. */ { "*tm regparm", 0, 0, false, true, true, - ignore_attribute, false }, + ignore_attribute, false, NULL }, { "no_split_stack", 0, 0, true, false, false, - handle_no_split_stack_attribute, false }, + handle_no_split_stack_attribute, false, NULL }, /* For internal use (marking of builtins and runtime functions) only. The name contains space to prevent its usage in source code. */ { "fn spec", 1, 1, false, true, true, - handle_fnspec_attribute, false }, + handle_fnspec_attribute, false, NULL }, { "warn_unused", 0, 0, false, false, false, - handle_warn_unused_attribute, false }, + handle_warn_unused_attribute, false, NULL }, { "returns_nonnull", 0, 0, false, true, true, - handle_returns_nonnull_attribute, false }, + handle_returns_nonnull_attribute, false, NULL }, { "omp declare simd", 0, -1, true, false, false, - handle_omp_declare_simd_attribute, false }, + handle_omp_declare_simd_attribute, false, NULL }, { "cilk simd function", 0, -1, true, false, false, - handle_omp_declare_simd_attribute, false }, + handle_omp_declare_simd_attribute, false, NULL }, { "simd", 0, 1, true, false, false, - handle_simd_attribute, false }, + handle_simd_attribute, false, NULL }, { "omp declare target", 0, 0, true, false, false, - handle_omp_declare_target_attribute, false }, + handle_omp_declare_target_attribute, false, + NULL }, { "omp declare target link", 0, 0, true, false, false, - handle_omp_declare_target_attribute, false }, + handle_omp_declare_target_attribute, false, + NULL }, { "alloc_align", 1, 1, false, true, true, - handle_alloc_align_attribute, false }, + handle_alloc_align_attribute, false, + attr_alloc_exclusions }, { "assume_aligned", 1, 2, false, true, true, - handle_assume_aligned_attribute, false }, + handle_assume_aligned_attribute, false, NULL }, { "designated_init", 0, 0, false, true, false, - handle_designated_init_attribute, false }, + handle_designated_init_attribute, false, NULL }, { "bnd_variable_size", 0, 0, true, false, false, - handle_bnd_variable_size_attribute, false }, + handle_bnd_variable_size_attribute, false, NULL }, { "bnd_legacy", 0, 0, true, false, false, - handle_bnd_legacy, false }, + handle_bnd_legacy, false, NULL }, { "bnd_instrument", 0, 0, true, false, false, - handle_bnd_instrument, false }, + handle_bnd_instrument, false, NULL }, { "fallthrough", 0, 0, false, false, false, - handle_fallthrough_attribute, false }, + handle_fallthrough_attribute, false, NULL }, { "patchable_function_entry", 1, 2, true, false, false, handle_patchable_function_entry_attribute, - false }, - { NULL, 0, 0, false, false, false, NULL, false } + false, NULL }, + { NULL, 0, 0, false, false, false, NULL, false, NULL } }; /* Give the specifications for the format attributes, used by C and all @@ -374,10 +482,10 @@ const struct attribute_spec c_common_format_attribute_table[] = /* { name, min_len, max_len, decl_req, type_req, fn_type_req, handler, affects_type_identity } */ { "format", 3, 3, false, true, true, - handle_format_attribute, false }, + handle_format_attribute, false, NULL }, { "format_arg", 1, 1, false, true, true, - handle_format_arg_attribute, false }, - { NULL, 0, 0, false, false, false, NULL, false } + handle_format_arg_attribute, false, NULL }, + { NULL, 0, 0, false, false, false, NULL, false, NULL } }; /* Returns TRUE iff the attribute indicated by ATTR_ID takes a plain @@ -515,14 +623,7 @@ handle_hot_attribute (tree *node, tree name, tree ARG_UNUSED (args), if (TREE_CODE (*node) == FUNCTION_DECL || TREE_CODE (*node) == LABEL_DECL) { - if (lookup_attribute ("cold", DECL_ATTRIBUTES (*node)) != NULL) - { - warning (OPT_Wattributes, "%qE attribute ignored due to conflict " - "with attribute %qs", name, "cold"); - *no_add_attrs = true; - } - /* Most of the rest of the hot processing is done later with - lookup_attribute. */ + /* Attribute hot processing is done later with lookup_attribute. */ } else { @@ -543,14 +644,7 @@ handle_cold_attribute (tree *node, tree name, tree ARG_UNUSED (args), if (TREE_CODE (*node) == FUNCTION_DECL || TREE_CODE (*node) == LABEL_DECL) { - if (lookup_attribute ("hot", DECL_ATTRIBUTES (*node)) != NULL) - { - warning (OPT_Wattributes, "%qE attribute ignored due to conflict " - "with attribute %qs", name, "hot"); - *no_add_attrs = true; - } - /* Most of the rest of the cold processing is done later with - lookup_attribute. */ + /* Attribute cold processing is done later with lookup_attribute. */ } else { @@ -1064,7 +1158,7 @@ handle_no_reorder_attribute (tree *pnode, static tree handle_const_attribute (tree *node, tree name, tree ARG_UNUSED (args), - int ARG_UNUSED (flags), bool *no_add_attrs) + int flags, bool *no_add_attrs) { tree type = TREE_TYPE (*node); @@ -1085,6 +1179,14 @@ handle_const_attribute (tree *node, tree name, tree ARG_UNUSED (args), *no_add_attrs = true; } + /* void __builtin_unreachable(void) is const. Accept other such + built-ins but warn on user-defined functions that return void. */ + if (!(flags & ATTR_FLAG_BUILT_IN) + && TREE_CODE (*node) == FUNCTION_DECL + && VOID_TYPE_P (TREE_TYPE (type))) + warning (OPT_Wattributes, "%qE attribute on function " + "returning %<void%>", name); + return NULL_TREE; } @@ -1667,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 hasn't been + yet. */ + tree last_decl = node[1] ? node[1] : *node; if (args) { @@ -1693,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; + + /* The alignment of the current declaration and that of the last + pushed declaration, determined on demand below. */ + unsigned curalign = 0; + unsigned lastalign = 0; - if ((i = check_user_alignment (align_expr, true)) == -1 - || !check_cxx_fundamental_alignment_constraints (*node, i, flags)) + if (pow2align == -1 + || !check_cxx_fundamental_alignment_constraints (*node, pow2align, flags)) *no_add_attrs = true; else if (is_type) { @@ -1717,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) @@ -1726,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 @@ -1737,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; } @@ -2476,8 +2607,15 @@ handle_pure_attribute (tree *node, tree name, tree ARG_UNUSED (args), int ARG_UNUSED (flags), bool *no_add_attrs) { if (TREE_CODE (*node) == FUNCTION_DECL) - DECL_PURE_P (*node) = 1; - /* ??? TODO: Support types. */ + { + tree type = TREE_TYPE (*node); + if (VOID_TYPE_P (TREE_TYPE (type))) + warning (OPT_Wattributes, "%qE attribute on function " + "returning %<void%>", name); + + DECL_PURE_P (*node) = 1; + /* ??? TODO: Support types. */ + } else { warning (OPT_Wattributes, "%qE attribute ignored", name); diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c index e970ab2..f7c8eac 100644 --- a/gcc/c-family/c-warn.c +++ b/gcc/c-family/c-warn.c @@ -2143,36 +2143,19 @@ diagnose_mismatched_attributes (tree olddecl, tree newdecl) newdecl); /* Diagnose inline __attribute__ ((noinline)) which is silly. */ + const char *noinline = "noinline"; + if (DECL_DECLARED_INLINE_P (newdecl) && DECL_UNINLINABLE (olddecl) - && lookup_attribute ("noinline", DECL_ATTRIBUTES (olddecl))) + && lookup_attribute (noinline, DECL_ATTRIBUTES (olddecl))) warned |= warning (OPT_Wattributes, "inline declaration of %qD follows " - "declaration with attribute noinline", newdecl); + "declaration with attribute %qs", newdecl, noinline); else if (DECL_DECLARED_INLINE_P (olddecl) && DECL_UNINLINABLE (newdecl) && lookup_attribute ("noinline", DECL_ATTRIBUTES (newdecl))) warned |= warning (OPT_Wattributes, "declaration of %q+D with attribute " - "noinline follows inline declaration ", newdecl); - else if (lookup_attribute ("noinline", DECL_ATTRIBUTES (newdecl)) - && lookup_attribute ("always_inline", DECL_ATTRIBUTES (olddecl))) - warned |= warning (OPT_Wattributes, "declaration of %q+D with attribute " - "%qs follows declaration with attribute %qs", - newdecl, "noinline", "always_inline"); - else if (lookup_attribute ("always_inline", DECL_ATTRIBUTES (newdecl)) - && lookup_attribute ("noinline", DECL_ATTRIBUTES (olddecl))) - warned |= warning (OPT_Wattributes, "declaration of %q+D with attribute " - "%qs follows declaration with attribute %qs", - newdecl, "always_inline", "noinline"); - else if (lookup_attribute ("cold", DECL_ATTRIBUTES (newdecl)) - && lookup_attribute ("hot", DECL_ATTRIBUTES (olddecl))) - warned |= warning (OPT_Wattributes, "declaration of %q+D with attribute " - "%qs follows declaration with attribute %qs", - newdecl, "cold", "hot"); - else if (lookup_attribute ("hot", DECL_ATTRIBUTES (newdecl)) - && lookup_attribute ("cold", DECL_ATTRIBUTES (olddecl))) - warned |= warning (OPT_Wattributes, "declaration of %q+D with attribute " - "%qs follows declaration with attribute %qs", - newdecl, "hot", "cold"); + "%qs follows inline declaration ", newdecl, noinline); + return warned; } diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c index a54e121..5852c0d 100644 --- a/gcc/c/c-decl.c +++ b/gcc/c/c-decl.c @@ -4589,7 +4589,16 @@ c_decl_attributes (tree *node, tree attributes, int flags) attributes = tree_cons (get_identifier ("omp declare target"), NULL_TREE, attributes); } - return decl_attributes (node, attributes, flags); + + /* Look up the current declaration with all the attributes merged + so far so that attributes on the current declaration that's + about to be pushed that conflict with the former can be detected, + diagnosed, and rejected as appropriate. */ + tree last_decl = lookup_name (DECL_NAME (*node)); + if (!last_decl) + last_decl = lookup_name_in_scope (DECL_NAME (*node), external_scope); + + return decl_attributes (node, attributes, flags, last_decl); } diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index b253ccc..0db600a 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -2490,9 +2490,14 @@ are automatically detected and this attribute is ignored. @cindex @code{const} function attribute @cindex functions that have no side effects Many functions do not examine any values except their arguments, and -have no effects except the return value. Basically this is just slightly -more strict class than the @code{pure} attribute below, since function is not -allowed to read global memory. +have no effects except to return a value. Calls to such functions lend +themselves to optimization such as common subexpression elimination. +The @code{const} attribute imposes greater restrictions on a function's +definition than the similar @code{pure} attribute below because it prohibits +the function from reading global variables. Consequently, the presence of +the attribute on a function declarations allows GCC to emit more efficient +code for some calls to the function. Decorating the same function with +both the @code{const} and the @code{pure} attribute is diagnnosed. @cindex pointer arguments Note that a function that has pointer arguments and examines the data @@ -3144,7 +3149,7 @@ to prevent recursion. @cindex functions that have no side effects Many functions have no effects except the return value and their return value depends only on the parameters and/or global variables. -Such a function can be subject +Calls to such functions can be subject to common subexpression elimination and loop optimization just as an arithmetic operator would be. These functions should be declared with the attribute @code{pure}. For example, @@ -3162,6 +3167,11 @@ Interesting non-pure functions are functions with infinite loops or those depending on volatile memory or other system resource, that may change between two consecutive calls (such as @code{feof} in a multithreading environment). +The @code{pure} attribute imposes similar but looser restrictions on +a function's defintion than the @code{const} attribute: it allows the +function to read global variables. Decorating the same function with +both the @code{pure} and the @code{const} attribute is diagnosed. + @item returns_nonnull @cindex @code{returns_nonnull} function attribute The @code{returns_nonnull} attribute specifies that the function diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c index 30e476d..9af84a0 100644 --- a/gcc/selftest-run-tests.c +++ b/gcc/selftest-run-tests.c @@ -25,6 +25,8 @@ along with GCC; see the file COPYING3. If not see #include "target.h" #include "langhooks.h" #include "options.h" +#include "stringpool.h" +#include "attribs.h" /* This function needed to be split out from selftest.c as it references tests from the whole source tree, and so is within @@ -83,6 +85,7 @@ selftest::run_tests () spellcheck_c_tests (); spellcheck_tree_c_tests (); tree_cfg_c_tests (); + attribute_c_tests (); /* This one relies on most of the above. */ function_tests_c_tests (); diff --git a/gcc/selftest.h b/gcc/selftest.h index 0572fef..608f4ee 100644 --- a/gcc/selftest.h +++ b/gcc/selftest.h @@ -170,6 +170,7 @@ extern const char *path_to_selftest_files; /* Declarations for specific families of tests (by source file), in alphabetical order. */ +extern void attribute_c_tests (); extern void bitmap_c_tests (); extern void diagnostic_c_tests (); extern void diagnostic_show_locus_c_tests (); diff --git a/gcc/testsuite/c-c++-common/attributes-3.c b/gcc/testsuite/c-c++-common/attributes-3.c index 821278c..9d3a61c 100644 --- a/gcc/testsuite/c-c++-common/attributes-3.c +++ b/gcc/testsuite/c-c++-common/attributes-3.c @@ -12,16 +12,16 @@ extern __attribute__((noinline)) int fn1 (void); /* { dg-message "previous decla extern inline int fn1 (void); /* { dg-warning "inline declaration of" } */ extern inline int fn2 (void); /* { dg-message "previous declaration" } */ -extern __attribute__((noinline)) int fn2 (void); /* { dg-warning "attribute noinline follows inline declaration" } */ +extern __attribute__((noinline)) int fn2 (void); /* { dg-warning "attribute .noinline. follows inline declaration" } */ extern __attribute__((always_inline)) int fn3 (void); /* { dg-message "previous declaration" } */ -extern __attribute__((noinline)) int fn3 (void); /* { dg-warning "attribute .noinline. follows declaration with attribute .always_inline." } */ +extern __attribute__((noinline)) int fn3 (void); /* { dg-warning "ignoring attribute .noinline. because it conflicts with attribute .always_inline." } */ extern __attribute__((noinline)) int fn4 (void); /* { dg-message "previous declaration" } */ -extern __attribute__((always_inline)) int fn4 (void); /* { dg-warning "attribute .always_inline. follows declaration with attribute .noinline." } */ +extern __attribute__((always_inline)) int fn4 (void); /* { dg-warning "ignoring attribute .always_inline. because it conflicts with attribute .noinline." } */ extern __attribute__((hot)) int fn5 (void); /* { dg-message "previous declaration" } */ -extern __attribute__((cold)) int fn5 (void); /* { dg-warning "attribute .cold. follows declaration with attribute .hot." } */ +extern __attribute__((cold)) int fn5 (void); /* { dg-warning "ignoring attribute .cold. because it conflicts with attribute .hot." } */ extern __attribute__((cold)) int fn6 (void); /* { dg-message "previous declaration" } */ -extern __attribute__((hot)) int fn6 (void); /* { dg-warning "attribute .hot. follows declaration with attribute .cold." } */ +extern __attribute__((hot)) int fn6 (void); /* { dg-warning "ignoring attribute .hot. because it conflicts with attribute .cold." } */ diff --git a/gcc/testsuite/gcc.dg/attr-noinline.c b/gcc/testsuite/gcc.dg/attr-noinline.c index c2a5b1d..13cc660 100644 --- a/gcc/testsuite/gcc.dg/attr-noinline.c +++ b/gcc/testsuite/gcc.dg/attr-noinline.c @@ -17,7 +17,7 @@ static void function_declaration_both_after(void) {t();} static void function_declaration_noinline_before(void) __attribute__((__noinline__)); /* { dg-message "note: previous declaration" } */ -static inline void function_declaration_noinline_before(void) {t();} /* { dg-warning "follows declaration with attribute noinline" } */ +static inline void function_declaration_noinline_before(void) {t();} /* { dg-warning "follows declaration with attribute .noinline." } */ static inline void function_declaration_noinline_after(void) {t();} /* { dg-message "note: previous definition" } */ @@ -41,7 +41,7 @@ static void function_declaration_inline_noinline_after(void) __attribute__((__no static void function_declaration_noinline_inline_before(void) __attribute__((__noinline__)); /* { dg-message "note: previous declaration" } */ -static inline void function_declaration_noinline_inline_before(void); /* { dg-warning "follows declaration with attribute noinline" } */ +static inline void function_declaration_noinline_inline_before(void); /* { dg-warning "follows declaration with attribute .noinline." } */ static void function_declaration_noinline_inline_before(void) {t();} diff --git a/gcc/testsuite/gcc.dg/pr44964.c b/gcc/testsuite/gcc.dg/pr44964.c index 1df1bde..6c252ee 100644 --- a/gcc/testsuite/gcc.dg/pr44964.c +++ b/gcc/testsuite/gcc.dg/pr44964.c @@ -2,8 +2,9 @@ /* { dg-options "-fkeep-inline-functions -O" } */ static inline __attribute__ ((const)) -void baz (int i) +int baz (int i) { + return i; } static __attribute__ ((always_inline)) diff --git a/gcc/testsuite/gcc.dg/torture/pr42363.c b/gcc/testsuite/gcc.dg/torture/pr42363.c index 9c9da13..ad0eac8 100644 --- a/gcc/testsuite/gcc.dg/torture/pr42363.c +++ b/gcc/testsuite/gcc.dg/torture/pr42363.c @@ -46,16 +46,18 @@ int bizr (void) return i + 1; } -/* This might be regarded as pure and folded, rather than inlined. - It's pure evil. */ +/* This might be regarded as pure and folded, rather than inlined, + but because it's pure evil it's diagnosed and the noreturn attribute + is dropped. The const attribute is dropped as well because it's + mutually exclusive with pure. */ static int __attribute__ ((pure, const, noreturn)) -barf (void) -{ +barf (void) { + /* { dg-warning "ignoring attribute .const." "const" { target *-*-* } .-1 } */ + /* { dg-warning "ignoring attribute .noreturn." "noreturn" { target *-*-* } .-2 } */ } /* { dg-warning "does return" } */ static int __attribute__ ((pure, const)) -bark (void) -{ +bark (void) { /* { dg-warning "ignoring attribute .const." } */ barf (); } diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c index 146b76c..b8c5654 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c @@ -113,17 +113,18 @@ int test9 (int *intarr) int test99 (int *intarr) { - extern int foo9 (int) __attribute__ ((pure)); + extern int foo99 (int) __attribute__ ((pure)); int h, v; g9 = 9; - h = foo9 (g9); + h = foo99 (g9); v = g9; if (v != 9) link_error (); return g9; } -extern int foo99 (int); +/* foo9 is const because of its declaration in test9. */ +extern int foo9 (int); int test999 (int *arr) { @@ -134,10 +135,12 @@ int test999 (int *arr) v1 = g9; if (v1 != 9) link_error (); - l = foo99 (l); + l = foo9 (l); return v1 + l; } +/* foo99 is pure because of its declaration in test99. */ +extern int foo9 (int); int test9999 (void) {
On 09/20/2017 12:04 PM, Martin Sebor wrote: > On 09/19/2017 03:00 PM, Joseph Myers wrote: >> On Tue, 19 Sep 2017, Martin Sebor wrote: >> >>>> In general, the data structures where you need to ensure manually that if >>>> >>>> attribute A is listed in EXCL for B, then attribute B is also listed in >>>> EXCL for A, seem concerning.  I'd expect either data structures that make >>>> >>>> such asymmetry impossible, or a self-test that verifies that the tables in >>>> >>>> use are in fact symmetric (unless there is some reason the symmetry is not >>>> >>>> in fact required and symmetric diagnostics still result from asymmetric >>>> tables - in which case the various combinations and orderings of >>>> gnu_inline and noinline definitely need tests to show that the diagnostics >>>> >>>> work). >>> >>> If I understand correctly what you're concerned about then I don't >>> think there are any such cases in the updated version of the patch. >> >> I don't see how you ensure that it's not possible to have such asymmetry. >> My point wasn't so much "there was a bug in the previous patch version" as >> >> "the choice of data structures for defining such exclusions is prone to >> such bugs".  Which can be addressed either by using different data >> structures (e.g. listing incompatible pairs in a single array) or by a >> self-test to verify symmetry so a compiler with asymmetry doesn't build. > > Okay, that's a useful thing to add.  It exposed a couple of missing > attribute exclusions that I had overlooked.  Thanks for the suggestion! > Attached is an incremental diff with just these changes to make review > easier and an updated patch. > > As an aside, there are a number of other possible logic errors in > the attribute specifications that could be detected by self-tests. > The one I ran into is misspelled attribute names.  The added test > detects misspelled names in exclusions, but not in the main specs. > > Martin > > gcc-81544-1-inc.diff > > > gcc-81544-1.diff > > > PR c/81544 - attribute noreturn and warn_unused_result on the same function accepted > > gcc/c/ChangeLog: > > PR c/81544 > * c-decl.c (c_decl_attributes): Look up existing declaration and > pass it to decl_attributes. > > gcc/c-family/ChangeLog: > > PR c/81544 > * c-attribs.c (attr_aligned_exclusions): New array. > (attr_alloc_exclusions, attr_cold_hot_exclusions): Same. > (attr_common_exclusions, attr_const_pure_exclusions): Same. > (attr_gnu_inline_exclusions, attr_inline_exclusions): Same. > (attr_noreturn_exclusions, attr_returns_twice_exclusions): Same. > (attr_warn_unused_result_exclusions): Same. > (handle_hot_attribute, handle_cold_attribute): Simplify. > (handle_const_attribute): Warn on function returning void. > (handle_pure_attribute): Same. > * c-warn.c (diagnose_mismatched_attributes): Simplify. > > gcc/ChangeLog: > > PR c/81544 > * attribs.c (empty_attribute_table): Initialize new member of > struct attribute_spec. > (decl_attributes): Add argument. Handle mutually exclusive > combinations of attributes. > * attribs.h (decl_attributes): Add default argument. > * selftest.h (attribute_c_tests): Declare. > * selftest-run-tests.c (selftest::run_tests): Call attribute_c_tests. > * tree-core.h (attribute_spec::exclusions, exclude): New type and > member. > * doc/extend.texi (Common Function Attributes): Update const and pure. > > gcc/testsuite/ChangeLog: > > PR c/81544 > * c-c++-common/Wattributes-2.c: New test. > * c-c++-common/Wattributes.c: New test. > * c-c++-common/attributes-3.c: Adjust. > * gcc.dg/attr-noinline.c: Adjust. > * gcc.dg/pr44964.c: Same. > * gcc.dg/torture/pr42363.c: Same. > * gcc.dg/tree-ssa/ssa-ccp-2.c: Same. OK. jeff
On 10/03/2017 12:23 AM, Jeff Law wrote: > On 09/20/2017 12:04 PM, Martin Sebor wrote: >> On 09/19/2017 03:00 PM, Joseph Myers wrote: >>> On Tue, 19 Sep 2017, Martin Sebor wrote: >>> >>>>> In general, the data structures where you need to ensure manually that if >>>>> >>>>> attribute A is listed in EXCL for B, then attribute B is also listed in >>>>> EXCL for A, seem concerning.  I'd expect either data structures that make >>>>> >>>>> such asymmetry impossible, or a self-test that verifies that the tables in >>>>> >>>>> use are in fact symmetric (unless there is some reason the symmetry is not >>>>> >>>>> in fact required and symmetric diagnostics still result from asymmetric >>>>> tables - in which case the various combinations and orderings of >>>>> gnu_inline and noinline definitely need tests to show that the diagnostics >>>>> >>>>> work). >>>> >>>> If I understand correctly what you're concerned about then I don't >>>> think there are any such cases in the updated version of the patch. >>> >>> I don't see how you ensure that it's not possible to have such asymmetry. >>> My point wasn't so much "there was a bug in the previous patch version" as >>> >>> "the choice of data structures for defining such exclusions is prone to >>> such bugs".  Which can be addressed either by using different data >>> structures (e.g. listing incompatible pairs in a single array) or by a >>> self-test to verify symmetry so a compiler with asymmetry doesn't build. >> >> Okay, that's a useful thing to add.  It exposed a couple of missing >> attribute exclusions that I had overlooked.  Thanks for the suggestion! >> Attached is an incremental diff with just these changes to make review >> easier and an updated patch. >> >> As an aside, there are a number of other possible logic errors in >> the attribute specifications that could be detected by self-tests. >> The one I ran into is misspelled attribute names.  The added test >> detects misspelled names in exclusions, but not in the main specs. >> >> Martin >> >> gcc-81544-1-inc.diff >> >> > > >> gcc-81544-1.diff >> >> >> PR c/81544 - attribute noreturn and warn_unused_result on the same function accepted >> >> gcc/c/ChangeLog: >> >> PR c/81544 >> * c-decl.c (c_decl_attributes): Look up existing declaration and >> pass it to decl_attributes. >> >> gcc/c-family/ChangeLog: >> >> PR c/81544 >> * c-attribs.c (attr_aligned_exclusions): New array. >> (attr_alloc_exclusions, attr_cold_hot_exclusions): Same. >> (attr_common_exclusions, attr_const_pure_exclusions): Same. >> (attr_gnu_inline_exclusions, attr_inline_exclusions): Same. >> (attr_noreturn_exclusions, attr_returns_twice_exclusions): Same. >> (attr_warn_unused_result_exclusions): Same. >> (handle_hot_attribute, handle_cold_attribute): Simplify. >> (handle_const_attribute): Warn on function returning void. >> (handle_pure_attribute): Same. >> * c-warn.c (diagnose_mismatched_attributes): Simplify. >> >> gcc/ChangeLog: >> >> PR c/81544 >> * attribs.c (empty_attribute_table): Initialize new member of >> struct attribute_spec. >> (decl_attributes): Add argument. Handle mutually exclusive >> combinations of attributes. >> * attribs.h (decl_attributes): Add default argument. >> * selftest.h (attribute_c_tests): Declare. >> * selftest-run-tests.c (selftest::run_tests): Call attribute_c_tests. >> * tree-core.h (attribute_spec::exclusions, exclude): New type and >> member. >> * doc/extend.texi (Common Function Attributes): Update const and pure. >> >> gcc/testsuite/ChangeLog: >> >> PR c/81544 >> * c-c++-common/Wattributes-2.c: New test. >> * c-c++-common/Wattributes.c: New test. >> * c-c++-common/attributes-3.c: Adjust. >> * gcc.dg/attr-noinline.c: Adjust. >> * gcc.dg/pr44964.c: Same. >> * gcc.dg/torture/pr42363.c: Same. >> * gcc.dg/tree-ssa/ssa-ccp-2.c: Same. > OK. > jeff > On targets enforcing a function alignment bigger than 4 bytes this triggers an error instead: +inline int ATTR ((aligned (4))) +finline_hot_noret_align (int); /* { dg-warning "ignoring attribute .aligned \\(4\\). because it conflicts with attribute .aligned \\(8\\)." } */ gcc/gcc/testsuite/c-c++-common/Wattributes.c:404:1: error: alignment for 'finline_hot_noret_align' must be at least 8^M
On 04/04/2018 06:16 PM, Andreas Krebbel wrote: > On 10/03/2017 12:23 AM, Jeff Law wrote: >> On 09/20/2017 12:04 PM, Martin Sebor wrote: >>> On 09/19/2017 03:00 PM, Joseph Myers wrote: >>>> On Tue, 19 Sep 2017, Martin Sebor wrote: >>>> >>>>>> In general, the data structures where you need to ensure manually that if >>>>>> >>>>>> attribute A is listed in EXCL for B, then attribute B is also listed in >>>>>> EXCL for A, seem concerning.  I'd expect either data structures that make >>>>>> >>>>>> such asymmetry impossible, or a self-test that verifies that the tables in >>>>>> >>>>>> use are in fact symmetric (unless there is some reason the symmetry is not >>>>>> >>>>>> in fact required and symmetric diagnostics still result from asymmetric >>>>>> tables - in which case the various combinations and orderings of >>>>>> gnu_inline and noinline definitely need tests to show that the diagnostics >>>>>> >>>>>> work). >>>>> >>>>> If I understand correctly what you're concerned about then I don't >>>>> think there are any such cases in the updated version of the patch. >>>> >>>> I don't see how you ensure that it's not possible to have such asymmetry. >>>> My point wasn't so much "there was a bug in the previous patch version" as >>>> >>>> "the choice of data structures for defining such exclusions is prone to >>>> such bugs".  Which can be addressed either by using different data >>>> structures (e.g. listing incompatible pairs in a single array) or by a >>>> self-test to verify symmetry so a compiler with asymmetry doesn't build. >>> >>> Okay, that's a useful thing to add.  It exposed a couple of missing >>> attribute exclusions that I had overlooked.  Thanks for the suggestion! >>> Attached is an incremental diff with just these changes to make review >>> easier and an updated patch. >>> >>> As an aside, there are a number of other possible logic errors in >>> the attribute specifications that could be detected by self-tests. >>> The one I ran into is misspelled attribute names.  The added test >>> detects misspelled names in exclusions, but not in the main specs. >>> >>> Martin >>> >>> gcc-81544-1-inc.diff >>> >>> >> >> >>> gcc-81544-1.diff >>> >>> >>> PR c/81544 - attribute noreturn and warn_unused_result on the same function accepted >>> >>> gcc/c/ChangeLog: >>> >>> PR c/81544 >>> * c-decl.c (c_decl_attributes): Look up existing declaration and >>> pass it to decl_attributes. >>> >>> gcc/c-family/ChangeLog: >>> >>> PR c/81544 >>> * c-attribs.c (attr_aligned_exclusions): New array. >>> (attr_alloc_exclusions, attr_cold_hot_exclusions): Same. >>> (attr_common_exclusions, attr_const_pure_exclusions): Same. >>> (attr_gnu_inline_exclusions, attr_inline_exclusions): Same. >>> (attr_noreturn_exclusions, attr_returns_twice_exclusions): Same. >>> (attr_warn_unused_result_exclusions): Same. >>> (handle_hot_attribute, handle_cold_attribute): Simplify. >>> (handle_const_attribute): Warn on function returning void. >>> (handle_pure_attribute): Same. >>> * c-warn.c (diagnose_mismatched_attributes): Simplify. >>> >>> gcc/ChangeLog: >>> >>> PR c/81544 >>> * attribs.c (empty_attribute_table): Initialize new member of >>> struct attribute_spec. >>> (decl_attributes): Add argument. Handle mutually exclusive >>> combinations of attributes. >>> * attribs.h (decl_attributes): Add default argument. >>> * selftest.h (attribute_c_tests): Declare. >>> * selftest-run-tests.c (selftest::run_tests): Call attribute_c_tests. >>> * tree-core.h (attribute_spec::exclusions, exclude): New type and >>> member. >>> * doc/extend.texi (Common Function Attributes): Update const and pure. >>> >>> gcc/testsuite/ChangeLog: >>> >>> PR c/81544 >>> * c-c++-common/Wattributes-2.c: New test. >>> * c-c++-common/Wattributes.c: New test. >>> * c-c++-common/attributes-3.c: Adjust. >>> * gcc.dg/attr-noinline.c: Adjust. >>> * gcc.dg/pr44964.c: Same. >>> * gcc.dg/torture/pr42363.c: Same. >>> * gcc.dg/tree-ssa/ssa-ccp-2.c: Same. >> OK. >> jeff >> > > On targets enforcing a function alignment bigger than 4 bytes this triggers an error instead: > > +inline int ATTR ((aligned (4))) > +finline_hot_noret_align (int); /* { dg-warning "ignoring attribute .aligned \\(4\\). because it > conflicts with attribute .aligned \\(8\\)." } */ > > gcc/gcc/testsuite/c-c++-common/Wattributes.c:404:1: error: alignment for 'finline_hot_noret_align' > must be at least 8^M > diff --git a/gcc/testsuite/c-c++-common/Wattributes.c b/gcc/testsuite/c-c++-common/Wattributes.c index 902bcb61c30..a260d018dcf 100644 --- a/gcc/testsuite/c-c++-common/Wattributes.c +++ b/gcc/testsuite/c-c++-common/Wattributes.c @@ -401,7 +401,8 @@ inline int ATTR ((warn_unused_result)) finline_hot_noret_align (int); /* { dg-warning "ignoring attribute .warn_unused_result. because it conflicts with attribute .noreturn." } */ inline int ATTR ((aligned (4))) -finline_hot_noret_align (int); /* { dg-warning "ignoring attribute .aligned \\(4\\). because it conflicts with attribute .aligned \\(8\\)." } */ + finline_hot_noret_align (int); /* { dg-warning "ignoring attribute .aligned \\(4\\). because it conflicts with attribute .aligned \\(8\\)." "" { target { ! s390*-*-* } } } */ +/* { dg-error "alignment for 'finline_hot_noret_align' must be at least 8" "" { target s390*-*-* } .-1 } */ inline int ATTR ((aligned (8))) finline_hot_noret_align (int); OK? -Andreas-
On 04/04/2018 10:51 AM, Andreas Krebbel wrote: > On 04/04/2018 06:16 PM, Andreas Krebbel wrote: >> On 10/03/2017 12:23 AM, Jeff Law wrote: >>> On 09/20/2017 12:04 PM, Martin Sebor wrote: >>>> On 09/19/2017 03:00 PM, Joseph Myers wrote: >>>>> On Tue, 19 Sep 2017, Martin Sebor wrote: >>>>> >>>>>>> In general, the data structures where you need to ensure manually that if >>>>>>> >>>>>>> attribute A is listed in EXCL for B, then attribute B is also listed in >>>>>>> EXCL for A, seem concerning. I'd expect either data structures that make >>>>>>> >>>>>>> such asymmetry impossible, or a self-test that verifies that the tables in >>>>>>> >>>>>>> use are in fact symmetric (unless there is some reason the symmetry is not >>>>>>> >>>>>>> in fact required and symmetric diagnostics still result from asymmetric >>>>>>> tables - in which case the various combinations and orderings of >>>>>>> gnu_inline and noinline definitely need tests to show that the diagnostics >>>>>>> >>>>>>> work). >>>>>> >>>>>> If I understand correctly what you're concerned about then I don't >>>>>> think there are any such cases in the updated version of the patch. >>>>> >>>>> I don't see how you ensure that it's not possible to have such asymmetry. >>>>> My point wasn't so much "there was a bug in the previous patch version" as >>>>> >>>>> "the choice of data structures for defining such exclusions is prone to >>>>> such bugs". Which can be addressed either by using different data >>>>> structures (e.g. listing incompatible pairs in a single array) or by a >>>>> self-test to verify symmetry so a compiler with asymmetry doesn't build. >>>> >>>> Okay, that's a useful thing to add. It exposed a couple of missing >>>> attribute exclusions that I had overlooked. Thanks for the suggestion! >>>> Attached is an incremental diff with just these changes to make review >>>> easier and an updated patch. >>>> >>>> As an aside, there are a number of other possible logic errors in >>>> the attribute specifications that could be detected by self-tests. >>>> The one I ran into is misspelled attribute names. The added test >>>> detects misspelled names in exclusions, but not in the main specs. >>>> >>>> Martin >>>> >>>> gcc-81544-1-inc.diff >>>> >>>> >>> >>> >>>> gcc-81544-1.diff >>>> >>>> >>>> PR c/81544 - attribute noreturn and warn_unused_result on the same function accepted >>>> >>>> gcc/c/ChangeLog: >>>> >>>> PR c/81544 >>>> * c-decl.c (c_decl_attributes): Look up existing declaration and >>>> pass it to decl_attributes. >>>> >>>> gcc/c-family/ChangeLog: >>>> >>>> PR c/81544 >>>> * c-attribs.c (attr_aligned_exclusions): New array. >>>> (attr_alloc_exclusions, attr_cold_hot_exclusions): Same. >>>> (attr_common_exclusions, attr_const_pure_exclusions): Same. >>>> (attr_gnu_inline_exclusions, attr_inline_exclusions): Same. >>>> (attr_noreturn_exclusions, attr_returns_twice_exclusions): Same. >>>> (attr_warn_unused_result_exclusions): Same. >>>> (handle_hot_attribute, handle_cold_attribute): Simplify. >>>> (handle_const_attribute): Warn on function returning void. >>>> (handle_pure_attribute): Same. >>>> * c-warn.c (diagnose_mismatched_attributes): Simplify. >>>> >>>> gcc/ChangeLog: >>>> >>>> PR c/81544 >>>> * attribs.c (empty_attribute_table): Initialize new member of >>>> struct attribute_spec. >>>> (decl_attributes): Add argument. Handle mutually exclusive >>>> combinations of attributes. >>>> * attribs.h (decl_attributes): Add default argument. >>>> * selftest.h (attribute_c_tests): Declare. >>>> * selftest-run-tests.c (selftest::run_tests): Call attribute_c_tests. >>>> * tree-core.h (attribute_spec::exclusions, exclude): New type and >>>> member. >>>> * doc/extend.texi (Common Function Attributes): Update const and pure. >>>> >>>> gcc/testsuite/ChangeLog: >>>> >>>> PR c/81544 >>>> * c-c++-common/Wattributes-2.c: New test. >>>> * c-c++-common/Wattributes.c: New test. >>>> * c-c++-common/attributes-3.c: Adjust. >>>> * gcc.dg/attr-noinline.c: Adjust. >>>> * gcc.dg/pr44964.c: Same. >>>> * gcc.dg/torture/pr42363.c: Same. >>>> * gcc.dg/tree-ssa/ssa-ccp-2.c: Same. >>> OK. >>> jeff >>> >> >> On targets enforcing a function alignment bigger than 4 bytes this triggers an error instead: >> >> +inline int ATTR ((aligned (4))) >> +finline_hot_noret_align (int); /* { dg-warning "ignoring attribute .aligned \\(4\\). because it >> conflicts with attribute .aligned \\(8\\)." } */ >> >> gcc/gcc/testsuite/c-c++-common/Wattributes.c:404:1: error: alignment for 'finline_hot_noret_align' >> must be at least 8^M >> > > diff --git a/gcc/testsuite/c-c++-common/Wattributes.c b/gcc/testsuite/c-c++-common/Wattributes.c > index 902bcb61c30..a260d018dcf 100644 > --- a/gcc/testsuite/c-c++-common/Wattributes.c > +++ b/gcc/testsuite/c-c++-common/Wattributes.c > @@ -401,7 +401,8 @@ inline int ATTR ((warn_unused_result)) > finline_hot_noret_align (int); /* { dg-warning "ignoring attribute .warn_unused_result. because it > conflicts with attribute .noreturn." } */ > > inline int ATTR ((aligned (4))) > -finline_hot_noret_align (int); /* { dg-warning "ignoring attribute .aligned \\(4\\). because it > conflicts with attribute .aligned \\(8\\)." } */ > + finline_hot_noret_align (int); /* { dg-warning "ignoring attribute .aligned \\(4\\). because it > conflicts with attribute .aligned \\(8\\)." "" { target { ! s390*-*-* } } } */ > +/* { dg-error "alignment for 'finline_hot_noret_align' must be at least 8" "" { target s390*-*-* } > .-1 } */ > > inline int ATTR ((aligned (8))) > finline_hot_noret_align (int); > > OK? It looks fine to me. I can't approve patches but this one looks trivially safe so I think you can commit it without a formal approval. Thanks for fixing the failure! Martin
On Wed, Apr 04, 2018 at 06:51:00PM +0200, Andreas Krebbel wrote: > > On targets enforcing a function alignment bigger than 4 bytes this triggers an error instead: > > > > +inline int ATTR ((aligned (4))) > > +finline_hot_noret_align (int); /* { dg-warning "ignoring attribute .aligned \\(4\\). because it > > conflicts with attribute .aligned \\(8\\)." } */ > > > > gcc/gcc/testsuite/c-c++-common/Wattributes.c:404:1: error: alignment for 'finline_hot_noret_align' > > must be at least 8^M > > > > diff --git a/gcc/testsuite/c-c++-common/Wattributes.c b/gcc/testsuite/c-c++-common/Wattributes.c > index 902bcb61c30..a260d018dcf 100644 > --- a/gcc/testsuite/c-c++-common/Wattributes.c > +++ b/gcc/testsuite/c-c++-common/Wattributes.c > @@ -401,7 +401,8 @@ inline int ATTR ((warn_unused_result)) > finline_hot_noret_align (int); /* { dg-warning "ignoring attribute .warn_unused_result. because it > conflicts with attribute .noreturn." } */ > > inline int ATTR ((aligned (4))) > -finline_hot_noret_align (int); /* { dg-warning "ignoring attribute .aligned \\(4\\). because it > conflicts with attribute .aligned \\(8\\)." } */ > + finline_hot_noret_align (int); /* { dg-warning "ignoring attribute .aligned \\(4\\). because it > conflicts with attribute .aligned \\(8\\)." "" { target { ! s390*-*-* } } } */ > +/* { dg-error "alignment for 'finline_hot_noret_align' must be at least 8" "" { target s390*-*-* } > .-1 } */ > > inline int ATTR ((aligned (8))) > finline_hot_noret_align (int); > > OK? Wouldn't it be better to just use aligned (8) and aligned (16) instead of aligned (4) and aligned (8)? Jakub
On 04/04/2018 07:34 PM, Jakub Jelinek wrote: > On Wed, Apr 04, 2018 at 06:51:00PM +0200, Andreas Krebbel wrote: >>> On targets enforcing a function alignment bigger than 4 bytes this triggers an error instead: >>> >>> +inline int ATTR ((aligned (4))) >>> +finline_hot_noret_align (int); /* { dg-warning "ignoring attribute .aligned \\(4\\). because it >>> conflicts with attribute .aligned \\(8\\)." } */ >>> >>> gcc/gcc/testsuite/c-c++-common/Wattributes.c:404:1: error: alignment for 'finline_hot_noret_align' >>> must be at least 8^M >>> >> >> diff --git a/gcc/testsuite/c-c++-common/Wattributes.c b/gcc/testsuite/c-c++-common/Wattributes.c >> index 902bcb61c30..a260d018dcf 100644 >> --- a/gcc/testsuite/c-c++-common/Wattributes.c >> +++ b/gcc/testsuite/c-c++-common/Wattributes.c >> @@ -401,7 +401,8 @@ inline int ATTR ((warn_unused_result)) >> finline_hot_noret_align (int); /* { dg-warning "ignoring attribute .warn_unused_result. because it >> conflicts with attribute .noreturn." } */ >> >> inline int ATTR ((aligned (4))) >> -finline_hot_noret_align (int); /* { dg-warning "ignoring attribute .aligned \\(4\\). because it >> conflicts with attribute .aligned \\(8\\)." } */ >> + finline_hot_noret_align (int); /* { dg-warning "ignoring attribute .aligned \\(4\\). because it >> conflicts with attribute .aligned \\(8\\)." "" { target { ! s390*-*-* } } } */ >> +/* { dg-error "alignment for 'finline_hot_noret_align' must be at least 8" "" { target s390*-*-* } >> .-1 } */ >> >> inline int ATTR ((aligned (8))) >> finline_hot_noret_align (int); >> >> OK? > > Wouldn't it be better to just use aligned (8) and aligned (16) instead of > aligned (4) and aligned (8)? aligned (8) does not trigger the warning anymore. Neither on s390 nor on x86. I don't think there is a way to ever see this warning on s390. Values >= 8 would be ok (no warning, no error) and values < 8 would trigger the error but not the warning. -Andreas- > > Jakub >
On Thu, Apr 05, 2018 at 09:01:02AM +0200, Andreas Krebbel wrote: > > Wouldn't it be better to just use aligned (8) and aligned (16) instead of > > aligned (4) and aligned (8)? > > aligned (8) does not trigger the warning anymore. Neither on s390 nor on x86. I don't think there > is a way to ever see this warning on s390. Values >= 8 would be ok (no warning, no error) and > values < 8 would trigger the error but not the warning. Your patch is ok then. Jakub
PR c/81544 - attribute noreturn and warn_unused_result on the same function accepted gcc/c/ChangeLog: PR c/81544 * c-decl.c (c_decl_attributes): Look up existing declaration and pass it to decl_attributes. gcc/c-family/ChangeLog: PR c/81544 * c-attribs.c (attr_aligned_exclusions): New array. (attr_alloc_exclusions, attr_cold_hot_exclusions): Same. (attr_common_exclusions, attr_const_pure_exclusions): Same. (attr_gnu_inline_exclusions, attr_inline_exclusions): Same. (attr_noreturn_exclusions, attr_returns_twice_exclusions): Same. (attr_warn_unused_result_exclusions): Same. (handle_hot_attribute, handle_cold_attribute): Simplify. (handle_const_attribute): Warn on function returning void. (handle_pure_attribute): Same. * c-warn.c (diagnose_mismatched_attributes): Simplify. gcc/testsuite/ChangeLog: PR c/81544 * c-c++-common/Wattributes-2.c: New test. * c-c++-common/Wattributes.c: New test. * c-c++-common/attributes-3.c: Adjust. * gcc.dg/attr-noinline.c: Adjust. * gcc.dg/pr44964.c: Same. * gcc.dg/torture/pr42363.c: Same. * gcc.dg/tree-ssa/ssa-ccp-2.c: Same. gcc/ChangeLog: PR c/81544 * attribs.c (empty_attribute_table): Initialize new member of struct attribute_spec. (decl_attributes): Add argument. Handle mutually exclusive combinations of attributes. * attribs.h (decl_attributes): Add default argument. * tree-core.h (attribute_spec::exclusions, exclude): New type and member. * doc/extend.texi (Common Function Attributes): Update const and pure. diff --git a/gcc/attribs.c b/gcc/attribs.c index 05fa8ef..8be5d9d 100644 --- a/gcc/attribs.c +++ b/gcc/attribs.c @@ -94,7 +94,7 @@ static bool attributes_initialized = false; static const struct attribute_spec empty_attribute_table[] = { - { NULL, 0, 0, false, false, false, NULL, false } + { NULL, 0, 0, false, false, false, NULL, false, NULL } }; /* Return base name of the attribute. Ie '__attr__' is turned into 'attr'. @@ -343,6 +343,97 @@ get_attribute_namespace (const_tree attr) return get_identifier ("gnu"); } +/* Check LAST_DECL and NODE of the same symbol for attributes that are + recorded in EXCL to be mutually exclusive with ATTRNAME, diagnose + them, and return true if any have been found. NODE can be a DECL + or a TYPE. */ + +static bool +diag_attr_exclusions (tree last_decl, tree node, tree attrname, + const attribute_spec *spec) +{ + const attribute_spec::exclusions *excl = spec->exclude; + + tree_code code = TREE_CODE (node); + + if ((code == FUNCTION_DECL && !excl->function + && (!excl->type || !spec->affects_type_identity)) + || (code == VAR_DECL && !excl->variable + && (!excl->type || !spec->affects_type_identity)) + || (((code == TYPE_DECL || RECORD_OR_UNION_TYPE_P (node)) && !excl->type))) + return false; + + /* True if an attribute that's mutually exclusive with ATTRNAME + has been found. */ + bool found = false; + + if (last_decl && last_decl != node && TREE_TYPE (last_decl) != node) + { + /* Check both the last DECL and its type for conflicts with + the attribute being added to the current decl or type. */ + found |= diag_attr_exclusions (last_decl, last_decl, attrname, spec); + tree decl_type = TREE_TYPE (last_decl); + found |= diag_attr_exclusions (last_decl, decl_type, attrname, spec); + } + + /* NODE is either the current DECL to which the attribute is being + applied or its TYPE. For the former, consider the attributes on + both the DECL and its type. */ + tree attrs[2]; + + if (DECL_P (node)) + { + attrs[0] = DECL_ATTRIBUTES (node); + attrs[1] = TYPE_ATTRIBUTES (TREE_TYPE (node)); + } + else + { + attrs[0] = TYPE_ATTRIBUTES (node); + attrs[1] = NULL_TREE; + } + + /* Iterate over the mutually exclusive attribute names and verify + that the symbol doesn't contain it. */ + for (unsigned i = 0; i != sizeof attrs / sizeof *attrs; ++i) + { + if (!attrs[i]) + continue; + + for ( ; excl->name; ++excl) + { + /* Avoid checking the attribute against itself. */ + if (is_attribute_p (excl->name, attrname)) + continue; + + if (!lookup_attribute (excl->name, attrs[i])) + continue; + + found = true; + + /* Print a note? */ + bool note = last_decl != NULL_TREE; + + if (TREE_CODE (node) == FUNCTION_DECL + && DECL_BUILT_IN (node)) + note &= warning (OPT_Wattributes, + "ignoring attribute %qE in declaration of " + "a built-in function qD because it conflicts " + "with attribute %qs", + attrname, node, excl->name); + else + note &= warning (OPT_Wattributes, + "ignoring attribute %qE because " + "it conflicts with attribute %qs", + attrname, excl->name); + + if (note) + inform (DECL_SOURCE_LOCATION (last_decl), + "previous declaration here"); + } + } + + return found; +} /* Process the attributes listed in ATTRIBUTES and install them in *NODE, which is either a DECL (including a TYPE_DECL) or a TYPE. If a DECL, @@ -354,7 +445,8 @@ get_attribute_namespace (const_tree attr) a decl attribute to the declaration rather than to its type). */ tree -decl_attributes (tree *node, tree attributes, int flags) +decl_attributes (tree *node, tree attributes, int flags, + tree last_decl /* = NULL_TREE */) { tree a; tree returned_attrs = NULL_TREE; @@ -433,6 +525,8 @@ decl_attributes (tree *node, tree attributes, int flags) targetm.insert_attributes (*node, &attributes); + /* Note that attributes on the same declaration are not necessarily + in the same order as in the source. */ for (a = attributes; a; a = TREE_CHAIN (a)) { tree ns = get_attribute_namespace (a); @@ -441,7 +535,6 @@ decl_attributes (tree *node, tree attributes, int flags) tree *anode = node; const struct attribute_spec *spec = lookup_scoped_attribute_spec (ns, name); - bool no_add_attrs = 0; int fn_ptr_quals = 0; tree fn_ptr_tmp = NULL_TREE; @@ -490,7 +583,8 @@ decl_attributes (tree *node, tree attributes, int flags) | (int) ATTR_FLAG_ARRAY_NEXT)) { /* Pass on this attribute to be tried again. */ - returned_attrs = tree_cons (name, args, returned_attrs); + tree attr = tree_cons (name, args, NULL_TREE); + returned_attrs = chainon (returned_attrs, attr); continue; } else @@ -535,7 +629,8 @@ decl_attributes (tree *node, tree attributes, int flags) else if (flags & (int) ATTR_FLAG_FUNCTION_NEXT) { /* Pass on this attribute to be tried again. */ - returned_attrs = tree_cons (name, args, returned_attrs); + tree attr = tree_cons (name, args, NULL_TREE); + returned_attrs = chainon (returned_attrs, attr); continue; } @@ -557,15 +652,56 @@ decl_attributes (tree *node, tree attributes, int flags) continue; } + bool no_add_attrs = false; + if (spec->handler != NULL) { int cxx11_flag = cxx11_attribute_p (a) ? ATTR_FLAG_CXX11 : 0; - returned_attrs = chainon ((*spec->handler) (anode, name, args, - flags|cxx11_flag, - &no_add_attrs), - returned_attrs); + /* Pass in an array of the current declaration followed + by the last pushed/merged declaration if one exists. + If the handler changes CUR_AND_LAST_DECL[0] replace + *ANODE with its value. */ + tree cur_and_last_decl[] = { *anode, last_decl }; + tree ret = (spec->handler) (cur_and_last_decl, name, args, + flags|cxx11_flag, &no_add_attrs); + + *anode = cur_and_last_decl[0]; + if (ret == error_mark_node) + { + warning (OPT_Wattributes, "%qE attribute ignored", name); + no_add_attrs = true; + } + else + 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. */ diff --git a/gcc/attribs.h b/gcc/attribs.h index d4a790b..a9eba0a 100644 --- a/gcc/attribs.h +++ b/gcc/attribs.h @@ -31,7 +31,7 @@ extern void init_attributes (void); from tree.h. Depending on these flags, some attributes may be returned to be applied at a later stage (for example, to apply a decl attribute to the declaration rather than to its type). */ -extern tree decl_attributes (tree *, tree, int); +extern tree decl_attributes (tree *, tree, int, tree = NULL_TREE); extern bool cxx11_attribute_p (const_tree); extern tree get_attribute_name (const_tree); diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c index 0d9ab2d..8a157f6 100644 --- a/gcc/c-family/c-attribs.c +++ b/gcc/c-family/c-attribs.c @@ -44,6 +44,7 @@ along with GCC; see the file COPYING3. If not see #include "tree-iterator.h" #include "opts.h" #include "gimplify.h" +#include "bitmap.h" static tree handle_packed_attribute (tree *, tree, tree, int, bool *); static tree handle_nocommon_attribute (tree *, tree, tree, int, bool *); @@ -146,6 +147,92 @@ static tree handle_fallthrough_attribute (tree *, tree, tree, int, bool *); static tree handle_patchable_function_entry_attribute (tree *, tree, tree, int, bool *); +/* Helper to define attribute exclusions. */ +#define ATTR_EXCL(name, function, type, variable) \ + { name, function, type, variable } + +/* Define attributes that are mutually exclusive with one another. */ +static const struct attribute_spec::exclusions attr_aligned_exclusions[] = +{ + /* Attribute name exclusion applies to: + function, type, variable */ + ATTR_EXCL ("aligned", true, false, false), + ATTR_EXCL ("packed", true, false, false), + ATTR_EXCL (NULL, false, false, false) +}; + +static const struct attribute_spec::exclusions attr_cold_hot_exclusions[] = +{ + ATTR_EXCL ("cold", true, true, true), + ATTR_EXCL ("hot", true, true, true), + ATTR_EXCL (NULL, false, false, false) +}; + +static const struct attribute_spec::exclusions attr_common_exclusions[] = +{ + ATTR_EXCL ("common", true, true, true), + ATTR_EXCL ("nocommon", true, true, true), + ATTR_EXCL (NULL, false, false, false), +}; + +static const struct attribute_spec::exclusions attr_gnu_inline_exclusions[] = +{ + ATTR_EXCL ("gnu_inline", true, true, true), + ATTR_EXCL ("noinline", true, true, true), + ATTR_EXCL (NULL, false, false, false), +}; + +static const struct attribute_spec::exclusions attr_inline_exclusions[] = +{ + ATTR_EXCL ("always_inline", true, true, true), + ATTR_EXCL ("noinline", true, true, true), + ATTR_EXCL (NULL, false, false, false), +}; + +static const struct attribute_spec::exclusions attr_noreturn_exclusions[] = +{ + ATTR_EXCL ("noreturn", true, true, true), + ATTR_EXCL ("alloc_align", true, true, true), + ATTR_EXCL ("alloc_size", true, true, true), + ATTR_EXCL ("const", true, true, true), + ATTR_EXCL ("malloc", true, true, true), + ATTR_EXCL ("pure", true, true, true), + ATTR_EXCL ("returns_twice", true, true, true), + ATTR_EXCL ("warn_unused_result", true, true, true), + ATTR_EXCL (NULL, false, false, false), +}; + +static const struct attribute_spec::exclusions +attr_warn_unused_result_exclusions[] = +{ + ATTR_EXCL ("noreturn", true, true, true), + ATTR_EXCL ("warn_unused_result", true, true, true), + ATTR_EXCL (NULL, false, false, false), +}; + +static const struct attribute_spec::exclusions attr_returns_twice_exclusions[] = +{ + ATTR_EXCL ("noreturn", true, true, true), + ATTR_EXCL (NULL, false, false, false), +}; + +/* Exclusions that apply to attribute alloc_align, alloc_size, and malloc. */ +static const struct attribute_spec::exclusions attr_alloc_exclusions[] = +{ + ATTR_EXCL ("const", true, true, true), + ATTR_EXCL ("noreturn", true, true, true), + ATTR_EXCL ("pure", true, true, true), + ATTR_EXCL (NULL, false, false, false), +}; + +static const struct attribute_spec::exclusions attr_const_pure_exclusions[] = +{ + ATTR_EXCL ("const", true, true, true), + ATTR_EXCL ("noreturn", true, true, true), + ATTR_EXCL ("pure", true, true, true), + ATTR_EXCL (NULL, false, false, false) +}; + /* Table of machine-independent attributes common to all C-like languages. All attributes referencing arguments should be additionally processed @@ -157,209 +244,230 @@ const struct attribute_spec c_common_attribute_table[] = /* { name, min_len, max_len, decl_req, type_req, fn_type_req, handler, affects_type_identity } */ { "packed", 0, 0, false, false, false, - handle_packed_attribute , false}, + handle_packed_attribute , false, + attr_aligned_exclusions }, { "nocommon", 0, 0, true, false, false, - handle_nocommon_attribute, false}, + handle_nocommon_attribute, false, + attr_common_exclusions }, { "common", 0, 0, true, false, false, - handle_common_attribute, false }, + handle_common_attribute, false, + attr_common_exclusions }, /* FIXME: logically, noreturn attributes should be listed as "false, true, true" and apply to function types. But implementing this would require all the places in the compiler that use TREE_THIS_VOLATILE on a decl to identify non-returning functions to be located and fixed to check the function type instead. */ { "noreturn", 0, 0, true, false, false, - handle_noreturn_attribute, false }, + handle_noreturn_attribute, false, + attr_noreturn_exclusions }, { "volatile", 0, 0, true, false, false, - handle_noreturn_attribute, false }, + handle_noreturn_attribute, false, NULL }, { "stack_protect", 0, 0, true, false, false, - handle_stack_protect_attribute, false }, + handle_stack_protect_attribute, false, NULL }, { "noinline", 0, 0, true, false, false, - handle_noinline_attribute, false }, + handle_noinline_attribute, false, + attr_inline_exclusions }, { "noclone", 0, 0, true, false, false, - handle_noclone_attribute, false }, + handle_noclone_attribute, false, NULL }, { "no_icf", 0, 0, true, false, false, - handle_noicf_attribute, false }, + handle_noicf_attribute, false, NULL }, { "noipa", 0, 0, true, false, false, - handle_noipa_attribute, false }, + handle_noipa_attribute, false, NULL }, { "leaf", 0, 0, true, false, false, - handle_leaf_attribute, false }, + handle_leaf_attribute, false, NULL }, { "always_inline", 0, 0, true, false, false, - handle_always_inline_attribute, false }, + handle_always_inline_attribute, false, + attr_inline_exclusions }, { "gnu_inline", 0, 0, true, false, false, - handle_gnu_inline_attribute, false }, + handle_gnu_inline_attribute, false, + attr_gnu_inline_exclusions }, { "artificial", 0, 0, true, false, false, - handle_artificial_attribute, false }, + handle_artificial_attribute, false, NULL }, { "flatten", 0, 0, true, false, false, - handle_flatten_attribute, false }, + handle_flatten_attribute, false, NULL }, { "used", 0, 0, true, false, false, - handle_used_attribute, false }, + handle_used_attribute, false, NULL }, { "unused", 0, 0, false, false, false, - handle_unused_attribute, false }, + handle_unused_attribute, false, NULL }, { "externally_visible", 0, 0, true, false, false, - handle_externally_visible_attribute, false }, + handle_externally_visible_attribute, false, + NULL }, { "no_reorder", 0, 0, true, false, false, - handle_no_reorder_attribute, false }, + handle_no_reorder_attribute, false, NULL }, /* The same comments as for noreturn attributes apply to const ones. */ { "const", 0, 0, true, false, false, - handle_const_attribute, false }, + handle_const_attribute, false, + attr_const_pure_exclusions }, { "scalar_storage_order", 1, 1, false, false, false, - handle_scalar_storage_order_attribute, false }, + handle_scalar_storage_order_attribute, false, + NULL }, { "transparent_union", 0, 0, false, false, false, - handle_transparent_union_attribute, false }, + handle_transparent_union_attribute, false, NULL }, { "constructor", 0, 1, true, false, false, - handle_constructor_attribute, false }, + handle_constructor_attribute, false, NULL }, { "destructor", 0, 1, true, false, false, - handle_destructor_attribute, false }, + handle_destructor_attribute, false, NULL }, { "mode", 1, 1, false, true, false, - handle_mode_attribute, false }, + handle_mode_attribute, false, NULL }, { "section", 1, 1, true, false, false, - handle_section_attribute, false }, + handle_section_attribute, false, NULL }, { "aligned", 0, 1, false, false, false, - handle_aligned_attribute, false }, + handle_aligned_attribute, false, + attr_aligned_exclusions }, { "weak", 0, 0, true, false, false, - handle_weak_attribute, false }, + handle_weak_attribute, false, NULL }, { "noplt", 0, 0, true, false, false, - handle_noplt_attribute, false }, + handle_noplt_attribute, false, NULL }, { "ifunc", 1, 1, true, false, false, - handle_ifunc_attribute, false }, + handle_ifunc_attribute, false, NULL }, { "alias", 1, 1, true, false, false, - handle_alias_attribute, false }, + handle_alias_attribute, false, NULL }, { "weakref", 0, 1, true, false, false, - handle_weakref_attribute, false }, + handle_weakref_attribute, false, NULL }, { "no_instrument_function", 0, 0, true, false, false, handle_no_instrument_function_attribute, - false }, + false, NULL }, { "no_profile_instrument_function", 0, 0, true, false, false, handle_no_profile_instrument_function_attribute, - false }, + false, NULL }, { "malloc", 0, 0, true, false, false, - handle_malloc_attribute, false }, + handle_malloc_attribute, false, + attr_alloc_exclusions }, { "returns_twice", 0, 0, true, false, false, - handle_returns_twice_attribute, false }, + handle_returns_twice_attribute, false, + attr_returns_twice_exclusions }, { "no_stack_limit", 0, 0, true, false, false, - handle_no_limit_stack_attribute, false }, + handle_no_limit_stack_attribute, false, NULL }, { "pure", 0, 0, true, false, false, - handle_pure_attribute, false }, + handle_pure_attribute, false, + attr_const_pure_exclusions }, { "transaction_callable", 0, 0, false, true, false, - handle_tm_attribute, false }, + handle_tm_attribute, false, NULL }, { "transaction_unsafe", 0, 0, false, true, false, - handle_tm_attribute, true }, + handle_tm_attribute, true, NULL }, { "transaction_safe", 0, 0, false, true, false, - handle_tm_attribute, true }, + handle_tm_attribute, true, NULL }, { "transaction_safe_dynamic", 0, 0, true, false, false, - handle_tm_attribute, false }, + handle_tm_attribute, false, NULL }, { "transaction_may_cancel_outer", 0, 0, false, true, false, - handle_tm_attribute, false }, + handle_tm_attribute, false, NULL }, /* ??? These two attributes didn't make the transition from the Intel language document to the multi-vendor language document. */ { "transaction_pure", 0, 0, false, true, false, - handle_tm_attribute, false }, + handle_tm_attribute, false, NULL }, { "transaction_wrap", 1, 1, true, false, false, - handle_tm_wrap_attribute, false }, + handle_tm_wrap_attribute, false, NULL }, /* For internal use (marking of builtins) only. The name contains space to prevent its usage in source code. */ { "no vops", 0, 0, true, false, false, - handle_novops_attribute, false }, + handle_novops_attribute, false, NULL }, { "deprecated", 0, 1, false, false, false, - handle_deprecated_attribute, false }, + handle_deprecated_attribute, false, NULL }, { "vector_size", 1, 1, false, true, false, - handle_vector_size_attribute, true }, + handle_vector_size_attribute, true, NULL }, { "visibility", 1, 1, false, false, false, - handle_visibility_attribute, false }, + handle_visibility_attribute, false, NULL }, { "tls_model", 1, 1, true, false, false, - handle_tls_model_attribute, false }, + handle_tls_model_attribute, false, NULL }, { "nonnull", 0, -1, false, true, true, - handle_nonnull_attribute, false }, + handle_nonnull_attribute, false, NULL }, { "nothrow", 0, 0, true, false, false, - handle_nothrow_attribute, false }, - { "may_alias", 0, 0, false, true, false, NULL, false }, + handle_nothrow_attribute, false, NULL }, + { "may_alias", 0, 0, false, true, false, NULL, false, NULL }, { "cleanup", 1, 1, true, false, false, - handle_cleanup_attribute, false }, + handle_cleanup_attribute, false, NULL }, { "warn_unused_result", 0, 0, false, true, true, - handle_warn_unused_result_attribute, false }, + handle_warn_unused_result_attribute, false, + attr_warn_unused_result_exclusions }, { "sentinel", 0, 1, false, true, true, - handle_sentinel_attribute, false }, + handle_sentinel_attribute, false, NULL }, /* For internal use (marking of builtins) only. The name contains space to prevent its usage in source code. */ { "type generic", 0, 0, false, true, true, - handle_type_generic_attribute, false }, + handle_type_generic_attribute, false, NULL }, { "alloc_size", 1, 2, false, true, true, - handle_alloc_size_attribute, false }, + handle_alloc_size_attribute, false, + attr_alloc_exclusions }, { "cold", 0, 0, true, false, false, - handle_cold_attribute, false }, + handle_cold_attribute, false, + attr_cold_hot_exclusions }, { "hot", 0, 0, true, false, false, - handle_hot_attribute, false }, + handle_hot_attribute, false, + attr_cold_hot_exclusions }, { "no_address_safety_analysis", 0, 0, true, false, false, handle_no_address_safety_analysis_attribute, - false }, + false, NULL }, { "no_sanitize", 1, 1, true, false, false, handle_no_sanitize_attribute, - false }, + false, NULL }, { "no_sanitize_address", 0, 0, true, false, false, handle_no_sanitize_address_attribute, - false }, + false, NULL }, { "no_sanitize_thread", 0, 0, true, false, false, handle_no_sanitize_thread_attribute, - false }, + false, NULL }, { "no_sanitize_undefined", 0, 0, true, false, false, handle_no_sanitize_undefined_attribute, - false }, + false, NULL }, { "asan odr indicator", 0, 0, true, false, false, handle_asan_odr_indicator_attribute, - false }, + false, NULL }, { "warning", 1, 1, true, false, false, - handle_error_attribute, false }, + handle_error_attribute, false, NULL }, { "error", 1, 1, true, false, false, - handle_error_attribute, false }, + handle_error_attribute, false, NULL }, { "target", 1, -1, true, false, false, - handle_target_attribute, false }, + handle_target_attribute, false, NULL }, { "target_clones", 1, -1, true, false, false, - handle_target_clones_attribute, false }, + handle_target_clones_attribute, false, NULL }, { "optimize", 1, -1, true, false, false, - handle_optimize_attribute, false }, + handle_optimize_attribute, false, NULL }, /* For internal use only. The leading '*' both prevents its usage in source code and signals that it may be overridden by machine tables. */ { "*tm regparm", 0, 0, false, true, true, - ignore_attribute, false }, + ignore_attribute, false, NULL }, { "no_split_stack", 0, 0, true, false, false, - handle_no_split_stack_attribute, false }, + handle_no_split_stack_attribute, false, NULL }, /* For internal use (marking of builtins and runtime functions) only. The name contains space to prevent its usage in source code. */ { "fn spec", 1, 1, false, true, true, - handle_fnspec_attribute, false }, + handle_fnspec_attribute, false, NULL }, { "warn_unused", 0, 0, false, false, false, - handle_warn_unused_attribute, false }, + handle_warn_unused_attribute, false, NULL }, { "returns_nonnull", 0, 0, false, true, true, - handle_returns_nonnull_attribute, false }, + handle_returns_nonnull_attribute, false, NULL }, { "omp declare simd", 0, -1, true, false, false, - handle_omp_declare_simd_attribute, false }, + handle_omp_declare_simd_attribute, false, NULL }, { "cilk simd function", 0, -1, true, false, false, - handle_omp_declare_simd_attribute, false }, + handle_omp_declare_simd_attribute, false, NULL }, { "simd", 0, 1, true, false, false, - handle_simd_attribute, false }, + handle_simd_attribute, false, NULL }, { "omp declare target", 0, 0, true, false, false, - handle_omp_declare_target_attribute, false }, + handle_omp_declare_target_attribute, false, + NULL }, { "omp declare target link", 0, 0, true, false, false, - handle_omp_declare_target_attribute, false }, + handle_omp_declare_target_attribute, false, + NULL }, { "alloc_align", 1, 1, false, true, true, - handle_alloc_align_attribute, false }, + handle_alloc_align_attribute, false, + attr_alloc_exclusions }, { "assume_aligned", 1, 2, false, true, true, - handle_assume_aligned_attribute, false }, + handle_assume_aligned_attribute, false, NULL }, { "designated_init", 0, 0, false, true, false, - handle_designated_init_attribute, false }, + handle_designated_init_attribute, false, NULL }, { "bnd_variable_size", 0, 0, true, false, false, - handle_bnd_variable_size_attribute, false }, + handle_bnd_variable_size_attribute, false, NULL }, { "bnd_legacy", 0, 0, true, false, false, - handle_bnd_legacy, false }, + handle_bnd_legacy, false, NULL }, { "bnd_instrument", 0, 0, true, false, false, - handle_bnd_instrument, false }, + handle_bnd_instrument, false, NULL }, { "fallthrough", 0, 0, false, false, false, - handle_fallthrough_attribute, false }, + handle_fallthrough_attribute, false, NULL }, { "patchable_function_entry", 1, 2, true, false, false, handle_patchable_function_entry_attribute, - false }, - { NULL, 0, 0, false, false, false, NULL, false } + false, NULL }, + { NULL, 0, 0, false, false, false, NULL, false, NULL } }; /* Give the specifications for the format attributes, used by C and all @@ -374,10 +482,10 @@ const struct attribute_spec c_common_format_attribute_table[] = /* { name, min_len, max_len, decl_req, type_req, fn_type_req, handler, affects_type_identity } */ { "format", 3, 3, false, true, true, - handle_format_attribute, false }, + handle_format_attribute, false, NULL }, { "format_arg", 1, 1, false, true, true, - handle_format_arg_attribute, false }, - { NULL, 0, 0, false, false, false, NULL, false } + handle_format_arg_attribute, false, NULL }, + { NULL, 0, 0, false, false, false, NULL, false, NULL } }; /* Returns TRUE iff the attribute indicated by ATTR_ID takes a plain @@ -515,14 +623,7 @@ handle_hot_attribute (tree *node, tree name, tree ARG_UNUSED (args), if (TREE_CODE (*node) == FUNCTION_DECL || TREE_CODE (*node) == LABEL_DECL) { - if (lookup_attribute ("cold", DECL_ATTRIBUTES (*node)) != NULL) - { - warning (OPT_Wattributes, "%qE attribute ignored due to conflict " - "with attribute %qs", name, "cold"); - *no_add_attrs = true; - } - /* Most of the rest of the hot processing is done later with - lookup_attribute. */ + /* Attribute hot processing is done later with lookup_attribute. */ } else { @@ -543,14 +644,7 @@ handle_cold_attribute (tree *node, tree name, tree ARG_UNUSED (args), if (TREE_CODE (*node) == FUNCTION_DECL || TREE_CODE (*node) == LABEL_DECL) { - if (lookup_attribute ("hot", DECL_ATTRIBUTES (*node)) != NULL) - { - warning (OPT_Wattributes, "%qE attribute ignored due to conflict " - "with attribute %qs", name, "hot"); - *no_add_attrs = true; - } - /* Most of the rest of the cold processing is done later with - lookup_attribute. */ + /* Attribute cold processing is done later with lookup_attribute. */ } else { @@ -1064,7 +1158,7 @@ handle_no_reorder_attribute (tree *pnode, static tree handle_const_attribute (tree *node, tree name, tree ARG_UNUSED (args), - int ARG_UNUSED (flags), bool *no_add_attrs) + int flags, bool *no_add_attrs) { tree type = TREE_TYPE (*node); @@ -1085,6 +1179,14 @@ handle_const_attribute (tree *node, tree name, tree ARG_UNUSED (args), *no_add_attrs = true; } + /* void __builtin_unreachable(void) is const. Accept other such + built-ins but warn on user-defined functions that return void. */ + if (!(flags & ATTR_FLAG_BUILT_IN) + && TREE_CODE (*node) == FUNCTION_DECL + && VOID_TYPE_P (TREE_TYPE (type))) + warning (OPT_Wattributes, "%qE attribute on function " + "returning %<void%>", name); + return NULL_TREE; } @@ -1667,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) { @@ -1693,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; + + /* The alignment of the current declaration and that of the last + pushed declaration, determined on demand below. */ + unsigned curalign = 0; + unsigned lastalign = 0; - if ((i = check_user_alignment (align_expr, true)) == -1 - || !check_cxx_fundamental_alignment_constraints (*node, i, flags)) + if (pow2align == -1 + || !check_cxx_fundamental_alignment_constraints (*node, pow2align, flags)) *no_add_attrs = true; else if (is_type) { @@ -1717,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) @@ -1726,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 @@ -1737,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; } @@ -2476,8 +2607,15 @@ handle_pure_attribute (tree *node, tree name, tree ARG_UNUSED (args), int ARG_UNUSED (flags), bool *no_add_attrs) { if (TREE_CODE (*node) == FUNCTION_DECL) - DECL_PURE_P (*node) = 1; - /* ??? TODO: Support types. */ + { + tree type = TREE_TYPE (*node); + if (VOID_TYPE_P (TREE_TYPE (type))) + warning (OPT_Wattributes, "%qE attribute on function " + "returning %<void%>", name); + + DECL_PURE_P (*node) = 1; + /* ??? TODO: Support types. */ + } else { warning (OPT_Wattributes, "%qE attribute ignored", name); diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c index e970ab2..f7c8eac 100644 --- a/gcc/c-family/c-warn.c +++ b/gcc/c-family/c-warn.c @@ -2143,36 +2143,19 @@ diagnose_mismatched_attributes (tree olddecl, tree newdecl) newdecl); /* Diagnose inline __attribute__ ((noinline)) which is silly. */ + const char *noinline = "noinline"; + if (DECL_DECLARED_INLINE_P (newdecl) && DECL_UNINLINABLE (olddecl) - && lookup_attribute ("noinline", DECL_ATTRIBUTES (olddecl))) + && lookup_attribute (noinline, DECL_ATTRIBUTES (olddecl))) warned |= warning (OPT_Wattributes, "inline declaration of %qD follows " - "declaration with attribute noinline", newdecl); + "declaration with attribute %qs", newdecl, noinline); else if (DECL_DECLARED_INLINE_P (olddecl) && DECL_UNINLINABLE (newdecl) && lookup_attribute ("noinline", DECL_ATTRIBUTES (newdecl))) warned |= warning (OPT_Wattributes, "declaration of %q+D with attribute " - "noinline follows inline declaration ", newdecl); - else if (lookup_attribute ("noinline", DECL_ATTRIBUTES (newdecl)) - && lookup_attribute ("always_inline", DECL_ATTRIBUTES (olddecl))) - warned |= warning (OPT_Wattributes, "declaration of %q+D with attribute " - "%qs follows declaration with attribute %qs", - newdecl, "noinline", "always_inline"); - else if (lookup_attribute ("always_inline", DECL_ATTRIBUTES (newdecl)) - && lookup_attribute ("noinline", DECL_ATTRIBUTES (olddecl))) - warned |= warning (OPT_Wattributes, "declaration of %q+D with attribute " - "%qs follows declaration with attribute %qs", - newdecl, "always_inline", "noinline"); - else if (lookup_attribute ("cold", DECL_ATTRIBUTES (newdecl)) - && lookup_attribute ("hot", DECL_ATTRIBUTES (olddecl))) - warned |= warning (OPT_Wattributes, "declaration of %q+D with attribute " - "%qs follows declaration with attribute %qs", - newdecl, "cold", "hot"); - else if (lookup_attribute ("hot", DECL_ATTRIBUTES (newdecl)) - && lookup_attribute ("cold", DECL_ATTRIBUTES (olddecl))) - warned |= warning (OPT_Wattributes, "declaration of %q+D with attribute " - "%qs follows declaration with attribute %qs", - newdecl, "hot", "cold"); + "%qs follows inline declaration ", newdecl, noinline); + return warned; } diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c index a54e121..5852c0d 100644 --- a/gcc/c/c-decl.c +++ b/gcc/c/c-decl.c @@ -4589,7 +4589,16 @@ c_decl_attributes (tree *node, tree attributes, int flags) attributes = tree_cons (get_identifier ("omp declare target"), NULL_TREE, attributes); } - return decl_attributes (node, attributes, flags); + + /* Look up the current declaration with all the attributes merged + so far so that attributes on the current declaration that's + about to be pushed that conflict with the former can be detected, + diagnosed, and rejected as appropriate. */ + tree last_decl = lookup_name (DECL_NAME (*node)); + if (!last_decl) + last_decl = lookup_name_in_scope (DECL_NAME (*node), external_scope); + + return decl_attributes (node, attributes, flags, last_decl); } diff --git a/gcc/testsuite/c-c++-common/Wattributes.c b/gcc/testsuite/c-c++-common/Wattributes.c new file mode 100644 index 0000000..60cd9dc --- /dev/null +++ b/gcc/testsuite/c-c++-common/Wattributes.c @@ -0,0 +1,463 @@ +/* { dg-do compile } + { dg-options "-Wall -Wattributes -ftrack-macro-expansion=0" } */ + +#define ATTR(attrlist) __attribute__ (attrlist) + +/* Exercise the handling of the mutually exclusive attributes + aligned and packed on a type definition. */ + +/* Pointless but benign. */ +struct ATTR ((aligned, aligned)) +AlignedAligned { int i; }; + +/* Aligned followed by packed on a type and vice versa has a valid use: + to decrease the alignment of a type to the specified boundary. */ +struct ATTR ((aligned, packed)) +AlignedPacked { int i; }; + +struct ATTR ((packed, aligned)) +PackedAligned { int i; }; + +struct ATTR ((aligned (2))) +AlignedMemberPacked +{ + int ATTR ((packed)) i; +}; + +struct ATTR ((packed)) +PackedMemberAligned +{ + int ATTR ((aligned (2))) i; +}; + +/* Silly but benign. */ +struct ATTR ((packed, packed)) +PackedPacked { int i; }; + + +/* Exercise the handling of the mutually exclusive attributes + aligned and packed on a function declaration. */ + +void ATTR ((aligned (8), packed)) +faligned8_1 (void); /* { dg-warning ".packed. attribute ignored" } */ + +void ATTR ((aligned (8))) +faligned8_2 (void); /* { dg-message "previous declaration here" "" { xfail *-*-* } } */ + +void ATTR ((packed)) +faligned8_2 (void); /* { dg-warning ".packed. attribute ignored" } */ + +/* Exercise the handling of the mutually exclusive attributes + always_inline and noinline. */ + +inline void ATTR ((always_inline)) +falways_inline1 (void); + +inline void ATTR ((__always_inline__)) +falways_inline1 (void); + +inline void ATTR ((always_inline, __always_inline__)) +falways_inline1 (void); + +/* Verify that repeating attribute always_inline doesn't trigger a warning. */ +inline void ATTR ((always_inline)) +falways_inline1 (void); /* { dg-message "previous declaration here" } */ + +void ATTR ((noinline)) +falways_inline1 (void) { } /* { dg-warning "ignoring attribute .noinline. because it conflicts with attribute .always_inline." } */ + +/* And again. */ +void ATTR ((always_inline)) +falways_inline (void); + + +/* Exercise the handling of the mutually exclusive attributes + noreturn and warn_unused_result. */ + +int ATTR ((__noreturn__)) +fnoret1 (void); + +int ATTR ((noreturn)) +fnoret1 (void); /* { dg-message "previous declaration here" } */ + +int ATTR ((warn_unused_result)) +fnoret1 (void); /* { dg-warning "ignoring attribute .warn_unused_result. because it conflicts with attribute .noreturn." } */ + +/* Verify that repeating attribute noreturn doesn't trigger a warning. */ +int ATTR ((noreturn)) fnoret1 (void); + +int call_noret1 (void) +{ + /* Verify that attribute warn_unused_result was, in fact, ignored + on the second declaration of fnoret1. */ + fnoret1 (); +} + +int ATTR ((noreturn, warn_unused_result)) +fnoret2 (void); /* { dg-warning "ignoring attribute .warn_unused_result. because it conflicts with attribute .noreturn." } */ + +/* Verify that repeating attribute noreturn doesn't trigger a warning. */ +int ATTR ((noreturn)) fnoret2 (void); + +int call_noret2 (void) +{ + /* Verify that attribute warn_unused_result was, in fact, ignored + on the second declaration of fnoret2. */ + fnoret2 (); +} + +/* Verify again but this time in different scopes. */ + +int ATTR ((noreturn)) +fnoret3 (void); /* { dg-message "previous declaration here" } */ + +void declare_noret3 (void) +{ + int ATTR ((warn_unused_result)) + fnoret3 (void); /* { dg-warning "ignoring attribute .warn_unused_result. because it conflicts with attribute .noreturn." } */ +} + +void declare_noret4 (void) +{ + int ATTR ((warn_unused_result)) + fnoret4 (void); /* { dg-message "previous declaration here" } */ +} + +int ATTR ((noreturn)) +fnoret4 (void); /* { dg-warning "ignoring attribute .noreturn. because it conflicts with attribute .warn_unused_result." } */ + + +/* And again, but with both declared in a different local scope. */ + +void declare_noret5_1 (void) +{ + int ATTR ((noreturn)) + fnoret5 (void); /* { dg-message "previous declaration here" } */ +} + +int declare_noret5_2 (void) +{ + int ATTR ((warn_unused_result)) + fnoret5 (void); /* { dg-warning "ignoring attribute .warn_unused_result. because it conflicts with attribute .noreturn." } */ + + /* Verify that no warning is issued below (because the warn_unused_result + attribute above was dropped). */ + fnoret5 (); +} + +/* Verify that attribute noreturn isn't diagnosed on a declaration + that was previously declared warn_unused_result and that attribute + was dropped (because the function returs void). */ + +void ATTR ((warn_unused_result)) +fnorety6 (void); /* { dg-warning ".warn_unused_result. attribute ignored" } */ + +void ATTR ((noreturn)) +fnoret6 (void); + + +/* Exercise the handling of the mutually exclusive attributes + noreturn and alloc_align. */ + +void* ATTR ((noreturn)) +fnoret_alloc_align1 (int); /* { dg-message "previous declaration here" } */ + +void* ATTR ((alloc_align (1))) +fnoret_alloc_align1 (int); /* { dg-warning "ignoring attribute .alloc_align. because it conflicts with attribute .noreturn." } */ + +void* ATTR ((noreturn, alloc_align (1))) +fnoret_alloc_align2 (int); /* { dg-warning "ignoring attribute .alloc_align. because it conflicts with attribute .noreturn." } */ + + +void* ATTR ((noreturn)) ATTR ((alloc_align (1))) +fnoret_alloc_align3 (int); /* { dg-warning "ignoring attribute .alloc_align. because it conflicts with attribute .noreturn." } */ + + +void* ATTR ((alloc_align (1))) +falloc_align_noret1 (int); /* { dg-message "previous declaration here" } */ + +void* ATTR ((noreturn)) +falloc_align_noret1 (int); /* { dg-warning "ignoring attribute .noreturn. because it conflicts with attribute .alloc_align." } */ + + +void* ATTR ((alloc_align (1), noreturn)) +falloc_align_noret2 (int); /* { dg-warning "ignoring attribute .(noreturn|alloc_align). because it conflicts with attribute .(alloc_align|noreturn)." } */ + +void* ATTR ((alloc_align (1))) ATTR ((noreturn)) +falloc_align_noret3 (int); /* { dg-warning "ignoring attribute .(noreturn|alloc_align). because it conflicts with attribute .(noreturn|alloc_align)." } */ + + +/* Exercise the handling of the mutually exclusive attributes + noreturn and alloc_size. */ + +void* ATTR ((noreturn)) +fnoret_alloc_size1 (int); /* { dg-message "previous declaration here" } */ + +void* ATTR ((alloc_size (1))) +fnoret_alloc_size1 (int); /* { dg-warning "ignoring attribute .alloc_size. because it conflicts with attribute .noreturn." } */ + +void* ATTR ((noreturn, alloc_size (1))) +fnoret_alloc_size2 (int); /* { dg-warning "ignoring attribute .alloc_size. because it conflicts with attribute .noreturn." } */ + + +void* ATTR ((noreturn)) ATTR ((alloc_size (1))) +fnoret_alloc_size3 (int); /* { dg-warning "ignoring attribute .alloc_size. because it conflicts with attribute .noreturn." } */ + + +void* ATTR ((alloc_size (1))) +falloc_size_noret1 (int); /* { dg-message "previous declaration here" } */ + +void* ATTR ((noreturn)) +falloc_size_noret1 (int); /* { dg-warning "ignoring attribute .noreturn. because it conflicts with attribute .alloc_size." } */ + + +void* ATTR ((alloc_size (1), noreturn)) +falloc_size_noret2 (int); /* { dg-warning "ignoring attribute .noreturn. because it conflicts with attribute .alloc_size." } */ + +void* ATTR ((alloc_size (1))) ATTR ((noreturn)) +falloc_size_noret3 (int); /* { dg-warning "ignoring attribute .noreturn. because it conflicts with attribute .alloc_size." } */ + + +/* Exercise the handling of the mutually exclusive attributes + noreturn and const. */ + +int ATTR ((noreturn)) +fnoret_const1 (int); /* { dg-message "previous declaration here" } */ + +int ATTR ((const)) +fnoret_const1 (int); /* { dg-warning "ignoring attribute .const. because it conflicts with attribute .noreturn." } */ + +/* Unfortunately, attributes on a single declarations may not be processed + in the same order as specified... */ +int ATTR ((noreturn, const)) +fnoret_const2 (int); /* { dg-warning "ignoring attribute .const. because it conflicts with attribute .noreturn." } */ + + +int ATTR ((noreturn)) ATTR ((const)) +fnoret_const3 (int); /* { dg-warning "ignoring attribute .const. because it conflicts with attribute .noreturn." } */ + + +int ATTR ((const)) +fconst_noret1 (int); /* { dg-message "previous declaration here" } */ + +int ATTR ((noreturn)) +fconst_noret1 (int); /* { dg-warning "ignoring attribute .noreturn. because it conflicts with attribute .const." } */ + + +int ATTR ((const, noreturn)) +fconst_noret2 (int); /* { dg-warning "ignoring attribute .noreturn. because it conflicts with attribute .const." } */ + +int ATTR ((const)) ATTR ((noreturn)) +fconst_noret3 (int); /* { dg-warning "ignoring attribute .noreturn. because it conflicts with attribute .const." } */ + + +/* Exercise the handling of the mutually exclusive attributes + noreturn and malloc. */ + +void* ATTR ((noreturn)) +fnoret_malloc1 (int); /* { dg-message "previous declaration here" } */ + +void* ATTR ((malloc)) +fnoret_malloc1 (int); /* { dg-warning "ignoring attribute .malloc. because it conflicts with attribute .noreturn." } */ + +/* Unfortunately, attributes on a single declarations may not be processed + in the same order as specified... */ +void* ATTR ((noreturn, malloc)) +fnoret_malloc2 (int); /* { dg-warning "ignoring attribute .malloc. because it conflicts with attribute .noreturn." } */ + + +void* ATTR ((noreturn)) ATTR ((malloc)) +fnoret_malloc3 (int); /* { dg-warning "ignoring attribute .malloc. because it conflicts with attribute .noreturn." } */ + + +void* ATTR ((__malloc__)) +fmalloc_noret1 (int); + +void* ATTR ((malloc)) +fmalloc_noret1 (int); /* { dg-message "previous declaration here" } */ + +void* ATTR ((noreturn)) +fmalloc_noret1 (int); /* { dg-warning "ignoring attribute .noreturn. because it conflicts with attribute .malloc." } */ + + +void* ATTR ((malloc, noreturn)) +fmalloc_noret2 (int); /* { dg-warning "ignoring attribute .noreturn. because it conflicts with attribute .malloc." } */ + +void* ATTR ((malloc)) ATTR ((noreturn)) +fmalloc_noret3 (int); /* { dg-warning "ignoring attribute .noreturn. because it conflicts with attribute .malloc." } */ + + +/* Exercise the handling of the mutually exclusive attributes + noreturn and pure. */ + +int ATTR ((noreturn)) +fnoret_pure1 (int); /* { dg-message "previous declaration here" } */ + +int ATTR ((pure)) +fnoret_pure1 (int); /* { dg-warning "ignoring attribute .pure. because it conflicts with attribute .noreturn." } */ + +/* Unfortunately, attributes on a single declarations may not be processed + in the same order as specified... */ +int ATTR ((noreturn, pure)) +fnoret_pure2 (int); /* { dg-warning "ignoring attribute .pure. because it conflicts with attribute .noreturn." } */ + + +int ATTR ((noreturn)) ATTR ((pure)) +fnoret_pure3 (int); /* { dg-warning "ignoring attribute .pure. because it conflicts with attribute .noreturn." } */ + + +int ATTR ((__pure__)) +fpure_noret1 (int); + +int ATTR ((pure)) +fpure_noret1 (int); /* { dg-message "previous declaration here" } */ + +int ATTR ((noreturn)) +fpure_noret1 (int); /* { dg-warning "ignoring attribute .noreturn. because it conflicts with attribute .pure." } */ + + +int ATTR ((pure, noreturn)) +fpure_noret2 (int); /* { dg-warning "ignoring attribute .noreturn. because it conflicts with attribute .pur." } */ + +int ATTR ((pure)) ATTR ((noreturn)) +fpure_noret3 (int); /* { dg-warning "ignoring attribute .noreturn. because it conflicts with attribute .pure." } */ + + +/* Exercise the handling of the mutually exclusive attributes + noreturn and returns_twice. */ + +int ATTR ((noreturn)) +fnoret_returns_twice1 (int); /* { dg-message "previous declaration here" } */ + +int ATTR ((returns_twice)) +fnoret_returns_twice1 (int); /* { dg-warning "ignoring attribute .returns_twice. because it conflicts with attribute .noreturn." } */ + +/* Unfortunately, attributes on a single declarations may not be processed + in the same order as specified... */ +int ATTR ((noreturn, returns_twice)) +fnoret_returns_twice2 (int); /* { dg-warning "ignoring attribute .returns_twice. because it conflicts with attribute .noreturn." } */ + + +int ATTR ((noreturn)) ATTR ((returns_twice)) +fnoret_returns_twice3 (int); /* { dg-warning "ignoring attribute .returns_twice. because it conflicts with attribute .noreturn." } */ + + +int ATTR ((returns_twice)) +freturns_twice_noret1 (int); /* { dg-message "previous declaration here" } */ + +int ATTR ((noreturn)) +freturns_twice_noret1 (int); /* { dg-warning "ignoring attribute .noreturn. because it conflicts with attribute .returns_twice." } */ + + +int ATTR ((returns_twice, noreturn)) +freturns_twice_noret2 (int); /* { dg-warning "ignoring attribute .noreturn. because it conflicts with attribute .returns_twice." } */ + +int ATTR ((returns_twice)) ATTR ((noreturn)) +freturns_twice_noret3 (int); /* { dg-warning "ignoring attribute .noreturn. because it conflicts with attribute .returns_twice." } */ + + +/* Exercise the interaction of multiple combinations of mutually + exclusive attributes specified on distinct declarations. */ + +inline int ATTR ((always_inline)) +finline_cold_noreturn (int); + +inline int ATTR ((cold)) +finline_cold_noreturn (int); + +inline int ATTR ((noreturn)) +finline_cold_noreturn (int); + +inline int ATTR ((noinline)) +finline_cold_noreturn (int); /* { dg-warning "ignoring attribute .noinline. because it conflicts with attribute .always_inline." } */ + +inline int ATTR ((hot)) +finline_cold_noreturn (int); /* { dg-warning "ignoring attribute .hot. because it conflicts with attribute .cold." } */ + +inline int ATTR ((warn_unused_result)) +finline_cold_noreturn (int); /* { dg-warning "ignoring attribute .warn_unused_result. because it conflicts with attribute .noreturn." } */ + +inline int ATTR ((always_inline)) +finline_cold_noreturn (int); + +/* Expect no warning for the missing return statement below because + the function is noreturn. */ +inline int ATTR ((noreturn)) +finline_cold_noreturn (int i) { (void)&i; __builtin_abort (); } + + +/* Exercise the interaction of multiple combinations of mutually + exclusive attributes with some specified on the same declaration + and some on distinct declarations. */ + +inline int ATTR ((always_inline, hot)) +finline_hot_noret_align (int); + +inline int ATTR ((noreturn, noinline)) +finline_hot_noret_align (int); /* { dg-warning "ignoring attribute .noinline. because it conflicts with attribute .always_inline." } */ + +inline int ATTR ((cold, aligned (8))) +finline_hot_noret_align (int); /* { dg-warning "ignoring attribute .cold. because it conflicts with attribute .hot." } */ + +inline int ATTR ((warn_unused_result)) +finline_hot_noret_align (int); /* { dg-warning "ignoring attribute .warn_unused_result. because it conflicts with attribute .noreturn." } */ + +inline int ATTR ((aligned (4))) +finline_hot_noret_align (int); /* { dg-warning "ignoring attribute .aligned \\(4\\). because it conflicts with attribute .aligned \\(8\\)." } */ + +inline int ATTR ((aligned (8))) +finline_hot_noret_align (int); + +inline int ATTR ((const)) +finline_hot_noret_align (int); /* { dg-warning "ignoring attribute .const. because it conflicts with attribute .noreturn." } */ + +/* Expect no warning for the missing return statement below because + the function is noreturn. */ +inline int ATTR ((noreturn)) +finline_hot_noret_align (int i) { (void)&i; __builtin_abort (); } + + +/* Exercise variable attributes. */ + +extern int ATTR ((common)) +decl_common1; /* { dg-message "previous declaration here" } */ + +extern int ATTR ((nocommon)) +decl_common1; /* { dg-warning "ignoring attribute .nocommon. because it conflicts with attribute .common." } */ + + +extern int ATTR ((nocommon)) +decl_common2; /* { dg-message "previous declaration here" } */ + +extern int ATTR ((common)) +decl_common2; /* { dg-warning "ignoring attribute .common. because it conflicts with attribute .nocommon." } */ + + +extern int ATTR ((common, nocommon)) +decl_common3; /* { dg-warning "ignoring attribute .nocommon. because it conflicts with attribute .common." } */ + + +extern int ATTR ((common, nocommon)) +decl_common4; /* { dg-warning "ignoring attribute .nocommon. because it conflicts with attribute .common." } */ + + +void declare_common5_in_local_scope (void) +{ + extern int ATTR ((common)) + decl_common5; /* { dg-message "previous declaration here" } */ + (void)&decl_common5; +} + +extern int ATTR ((nocommon)) +decl_common5; /* { dg-warning "ignoring attribute .nocommon. because it conflicts with attribute .common." } */ + + +extern int ATTR ((nocommon)) +decl_common6; /* { dg-message "previous declaration here" } */ + +void declare_common6_in_local_scope (void) +{ + extern int ATTR ((common)) + decl_common6; /* { dg-warning "ignoring attribute .common. because it conflicts with attribute .nocommon." } */ + (void)&decl_common6; +} diff --git a/gcc/testsuite/c-c++-common/attributes-3.c b/gcc/testsuite/c-c++-common/attributes-3.c index 821278c..9d3a61c 100644 --- a/gcc/testsuite/c-c++-common/attributes-3.c +++ b/gcc/testsuite/c-c++-common/attributes-3.c @@ -12,16 +12,16 @@ extern __attribute__((noinline)) int fn1 (void); /* { dg-message "previous decla extern inline int fn1 (void); /* { dg-warning "inline declaration of" } */ extern inline int fn2 (void); /* { dg-message "previous declaration" } */ -extern __attribute__((noinline)) int fn2 (void); /* { dg-warning "attribute noinline follows inline declaration" } */ +extern __attribute__((noinline)) int fn2 (void); /* { dg-warning "attribute .noinline. follows inline declaration" } */ extern __attribute__((always_inline)) int fn3 (void); /* { dg-message "previous declaration" } */ -extern __attribute__((noinline)) int fn3 (void); /* { dg-warning "attribute .noinline. follows declaration with attribute .always_inline." } */ +extern __attribute__((noinline)) int fn3 (void); /* { dg-warning "ignoring attribute .noinline. because it conflicts with attribute .always_inline." } */ extern __attribute__((noinline)) int fn4 (void); /* { dg-message "previous declaration" } */ -extern __attribute__((always_inline)) int fn4 (void); /* { dg-warning "attribute .always_inline. follows declaration with attribute .noinline." } */ +extern __attribute__((always_inline)) int fn4 (void); /* { dg-warning "ignoring attribute .always_inline. because it conflicts with attribute .noinline." } */ extern __attribute__((hot)) int fn5 (void); /* { dg-message "previous declaration" } */ -extern __attribute__((cold)) int fn5 (void); /* { dg-warning "attribute .cold. follows declaration with attribute .hot." } */ +extern __attribute__((cold)) int fn5 (void); /* { dg-warning "ignoring attribute .cold. because it conflicts with attribute .hot." } */ extern __attribute__((cold)) int fn6 (void); /* { dg-message "previous declaration" } */ -extern __attribute__((hot)) int fn6 (void); /* { dg-warning "attribute .hot. follows declaration with attribute .cold." } */ +extern __attribute__((hot)) int fn6 (void); /* { dg-warning "ignoring attribute .hot. because it conflicts with attribute .cold." } */ diff --git a/gcc/testsuite/gcc.dg/attr-noinline.c b/gcc/testsuite/gcc.dg/attr-noinline.c index c2a5b1d..13cc660 100644 --- a/gcc/testsuite/gcc.dg/attr-noinline.c +++ b/gcc/testsuite/gcc.dg/attr-noinline.c @@ -17,7 +17,7 @@ static void function_declaration_both_after(void) {t();} static void function_declaration_noinline_before(void) __attribute__((__noinline__)); /* { dg-message "note: previous declaration" } */ -static inline void function_declaration_noinline_before(void) {t();} /* { dg-warning "follows declaration with attribute noinline" } */ +static inline void function_declaration_noinline_before(void) {t();} /* { dg-warning "follows declaration with attribute .noinline." } */ static inline void function_declaration_noinline_after(void) {t();} /* { dg-message "note: previous definition" } */ @@ -41,7 +41,7 @@ static void function_declaration_inline_noinline_after(void) __attribute__((__no static void function_declaration_noinline_inline_before(void) __attribute__((__noinline__)); /* { dg-message "note: previous declaration" } */ -static inline void function_declaration_noinline_inline_before(void); /* { dg-warning "follows declaration with attribute noinline" } */ +static inline void function_declaration_noinline_inline_before(void); /* { dg-warning "follows declaration with attribute .noinline." } */ static void function_declaration_noinline_inline_before(void) {t();} diff --git a/gcc/testsuite/gcc.dg/pr44964.c b/gcc/testsuite/gcc.dg/pr44964.c index 1df1bde..6c252ee 100644 --- a/gcc/testsuite/gcc.dg/pr44964.c +++ b/gcc/testsuite/gcc.dg/pr44964.c @@ -2,8 +2,9 @@ /* { dg-options "-fkeep-inline-functions -O" } */ static inline __attribute__ ((const)) -void baz (int i) +int baz (int i) { + return i; } static __attribute__ ((always_inline)) diff --git a/gcc/testsuite/gcc.dg/torture/pr42363.c b/gcc/testsuite/gcc.dg/torture/pr42363.c index 9c9da13..ad0eac8 100644 --- a/gcc/testsuite/gcc.dg/torture/pr42363.c +++ b/gcc/testsuite/gcc.dg/torture/pr42363.c @@ -46,16 +46,18 @@ int bizr (void) return i + 1; } -/* This might be regarded as pure and folded, rather than inlined. - It's pure evil. */ +/* This might be regarded as pure and folded, rather than inlined, + but because it's pure evil it's diagnosed and the noreturn attribute + is dropped. The const attribute is dropped as well because it's + mutually exclusive with pure. */ static int __attribute__ ((pure, const, noreturn)) -barf (void) -{ +barf (void) { + /* { dg-warning "ignoring attribute .const." "const" { target *-*-* } .-1 } */ + /* { dg-warning "ignoring attribute .noreturn." "noreturn" { target *-*-* } .-2 } */ } /* { dg-warning "does return" } */ static int __attribute__ ((pure, const)) -bark (void) -{ +bark (void) { /* { dg-warning "ignoring attribute .const." } */ barf (); } diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c index 146b76c..b8c5654 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c @@ -113,17 +113,18 @@ int test9 (int *intarr) int test99 (int *intarr) { - extern int foo9 (int) __attribute__ ((pure)); + extern int foo99 (int) __attribute__ ((pure)); int h, v; g9 = 9; - h = foo9 (g9); + h = foo99 (g9); v = g9; if (v != 9) link_error (); return g9; } -extern int foo99 (int); +/* foo9 is const because of its declaration in test9. */ +extern int foo9 (int); int test999 (int *arr) { @@ -134,10 +135,12 @@ int test999 (int *arr) v1 = g9; if (v1 != 9) link_error (); - l = foo99 (l); + l = foo9 (l); return v1 + l; } +/* foo99 is pure because of its declaration in test99. */ +extern int foo9 (int); int test9999 (void) { diff --git a/gcc/tree-core.h b/gcc/tree-core.h index 278d0c9..d7bf757 100644 --- a/gcc/tree-core.h +++ b/gcc/tree-core.h @@ -1933,6 +1933,20 @@ struct attribute_spec { int flags, bool *no_add_attrs); /* Specifies if attribute affects type's identity. */ bool affects_type_identity; + + /* Specifies the name of an attribute that's mutually exclusive with + this one, and whether the relationship applies to the function, + variable, or type form of the attribute. */ + struct exclusions { + const char *name; + bool function; + bool variable; + bool type; + }; + + /* An array of attribute exclusions describing names of other attributes + that this attribute is mutually exclusive with. */ + const exclusions *exclude; }; /* These functions allow a front-end to perform a manual layout of a diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index b253ccc..b6b71d8 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -2490,9 +2490,14 @@ are automatically detected and this attribute is ignored. @cindex @code{const} function attribute @cindex functions that have no side effects Many functions do not examine any values except their arguments, and -have no effects except the return value. Basically this is just slightly -more strict class than the @code{pure} attribute below, since function is not -allowed to read global memory. +have no effects except to return a value. Calls to such functions lend +themselves to optimization such as common subexpression elimination. +The @code{const} attribute imposes greater restrictions on a function's +definition than the similar @code{pure} attribute below because it prohibits +the function from reading global variables. Consequently, the presence of +the attribute on a function declaration allows GCC to emit more efficient +code for some calls to the function. Decorating the same function with +both the @code{const} and the @code{pure} attribute is diagnnosed. @cindex pointer arguments Note that a function that has pointer arguments and examines the data @@ -3144,7 +3149,7 @@ to prevent recursion. @cindex functions that have no side effects Many functions have no effects except the return value and their return value depends only on the parameters and/or global variables. -Such a function can be subject +Calls to such functions can be subject to common subexpression elimination and loop optimization just as an arithmetic operator would be. These functions should be declared with the attribute @code{pure}. For example, @@ -3162,6 +3167,11 @@ Interesting non-pure functions are functions with infinite loops or those depending on volatile memory or other system resource, that may change between two consecutive calls (such as @code{feof} in a multithreading environment). +The @code{pure} attribute imposes similar but looser restrictions on +a function's defintion than the @code{const} attribute: it allows the +function to read global variables. Decorating the same function with +both the @code{pure} and the @code{const} attribute is diagnosed. + @item returns_nonnull @cindex @code{returns_nonnull} function attribute The @code{returns_nonnull} attribute specifies that the function