[clang] thread-safety: Support the new capability-based names for all related attributes. (PR #99919)
Pierre d'Herbemont via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 22 12:08:02 PDT 2024
https://github.com/pdherbemont created https://github.com/llvm/llvm-project/pull/99919
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.
>From 7454358c9a7a2e3e182175d5cf9d4e3792c81764 Mon Sep 17 00:00:00 2001
From: Pierre d'Herbemont <pdherbemont at apple.com>
Date: Mon, 22 Jul 2024 16:22:28 +0200
Subject: [PATCH] thread-safety: Support the new capability-based names for all
related attributes.
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.
---
clang/docs/ThreadSafetyAnalysis.rst | 6 ++---
.../Analysis/Analyses/ThreadSafetyCommon.h | 2 +-
clang/include/clang/Basic/Attr.td | 15 +++++++-----
clang/lib/AST/ASTImporter.cpp | 8 +++----
clang/lib/Analysis/ThreadSafety.cpp | 12 +++++-----
clang/lib/Analysis/ThreadSafetyCommon.cpp | 4 ++--
clang/lib/Sema/SemaDeclAttr.cpp | 20 +++++++++-------
clang/lib/Sema/SemaDeclCXX.cpp | 4 ++--
...a-attribute-supported-attributes-list.test | 2 +-
clang/test/Parser/cxx11-stmt-attributes.cpp | 4 ++++
.../test/SemaCXX/thread-safety-annotations.h | 9 ++++---
clang/unittests/AST/ASTImporterTest.cpp | 24 +++++++++----------
12 files changed, 61 insertions(+), 49 deletions(-)
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());
}
More information about the cfe-commits
mailing list