The release_capability, release_shared_capability and release_generic_capability...

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

diff --git a/include/clang/Analysis/Analyses/ThreadSafety.h b/include/clang/Analysis/Analyses/ThreadSafety.h
index 5def3dd3dfd197f1554f855dd6f0cd88073b5327..417451dbf056abd02bc31552d066260d643c5524 100644 (file)
--- a/include/clang/Analysis/Analyses/ThreadSafety.h
+++ b/include/clang/Analysis/Analyses/ThreadSafety.h
@@ -39,7 +39,8 @@ enum ProtectedOperationKind {
 /// mutex.
 enum LockKind {
   LK_Shared, ///< Shared/reader lock of a mutex.
-  LK_Exclusive ///< Exclusive/writer lock of a mutex.
+  LK_Exclusive, ///< Exclusive/writer lock of a mutex.
+  LK_Generic  ///<  Can be either Shared or Exclusive
 };
 
 /// This enum distinguishes between different ways to access (read or write) a
@@ -82,6 +83,18 @@ public:
   /// \param Loc -- The SourceLocation of the Unlock
   virtual void handleUnmatchedUnlock(Name LockName, SourceLocation Loc) {}
 
+  /// Warn about an unlock function call that attempts to unlock a lock with
+  /// the incorrect lock kind. For instance, a shared lock being unlocked
+  /// exclusively, or vice versa.
+  /// \param LockName -- A StringRef name for the lock expression, to be printed
+  /// in the error message.
+  /// \param Expected -- the kind of lock expected.
+  /// \param Received -- the kind of lock received.
+  /// \param Loc -- The SourceLocation of the Unlock.
+  virtual void handleIncorrectUnlockKind(Name LockName, LockKind Expected,
+                                         LockKind Received,
+                                         SourceLocation Loc) {}
+
   /// Warn about lock function calls for locks which are already held.
   /// \param LockName -- A StringRef name for the lock expression, to be printed
   /// in the error message.

diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td
index c567cf29ebeebaa3f7f971ddc1ba1b1ac383e92c..3eb5d4e0f3fd7684c9bbc8ce3d3c84f86c812464 100644 (file)
--- a/include/clang/Basic/Attr.td
+++ b/include/clang/Basic/Attr.td
@@ -1388,7 +1388,8 @@ def ReleaseCapability : InheritableAttr {
                      CXX11<"clang", "release_shared_capability">]>,
                    Accessor<"isGeneric",
                      [GNU<"release_generic_capability">,
-                      CXX11<"clang", "release_generic_capability">]>];
+                      CXX11<"clang", "release_generic_capability">,
+                      GNU<"unlock_function">]>];
   let Documentation = [ReleaseCapabilityDocs];
 }
 

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 5e459305126550441fb295d0511397d6e5e39f8b..179c605aeb6f0e3878b1e3cdf5393980874d1307 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2201,6 +2201,10 @@ def err_attribute_argument_out_of_range : Error<
 def warn_unlock_but_no_lock : Warning<
   "unlocking '%0' that was not locked">,
   InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
+def warn_unlock_kind_mismatch : Warning<
+  "unlocking '%0' using %select{shared|exclusive}1 access, expected "
+  "%select{shared|exclusive}2 access">,
+  InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
 def warn_double_lock : Warning<
   "locking '%0' that is already locked">,
   InGroup<ThreadSafetyAnalysis>, DefaultIgnore;

diff --git a/lib/Analysis/ThreadSafety.cpp b/lib/Analysis/ThreadSafety.cpp
index cde19b531e04b014a26417954cb5f781ec72c33d..5f7ce2d59281878150a3cc420439a7c6a6d08fee 100644 (file)
--- a/lib/Analysis/ThreadSafety.cpp
+++ b/lib/Analysis/ThreadSafety.cpp
@@ -1430,7 +1430,7 @@ public:
 
   void addLock(FactSet &FSet, const SExpr &Mutex, const LockData &LDat);
   void removeLock(FactSet &FSet, const SExpr &Mutex,
-                  SourceLocation UnlockLoc, bool FullyRemove=false);
+                  SourceLocation UnlockLoc, bool FullyRemove, LockKind Kind);
 
   template <typename AttrType>
   void getMutexIDs(MutexIDList &Mtxs, AttrType *Attr, Expr *Exp,
@@ -1486,10 +1486,9 @@ void ThreadSafetyAnalyzer::addLock(FactSet &FSet, const SExpr &Mutex,
 /// \brief Remove a lock from the lockset, warning if the lock is not there.
 /// \param Mutex The lock expression corresponding to the lock to be removed
 /// \param UnlockLoc The source location of the unlock (only used in error msg)
-void ThreadSafetyAnalyzer::removeLock(FactSet &FSet,
-                                      const SExpr &Mutex,
+void ThreadSafetyAnalyzer::removeLock(FactSet &FSet, const SExpr &Mutex,
                                       SourceLocation UnlockLoc,
-                                      bool FullyRemove) {
+                                      bool FullyRemove, LockKind ReceivedKind) {
   if (Mutex.shouldIgnore())
     return;
 
@@ -1499,6 +1498,14 @@ void ThreadSafetyAnalyzer::removeLock(FactSet &FSet,
     return;
   }
 
+  // Generic lock removal doesn't care about lock kind mismatches, but
+  // otherwise diagnose when the lock kinds are mismatched.
+  if (ReceivedKind != LK_Generic && LDat->LKind != ReceivedKind) {
+    Handler.handleIncorrectUnlockKind(Mutex.toString(), LDat->LKind,
+                                      ReceivedKind, UnlockLoc);
+    return;
+  }
+
   if (LDat->UnderlyingMutex.isValid()) {
     // This is scoped lockable object, which manages the real mutex.
     if (FullyRemove) {
@@ -1926,9 +1933,8 @@ void BuildLockset::checkPtAccess(const Expr *Exp, AccessKind AK) {
 void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) {
   SourceLocation Loc = Exp->getExprLoc();
   const AttrVec &ArgAttrs = D->getAttrs();
-  MutexIDList ExclusiveLocksToAdd;
-  MutexIDList SharedLocksToAdd;
-  MutexIDList LocksToRemove;
+  MutexIDList ExclusiveLocksToAdd, SharedLocksToAdd;
+  MutexIDList ExclusiveLocksToRemove, SharedLocksToRemove, GenericLocksToRemove;
 
   for(unsigned i = 0; i < ArgAttrs.size(); ++i) {
     Attr *At = const_cast<Attr*>(ArgAttrs[i]);
@@ -1973,7 +1979,12 @@ void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) {
       // mutexes from the lockset, and flag a warning if they are not there.
       case attr::ReleaseCapability: {
         auto *A = cast<ReleaseCapabilityAttr>(At);
-        Analyzer->getMutexIDs(LocksToRemove, A, Exp, D, VD);
+        if (A->isGeneric())
+          Analyzer->getMutexIDs(GenericLocksToRemove, A, Exp, D, VD);
+        else if (A->isShared())
+          Analyzer->getMutexIDs(SharedLocksToRemove, A, Exp, D, VD);
+        else
+          Analyzer->getMutexIDs(ExclusiveLocksToRemove, A, Exp, D, VD);
         break;
       }
 
@@ -2014,14 +2025,10 @@ void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) {
   }
 
   // Add locks.
-  for (unsigned i=0,n=ExclusiveLocksToAdd.size(); i<n; ++i) {
-    Analyzer->addLock(FSet, ExclusiveLocksToAdd[i],
-                            LockData(Loc, LK_Exclusive, isScopedVar));
-  }
-  for (unsigned i=0,n=SharedLocksToAdd.size(); i<n; ++i) {
-    Analyzer->addLock(FSet, SharedLocksToAdd[i],
-                            LockData(Loc, LK_Shared, isScopedVar));
-  }
+  for (const auto &M : ExclusiveLocksToAdd)
+    Analyzer->addLock(FSet, M, LockData(Loc, LK_Exclusive, isScopedVar));
+  for (const auto &M : SharedLocksToAdd)
+    Analyzer->addLock(FSet, M, LockData(Loc, LK_Shared, isScopedVar));
 
   // Add the managing object as a dummy mutex, mapped to the underlying mutex.
   // FIXME -- this doesn't work if we acquire multiple locks.
@@ -2030,22 +2037,21 @@ void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) {
     DeclRefExpr DRE(VD, false, VD->getType(), VK_LValue, VD->getLocation());
     SExpr SMutex(&DRE, 0, 0);
 
-    for (unsigned i=0,n=ExclusiveLocksToAdd.size(); i<n; ++i) {
-      Analyzer->addLock(FSet, SMutex, LockData(MLoc, LK_Exclusive,
-                                               ExclusiveLocksToAdd[i]));
-    }
-    for (unsigned i=0,n=SharedLocksToAdd.size(); i<n; ++i) {
-      Analyzer->addLock(FSet, SMutex, LockData(MLoc, LK_Shared,
-                                               SharedLocksToAdd[i]));
-    }
+    for (const auto &M : ExclusiveLocksToAdd)
+      Analyzer->addLock(FSet, SMutex, LockData(MLoc, LK_Exclusive, M));
+    for (const auto &M : SharedLocksToAdd)
+      Analyzer->addLock(FSet, SMutex, LockData(MLoc, LK_Shared, M));
   }
 
   // Remove locks.
   // FIXME -- should only fully remove if the attribute refers to 'this'.
   bool Dtor = isa<CXXDestructorDecl>(D);
-  for (unsigned i=0,n=LocksToRemove.size(); i<n; ++i) {
-    Analyzer->removeLock(FSet, LocksToRemove[i], Loc, Dtor);
-  }
+  for (const auto &M : ExclusiveLocksToRemove)
+    Analyzer->removeLock(FSet, M, Loc, Dtor, LK_Exclusive);
+  for (const auto &M : SharedLocksToRemove)
+    Analyzer->removeLock(FSet, M, Loc, Dtor, LK_Shared);
+  for (const auto &M : GenericLocksToRemove)
+    Analyzer->removeLock(FSet, M, Loc, Dtor, LK_Generic);
 }
 
 

diff --git a/lib/Sema/AnalysisBasedWarnings.cpp b/lib/Sema/AnalysisBasedWarnings.cpp
index ecf6d51ef1c8f9ec3b2cfe16c4f662c0b27a4a61..efaf966562cf1ce3bf81a200c880ced40b3594f0 100644 (file)
--- a/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/lib/Sema/AnalysisBasedWarnings.cpp
@@ -1452,7 +1452,15 @@ class ThreadSafetyReporter : public clang::thread_safety::ThreadSafetyHandler {
   void handleUnmatchedUnlock(Name LockName, SourceLocation Loc) override {
     warnLockMismatch(diag::warn_unlock_but_no_lock, LockName, Loc);
   }
-
+  void handleIncorrectUnlockKind(Name LockName, LockKind Expected,
+                                 LockKind Received,
+                                 SourceLocation Loc) override {
+    if (Loc.isInvalid())
+      Loc = FunLocation;
+    PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_unlock_kind_mismatch)
+                                         << LockName << Received << Expected);
+    Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
+  }
   void handleDoubleLock(Name LockName, SourceLocation Loc) override {
     warnLockMismatch(diag::warn_double_lock, LockName, Loc);
   }

diff --git a/test/Sema/warn-thread-safety-analysis.c b/test/Sema/warn-thread-safety-analysis.c
index 1796f74caab3650a2ebfefeb437e3072782d57f8..62460f99fa612ff20abeccc4799522dbbedd74e2 100644 (file)
--- a/test/Sema/warn-thread-safety-analysis.c
+++ b/test/Sema/warn-thread-safety-analysis.c
@@ -35,6 +35,8 @@ struct Foo {
 void mutex_exclusive_lock(struct Mutex *mu) EXCLUSIVE_LOCK_FUNCTION(mu);
 void mutex_shared_lock(struct Mutex *mu) SHARED_LOCK_FUNCTION(mu);
 void mutex_unlock(struct Mutex *mu) UNLOCK_FUNCTION(mu);
+void mutex_shared_unlock(struct Mutex *mu) __attribute__((release_shared_capability(mu)));
+void mutex_exclusive_unlock(struct Mutex *mu) __attribute__((release_capability(mu)));
 
 // Define global variables.
 struct Mutex mu1;
@@ -114,5 +116,13 @@ int main() {
   (void)(*d_ == 1);
   mutex_unlock(foo_.mu_);
 
+  mutex_exclusive_lock(&mu1);
+  mutex_shared_unlock(&mu1); // expected-warning {{unlocking 'mu1' using shared access, expected exclusive access}}
+  mutex_exclusive_unlock(&mu1);
+
+  mutex_shared_lock(&mu1);
+  mutex_exclusive_unlock(&mu1); // expected-warning {{unlocking 'mu1' using exclusive access, expected shared access}}
+  mutex_shared_unlock(&mu1);
+
   return 0;
 }