This revision was automatically updated to reflect the committed changes. Closed by commit rG80fd37f9d66e: [analyzer] PthreadLock: Fix return value modeling for XNU lock functions. (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/D37806?vs=115029&id=240197#toc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37806/new/ https://reviews.llvm.org/D37806 Files: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp clang/test/Analysis/Inputs/system-header-simulator-for-pthread-lock.h 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 @@ -22,7 +22,9 @@ extern int pthread_mutex_trylock(pthread_mutex_t *); extern int pthread_mutex_destroy(pthread_mutex_t *); extern int pthread_mutex_init(pthread_mutex_t *mutex, const pthread_mutexattr_t *mutexattr); -extern int lck_mtx_lock(lck_mtx_t *); -extern int lck_mtx_unlock(lck_mtx_t *); -extern int lck_mtx_try_lock(lck_mtx_t *); + +typedef int boolean_t; +extern void lck_mtx_lock(lck_mtx_t *); +extern void lck_mtx_unlock(lck_mtx_t *); +extern boolean_t lck_mtx_try_lock(lck_mtx_t *); extern void lck_mtx_destroy(lck_mtx_t *lck, lck_grp_t *grp); Index: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp @@ -226,12 +226,6 @@ if (sym) state = resolvePossiblyDestroyedMutex(state, lockR, sym); - SVal X = C.getSVal(CE); - if (X.isUnknownOrUndef()) - return; - - DefinedSVal retVal = X.castAs<DefinedSVal>(); - if (const LockState *LState = state->get<LockMap>(lockR)) { if (LState->isLocked()) { if (!BT_doublelock) @@ -254,25 +248,35 @@ ProgramStateRef lockSucc = state; if (isTryLock) { // Bifurcate the state, and allow a mode where the lock acquisition fails. - ProgramStateRef lockFail; - switch (semantics) { - case PthreadSemantics: - std::tie(lockFail, lockSucc) = state->assume(retVal); - break; - case XNUSemantics: - std::tie(lockSucc, lockFail) = state->assume(retVal); - break; - default: - llvm_unreachable("Unknown tryLock locking semantics"); + SVal RetVal = state->getSVal(CE, C.getLocationContext()); + if (auto DefinedRetVal = RetVal.getAs<DefinedSVal>()) { + ProgramStateRef lockFail; + switch (semantics) { + case PthreadSemantics: + std::tie(lockFail, lockSucc) = state->assume(*DefinedRetVal); + break; + case XNUSemantics: + std::tie(lockSucc, lockFail) = state->assume(*DefinedRetVal); + break; + default: + llvm_unreachable("Unknown tryLock locking semantics"); + } + assert(lockFail && lockSucc); + C.addTransition(lockFail); } - assert(lockFail && lockSucc); - C.addTransition(lockFail); - + // We might want to handle the case when the mutex lock function was inlined + // and returned an Unknown or Undefined value. } else if (semantics == PthreadSemantics) { // Assume that the return value was 0. - lockSucc = state->assume(retVal, false); - assert(lockSucc); - + SVal RetVal = state->getSVal(CE, C.getLocationContext()); + if (auto DefinedRetVal = RetVal.getAs<DefinedSVal>()) { + // FIXME: If the lock function was inlined and returned true, + // we need to behave sanely - at least generate sink. + lockSucc = state->assume(*DefinedRetVal, false); + assert(lockSucc); + } + // We might want to handle the case when the mutex lock function was inlined + // and returned an Unknown or Undefined value. } else { // XNU locking semantics return void on non-try locks assert((semantics == XNUSemantics) && "Unknown locking semantics");
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 @@ -22,7 +22,9 @@ extern int pthread_mutex_trylock(pthread_mutex_t *); extern int pthread_mutex_destroy(pthread_mutex_t *); extern int pthread_mutex_init(pthread_mutex_t *mutex, const pthread_mutexattr_t *mutexattr); -extern int lck_mtx_lock(lck_mtx_t *); -extern int lck_mtx_unlock(lck_mtx_t *); -extern int lck_mtx_try_lock(lck_mtx_t *); + +typedef int boolean_t; +extern void lck_mtx_lock(lck_mtx_t *); +extern void lck_mtx_unlock(lck_mtx_t *); +extern boolean_t lck_mtx_try_lock(lck_mtx_t *); extern void lck_mtx_destroy(lck_mtx_t *lck, lck_grp_t *grp); Index: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp @@ -226,12 +226,6 @@ if (sym) state = resolvePossiblyDestroyedMutex(state, lockR, sym); - SVal X = C.getSVal(CE); - if (X.isUnknownOrUndef()) - return; - - DefinedSVal retVal = X.castAs<DefinedSVal>(); - if (const LockState *LState = state->get<LockMap>(lockR)) { if (LState->isLocked()) { if (!BT_doublelock) @@ -254,25 +248,35 @@ ProgramStateRef lockSucc = state; if (isTryLock) { // Bifurcate the state, and allow a mode where the lock acquisition fails. - ProgramStateRef lockFail; - switch (semantics) { - case PthreadSemantics: - std::tie(lockFail, lockSucc) = state->assume(retVal); - break; - case XNUSemantics: - std::tie(lockSucc, lockFail) = state->assume(retVal); - break; - default: - llvm_unreachable("Unknown tryLock locking semantics"); + SVal RetVal = state->getSVal(CE, C.getLocationContext()); + if (auto DefinedRetVal = RetVal.getAs<DefinedSVal>()) { + ProgramStateRef lockFail; + switch (semantics) { + case PthreadSemantics: + std::tie(lockFail, lockSucc) = state->assume(*DefinedRetVal); + break; + case XNUSemantics: + std::tie(lockSucc, lockFail) = state->assume(*DefinedRetVal); + break; + default: + llvm_unreachable("Unknown tryLock locking semantics"); + } + assert(lockFail && lockSucc); + C.addTransition(lockFail); } - assert(lockFail && lockSucc); - C.addTransition(lockFail); - + // We might want to handle the case when the mutex lock function was inlined + // and returned an Unknown or Undefined value. } else if (semantics == PthreadSemantics) { // Assume that the return value was 0. - lockSucc = state->assume(retVal, false); - assert(lockSucc); - + SVal RetVal = state->getSVal(CE, C.getLocationContext()); + if (auto DefinedRetVal = RetVal.getAs<DefinedSVal>()) { + // FIXME: If the lock function was inlined and returned true, + // we need to behave sanely - at least generate sink. + lockSucc = state->assume(*DefinedRetVal, false); + assert(lockSucc); + } + // We might want to handle the case when the mutex lock function was inlined + // and returned an Unknown or Undefined value. } else { // XNU locking semantics return void on non-try locks assert((semantics == XNUSemantics) && "Unknown locking semantics");
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits