[cfe-commits] r150701 - in /cfe/trunk: lib/Analysis/ThreadSafety.cpp test/SemaCXX/warn-thread-safety-analysis.cpp

[DeLesley Hutchins]

Author: delesley
Date: Thu Feb 16 11:13:43 2012
New Revision: 150701

URL: http://llvm.org/viewvc/llvm-project?rev=150701&view=rev
Log:
Thread-safety analysis: Disable checking inside constructors, destructors, lock, and unlock functions

Modified:
    cfe/trunk/lib/Analysis/ThreadSafety.cpp
    cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp

Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=150701&r1=150700&r2=150701&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Thu Feb 16 11:13:43 2012
@@ -1432,6 +1432,14 @@
     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 @@
   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;
       }
     }
   }

Modified: cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp?rev=150701&r1=150700&r2=150701&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Thu Feb 16 11:13:43 2012
@@ -313,12 +313,13 @@
 
 // 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 @@
   }
 };
 
+
+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