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

Timm Baeder via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 1 08:19:15 PDT 2024


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


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

>From 1c117257921fc1c246fbb9f51e3c95d6dc6d295e 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 1/6] 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 0866b09bab2995..169eef811f5793 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 64e6d0407b0ce3..f218a82122c881 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4031,7 +4031,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 5577f45aa5217f..25c63f130ca75e 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -2263,6 +2263,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
@@ -2282,6 +2313,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 6496a33b8f5a50..fb4f97096fb962 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;
   }

>From f3ccd5cc229ba4d6476efeb04701ade4c0d844e5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Mon, 30 Sep 2024 13:13:06 +0200
Subject: [PATCH 2/6] Revert "Warn on RequiresCapability attribute mismatch"

This reverts commit 5f411735a5366499481c09a317aa170af44796f3.
---
 .../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, 1 insertion(+), 48 deletions(-)

diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafety.h b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
index 169eef811f5793..0866b09bab2995 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafety.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
@@ -26,7 +26,6 @@ namespace clang {
 class AnalysisDeclContext;
 class FunctionDecl;
 class NamedDecl;
-class Attr;
 
 namespace threadSafety {
 
@@ -231,9 +230,6 @@ 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 f218a82122c881..64e6d0407b0ce3 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4031,9 +4031,7 @@ 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 25c63f130ca75e..5577f45aa5217f 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -2263,37 +2263,6 @@ 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
@@ -2313,9 +2282,6 @@ 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 fb4f97096fb962..6496a33b8f5a50 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2073,13 +2073,6 @@ 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;
   }

>From 804ed367fc75af0c2648e99335040151b206a051 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Mon, 30 Sep 2024 14:48:58 +0200
Subject: [PATCH 3/6] Warn on RequiresCapability attribute mismatches between
 declarations

---
 .../clang/Basic/DiagnosticSemaKinds.td        |  3 +++
 clang/lib/Sema/SemaDeclAttr.cpp               | 24 +++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 64e6d0407b0ce3..1bf13eb2d9b90e 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4031,6 +4031,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 declarations of %0">,
+  InGroup<ThreadSafetyAttributes>, DefaultIgnore;
 
 
 // Thread safety warnings negative capabilities
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index c9b9f3a0007daa..98147101487ec4 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -5794,6 +5794,30 @@ static void handleRequiresCapabilityAttr(Sema &S, Decl *D,
       RequiresCapabilityAttr(S.Context, AL, Args.data(), Args.size());
 
   D->addAttr(RCA);
+
+  if (const auto *FD = dyn_cast<FunctionDecl>(D)) {
+
+    auto collectExprs = [](const FunctionDecl *FuncDecl) {
+      std::set<const ValueDecl*> Args;
+      for (const auto *A : FuncDecl->specific_attrs<RequiresCapabilityAttr>()) {
+        for (const Expr *E : A->args()) {
+          if (const auto *DRE = dyn_cast<DeclRefExpr>(E))
+            Args.insert(DRE->getDecl());
+        }
+      }
+      return Args;
+    };
+    auto ThisDecl = collectExprs(FD);
+    for (const FunctionDecl *P = FD->getPreviousDecl(); P;
+         P = P->getPreviousDecl()) {
+      auto PrevDecl = collectExprs(P);
+      // FIXME: It would be nice to mention _what_ attribute isn't matched and maybe
+      // even where the previous declaration was?
+      if (ThisDecl.size() != PrevDecl.size())
+        S.Diag(D->getLocation(), diag::warn_attribute_mismatch) << FD;
+    }
+
+  }
 }
 
 static void handleDeprecatedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {

>From f59156611313793fba190f88262be52bcc88370c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Tue, 1 Oct 2024 15:44:19 +0200
Subject: [PATCH 4/6] Revert "Warn on RequiresCapability attribute mismatches
 between declarations"

This reverts commit 804ed367fc75af0c2648e99335040151b206a051.
---
 .../clang/Basic/DiagnosticSemaKinds.td        |  3 ---
 clang/lib/Sema/SemaDeclAttr.cpp               | 24 -------------------
 2 files changed, 27 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 1bf13eb2d9b90e..64e6d0407b0ce3 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4031,9 +4031,6 @@ 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 declarations of %0">,
-  InGroup<ThreadSafetyAttributes>, DefaultIgnore;
 
 
 // Thread safety warnings negative capabilities
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 98147101487ec4..c9b9f3a0007daa 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -5794,30 +5794,6 @@ static void handleRequiresCapabilityAttr(Sema &S, Decl *D,
       RequiresCapabilityAttr(S.Context, AL, Args.data(), Args.size());
 
   D->addAttr(RCA);
-
-  if (const auto *FD = dyn_cast<FunctionDecl>(D)) {
-
-    auto collectExprs = [](const FunctionDecl *FuncDecl) {
-      std::set<const ValueDecl*> Args;
-      for (const auto *A : FuncDecl->specific_attrs<RequiresCapabilityAttr>()) {
-        for (const Expr *E : A->args()) {
-          if (const auto *DRE = dyn_cast<DeclRefExpr>(E))
-            Args.insert(DRE->getDecl());
-        }
-      }
-      return Args;
-    };
-    auto ThisDecl = collectExprs(FD);
-    for (const FunctionDecl *P = FD->getPreviousDecl(); P;
-         P = P->getPreviousDecl()) {
-      auto PrevDecl = collectExprs(P);
-      // FIXME: It would be nice to mention _what_ attribute isn't matched and maybe
-      // even where the previous declaration was?
-      if (ThisDecl.size() != PrevDecl.size())
-        S.Diag(D->getLocation(), diag::warn_attribute_mismatch) << FD;
-    }
-
-  }
 }
 
 static void handleDeprecatedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {

>From cf9728c57e2ffec67a59d748a54f573d5cacac08 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Tue, 1 Oct 2024 15:44:29 +0200
Subject: [PATCH 5/6] Reapply "Warn on RequiresCapability attribute mismatch"

This reverts commit f3ccd5cc229ba4d6476efeb04701ade4c0d844e5.
---
 .../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 0866b09bab2995..169eef811f5793 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 64e6d0407b0ce3..f218a82122c881 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4031,7 +4031,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 5577f45aa5217f..25c63f130ca75e 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -2263,6 +2263,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
@@ -2282,6 +2313,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 6496a33b8f5a50..fb4f97096fb962 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;
   }

>From d78dbaba7274701c2a89cbf87565d0f380eb334a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Tue, 1 Oct 2024 17:18:42 +0200
Subject: [PATCH 6/6] -

---
 clang/lib/Analysis/ThreadSafety.cpp | 43 ++++++++++++-----------------
 1 file changed, 18 insertions(+), 25 deletions(-)

diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index 25c63f130ca75e..01ef18f7eba373 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1073,6 +1073,8 @@ class ThreadSafetyAnalyzer {
                    ProtectedOperationKind POK);
   void checkPtAccess(const FactSet &FSet, const Expr *Exp, AccessKind AK,
                      ProtectedOperationKind POK);
+
+  void checkMismatchedFunctionAttrs(const FunctionDecl *FD);
 };
 
 } // namespace
@@ -2263,34 +2265,25 @@ 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);
-  }
+void ThreadSafetyAnalyzer::checkMismatchedFunctionAttrs(
+    const FunctionDecl *FD) {
+  FD = FD->getMostRecentDecl();
 
-  return Args;
-}
-
-static void diagnoseMismatchedFunctionAttrs(const FunctionDecl *FD,
-                                            ThreadSafetyHandler &Handler) {
-  assert(FD);
-  FD = FD->getDefinition();
-  assert(FD);
-  auto FDArgs = collectAttrArgs<RequiresCapabilityAttr>(FD);
+  auto collectCapabilities = [&](const FunctionDecl *FD) {
+    SmallVector<CapabilityExpr> Args;
+    for (const auto *A : FD->specific_attrs<RequiresCapabilityAttr>()) {
+      for (const Expr *E : A->args())
+        Args.push_back(SxBuilder.translateAttrExpr(E, nullptr));
+    }
+    return Args;
+  };
 
+  auto FDArgs = collectCapabilities(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);
-      }
-    }
+    auto DArgs = collectCapabilities(D);
+    if (DArgs.size() != FDArgs.size())
+      Handler.handleAttributeMismatch(FD, D);
   }
 }
 
@@ -2314,7 +2307,7 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
   CurrentFunction = dyn_cast<FunctionDecl>(D);
 
   if (CurrentFunction)
-    diagnoseMismatchedFunctionAttrs(CurrentFunction, Handler);
+    checkMismatchedFunctionAttrs(CurrentFunction);
 
   if (D->hasAttr<NoThreadSafetyAnalysisAttr>())
     return;



More information about the cfe-commits mailing list