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.

llvm-svn: 159008
This commit is contained in:
DeLesley Hutchins 2012-06-22 17:07:28 +00:00
parent 804b2ab79b
commit ebbf77016d
2 changed files with 82 additions and 60 deletions

View File

@ -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);
}

View File

@ -178,14 +178,11 @@ void sls_fun_bad_3() {
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_7() {
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_9() {
}
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 @@ void test() {
}
} // 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