Allow thread safety annotation lock upgrading and downgrading.

Patch thanks to Aaron Puchert!

llvm-svn: 338024

2 files changed, 65 insertions, 19 deletions

diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index 8895dec02b9..03cc234dce5 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -109,9 +109,7 @@ class FactSet;
 /// along with additional information, such as where it was acquired, whether
 /// it is exclusive or shared, etc.
 ///
-/// FIXME: this analysis does not currently support either re-entrant
-/// locking or lock "upgrading" and "downgrading" between exclusive and
-/// shared.
+/// FIXME: this analysis does not currently support re-entrant locking.
 class FactEntry : public CapabilityExpr {
 private:
   /// Exclusive or shared.
@@ -1737,8 +1735,7 @@ void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) {
     }
   }
 
-  for(Attr *Atconst : D->attrs()) {
-    auto *At = const_cast<Attr *>(Atconst);
+  for(const Attr *At : D->attrs()) {
     switch (At->getKind()) {
       // When we encounter a lock function, we need to add the lock to our
       // lockset.
@@ -1838,6 +1835,16 @@ void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) {
     }
   }
 
+  // Remove locks first to allow lock upgrading/downgrading.
+  // FIXME -- should only fully remove if the attribute refers to 'this'.
+  bool Dtor = isa<CXXDestructorDecl>(D);
+  for (const auto &M : ExclusiveLocksToRemove)
+    Analyzer->removeLock(FSet, M, Loc, Dtor, LK_Exclusive, CapDiagKind);
+  for (const auto &M : SharedLocksToRemove)
+    Analyzer->removeLock(FSet, M, Loc, Dtor, LK_Shared, CapDiagKind);
+  for (const auto &M : GenericLocksToRemove)
+    Analyzer->removeLock(FSet, M, Loc, Dtor, LK_Generic, CapDiagKind);
+
   // Add locks.
   for (const auto &M : ExclusiveLocksToAdd)
     Analyzer->addLock(FSet, llvm::make_unique<LockableFactEntry>(
@@ -1864,16 +1871,6 @@ void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) {
                           Scp, MLoc, ExclusiveLocksToAdd, SharedLocksToAdd),
                       CapDiagKind);
   }
-
-  // Remove locks.
-  // FIXME -- should only fully remove if the attribute refers to 'this'.
-  bool Dtor = isa<CXXDestructorDecl>(D);
-  for (const auto &M : ExclusiveLocksToRemove)
-    Analyzer->removeLock(FSet, M, Loc, Dtor, LK_Exclusive, CapDiagKind);
-  for (const auto &M : SharedLocksToRemove)
-    Analyzer->removeLock(FSet, M, Loc, Dtor, LK_Shared, CapDiagKind);
-  for (const auto &M : GenericLocksToRemove)
-    Analyzer->removeLock(FSet, M, Loc, Dtor, LK_Generic, CapDiagKind);
 }
 
 /// For unary operations which read and write a variable, we need to
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index 2c59a518c42..51535be97d9 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -22,8 +22,6 @@
 #define SHARED_LOCK_FUNCTION(...)       __attribute__((acquire_shared_capability(__VA_ARGS__)))
 #define EXCLUSIVE_TRYLOCK_FUNCTION(...) __attribute__((try_acquire_capability(__VA_ARGS__)))
 #define SHARED_TRYLOCK_FUNCTION(...)    __attribute__((try_acquire_shared_capability(__VA_ARGS__)))
-#define EXCLUSIVE_UNLOCK_FUNCTION(...)  __attribute__((release_capability(__VA_ARGS__)))
-#define SHARED_UNLOCK_FUNCTION(...)     __attribute__((release_shared_capability(__VA_ARGS__)))
 #define EXCLUSIVE_LOCKS_REQUIRED(...)   __attribute__((requires_capability(__VA_ARGS__)))
 #define SHARED_LOCKS_REQUIRED(...)      __attribute__((requires_shared_capability(__VA_ARGS__)))
 #else
@@ -34,11 +32,11 @@
 #define SHARED_LOCK_FUNCTION(...)       __attribute__((shared_lock_function(__VA_ARGS__)))
 #define EXCLUSIVE_TRYLOCK_FUNCTION(...) __attribute__((exclusive_trylock_function(__VA_ARGS__)))
 #define SHARED_TRYLOCK_FUNCTION(...)    __attribute__((shared_trylock_function(__VA_ARGS__)))
-#define EXCLUSIVE_UNLOCK_FUNCTION(...)  __attribute__((unlock_function(__VA_ARGS__)))
-#define SHARED_UNLOCK_FUNCTION(...)     __attribute__((unlock_function(__VA_ARGS__)))
 #define EXCLUSIVE_LOCKS_REQUIRED(...)   __attribute__((exclusive_locks_required(__VA_ARGS__)))
 #define SHARED_LOCKS_REQUIRED(...)      __attribute__((shared_locks_required(__VA_ARGS__)))
 #endif
+#define EXCLUSIVE_UNLOCK_FUNCTION(...)  __attribute__((release_capability(__VA_ARGS__)))
+#define SHARED_UNLOCK_FUNCTION(...)     __attribute__((release_shared_capability(__VA_ARGS__)))
 #define UNLOCK_FUNCTION(...)            __attribute__((unlock_function(__VA_ARGS__)))
 #define LOCK_RETURNED(x)                __attribute__((lock_returned(x)))
 #define LOCKS_EXCLUDED(...)             __attribute__((locks_excluded(__VA_ARGS__)))
@@ -50,10 +48,15 @@ class LOCKABLE Mutex {
   void Lock() EXCLUSIVE_LOCK_FUNCTION();
   void ReaderLock() SHARED_LOCK_FUNCTION();
   void Unlock() UNLOCK_FUNCTION();
+  void ExclusiveUnlock() EXCLUSIVE_UNLOCK_FUNCTION();
+  void ReaderUnlock() SHARED_UNLOCK_FUNCTION();
   bool TryLock() EXCLUSIVE_TRYLOCK_FUNCTION(true);
   bool ReaderTryLock() SHARED_TRYLOCK_FUNCTION(true);
   void LockWhen(const int &cond) EXCLUSIVE_LOCK_FUNCTION();
 
+  void PromoteShared() SHARED_UNLOCK_FUNCTION() EXCLUSIVE_LOCK_FUNCTION();
+  void DemoteExclusive() EXCLUSIVE_UNLOCK_FUNCTION() SHARED_LOCK_FUNCTION();
+
   // for negative capabilities
   const Mutex& operator!() const { return *this; }
 
@@ -704,6 +707,26 @@ void shared_fun_8() {
   sls_mu.Unlock();
 }
 
+void shared_fun_9() {
+  sls_mu.Lock();
+  sls_mu.ExclusiveUnlock();
+
+  sls_mu.ReaderLock();
+  sls_mu.ReaderUnlock();
+}
+
+void shared_fun_10() {
+  sls_mu.Lock();
+  sls_mu.DemoteExclusive();
+  sls_mu.ReaderUnlock();
+}
+
+void shared_fun_11() {
+  sls_mu.ReaderLock();
+  sls_mu.PromoteShared();
+  sls_mu.Unlock();
+}
+
 void shared_bad_0() {
   sls_mu.Lock();  // \
     // expected-warning {{mutex 'sls_mu' is acquired exclusively and shared in the same scope}}
@@ -737,6 +760,32 @@ void shared_bad_2() {
   sls_mu.Unlock();
 }
 
+void shared_bad_3() {
+  sls_mu.Lock();
+  sls_mu.ReaderUnlock(); // \
+    // expected-warning {{releasing mutex 'sls_mu' using shared access, expected exclusive access}}
+}
+
+void shared_bad_4() {
+  sls_mu.ReaderLock();
+  sls_mu.ExclusiveUnlock(); // \
+    // expected-warning {{releasing mutex 'sls_mu' using exclusive access, expected shared access}}
+}
+
+void shared_bad_5() {
+  sls_mu.Lock();
+  sls_mu.PromoteShared(); // \
+    // expected-warning {{releasing mutex 'sls_mu' using shared access, expected exclusive access}}
+  sls_mu.ExclusiveUnlock();
+}
+
+void shared_bad_6() {
+  sls_mu.ReaderLock();
+  sls_mu.DemoteExclusive(); // \
+    // expected-warning {{releasing mutex 'sls_mu' using exclusive access, expected shared access}}
+  sls_mu.ReaderUnlock();
+}
+
 // FIXME: Add support for functions (not only methods)
 class LRBar {
  public: