r179046 - Thread safety analysis: turn on checking within lock and unlock functions.
DeLesley Hutchins
delesley at google.com
Mon Apr 8 13:11:11 PDT 2013
Author: delesley
Date: Mon Apr 8 15:11:11 2013
New Revision: 179046
URL: http://llvm.org/viewvc/llvm-project?rev=179046&view=rev
Log:
Thread safety analysis: turn on checking within lock and unlock functions.
These checks are enabled with the -Wthread-safety-beta flag.
Modified:
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/lib/Sema/AnalysisBasedWarnings.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=179046&r1=179045&r2=179046&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Mon Apr 8 15:11:11 2013
@@ -2279,6 +2279,10 @@ void ThreadSafetyAnalyzer::runAnalysis(A
// 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(A
} 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(A
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,
Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=179046&r1=179045&r2=179046&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
+++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Mon Apr 8 15:11:11 2013
@@ -1333,8 +1333,12 @@ class ThreadSafetyReporter : public clan
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()));
}
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=179046&r1=179045&r2=179046&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Mon Apr 8 15:11:11 2013
@@ -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_FUNCTI
}
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(get
} // 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
+
More information about the cfe-commits
mailing list