r205275 - [analyzer] Lock checker: make sure locks aren't used after being destroyed.
Jordan Rose
jordan_rose at apple.com
Mon Mar 31 20:40:47 PDT 2014
Author: jrose
Date: Mon Mar 31 22:40:47 2014
New Revision: 205275
URL: http://llvm.org/viewvc/llvm-project?rev=205275&view=rev
Log:
[analyzer] Lock checker: make sure locks aren't used after being destroyed.
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=205275&r1=205274&r2=205275&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp Mon Mar 31 22:40:47 2014
@@ -24,9 +24,35 @@ using namespace clang;
using namespace ento;
namespace {
+
+struct LockState {
+ enum Kind { Destroyed, Locked, Unlocked } K;
+
+private:
+ LockState(Kind K) : K(K) {}
+
+public:
+ static LockState getLocked(void) { return LockState(Locked); }
+ static LockState getUnlocked(void) { return LockState(Unlocked); }
+ static LockState getDestroyed(void) { return LockState(Destroyed); }
+
+ bool operator==(const LockState &X) const {
+ return K == X.K;
+ }
+
+ bool isLocked() const { return K == Locked; }
+ bool isUnlocked() const { return K == Unlocked; }
+ bool isDestroyed() const { return K == Destroyed; }
+
+ void Profile(llvm::FoldingSetNodeID &ID) const {
+ ID.AddInteger(K);
+ }
+};
+
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_destroylock;
mutable std::unique_ptr<BugType> BT_lor;
enum LockingSemantics {
NotApplicable = 0,
@@ -40,13 +66,15 @@ public:
bool isTryLock, enum LockingSemantics semantics) const;
void ReleaseLock(CheckerContext &C, const CallExpr *CE, SVal lock) const;
+ void DestroyLock(CheckerContext &C, const CallExpr *CE, SVal Lock) const;
+ void reportUseDestroyedBug(CheckerContext &C, const CallExpr *CE) const;
};
} // end anonymous namespace
// GDM Entry for tracking lock state.
REGISTER_LIST_WITH_PROGRAMSTATE(LockSet, const MemRegion *)
-REGISTER_SET_WITH_PROGRAMSTATE(UnlockSet, const MemRegion *)
+REGISTER_MAP_WITH_PROGRAMSTATE(LockMap, const MemRegion *, LockState)
void PthreadLockChecker::checkPostStmt(const CallExpr *CE,
CheckerContext &C) const {
@@ -56,7 +84,7 @@ void PthreadLockChecker::checkPostStmt(c
if (FName.empty())
return;
- if (CE->getNumArgs() != 1)
+ if (CE->getNumArgs() != 1 && CE->getNumArgs() != 2)
return;
if (FName == "pthread_mutex_lock" ||
@@ -84,6 +112,9 @@ void PthreadLockChecker::checkPostStmt(c
FName == "lck_mtx_unlock" ||
FName == "lck_rw_done")
ReleaseLock(C, CE, state->getSVal(CE->getArg(0), LCtx));
+ else if (FName == "pthread_mutex_destroy" ||
+ FName == "lck_mtx_destroy")
+ DestroyLock(C, CE, state->getSVal(CE->getArg(0), LCtx));
}
void PthreadLockChecker::AcquireLock(CheckerContext &C, const CallExpr *CE,
@@ -102,18 +133,24 @@ void PthreadLockChecker::AcquireLock(Che
DefinedSVal retVal = X.castAs<DefinedSVal>();
- if (state->contains<LockSet>(lockR)) {
- if (!BT_doublelock)
- BT_doublelock.reset(new BugType(this, "Double locking", "Lock checker"));
- ExplodedNode *N = C.generateSink();
- if (!N)
+ if (const LockState *LState = state->get<LockMap>(lockR)) {
+ if (LState->isLocked()) {
+ if (!BT_doublelock)
+ BT_doublelock.reset(new BugType(this, "Double locking",
+ "Lock checker"));
+ ExplodedNode *N = C.generateSink();
+ if (!N)
+ return;
+ BugReport *report = new BugReport(*BT_doublelock,
+ "This lock has already been acquired",
+ N);
+ report->addRange(CE->getArg(0)->getSourceRange());
+ C.emitReport(report);
return;
- BugReport *report = new BugReport(*BT_doublelock,
- "This lock has already "
- "been acquired", N);
- report->addRange(CE->getArg(0)->getSourceRange());
- C.emitReport(report);
- return;
+ } else if (LState->isDestroyed()) {
+ reportUseDestroyedBug(C, CE);
+ return;
+ }
}
ProgramStateRef lockSucc = state;
@@ -146,7 +183,7 @@ void PthreadLockChecker::AcquireLock(Che
// Record that the lock was acquired.
lockSucc = lockSucc->add<LockSet>(lockR);
- lockSucc = lockSucc->remove<UnlockSet>(lockR);
+ lockSucc = lockSucc->set<LockMap>(lockR, LockState::getLocked());
C.addTransition(lockSucc);
}
@@ -159,19 +196,24 @@ void PthreadLockChecker::ReleaseLock(Che
ProgramStateRef state = C.getState();
- if (state->contains<UnlockSet>(lockR)) {
- if (!BT_doubleunlock)
- BT_doubleunlock.reset(new BugType(this, "Double unlocking",
- "Lock checker"));
- ExplodedNode *N = C.generateSink();
- if (!N)
+ if (const LockState *LState = state->get<LockMap>(lockR)) {
+ if (LState->isUnlocked()) {
+ 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_doubleunlock,
+ "This lock has already been unlocked",
+ N);
+ Report->addRange(CE->getArg(0)->getSourceRange());
+ C.emitReport(Report);
return;
- BugReport *Report = new BugReport(*BT_doubleunlock,
- "This lock has already been unlocked",
- N);
- Report->addRange(CE->getArg(0)->getSourceRange());
- C.emitReport(Report);
- return;
+ } else if (LState->isDestroyed()) {
+ reportUseDestroyedBug(C, CE);
+ return;
+ }
}
LockSetTy LS = state->get<LockSet>();
@@ -195,14 +237,64 @@ void PthreadLockChecker::ReleaseLock(Che
C.emitReport(report);
return;
}
+ // Record that the lock was released.
+ state = state->set<LockSet>(LS.getTail());
}
- // Record that the lock was released.
- state = state->set<LockSet>(LS.getTail());
- state = state->add<UnlockSet>(lockR);
+ state = state->set<LockMap>(lockR, LockState::getUnlocked());
C.addTransition(state);
}
+void PthreadLockChecker::DestroyLock(CheckerContext &C, const CallExpr *CE,
+ SVal Lock) const {
+
+ const MemRegion *LockR = Lock.getAsRegion();
+ if (!LockR)
+ return;
+
+ ProgramStateRef State = C.getState();
+
+ const LockState *LState = State->get<LockMap>(LockR);
+ if (!LState || LState->isUnlocked()) {
+ State = State->set<LockMap>(LockR, LockState::getDestroyed());
+ C.addTransition(State);
+ return;
+ }
+
+ StringRef Message;
+
+ if (LState->isLocked()) {
+ Message = "This lock is still locked";
+ } else {
+ Message = "This lock has already been destroyed";
+ }
+
+ if (!BT_destroylock)
+ BT_destroylock.reset(new BugType(this, "Destroy invalid lock",
+ "Lock checker"));
+ ExplodedNode *N = C.generateSink();
+ if (!N)
+ return;
+ BugReport *Report = new BugReport(*BT_destroylock, Message, N);
+ Report->addRange(CE->getArg(0)->getSourceRange());
+ C.emitReport(Report);
+}
+
+void PthreadLockChecker::reportUseDestroyedBug(CheckerContext &C,
+ const CallExpr *CE) const {
+ if (!BT_destroylock)
+ BT_destroylock.reset(new BugType(this, "Use destroyed lock",
+ "Lock checker"));
+ ExplodedNode *N = C.generateSink();
+ if (!N)
+ return;
+ BugReport *Report = new BugReport(*BT_destroylock,
+ "This lock has already been destroyed",
+ N);
+ Report->addRange(CE->getArg(0)->getSourceRange());
+ C.emitReport(Report);
+}
+
void ento::registerPthreadLockChecker(CheckerManager &mgr) {
mgr.registerChecker<PthreadLockChecker>();
}
Modified: cfe/trunk/test/Analysis/pthreadlock.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/pthreadlock.c?rev=205275&r1=205274&r2=205275&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/pthreadlock.c (original)
+++ cfe/trunk/test/Analysis/pthreadlock.c Mon Mar 31 22:40:47 2014
@@ -6,17 +6,24 @@ typedef struct {
void *foo;
} pthread_mutex_t;
+typedef struct {
+ void *foo;
+} lck_grp_t;
+
typedef pthread_mutex_t lck_mtx_t;
extern int pthread_mutex_lock(pthread_mutex_t *);
extern int pthread_mutex_unlock(pthread_mutex_t *);
extern int pthread_mutex_trylock(pthread_mutex_t *);
+extern int pthread_mutex_destroy(pthread_mutex_t *);
extern int lck_mtx_lock(lck_mtx_t *);
extern int lck_mtx_unlock(lck_mtx_t *);
extern int lck_mtx_try_lock(lck_mtx_t *);
+extern void lck_mtx_destroy(lck_mtx_t *lck, lck_grp_t *grp);
pthread_mutex_t mtx1, mtx2;
lck_mtx_t lck1, lck2;
+lck_grp_t grp1;
void
ok1(void)
@@ -94,6 +101,43 @@ ok10(void)
}
void
+ok11(void)
+{
+ pthread_mutex_destroy(&mtx1); // no-warning
+}
+
+void
+ok12(void)
+{
+ pthread_mutex_destroy(&mtx1); // no-warning
+ pthread_mutex_destroy(&mtx2); // no-warning
+}
+
+void
+ok13(void)
+{
+ pthread_mutex_unlock(&mtx1); // no-warning
+ pthread_mutex_destroy(&mtx1); // no-warning
+}
+
+void
+ok14(void)
+{
+ pthread_mutex_unlock(&mtx1); // no-warning
+ pthread_mutex_destroy(&mtx1); // no-warning
+ pthread_mutex_unlock(&mtx2); // no-warning
+ pthread_mutex_destroy(&mtx2); // no-warning
+}
+
+void
+ok15(void)
+{
+ pthread_mutex_lock(&mtx1); // no-warning
+ pthread_mutex_unlock(&mtx1); // no-warning
+ pthread_mutex_destroy(&mtx1); // no-warning
+}
+
+void
bad1(void)
{
pthread_mutex_lock(&mtx1); // no-warning
@@ -231,3 +275,59 @@ bad15(void)
pthread_mutex_lock(&mtx1); // no-warning
pthread_mutex_unlock(&mtx2); // expected-warning{{This lock has already been unlocked}}
}
+
+void
+bad16(void)
+{
+ pthread_mutex_destroy(&mtx1); // no-warning
+ pthread_mutex_lock(&mtx1); // expected-warning{{This lock has already been destroyed}}
+}
+
+void
+bad17(void)
+{
+ pthread_mutex_destroy(&mtx1); // no-warning
+ pthread_mutex_unlock(&mtx1); // expected-warning{{This lock has already been destroyed}}
+}
+
+void
+bad18(void)
+{
+ pthread_mutex_destroy(&mtx1); // no-warning
+ pthread_mutex_destroy(&mtx1); // expected-warning{{This lock has already been destroyed}}
+}
+
+void
+bad19(void)
+{
+ pthread_mutex_lock(&mtx1); // no-warning
+ pthread_mutex_destroy(&mtx1); // expected-warning{{This lock is still locked}}
+}
+
+void
+bad20(void)
+{
+ lck_mtx_destroy(&mtx1, &grp1); // no-warning
+ lck_mtx_lock(&mtx1); // expected-warning{{This lock has already been destroyed}}
+}
+
+void
+bad21(void)
+{
+ lck_mtx_destroy(&mtx1, &grp1); // no-warning
+ lck_mtx_unlock(&mtx1); // expected-warning{{This lock has already been destroyed}}
+}
+
+void
+bad22(void)
+{
+ lck_mtx_destroy(&mtx1, &grp1); // no-warning
+ lck_mtx_destroy(&mtx1, &grp1); // expected-warning{{This lock has already been destroyed}}
+}
+
+void
+bad23(void)
+{
+ lck_mtx_lock(&mtx1); // no-warning
+ lck_mtx_destroy(&mtx1, &grp1); // expected-warning{{This lock is still locked}}
+}
More information about the cfe-commits
mailing list