[cfe-commits] r159008 - in /cfe/trunk: lib/Analysis/ThreadSafety.cpp test/SemaCXX/warn-thread-safety-analysis.cpp
DeLesley Hutchins
delesley at google.com
Fri Jun 22 10:07:28 PDT 2012
Author: delesley
Date: Fri Jun 22 12:07:28 2012
New Revision: 159008
URL: http://llvm.org/viewvc/llvm-project?rev=159008&view=rev
Log:
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.
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=159008&r1=159007&r2=159008&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Fri Jun 22 12:07:28 2012
@@ -898,13 +898,13 @@
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 @@
}
-/// \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 @@
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 @@
}
+
/// \brief Compute the intersection of two locksets and issue warnings for any
/// locks in the symmetric difference.
///
@@ -1466,15 +1471,17 @@
/// 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 @@
} else {
Handler.handleMutexHeldEndOfScope(LSet2Mutex.getName(),
LSet2LockData.AcquireLoc,
- Block1.getLocation(Side1), LEK);
+ JoinLoc, LEK);
}
}
@@ -1500,7 +1507,7 @@
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 @@
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 @@
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 @@
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 @@
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);
}
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=159008&r1=159007&r2=159008&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Fri Jun 22 12:07:28 2012
@@ -178,14 +178,11 @@
void sls_fun_bad_4() {
if (getBool())
- sls_mu.Lock(); // \
- expected-warning{{mutex 'sls_mu2' is not locked on every path through here}} \
- expected-note{{mutex acquired here}}
-
+ sls_mu.Lock(); // expected-note{{mutex acquired here}}
else
- sls_mu2.Lock(); // \
- expected-note{{mutex acquired here}}
-} // expected-warning{{mutex 'sls_mu' is not locked on every path through here}}
+ sls_mu2.Lock(); // expected-note{{mutex acquired here}}
+} // expected-warning{{mutex 'sls_mu' is not locked on every path through here}} \
+ // expected-warning{{mutex 'sls_mu2' is not locked on every path through here}}
void sls_fun_bad_5() {
sls_mu.Lock(); // expected-note {{mutex acquired here}}
@@ -226,15 +223,14 @@
void sls_fun_bad_8() {
sls_mu.Lock(); // expected-note{{mutex acquired here}}
- // FIXME: TERRIBLE SOURCE LOCATION!
- do { // expected-warning{{expecting mutex 'sls_mu' to be locked at start of each loop}}
- sls_mu.Unlock();
+ do {
+ sls_mu.Unlock(); // expected-warning{{expecting mutex 'sls_mu' to be locked at start of each loop}}
} while (getBool());
}
void sls_fun_bad_9() {
do {
- sls_mu.Lock(); // \
+ sls_mu.Lock(); // \
// expected-warning{{expecting mutex 'sls_mu' to be locked at start of each loop}} \
// expected-note{{mutex acquired here}}
} while (getBool());
@@ -242,15 +238,15 @@
}
void sls_fun_bad_10() {
- sls_mu.Lock(); // expected-note 2{{mutex acquired here}}
- while(getBool()) {
- sls_mu.Unlock(); // expected-warning{{expecting mutex 'sls_mu' to be locked at start of each loop}}
+ sls_mu.Lock(); // expected-note 2{{mutex acquired here}}
+ while(getBool()) { // expected-warning{{expecting mutex 'sls_mu' to be locked at start of each loop}}
+ sls_mu.Unlock();
}
} // expected-warning{{mutex 'sls_mu' is still locked at the end of function}}
void sls_fun_bad_11() {
while (getBool()) { // \
- expected-warning{{expecting mutex 'sls_mu' to be locked at start of each loop}}
+ expected-warning{{expecting mutex 'sls_mu' to be locked at start of each loop}}
sls_mu.Lock(); // expected-note {{mutex acquired here}}
}
sls_mu.Unlock(); // \
@@ -2239,6 +2235,26 @@
}
-} // end namespace
+} // end namespace MoreLockExpressions
+
+
+namespace TrylockJoinPoint {
+
+class Foo {
+ Mutex mu;
+ bool c;
+
+ void foo() {
+ if (c) {
+ if (!mu.TryLock())
+ return;
+ } else {
+ mu.Lock();
+ }
+ mu.Unlock();
+ }
+};
+
+} // end namespace TrylockJoinPoint
More information about the cfe-commits
mailing list