From: Jordy Rose Date: Tue, 19 Jul 2011 20:21:41 +0000 (+0000) Subject: [analysis] Add checks for double-locking and lock order reversal bugs for X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=dcb1d5d681d857eb7f534dec1f2b3d5a9f81d1f1;p=clang [analysis] Add checks for double-locking and lock order reversal bugs for pthread and XNU locks. Patch by Rui Paulo! git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@135515 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp b/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp index 74199bb1f6..df54d49823 100644 --- a/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp @@ -1,4 +1,4 @@ -//===--- PthreadLockChecker.h - Undefined arguments checker ----*- C++ -*--===// +//===--- PthreadLockChecker.cpp - Check for locking problems ---*- C++ -*--===// // // The LLVM Compiler Infrastructure // @@ -17,24 +17,29 @@ #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/PathSensitive/GRStateTrait.h" -#include "llvm/ADT/ImmutableSet.h" +#include "llvm/ADT/ImmutableList.h" using namespace clang; using namespace ento; namespace { -class PthreadLockChecker - : public Checker< check::PostStmt > { +class PthreadLockChecker : public Checker< check::PostStmt > { + mutable llvm::OwningPtr BT_doublelock; + mutable llvm::OwningPtr BT_lor; + enum LockingSemantics { + NotApplicable = 0, + PthreadSemantics, + XNUSemantics + }; public: void checkPostStmt(const CallExpr *CE, CheckerContext &C) const; - void AcquireLock(CheckerContext &C, const CallExpr *CE, - SVal lock, bool isTryLock) const; + void AcquireLock(CheckerContext &C, const CallExpr *CE, SVal lock, + bool isTryLock, enum LockingSemantics semantics) const; - void ReleaseLock(CheckerContext &C, const CallExpr *CE, - SVal lock) const; - + void ReleaseLock(CheckerContext &C, const CallExpr *CE, SVal lock) const; }; } // end anonymous namespace @@ -43,7 +48,7 @@ namespace { class LockSet {}; } namespace clang { namespace ento { template <> struct GRStateTrait : - public GRStatePartialTrait > { + public GRStatePartialTrait > { static void* GDMIndex() { static int x = 0; return &x; } }; } // end GR namespace @@ -64,26 +69,35 @@ void PthreadLockChecker::checkPostStmt(const CallExpr *CE, if (!II) // if no identifier, not a simple C function return; llvm::StringRef FName = II->getName(); - - if (FName == "pthread_mutex_lock") { - if (CE->getNumArgs() != 1) - return; - AcquireLock(C, CE, state->getSVal(CE->getArg(0)), false); - } - else if (FName == "pthread_mutex_trylock") { - if (CE->getNumArgs() != 1) - return; - AcquireLock(C, CE, state->getSVal(CE->getArg(0)), true); - } - else if (FName == "pthread_mutex_unlock") { - if (CE->getNumArgs() != 1) - return; + + if (CE->getNumArgs() != 1) + return; + if (FName == "pthread_mutex_lock" || + FName == "pthread_rwlock_rdlock" || + FName == "pthread_rwlock_wrlock") + AcquireLock(C, CE, state->getSVal(CE->getArg(0)), false, PthreadSemantics); + else if (FName == "lck_mtx_lock" || + FName == "lck_rw_lock_exclusive" || + FName == "lck_rw_lock_shared") + AcquireLock(C, CE, state->getSVal(CE->getArg(0)), false, XNUSemantics); + else if (FName == "pthread_mutex_trylock" || + FName == "pthread_rwlock_tryrdlock" || + FName == "pthread_rwlock_tryrwlock") + AcquireLock(C, CE, state->getSVal(CE->getArg(0)), true, PthreadSemantics); + else if (FName == "lck_mtx_try_lock" || + FName == "lck_rw_try_lock_exclusive" || + FName == "lck_rw_try_lock_shared") + AcquireLock(C, CE, state->getSVal(CE->getArg(0)), true, XNUSemantics); + else if (FName == "pthread_mutex_unlock" || + FName == "pthread_rwlock_unlock" || + FName == "lck_mtx_unlock" || + FName == "lck_rw_done") ReleaseLock(C, CE, state->getSVal(CE->getArg(0))); - } } void PthreadLockChecker::AcquireLock(CheckerContext &C, const CallExpr *CE, - SVal lock, bool isTryLock) const { + SVal lock, bool isTryLock, + enum LockingSemantics semantics) const { const MemRegion *lockR = lock.getAsRegion(); if (!lockR) @@ -96,26 +110,55 @@ void PthreadLockChecker::AcquireLock(CheckerContext &C, const CallExpr *CE, return; DefinedSVal retVal = cast(X); + + llvm::ImmutableList LS = state->get(); + + if (state->contains(lockR)) { + if (!BT_doublelock) + BT_doublelock.reset(new BugType("Double locking", "Lock checker")); + ExplodedNode *N = C.generateSink(); + if (!N) + return; + EnhancedBugReport *report = new EnhancedBugReport(*BT_doublelock, + "This lock has already " + "been acquired", N); + report->addRange(CE->getArg(0)->getSourceRange()); + C.EmitReport(report); + return; + } + const GRState *lockSucc = state; - if (isTryLock) { - // Bifurcate the state, and allow a mode where the lock acquisition fails. + // Bifurcate the state, and allow a mode where the lock acquisition fails. const GRState *lockFail; - llvm::tie(lockFail, lockSucc) = state->assume(retVal); + switch (semantics) { + case PthreadSemantics: + llvm::tie(lockFail, lockSucc) = state->assume(retVal); + break; + case XNUSemantics: + llvm::tie(lockSucc, lockFail) = state->assume(retVal); + break; + default: + llvm_unreachable("Unknown tryLock locking semantics"); + break; + } assert(lockFail && lockSucc); - C.addTransition(C.generateNode(CE, lockFail)); - } - else { - // Assume that the return value was 0. + C.addTransition(lockFail); + + } else if (semantics == PthreadSemantics) { + // Assume that the return value was 0. lockSucc = state->assume(retVal, false); assert(lockSucc); + + } else { + // XNU locking semantics return void on non-try locks + assert((semantics == XNUSemantics) && "Unknown locking semantics"); + lockSucc = state; } - // Record that the lock was acquired. + // Record that the lock was acquired. lockSucc = lockSucc->add(lockR); - - C.addTransition(lockSucc != state ? C.generateNode(CE, lockSucc) : - C.getPredecessor()); + C.addTransition(lockSucc); } void PthreadLockChecker::ReleaseLock(CheckerContext &C, const CallExpr *CE, @@ -126,18 +169,36 @@ void PthreadLockChecker::ReleaseLock(CheckerContext &C, const CallExpr *CE, return; const GRState *state = C.getState(); + llvm::ImmutableList LS = state->get(); - // Record that the lock was released. - // FIXME: Handle unlocking locks that were never acquired. This may - // require IPA for wrappers. - const GRState *unlockState = state->remove(lockR); - - if (state == unlockState) + // FIXME: Better analysis requires IPA for wrappers. + // FIXME: check for double unlocks + if (LS.isEmpty()) return; - C.addTransition(C.generateNode(CE, unlockState)); + const MemRegion *firstLockR = LS.getHead(); + if (firstLockR != lockR) { + if (!BT_lor) + BT_lor.reset(new BugType("Lock order reversal", "Lock checker")); + ExplodedNode *N = C.generateSink(); + if (!N) + return; + EnhancedBugReport *report = new EnhancedBugReport(*BT_lor, + "This was not the most " + "recently acquired lock. " + "Possible lock order " + "reversal", N); + report->addRange(CE->getArg(0)->getSourceRange()); + C.EmitReport(report); + return; + } + + // Record that the lock was released. + state = state->set(LS.getTail()); + C.addTransition(state); } + void ento::registerPthreadLockChecker(CheckerManager &mgr) { mgr.registerChecker(); } diff --git a/test/Analysis/pthreadlock.c b/test/Analysis/pthreadlock.c new file mode 100644 index 0000000000..9ca3ed5192 --- /dev/null +++ b/test/Analysis/pthreadlock.c @@ -0,0 +1,137 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=unix.experimental.PthreadLock -verify %s + +// Tests performing normal locking patterns and wrong locking orders + +typedef struct { + void *foo; +} pthread_mutex_t; + +typedef pthread_mutex_t lck_mtx_t; + +extern int pthread_mutex_lock(pthread_mutex_t *); +extern int pthread_mutex_unlock(pthread_mutex_t *); +extern int pthread_mutex_trylock(pthread_mutex_t *); +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 *); + +pthread_mutex_t mtx1, mtx2; +lck_mtx_t lck1, lck2; + +void +ok1(void) +{ + pthread_mutex_lock(&mtx1); // no-warning +} + +void +ok2(void) +{ + pthread_mutex_unlock(&mtx1); // no-warning +} + +void +ok3(void) +{ + pthread_mutex_lock(&mtx1); // no-warning + pthread_mutex_unlock(&mtx1); // no-warning + pthread_mutex_lock(&mtx1); // no-warning + pthread_mutex_unlock(&mtx1); // no-warning +} + +void +ok4(void) +{ + pthread_mutex_lock(&mtx1); // no-warning + pthread_mutex_unlock(&mtx1); // no-warning + pthread_mutex_lock(&mtx2); // no-warning + pthread_mutex_unlock(&mtx2); // no-warning +} + +void +ok5(void) +{ + if (pthread_mutex_trylock(&mtx1) == 0) // no-warning + pthread_mutex_unlock(&mtx1); // no-warning +} + +void +ok6(void) +{ + lck_mtx_lock(&lck1); // no-warning +} + +void +ok7(void) +{ + if (lck_mtx_try_lock(&lck1) != 0) // no-warning + lck_mtx_unlock(&lck1); // no-warning +} + +void +bad1(void) +{ + pthread_mutex_lock(&mtx1); // no-warning + pthread_mutex_lock(&mtx1); // expected-warning{{This lock has already been acquired}} +} + +void +bad2(void) +{ + pthread_mutex_lock(&mtx1); // no-warning + pthread_mutex_unlock(&mtx1); // no-warning + pthread_mutex_lock(&mtx1); // no-warning + pthread_mutex_lock(&mtx1); // expected-warning{{This lock has already been acquired}} +} + +void +bad3(void) +{ + pthread_mutex_lock(&mtx1); // no-warning + pthread_mutex_lock(&mtx2); // no-warning + pthread_mutex_unlock(&mtx1); // expected-warning{{This was not the most recently acquired lock}} + pthread_mutex_unlock(&mtx2); +} + +void +bad4(void) +{ + if (pthread_mutex_trylock(&mtx1)) // no-warning + return; + pthread_mutex_lock(&mtx2); // no-warning + pthread_mutex_unlock(&mtx1); // expected-warning{{This was not the most recently acquired lock}} +} + +void +bad5(void) +{ + lck_mtx_lock(&lck1); // no-warning + lck_mtx_lock(&lck1); // expected-warning{{This lock has already been acquired}} +} + +void +bad6(void) +{ + lck_mtx_lock(&lck1); // no-warning + lck_mtx_unlock(&lck1); // no-warning + lck_mtx_lock(&lck1); // no-warning + lck_mtx_lock(&lck1); // expected-warning{{This lock has already been acquired}} +} + +void +bad7(void) +{ + lck_mtx_lock(&lck1); // no-warning + lck_mtx_lock(&lck2); // no-warning + lck_mtx_unlock(&lck1); // expected-warning{{This was not the most recently acquired lock}} + lck_mtx_unlock(&lck2); +} + +void +bad8(void) +{ + if (lck_mtx_try_lock(&lck1) == 0) // no-warning + return; + lck_mtx_lock(&lck2); // no-warning + lck_mtx_unlock(&lck1); // expected-warning{{This was not the most recently acquired lock}} +}