Thread safety analysis: Support deferring locks

[aaronpuchert]

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.

[aaron.ballman]

LGTM! Should we update the public docs for this change as well? Specifically, I am wondering if we should update https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#mutexheader

[aaronpuchert]

In D81332#2079489, @aaron.ballman wrote:

LGTM! Should we update the public docs for this change as well? Specifically, I am wondering if we should update https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#mutexheader

Yes, some existing functionality seems undocumented as well (like adopting locks), which is why I was planning a bigger change.

My plan is basically to go through the test cases (which seem to be quite complete in terms of what we intend to support) and see what deserves mention in the official docs.

clang/lib/Analysis/ThreadSafety.cpp

using UnderlyingCapability =
       llvm::PointerIntPair<const til::SExpr *, 2, UnderlyingCapabilityKind>;
 
   SmallVector<UnderlyingCapability, 4> UnderlyingMutexes;
 
 public:
   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);
   }
 
   void addExclusiveUnlock(const CapabilityExpr &M) {
     UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedExclusive);
   }
 
   void addSharedUnlock(const CapabilityExpr &M) {

 /// and check that the appropriate locks are held. Non-const method calls with
 /// the same signature as const method calls can be also treated as reads.
 ///
 void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
                               VarDecl *VD) {
   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
   bool isScopedVar = false;
   if (VD) {
     if (const auto *CD = dyn_cast<const CXXConstructorDecl>(D)) {
       const CXXRecordDecl* PD = CD->getParent();
       if (PD && PD->hasAttr<ScopedLockableAttr>())
switch (At->getKind()) {
 
       case attr::RequiresCapability: {
         const auto *A = cast<RequiresCapabilityAttr>(At);
         for (auto *Arg : A->args()) {
           warnIfMutexNotHeld(D, Exp, A->isShared() ? AK_Read : AK_Written, Arg,
                              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;
       }
 
       // Ignore attributes unrelated to thread-safety
       default:
         break;
     }
   }
if (isScopedVar) {
     SourceLocation MLoc = VD->getLocation();
     DeclRefExpr DRE(VD->getASTContext(), VD, false, VD->getType(), VK_LValue,
                     VD->getLocation());
     // FIXME: does this store a pointer to DRE?
     CapabilityExpr Scp = Analyzer->SxBuilder.translateAttrExpr(&DRE, nullptr);
 
     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)
       ScopedEntry->addSharedUnlock(M);
     Analyzer->addLock(FSet, std::move(ScopedEntry), CapDiagKind);
   }
 }
 

clang/test/SemaCXX/warn-thread-safety-analysis.cpp

 }
 
 
 } // end namespace ReleasableScopedLock
 
 
 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();
   void Unlock() UNLOCK_FUNCTION();
 };
 
 struct SharedTraits {};
 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();
 
   void Lock() EXCLUSIVE_LOCK_FUNCTION();
   void Unlock() UNLOCK_FUNCTION();
 
   void ReaderLock() SHARED_LOCK_FUNCTION();
void relock() {
   scope.Unlock();
 
   x = 3; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
 
   scope.Lock();
   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);
   x = 2; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
   scope.ReaderUnlock();
 
   print(x); // expected-warning {{reading variable 'x' requires holding mutex 'mu'}}
 
void relockShared() {
   print(x);
   x = 4; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
 
   scope.PromoteShared();
   print(x);
   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();
   scope.Unlock(); // expected-warning {{releasing mutex 'mu' that was not held}}
 }
 
 void doubleLock1() {
   RelockableExclusiveMutexLock scope(&mu); // expected-note{{mutex acquired here}}