r205274 - [analyzer] Add double-unlock detection to PthreadLockChecker.
Jordan Rose
jordan_rose at apple.com
Mon Mar 31 20:40:39 PDT 2014
Author: jrose
Date: Mon Mar 31 22:40:38 2014
New Revision: 205274
URL: http://llvm.org/viewvc/llvm-project?rev=205274&view=rev
Log:
[analyzer] Add double-unlock detection to PthreadLockChecker.
We've decided to punt on supporting recursive locks for now; the common case
is non-recursive.
Patch by Daniel Fahlgren!
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
cfe/trunk/test/Analysis/pthreadlock.c
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp?rev=205274&r1=205273&r2=205274&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp Mon Mar 31 22:40:38 2014
@@ -26,6 +26,7 @@ using namespace ento;
namespace {
class PthreadLockChecker : public Checker< check::PostStmt<CallExpr> > {
mutable std::unique_ptr<BugType> BT_doublelock;
+ mutable std::unique_ptr<BugType> BT_doubleunlock;
mutable std::unique_ptr<BugType> BT_lor;
enum LockingSemantics {
NotApplicable = 0,
@@ -45,6 +46,7 @@ public:
// GDM Entry for tracking lock state.
REGISTER_LIST_WITH_PROGRAMSTATE(LockSet, const MemRegion *)
+REGISTER_SET_WITH_PROGRAMSTATE(UnlockSet, const MemRegion *)
void PthreadLockChecker::checkPostStmt(const CallExpr *CE,
CheckerContext &C) const {
@@ -144,6 +146,7 @@ void PthreadLockChecker::AcquireLock(Che
// Record that the lock was acquired.
lockSucc = lockSucc->add<LockSet>(lockR);
+ lockSucc = lockSucc->remove<UnlockSet>(lockR);
C.addTransition(lockSucc);
}
@@ -155,32 +158,48 @@ void PthreadLockChecker::ReleaseLock(Che
return;
ProgramStateRef state = C.getState();
- LockSetTy LS = state->get<LockSet>();
- // FIXME: Better analysis requires IPA for wrappers.
- // FIXME: check for double unlocks
- if (LS.isEmpty())
- return;
-
- const MemRegion *firstLockR = LS.getHead();
- if (firstLockR != lockR) {
- if (!BT_lor)
- BT_lor.reset(new BugType(this, "Lock order reversal", "Lock checker"));
+ if (state->contains<UnlockSet>(lockR)) {
+ if (!BT_doubleunlock)
+ BT_doubleunlock.reset(new BugType(this, "Double unlocking",
+ "Lock checker"));
ExplodedNode *N = C.generateSink();
if (!N)
return;
- BugReport *report = new BugReport(*BT_lor,
- "This was not the most "
- "recently acquired lock. "
- "Possible lock order "
- "reversal", N);
- report->addRange(CE->getArg(0)->getSourceRange());
- C.emitReport(report);
+ BugReport *Report = new BugReport(*BT_doubleunlock,
+ "This lock has already been unlocked",
+ N);
+ Report->addRange(CE->getArg(0)->getSourceRange());
+ C.emitReport(Report);
return;
}
+ LockSetTy LS = state->get<LockSet>();
+
+ // FIXME: Better analysis requires IPA for wrappers.
+
+ if (!LS.isEmpty()) {
+ const MemRegion *firstLockR = LS.getHead();
+ if (firstLockR != lockR) {
+ if (!BT_lor)
+ BT_lor.reset(new BugType(this, "Lock order reversal", "Lock checker"));
+ ExplodedNode *N = C.generateSink();
+ if (!N)
+ return;
+ BugReport *report = new BugReport(*BT_lor,
+ "This was not the most recently "
+ "acquired lock. Possible lock order "
+ "reversal",
+ N);
+ report->addRange(CE->getArg(0)->getSourceRange());
+ C.emitReport(report);
+ return;
+ }
+ }
+
// Record that the lock was released.
state = state->set<LockSet>(LS.getTail());
+ state = state->add<UnlockSet>(lockR);
C.addTransition(state);
}
Modified: cfe/trunk/test/Analysis/pthreadlock.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/pthreadlock.c?rev=205274&r1=205273&r2=205274&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/pthreadlock.c (original)
+++ cfe/trunk/test/Analysis/pthreadlock.c Mon Mar 31 22:40:38 2014
@@ -69,6 +69,31 @@ ok7(void)
}
void
+ok8(void)
+{
+ pthread_mutex_lock(&mtx1); // no-warning
+ pthread_mutex_lock(&mtx2); // no-warning
+ pthread_mutex_unlock(&mtx2); // no-warning
+ pthread_mutex_unlock(&mtx1); // no-warning
+}
+
+void
+ok9(void)
+{
+ pthread_mutex_unlock(&mtx1); // no-warning
+ if (pthread_mutex_trylock(&mtx1) == 0) // no-warning
+ pthread_mutex_unlock(&mtx1); // no-warning
+}
+
+void
+ok10(void)
+{
+ if (pthread_mutex_trylock(&mtx1) != 0) // no-warning
+ pthread_mutex_lock(&mtx1); // no-warning
+ pthread_mutex_unlock(&mtx1); // no-warning
+}
+
+void
bad1(void)
{
pthread_mutex_lock(&mtx1); // no-warning
@@ -135,3 +160,74 @@ bad8(void)
lck_mtx_lock(&lck2); // no-warning
lck_mtx_unlock(&lck1); // expected-warning{{This was not the most recently acquired lock}}
}
+
+void
+bad9(void)
+{
+ lck_mtx_unlock(&lck1); // no-warning
+ lck_mtx_unlock(&lck1); // expected-warning{{This lock has already been unlocked}}
+}
+
+void
+bad10(void)
+{
+ lck_mtx_lock(&lck1); // no-warning
+ lck_mtx_unlock(&lck1); // no-warning
+ lck_mtx_unlock(&lck1); // expected-warning{{This lock has already been unlocked}}
+}
+
+static void
+bad11_sub(pthread_mutex_t *lock)
+{
+ lck_mtx_unlock(lock); // expected-warning{{This lock has already been unlocked}}
+}
+
+void
+bad11(int i)
+{
+ lck_mtx_lock(&lck1); // no-warning
+ lck_mtx_unlock(&lck1); // no-warning
+ if (i < 5)
+ bad11_sub(&lck1);
+}
+
+void
+bad12(void)
+{
+ pthread_mutex_lock(&mtx1); // no-warning
+ pthread_mutex_unlock(&mtx1); // no-warning
+ pthread_mutex_lock(&mtx1); // no-warning
+ pthread_mutex_unlock(&mtx1); // no-warning
+ pthread_mutex_unlock(&mtx1); // expected-warning{{This lock has already been unlocked}}
+}
+
+void
+bad13(void)
+{
+ pthread_mutex_lock(&mtx1); // no-warning
+ pthread_mutex_unlock(&mtx1); // no-warning
+ pthread_mutex_lock(&mtx2); // no-warning
+ pthread_mutex_unlock(&mtx2); // no-warning
+ pthread_mutex_unlock(&mtx1); // expected-warning{{This lock has already been unlocked}}
+}
+
+void
+bad14(void)
+{
+ pthread_mutex_lock(&mtx1); // no-warning
+ pthread_mutex_lock(&mtx2); // no-warning
+ pthread_mutex_unlock(&mtx2); // no-warning
+ pthread_mutex_unlock(&mtx1); // no-warning
+ pthread_mutex_unlock(&mtx2); // expected-warning{{This lock has already been unlocked}}
+}
+
+void
+bad15(void)
+{
+ pthread_mutex_lock(&mtx1); // no-warning
+ pthread_mutex_lock(&mtx2); // no-warning
+ pthread_mutex_unlock(&mtx2); // no-warning
+ pthread_mutex_unlock(&mtx1); // no-warning
+ pthread_mutex_lock(&mtx1); // no-warning
+ pthread_mutex_unlock(&mtx2); // expected-warning{{This lock has already been unlocked}}
+}
More information about the cfe-commits
mailing list