From 829c6bc04a6fd39845ada8498b149fdcc99139db Mon Sep 17 00:00:00 2001
From: Gabor Horvath <xazax.hun@gmail.com>
Date: Fri, 10 Mar 2017 14:50:12 +0000
Subject: [PATCH] [analyzer] Extend block in critical section check with C11
 and Pthread APIs.

Patch by Zoltan Daniel Torok!

Differential Revision: https://reviews.llvm.org/D29567

llvm-svn: 297461
---
 .../BlockInCriticalSectionChecker.cpp         | 70 +++++++++++++----
 .../Analysis/block-in-critical-section.cpp    | 76 +++++++++++++++++--
 clang/www/analyzer/alpha_checks.html          | 24 ++++++
 3 files changed, 150 insertions(+), 20 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
index 082a4873217b..d19630eeef77 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
@@ -29,7 +29,9 @@ namespace {
 class BlockInCriticalSectionChecker : public Checker<check::PostCall,
                                                      check::PreCall> {
 
-  CallDescription LockFn, UnlockFn, SleepFn, GetcFn, FgetsFn, ReadFn, RecvFn;
+  CallDescription LockFn, UnlockFn, SleepFn, GetcFn, FgetsFn, ReadFn, RecvFn,
+                  PthreadLockFn, PthreadTryLockFn, PthreadUnlockFn,
+                  MtxLock, MtxTimedLock, MtxTryLock, MtxUnlock;
 
   std::unique_ptr<BugType> BlockInCritSectionBugType;
 
@@ -40,6 +42,10 @@ class BlockInCriticalSectionChecker : public Checker<check::PostCall,
 public:
   BlockInCriticalSectionChecker();
 
+  bool isBlockingFunction(const CallEvent &Call) const;
+  bool isLockFunction(const CallEvent &Call) const;
+  bool isUnlockFunction(const CallEvent &Call) const;
+
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
 
   /// Process unlock.
@@ -55,34 +61,69 @@ REGISTER_TRAIT_WITH_PROGRAMSTATE(MutexCounter, unsigned)
 
 BlockInCriticalSectionChecker::BlockInCriticalSectionChecker()
     : LockFn("lock"), UnlockFn("unlock"), SleepFn("sleep"), GetcFn("getc"),
-      FgetsFn("fgets"), ReadFn("read"), RecvFn("recv") {
+      FgetsFn("fgets"), ReadFn("read"), RecvFn("recv"),
+      PthreadLockFn("pthread_mutex_lock"),
+      PthreadTryLockFn("pthread_mutex_trylock"),
+      PthreadUnlockFn("pthread_mutex_unlock"),
+      MtxLock("mtx_lock"),
+      MtxTimedLock("mtx_timedlock"),
+      MtxTryLock("mtx_trylock"),
+      MtxUnlock("mtx_unlock") {
   // Initialize the bug type.
   BlockInCritSectionBugType.reset(
       new BugType(this, "Call to blocking function in critical section",
                         "Blocking Error"));
 }
 
+bool BlockInCriticalSectionChecker::isBlockingFunction(const CallEvent &Call) const {
+  if (Call.isCalled(SleepFn)
+      || Call.isCalled(GetcFn)
+      || Call.isCalled(FgetsFn)
+      || Call.isCalled(ReadFn)
+      || Call.isCalled(RecvFn)) {
+    return true;
+  }
+  return false;
+}
+
+bool BlockInCriticalSectionChecker::isLockFunction(const CallEvent &Call) const {
+  if (Call.isCalled(LockFn)
+      || Call.isCalled(PthreadLockFn)
+      || Call.isCalled(PthreadTryLockFn)
+      || Call.isCalled(MtxLock)
+      || Call.isCalled(MtxTimedLock)
+      || Call.isCalled(MtxTryLock)) {
+    return true;
+  }
+  return false;
+}
+
+bool BlockInCriticalSectionChecker::isUnlockFunction(const CallEvent &Call) const {
+  if (Call.isCalled(UnlockFn)
+       || Call.isCalled(PthreadUnlockFn)
+       || Call.isCalled(MtxUnlock)) {
+    return true;
+  }
+  return false;
+}
+
 void BlockInCriticalSectionChecker::checkPreCall(const CallEvent &Call,
                                                  CheckerContext &C) const {
 }
 
 void BlockInCriticalSectionChecker::checkPostCall(const CallEvent &Call,
                                                   CheckerContext &C) const {
-  if (!Call.isCalled(LockFn)
-      && !Call.isCalled(SleepFn)
-      && !Call.isCalled(GetcFn)
-      && !Call.isCalled(FgetsFn)
-      && !Call.isCalled(ReadFn)
-      && !Call.isCalled(RecvFn)
-      && !Call.isCalled(UnlockFn))
+  if (!isBlockingFunction(Call)
+      && !isLockFunction(Call)
+      && !isUnlockFunction(Call))
     return;
 
   ProgramStateRef State = C.getState();
   unsigned mutexCount = State->get<MutexCounter>();
-  if (Call.isCalled(UnlockFn) && mutexCount > 0) {
+  if (isUnlockFunction(Call) && mutexCount > 0) {
     State = State->set<MutexCounter>(--mutexCount);
     C.addTransition(State);
-  } else if (Call.isCalled(LockFn)) {
+  } else if (isLockFunction(Call)) {
     State = State->set<MutexCounter>(++mutexCount);
     C.addTransition(State);
   } else if (mutexCount > 0) {
@@ -97,8 +138,11 @@ void BlockInCriticalSectionChecker::reportBlockInCritSection(
   if (!ErrNode)
     return;
 
-  auto R = llvm::make_unique<BugReport>(*BlockInCritSectionBugType,
-      "A blocking function %s is called inside a critical section.", ErrNode);
+  std::string msg;
+  llvm::raw_string_ostream os(msg);
+  os << "Call to blocking function '" << Call.getCalleeIdentifier()->getName()
+     << "' inside of critical section";
+  auto R = llvm::make_unique<BugReport>(*BlockInCritSectionBugType, os.str(), ErrNode);
   R->addRange(Call.getSourceRange());
   R->markInteresting(BlockDescSym);
   C.emitReport(std::move(R));
diff --git a/clang/test/Analysis/block-in-critical-section.cpp b/clang/test/Analysis/block-in-critical-section.cpp
index c940ad563f7c..c65cc612cf73 100644
--- a/clang/test/Analysis/block-in-critical-section.cpp
+++ b/clang/test/Analysis/block-in-critical-section.cpp
@@ -9,29 +9,91 @@ struct mutex {
 };
 }
 
-void testBlockInCriticalSection() {
+void getc() {}
+void fgets() {}
+void read() {}
+void recv() {}
+
+void pthread_mutex_lock() {}
+void pthread_mutex_trylock() {}
+void pthread_mutex_unlock() {}
+
+void mtx_lock() {}
+void mtx_timedlock() {}
+void mtx_trylock() {}
+void mtx_unlock() {}
+
+void testBlockInCriticalSectionWithStdMutex() {
   std::mutex m;
   m.lock();
-  sleep(3); // expected-warning {{A blocking function %s is called inside a critical section}}
+  sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+  getc(); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
+  fgets(); // expected-warning {{Call to blocking function 'fgets' inside of critical section}}
+  read(); // expected-warning {{Call to blocking function 'read' inside of critical section}}
+  recv(); // expected-warning {{Call to blocking function 'recv' inside of critical section}}
   m.unlock();
 }
 
+void testBlockInCriticalSectionWithPthreadMutex() {
+  pthread_mutex_lock();
+  sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+  getc(); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
+  fgets(); // expected-warning {{Call to blocking function 'fgets' inside of critical section}}
+  read(); // expected-warning {{Call to blocking function 'read' inside of critical section}}
+  recv(); // expected-warning {{Call to blocking function 'recv' inside of critical section}}
+  pthread_mutex_unlock();
+
+  pthread_mutex_trylock();
+  sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+  getc(); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
+  fgets(); // expected-warning {{Call to blocking function 'fgets' inside of critical section}}
+  read(); // expected-warning {{Call to blocking function 'read' inside of critical section}}
+  recv(); // expected-warning {{Call to blocking function 'recv' inside of critical section}}
+  pthread_mutex_unlock();
+}
+
+void testBlockInCriticalSectionC11Locks() {
+  mtx_lock();
+  sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+  getc(); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
+  fgets(); // expected-warning {{Call to blocking function 'fgets' inside of critical section}}
+  read(); // expected-warning {{Call to blocking function 'read' inside of critical section}}
+  recv(); // expected-warning {{Call to blocking function 'recv' inside of critical section}}
+  mtx_unlock();
+
+  mtx_timedlock();
+  sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+  getc(); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
+  fgets(); // expected-warning {{Call to blocking function 'fgets' inside of critical section}}
+  read(); // expected-warning {{Call to blocking function 'read' inside of critical section}}
+  recv(); // expected-warning {{Call to blocking function 'recv' inside of critical section}}
+  mtx_unlock();
+
+  mtx_trylock();
+  sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+  getc(); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
+  fgets(); // expected-warning {{Call to blocking function 'fgets' inside of critical section}}
+  read(); // expected-warning {{Call to blocking function 'read' inside of critical section}}
+  recv(); // expected-warning {{Call to blocking function 'recv' inside of critical section}}
+  mtx_unlock();
+}
+
 void testBlockInCriticalSectionWithNestedMutexes() {
   std::mutex m, n, k;
   m.lock();
   n.lock();
   k.lock();
-  sleep(3); // expected-warning {{A blocking function %s is called inside a critical section}}
+  sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
   k.unlock();
-  sleep(5); // expected-warning {{A blocking function %s is called inside a critical section}}
+  sleep(5); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
   n.unlock();
-  sleep(3); // expected-warning {{A blocking function %s is called inside a critical section}}
+  sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
   m.unlock();
   sleep(3); // no-warning
 }
 
 void f() {
-  sleep(1000); // expected-warning {{A blocking function %s is called inside a critical section}}
+  sleep(1000); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
 }
 
 void testBlockInCriticalSectionInterProcedural() {
@@ -46,5 +108,5 @@ void testBlockInCriticalSectionUnexpectedUnlock() {
   m.unlock();
   sleep(1); // no-warning
   m.lock();
-  sleep(1); // expected-warning {{A blocking function %s is called inside a critical section}}
+  sleep(1); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
 }
diff --git a/clang/www/analyzer/alpha_checks.html b/clang/www/analyzer/alpha_checks.html
index 0312d16aced4..ce9392b9960c 100644
--- a/clang/www/analyzer/alpha_checks.html
+++ b/clang/www/analyzer/alpha_checks.html
@@ -910,6 +910,30 @@ void test(char *y) {
 }
 </pre></div></div></td></tr>
 
+
+<tr><td><div class="namedescr expandable"><span class="name">
+alpha.unix.cstring.BlockInCriticalSection</span><span class="lang">
+(C)</span><div class="descr">
+Check for calls to blocking functions inside a critical section; applies
+to:<div class=functions>
+lock, unlock<br>
+sleep<br>
+getc<br>
+fgets<br>
+read<br>
+recv<br>
+pthread_mutex_lock, pthread_mutex_trylock, pthread_mutex_unlock<br>
+mtx_lock, mtx_timedlock, mtx_trylock, mtx_unlock<br>
+</div></div></div></td>
+<td><div class="exampleContainer expandable">
+<div class="example"><pre>
+void testBlockInCriticalSection() {
+  std::mutex m;
+  m.lock();
+  sleep(3); // warn
+  m.unlock();
+}
+</pre></div></div></td></tr>
 </tbody></table>
 
 </div> <!-- page -->