[clang] [clang][ptrauth] reject incorrectly placed ptrauth qualifier (PR #138376)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Oliver Hunt (ojhunt)

<details>
<summary>Changes</summary>

When parsing a function pointer typed field we use Parser::ParseTypeQualifierListOpt, but then drops subsequent type attributes and qualifiers unless explicitly handled.

There is an existing solution for the _Atomic qualifier, and this PR simply extends that to __ptrauth for now. In future we may want to investigate a more robust mechanism to ensure type qualifiers are not silently dropped so that future type qualifiers do not suffer the same problem.

---
Full diff: https://github.com/llvm/llvm-project/pull/138376.diff


4 Files Affected:

- (modified) clang/include/clang/Parse/Parser.h (+1-1) 
- (modified) clang/lib/Parse/ParseDecl.cpp (+7-4) 
- (modified) clang/lib/Parse/ParseDeclCXX.cpp (+1-1) 
- (added) clang/test/Sema/ptrauth-struct-function-ptr-field.c (+7) 


``````````diff
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 00e4b980bf44a..46a2d26beb7f9 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -3253,7 +3253,7 @@ class Parser : public CodeCompletionHandler {
 
   void ParseTypeQualifierListOpt(
       DeclSpec &DS, unsigned AttrReqs = AR_AllAttributesParsed,
-      bool AtomicAllowed = true, bool IdentifierRequired = false,
+      bool AtomicOrPtrauthAllowed = true, bool IdentifierRequired = false,
       std::optional<llvm::function_ref<void()>> CodeCompletionHandler =
           std::nullopt);
   void ParseDirectDeclarator(Declarator &D);
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 4cfb8de615f05..4fe3565687905 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -6511,7 +6511,7 @@ bool Parser::isConstructorDeclarator(bool IsUnqualified, bool DeductionGuide,
 /// Note: vendor can be GNU, MS, etc and can be explicitly controlled via
 /// AttrRequirements bitmask values.
 void Parser::ParseTypeQualifierListOpt(
-    DeclSpec &DS, unsigned AttrReqs, bool AtomicAllowed,
+    DeclSpec &DS, unsigned AttrReqs, bool AtomicOrPtrauthAllowed,
     bool IdentifierRequired,
     std::optional<llvm::function_ref<void()>> CodeCompletionHandler) {
   if ((AttrReqs & AR_CXX11AttributesParsed) &&
@@ -6551,7 +6551,7 @@ void Parser::ParseTypeQualifierListOpt(
                                  getLangOpts());
       break;
     case tok::kw__Atomic:
-      if (!AtomicAllowed)
+      if (!AtomicOrPtrauthAllowed)
         goto DoneWithTypeQuals;
       diagnoseUseOfC11Keyword(Tok);
       isInvalid = DS.SetTypeQual(DeclSpec::TQ_atomic, Loc, PrevSpec, DiagID,
@@ -6584,6 +6584,8 @@ void Parser::ParseTypeQualifierListOpt(
 
     // __ptrauth qualifier.
     case tok::kw___ptrauth:
+      if (!AtomicOrPtrauthAllowed)
+        goto DoneWithTypeQuals;
       ParsePtrauthQualifier(DS.getAttributes());
       EndLoc = PrevTokLocation;
       continue;
@@ -6860,7 +6862,8 @@ void Parser::ParseDeclaratorInternal(Declarator &D,
                     ((D.getContext() != DeclaratorContext::CXXNew)
                          ? AR_GNUAttributesParsed
                          : AR_GNUAttributesParsedAndRejected);
-    ParseTypeQualifierListOpt(DS, Reqs, true, !D.mayOmitIdentifier());
+    ParseTypeQualifierListOpt(DS, Reqs, /*AtomicOrPtrauthAllowed=*/true,
+                              !D.mayOmitIdentifier());
     D.ExtendWithDeclSpec(DS);
 
     // Recursively parse the declarator.
@@ -7725,7 +7728,7 @@ void Parser::ParseFunctionDeclarator(Declarator &D,
       // Parse cv-qualifier-seq[opt].
       ParseTypeQualifierListOpt(
           DS, AR_NoAttributesParsed,
-          /*AtomicAllowed*/ false,
+          /*AtomicOrPtrauthAllowed=*/false,
           /*IdentifierRequired=*/false, llvm::function_ref<void()>([&]() {
             Actions.CodeCompletion().CodeCompleteFunctionQualifiers(DS, D);
           }));
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 7e0a8af07a3be..6ee2cd31311ef 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -2802,7 +2802,7 @@ void Parser::MaybeParseAndDiagnoseDeclSpecAfterCXX11VirtSpecifierSeq(
   // GNU-style and C++11 attributes are not allowed here, but they will be
   // handled by the caller.  Diagnose everything else.
   ParseTypeQualifierListOpt(
-      DS, AR_NoAttributesParsed, false,
+      DS, AR_NoAttributesParsed, /*AtomicOrPtrauthAllowed=*/false,
       /*IdentifierRequired=*/false, llvm::function_ref<void()>([&]() {
         Actions.CodeCompletion().CodeCompleteFunctionQualifiers(DS, D, &VS);
       }));
diff --git a/clang/test/Sema/ptrauth-struct-function-ptr-field.c b/clang/test/Sema/ptrauth-struct-function-ptr-field.c
new file mode 100644
index 0000000000000..f9eccd559c37d
--- /dev/null
+++ b/clang/test/Sema/ptrauth-struct-function-ptr-field.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios -x c   -fsyntax-only -verify -fptrauth-intrinsics %s -fexperimental-new-constant-interpreter
+// RUN: %clang_cc1 -triple arm64-apple-ios -x c++ -fsyntax-only -verify -fptrauth-intrinsics %s -fexperimental-new-constant-interpreter
+
+struct Foo {
+  void (*f)(int) __ptrauth(1,1,1);
+  // expected-error at -1 {{expected ';' at end of declaration list}}
+};

``````````

</details>


https://github.com/llvm/llvm-project/pull/138376