maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

Thread safety: refactoring various out of scope warnings to use the same inteface. This eliminates a lot of unnecessary duplicated code. 

llvm-svn: 139801
This commit is contained in:
Caitlin Sadowski 2011-09-15 17:25:19 +00:00
parent 76abb3b559
commit af9b7c5f8b
5 changed files with 78 additions and 129 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
clang/include/clang/Analysis/Analyses/ThreadSafety.h
View File

@ -49,6 +49,20 @@ enum AccessKind {
AK_Written /// Writing a variable
};
/// This enum distinguishes between different situations where we warn due to
/// inconsistent locking.
/// \enum SK_LockedSomeLoopIterations -- a mutex is locked for some but not all
/// loop iterations.
/// \enum SK_LockedSomePredecessors -- a mutex is locked in some but not all
/// predecessors of a CFGBlock.
/// \enum SK_LockedAtEndOfFunction -- a mutex is still locked at the end of a
/// function.
enum LockErrorKind {
LEK_LockedSomeLoopIterations,
LEK_LockedSomePredecessors,
LEK_LockedAtEndOfFunction
};
/// Handler class for thread safety warnings.
class ThreadSafetyHandler {
public:
@ -73,27 +87,16 @@ public:
virtual void handleDoubleLock(Name LockName, SourceLocation Loc) {}
/// Warn about situations where a mutex is sometimes held and sometimes not.
/// For example, a mutex is locked on an "if" branch but not the "else"
/// branch.
/// The three situations are:
/// 1. a mutex is locked on an "if" branch but not the "else" branch,
/// 2, or a mutex is only held at the start of some loop iterations,
/// 3. or when a mutex is locked but not unlocked inside a function.
/// \param LockName -- A StringRef name for the lock expression, to be printed
/// in the error message.
/// \param Loc -- The location of the lock expression where the mutex is
/// locked
virtual void handleMutexHeldEndOfScope(Name LockName, SourceLocation Loc){}
/// Warn when a mutex is only held at the start of some loop iterations.
/// \param LockName -- A StringRef name for the lock expression, to be printed
/// in the error message.
/// \param Loc -- The Loc of the lock expression.
virtual void handleNoLockLoopEntry(Name LockName, SourceLocation Loc) {}
/// Warn when a mutex is locked but not unlocked inside a function.
/// \param LockName -- A StringRef name for the lock expression, to be printed
/// in the error message.
/// \param FunName -- The name of the function
/// \param Loc -- The location of the lock expression
virtual void handleNoUnlock(Name LockName, Name FunName,
SourceLocation Loc) {}
/// \param Loc -- The location of the lock expression where the mutex is locked
/// \param LEK -- which of the three above cases we should warn for
virtual void handleMutexHeldEndOfScope(Name LockName, SourceLocation Loc,
LockErrorKind LEK){}
/// Warn when a mutex is held exclusively and shared at the same point. For
/// example, if a mutex is locked exclusively during an if branch and shared

2
clang/include/clang/Basic/DiagnosticSemaKinds.td
View File

@ -1398,7 +1398,7 @@ def warn_double_lock : Warning<
"locking '%0' that is already locked">,
InGroup<ThreadSafety>, DefaultIgnore;
def warn_no_unlock : Warning<
"mutex '%0' is still held at the end of function '%1'">,
"mutex '%0' is still held at the end of function">,
InGroup<ThreadSafety>, DefaultIgnore;
// FIXME: improve the error message about locks not in scope
def warn_lock_at_end_of_scope : Warning<

101
clang/lib/Analysis/ThreadSafety.cpp
View File

@ -598,13 +598,18 @@ void BuildLockset::VisitCXXMemberCallExpr(CXXMemberCallExpr *Exp) {
} // end anonymous namespace
/// \brief Flags a warning for each lock that is in LSet2 but not LSet1, or
/// else mutexes that are held shared in one lockset and exclusive in the other.
static Lockset warnIfNotInFirstSetOrNotSameKind(ThreadSafetyHandler &Handler,
const Lockset LSet1,
const Lockset LSet2,
Lockset Intersection,
Lockset::Factory &Fact) {
/// \brief Compute the intersection of two locksets and issue warnings for any
/// locks in the symmetric difference.
///
/// This function is used at a merge point in the CFG when comparing the lockset
/// of each branch being merged. For example, given the following sequence:
/// 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.
static Lockset intersectAndWarn(ThreadSafetyHandler &Handler,
const Lockset LSet1, const Lockset LSet2,
Lockset::Factory &Fact, LockErrorKind LEK) {
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();
@ -618,34 +623,16 @@ static Lockset warnIfNotInFirstSetOrNotSameKind(ThreadSafetyHandler &Handler,
}
} else {
Handler.handleMutexHeldEndOfScope(LSet2Mutex.getName(),
LSet2LockData.AcquireLoc);
LSet2LockData.AcquireLoc, LEK);
}
}
return Intersection;
}
/// \brief Compute the intersection of two locksets and issue warnings for any
/// locks in the symmetric difference.
///
/// This function is used at a merge point in the CFG when comparing the lockset
/// of each branch being merged. For example, given the following sequence:
/// 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.
static Lockset intersectAndWarn(ThreadSafetyHandler &Handler,
const Lockset LSet1, const Lockset LSet2,
Lockset::Factory &Fact) {
Lockset Intersection = LSet1;
Intersection = warnIfNotInFirstSetOrNotSameKind(Handler, LSet1, LSet2,
Intersection, Fact);
for (Lockset::iterator I = LSet1.begin(), E = LSet1.end(); I != E; ++I) {
if (!LSet2.contains(I.getKey())) {
const MutexID &Mutex = I.getKey();
const LockData &MissingLock = I.getData();
Handler.handleMutexHeldEndOfScope(Mutex.getName(),
MissingLock.AcquireLoc);
MissingLock.AcquireLoc, LEK);
Intersection = Fact.remove(Intersection, Mutex);
}
}
@ -669,34 +656,6 @@ static SourceLocation getFirstStmtLocation(CFGBlock *Block) {
return Loc;
}
/// \brief Warn about different locksets along backedges of loops.
/// This function is called when we encounter a back edge. At that point,
/// we need to verify that the lockset before taking the backedge is the
/// same as the lockset before entering the loop.
///
/// \param LoopEntrySet Locks before starting the loop
/// \param LoopReentrySet Locks in the last CFG block of the loop
static void warnBackEdgeUnequalLocksets(ThreadSafetyHandler &Handler,
const Lockset LoopReentrySet,
const Lockset LoopEntrySet,
SourceLocation FirstLocInLoop,
Lockset::Factory &Fact) {
assert(FirstLocInLoop.isValid());
// Warn for locks held at the start of the loop, but not the end.
for (Lockset::iterator I = LoopEntrySet.begin(), E = LoopEntrySet.end();
I != E; ++I) {
if (!LoopReentrySet.contains(I.getKey())) {
// We report this error at the location of the first statement in a loop
Handler.handleNoLockLoopEntry(I.getKey().getName(), FirstLocInLoop);
}
}
// Warn for locks held at the end of the loop, but not at the start.
warnIfNotInFirstSetOrNotSameKind(Handler, LoopEntrySet, LoopReentrySet,
LoopReentrySet, Fact);
}
namespace clang {
namespace thread_safety {
/// \brief Check a function's CFG for thread-safety violations.
@ -763,7 +722,8 @@ void runThreadSafetyAnalysis(AnalysisContext &AC,
LocksetInitialized = true;
} else {
Entryset = intersectAndWarn(Handler, Entryset,
ExitLocksets[PrevBlockID], LocksetFactory);
ExitLocksets[PrevBlockID], LocksetFactory,
LEK_LockedSomePredecessors);
}
}
@ -787,36 +747,17 @@ void runThreadSafetyAnalysis(AnalysisContext &AC,
continue;
CFGBlock *FirstLoopBlock = *SI;
SourceLocation FirstLoopLocation = getFirstStmtLocation(FirstLoopBlock);
assert(FirstLoopLocation.isValid());
// Fail gracefully in release code.
if (!FirstLoopLocation.isValid())
continue;
Lockset PreLoop = EntryLocksets[FirstLoopBlock->getBlockID()];
Lockset LoopEnd = ExitLocksets[CurrBlockID];
warnBackEdgeUnequalLocksets(Handler, LoopEnd, PreLoop, FirstLoopLocation,
LocksetFactory);
intersectAndWarn(Handler, LoopEnd, PreLoop, LocksetFactory,
LEK_LockedSomeLoopIterations);
}
}
Lockset InitialLockset = EntryLocksets[CFGraph->getEntry().getBlockID()];
Lockset FinalLockset = ExitLocksets[CFGraph->getExit().getBlockID()];
if (!FinalLockset.isEmpty()) {
for (Lockset::iterator I=FinalLockset.begin(), E=FinalLockset.end();
I != E; ++I) {
const MutexID &Mutex = I.getKey();
const LockData &MissingLock = I.getData();
std::string FunName = "<unknown>";
if (const NamedDecl *ContextDecl = dyn_cast<NamedDecl>(AC.getDecl())) {
FunName = ContextDecl->getDeclName().getAsString();
}
Handler.handleNoUnlock(Mutex.getName(), FunName, MissingLock.AcquireLoc);
}
}
intersectAndWarn(Handler, InitialLockset, FinalLockset, LocksetFactory,
LEK_LockedAtEndOfFunction);
}
/// \brief Helper function that returns a LockKind required for the given level

27
clang/lib/Sema/AnalysisBasedWarnings.cpp
View File

@ -633,20 +633,23 @@ class ThreadSafetyReporter : public clang::thread_safety::ThreadSafetyHandler {
warnLockMismatch(diag::warn_double_lock, LockName, Loc);
}
void handleMutexHeldEndOfScope(Name LockName, SourceLocation Loc){
warnLockMismatch(diag::warn_lock_at_end_of_scope, LockName, Loc);
void handleMutexHeldEndOfScope(Name LockName, SourceLocation Loc,
LockErrorKind LEK){
unsigned DiagID = 0;
switch (LEK) {
case LEK_LockedSomePredecessors:
DiagID = diag::warn_lock_at_end_of_scope;
break;
case LEK_LockedSomeLoopIterations:
DiagID = diag::warn_expecting_lock_held_on_loop;
break;
case LEK_LockedAtEndOfFunction:
DiagID = diag::warn_no_unlock;
break;
}
warnLockMismatch(DiagID, LockName, Loc);
}
void handleNoLockLoopEntry(Name LockName, SourceLocation Loc) {
warnLockMismatch(diag::warn_expecting_lock_held_on_loop, LockName, Loc);
}
void handleNoUnlock(Name LockName, llvm::StringRef FunName,
SourceLocation Loc) {
PartialDiagnostic Warning =
S.PDiag(diag::warn_no_unlock) << LockName << FunName;
Warnings.push_back(DelayedDiag(Loc, Warning));
}
void handleExclusiveAndShared(Name LockName, SourceLocation Loc1,
SourceLocation Loc2) {

36
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
View File

@ -176,9 +176,9 @@ void sls_fun_bad_6() {
}
void sls_fun_bad_7() {
sls_mu.Lock();
while (getBool()) { // \
// expected-warning{{expecting lock on 'sls_mu' to be held at start of each loop}}
sls_mu.Lock(); // \
// expected-warning{{expecting lock on 'sls_mu' to be held at start of each loop}}
while (getBool()) {
sls_mu.Unlock();
if (getBool()) {
if (getBool()) {
@ -192,26 +192,26 @@ void sls_fun_bad_7() {
}
void sls_fun_bad_8() {
sls_mu.Lock();
sls_mu.Lock(); // \
// expected-warning{{expecting lock on 'sls_mu' to be held at start of each loop}}
do {
sls_mu.Unlock(); // \
// expected-warning{{expecting lock on 'sls_mu' to be held at start of each loop}}
sls_mu.Unlock();
} while (getBool());
}
void sls_fun_bad_9() {
do {
sls_mu.Lock(); // \
// expected-warning{{mutex 'sls_mu' is still held at the end of its scope}}
// expected-warning{{expecting lock on 'sls_mu' to be held at start of each loop}}
} while (getBool());
sls_mu.Unlock();
}
void sls_fun_bad_10() {
sls_mu.Lock(); // \
// expected-warning{{mutex 'sls_mu' is still held at the end of function 'sls_fun_bad_10'}}
while(getBool()) { // \
// expected-warning{{expecting lock on 'sls_mu' to be held at start of each loop}}
// expected-warning{{mutex 'sls_mu' is still held at the end of function 'sls_fun_bad_10'}} \
// expected-warning{{expecting lock on 'sls_mu' to be held at start of each loop}}
while(getBool()) {
sls_mu.Unlock();
}
}
@ -219,7 +219,7 @@ void sls_fun_bad_10() {
void sls_fun_bad_11() {
while (getBool()) {
sls_mu.Lock(); // \
// expected-warning{{mutex 'sls_mu' is still held at the end of its scope}}
// expected-warning{{expecting lock on 'sls_mu' to be held at start of each loop}}
}
sls_mu.Unlock(); // \
// expected-warning{{unlocking 'sls_mu' that was not locked}}
@ -519,11 +519,12 @@ void shared_fun_0() {
}
void shared_fun_1() {
sls_mu.ReaderLock();
sls_mu.ReaderLock(); // \
// expected-warning {{lock 'sls_mu' is exclusive and shared in the same scope}}
do {
sls_mu.Unlock();
sls_mu.Lock(); // \
// expected-warning {{lock 'sls_mu' is exclusive and shared in the same scope}}
sls_mu.Lock(); // \
// expected-note {{the other lock of mutex 'sls_mu' is here}}
} while (getBool());
sls_mu.Unlock();
}
@ -557,11 +558,12 @@ void shared_fun_8() {
}
void shared_bad_0() {
sls_mu.Lock();
sls_mu.Lock(); // \
// expected-warning {{lock 'sls_mu' is exclusive and shared in the same scope}}
do {
sls_mu.Unlock();
sls_mu.ReaderLock(); // \
// expected-warning {{lock 'sls_mu' is exclusive and shared in the same scope}}
sls_mu.ReaderLock(); // \
// expected-note {{the other lock of mutex 'sls_mu' is here}}
} while (getBool());
sls_mu.Unlock();
}