r204469 - The release_capability, release_shared_capability and release_generic_capability functions are now functionally distinct for capability analysis. The unlock_function attribute maps directly to release_generic_capability.

Aaron Ballman aaron at aaronballman.com
Fri Mar 21 07:48:49 PDT 2014


Author: aaronballman
Date: Fri Mar 21 09:48:48 2014
New Revision: 204469

URL: http://llvm.org/viewvc/llvm-project?rev=204469&view=rev
Log:
The release_capability, release_shared_capability and release_generic_capability functions are now functionally distinct for capability analysis. The unlock_function attribute maps directly to release_generic_capability.

Modified:
    cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h
    cfe/trunk/include/clang/Basic/Attr.td
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/lib/Analysis/ThreadSafety.cpp
    cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
    cfe/trunk/test/Sema/warn-thread-safety-analysis.c

Modified: cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h?rev=204469&r1=204468&r2=204469&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h Fri Mar 21 09:48:48 2014
@@ -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.

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=204469&r1=204468&r2=204469&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Fri Mar 21 09:48:48 2014
@@ -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];
 }
 

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=204469&r1=204468&r2=204469&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Mar 21 09:48:48 2014
@@ -2201,6 +2201,10 @@ def err_attribute_argument_out_of_range
 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;

Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=204469&r1=204468&r2=204469&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Fri Mar 21 09:48:48 2014
@@ -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(FactS
 /// \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(Fa
     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 E
 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,
       // 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,
   }
 
   // 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,
     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);
 }
 
 

Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=204469&r1=204468&r2=204469&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
+++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Fri Mar 21 09:48:48 2014
@@ -1452,7 +1452,15 @@ class ThreadSafetyReporter : public clan
   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);
   }

Modified: cfe/trunk/test/Sema/warn-thread-safety-analysis.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-thread-safety-analysis.c?rev=204469&r1=204468&r2=204469&view=diff
==============================================================================
--- cfe/trunk/test/Sema/warn-thread-safety-analysis.c (original)
+++ cfe/trunk/test/Sema/warn-thread-safety-analysis.c Fri Mar 21 09:48:48 2014
@@ -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;
 }





More information about the cfe-commits mailing list