[clang] thread-safety: Support the new capability-based names for all related attributes. (PR #99919)

via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 22 12:08:30 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-analysis

@llvm/pr-subscribers-clang

Author: Pierre d'Herbemont (pdherbemont)

<details>
<summary>Changes</summary>

For some reason some attributes weren't renamed in code but were
documented with the new names. I am not sure why this was the case and
maybe I am missing something.

Those are:
- scoped_lockable -> scoped_capability
- lock_returned -> capability_returned
- locks_excluded -> capabilities_excluded

This patch makes it so that we support the new name as documented. And
change the test to support the old and new name like for other
attributes.


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


12 Files Affected:

- (modified) clang/docs/ThreadSafetyAnalysis.rst (+3-3) 
- (modified) clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h (+1-1) 
- (modified) clang/include/clang/Basic/Attr.td (+9-6) 
- (modified) clang/lib/AST/ASTImporter.cpp (+4-4) 
- (modified) clang/lib/Analysis/ThreadSafety.cpp (+6-6) 
- (modified) clang/lib/Analysis/ThreadSafetyCommon.cpp (+2-2) 
- (modified) clang/lib/Sema/SemaDeclAttr.cpp (+11-9) 
- (modified) clang/lib/Sema/SemaDeclCXX.cpp (+2-2) 
- (modified) clang/test/Misc/pragma-attribute-supported-attributes-list.test (+1-1) 
- (modified) clang/test/Parser/cxx11-stmt-attributes.cpp (+4) 
- (modified) clang/test/SemaCXX/thread-safety-annotations.h (+6-3) 
- (modified) clang/unittests/AST/ASTImporterTest.cpp (+12-12) 


``````````diff
diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst
index cc4089b97b492..995d29ffb6ef0 100644
--- a/clang/docs/ThreadSafetyAnalysis.rst
+++ b/clang/docs/ThreadSafetyAnalysis.rst
@@ -801,7 +801,7 @@ implementation.
     THREAD_ANNOTATION_ATTRIBUTE__(capability(x))
 
   #define SCOPED_CAPABILITY \
-    THREAD_ANNOTATION_ATTRIBUTE__(scoped_lockable)
+    THREAD_ANNOTATION_ATTRIBUTE__(scoped_capability)
 
   #define GUARDED_BY(x) \
     THREAD_ANNOTATION_ATTRIBUTE__(guarded_by(x))
@@ -843,7 +843,7 @@ implementation.
     THREAD_ANNOTATION_ATTRIBUTE__(try_acquire_shared_capability(__VA_ARGS__))
 
   #define EXCLUDES(...) \
-    THREAD_ANNOTATION_ATTRIBUTE__(locks_excluded(__VA_ARGS__))
+    THREAD_ANNOTATION_ATTRIBUTE__(capabilities_excluded(__VA_ARGS__))
 
   #define ASSERT_CAPABILITY(x) \
     THREAD_ANNOTATION_ATTRIBUTE__(assert_capability(x))
@@ -852,7 +852,7 @@ implementation.
     THREAD_ANNOTATION_ATTRIBUTE__(assert_shared_capability(x))
 
   #define RETURN_CAPABILITY(x) \
-    THREAD_ANNOTATION_ATTRIBUTE__(lock_returned(x))
+    THREAD_ANNOTATION_ATTRIBUTE__(capability_returned(x))
 
   #define NO_THREAD_SAFETY_ANALYSIS \
     THREAD_ANNOTATION_ATTRIBUTE__(no_thread_safety_analysis)
diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
index e99c5b2466334..b76188cbe7b74 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
@@ -346,7 +346,7 @@ class SExprBuilder {
   /// the appropriate mutex expression in the lexical context where the function
   /// is called.  PrevCtx holds the context in which the arguments themselves
   /// should be evaluated; multiple calling contexts can be chained together
-  /// by the lock_returned attribute.
+  /// by the capability_returned attribute.
   struct CallingContext {
     // The previous context; or 0 if none.
     CallingContext  *Prev;
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 4825979a974d2..cfa51d3ebca3d 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -3583,8 +3583,9 @@ def Lockable : InheritableAttr {
   let ASTNode = 0;  // Replaced by Capability
 }
 
-def ScopedLockable : InheritableAttr {
-  let Spellings = [Clang<"scoped_lockable", 0>];
+def ScopedCapability : InheritableAttr {
+  let Spellings = [Clang<"scoped_capability", 0>,
+                   Clang<"scoped_lockable", 0>];
   let Subjects = SubjectList<[Record]>;
   let Documentation = [Undocumented];
   let SimpleHandler = 1;
@@ -3779,8 +3780,9 @@ def SharedTrylockFunction : InheritableAttr {
   let Documentation = [Undocumented];
 }
 
-def LockReturned : InheritableAttr {
-  let Spellings = [GNU<"lock_returned">];
+def CapabilityReturned : InheritableAttr {
+  let Spellings = [GNU<"capability_returned">,
+                   GNU<"lock_returned">];
   let Args = [ExprArgument<"Arg">];
   let LateParsed = LateAttrParseStandard;
   let TemplateDependent = 1;
@@ -3789,8 +3791,9 @@ def LockReturned : InheritableAttr {
   let Documentation = [Undocumented];
 }
 
-def LocksExcluded : InheritableAttr {
-  let Spellings = [GNU<"locks_excluded">];
+def CapabilitiesExcluded : InheritableAttr {
+  let Spellings = [GNU<"capabilities_excluded">,
+                   GNU<"locks_excluded">];
   let Args = [VariadicExprArgument<"Args">];
   let LateParsed = LateAttrParseStandard;
   let TemplateDependent = 1;
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 0c27f6f5df2da..51cf6ea427eba 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -9359,13 +9359,13 @@ Expected<Attr *> ASTImporter::Import(const Attr *FromAttr) {
                   From->args_size());
     break;
   }
-  case attr::LockReturned: {
-    const auto *From = cast<LockReturnedAttr>(FromAttr);
+  case attr::CapabilityReturned: {
+    const auto *From = cast<CapabilityReturnedAttr>(FromAttr);
     AI.importAttr(From, AI.importArg(From->getArg()).value());
     break;
   }
-  case attr::LocksExcluded: {
-    const auto *From = cast<LocksExcludedAttr>(FromAttr);
+  case attr::CapabilitiesExcluded: {
+    const auto *From = cast<CapabilitiesExcludedAttr>(FromAttr);
     AI.importAttr(From,
                   AI.importArrayArg(From->args(), From->args_size()).value(),
                   From->args_size());
diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index e25b843c9bf83..46b58c415a795 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -887,7 +887,7 @@ class LockableFactEntry : public FactEntry {
   }
 };
 
-class ScopedLockableFactEntry : public FactEntry {
+class ScopedCapabilityFactEntry : public FactEntry {
 private:
   enum UnderlyingCapabilityKind {
     UCK_Acquired,          ///< Any kind of acquired capability.
@@ -903,7 +903,7 @@ class ScopedLockableFactEntry : public FactEntry {
   SmallVector<UnderlyingCapability, 2> UnderlyingMutexes;
 
 public:
-  ScopedLockableFactEntry(const CapabilityExpr &CE, SourceLocation Loc)
+  ScopedCapabilityFactEntry(const CapabilityExpr &CE, SourceLocation Loc)
       : FactEntry(CE, LK_Exclusive, Loc, Acquired) {}
 
   void addLock(const CapabilityExpr &M) {
@@ -1812,7 +1812,7 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
       assert(inserted.second && "Are we visiting the same expression again?");
       if (isa<CXXConstructExpr>(Exp))
         Self = Placeholder.first;
-      if (TagT->getDecl()->hasAttr<ScopedLockableAttr>())
+      if (TagT->getDecl()->hasAttr<ScopedCapabilityAttr>())
         Scp = CapabilityExpr(Placeholder.first, Placeholder.second, false);
     }
 
@@ -1896,8 +1896,8 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
         break;
       }
 
-      case attr::LocksExcluded: {
-        const auto *A = cast<LocksExcludedAttr>(At);
+      case attr::CapabilitiesExcluded: {
+        const auto *A = cast<CapabilitiesExcludedAttr>(At);
         for (auto *Arg : A->args()) {
           Analyzer->warnIfMutexHeld(FSet, D, Exp, Arg, Self, Loc);
           // use for deferring a lock
@@ -1935,7 +1935,7 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
 
   if (!Scp.shouldIgnore()) {
     // Add the managing object as a dummy mutex, mapped to the underlying mutex.
-    auto ScopedEntry = std::make_unique<ScopedLockableFactEntry>(Scp, Loc);
+    auto ScopedEntry = std::make_unique<ScopedCapabilityFactEntry>(Scp, Loc);
     for (const auto &M : ExclusiveLocksToAdd)
       ScopedEntry->addLock(M);
     for (const auto &M : SharedLocksToAdd)
diff --git a/clang/lib/Analysis/ThreadSafetyCommon.cpp b/clang/lib/Analysis/ThreadSafetyCommon.cpp
index 3e8c959ccee4f..bd7bca0916345 100644
--- a/clang/lib/Analysis/ThreadSafetyCommon.cpp
+++ b/clang/lib/Analysis/ThreadSafetyCommon.cpp
@@ -420,10 +420,10 @@ til::SExpr *SExprBuilder::translateCallExpr(const CallExpr *CE,
                                             CallingContext *Ctx,
                                             const Expr *SelfE) {
   if (CapabilityExprMode) {
-    // Handle LOCK_RETURNED
+    // Handle CAPABILITY_RETURNED
     if (const FunctionDecl *FD = CE->getDirectCallee()) {
       FD = FD->getMostRecentDecl();
-      if (LockReturnedAttr *At = FD->getAttr<LockReturnedAttr>()) {
+      if (CapabilityReturnedAttr *At = FD->getAttr<CapabilityReturnedAttr>()) {
         CallingContext LRCallCtx(Ctx);
         LRCallCtx.AttrDecl = CE->getDirectCallee();
         LRCallCtx.SelfArg = SelfE;
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 5fd8622c90dd8..4f9df1ed019be 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -339,7 +339,7 @@ static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D,
       const CXXRecordDecl *RD = MD->getParent();
       // FIXME -- need to check this again on template instantiation
       if (!checkRecordDeclForAttr<CapabilityAttr>(RD) &&
-          !checkRecordDeclForAttr<ScopedLockableAttr>(RD))
+          !checkRecordDeclForAttr<ScopedCapabilityAttr>(RD))
         S.Diag(AL.getLoc(),
                diag::warn_thread_attribute_not_on_capability_member)
             << AL << MD->getParent();
@@ -633,7 +633,8 @@ static void handleExclusiveTrylockFunctionAttr(Sema &S, Decl *D,
       S.Context, AL, AL.getArgAsExpr(0), Args.data(), Args.size()));
 }
 
-static void handleLockReturnedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+static void handleCapabilityReturnedAttr(Sema &S, Decl *D,
+                                         const ParsedAttr &AL) {
   // check that the argument is lockable object
   SmallVector<Expr*, 1> Args;
   checkAttrArgsAreCapabilityObjs(S, D, AL, Args);
@@ -641,10 +642,11 @@ static void handleLockReturnedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   if (Size == 0)
     return;
 
-  D->addAttr(::new (S.Context) LockReturnedAttr(S.Context, AL, Args[0]));
+  D->addAttr(::new (S.Context) CapabilityReturnedAttr(S.Context, AL, Args[0]));
 }
 
-static void handleLocksExcludedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+static void handleCapabilitiesExcludedAttr(Sema &S, Decl *D,
+                                           const ParsedAttr &AL) {
   if (!AL.checkAtLeastNumArgs(S, 1))
     return;
 
@@ -657,7 +659,7 @@ static void handleLocksExcludedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   Expr **StartArg = &Args[0];
 
   D->addAttr(::new (S.Context)
-                 LocksExcludedAttr(S.Context, AL, StartArg, Size));
+                 CapabilitiesExcludedAttr(S.Context, AL, StartArg, Size));
 }
 
 static bool checkFunctionConditionAttr(Sema &S, Decl *D, const ParsedAttr &AL,
@@ -6927,11 +6929,11 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
   case ParsedAttr::AT_ExclusiveTrylockFunction:
     handleExclusiveTrylockFunctionAttr(S, D, AL);
     break;
-  case ParsedAttr::AT_LockReturned:
-    handleLockReturnedAttr(S, D, AL);
+  case ParsedAttr::AT_CapabilityReturned:
+    handleCapabilityReturnedAttr(S, D, AL);
     break;
-  case ParsedAttr::AT_LocksExcluded:
-    handleLocksExcludedAttr(S, D, AL);
+  case ParsedAttr::AT_CapabilitiesExcluded:
+    handleCapabilitiesExcludedAttr(S, D, AL);
     break;
   case ParsedAttr::AT_SharedTrylockFunction:
     handleSharedTrylockFunctionAttr(S, D, AL);
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 04b8d88cae217..8d059037f592c 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18792,9 +18792,9 @@ bool Sema::checkThisInStaticMemberFunctionAttributes(CXXMethodDecl *Method) {
     } else if (const auto *STLF = dyn_cast<SharedTrylockFunctionAttr>(A)) {
       Arg = STLF->getSuccessValue();
       Args = llvm::ArrayRef(STLF->args_begin(), STLF->args_size());
-    } else if (const auto *LR = dyn_cast<LockReturnedAttr>(A))
+    } else if (const auto *LR = dyn_cast<CapabilityReturnedAttr>(A))
       Arg = LR->getArg();
-    else if (const auto *LE = dyn_cast<LocksExcludedAttr>(A))
+    else if (const auto *LE = dyn_cast<CapabilitiesExcludedAttr>(A))
       Args = llvm::ArrayRef(LE->args_begin(), LE->args_size());
     else if (const auto *RC = dyn_cast<RequiresCapabilityAttr>(A))
       Args = llvm::ArrayRef(RC->args_begin(), RC->args_size());
diff --git a/clang/test/Misc/pragma-attribute-supported-attributes-list.test b/clang/test/Misc/pragma-attribute-supported-attributes-list.test
index 33f9c2f51363c..c5752a34a344c 100644
--- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ b/clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -176,7 +176,7 @@
 // CHECK-NEXT: ReturnsNonNull (SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
 // CHECK-NEXT: SYCLSpecialClass (SubjectMatchRule_record)
-// CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)
+// CHECK-NEXT: ScopedCapability (SubjectMatchRule_record)
 // CHECK-NEXT: Section (SubjectMatchRule_function, SubjectMatchRule_variable_is_global, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property)
 // CHECK-NEXT: SetTypestate (SubjectMatchRule_function_is_member)
 // CHECK-NEXT: SpeculativeLoadHardening (SubjectMatchRule_function, SubjectMatchRule_objc_method)
diff --git a/clang/test/Parser/cxx11-stmt-attributes.cpp b/clang/test/Parser/cxx11-stmt-attributes.cpp
index 75fb37ea9fb44..46454889c423b 100644
--- a/clang/test/Parser/cxx11-stmt-attributes.cpp
+++ b/clang/test/Parser/cxx11-stmt-attributes.cpp
@@ -52,6 +52,10 @@ void foo(int i) {
   } catch (...) {
   }
 
+  [[capability_returned]] try { // expected-warning {{unknown attribute 'capability_returned' ignored}}
+  } catch (...) {
+  }
+
   [[weakref]] return; // expected-warning {{unknown attribute 'weakref' ignored}}
 
   [[carries_dependency]] ; // expected-error {{'carries_dependency' attribute cannot be applied to a statement}}
diff --git a/clang/test/SemaCXX/thread-safety-annotations.h b/clang/test/SemaCXX/thread-safety-annotations.h
index d89bcf8ff4706..8073398cdaaac 100644
--- a/clang/test/SemaCXX/thread-safety-annotations.h
+++ b/clang/test/SemaCXX/thread-safety-annotations.h
@@ -11,6 +11,9 @@
 #define SHARED_TRYLOCK_FUNCTION(...)    __attribute__((try_acquire_shared_capability(__VA_ARGS__)))
 #define EXCLUSIVE_LOCKS_REQUIRED(...)   __attribute__((requires_capability(__VA_ARGS__)))
 #define SHARED_LOCKS_REQUIRED(...)      __attribute__((requires_shared_capability(__VA_ARGS__)))
+#define SCOPED_LOCKABLE                 __attribute__((scoped_capability))
+#define LOCK_RETURNED(x)                __attribute__((capability_returned(x)))
+#define LOCKS_EXCLUDED(...)             __attribute__((capabilities_excluded(__VA_ARGS__)))
 #else
 #define LOCKABLE                        __attribute__((lockable))
 #define ASSERT_EXCLUSIVE_LOCK(...)      __attribute__((assert_exclusive_lock(__VA_ARGS__)))
@@ -22,6 +25,9 @@
 #define SHARED_TRYLOCK_FUNCTION(...)    __attribute__((shared_trylock_function(__VA_ARGS__)))
 #define EXCLUSIVE_LOCKS_REQUIRED(...)   __attribute__((exclusive_locks_required(__VA_ARGS__)))
 #define SHARED_LOCKS_REQUIRED(...)      __attribute__((shared_locks_required(__VA_ARGS__)))
+#define SCOPED_LOCKABLE                 __attribute__((scoped_lockable))
+#define LOCK_RETURNED(x)                __attribute__((lock_returned(x)))
+#define LOCKS_EXCLUDED(...)             __attribute__((locks_excluded(__VA_ARGS__)))
 #endif
 
 // Lock semantics only
@@ -35,9 +41,6 @@
 #define PT_GUARDED_BY(x)                __attribute__((pt_guarded_by(x)))
 
 // Common
-#define SCOPED_LOCKABLE                 __attribute__((scoped_lockable))
 #define ACQUIRED_AFTER(...)             __attribute__((acquired_after(__VA_ARGS__)))
 #define ACQUIRED_BEFORE(...)            __attribute__((acquired_before(__VA_ARGS__)))
-#define LOCK_RETURNED(x)                __attribute__((lock_returned(x)))
-#define LOCKS_EXCLUDED(...)             __attribute__((locks_excluded(__VA_ARGS__)))
 #define NO_THREAD_SAFETY_ANALYSIS       __attribute__((no_thread_safety_analysis))
diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp
index 92f9bae6cb064..bfa558777c798 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -7819,10 +7819,10 @@ TEST_P(ImportAttributes, ImportPtGuardedVar) {
                       ToAttr);
 }
 
-TEST_P(ImportAttributes, ImportScopedLockable) {
-  ScopedLockableAttr *FromAttr, *ToAttr;
-  importAttr<CXXRecordDecl>("struct __attribute__((scoped_lockable)) test {};",
-                            FromAttr, ToAttr);
+TEST_P(ImportAttributes, ImportScopedCapability) {
+  ScopedCapabilityAttr *FromAttr, *ToAttr;
+  importAttr<CXXRecordDecl>(
+      "struct __attribute__((scoped_capability)) test {};", FromAttr, ToAttr);
 }
 
 TEST_P(ImportAttributes, ImportCapability) {
@@ -7965,19 +7965,19 @@ TEST_P(ImportAttributes, ImportSharedTrylockFunction) {
   checkImportVariadicArg(FromAttr->args(), ToAttr->args());
 }
 
-TEST_P(ImportAttributes, ImportLockReturned) {
-  LockReturnedAttr *FromAttr, *ToAttr;
+TEST_P(ImportAttributes, ImportCapabilityReturned) {
+  CapabilityReturnedAttr *FromAttr, *ToAttr;
   importAttr<FunctionDecl>(
-      "void test(int A1) __attribute__((lock_returned(A1)));", FromAttr,
+      "void test(int A1) __attribute__((capability_returned(A1)));", FromAttr,
       ToAttr);
   checkImported(FromAttr->getArg(), ToAttr->getArg());
 }
 
-TEST_P(ImportAttributes, ImportLocksExcluded) {
-  LocksExcludedAttr *FromAttr, *ToAttr;
-  importAttr<FunctionDecl>(
-      "void test(int A1, int A2) __attribute__((locks_excluded(A1, A2)));",
-      FromAttr, ToAttr);
+TEST_P(ImportAttributes, ImportCapabilitiesExcluded) {
+  CapabilitiesExcludedAttr *FromAttr, *ToAttr;
+  importAttr<FunctionDecl>("void test(int A1, int A2) "
+                           "__attribute__((capabilities_excluded(A1, A2)));",
+                           FromAttr, ToAttr);
   checkImportVariadicArg(FromAttr->args(), ToAttr->args());
 }
 

``````````

</details>


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


More information about the cfe-commits mailing list