aaronpuchert created this revision. aaronpuchert added reviewers: aaron.ballman, delesley. Herald added a project: clang. Herald added a subscriber: cfe-commits.
The standard std::unique_lock can be constructed to manage a lock without initially acquiring it by passing std::defer_lock as second parameter. It can be acquired later by calling lock(). To support this, we use the locks_excluded attribute. This might seem like an odd choice at first, but its consistent with the other annotations we support on scoped capability constructors. By excluding the lock we state that it is currently not in use and the function doesn't change that, which is exactly what the constructor does. Along the way we slightly simplify handling of scoped capabilities. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D81332 Files: clang/lib/Analysis/ThreadSafety.cpp clang/test/SemaCXX/warn-thread-safety-analysis.cpp
Index: clang/test/SemaCXX/warn-thread-safety-analysis.cpp =================================================================== --- clang/test/SemaCXX/warn-thread-safety-analysis.cpp +++ clang/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -2625,9 +2625,12 @@ namespace RelockableScopedLock { +class DeferTraits {}; + class SCOPED_LOCKABLE RelockableExclusiveMutexLock { public: RelockableExclusiveMutexLock(Mutex *mu) EXCLUSIVE_LOCK_FUNCTION(mu); + RelockableExclusiveMutexLock(Mutex *mu, DeferTraits) LOCKS_EXCLUDED(mu); ~RelockableExclusiveMutexLock() EXCLUSIVE_UNLOCK_FUNCTION(); void Lock() EXCLUSIVE_LOCK_FUNCTION(); @@ -2639,6 +2642,7 @@ class SCOPED_LOCKABLE RelockableMutexLock { public: + RelockableMutexLock(Mutex *mu, DeferTraits) LOCKS_EXCLUDED(mu); RelockableMutexLock(Mutex *mu, SharedTraits) SHARED_LOCK_FUNCTION(mu); RelockableMutexLock(Mutex *mu, ExclusiveTraits) EXCLUSIVE_LOCK_FUNCTION(mu); ~RelockableMutexLock() UNLOCK_FUNCTION(); @@ -2669,6 +2673,13 @@ x = 4; } +void deferLock() { + RelockableExclusiveMutexLock scope(&mu, DeferTraits{}); + x = 2; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}} + scope.Lock(); + x = 3; +} + void relockExclusive() { RelockableMutexLock scope(&mu, SharedTraits{}); print(x); @@ -2703,6 +2714,14 @@ x = 5; } +void deferLockShared() { + RelockableMutexLock scope(&mu, DeferTraits{}); + print(x); // expected-warning {{reading variable 'x' requires holding mutex 'mu'}} + scope.ReaderLock(); + print(x); + x = 2; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}} +} + void doubleUnlock() { RelockableExclusiveMutexLock scope(&mu); scope.Unlock(); Index: clang/lib/Analysis/ThreadSafety.cpp =================================================================== --- clang/lib/Analysis/ThreadSafety.cpp +++ clang/lib/Analysis/ThreadSafety.cpp @@ -905,11 +905,7 @@ ScopedLockableFactEntry(const CapabilityExpr &CE, SourceLocation Loc) : FactEntry(CE, LK_Exclusive, Loc, false) {} - void addExclusiveLock(const CapabilityExpr &M) { - UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired); - } - - void addSharedLock(const CapabilityExpr &M) { + void addLock(const CapabilityExpr &M) { UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired); } @@ -1801,7 +1797,7 @@ SourceLocation Loc = Exp->getExprLoc(); CapExprSet ExclusiveLocksToAdd, SharedLocksToAdd; CapExprSet ExclusiveLocksToRemove, SharedLocksToRemove, GenericLocksToRemove; - CapExprSet ScopedExclusiveReqs, ScopedSharedReqs; + CapExprSet ScopedReqsAndExcludes; StringRef CapDiagKind = "mutex"; // Figure out if we're constructing an object of scoped lockable class @@ -1892,19 +1888,20 @@ POK_FunctionCall, ClassifyDiagnostic(A), Exp->getExprLoc()); // use for adopting a lock - if (isScopedVar) { - Analyzer->getMutexIDs(A->isShared() ? ScopedSharedReqs - : ScopedExclusiveReqs, - A, Exp, D, VD); - } + if (isScopedVar) + Analyzer->getMutexIDs(ScopedReqsAndExcludes, A, Exp, D, VD); } break; } case attr::LocksExcluded: { const auto *A = cast<LocksExcludedAttr>(At); - for (auto *Arg : A->args()) + for (auto *Arg : A->args()) { warnIfMutexHeld(D, Exp, Arg, ClassifyDiagnostic(A)); + // use for deferring a lock + if (isScopedVar) + Analyzer->getMutexIDs(ScopedReqsAndExcludes, A, Exp, D, VD); + } break; } @@ -1944,13 +1941,11 @@ auto ScopedEntry = std::make_unique<ScopedLockableFactEntry>(Scp, MLoc); for (const auto &M : ExclusiveLocksToAdd) - ScopedEntry->addExclusiveLock(M); - for (const auto &M : ScopedExclusiveReqs) - ScopedEntry->addExclusiveLock(M); + ScopedEntry->addLock(M); for (const auto &M : SharedLocksToAdd) - ScopedEntry->addSharedLock(M); - for (const auto &M : ScopedSharedReqs) - ScopedEntry->addSharedLock(M); + ScopedEntry->addLock(M); + for (const auto &M : ScopedReqsAndExcludes) + ScopedEntry->addLock(M); for (const auto &M : ExclusiveLocksToRemove) ScopedEntry->addExclusiveUnlock(M); for (const auto &M : SharedLocksToRemove)
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits