The existing implementation of this function was convoluted, and had
multiple control flow errors that became apparent to me while reading
the code:
1. The initial early return only checked the properties of the first
exclusion in the list, when these properties could be different for
subsequent exclusions.
2. excl was not reset within the outer loop, so the inner loop body
would only execute during the first iteration of the outer loop. This
effectively meant that the value of attrs[1] was ignored.
3. The function called itself recursively twice, with both last_decl and
TREE_TYPE (last_decl) as parameters. The second recursive call should
have been redundant, since attrs[1] = TREE_TYPE (last_decl) during the
first recursive call.
This patch eliminated the early return, and combines the checks with
those present within the inner loop. It also fixes the inner loop
initialisation, and modifies the outer loop to iterate over nodes
instead of their attributes. This latter change allows the recursion to
be eliminated, by extending the new nodes array to include last_decl
(and its type) as well.
This patch provides an alternative fix for PR114634, although I wasn't
aware of that issue until rebasing on top of Jakub's fix.
I am not aware of any other compiler bugs resulting from these issues.
However, if the exclusions for target_clones were listed in the opposite
order, then it would have broken detection of the always_inline
exclusion on aarch64 (where TARGET_HAS_FMV_TARGET_ATTRIBUTE is false).
Is this ok for master?
gcc/ChangeLog:
* attribs.cc (diag_attr_exclusions): Fix and refactor.
diff --git a/gcc/attribs.cc b/gcc/attribs.cc
index
3ab0b0fd87a4404a593b2de365ea5226e31fe24a..431dd4255e68e92dd8d10bbb21ea079e50811faa
100644
--- a/gcc/attribs.cc
+++ b/gcc/attribs.cc
@@ -433,84 +433,69 @@ get_attribute_namespace (const_tree attr)
or a TYPE. */
static bool
-diag_attr_exclusions (tree last_decl, tree node, tree attrname,
+diag_attr_exclusions (tree last_decl, tree base_node, tree attrname,
const attribute_spec *spec)
{
- const attribute_spec::exclusions *excl = spec->exclude;
- tree_code code = TREE_CODE (node);
+ /* BASE_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. Check both LAST_DECL and its type as well. */
- 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;
+ tree nodes[4] = { NULL_TREE, NULL_TREE, NULL_TREE, NULL_TREE };
- /* True if an attribute that's mutually exclusive with ATTRNAME
- has been found. */
- bool found = false;
+ nodes[0] = base_node;
+ if (DECL_P (base_node))
+ nodes[1] = (TREE_TYPE (base_node));
- if (last_decl && last_decl != node && TREE_TYPE (last_decl) != node)
+ if (last_decl)
{
- /* 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);
+ nodes[2] = last_decl;
+ if (DECL_P (last_decl))
+ nodes[3] = TREE_TYPE (last_decl);
}
- /* 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);
- if (TREE_TYPE (node))
- attrs[1] = TYPE_ATTRIBUTES (TREE_TYPE (node));
- else
- /* TREE_TYPE can be NULL e.g. while processing attributes on
- enumerators. */
- attrs[1] = NULL_TREE;
- }
- else
- {
- attrs[0] = TYPE_ATTRIBUTES (node);
- attrs[1] = NULL_TREE;
- }
+ /* True if an attribute that's mutually exclusive with ATTRNAME
+ has been found. */
+ bool found = false;
/* Iterate over the mutually exclusive attribute names and verify
that the symbol doesn't contain it. */
- for (unsigned i = 0; i != ARRAY_SIZE (attrs); ++i)
+ for (unsigned i = 0; i != ARRAY_SIZE (nodes); ++i)
{
- if (!attrs[i])
+ tree node = nodes[i];
+
+ if (!node)
continue;
- for ( ; excl->name; ++excl)
+ tree attr;
+ if DECL_P (node)
+ attr = DECL_ATTRIBUTES (node);
+ else
+ attr = TYPE_ATTRIBUTES (node);
+
+ tree_code code = TREE_CODE (node);
+
+ for (auto excl = spec->exclude; excl->name; ++excl)
{
/* Avoid checking the attribute against itself. */
if (is_attribute_p (excl->name, attrname))
continue;
- if (!lookup_attribute (excl->name, attrs[i]))
+ if (!lookup_attribute (excl->name, attr))
continue;
/* An exclusion may apply either to a function declaration,
type declaration, or a field/variable declaration, or
any subset of the three. */
- if (TREE_CODE (node) == FUNCTION_DECL
- && !excl->function)
+ if (code == FUNCTION_DECL && !excl->function
+ && (!excl->type || !spec->affects_type_identity))
continue;
- if (TREE_CODE (node) == TYPE_DECL
- && !excl->type)
+ if ((code == VAR_DECL || code == FIELD_DECL) && !excl->variable
+ && (!excl->type || !spec->affects_type_identity))
continue;
- if ((TREE_CODE (node) == FIELD_DECL
- || VAR_P (node))
- && !excl->variable)
+ if (((code == TYPE_DECL || RECORD_OR_UNION_TYPE_P (node)) &&
!excl->type))
continue;
found = true;
