From: DeLesley Hutchins <delesley@google.com>
Date: Thu, 16 Feb 2012 17:13:43 +0000 (+0000)
Subject: Thread-safety analysis: Disable checking inside constructors, destructors, lock,... 
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=2f13bec63b0236b169b026b7bc852da51ee029a7;p=clang

Thread-safety analysis: Disable checking inside constructors, destructors, lock, and unlock functions

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

diff --git a/lib/Analysis/ThreadSafety.cpp b/lib/Analysis/ThreadSafety.cpp
index 5a08ec6b21..1370d5dbac 100644
--- a/lib/Analysis/ThreadSafety.cpp
+++ b/lib/Analysis/ThreadSafety.cpp
@@ -1432,6 +1432,14 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
     return;  // Ignore anonymous functions for now.
   if (D->getAttr<NoThreadSafetyAnalysisAttr>())
     return;
+  // FIXME: Do something a bit more intelligent inside constructor and
+  // destructor code.  Constructors and destructors must assume unique access
+  // to 'this', so checks on member variable access is disabled, but we should
+  // still enable checks on other objects.
+  if (isa<CXXConstructorDecl>(D))
+    return;  // Don't check inside constructors.
+  if (isa<CXXDestructorDecl>(D))
+    return;  // Don't check inside destructors.
 
   std::vector<CFGBlockInfo> BlockInfo(CFGraph->getNumBlockIDs(),
     CFGBlockInfo::getEmptyBlockInfo(LocksetFactory, LocalVarMap));
@@ -1449,30 +1457,40 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
   findBlockLocations(CFGraph, SortedGraph, BlockInfo);
 
   // Add locks from exclusive_locks_required and shared_locks_required
-  // to initial lockset.
+  // to initial lockset. Also turn off checking for lock and unlock functions.
+  // FIXME: is there a more intelligent way to check lock/unlock functions?
   if (!SortedGraph->empty() && D->hasAttrs()) {
     const CFGBlock *FirstBlock = *SortedGraph->begin();
     Lockset &InitialLockset = BlockInfo[FirstBlock->getBlockID()].EntrySet;
     const AttrVec &ArgAttrs = D->getAttrs();
-    for(unsigned i = 0; i < ArgAttrs.size(); ++i) {
+    for (unsigned i = 0; i < ArgAttrs.size(); ++i) {
       Attr *Attr = ArgAttrs[i];
       SourceLocation AttrLoc = Attr->getLocation();
       if (SharedLocksRequiredAttr *SLRAttr
             = dyn_cast<SharedLocksRequiredAttr>(Attr)) {
         for (SharedLocksRequiredAttr::args_iterator
-            SLRIter = SLRAttr->args_begin(),
-            SLREnd = SLRAttr->args_end(); SLRIter != SLREnd; ++SLRIter)
+             SLRIter = SLRAttr->args_begin(),
+             SLREnd = SLRAttr->args_end(); SLRIter != SLREnd; ++SLRIter)
           InitialLockset = addLock(InitialLockset,
                                    *SLRIter, D, LK_Shared,
                                    AttrLoc);
       } else if (ExclusiveLocksRequiredAttr *ELRAttr
                    = dyn_cast<ExclusiveLocksRequiredAttr>(Attr)) {
         for (ExclusiveLocksRequiredAttr::args_iterator
-            ELRIter = ELRAttr->args_begin(),
-            ELREnd = ELRAttr->args_end(); ELRIter != ELREnd; ++ELRIter)
+             ELRIter = ELRAttr->args_begin(),
+             ELREnd = ELRAttr->args_end(); ELRIter != ELREnd; ++ELRIter)
           InitialLockset = addLock(InitialLockset,
                                    *ELRIter, D, LK_Exclusive,
                                    AttrLoc);
+      } else if (isa<UnlockFunctionAttr>(Attr)) {
+        // Don't try to check unlock functions for now
+        return;
+      } else if (isa<ExclusiveLockFunctionAttr>(Attr)) {
+        // Don't try to check lock functions for now
+        return;
+      } else if (isa<SharedLockFunctionAttr>(Attr)) {
+        // Don't try to check lock functions for now
+        return;
       }
     }
   }
diff --git a/test/SemaCXX/warn-thread-safety-analysis.cpp b/test/SemaCXX/warn-thread-safety-analysis.cpp
index 20d816a41d..58f351fcd8 100644
--- a/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -313,12 +313,13 @@ Mutex wmu;
 
 // Test diagnostics for other method names.
 class WeirdMethods {
+  // FIXME: can't currently check inside constructors and destructors.
   WeirdMethods() {
-    wmu.Lock(); // expected-note {{mutex acquired here}}
-  } // expected-warning {{mutex 'wmu' is still locked at the end of function}}
+    wmu.Lock(); // EXPECTED-NOTE {{mutex acquired here}}
+  } // EXPECTED-WARNING {{mutex 'wmu' is still locked at the end of function}}
   ~WeirdMethods() {
-    wmu.Lock(); // expected-note {{mutex acquired here}}
-  } // expected-warning {{mutex 'wmu' is still locked at the end of function}}
+    wmu.Lock(); // EXPECTED-NOTE {{mutex acquired here}}
+  } // EXPECTED-WARNING {{mutex 'wmu' is still locked at the end of function}}
   void operator++() {
     wmu.Lock(); // expected-note {{mutex acquired here}}
   } // expected-warning {{mutex 'wmu' is still locked at the end of function}}
@@ -2078,6 +2079,22 @@ public:
   }
 };
 
+
+class LOCKABLE MyLock2 {
+public:
+  Mutex mu_;
+  int foo GUARDED_BY(this);
+
+  // don't check inside lock and unlock functions
+  void lock()   EXCLUSIVE_LOCK_FUNCTION() { mu_.Lock();   }
+  void unlock() UNLOCK_FUNCTION()         { mu_.Unlock(); }
+
+  // don't check inside constructors and destructors
+  MyLock2()  { foo = 1; }
+  ~MyLock2() { foo = 0; }
+};
+
+
 } // end namespace SelfLockingTest