NoQ created this revision.
Use `CallEvent` and `CallDescription` everywhere. Unhardcode argument numbers
in AcquireLock() etc. Have a list of supported functions in one place. Other
misc cleanup. No functional change intended anywhere.
https://reviews.llvm.org/D37809
Files:
lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
Index: test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
===================================================================
--- test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
+++ test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
@@ -4,36 +4,49 @@
#pragma clang system_header
typedef struct {
- void *foo;
-} pthread_mutex_t;
-
-typedef struct {
- void *foo;
-} pthread_mutexattr_t;
-
-typedef struct {
- void *foo;
-} lck_grp_t;
-
-typedef struct {
void *foo;
-} lck_rw_t;
+} OpaqueThing;
+
+// Pthread.
+typedef OpaqueThing pthread_mutex_t;
+typedef OpaqueThing pthread_rwlock_t;
+typedef OpaqueThing pthread_mutexattr_t;
+
+// XNU.
+typedef OpaqueThing lck_mtx_t;
+typedef OpaqueThing lck_rw_t;
+typedef OpaqueThing lck_grp_t;
+typedef OpaqueThing lck_attr_t;
+typedef int boolean_t;
-typedef pthread_mutex_t lck_mtx_t;
+// Init.
+extern int pthread_mutex_init(pthread_mutex_t *mutex,
+ const pthread_mutexattr_t *mutexattr);
+// Acquire.
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 pthread_mutex_init(pthread_mutex_t *mutex, const pthread_mutexattr_t *mutexattr);
-
-typedef int boolean_t;
+// TODO: pthread_rwlock_rdlock.
+// TODO: pthread_rwlock_wrlock.
extern void lck_mtx_lock(lck_mtx_t *);
-extern void lck_mtx_unlock(lck_mtx_t *);
+extern void lck_rw_lock_exclusive(lck_rw_t *lck);
+extern void lck_rw_lock_shared(lck_rw_t *lck);
+
+// Try.
+extern int pthread_mutex_trylock(pthread_mutex_t *);
+// TODO: pthread_rwlock_tryrdlock.
+// TODO: pthread_rwlock_trywrlock.
extern boolean_t lck_mtx_try_lock(lck_mtx_t *);
-extern void lck_mtx_destroy(lck_mtx_t *lck, lck_grp_t *grp);
+// TODO: lck_rw_try_lock_exclusive.
+// TODO: lck_rw_try_lock_shared.
-extern void lck_rw_lock_exclusive(lck_rw_t *lck);
+// Release.
+extern int pthread_mutex_unlock(pthread_mutex_t *);
+// TODO: pthread_rwlock_tryrdlock.
+// TODO: pthread_rwlock_trywrlock.
+extern void lck_mtx_unlock(lck_mtx_t *);
extern void lck_rw_unlock_exclusive(lck_rw_t *lck);
-extern void lck_rw_lock_shared(lck_rw_t *lck);
extern void lck_rw_unlock_shared(lck_rw_t *lck);
+
+// Destroy.
+extern int pthread_mutex_destroy(pthread_mutex_t *);
+extern void lck_mtx_destroy(lck_mtx_t *lck, lck_grp_t *grp);
Index: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -17,7 +17,7 @@
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
using namespace clang;
using namespace ento;
@@ -67,7 +67,7 @@
};
class PthreadLockChecker
- : public Checker<check::PostStmt<CallExpr>, check::DeadSymbols> {
+ : public Checker<check::PostCall, check::DeadSymbols> {
mutable std::unique_ptr<BugType> BT_doublelock;
mutable std::unique_ptr<BugType> BT_doubleunlock;
mutable std::unique_ptr<BugType> BT_destroylock;
@@ -78,23 +78,54 @@
PthreadSemantics,
XNUSemantics
};
-public:
- void checkPostStmt(const CallExpr *CE, CheckerContext &C) const;
- void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
- void printState(raw_ostream &Out, ProgramStateRef State,
- const char *NL, const char *Sep) const override;
- void AcquireLock(CheckerContext &C, const CallExpr *CE, SVal lock,
- bool isTryLock, enum LockingSemantics semantics) const;
+ typedef void (PthreadLockChecker::*FnCheck)(const CallEvent &Call,
+ CheckerContext &C) const;
+ struct SupportedAPI {
+ CallDescription Name;
+ FnCheck Callback;
+ };
+ typedef SmallVector<SupportedAPI, 32> SupportedAPIList;
+ SupportedAPIList SupportedAPIs;
- void ReleaseLock(CheckerContext &C, const CallExpr *CE, SVal lock) const;
- void DestroyLock(CheckerContext &C, const CallExpr *CE, SVal Lock,
- enum LockingSemantics semantics) const;
- void InitLock(CheckerContext &C, const CallExpr *CE, SVal Lock) const;
- void reportUseDestroyedBug(CheckerContext &C, const CallExpr *CE) const;
ProgramStateRef resolvePossiblyDestroyedMutex(ProgramStateRef state,
const MemRegion *lockR,
const SymbolRef *sym) const;
+ void reportUseDestroyedBug(const CallEvent &Call, CheckerContext &C,
+ unsigned ArgNo) const;
+
+ // Init.
+ void InitAnyLock(const CallEvent &Call, CheckerContext &C) const;
+ void InitLockAux(const CallEvent &Call, CheckerContext &C, unsigned ArgNo,
+ SVal Lock) const;
+
+ // Lock, Try-lock.
+ void AcquirePthreadLock(const CallEvent &Call, CheckerContext &C) const;
+ void AcquireXNULock(const CallEvent &Call, CheckerContext &C) const;
+ void TryPthreadLock(const CallEvent &Call, CheckerContext &C) const;
+ void TryXNULock(const CallEvent &Call, CheckerContext &C) const;
+ void AcquireLockAux(const CallEvent &Call, CheckerContext &C, unsigned ArgNo,
+ SVal lock, bool isTryLock,
+ enum LockingSemantics semantics) const;
+
+ // Release.
+ void ReleaseAnyLock(const CallEvent &Call, CheckerContext &C) const;
+ void ReleaseLockAux(const CallEvent &Call, CheckerContext &C, unsigned ArgNo,
+ SVal lock) const;
+
+ // Destroy.
+ void DestroyPthreadLock(const CallEvent &Call, CheckerContext &C) const;
+ void DestroyXNULock(const CallEvent &Call, CheckerContext &C) const;
+ void DestroyLockAux(const CallEvent &Call, CheckerContext &C, unsigned ArgNo,
+ SVal Lock, enum LockingSemantics semantics) const;
+
+public:
+ PthreadLockChecker();
+
+ void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
+ void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
+ void printState(raw_ostream &Out, ProgramStateRef State,
+ const char *NL, const char *Sep) const override;
};
} // end anonymous namespace
@@ -107,50 +138,52 @@
// Return values for unresolved calls to pthread_mutex_destroy().
REGISTER_MAP_WITH_PROGRAMSTATE(DestroyRetVal, const MemRegion *, SymbolRef)
-void PthreadLockChecker::checkPostStmt(const CallExpr *CE,
+PthreadLockChecker::PthreadLockChecker() : SupportedAPIs({
+ // Init.
+ {{"pthread_mutex_init", 2}, &PthreadLockChecker::InitAnyLock},
+ // TODO: pthread_rwlock_init(2 arguments).
+ // TODO: lck_mtx_init(3 arguments).
+ // TODO: lck_mtx_alloc_init(2 arguments) => returns the mutex.
+ // TODO: lck_rw_init(3 arguments).
+ // TODO: lck_rw_alloc_init(2 arguments) => returns the mutex.
+
+ // Acquire.
+ {{"pthread_mutex_lock", 1}, &PthreadLockChecker::AcquirePthreadLock},
+ {{"pthread_rwlock_rdlock", 1}, &PthreadLockChecker::AcquirePthreadLock},
+ {{"pthread_rwlock_wrlock", 1}, &PthreadLockChecker::AcquirePthreadLock},
+ {{"lck_mtx_lock", 1}, &PthreadLockChecker::AcquireXNULock},
+ {{"lck_rw_lock_exclusive", 1}, &PthreadLockChecker::AcquireXNULock},
+ {{"lck_rw_lock_shared", 1}, &PthreadLockChecker::AcquireXNULock},
+
+ // Try.
+ {{"pthread_mutex_trylock", 1}, &PthreadLockChecker::TryPthreadLock},
+ {{"pthread_rwlock_tryrdlock", 1}, &PthreadLockChecker::TryPthreadLock},
+ {{"pthread_rwlock_trywrlock", 1}, &PthreadLockChecker::TryPthreadLock},
+ {{"lck_mtx_try_lock", 1}, &PthreadLockChecker::TryXNULock},
+ {{"lck_rw_try_lock_exclusive", 1}, &PthreadLockChecker::TryXNULock},
+ {{"lck_rw_try_lock_shared", 1}, &PthreadLockChecker::TryXNULock},
+
+ // Release.
+ {{"pthread_mutex_unlock", 1}, &PthreadLockChecker::ReleaseAnyLock},
+ {{"pthread_rwlock_unlock", 1}, &PthreadLockChecker::ReleaseAnyLock},
+ {{"lck_mtx_unlock", 1}, &PthreadLockChecker::ReleaseAnyLock},
+ {{"lck_rw_unlock_exclusive", 1}, &PthreadLockChecker::ReleaseAnyLock},
+ {{"lck_rw_unlock_shared", 1}, &PthreadLockChecker::ReleaseAnyLock},
+ {{"lck_rw_done", 1}, &PthreadLockChecker::ReleaseAnyLock},
+
+ // Destroy.
+ {{"pthread_mutex_destroy", 1}, &PthreadLockChecker::DestroyPthreadLock},
+ {{"lck_mtx_destroy", 2}, &PthreadLockChecker::DestroyXNULock},
+ // TODO: pthread_rwlock_destroy(1 argument).
+ // TODO: lck_rw_destroy(2 arguments).
+}) {}
+
+void PthreadLockChecker::checkPostCall(const CallEvent &Call,
CheckerContext &C) const {
- ProgramStateRef state = C.getState();
- const LocationContext *LCtx = C.getLocationContext();
- StringRef FName = C.getCalleeName(CE);
- if (FName.empty())
- return;
-
- if (CE->getNumArgs() != 1 && CE->getNumArgs() != 2)
- return;
-
- if (FName == "pthread_mutex_lock" ||
- FName == "pthread_rwlock_rdlock" ||
- FName == "pthread_rwlock_wrlock")
- AcquireLock(C, CE, state->getSVal(CE->getArg(0), LCtx),
- false, PthreadSemantics);
- else if (FName == "lck_mtx_lock" ||
- FName == "lck_rw_lock_exclusive" ||
- FName == "lck_rw_lock_shared")
- AcquireLock(C, CE, state->getSVal(CE->getArg(0), LCtx),
- false, XNUSemantics);
- else if (FName == "pthread_mutex_trylock" ||
- FName == "pthread_rwlock_tryrdlock" ||
- FName == "pthread_rwlock_trywrlock")
- AcquireLock(C, CE, state->getSVal(CE->getArg(0), LCtx),
- true, PthreadSemantics);
- else if (FName == "lck_mtx_try_lock" ||
- FName == "lck_rw_try_lock_exclusive" ||
- FName == "lck_rw_try_lock_shared")
- AcquireLock(C, CE, state->getSVal(CE->getArg(0), LCtx),
- true, XNUSemantics);
- else if (FName == "pthread_mutex_unlock" ||
- FName == "pthread_rwlock_unlock" ||
- FName == "lck_mtx_unlock" ||
- FName == "lck_rw_unlock_exclusive" ||
- FName == "lck_rw_unlock_shared" ||
- FName == "lck_rw_done")
- ReleaseLock(C, CE, state->getSVal(CE->getArg(0), LCtx));
- else if (FName == "pthread_mutex_destroy")
- DestroyLock(C, CE, state->getSVal(CE->getArg(0), LCtx), PthreadSemantics);
- else if (FName == "lck_mtx_destroy")
- DestroyLock(C, CE, state->getSVal(CE->getArg(0), LCtx), XNUSemantics);
- else if (FName == "pthread_mutex_init")
- InitLock(C, CE, state->getSVal(CE->getArg(0), LCtx));
+ for (const auto &I: SupportedAPIs) {
+ if (Call.isCalled(I.Name))
+ (this->*I.Callback)(Call, C);
+ }
}
// When a lock is destroyed, in some semantics(like PthreadSemantics) we are not
@@ -219,9 +252,30 @@
}
}
-void PthreadLockChecker::AcquireLock(CheckerContext &C, const CallExpr *CE,
- SVal lock, bool isTryLock,
- enum LockingSemantics semantics) const {
+void PthreadLockChecker::AcquirePthreadLock(const CallEvent &Call,
+ CheckerContext &C) const {
+ AcquireLockAux(Call, C, 0, Call.getArgSVal(0), false, PthreadSemantics);
+}
+
+void PthreadLockChecker::AcquireXNULock(const CallEvent &Call,
+ CheckerContext &C) const {
+ AcquireLockAux(Call, C, 0, Call.getArgSVal(0), false, XNUSemantics);
+}
+
+void PthreadLockChecker::TryPthreadLock(const CallEvent &Call,
+ CheckerContext &C) const {
+ AcquireLockAux(Call, C, 0, Call.getArgSVal(0), true, PthreadSemantics);
+}
+
+void PthreadLockChecker::TryXNULock(const CallEvent &Call,
+ CheckerContext &C) const {
+ AcquireLockAux(Call, C, 0, Call.getArgSVal(0), true, PthreadSemantics);
+}
+
+void PthreadLockChecker::AcquireLockAux(const CallEvent &Call,
+ CheckerContext &C, unsigned ArgNo,
+ SVal lock, bool isTryLock,
+ enum LockingSemantics semantics) const {
const MemRegion *lockR = lock.getAsRegion();
if (!lockR)
@@ -242,19 +296,19 @@
return;
auto report = llvm::make_unique<BugReport>(
*BT_doublelock, "This lock has already been acquired", N);
- report->addRange(CE->getArg(0)->getSourceRange());
+ report->addRange(Call.getArgExpr(ArgNo)->getSourceRange());
C.emitReport(std::move(report));
return;
} else if (LState->isDestroyed()) {
- reportUseDestroyedBug(C, CE);
+ reportUseDestroyedBug(Call, C, ArgNo);
return;
}
}
ProgramStateRef lockSucc = state;
if (isTryLock) {
// Bifurcate the state, and allow a mode where the lock acquisition fails.
- SVal RetVal = state->getSVal(CE, C.getLocationContext());
+ SVal RetVal = Call.getReturnValue();
if (auto DefinedRetVal = RetVal.getAs<DefinedSVal>()) {
ProgramStateRef lockFail;
switch (semantics) {
@@ -274,7 +328,7 @@
// and returned an Unknown or Undefined value.
} else if (semantics == PthreadSemantics) {
// Assume that the return value was 0.
- SVal RetVal = state->getSVal(CE, C.getLocationContext());
+ SVal RetVal = Call.getReturnValue();
if (auto DefinedRetVal = RetVal.getAs<DefinedSVal>()) {
// FIXME: If the lock function was inlined and returned true,
// we need to behave sanely - at least generate sink.
@@ -295,8 +349,14 @@
C.addTransition(lockSucc);
}
-void PthreadLockChecker::ReleaseLock(CheckerContext &C, const CallExpr *CE,
- SVal lock) const {
+void PthreadLockChecker::ReleaseAnyLock(const CallEvent &Call,
+ CheckerContext &C) const {
+ ReleaseLockAux(Call, C, 0, Call.getArgSVal(0));
+}
+
+void PthreadLockChecker::ReleaseLockAux(const CallEvent &Call,
+ CheckerContext &C, unsigned ArgNo,
+ SVal lock) const {
const MemRegion *lockR = lock.getAsRegion();
if (!lockR)
@@ -317,19 +377,17 @@
return;
auto Report = llvm::make_unique<BugReport>(
*BT_doubleunlock, "This lock has already been unlocked", N);
- Report->addRange(CE->getArg(0)->getSourceRange());
+ Report->addRange(Call.getArgExpr(ArgNo)->getSourceRange());
C.emitReport(std::move(Report));
return;
} else if (LState->isDestroyed()) {
- reportUseDestroyedBug(C, CE);
+ reportUseDestroyedBug(Call, C, ArgNo);
return;
}
}
LockSetTy LS = state->get<LockSet>();
- // FIXME: Better analysis requires IPA for wrappers.
-
if (!LS.isEmpty()) {
const MemRegion *firstLockR = LS.getHead();
if (firstLockR != lockR) {
@@ -341,7 +399,7 @@
auto report = llvm::make_unique<BugReport>(
*BT_lor, "This was not the most recently acquired lock. Possible "
"lock order reversal", N);
- report->addRange(CE->getArg(0)->getSourceRange());
+ report->addRange(Call.getArgExpr(ArgNo)->getSourceRange());
C.emitReport(std::move(report));
return;
}
@@ -353,9 +411,20 @@
C.addTransition(state);
}
-void PthreadLockChecker::DestroyLock(CheckerContext &C, const CallExpr *CE,
- SVal Lock,
- enum LockingSemantics semantics) const {
+void PthreadLockChecker::DestroyPthreadLock(const CallEvent &Call,
+ CheckerContext &C) const {
+ DestroyLockAux(Call, C, 0, Call.getArgSVal(0), PthreadSemantics);
+}
+
+void PthreadLockChecker::DestroyXNULock(const CallEvent &Call,
+ CheckerContext &C) const {
+ DestroyLockAux(Call, C, 0, Call.getArgSVal(0), XNUSemantics);
+}
+
+void PthreadLockChecker::DestroyLockAux(const CallEvent &Call,
+ CheckerContext &C, unsigned ArgNo,
+ SVal Lock,
+ enum LockingSemantics semantics) const {
const MemRegion *LockR = Lock.getAsRegion();
if (!LockR)
@@ -372,7 +441,7 @@
// PthreadSemantics
if (semantics == PthreadSemantics) {
if (!LState || LState->isUnlocked()) {
- SymbolRef sym = C.getSVal(CE).getAsSymbol();
+ SymbolRef sym = Call.getReturnValue().getAsSymbol();
if (!sym) {
State = State->remove<LockMap>(LockR);
C.addTransition(State);
@@ -410,12 +479,17 @@
if (!N)
return;
auto Report = llvm::make_unique<BugReport>(*BT_destroylock, Message, N);
- Report->addRange(CE->getArg(0)->getSourceRange());
+ Report->addRange(Call.getArgExpr(ArgNo)->getSourceRange());
C.emitReport(std::move(Report));
}
-void PthreadLockChecker::InitLock(CheckerContext &C, const CallExpr *CE,
- SVal Lock) const {
+void PthreadLockChecker::InitAnyLock(const CallEvent &Call,
+ CheckerContext &C) const {
+ InitLockAux(Call, C, 0, Call.getArgSVal(0));
+}
+
+void PthreadLockChecker::InitLockAux(const CallEvent &Call, CheckerContext &C,
+ unsigned ArgNo, SVal Lock) const {
const MemRegion *LockR = Lock.getAsRegion();
if (!LockR)
@@ -449,21 +523,22 @@
if (!N)
return;
auto Report = llvm::make_unique<BugReport>(*BT_initlock, Message, N);
- Report->addRange(CE->getArg(0)->getSourceRange());
+ Report->addRange(Call.getArgExpr(ArgNo)->getSourceRange());
C.emitReport(std::move(Report));
}
-void PthreadLockChecker::reportUseDestroyedBug(CheckerContext &C,
- const CallExpr *CE) const {
+void PthreadLockChecker::reportUseDestroyedBug(const CallEvent &Call,
+ CheckerContext &C,
+ unsigned ArgNo) const {
if (!BT_destroylock)
BT_destroylock.reset(new BugType(this, "Use destroyed lock",
"Lock checker"));
ExplodedNode *N = C.generateErrorNode();
if (!N)
return;
auto Report = llvm::make_unique<BugReport>(
*BT_destroylock, "This lock has already been destroyed", N);
- Report->addRange(CE->getArg(0)->getSourceRange());
+ Report->addRange(Call.getArgExpr(ArgNo)->getSourceRange());
C.emitReport(std::move(Report));
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
