[analyzer] lock_guard and unique_lock extension for BlockInCriticalSe…

…ction checker A patch by zdtorok (Zoltán Dániel Török)! Differential Revision: https://reviews.llvm.org/D33729 llvm-svn: 316892
llvm · Oct 30, 2017 · 9a8c8bf · 9a8c8bf
1 parent d0208b4
commit 9a8c8bf
Show file tree

Hide file tree

Showing 2 changed files with 85 additions and 11 deletions.
diff --git a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
@@ -26,15 +26,22 @@ using namespace ento;
 
 namespace {
 
-class BlockInCriticalSectionChecker : public Checker<check::PostCall,
-                                                     check::PreCall> {
+class BlockInCriticalSectionChecker : public Checker<check::PostCall> {
+
+  mutable IdentifierInfo *IILockGuard, *IIUniqueLock;
 
   CallDescription LockFn, UnlockFn, SleepFn, GetcFn, FgetsFn, ReadFn, RecvFn,
                   PthreadLockFn, PthreadTryLockFn, PthreadUnlockFn,
                   MtxLock, MtxTimedLock, MtxTryLock, MtxUnlock;
 
+  StringRef ClassLockGuard, ClassUniqueLock;
+
+  mutable bool IdentifierInfoInitialized;
+
   std::unique_ptr<BugType> BlockInCritSectionBugType;
 
+  void initIdentifierInfo(ASTContext &Ctx) const;
+
   void reportBlockInCritSection(SymbolRef FileDescSym,
                                 const CallEvent &call,
                                 CheckerContext &C) const;
@@ -46,35 +53,49 @@ class BlockInCriticalSectionChecker : public Checker<check::PostCall,
   bool isLockFunction(const CallEvent &Call) const;
   bool isUnlockFunction(const CallEvent &Call) const;
 
-  void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
-
   /// Process unlock.
   /// Process lock.
   /// Process blocking functions (sleep, getc, fgets, read, recv)
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
-
 };
 
 } // end anonymous namespace
 
 REGISTER_TRAIT_WITH_PROGRAMSTATE(MutexCounter, unsigned)
 
 BlockInCriticalSectionChecker::BlockInCriticalSectionChecker()
-    : LockFn("lock"), UnlockFn("unlock"), SleepFn("sleep"), GetcFn("getc"),
+    : IILockGuard(nullptr), IIUniqueLock(nullptr),
+      LockFn("lock"), UnlockFn("unlock"), SleepFn("sleep"), GetcFn("getc"),
       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") {
+      MtxUnlock("mtx_unlock"),
+      ClassLockGuard("lock_guard"),
+      ClassUniqueLock("unique_lock"),
+      IdentifierInfoInitialized(false) {
   // Initialize the bug type.
   BlockInCritSectionBugType.reset(
       new BugType(this, "Call to blocking function in critical section",
                         "Blocking Error"));
 }
 
+void BlockInCriticalSectionChecker::initIdentifierInfo(ASTContext &Ctx) const {
+  if (!IdentifierInfoInitialized) {
+    /* In case of checking C code, or when the corresponding headers are not
+     * included, we might end up query the identifier table every time when this
+     * function is called instead of early returning it. To avoid this, a bool
+     * variable (IdentifierInfoInitialized) is used and the function will be run
+     * only once. */
+    IILockGuard  = &Ctx.Idents.get(ClassLockGuard);
+    IIUniqueLock = &Ctx.Idents.get(ClassUniqueLock);
+    IdentifierInfoInitialized = true;
+  }
+}
+
 bool BlockInCriticalSectionChecker::isBlockingFunction(const CallEvent &Call) const {
   if (Call.isCalled(SleepFn)
       || Call.isCalled(GetcFn)
@@ -87,6 +108,12 @@ bool BlockInCriticalSectionChecker::isBlockingFunction(const CallEvent &Call) co
 }
 
 bool BlockInCriticalSectionChecker::isLockFunction(const CallEvent &Call) const {
+  if (const auto *Ctor = dyn_cast<CXXConstructorCall>(&Call)) {
+    auto IdentifierInfo = Ctor->getDecl()->getParent()->getIdentifier();
+    if (IdentifierInfo == IILockGuard || IdentifierInfo == IIUniqueLock)
+      return true;
+  }
+
   if (Call.isCalled(LockFn)
       || Call.isCalled(PthreadLockFn)
       || Call.isCalled(PthreadTryLockFn)
@@ -99,6 +126,13 @@ bool BlockInCriticalSectionChecker::isLockFunction(const CallEvent &Call) const
 }
 
 bool BlockInCriticalSectionChecker::isUnlockFunction(const CallEvent &Call) const {
+  if (const auto *Dtor = dyn_cast<CXXDestructorCall>(&Call)) {
+    const auto *DRecordDecl = dyn_cast<CXXRecordDecl>(Dtor->getDecl()->getParent());
+    auto IdentifierInfo = DRecordDecl->getIdentifier();
+    if (IdentifierInfo == IILockGuard || IdentifierInfo == IIUniqueLock)
+      return true;
+  }
+
   if (Call.isCalled(UnlockFn)
        || Call.isCalled(PthreadUnlockFn)
        || Call.isCalled(MtxUnlock)) {
@@ -107,12 +141,10 @@ bool BlockInCriticalSectionChecker::isUnlockFunction(const CallEvent &Call) cons
   return false;
 }
 
-void BlockInCriticalSectionChecker::checkPreCall(const CallEvent &Call,
-                                                 CheckerContext &C) const {
-}
-
 void BlockInCriticalSectionChecker::checkPostCall(const CallEvent &Call,
                                                   CheckerContext &C) const {
+  initIdentifierInfo(C.getASTContext());
+
   if (!isBlockingFunction(Call)
       && !isLockFunction(Call)
       && !isUnlockFunction(Call))

diff --git a/clang/test/Analysis/block-in-critical-section.cpp b/clang/test/Analysis/block-in-critical-section.cpp
@@ -7,6 +7,20 @@ struct mutex {
   void lock() {}
   void unlock() {}
 };
+template<typename T>
+struct lock_guard {
+  lock_guard<T>(std::mutex) {}
+  ~lock_guard<T>() {}
+};
+template<typename T>
+struct unique_lock {
+  unique_lock<T>(std::mutex) {}
+  ~unique_lock<T>() {}
+};
+template<typename T>
+struct not_real_lock {
+  not_real_lock<T>(std::mutex) {}
+};
 }
 
 void getc() {}
@@ -110,3 +124,31 @@ void testBlockInCriticalSectionUnexpectedUnlock() {
   m.lock();
   sleep(1); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
 }
+
+void testBlockInCriticalSectionLockGuard() {
+  std::mutex g_mutex;
+  std::not_real_lock<std::mutex> not_real_lock(g_mutex);
+  sleep(1); // no-warning
+
+  std::lock_guard<std::mutex> lock(g_mutex);
+  sleep(1); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+}
+
+void testBlockInCriticalSectionLockGuardNested() {
+  testBlockInCriticalSectionLockGuard();
+  sleep(1); // no-warning
+}
+
+void testBlockInCriticalSectionUniqueLock() {
+  std::mutex g_mutex;
+  std::not_real_lock<std::mutex> not_real_lock(g_mutex);
+  sleep(1); // no-warning
+
+  std::unique_lock<std::mutex> lock(g_mutex);
+  sleep(1); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+}
+
+void testBlockInCriticalSectionUniqueLockNested() {
+  testBlockInCriticalSectionUniqueLock();
+  sleep(1); // no-warning
+}