public inbox for gcc-cvs@sourceware.org help / color / mirror / Atom feed
From: Alex Coplan <acoplan@gcc.gnu.org> To: gcc-cvs@gcc.gnu.org Subject: [gcc(refs/vendors/ARM/heads/morello)] c, cp: Fix bad attribute handling with prefix __capability Date: Tue, 17 Jan 2023 10:47:15 +0000 (GMT) [thread overview] Message-ID: <20230117104715.EBF6D3858412@sourceware.org> (raw) 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" } */ +}
reply other threads:[~2023-01-17 10:47 UTC|newest] Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20230117104715.EBF6D3858412@sourceware.org \ --to=acoplan@gcc.gnu.org \ --cc=gcc-cvs@gcc.gnu.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).