NoQ created this revision.
As usual, we need to invalidate mutex states when something may touch them.
Implement this boilerplate for the thread lock checker.
The previous refactoring is handy for listing functions on which we don't need
to invalidate mutex states because we model them instead.
Additionally, i don't invalidate mutex states when any system header function
is called, unless the mutex is passed directly into it. The TODO here would be
to model *all* system functions that may change mutex states, and then disable
invalidation for the rest of them even if they take a mutex; that's what other
checkers do.
https://reviews.llvm.org/D37812
Files:
lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h
test/Analysis/pthreadlock.c
Index: test/Analysis/pthreadlock.c
===================================================================
--- test/Analysis/pthreadlock.c
+++ test/Analysis/pthreadlock.c
@@ -221,6 +221,27 @@
lck_rw_unlock_exclusive(&rw); // no-warning
}
+void escape_mutex(pthread_mutex_t *m);
+void ok30(void) {
+ pthread_mutex_t local_mtx;
+ pthread_mutex_init(&local_mtx, NULL);
+ pthread_mutex_lock(&local_mtx);
+ escape_mutex(&local_mtx);
+ pthread_mutex_lock(&local_mtx); // no-warning
+ pthread_mutex_unlock(&local_mtx);
+ pthread_mutex_destroy(&local_mtx);
+}
+
+void ok31(void) {
+ pthread_mutex_t local_mtx;
+ pthread_mutex_init(&local_mtx, NULL);
+ pthread_mutex_lock(&local_mtx);
+ fake_system_function_that_takes_a_mutex(&local_mtx);
+ pthread_mutex_lock(&local_mtx); // no-warning
+ pthread_mutex_unlock(&local_mtx);
+ pthread_mutex_destroy(&local_mtx);
+}
+
void
bad1(void)
{
@@ -486,3 +507,9 @@
lck_rw_lock_exclusive(&rw);
lck_rw_unlock_shared(&rw); // FIXME: warn - should be exclusive?
}
+
+void bad33(void) {
+ pthread_mutex_lock(pmtx);
+ fake_system_function();
+ pthread_mutex_lock(pmtx); // expected-warning{{This lock has already been acquired}}
+}
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
@@ -3,6 +3,8 @@
// should not invalidate regions of their arguments.
#pragma clang system_header
+extern void fake_system_function();
+
typedef struct {
void *foo;
} OpaqueThing;
@@ -12,6 +14,8 @@
typedef OpaqueThing pthread_rwlock_t;
typedef OpaqueThing pthread_mutexattr_t;
+extern void fake_system_function_that_takes_a_mutex(pthread_mutex_t *mtx);
+
// XNU.
typedef OpaqueThing lck_mtx_t;
typedef OpaqueThing lck_rw_t;
Index: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -66,8 +66,8 @@
}
};
-class PthreadLockChecker
- : public Checker<check::PostCall, check::DeadSymbols> {
+class PthreadLockChecker : public Checker<check::PostCall, check::DeadSymbols,
+ check::RegionChanges> {
mutable std::unique_ptr<BugType> BT_doublelock;
mutable std::unique_ptr<BugType> BT_doubleunlock;
mutable std::unique_ptr<BugType> BT_destroylock;
@@ -124,6 +124,11 @@
void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
+ ProgramStateRef
+ checkRegionChanges(ProgramStateRef State, const InvalidatedSymbols *Symbols,
+ ArrayRef<const MemRegion *> ExplicitRegions,
+ ArrayRef<const MemRegion *> Regions,
+ const LocationContext *LCtx, const CallEvent *Call) const;
void printState(raw_ostream &Out, ProgramStateRef State,
const char *NL, const char *Sep) const override;
};
@@ -567,6 +572,38 @@
C.addTransition(State);
}
+ProgramStateRef PthreadLockChecker::checkRegionChanges(
+ ProgramStateRef State, const InvalidatedSymbols *Symbols,
+ ArrayRef<const MemRegion *> ExplicitRegions,
+ ArrayRef<const MemRegion *> Regions, const LocationContext *LCtx,
+ const CallEvent *Call) const {
+
+ bool IsLibraryFunction = false;
+ if (Call && Call->isGlobalCFunction()) {
+ // Avoid invalidating mutex state when a known supported function is called.
+ for (auto I : SupportedAPIs)
+ if (Call->isCalled(I.Name))
+ return State;
+ if (Call->isInSystemHeader())
+ IsLibraryFunction = true;
+ }
+
+ for (auto R : Regions) {
+ // We assume that system library function wouldn't touch the mutex unless
+ // it takes the mutex explicitly as an argument.
+ if (IsLibraryFunction &&
+ std::find(ExplicitRegions.begin(), ExplicitRegions.end(), R) ==
+ ExplicitRegions.end())
+ continue;
+
+ State = State->remove<LockMap>(R);
+ State = State->remove<DestroyRetVal>(R);
+ // TODO: Destroy the lock stack as well.
+ }
+
+ return State;
+}
+
void ento::registerPthreadLockChecker(CheckerManager &mgr) {
mgr.registerChecker<PthreadLockChecker>();
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
