Author: Aaron Puchert Date: 2020-06-08T17:00:29+02:00 New Revision: 1850f56c8abae637c2cc1b8d27b8577c5700101a
URL: https://github.com/llvm/llvm-project/commit/1850f56c8abae637c2cc1b8d27b8577c5700101a DIFF: https://github.com/llvm/llvm-project/commit/1850f56c8abae637c2cc1b8d27b8577c5700101a.diff LOG: Thread safety analysis: Support deferring locks Summary: 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. Reviewers: aaron.ballman, delesley Reviewed By: aaron.ballman Differential Revision: https://reviews.llvm.org/D81332 Added: Modified: clang/lib/Analysis/ThreadSafety.cpp clang/test/SemaCXX/warn-thread-safety-analysis.cpp Removed: ################################################################################ diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp index e0ff23df5ab4..6e518e7d38ef 100644 --- a/clang/lib/Analysis/ThreadSafety.cpp +++ b/clang/lib/Analysis/ThreadSafety.cpp @@ -905,11 +905,7 @@ class ScopedLockableFactEntry : public FactEntry { 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 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D, 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 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D, 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 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D, 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) diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp index cdb22cd22a99..02700147a032 100644 --- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -2625,9 +2625,12 @@ void Foo::test5() { 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 @@ struct ExclusiveTraits {}; 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 @@ void relock() { 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 @@ void relockShared() { 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(); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
