From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 7810) id EBF6D3858412; Tue, 17 Jan 2023 10:47:15 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org EBF6D3858412 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1673952435; bh=bcPTM/D0To6i5hVrpg+nRlNzBSVA9iQaJmoOwUAuPVE=; h=From:To:Subject:Date:From; b=hO2OH9vui8k5UGBr+0LwzZl4rxaX4o2DV7XFAGn3iKaRikovWMZM6XVFyQl/EFCGK TrtquT3TRWRv0V7DbbdrKRPyth3S5mYsE/uggGf5i0xVUvZBaNZoPvAtEuFx6FP7x8 5Ao/HDHezuwr7CeNZE8Fhjt5kSOu5PaOYXGDRcgU= Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: Alex Coplan To: gcc-cvs@gcc.gnu.org Subject: [gcc(refs/vendors/ARM/heads/morello)] c, cp: Fix bad attribute handling with prefix __capability X-Act-Checkin: gcc X-Git-Author: Alex Coplan X-Git-Refname: refs/vendors/ARM/heads/morello X-Git-Oldrev: aa8dde4799379b90301785bf0beff2c20935eacc X-Git-Newrev: 350b66b67e7cac124347bceda754a82c2c440360 Message-Id: <20230117104715.EBF6D3858412@sourceware.org> Date: Tue, 17 Jan 2023 10:47:15 +0000 (GMT) List-Id: https://gcc.gnu.org/g:350b66b67e7cac124347bceda754a82c2c440360 commit 350b66b67e7cac124347bceda754a82c2c440360 Author: Alex Coplan 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" } */ +}