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
