Hi jordan_rose, zaks.anna, krememek,
Regions passed to pthread* functions are invalidated because the call is
evaluated conservatively. This leads to false positives (tests included). This
patch replaces PostStmt<CallExpr> with evalCall for such functions.
http://reviews.llvm.org/D5247
Files:
lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
test/Analysis/pthreadlock.c
Index: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -49,7 +49,8 @@
}
};
-class PthreadLockChecker : public Checker< check::PostStmt<CallExpr> > {
+class PthreadLockChecker : public Checker< eval::Call,
+ check::PostStmt<CallExpr> > {
mutable std::unique_ptr<BugType> BT_doublelock;
mutable std::unique_ptr<BugType> BT_doubleunlock;
mutable std::unique_ptr<BugType> BT_destroylock;
@@ -62,7 +63,8 @@
};
public:
void checkPostStmt(const CallExpr *CE, CheckerContext &C) const;
-
+ bool evalCall(const CallExpr *CE, CheckerContext &C) const;
+
void AcquireLock(CheckerContext &C, const CallExpr *CE, SVal lock,
bool isTryLock, enum LockingSemantics semantics) const;
@@ -78,44 +80,64 @@
REGISTER_MAP_WITH_PROGRAMSTATE(LockMap, const MemRegion *, LockState)
-void PthreadLockChecker::checkPostStmt(const CallExpr *CE,
- CheckerContext &C) const {
+bool PthreadLockChecker::evalCall(const CallExpr *CE, CheckerContext &C) const {
ProgramStateRef state = C.getState();
const LocationContext *LCtx = C.getLocationContext();
StringRef FName = C.getCalleeName(CE);
if (FName.empty())
- return;
+ return false;
if (CE->getNumArgs() != 1 && CE->getNumArgs() != 2)
- return;
+ return false;
if (FName == "pthread_mutex_lock" ||
FName == "pthread_rwlock_rdlock" ||
- FName == "pthread_rwlock_wrlock")
+ FName == "pthread_rwlock_wrlock") {
AcquireLock(C, CE, state->getSVal(CE->getArg(0), LCtx),
false, PthreadSemantics);
- else if (FName == "lck_mtx_lock" ||
+ return true;
+ } else if (FName == "lck_mtx_lock" ||
FName == "lck_rw_lock_exclusive" ||
- FName == "lck_rw_lock_shared")
+ FName == "lck_rw_lock_shared") {
AcquireLock(C, CE, state->getSVal(CE->getArg(0), LCtx),
false, XNUSemantics);
- else if (FName == "pthread_mutex_trylock" ||
+ return true;
+ } else if (FName == "pthread_mutex_trylock" ||
FName == "pthread_rwlock_tryrdlock" ||
- FName == "pthread_rwlock_trywrlock")
+ FName == "pthread_rwlock_trywrlock") {
AcquireLock(C, CE, state->getSVal(CE->getArg(0), LCtx),
true, PthreadSemantics);
- else if (FName == "lck_mtx_try_lock" ||
+ return true;
+ } else if (FName == "lck_mtx_try_lock" ||
FName == "lck_rw_try_lock_exclusive" ||
- FName == "lck_rw_try_lock_shared")
+ FName == "lck_rw_try_lock_shared") {
AcquireLock(C, CE, state->getSVal(CE->getArg(0), LCtx),
true, XNUSemantics);
- else if (FName == "pthread_mutex_unlock" ||
+ return true;
+ } else if (FName == "pthread_mutex_unlock" ||
FName == "pthread_rwlock_unlock" ||
FName == "lck_mtx_unlock" ||
- FName == "lck_rw_done")
+ FName == "lck_rw_done") {
ReleaseLock(C, CE, state->getSVal(CE->getArg(0), LCtx));
- else if (FName == "pthread_mutex_destroy" ||
- FName == "lck_mtx_destroy")
+ return true;
+ }
+ return false;
+}
+
+
+void PthreadLockChecker::checkPostStmt(const CallExpr *CE,
+ 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_destroy" ||
+ FName == "lck_mtx_destroy")
DestroyLock(C, CE, state->getSVal(CE->getArg(0), LCtx));
else if (FName == "pthread_mutex_init")
InitLock(C, CE, state->getSVal(CE->getArg(0), LCtx));
@@ -130,12 +152,11 @@
return;
ProgramStateRef state = C.getState();
-
- SVal X = state->getSVal(CE, C.getLocationContext());
- if (X.isUnknownOrUndef())
- return;
-
- DefinedSVal retVal = X.castAs<DefinedSVal>();
+ const LocationContext *LCtx = C.getLocationContext();
+ DefinedSVal retVal = C.getSValBuilder() .conjureSymbolVal(nullptr, CE, LCtx,
+ C.blockCount())
+ .castAs<DefinedSVal>();
+ state = state->BindExpr(CE, LCtx, retVal);
if (const LockState *LState = state->get<LockMap>(lockR)) {
if (LState->isLocked()) {
@@ -246,6 +267,11 @@
}
state = state->set<LockMap>(lockR, LockState::getUnlocked());
+ const LocationContext *LCtx = C.getLocationContext();
+ DefinedSVal retVal = C.getSValBuilder().conjureSymbolVal(nullptr, CE, LCtx,
+ C.blockCount())
+ .castAs<DefinedSVal>();
+ state = state->BindExpr(CE, LCtx, retVal);
C.addTransition(state);
}
Index: test/Analysis/pthreadlock.c
===================================================================
--- test/Analysis/pthreadlock.c
+++ test/Analysis/pthreadlock.c
@@ -30,6 +30,8 @@
lck_mtx_t lck1, lck2;
lck_grp_t grp1;
+pthread_mutex_t *pmtx;
+
#define NULL 0
void
@@ -184,6 +186,21 @@
}
void
+ok21(void) {
+ pthread_mutex_lock(pmtx); // no-warning
+ pthread_mutex_unlock(pmtx); // no-warning
+}
+
+void
+ok22(void) {
+ pthread_mutex_lock(pmtx); // no-warning
+ pthread_mutex_unlock(pmtx); // no-warning
+ pthread_mutex_lock(pmtx); // no-warning
+ pthread_mutex_unlock(pmtx); // no-warning
+}
+
+
+void
bad1(void)
{
pthread_mutex_lock(&mtx1); // no-warning
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
