[analysis] Add checks for double-locking and lock order reversal bugs for

pthread and XNU locks. Patch by Rui Paulo!

llvm-svn: 135515

clang/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
+class PthreadLockChecker : public Checker< check::PostStmt<CallExpr> > {
-  : public Checker< check::PostStmt<CallExpr> > {
+  mutable llvm::OwningPtr<BugType> BT_doublelock;
+  mutable llvm::OwningPtr<BugType> BT_lor;
+  enum LockingSemantics {
+    NotApplicable = 0,
+    PthreadSemantics,
+    XNUSemantics
+  };
 public:
   void checkPostStmt(const CallExpr *CE, CheckerContext &C) const;
     
-  void AcquireLock(CheckerContext &C, const CallExpr *CE,
+  void AcquireLock(CheckerContext &C, const CallExpr *CE, SVal lock,
-                   SVal lock, bool isTryLock) const;
+                   bool isTryLock, enum LockingSemantics semantics) const;
     
-  void ReleaseLock(CheckerContext &C, const CallExpr *CE,
+  void ReleaseLock(CheckerContext &C, const CallExpr *CE, SVal lock) const;
-                    SVal lock) const;
-
 };
 } // end anonymous namespace
 
@@ -43,7 +48,7 @@ namespace { class LockSet {}; }
 namespace clang {
 namespace ento {
 template <> struct GRStateTrait<LockSet> :
-  public GRStatePartialTrait<llvm::ImmutableSet<const MemRegion*> > {
+  public GRStatePartialTrait<llvm::ImmutableList<const MemRegion*> > {
     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)
-    if (CE->getNumArgs() != 1)
+    return;
-      return;
+  if (FName == "pthread_mutex_lock" ||
-    AcquireLock(C, CE, state->getSVal(CE->getArg(0)), false);
+      FName == "pthread_rwlock_rdlock" ||
-  }
+      FName == "pthread_rwlock_wrlock")
-  else if (FName == "pthread_mutex_trylock") {
+    AcquireLock(C, CE, state->getSVal(CE->getArg(0)), false, PthreadSemantics);
-    if (CE->getNumArgs() != 1)
+  else if (FName == "lck_mtx_lock" ||
-      return;
+           FName == "lck_rw_lock_exclusive" ||
-    AcquireLock(C, CE, state->getSVal(CE->getArg(0)), true);
+           FName == "lck_rw_lock_shared") 
-  }  
+    AcquireLock(C, CE, state->getSVal(CE->getArg(0)), false, XNUSemantics);
-  else if (FName == "pthread_mutex_unlock") {
+  else if (FName == "pthread_mutex_trylock" ||
-    if (CE->getNumArgs() != 1)
+           FName == "pthread_rwlock_tryrdlock" ||
-      return;
+           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<DefinedSVal>(X);
-  const GRState *lockSucc = state;
+
-  
+  llvm::ImmutableList<const MemRegion*> LS = state->get<LockSet>();
-  if (isTryLock) {
+
-      // Bifurcate the state, and allow a mode where the lock acquisition fails.
+  if (state->contains<LockSet>(lockR)) {
-    const GRState *lockFail;
+    if (!BT_doublelock)
-    llvm::tie(lockFail, lockSucc) = state->assume(retVal);    
+      BT_doublelock.reset(new BugType("Double locking", "Lock checker"));
-    assert(lockFail && lockSucc);
+    ExplodedNode *N = C.generateSink();
-    C.addTransition(C.generateNode(CE, lockFail));
+    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;
   }
-  else {
+
-      // Assume that the return value was 0.
+  const GRState *lockSucc = state;
+  if (isTryLock) {
+    // Bifurcate the state, and allow a mode where the lock acquisition fails.
+    const GRState *lockFail;
+    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(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<LockSet>(lockR);
-  
+  C.addTransition(lockSucc);
-  C.addTransition(lockSucc != state ? C.generateNode(CE, lockSucc) :
-                  C.getPredecessor());
 }
 
 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<const MemRegion*> LS = state->get<LockSet>();
 
-  // Record that the lock was released.  
+  // FIXME: Better analysis requires IPA for wrappers.
-  // FIXME: Handle unlocking locks that were never acquired.  This may
+  // FIXME: check for double unlocks
-  // require IPA for wrappers.
+  if (LS.isEmpty())
-  const GRState *unlockState = state->remove<LockSet>(lockR);
-  
-  if (state == unlockState)
     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<LockSet>(LS.getTail());
+  C.addTransition(state);
 }
 
+
 void ento::registerPthreadLockChecker(CheckerManager &mgr) {
   mgr.registerChecker<PthreadLockChecker>();
 }

clang/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}}
+}