diff --git a/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp @@ -66,7 +66,8 @@ }; class PthreadLockChecker - : public Checker { + : public Checker { BugType BT_doublelock{this, "Double locking", "Lock checker"}, BT_doubleunlock{this, "Double unlocking", "Lock checker"}, BT_destroylock{this, "Use destroyed lock", "Lock checker"}, @@ -155,6 +156,11 @@ public: void checkPostCall(const CallEvent &Call, CheckerContext &C) const; void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const; + ProgramStateRef + checkRegionChanges(ProgramStateRef State, const InvalidatedSymbols *Symbols, + ArrayRef ExplicitRegions, + ArrayRef Regions, + const LocationContext *LCtx, const CallEvent *Call) const; void printState(raw_ostream &Out, ProgramStateRef State, const char *NL, const char *Sep) const override; }; @@ -546,6 +552,42 @@ C.addTransition(State); } +ProgramStateRef PthreadLockChecker::checkRegionChanges( + ProgramStateRef State, const InvalidatedSymbols *Symbols, + ArrayRef ExplicitRegions, + ArrayRef 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. + if (Callbacks.lookup(*Call)) + 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. + // FIXME: This is a bit quadratic. + if (IsLibraryFunction && + std::find(ExplicitRegions.begin(), ExplicitRegions.end(), R) == + ExplicitRegions.end()) + continue; + + State = State->remove(R); + State = State->remove(R); + + // TODO: We need to invalidate the lock stack as well. This is tricky + // to implement correctly and efficiently though, because the effects + // of mutex escapes on lock order may be fairly varied. + } + + return State; +} + void ento::registerPthreadLockChecker(CheckerManager &mgr) { mgr.registerChecker(); } diff --git a/clang/test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h b/clang/test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h --- a/clang/test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h +++ b/clang/test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h @@ -21,6 +21,9 @@ typedef pthread_mutex_t lck_mtx_t; +extern void fake_system_function(); +extern void fake_system_function_that_takes_a_mutex(pthread_mutex_t *mtx); + extern int pthread_mutex_lock(pthread_mutex_t *); extern int pthread_mutex_unlock(pthread_mutex_t *); extern int pthread_mutex_trylock(pthread_mutex_t *); diff --git a/clang/test/Analysis/pthreadlock.c b/clang/test/Analysis/pthreadlock.c --- a/clang/test/Analysis/pthreadlock.c +++ b/clang/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}} +}