This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGdd22be1e3d98: [analyzer] PthreadLock: Implement mutex escaping. (authored by dergachev.a). Herald added subscribers: Charusso, dkrupp, donat.nagy, jfb, Szelethus, mikhail.ramalho, rnkovacs, szepet, baloghadamsoftware. Herald added a project: clang.
Changed prior to commit: https://reviews.llvm.org/D37812?vs=115053&id=240199#toc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37812/new/ https://reviews.llvm.org/D37812 Files: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp clang/test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h clang/test/Analysis/pthreadlock.c
Index: clang/test/Analysis/pthreadlock.c =================================================================== --- clang/test/Analysis/pthreadlock.c +++ 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}} +} Index: clang/test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h =================================================================== --- clang/test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h +++ 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 *); Index: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp @@ -66,7 +66,8 @@ }; class PthreadLockChecker - : public Checker<check::PostCall, check::DeadSymbols> { + : public Checker<check::PostCall, check::DeadSymbols, + check::RegionChanges> { 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<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; }; @@ -546,6 +552,42 @@ 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. + 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<LockMap>(R); + State = State->remove<DestroyRetVal>(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<PthreadLockChecker>(); }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits