Thread safety analysis: fixes a bug in which locksets are not handled

properly if there is a join point in the control flow graph that involves
a trylock.  Also changes the source locations of some warnings to be
more consistent.

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

1 files changed, 50 insertions, 44 deletions

diff --git a/lib/Analysis/ThreadSafety.cpp b/lib/Analysis/ThreadSafety.cpp
index 80bebb6144..075324da5f 100644
--- a/lib/Analysis/ThreadSafety.cpp
+++ b/lib/Analysis/ThreadSafety.cpp
@@ -898,13 +898,13 @@ public:
                      Expr *BrE, bool Neg);
   const CallExpr* getTrylockCallExpr(const Stmt *Cond, LocalVarContext C,
                                      bool &Negate);
-  Lockset handleTrylock(const Lockset &LSet,
-                        const CFGBlock* PredBlock,
-                        const CFGBlock *CurrBlock);
 
-  Lockset intersectAndWarn(const CFGBlockInfo &Block1, CFGBlockSide Side1,
-                           const CFGBlockInfo &Block2, CFGBlockSide Side2,
-                           LockErrorKind LEK);
+  Lockset getEdgeLockset(const Lockset &ExitSet,
+                         const CFGBlock* PredBlock,
+                         const CFGBlock *CurrBlock);
+
+  Lockset intersectAndWarn(const Lockset &LSet1, const Lockset &LSet2,
+                           SourceLocation JoinLoc, LockErrorKind LEK);
 
   void runAnalysis(AnalysisDeclContext &AC);
 };
@@ -1106,11 +1106,15 @@ const CallExpr* ThreadSafetyAnalyzer::getTrylockCallExpr(const Stmt *Cond,
 }
 
 
-/// \brief Process a conditional branch from a previous block to the current
-/// block, looking for trylock calls.
-Lockset ThreadSafetyAnalyzer::handleTrylock(const Lockset &LSet,
-                                            const CFGBlock *PredBlock,
-                                            const CFGBlock *CurrBlock) {
+/// \brief Find the lockset that holds on the edge between PredBlock
+/// and CurrBlock.  The edge set is the exit set of PredBlock (passed
+/// as the ExitSet parameter) plus any trylocks, which are conditionally held.
+Lockset ThreadSafetyAnalyzer::getEdgeLockset(const Lockset &ExitSet,
+                                             const CFGBlock *PredBlock,
+                                             const CFGBlock *CurrBlock) {
+  if (!PredBlock->getTerminatorCondition())
+    return ExitSet;
+
   bool Negate = false;
   const Stmt *Cond = PredBlock->getTerminatorCondition();
   const CFGBlockInfo *PredBlockInfo = &BlockInfo[PredBlock->getBlockID()];
@@ -1119,13 +1123,13 @@ Lockset ThreadSafetyAnalyzer::handleTrylock(const Lockset &LSet,
   CallExpr *Exp = const_cast<CallExpr*>(
     getTrylockCallExpr(Cond, LVarCtx, Negate));
   if (!Exp)
-    return LSet;
+    return ExitSet;
 
   NamedDecl *FunDecl = dyn_cast_or_null<NamedDecl>(Exp->getCalleeDecl());
   if(!FunDecl || !FunDecl->hasAttrs())
-    return LSet;
+    return ExitSet;
 
-  Lockset Result = LSet;
+  Lockset Result = ExitSet;
 
   // If the condition is a call to a Trylock function, then grab the attributes
   AttrVec &ArgAttrs = FunDecl->getAttrs();
@@ -1458,6 +1462,7 @@ void BuildLockset::VisitDeclStmt(DeclStmt *S) {
 }
 
 
+
 /// \brief Compute the intersection of two locksets and issue warnings for any
 /// locks in the symmetric difference.
 ///
@@ -1466,15 +1471,17 @@ void BuildLockset::VisitDeclStmt(DeclStmt *S) {
 /// A; if () then B; else C; D; we need to check that the lockset after B and C
 /// are the same. In the event of a difference, we use the intersection of these
 /// two locksets at the start of D.
-Lockset ThreadSafetyAnalyzer::intersectAndWarn(const CFGBlockInfo &Block1,
-                                               CFGBlockSide Side1,
-                                               const CFGBlockInfo &Block2,
-                                               CFGBlockSide Side2,
+///
+/// \param LSet1 The first lockset.
+/// \param LSet2 The second lockset.
+/// \param JoinLoc The location of the join point for error reporting
+/// \param LEK The error message to report.
+Lockset ThreadSafetyAnalyzer::intersectAndWarn(const Lockset &LSet1,
+                                               const Lockset &LSet2,
+                                               SourceLocation JoinLoc,
                                                LockErrorKind LEK) {
-  Lockset LSet1 = Block1.getSet(Side1);
-  Lockset LSet2 = Block2.getSet(Side2);
-
   Lockset Intersection = LSet1;
+
   for (Lockset::iterator I = LSet2.begin(), E = LSet2.end(); I != E; ++I) {
     const MutexID &LSet2Mutex = I.getKey();
     const LockData &LSet2LockData = I.getData();
@@ -1490,7 +1497,7 @@ Lockset ThreadSafetyAnalyzer::intersectAndWarn(const CFGBlockInfo &Block1,
     } else {
       Handler.handleMutexHeldEndOfScope(LSet2Mutex.getName(),
                                         LSet2LockData.AcquireLoc,
-                                        Block1.getLocation(Side1), LEK);
+                                        JoinLoc, LEK);
     }
   }
 
@@ -1500,7 +1507,7 @@ Lockset ThreadSafetyAnalyzer::intersectAndWarn(const CFGBlockInfo &Block1,
       const LockData &MissingLock = I.getData();
       Handler.handleMutexHeldEndOfScope(Mutex.getName(),
                                         MissingLock.AcquireLoc,
-                                        Block2.getLocation(Side2), LEK);
+                                        JoinLoc, LEK);
       Intersection = LocksetFactory.remove(Intersection, Mutex);
     }
   }
@@ -1518,6 +1525,8 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
   if (!CFGraph) return;
   const NamedDecl *D = dyn_cast_or_null<NamedDecl>(AC.getDecl());
 
+  // AC.dumpCFG(true);
+
   if (!D)
     return;  // Ignore anonymous functions for now.
   if (D->getAttr<NoThreadSafetyAnalysisAttr>())
@@ -1631,14 +1640,16 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
 
       int PrevBlockID = (*PI)->getBlockID();
       CFGBlockInfo *PrevBlockInfo = &BlockInfo[PrevBlockID];
+      Lockset PrevLockset =
+        getEdgeLockset(PrevBlockInfo->ExitSet, *PI, CurrBlock);
 
       if (!LocksetInitialized) {
-        CurrBlockInfo->EntrySet = PrevBlockInfo->ExitSet;
+        CurrBlockInfo->EntrySet = PrevLockset;
         LocksetInitialized = true;
       } else {
         CurrBlockInfo->EntrySet =
-          intersectAndWarn(*CurrBlockInfo, CBS_Entry,
-                           *PrevBlockInfo, CBS_Exit,
+          intersectAndWarn(CurrBlockInfo->EntrySet, PrevLockset,
+                           CurrBlockInfo->EntryLoc,
                            LEK_LockedSomePredecessors);
       }
     }
@@ -1663,24 +1674,17 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
         const Stmt *Terminator = PrevBlock->getTerminator();
         bool IsLoop = Terminator && isa<ContinueStmt>(Terminator);
 
+        Lockset PrevLockset =
+          getEdgeLockset(PrevBlockInfo->ExitSet, PrevBlock, CurrBlock);
+
         // Do not update EntrySet.
-        intersectAndWarn(*CurrBlockInfo, CBS_Entry, *PrevBlockInfo, CBS_Exit,
+        intersectAndWarn(CurrBlockInfo->EntrySet, PrevLockset,
+                         PrevBlockInfo->ExitLoc,
                          IsLoop ? LEK_LockedSomeLoopIterations
                                 : LEK_LockedSomePredecessors);
       }
     }
 
-    // If the previous block ended in a trylock, then grab any extra mutexes
-    // from the trylock.
-    for (CFGBlock::const_pred_iterator PI = CurrBlock->pred_begin(),
-         PE = CurrBlock->pred_end(); PI != PE; ++PI) {
-      // If the predecessor ended in a branch, then process any trylocks.
-      if ((*PI)->getTerminatorCondition()) {
-        CurrBlockInfo->EntrySet = handleTrylock(CurrBlockInfo->EntrySet,
-                                                *PI, CurrBlock);
-      }
-    }
-
     BuildLockset LocksetBuilder(this, *CurrBlockInfo);
 
     // Visit all the statements in the basic block.
@@ -1725,18 +1729,20 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
         continue;
 
       CFGBlock *FirstLoopBlock = *SI;
-      CFGBlockInfo &PreLoop = BlockInfo[FirstLoopBlock->getBlockID()];
-      CFGBlockInfo &LoopEnd = BlockInfo[CurrBlockID];
-      intersectAndWarn(LoopEnd, CBS_Exit, PreLoop, CBS_Entry,
+      CFGBlockInfo *PreLoop = &BlockInfo[FirstLoopBlock->getBlockID()];
+      CFGBlockInfo *LoopEnd = &BlockInfo[CurrBlockID];
+      intersectAndWarn(LoopEnd->ExitSet, PreLoop->EntrySet,
+                       PreLoop->EntryLoc,
                        LEK_LockedSomeLoopIterations);
     }
   }
 
-  CFGBlockInfo &Initial = BlockInfo[CFGraph->getEntry().getBlockID()];
-  CFGBlockInfo &Final = BlockInfo[CFGraph->getExit().getBlockID()];
+  CFGBlockInfo *Initial = &BlockInfo[CFGraph->getEntry().getBlockID()];
+  CFGBlockInfo *Final   = &BlockInfo[CFGraph->getExit().getBlockID()];
 
   // FIXME: Should we call this function for all blocks which exit the function?
-  intersectAndWarn(Initial, CBS_Entry, Final, CBS_Exit,
+  intersectAndWarn(Initial->EntrySet, Final->ExitSet,
+                   Final->ExitLoc,
                    LEK_LockedAtEndOfFunction);
 }