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

Reply via email to