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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits