Thread safety analysis: turn on checking within lock and unlock functions.

These checks are enabled with the -Wthread-safety-beta flag.

llvm-svn: 179046
This commit is contained in:
DeLesley Hutchins 2013-04-08 20:11:11 +00:00
parent ed61b06fa8
commit fd374bb3dd
3 changed files with 134 additions and 22 deletions

View File

@ -2279,6 +2279,10 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
// Fill in source locations for all CFGBlocks.
findBlockLocations(CFGraph, SortedGraph, BlockInfo);
MutexIDList ExclusiveLocksAcquired;
MutexIDList SharedLocksAcquired;
MutexIDList LocksReleased;
// Add locks from exclusive_locks_required and shared_locks_required
// to initial lockset. Also turn off checking for lock and unlock functions.
// FIXME: is there a more intelligent way to check lock/unlock functions?
@ -2300,15 +2304,30 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
} else if (SharedLocksRequiredAttr *A
= dyn_cast<SharedLocksRequiredAttr>(Attr)) {
getMutexIDs(SharedLocksToAdd, A, (Expr*) 0, D);
} else if (isa<UnlockFunctionAttr>(Attr)) {
// Don't try to check unlock functions for now
return;
} else if (isa<ExclusiveLockFunctionAttr>(Attr)) {
// Don't try to check lock functions for now
return;
} else if (isa<SharedLockFunctionAttr>(Attr)) {
// Don't try to check lock functions for now
return;
} else if (UnlockFunctionAttr *A = dyn_cast<UnlockFunctionAttr>(Attr)) {
if (!Handler.issueBetaWarnings())
return;
// UNLOCK_FUNCTION() is used to hide the underlying lock implementation.
// We must ignore such methods.
if (A->args_size() == 0)
return;
// FIXME -- deal with exclusive vs. shared unlock functions?
getMutexIDs(ExclusiveLocksToAdd, A, (Expr*) 0, D);
getMutexIDs(LocksReleased, A, (Expr*) 0, D);
} else if (ExclusiveLockFunctionAttr *A
= dyn_cast<ExclusiveLockFunctionAttr>(Attr)) {
if (!Handler.issueBetaWarnings())
return;
if (A->args_size() == 0)
return;
getMutexIDs(ExclusiveLocksAcquired, A, (Expr*) 0, D);
} else if (SharedLockFunctionAttr *A
= dyn_cast<SharedLockFunctionAttr>(Attr)) {
if (!Handler.issueBetaWarnings())
return;
if (A->args_size() == 0)
return;
getMutexIDs(SharedLocksAcquired, A, (Expr*) 0, D);
} else if (isa<ExclusiveTrylockFunctionAttr>(Attr)) {
// Don't try to check trylock functions for now
return;
@ -2491,8 +2510,27 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
if (!Final->Reachable)
return;
// By default, we expect all locks held on entry to be held on exit.
FactSet ExpectedExitSet = Initial->EntrySet;
// Adjust the expected exit set by adding or removing locks, as declared
// by *-LOCK_FUNCTION and UNLOCK_FUNCTION. The intersect below will then
// issue the appropriate warning.
// FIXME: the location here is not quite right.
for (unsigned i=0,n=ExclusiveLocksAcquired.size(); i<n; ++i) {
ExpectedExitSet.addLock(FactMan, ExclusiveLocksAcquired[i],
LockData(D->getLocation(), LK_Exclusive));
}
for (unsigned i=0,n=SharedLocksAcquired.size(); i<n; ++i) {
ExpectedExitSet.addLock(FactMan, SharedLocksAcquired[i],
LockData(D->getLocation(), LK_Shared));
}
for (unsigned i=0,n=LocksReleased.size(); i<n; ++i) {
ExpectedExitSet.removeLock(FactMan, LocksReleased[i]);
}
// FIXME: Should we call this function for all blocks which exit the function?
intersectAndWarn(Initial->EntrySet, Final->ExitSet,
intersectAndWarn(ExpectedExitSet, Final->ExitSet,
Final->ExitLoc,
LEK_LockedAtEndOfFunction,
LEK_NotLockedAtEndOfFunction,

View File

@ -1333,8 +1333,12 @@ class ThreadSafetyReporter : public clang::thread_safety::ThreadSafetyHandler {
LocEndOfScope = FunEndLocation;
PartialDiagnosticAt Warning(LocEndOfScope, S.PDiag(DiagID) << LockName);
PartialDiagnosticAt Note(LocLocked, S.PDiag(diag::note_locked_here));
Warnings.push_back(DelayedDiag(Warning, OptionalNotes(1, Note)));
if (LocLocked.isValid()) {
PartialDiagnosticAt Note(LocLocked, S.PDiag(diag::note_locked_here));
Warnings.push_back(DelayedDiag(Warning, OptionalNotes(1, Note)));
return;
}
Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
}

View File

@ -1475,8 +1475,8 @@ namespace substitution_test {
public:
Mutex mu;
void lockData() __attribute__((exclusive_lock_function(mu))) { }
void unlockData() __attribute__((unlock_function(mu))) { }
void lockData() __attribute__((exclusive_lock_function(mu)));
void unlockData() __attribute__((unlock_function(mu)));
void doSomething() __attribute__((exclusive_locks_required(mu))) { }
};
@ -1484,8 +1484,8 @@ namespace substitution_test {
class DataLocker {
public:
void lockData (MyData *d) __attribute__((exclusive_lock_function(d->mu))) { }
void unlockData(MyData *d) __attribute__((unlock_function(d->mu))) { }
void lockData (MyData *d) __attribute__((exclusive_lock_function(d->mu)));
void unlockData(MyData *d) __attribute__((unlock_function(d->mu)));
};
@ -2858,7 +2858,7 @@ void Foo::lock1() EXCLUSIVE_LOCK_FUNCTION(mu1_) {
}
void Foo::slock1() SHARED_LOCK_FUNCTION(mu1_) {
mu1_.Lock();
mu1_.ReaderLock();
}
void Foo::lock3() EXCLUSIVE_LOCK_FUNCTION(mu1_, mu2_, mu3_) {
@ -3640,8 +3640,8 @@ class Foo {
LOCKS_EXCLUDED(mu2_);
void lock() EXCLUSIVE_LOCK_FUNCTION(mu1_)
EXCLUSIVE_LOCK_FUNCTION(mu2_);
void readerlock() EXCLUSIVE_LOCK_FUNCTION(mu1_)
EXCLUSIVE_LOCK_FUNCTION(mu2_);
void readerlock() SHARED_LOCK_FUNCTION(mu1_)
SHARED_LOCK_FUNCTION(mu2_);
void unlock() UNLOCK_FUNCTION(mu1_)
UNLOCK_FUNCTION(mu2_);
bool trylock() EXCLUSIVE_TRYLOCK_FUNCTION(true, mu1_)
@ -3663,9 +3663,9 @@ void Foo::foo2() {
}
void Foo::foo3() { }
void Foo::lock() { }
void Foo::readerlock() { }
void Foo::unlock() { }
void Foo::lock() { mu1_.Lock(); mu2_.Lock(); }
void Foo::readerlock() { mu1_.ReaderLock(); mu2_.ReaderLock(); }
void Foo::unlock() { mu1_.Unlock(); mu2_.Unlock(); }
bool Foo::trylock() { return true; }
bool Foo::readertrylock() { return true; }
@ -3915,3 +3915,73 @@ void bar2() EXCLUSIVE_LOCKS_REQUIRED(getMutex1(), getMutex2());
} // end namespace UnevaluatedContextTest
namespace LockUnlockFunctionTest {
// Check built-in lock functions
class LOCKABLE MyLockable {
public:
void lock() EXCLUSIVE_LOCK_FUNCTION() { mu_.Lock(); }
void readerLock() SHARED_LOCK_FUNCTION() { mu_.ReaderLock(); }
void unlock() UNLOCK_FUNCTION() { mu_.Unlock(); }
private:
Mutex mu_;
};
class Foo {
public:
// Correct lock/unlock functions
void lock() EXCLUSIVE_LOCK_FUNCTION(mu_) {
mu_.Lock();
}
void readerLock() SHARED_LOCK_FUNCTION(mu_) {
mu_.ReaderLock();
}
void unlock() UNLOCK_FUNCTION(mu_) {
mu_.Unlock();
}
// Check failure to lock.
void lockBad() EXCLUSIVE_LOCK_FUNCTION(mu_) { // expected-note {{mutex acquired here}}
mu2_.Lock();
mu2_.Unlock();
} // expected-warning {{expecting mutex 'mu_' to be locked at the end of function}}
void readerLockBad() SHARED_LOCK_FUNCTION(mu_) { // expected-note {{mutex acquired here}}
mu2_.Lock();
mu2_.Unlock();
} // expected-warning {{expecting mutex 'mu_' to be locked at the end of function}}
void unlockBad() UNLOCK_FUNCTION(mu_) { // expected-note {{mutex acquired here}}
mu2_.Lock();
mu2_.Unlock();
} // expected-warning {{mutex 'mu_' is still locked at the end of function}}
// Check locking the wrong thing.
void lockBad2() EXCLUSIVE_LOCK_FUNCTION(mu_) { // expected-note {{mutex acquired here}}
mu2_.Lock(); // expected-note {{mutex acquired here}}
} // expected-warning {{expecting mutex 'mu_' to be locked at the end of function}} \
// expected-warning {{mutex 'mu2_' is still locked at the end of function}}
void readerLockBad2() SHARED_LOCK_FUNCTION(mu_) { // expected-note {{mutex acquired here}}
mu2_.ReaderLock(); // expected-note {{mutex acquired here}}
} // expected-warning {{expecting mutex 'mu_' to be locked at the end of function}} \
// expected-warning {{mutex 'mu2_' is still locked at the end of function}}
void unlockBad2() UNLOCK_FUNCTION(mu_) { // expected-note {{mutex acquired here}}
mu2_.Unlock(); // expected-warning {{unlocking 'mu2_' that was not locked}}
} // expected-warning {{mutex 'mu_' is still locked at the end of function}}
private:
Mutex mu_;
Mutex mu2_;
};
} // end namespace LockUnlockFunctionTest