[clang] [clang] WIP: Warn on mismatched RequiresCapability attributes (PR #67520)

[Timm Baeder via cfe-commits]

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

>From 800ce47e967593ec149e0187abf6d2cb3ee1b1b5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Thu, 20 Jun 2024 07:39:20 +0200
Subject: [PATCH] Warn on RequiresCapability attribute mismatch

---
 .../clang/Analysis/Analyses/ThreadSafety.h    |  4 +++
 .../clang/Basic/DiagnosticSemaKinds.td        |  4 ++-
 clang/lib/Analysis/ThreadSafety.cpp           | 34 +++++++++++++++++++
 clang/lib/Sema/AnalysisBasedWarnings.cpp      |  7 ++++
 4 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafety.h b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
index 0866b09bab299..169eef811f579 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafety.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
@@ -26,6 +26,7 @@ namespace clang {
 class AnalysisDeclContext;
 class FunctionDecl;
 class NamedDecl;
+class Attr;
 
 namespace threadSafety {
 
@@ -230,6 +231,9 @@ class ThreadSafetyHandler {
   /// Warn that there is a cycle in acquired_before/after dependencies.
   virtual void handleBeforeAfterCycle(Name L1Name, SourceLocation Loc) {}
 
+  virtual void handleAttributeMismatch(const FunctionDecl *FD1,
+                                       const FunctionDecl *FD2) {}
+
   /// Called by the analysis when starting analysis of a function.
   /// Used to issue suggestions for changes to annotations.
   virtual void enterFunction(const FunctionDecl *FD) {}
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 14736784cff5f..4e8e6f66ebc8f 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3978,7 +3978,9 @@ def warn_acquired_before : Warning<
 def warn_acquired_before_after_cycle : Warning<
   "cycle in acquired_before/after dependencies, starting with '%0'">,
   InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
-
+def warn_attribute_mismatch : Warning<
+  "attribute mismatch between function definition and declaration of %0">,
+  InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
 
 // Thread safety warnings negative capabilities
 def warn_acquire_requires_negative_cap : Warning<
diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index e25b843c9bf83..6051a5e9d6bd9 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -2260,6 +2260,37 @@ static bool neverReturns(const CFGBlock *B) {
   return false;
 }
 
+template <typename AttrT>
+static SmallVector<const Expr *> collectAttrArgs(const FunctionDecl *FD) {
+  SmallVector<const Expr *> Args;
+  for (const AttrT *A : FD->specific_attrs<AttrT>()) {
+    for (const Expr *E : A->args())
+      Args.push_back(E);
+  }
+
+  return Args;
+}
+
+static void diagnoseMismatchedFunctionAttrs(const FunctionDecl *FD,
+                                            ThreadSafetyHandler &Handler) {
+  assert(FD);
+  FD = FD->getDefinition();
+  assert(FD);
+  auto FDArgs = collectAttrArgs<RequiresCapabilityAttr>(FD);
+
+  for (const FunctionDecl *D = FD->getPreviousDecl(); D;
+       D = D->getPreviousDecl()) {
+    auto DArgs = collectAttrArgs<RequiresCapabilityAttr>(D);
+
+    for (const Expr *E : FDArgs) {
+      if (!llvm::is_contained(DArgs, E)) {
+        // FD requires E, but D doesn't.
+        Handler.handleAttributeMismatch(FD, D);
+      }
+    }
+  }
+}
+
 /// Check a function's CFG for thread-safety violations.
 ///
 /// We traverse the blocks in the CFG, compute the set of mutexes that are held
@@ -2279,6 +2310,9 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
   const NamedDecl *D = walker.getDecl();
   CurrentFunction = dyn_cast<FunctionDecl>(D);
 
+  if (CurrentFunction)
+    diagnoseMismatchedFunctionAttrs(CurrentFunction, Handler);
+
   if (D->hasAttr<NoThreadSafetyAnalysisAttr>())
     return;
 
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 0f604c61fa3af..02d60a803d77a 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2073,6 +2073,13 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler {
     Warnings.emplace_back(std::move(Warning), getNotes());
   }
 
+  void handleAttributeMismatch(const FunctionDecl *FD1,
+                               const FunctionDecl *FD2) override {
+    PartialDiagnosticAt Warning(FD2->getLocation(),
+                                S.PDiag(diag::warn_attribute_mismatch) << FD1);
+    Warnings.emplace_back(std::move(Warning), getNotes());
+  }
+
   void enterFunction(const FunctionDecl* FD) override {
     CurrentFunction = FD;
   }