[gcc(refs/vendors/ARM/heads/morello)] c, cp: Fix bad attribute handling with prefix __capability

[gcc(refs/vendors/ARM/heads/morello)] c, cp: Fix bad attribute handling with prefix __capability

[Alex Coplan]

https://gcc.gnu.org/g:350b66b67e7cac124347bceda754a82c2c440360

commit 350b66b67e7cac124347bceda754a82c2c440360
Author: Alex Coplan <alex.coplan@arm.com>
Date:   Wed Dec 21 13:31:28 2022 +0000

    c, cp: Fix bad attribute handling with prefix __capability
    
    For the following testcase:
    
    void __capability * __attribute__((__warn_unused_result__))
    f(void);
    
    prior to this patch, we would incorrectly diagnose this with:
    
    warning: 'warn_unused_result' attribute only applies to function types
    [-Wattributes]
    
    which suggests we were incorrectly applying the attribute to the return
    type instead of the function type. On closer inspection, the
    existing code in grokdeclarator which handles applying the
    deprecated capability attribute seems to be the cause of the problem.
    
    For the testcase above, in the C FE's grokdeclarator, we see the
    declarator structure:
    
      cdk_pointer -> cdk_attrs -> cdk_function -> cdk_id
    
    now in the main loop of grokdeclarator, we first try to apply the
    warn_unused_result attribute to the pointer type when processing the
    cdk_attrs. The cdk_attrs handling sets up attr_flags according to the
    inner declarator, in particular it has:
    
      else if (inner_decl->kind == cdk_function)
        attr_flags |= (int) ATTR_FLAG_FUNCTION_NEXT;
    
    and these flags then get passed down to decl_attributes. The
    warn_unused_result attribute spec is marked with function_type_required.
    In the case where we know the inner declarator is a function, there is
    some logic in decl_attributes to avoid emitting a diagnostic:
    
      else if (flags & (int) ATTR_FLAG_FUNCTION_NEXT)
        {
          /* Pass on this attribute to be tried again.  */
          tree attr = tree_cons (name, args, NULL_TREE);
          returned_attrs = chainon (returned_attrs, attr);
          continue;
        }
    
    so we just return the attribute to be applied later in this case. Later
    on in the flow of grokdeclarator, we try applying the (prefix)
    capability attribute to the pointer type. In this case, we add the
    "cheri capability" attribute to the existing list of returned_attrs
    (which contains warn_unused_result) and try applying the two together.
    However, since we fail to set any attribute flags here, decl_attributes
    emits the diagnostic shown above.
    
    There seem to be two options to fix this:
    1. Extend the capability attribute handling to inspect the inner
       declarator and set the attribute flags appropriately.
    2. Make the capability attribute handling entirely independent of
       returned_attrs.
    
    It seems that option 1 would work, but would make the code unnecessarily
    complicated. In fact, there appears to be no need for the deprecated
    capability attribute handling to involve returned_attrs at all, since:
     - The capability attribute should always apply successfully (it should
       never get returned by decl_attributes).
     - returned_attrs should always get applied eventually by the
       pre-existing code.
    
    Hence, in this patch, we adjust the capability attribute handling to be
    entirely independent of returned_attrs. We assert that decl_attributes
    never rejects the capability attribute, and we only ever try to apply
    the capability attribute in isolation.
    
    As well as testing that the bogus warning isn't emitted, we check that
    the warn_unused_result attribute gets properly applied in the above case
    by triggering -Wunused-result in the updated test.

Diff:
---
 gcc/c/c-decl.c                                                    | 5 ++---
 gcc/cp/decl.c                                                     | 5 ++---
 .../gcc.target/aarch64/morello/capability_keyword_warnings.c      | 8 ++++++++
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 23aebda70de..156914a9cd1 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -6364,9 +6364,8 @@ grokdeclarator (const struct c_declarator *declarator,
 	      if (!capability_type_p (type))
 		{
 		  tree attr_name = get_identifier ("cheri capability");
-		  tree new_attrs = tree_cons (attr_name, NULL_TREE,
-					  returned_attrs);
-		  returned_attrs = decl_attributes (&type, new_attrs, 0);
+		  tree new_attrs = build_tree_list (attr_name, NULL_TREE);
+		  gcc_assert (!decl_attributes (&type, new_attrs, 0));
 		}
 	      deprecated_capability_uses = 1;
 
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index cda5ef2a0a8..95d22cad945 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -12013,9 +12013,8 @@ grokdeclarator (const cp_declarator *declarator,
 	  if (!capability_type_p (type))
 	    {
 	      tree attr_name = get_identifier ("cheri capability");
-	      tree to_apply = tree_cons (attr_name, NULL_TREE,
-					 returned_attrs);
-	      returned_attrs = decl_attributes (&type, to_apply, 0);
+	      tree to_apply = build_tree_list (attr_name, NULL_TREE);
+	      gcc_assert (!decl_attributes (&type, to_apply, 0));
 	    }
 
 	  /* Raise the deprecated declaration warning, but only if
diff --git a/gcc/testsuite/gcc.target/aarch64/morello/capability_keyword_warnings.c b/gcc/testsuite/gcc.target/aarch64/morello/capability_keyword_warnings.c
index 967b8c8fe0c..9b53efe6d0d 100644
--- a/gcc/testsuite/gcc.target/aarch64/morello/capability_keyword_warnings.c
+++ b/gcc/testsuite/gcc.target/aarch64/morello/capability_keyword_warnings.c
@@ -45,3 +45,11 @@ struct cheri_object4
 {
   void *var17, __attribute((used)) *var18; /* { dg-error "" } */
 };/* { dg-warning "" } */
+
+void __capability * __attribute__((__warn_unused_result__))
+f17 (void); /* { dg-warning "use of '__capability' before the pointer type is deprecated" } */
+
+void f18 (void)
+{
+  f17 (); /* { dg-warning "ignoring return value" } */
+}