[clang] [clang][TSA] Make RequiresCapability a DeclOrType attribute (PR #67095)

[Timm Baeder via cfe-commits]

Timm =?utf-8?q?Bäder?= <tbaeder at redhat.com>
Message-ID:
In-Reply-To: <llvm/llvm-project/pull/67095/clang at github.com>


https://github.com/tbaederr updated https://github.com/llvm/llvm-project/pull/67095

>From 174144b3d2101964a438a22644cbfa4c2314700b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Fri, 22 Sep 2023 08:42:05 +0200
Subject: [PATCH 1/2] [clang][TSA] Make RequiresCapability a DeclOrType
 attribute

---
 clang/include/clang/Basic/Attr.td             |  6 +++---
 clang/test/Sema/warn-thread-safety-analysis.c | 11 +++++++++++
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 21a3b5226623cf2..7eab87dac6921f1 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -3305,7 +3305,7 @@ def ReleaseCapability : InheritableAttr {
   let Documentation = [ReleaseCapabilityDocs];
 }
 
-def RequiresCapability : InheritableAttr {
+def RequiresCapability : DeclOrTypeAttr {
   let Spellings = [Clang<"requires_capability", 0>,
                    Clang<"exclusive_locks_required", 0>,
                    Clang<"requires_shared_capability", 0>,
@@ -3314,8 +3314,8 @@ def RequiresCapability : InheritableAttr {
   let LateParsed = 1;
   let TemplateDependent = 1;
   let ParseArgumentsAsUnevaluated = 1;
-  let InheritEvenIfAlreadyPresent = 1;
-  let Subjects = SubjectList<[Function]>;
+  /*let InheritEvenIfAlreadyPresent = 1;*/
+  /*let Subjects = SubjectList<[Function]>;*/
   let Accessors = [Accessor<"isShared", [Clang<"requires_shared_capability", 0>,
                                          Clang<"shared_locks_required", 0>]>];
   let Documentation = [Undocumented];
diff --git a/clang/test/Sema/warn-thread-safety-analysis.c b/clang/test/Sema/warn-thread-safety-analysis.c
index 642ea88ec3c96f7..cd852efac21cb81 100644
--- a/clang/test/Sema/warn-thread-safety-analysis.c
+++ b/clang/test/Sema/warn-thread-safety-analysis.c
@@ -136,6 +136,17 @@ int main(void) {
     // Cleanup happens automatically -> no warning.
   }
 
+  /// Function pointers
+  {
+    int __attribute__((requires_capability(&mu1))) (*function_ptr)(int) = Foo_fun1;
+
+    function_ptr(5); // expected-warning {{calling function 'function_ptr' requires holding mutex 'mu1'}}
+
+    mutex_exclusive_lock(&mu1);
+    int five = function_ptr(5);
+    mutex_exclusive_unlock(&mu1);
+  }
+
   return 0;
 }
 

>From 5dd9266c4e0b0a02cbb02a29bf2606224ccf2820 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Tue, 26 Sep 2023 09:57:41 +0200
Subject: [PATCH 2/2] Restrict to functions and function pointer decls

---
 clang/include/clang/Basic/Attr.td |  2 +-
 clang/lib/Sema/SemaDeclAttr.cpp   | 10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 7eab87dac6921f1..dd2625aaabf16bd 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -3314,7 +3314,7 @@ def RequiresCapability : DeclOrTypeAttr {
   let LateParsed = 1;
   let TemplateDependent = 1;
   let ParseArgumentsAsUnevaluated = 1;
-  /*let InheritEvenIfAlreadyPresent = 1;*/
+  let InheritEvenIfAlreadyPresent = 1;
   /*let Subjects = SubjectList<[Function]>;*/
   let Accessors = [Accessor<"isShared", [Clang<"requires_shared_capability", 0>,
                                          Clang<"shared_locks_required", 0>]>];
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 090a54eedaa07d0..7499b3dfc4b2995 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -8141,6 +8141,16 @@ static void handleRequiresCapabilityAttr(Sema &S, Decl *D,
   if (!AL.checkAtLeastNumArgs(S, 1))
     return;
 
+  // We allow this on function declaration as well as
+  // variable declarations of function pointer type.
+  if (!D->isFunctionPointerType() && !isa<FunctionDecl>(D)) {
+    // FIXME: Diagnostic should say "functions and function pointer decls" now I
+    // guess.
+    S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
+        << AL << AL.isRegularKeywordAttribute() << ExpectedFunction;
+    return;
+  }
+
   // check that all arguments are lockable objects
   SmallVector<Expr*, 1> Args;
   checkAttrArgsAreCapabilityObjs(S, D, AL, Args);