diff options
-rw-r--r-- | lib/Analysis/ThreadSafety.cpp | 39 | ||||
-rw-r--r-- | test/SemaCXX/warn-thread-safety-analysis.cpp | 21 |
2 files changed, 56 insertions, 4 deletions
diff --git a/lib/Analysis/ThreadSafety.cpp b/lib/Analysis/ThreadSafety.cpp index 6f76f1b947..69ea830bdd 100644 --- a/lib/Analysis/ThreadSafety.cpp +++ b/lib/Analysis/ThreadSafety.cpp @@ -1429,6 +1429,7 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) { // union because the real error is probably that we forgot to unlock M on // all code paths. bool LocksetInitialized = false; + llvm::SmallVector<CFGBlock*, 8> SpecialBlocks; for (CFGBlock::const_pred_iterator PI = CurrBlock->pred_begin(), PE = CurrBlock->pred_end(); PI != PE; ++PI) { @@ -1436,6 +1437,17 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) { if (*PI == 0 || !VisitedBlocks.alreadySet(*PI)) continue; + // If the previous block ended in a 'continue' or 'break' statement, then + // a difference in locksets is probably due to a bug in that block, rather + // than in some other predecessor. In that case, keep the other + // predecessor's lockset. + if (const Stmt *Terminator = (*PI)->getTerminator()) { + if (isa<ContinueStmt>(Terminator) || isa<BreakStmt>(Terminator)) { + SpecialBlocks.push_back(*PI); + continue; + } + } + int PrevBlockID = (*PI)->getBlockID(); CFGBlockInfo *PrevBlockInfo = &BlockInfo[PrevBlockID]; @@ -1449,6 +1461,33 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) { } } + // Process continue and break blocks. Assume that the lockset for the + // resulting block is unaffected by any discrepancies in them. + for (unsigned SpecialI = 0, SpecialN = SpecialBlocks.size(); + SpecialI < SpecialN; ++SpecialI) { + CFGBlock *PrevBlock = SpecialBlocks[SpecialI]; + int PrevBlockID = PrevBlock->getBlockID(); + CFGBlockInfo *PrevBlockInfo = &BlockInfo[PrevBlockID]; + + if (!LocksetInitialized) { + CurrBlockInfo->EntrySet = PrevBlockInfo->ExitSet; + LocksetInitialized = true; + } else { + // Determine whether this edge is a loop terminator for diagnostic + // purposes. FIXME: A 'break' statement might be a loop terminator, but + // it might also be part of a switch. Also, a subsequent destructor + // might add to the lockset, in which case the real issue might be a + // double lock on the other path. + const Stmt *Terminator = PrevBlock->getTerminator(); + bool IsLoop = Terminator && isa<ContinueStmt>(Terminator); + + // Do not update EntrySet. + intersectAndWarn(CurrBlockInfo->EntrySet, PrevBlockInfo->ExitSet, + IsLoop ? LEK_LockedSomeLoopIterations + : LEK_LockedSomePredecessors); + } + } + BuildLockset LocksetBuilder(this, *CurrBlockInfo); CFGBlock::const_pred_iterator PI = CurrBlock->pred_begin(), PE = CurrBlock->pred_end(); diff --git a/test/SemaCXX/warn-thread-safety-analysis.cpp b/test/SemaCXX/warn-thread-safety-analysis.cpp index dfec94ca91..634a233430 100644 --- a/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -208,8 +208,7 @@ void sls_fun_bad_6() { } void sls_fun_bad_7() { - sls_mu.Lock(); // \ - // expected-warning{{expecting mutex 'sls_mu' to be locked at start of each loop}} + sls_mu.Lock(); while (getBool()) { sls_mu.Unlock(); if (getBool()) { @@ -218,7 +217,7 @@ void sls_fun_bad_7() { } } sls_mu.Lock(); // \ - // expected-warning{{mutex 'sls_mu' is still locked at the end of its scope}} + // expected-warning{{expecting mutex 'sls_mu' to be locked at start of each loop}} } sls_mu.Unlock(); } @@ -257,6 +256,21 @@ void sls_fun_bad_11() { // expected-warning{{unlocking 'sls_mu' that was not locked}} } +void sls_fun_bad_12() { + sls_mu.Lock(); // \ + // expected-warning{{mutex 'sls_mu' is still locked at the end of its scope}} + while (getBool()) { + sls_mu.Unlock(); + if (getBool()) { + if (getBool()) { + break; + } + } + sls_mu.Lock(); + } + sls_mu.Unlock(); +} + //-----------------------------------------// // Handling lock expressions in attribute args // -------------------------------------------// @@ -1900,4 +1914,3 @@ void test() { } }; - |