Thread safety analysis: add two new attributes to the thread safety analysis:

assert_exclusive_lock and assert_shared_lock.  These attributes are used to
mark functions that dynamically check (i.e. assert) that a lock is held.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@182170 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td
index 441a79a23b263ca5c802f820f3fdbd02d25c7430..7c59c16f148286b1ed59f027532daa03911a7525 100644 (file)
--- a/include/clang/Basic/Attr.td
+++ b/include/clang/Basic/Attr.td
@@ -852,6 +852,20 @@ def SharedLockFunction : InheritableAttr {
   let TemplateDependent = 1;
 }
 
+def AssertExclusiveLock : InheritableAttr {
+  let Spellings = [GNU<"assert_exclusive_lock">];
+  let Args = [VariadicExprArgument<"Args">];
+  let LateParsed = 1;
+  let TemplateDependent = 1;
+}
+
+def AssertSharedLock : InheritableAttr {
+  let Spellings = [GNU<"assert_shared_lock">];
+  let Args = [VariadicExprArgument<"Args">];
+  let LateParsed = 1;
+  let TemplateDependent = 1;
+}
+
 // The first argument is an integer or boolean value specifying the return value
 // of a successful lock acquisition.
 def ExclusiveTrylockFunction : InheritableAttr {

diff --git a/lib/Analysis/ThreadSafety.cpp b/lib/Analysis/ThreadSafety.cpp
index 479d9a301f4b61e7537d916dc45f314bbdfa4963..c12dc64fc140a37c94c38b167d74d338a2bbf033 100644 (file)
--- a/lib/Analysis/ThreadSafety.cpp
+++ b/lib/Analysis/ThreadSafety.cpp
@@ -750,16 +750,18 @@ struct LockData {
   ///
   /// FIXME: add support for re-entrant locking and lock up/downgrading
   LockKind LKind;
+  bool     Asserted;           // for asserted locks
   bool     Managed;            // for ScopedLockable objects
   SExpr    UnderlyingMutex;    // for ScopedLockable objects
 
-  LockData(SourceLocation AcquireLoc, LockKind LKind, bool M = false)
-    : AcquireLoc(AcquireLoc), LKind(LKind), Managed(M),
+  LockData(SourceLocation AcquireLoc, LockKind LKind, bool M=false,
+           bool Asrt=false)
+    : AcquireLoc(AcquireLoc), LKind(LKind), Asserted(Asrt), Managed(M),
       UnderlyingMutex(Decl::EmptyShell())
   {}
 
   LockData(SourceLocation AcquireLoc, LockKind LKind, const SExpr &Mu)
-    : AcquireLoc(AcquireLoc), LKind(LKind), Managed(false),
+    : AcquireLoc(AcquireLoc), LKind(LKind), Asserted(false), Managed(false),
       UnderlyingMutex(Mu)
   {}
 
@@ -1484,7 +1486,8 @@ void ThreadSafetyAnalyzer::addLock(FactSet &FSet, const SExpr &Mutex,
     return;
 
   if (FSet.findLock(FactMan, Mutex)) {
-    Handler.handleDoubleLock(Mutex.toString(), LDat.AcquireLoc);
+    if (!LDat.Asserted)
+      Handler.handleDoubleLock(Mutex.toString(), LDat.AcquireLoc);
   } else {
     FSet.addLock(FactMan, Mutex, LDat);
   }
@@ -1909,6 +1912,7 @@ void BuildLockset::checkPtAccess(const Expr *Exp, AccessKind AK) {
 /// the same signature as const method calls can be also treated as reads.
 ///
 void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) {
+  SourceLocation Loc = Exp->getExprLoc();
   const AttrVec &ArgAttrs = D->getAttrs();
   MutexIDList ExclusiveLocksToAdd;
   MutexIDList SharedLocksToAdd;
@@ -1933,6 +1937,32 @@ void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) {
         break;
       }
 
+      // An assert will add a lock to the lockset, but will not generate
+      // a warning if it is already there, and will not generate a warning
+      // if it is not removed.
+      case attr::AssertExclusiveLock: {
+        AssertExclusiveLockAttr *A = cast<AssertExclusiveLockAttr>(At);
+
+        MutexIDList AssertLocks;
+        Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD);
+        for (unsigned i=0,n=AssertLocks.size(); i<n; ++i) {
+          Analyzer->addLock(FSet, AssertLocks[i],
+                            LockData(Loc, LK_Exclusive, false, true));
+        }
+        break;
+      }
+      case attr::AssertSharedLock: {
+        AssertSharedLockAttr *A = cast<AssertSharedLockAttr>(At);
+
+        MutexIDList AssertLocks;
+        Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD);
+        for (unsigned i=0,n=AssertLocks.size(); i<n; ++i) {
+          Analyzer->addLock(FSet, AssertLocks[i],
+                            LockData(Loc, LK_Shared, false, true));
+        }
+        break;
+      }
+
       // When we encounter an unlock function, we need to remove unlocked
       // mutexes from the lockset, and flag a warning if they are not there.
       case attr::UnlockFunction: {
@@ -1986,7 +2016,6 @@ void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) {
   }
 
   // Add locks.
-  SourceLocation Loc = Exp->getExprLoc();
   for (unsigned i=0,n=ExclusiveLocksToAdd.size(); i<n; ++i) {
     Analyzer->addLock(FSet, ExclusiveLocksToAdd[i],
                             LockData(Loc, LK_Exclusive, isScopedVar));
@@ -2165,7 +2194,7 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet &FSet1,
     const SExpr &FSet2Mutex = FactMan[*I].MutID;
     const LockData &LDat2 = FactMan[*I].LDat;
 
-    if (const LockData *LDat1 = FSet1.findLock(FactMan, FSet2Mutex)) {
+    if (LockData *LDat1 = FSet1.findLock(FactMan, FSet2Mutex)) {
       if (LDat1->LKind != LDat2.LKind) {
         Handler.handleExclusiveAndShared(FSet2Mutex.toString(),
                                          LDat2.AcquireLoc,
@@ -2175,6 +2204,10 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet &FSet1,
           FSet1.addLock(FactMan, FSet2Mutex, LDat2);
         }
       }
+      if (LDat1->Asserted && !LDat2.Asserted) {
+        // The non-asserted lock is the one we want to track.
+        *LDat1 = LDat2;
+      }
     } else {
       if (LDat2.UnderlyingMutex.isValid()) {
         if (FSet2.findLock(FactMan, LDat2.UnderlyingMutex)) {
@@ -2186,7 +2219,7 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet &FSet1,
                                             JoinLoc, LEK1);
         }
       }
-      else if (!LDat2.Managed && !FSet2Mutex.isUniversal())
+      else if (!LDat2.Managed && !FSet2Mutex.isUniversal() && !LDat2.Asserted)
         Handler.handleMutexHeldEndOfScope(FSet2Mutex.toString(),
                                           LDat2.AcquireLoc,
                                           JoinLoc, LEK1);
@@ -2209,7 +2242,7 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet &FSet1,
                                             JoinLoc, LEK1);
         }
       }
-      else if (!LDat1.Managed && !FSet1Mutex.isUniversal())
+      else if (!LDat1.Managed && !FSet1Mutex.isUniversal() && !LDat1.Asserted)
         Handler.handleMutexHeldEndOfScope(FSet1Mutex.toString(),
                                           LDat1.AcquireLoc,
                                           JoinLoc, LEK2);

diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp
index d9b8e160610475fde157fc6d7524d029fac057df..c365bfc508ca7bfc1cb225643ff33038d8be001f 100644 (file)
--- a/lib/Sema/SemaDeclAttr.cpp
+++ b/lib/Sema/SemaDeclAttr.cpp
@@ -788,6 +788,34 @@ static void handleExclusiveLockFunctionAttr(Sema &S, Decl *D,
                                        Attr.getAttributeSpellingListIndex()));
 }
 
+static void handleAssertSharedLockAttr(Sema &S, Decl *D,
+                                       const AttributeList &Attr) {
+  SmallVector<Expr*, 1> Args;
+  if (!checkLockFunAttrCommon(S, D, Attr, Args))
+    return;
+
+  unsigned Size = Args.size();
+  Expr **StartArg = Size == 0 ? 0 : &Args[0];
+  D->addAttr(::new (S.Context)
+             AssertSharedLockAttr(Attr.getRange(), S.Context, StartArg, Size,
+                                  Attr.getAttributeSpellingListIndex()));
+}
+
+static void handleAssertExclusiveLockAttr(Sema &S, Decl *D,
+                                          const AttributeList &Attr) {
+  SmallVector<Expr*, 1> Args;
+  if (!checkLockFunAttrCommon(S, D, Attr, Args))
+    return;
+
+  unsigned Size = Args.size();
+  Expr **StartArg = Size == 0 ? 0 : &Args[0];
+  D->addAttr(::new (S.Context)
+             AssertExclusiveLockAttr(Attr.getRange(), S.Context,
+                                     StartArg, Size,
+                                     Attr.getAttributeSpellingListIndex()));
+}
+
+
 static bool checkTryLockFunAttrCommon(Sema &S, Decl *D,
                                       const AttributeList &Attr,
                                       SmallVector<Expr*, 2> &Args) {
@@ -4918,6 +4946,12 @@ static void ProcessInheritableDeclAttr(Sema &S, Scope *scope, Decl *D,
     break;
 
   // Thread safety attributes:
+  case AttributeList::AT_AssertExclusiveLock:
+    handleAssertExclusiveLockAttr(S, D, Attr);
+    break;
+  case AttributeList::AT_AssertSharedLock:
+    handleAssertSharedLockAttr(S, D, Attr);
+    break;
   case AttributeList::AT_GuardedVar:
     handleGuardedVarAttr(S, D, Attr);
     break;

diff --git a/test/SemaCXX/warn-thread-safety-analysis.cpp b/test/SemaCXX/warn-thread-safety-analysis.cpp
index bc4b40ead73d7040c6b769faebc6b68b66f618d8..20b1a56354147afaa5f68edd79fc0f1971326aaa 100644 (file)
--- a/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -11,8 +11,10 @@
 #define PT_GUARDED_VAR      __attribute__ ((pt_guarded_var))
 #define ACQUIRED_AFTER(...) __attribute__ ((acquired_after(__VA_ARGS__)))
 #define ACQUIRED_BEFORE(...) __attribute__ ((acquired_before(__VA_ARGS__)))
-#define EXCLUSIVE_LOCK_FUNCTION(...)   __attribute__ ((exclusive_lock_function(__VA_ARGS__)))
-#define SHARED_LOCK_FUNCTION(...)      __attribute__ ((shared_lock_function(__VA_ARGS__)))
+#define EXCLUSIVE_LOCK_FUNCTION(...)    __attribute__ ((exclusive_lock_function(__VA_ARGS__)))
+#define SHARED_LOCK_FUNCTION(...)       __attribute__ ((shared_lock_function(__VA_ARGS__)))
+#define ASSERT_EXCLUSIVE_LOCK(...)      __attribute__ ((assert_exclusive_lock(__VA_ARGS__)))
+#define ASSERT_SHARED_LOCK(...)         __attribute__ ((assert_shared_lock(__VA_ARGS__)))
 #define EXCLUSIVE_TRYLOCK_FUNCTION(...) __attribute__ ((exclusive_trylock_function(__VA_ARGS__)))
 #define SHARED_TRYLOCK_FUNCTION(...)    __attribute__ ((shared_trylock_function(__VA_ARGS__)))
 #define UNLOCK_FUNCTION(...)            __attribute__ ((unlock_function(__VA_ARGS__)))
@@ -33,6 +35,9 @@ class  __attribute__((lockable)) Mutex {
   bool TryLock() __attribute__((exclusive_trylock_function(true)));
   bool ReaderTryLock() __attribute__((shared_trylock_function(true)));
   void LockWhen(const int &cond) __attribute__((exclusive_lock_function));
+
+  void AssertHeld()       ASSERT_EXCLUSIVE_LOCK();
+  void AssertReaderHeld() ASSERT_SHARED_LOCK();
 };
 
 class __attribute__((scoped_lockable)) MutexLock {
@@ -3985,3 +3990,105 @@ private:
 
 }  // end namespace LockUnlockFunctionTest
 
+
+namespace AssertHeldTest {
+
+class Foo {
+public:
+  int c;
+  int a GUARDED_BY(mu_);
+  Mutex mu_;
+
+  void test1() {
+    mu_.AssertHeld();
+    int b = a;
+    a = 0;
+  }
+
+  void test2() {
+    mu_.AssertReaderHeld();
+    int b = a;
+    a = 0;   // expected-warning {{writing variable 'a' requires locking 'mu_' exclusively}}
+  }
+
+  void test3() {
+    if (c) {
+      mu_.AssertHeld();
+    }
+    else {
+      mu_.AssertHeld();
+    }
+    int b = a;
+    a = 0;
+  }
+
+  void test4() EXCLUSIVE_LOCKS_REQUIRED(mu_) {
+    mu_.AssertHeld();
+    int b = a;
+    a = 0;
+  }
+
+  void test5() UNLOCK_FUNCTION(mu_) {
+    mu_.AssertHeld();
+    mu_.Unlock();
+  }
+
+  void test6() {
+    mu_.AssertHeld();
+    mu_.Unlock();
+  }  // should this be a warning?
+
+  void test7() {
+    if (c) {
+      mu_.AssertHeld();
+    }
+    else {
+      mu_.Lock();
+    }
+    int b = a;
+    a = 0;
+    mu_.Unlock();
+  }
+
+  void test8() {
+    if (c) {
+      mu_.Lock();
+    }
+    else {
+      mu_.AssertHeld();
+    }
+    int b = a;
+    a = 0;
+    mu_.Unlock();
+  }
+
+  void test9() {
+    if (c) {
+      mu_.AssertHeld();
+    }
+    else {
+      mu_.Lock();  // expected-note {{mutex acquired here}}
+    }
+  }  // expected-warning {{mutex 'mu_' is still locked at the end of function}}
+
+  void test10() {
+    if (c) {
+      mu_.Lock();  // expected-note {{mutex acquired here}}
+    }
+    else {
+      mu_.AssertHeld();
+    }
+  }  // expected-warning {{mutex 'mu_' is still locked at the end of function}}
+
+  void assertMu() ASSERT_EXCLUSIVE_LOCK(mu_);
+
+  void test11() {
+    assertMu();
+    int b = a;
+    a = 0;
+  }
+};
+
+}  // end namespace AssertHeldTest
+
+

diff --git a/test/SemaCXX/warn-thread-safety-parsing.cpp b/test/SemaCXX/warn-thread-safety-parsing.cpp
index df9415cf8601aa3057dd8f98603e8775c803fc96..ee6ec92752126123c5ae554aad1a727e501ecd28 100644 (file)
--- a/test/SemaCXX/warn-thread-safety-parsing.cpp
+++ b/test/SemaCXX/warn-thread-safety-parsing.cpp
@@ -8,8 +8,10 @@
 #define PT_GUARDED_VAR      __attribute__ ((pt_guarded_var))
 #define ACQUIRED_AFTER(...) __attribute__ ((acquired_after(__VA_ARGS__)))
 #define ACQUIRED_BEFORE(...) __attribute__ ((acquired_before(__VA_ARGS__)))
-#define EXCLUSIVE_LOCK_FUNCTION(...)   __attribute__ ((exclusive_lock_function(__VA_ARGS__)))
-#define SHARED_LOCK_FUNCTION(...)      __attribute__ ((shared_lock_function(__VA_ARGS__)))
+#define EXCLUSIVE_LOCK_FUNCTION(...)    __attribute__ ((exclusive_lock_function(__VA_ARGS__)))
+#define SHARED_LOCK_FUNCTION(...)       __attribute__ ((shared_lock_function(__VA_ARGS__)))
+#define ASSERT_EXCLUSIVE_LOCK(...)      __attribute__ ((assert_exclusive_lock(__VA_ARGS__)))
+#define ASSERT_SHARED_LOCK(...)         __attribute__ ((assert_shared_lock(__VA_ARGS__)))
 #define EXCLUSIVE_TRYLOCK_FUNCTION(...) __attribute__ ((exclusive_trylock_function(__VA_ARGS__)))
 #define SHARED_TRYLOCK_FUNCTION(...)    __attribute__ ((shared_trylock_function(__VA_ARGS__)))
 #define UNLOCK_FUNCTION(...)            __attribute__ ((unlock_function(__VA_ARGS__)))
@@ -24,7 +26,15 @@
 
 class LOCKABLE Mutex {
   public:
-  void Lock();
+  void Lock()          EXCLUSIVE_LOCK_FUNCTION();
+  void ReaderLock()    SHARED_LOCK_FUNCTION();
+  void Unlock()        UNLOCK_FUNCTION();
+
+  bool TryLock()       EXCLUSIVE_TRYLOCK_FUNCTION(true);
+  bool ReaderTryLock() SHARED_TRYLOCK_FUNCTION(true);
+
+  void AssertHeld()       ASSERT_EXCLUSIVE_LOCK();
+  void AssertReaderHeld() ASSERT_SHARED_LOCK();
 };
 
 class UnlockableMu{