[clang][analyzer] Improve BlockInCriticalSectionsChecker (#80029)

* Add support for multiple, potentially overlapping critical sections:
  The checker can now simultaneously handle several mutex's critical
  sections without confusing them.
* Implement the handling of recursive mutexes:
  By identifying the lock events, recursive mutexes are now supported.
  A lock event is a pair of a lock expression, and the SVal of the mutex
  that it locks, so even multiple locks of the same mutex (and even by
  the same expression) is now supported.
* Refine the note tags generated by the checker:
  The note tags now correctly show just for mutexes that are
  active at the point of error, and multiple acquisitions of the same mutex
  are also noted.

2 files changed, 510 insertions, 146 deletions

diff --git a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
index 66e080a..e437391 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
@@ -20,48 +20,178 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringExtras.h"
+
+#include <iterator>
+#include <utility>
+#include <variant>
 
 using namespace clang;
 using namespace ento;
 
 namespace {
+
+struct CritSectionMarker {
+  const Expr *LockExpr{};
+  const MemRegion *LockReg{};
+
+  void Profile(llvm::FoldingSetNodeID &ID) const {
+    ID.Add(LockExpr);
+    ID.Add(LockReg);
+  }
+
+  [[nodiscard]] constexpr bool
+  operator==(const CritSectionMarker &Other) const noexcept {
+    return LockExpr == Other.LockExpr && LockReg == Other.LockReg;
+  }
+  [[nodiscard]] constexpr bool
+  operator!=(const CritSectionMarker &Other) const noexcept {
+    return !(*this == Other);
+  }
+};
+
+class CallDescriptionBasedMatcher {
+  CallDescription LockFn;
+  CallDescription UnlockFn;
+
+public:
+  CallDescriptionBasedMatcher(CallDescription &&LockFn,
+                              CallDescription &&UnlockFn)
+      : LockFn(std::move(LockFn)), UnlockFn(std::move(UnlockFn)) {}
+  [[nodiscard]] bool matches(const CallEvent &Call, bool IsLock) const {
+    if (IsLock) {
+      return LockFn.matches(Call);
+    }
+    return UnlockFn.matches(Call);
+  }
+};
+
+class FirstArgMutexDescriptor : public CallDescriptionBasedMatcher {
+public:
+  FirstArgMutexDescriptor(CallDescription &&LockFn, CallDescription &&UnlockFn)
+      : CallDescriptionBasedMatcher(std::move(LockFn), std::move(UnlockFn)) {}
+
+  [[nodiscard]] const MemRegion *getRegion(const CallEvent &Call, bool) const {
+    return Call.getArgSVal(0).getAsRegion();
+  }
+};
+
+class MemberMutexDescriptor : public CallDescriptionBasedMatcher {
+public:
+  MemberMutexDescriptor(CallDescription &&LockFn, CallDescription &&UnlockFn)
+      : CallDescriptionBasedMatcher(std::move(LockFn), std::move(UnlockFn)) {}
+
+  [[nodiscard]] const MemRegion *getRegion(const CallEvent &Call, bool) const {
+    return cast<CXXMemberCall>(Call).getCXXThisVal().getAsRegion();
+  }
+};
+
+class RAIIMutexDescriptor {
+  mutable const IdentifierInfo *Guard{};
+  mutable bool IdentifierInfoInitialized{};
+  mutable llvm::SmallString<32> GuardName{};
+
+  void initIdentifierInfo(const CallEvent &Call) 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.
+      Guard = &Call.getCalleeAnalysisDeclContext()->getASTContext().Idents.get(
+          GuardName);
+      IdentifierInfoInitialized = true;
+    }
+  }
+
+  template <typename T> bool matchesImpl(const CallEvent &Call) const {
+    const T *C = dyn_cast<T>(&Call);
+    if (!C)
+      return false;
+    const IdentifierInfo *II =
+        cast<CXXRecordDecl>(C->getDecl()->getParent())->getIdentifier();
+    return II == Guard;
+  }
+
+public:
+  RAIIMutexDescriptor(StringRef GuardName) : GuardName(GuardName) {}
+  [[nodiscard]] bool matches(const CallEvent &Call, bool IsLock) const {
+    initIdentifierInfo(Call);
+    if (IsLock) {
+      return matchesImpl<CXXConstructorCall>(Call);
+    }
+    return matchesImpl<CXXDestructorCall>(Call);
+  }
+  [[nodiscard]] const MemRegion *getRegion(const CallEvent &Call,
+                                           bool IsLock) const {
+    const MemRegion *LockRegion = nullptr;
+    if (IsLock) {
+      if (std::optional<SVal> Object = Call.getReturnValueUnderConstruction()) {
+        LockRegion = Object->getAsRegion();
+      }
+    } else {
+      LockRegion = cast<CXXDestructorCall>(Call).getCXXThisVal().getAsRegion();
+    }
+    return LockRegion;
+  }
+};
+
+using MutexDescriptor =
+    std::variant<FirstArgMutexDescriptor, MemberMutexDescriptor,
+                 RAIIMutexDescriptor>;
+
 class BlockInCriticalSectionChecker : public Checker<check::PostCall> {
-  mutable IdentifierInfo *IILockGuard = nullptr;
-  mutable IdentifierInfo *IIUniqueLock = nullptr;
-  mutable bool IdentifierInfoInitialized = false;
-
-  const CallDescription LockFn{{"lock"}};
-  const CallDescription UnlockFn{{"unlock"}};
-  const CallDescription SleepFn{{"sleep"}};
-  const CallDescription GetcFn{{"getc"}};
-  const CallDescription FgetsFn{{"fgets"}};
-  const CallDescription ReadFn{{"read"}};
-  const CallDescription RecvFn{{"recv"}};
-  const CallDescription PthreadLockFn{{"pthread_mutex_lock"}};
-  const CallDescription PthreadTryLockFn{{"pthread_mutex_trylock"}};
-  const CallDescription PthreadUnlockFn{{"pthread_mutex_unlock"}};
-  const CallDescription MtxLock{{"mtx_lock"}};
-  const CallDescription MtxTimedLock{{"mtx_timedlock"}};
-  const CallDescription MtxTryLock{{"mtx_trylock"}};
-  const CallDescription MtxUnlock{{"mtx_unlock"}};
-
-  const llvm::StringLiteral ClassLockGuard{"lock_guard"};
-  const llvm::StringLiteral ClassUniqueLock{"unique_lock"};
+private:
+  const std::array<MutexDescriptor, 8> MutexDescriptors{
+      MemberMutexDescriptor(
+          CallDescription(/*QualifiedName=*/{"std", "mutex", "lock"},
+                          /*RequiredArgs=*/0),
+          CallDescription({"std", "mutex", "unlock"}, 0)),
+      FirstArgMutexDescriptor(CallDescription({"pthread_mutex_lock"}, 1),
+                              CallDescription({"pthread_mutex_unlock"}, 1)),
+      FirstArgMutexDescriptor(CallDescription({"mtx_lock"}, 1),
+                              CallDescription({"mtx_unlock"}, 1)),
+      FirstArgMutexDescriptor(CallDescription({"pthread_mutex_trylock"}, 1),
+                              CallDescription({"pthread_mutex_unlock"}, 1)),
+      FirstArgMutexDescriptor(CallDescription({"mtx_trylock"}, 1),
+                              CallDescription({"mtx_unlock"}, 1)),
+      FirstArgMutexDescriptor(CallDescription({"mtx_timedlock"}, 1),
+                              CallDescription({"mtx_unlock"}, 1)),
+      RAIIMutexDescriptor("lock_guard"),
+      RAIIMutexDescriptor("unique_lock")};
+
+  const std::array<CallDescription, 5> BlockingFunctions{
+      ArrayRef{StringRef{"sleep"}}, ArrayRef{StringRef{"getc"}},
+      ArrayRef{StringRef{"fgets"}}, ArrayRef{StringRef{"read"}},
+      ArrayRef{StringRef{"recv"}}};
 
   const BugType BlockInCritSectionBugType{
       this, "Call to blocking function in critical section", "Blocking Error"};
 
-  void initIdentifierInfo(ASTContext &Ctx) const;
+  void reportBlockInCritSection(const CallEvent &call, CheckerContext &C) const;
 
-  void reportBlockInCritSection(SymbolRef FileDescSym,
-                                const CallEvent &call,
-                                CheckerContext &C) const;
+  [[nodiscard]] const NoteTag *createCritSectionNote(CritSectionMarker M,
+                                                     CheckerContext &C) const;
 
-public:
-  bool isBlockingFunction(const CallEvent &Call) const;
-  bool isLockFunction(const CallEvent &Call) const;
-  bool isUnlockFunction(const CallEvent &Call) const;
+  [[nodiscard]] std::optional<MutexDescriptor>
+  checkDescriptorMatch(const CallEvent &Call, CheckerContext &C,
+                       bool IsLock) const;
+
+  void handleLock(const MutexDescriptor &Mutex, const CallEvent &Call,
+                  CheckerContext &C) const;
 
+  void handleUnlock(const MutexDescriptor &Mutex, const CallEvent &Call,
+                    CheckerContext &C) const;
+
+  [[nodiscard]] bool isBlockingInCritSection(const CallEvent &Call,
+                                             CheckerContext &C) const;
+
+public:
   /// Process unlock.
   /// Process lock.
   /// Process blocking functions (sleep, getc, fgets, read, recv)
@@ -70,73 +200,118 @@ public:
 
 } // end anonymous namespace
 
-REGISTER_TRAIT_WITH_PROGRAMSTATE(MutexCounter, unsigned)
-
-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;
-  }
+REGISTER_LIST_WITH_PROGRAMSTATE(ActiveCritSections, CritSectionMarker)
+
+namespace std {
+// Iterator traits for ImmutableList data structure
+// that enable the use of STL algorithms.
+// TODO: Move these to llvm::ImmutableList when overhauling immutable data
+// structures for proper iterator concept support.
+template <>
+struct iterator_traits<
+    typename llvm::ImmutableList<CritSectionMarker>::iterator> {
+  using iterator_category = std::forward_iterator_tag;
+  using value_type = CritSectionMarker;
+  using difference_type = std::ptrdiff_t;
+  using reference = CritSectionMarker &;
+  using pointer = CritSectionMarker *;
+};
+} // namespace std
+
+std::optional<MutexDescriptor>
+BlockInCriticalSectionChecker::checkDescriptorMatch(const CallEvent &Call,
+                                                    CheckerContext &C,
+                                                    bool IsLock) const {
+  const auto Descriptor =
+      llvm::find_if(MutexDescriptors, [&Call, IsLock](auto &&Descriptor) {
+        return std::visit(
+            [&Call, IsLock](auto &&DescriptorImpl) {
+              return DescriptorImpl.matches(Call, IsLock);
+            },
+            Descriptor);
+      });
+  if (Descriptor != MutexDescriptors.end())
+    return *Descriptor;
+  return std::nullopt;
 }
 
-bool BlockInCriticalSectionChecker::isBlockingFunction(const CallEvent &Call) const {
-  return matchesAny(Call, SleepFn, GetcFn, FgetsFn, ReadFn, RecvFn);
+static const MemRegion *getRegion(const CallEvent &Call,
+                                  const MutexDescriptor &Descriptor,
+                                  bool IsLock) {
+  return std::visit(
+      [&Call, IsLock](auto &&Descriptor) {
+        return Descriptor.getRegion(Call, IsLock);
+      },
+      Descriptor);
 }
 
-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;
-  }
+void BlockInCriticalSectionChecker::handleLock(
+    const MutexDescriptor &LockDescriptor, const CallEvent &Call,
+    CheckerContext &C) const {
+  const MemRegion *MutexRegion =
+      getRegion(Call, LockDescriptor, /*IsLock=*/true);
+  if (!MutexRegion)
+    return;
 
-  return matchesAny(Call, LockFn, PthreadLockFn, PthreadTryLockFn, MtxLock,
-                    MtxTimedLock, MtxTryLock);
+  const CritSectionMarker MarkToAdd{Call.getOriginExpr(), MutexRegion};
+  ProgramStateRef StateWithLockEvent =
+      C.getState()->add<ActiveCritSections>(MarkToAdd);
+  C.addTransition(StateWithLockEvent, createCritSectionNote(MarkToAdd, C));
 }
 
-bool BlockInCriticalSectionChecker::isUnlockFunction(const CallEvent &Call) const {
-  if (const auto *Dtor = dyn_cast<CXXDestructorCall>(&Call)) {
-    const auto *DRecordDecl = cast<CXXRecordDecl>(Dtor->getDecl()->getParent());
-    auto IdentifierInfo = DRecordDecl->getIdentifier();
-    if (IdentifierInfo == IILockGuard || IdentifierInfo == IIUniqueLock)
-      return true;
+void BlockInCriticalSectionChecker::handleUnlock(
+    const MutexDescriptor &UnlockDescriptor, const CallEvent &Call,
+    CheckerContext &C) const {
+  const MemRegion *MutexRegion =
+      getRegion(Call, UnlockDescriptor, /*IsLock=*/false);
+  if (!MutexRegion)
+    return;
+
+  ProgramStateRef State = C.getState();
+  const auto ActiveSections = State->get<ActiveCritSections>();
+  const auto MostRecentLock =
+      llvm::find_if(ActiveSections, [MutexRegion](auto &&Marker) {
+        return Marker.LockReg == MutexRegion;
+      });
+  if (MostRecentLock == ActiveSections.end())
+    return;
+
+  // Build a new ImmutableList without this element.
+  auto &Factory = State->get_context<ActiveCritSections>();
+  llvm::ImmutableList<CritSectionMarker> NewList = Factory.getEmptyList();
+  for (auto It = ActiveSections.begin(), End = ActiveSections.end(); It != End;
+       ++It) {
+    if (It != MostRecentLock)
+      NewList = Factory.add(*It, NewList);
   }
 
-  return matchesAny(Call, UnlockFn, PthreadUnlockFn, MtxUnlock);
+  State = State->set<ActiveCritSections>(NewList);
+  C.addTransition(State);
+}
+
+bool BlockInCriticalSectionChecker::isBlockingInCritSection(
+    const CallEvent &Call, CheckerContext &C) const {
+  return llvm::any_of(BlockingFunctions,
+                      [&Call](auto &&Fn) { return Fn.matches(Call); }) &&
+         !C.getState()->get<ActiveCritSections>().isEmpty();
 }
 
 void BlockInCriticalSectionChecker::checkPostCall(const CallEvent &Call,
                                                   CheckerContext &C) const {
-  initIdentifierInfo(C.getASTContext());
-
-  if (!isBlockingFunction(Call)
-      && !isLockFunction(Call)
-      && !isUnlockFunction(Call))
-    return;
-
-  ProgramStateRef State = C.getState();
-  unsigned mutexCount = State->get<MutexCounter>();
-  if (isUnlockFunction(Call) && mutexCount > 0) {
-    State = State->set<MutexCounter>(--mutexCount);
-    C.addTransition(State);
-  } else if (isLockFunction(Call)) {
-    State = State->set<MutexCounter>(++mutexCount);
-    C.addTransition(State);
-  } else if (mutexCount > 0) {
-    SymbolRef BlockDesc = Call.getReturnValue().getAsSymbol();
-    reportBlockInCritSection(BlockDesc, Call, C);
+  if (isBlockingInCritSection(Call, C)) {
+    reportBlockInCritSection(Call, C);
+  } else if (std::optional<MutexDescriptor> LockDesc =
+                 checkDescriptorMatch(Call, C, /*IsLock=*/true)) {
+    handleLock(*LockDesc, Call, C);
+  } else if (std::optional<MutexDescriptor> UnlockDesc =
+                 checkDescriptorMatch(Call, C, /*IsLock=*/false)) {
+    handleUnlock(*UnlockDesc, Call, C);
   }
 }
 
 void BlockInCriticalSectionChecker::reportBlockInCritSection(
-    SymbolRef BlockDescSym, const CallEvent &Call, CheckerContext &C) const {
-  ExplodedNode *ErrNode = C.generateNonFatalErrorNode();
+    const CallEvent &Call, CheckerContext &C) const {
+  ExplodedNode *ErrNode = C.generateNonFatalErrorNode(C.getState());
   if (!ErrNode)
     return;
 
@@ -147,14 +322,63 @@ void BlockInCriticalSectionChecker::reportBlockInCritSection(
   auto R = std::make_unique<PathSensitiveBugReport>(BlockInCritSectionBugType,
                                                     os.str(), ErrNode);
   R->addRange(Call.getSourceRange());
-  R->markInteresting(BlockDescSym);
+  R->markInteresting(Call.getReturnValue());
   C.emitReport(std::move(R));
 }
 
+const NoteTag *
+BlockInCriticalSectionChecker::createCritSectionNote(CritSectionMarker M,
+                                                     CheckerContext &C) const {
+  const BugType *BT = &this->BlockInCritSectionBugType;
+  return C.getNoteTag([M, BT](PathSensitiveBugReport &BR,
+                              llvm::raw_ostream &OS) {
+    if (&BR.getBugType() != BT)
+      return;
+
+    // Get the lock events for the mutex of the current line's lock event.
+    const auto CritSectionBegins =
+        BR.getErrorNode()->getState()->get<ActiveCritSections>();
+    llvm::SmallVector<CritSectionMarker, 4> LocksForMutex;
+    llvm::copy_if(
+        CritSectionBegins, std::back_inserter(LocksForMutex),
+        [M](const auto &Marker) { return Marker.LockReg == M.LockReg; });
+    if (LocksForMutex.empty())
+      return;
+
+    // As the ImmutableList builds the locks by prepending them, we
+    // reverse the list to get the correct order.
+    std::reverse(LocksForMutex.begin(), LocksForMutex.end());
+
+    // Find the index of the lock expression in the list of all locks for a
+    // given mutex (in acquisition order).
+    const auto Position =
+        llvm::find_if(std::as_const(LocksForMutex), [M](const auto &Marker) {
+          return Marker.LockExpr == M.LockExpr;
+        });
+    if (Position == LocksForMutex.end())
+      return;
+
+    // If there is only one lock event, we don't need to specify how many times
+    // the critical section was entered.
+    if (LocksForMutex.size() == 1) {
+      OS << "Entering critical section here";
+      return;
+    }
+
+    const auto IndexOfLock =
+        std::distance(std::as_const(LocksForMutex).begin(), Position);
+
+    const auto OrdinalOfLock = IndexOfLock + 1;
+    OS << "Entering critical section for the " << OrdinalOfLock
+       << llvm::getOrdinalSuffix(OrdinalOfLock) << " time here";
+  });
+}
+
 void ento::registerBlockInCriticalSectionChecker(CheckerManager &mgr) {
   mgr.registerChecker<BlockInCriticalSectionChecker>();
 }
 
-bool ento::shouldRegisterBlockInCriticalSectionChecker(const CheckerManager &mgr) {
+bool ento::shouldRegisterBlockInCriticalSectionChecker(
+    const CheckerManager &mgr) {
   return true;
 }
diff --git a/clang/test/Analysis/block-in-critical-section.cpp b/clang/test/Analysis/block-in-critical-section.cpp
index fcf6188f..87c26b9 100644
--- a/clang/test/Analysis/block-in-critical-section.cpp
+++ b/clang/test/Analysis/block-in-critical-section.cpp
@@ -1,4 +1,8 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.BlockInCriticalSection -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 \
+// RUN:   -analyzer-checker=alpha.unix.BlockInCriticalSection \
+// RUN:   -std=c++11 \
+// RUN:   -analyzer-output text \
+// RUN:   -verify %s
 
 void sleep(int x) {}
 
@@ -21,108 +25,242 @@ template<typename T>
 struct not_real_lock {
   not_real_lock<T>(std::mutex) {}
 };
-}
+} // namespace std
+
+struct FILE;
+int getc(FILE *stream);
+char* fgets(char *str, FILE *stream);
+using ssize_t = long long;
+using size_t = unsigned long long;
+ssize_t read(int fd, void *buf, size_t count);
+ssize_t recv(int sockfd, void *buf, size_t len, int flags);
 
-void getc() {}
-void fgets() {}
-void read() {}
-void recv() {}
+struct pthread_mutex_t;
+void pthread_mutex_lock(pthread_mutex_t *mutex);
+void pthread_mutex_trylock(pthread_mutex_t *mutex);
+void pthread_mutex_unlock(pthread_mutex_t *mutex);
 
-void pthread_mutex_lock() {}
-void pthread_mutex_trylock() {}
-void pthread_mutex_unlock() {}
+struct mtx_t;
+void mtx_lock(mtx_t *mutex);
+void mtx_timedlock(mtx_t *mutex);
+void mtx_trylock(mtx_t *mutex);
+void mtx_unlock(mtx_t *mutex);
 
-void mtx_lock() {}
-void mtx_timedlock() {}
-void mtx_trylock() {}
-void mtx_unlock() {}
+// global params for dummy function calls
+FILE *stream;
+char *str;
+int fd;
+void *buf;
+size_t count;
+int sockfd;
+size_t len;
+int flags;
 
 void testBlockInCriticalSectionWithStdMutex() {
   std::mutex m;
-  m.lock();
+  m.lock(); // expected-note 5{{Entering critical section here}}
   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}}
+            // expected-note@-1 {{Call to blocking function 'sleep' inside of critical section}}
+  getc(stream); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
+          // expected-note@-1 {{Call to blocking function 'getc' inside of critical section}}
+  fgets(str, stream); // expected-warning {{Call to blocking function 'fgets' inside of critical section}}
+           // expected-note@-1 {{Call to blocking function 'fgets' inside of critical section}}
+  read(fd, buf, count); // expected-warning {{Call to blocking function 'read' inside of critical section}}
+          // expected-note@-1 {{Call to blocking function 'read' inside of critical section}}
+  recv(sockfd, buf, count, flags); // expected-warning {{Call to blocking function 'recv' inside of critical section}}
+          // expected-note@-1 {{Call to blocking function 'recv' inside of critical section}}
   m.unlock();
 }
 
-void testBlockInCriticalSectionWithPthreadMutex() {
-  pthread_mutex_lock();
+void testBlockInCriticalSectionWithPthreadMutex(pthread_mutex_t *mutex) {
+  pthread_mutex_lock(mutex); // expected-note 5{{Entering critical section here}}
   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();
+            // expected-note@-1 {{Call to blocking function 'sleep' inside of critical section}}
+  getc(stream); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
+          // expected-note@-1 {{Call to blocking function 'getc' inside of critical section}}
+  fgets(str, stream); // expected-warning {{Call to blocking function 'fgets' inside of critical section}}
+           // expected-note@-1 {{Call to blocking function 'fgets' inside of critical section}}
+  read(fd, buf, count); // expected-warning {{Call to blocking function 'read' inside of critical section}}
+          // expected-note@-1 {{Call to blocking function 'read' inside of critical section}}
+  recv(sockfd, buf, count, flags); // expected-warning {{Call to blocking function 'recv' inside of critical section}}
+          // expected-note@-1 {{Call to blocking function 'recv' inside of critical section}}
+  pthread_mutex_unlock(mutex);
 
-  pthread_mutex_trylock();
+  pthread_mutex_trylock(mutex); // expected-note 5{{Entering critical section here}}
   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();
+            // expected-note@-1 {{Call to blocking function 'sleep' inside of critical section}}
+  getc(stream); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
+          // expected-note@-1 {{Call to blocking function 'getc' inside of critical section}}
+  fgets(str, stream); // expected-warning {{Call to blocking function 'fgets' inside of critical section}}
+           // expected-note@-1 {{Call to blocking function 'fgets' inside of critical section}}
+  read(fd, buf, count); // expected-warning {{Call to blocking function 'read' inside of critical section}}
+          // expected-note@-1 {{Call to blocking function 'read' inside of critical section}}
+  recv(sockfd, buf, count, flags); // expected-warning {{Call to blocking function 'recv' inside of critical section}}
+          // expected-note@-1 {{Call to blocking function 'recv' inside of critical section}}
+  pthread_mutex_unlock(mutex);
 }
 
-void testBlockInCriticalSectionC11Locks() {
-  mtx_lock();
+void testBlockInCriticalSectionC11Locks(mtx_t *mutex) {
+  mtx_lock(mutex); // expected-note 5{{Entering critical section here}}
   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();
+            // expected-note@-1 {{Call to blocking function 'sleep' inside of critical section}}
+  getc(stream); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
+          // expected-note@-1 {{Call to blocking function 'getc' inside of critical section}}
+  fgets(str, stream); // expected-warning {{Call to blocking function 'fgets' inside of critical section}}
+           // expected-note@-1 {{Call to blocking function 'fgets' inside of critical section}}
+  read(fd, buf, count); // expected-warning {{Call to blocking function 'read' inside of critical section}}
+          // expected-note@-1 {{Call to blocking function 'read' inside of critical section}}
+  recv(sockfd, buf, count, flags); // expected-warning {{Call to blocking function 'recv' inside of critical section}}
+          // expected-note@-1 {{Call to blocking function 'recv' inside of critical section}}
+  mtx_unlock(mutex);
 
-  mtx_timedlock();
+  mtx_timedlock(mutex); // expected-note 5{{Entering critical section here}}
   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();
+            // expected-note@-1 {{Call to blocking function 'sleep' inside of critical section}}
+  getc(stream); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
+          // expected-note@-1 {{Call to blocking function 'getc' inside of critical section}}
+  fgets(str, stream); // expected-warning {{Call to blocking function 'fgets' inside of critical section}}
+           // expected-note@-1 {{Call to blocking function 'fgets' inside of critical section}}
+  read(fd, buf, count); // expected-warning {{Call to blocking function 'read' inside of critical section}}
+          // expected-note@-1 {{Call to blocking function 'read' inside of critical section}}
+  recv(sockfd, buf, count, flags); // expected-warning {{Call to blocking function 'recv' inside of critical section}}
+          // expected-note@-1 {{Call to blocking function 'recv' inside of critical section}}
+  mtx_unlock(mutex);
 
-  mtx_trylock();
+  mtx_trylock(mutex); // expected-note 5{{Entering critical section here}}
   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();
+            // expected-note@-1 {{Call to blocking function 'sleep' inside of critical section}}
+  getc(stream); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
+          // expected-note@-1 {{Call to blocking function 'getc' inside of critical section}}
+  fgets(str, stream); // expected-warning {{Call to blocking function 'fgets' inside of critical section}}
+           // expected-note@-1 {{Call to blocking function 'fgets' inside of critical section}}
+  read(fd, buf, count); // expected-warning {{Call to blocking function 'read' inside of critical section}}
+          // expected-note@-1 {{Call to blocking function 'read' inside of critical section}}
+  recv(sockfd, buf, count, flags); // expected-warning {{Call to blocking function 'recv' inside of critical section}}
+          // expected-note@-1 {{Call to blocking function 'recv' inside of critical section}}
+  mtx_unlock(mutex);
+}
+
+void testMultipleBlockingCalls() {
+  std::mutex m;
+  m.lock(); // expected-note 1{{Entering critical section here}}
+  sleep(1); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+            // expected-note@-1 {{Call to blocking function 'sleep' inside of critical section}}
+  m.unlock();
+  sleep(2); // no-warning
 }
 
-void testBlockInCriticalSectionWithNestedMutexes() {
+void testMultipleMutexesMultipleBlockingCalls() {
   std::mutex m, n, k;
-  m.lock();
-  n.lock();
-  k.lock();
-  sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+  m.lock(); // expected-note 2{{Entering critical section here}}
+  n.lock(); // expected-note 2{{Entering critical section here}}
+  k.lock(); // expected-note 1{{Entering critical section here}}
+  sleep(1); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+            // expected-note@-1 {{Call to blocking function 'sleep' inside of critical section}}
   k.unlock();
-  sleep(5); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+  sleep(2); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+            // expected-note@-1 {{Call to blocking function 'sleep' inside of critical section}}
+}
+
+
+void testRecursiveAcquisition() {
+  std::mutex m;
+  m.lock(); // expected-note {{Entering critical section for the 1st time here}}
+  m.lock(); // expected-note {{Entering critical section for the 2nd time here}}
+  sleep(1); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+            // expected-note@-1 {{Call to blocking function 'sleep' inside of critical section}}
+  m.unlock();
+  m.unlock();
+}
+
+void testRecursiveAcquisitionWithMultipleBlockingCalls() {
+  std::mutex m;
+  m.lock(); // expected-note 1{{Entering critical section for the 1st time here}}
+            // expected-note@-1 {{Entering critical section here}}
+  m.lock(); // expected-note 1{{Entering critical section for the 2nd time here}}
+  sleep(1); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+            // expected-note@-1 {{Call to blocking function 'sleep' inside of critical section}}
+  m.unlock();
+  // this next 'sleep' call is only in the critical section of the first lock
+  sleep(2); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+            // expected-note@-1 {{Call to blocking function 'sleep' inside of critical section}}
+  m.unlock();
+}
+
+void testRecursiveAcquisitionWithMultipleMutexes() {
+  std::mutex m, n;
+  m.lock(); // expected-note 1{{Entering critical section here}}
+  n.lock(); // expected-note 2{{Entering critical section here}}
+  sleep(1); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+            // expected-note@-1 {{Call to blocking function 'sleep' inside of critical section}}
+  m.unlock();
+  // this next 'sleep' call is only in the critical section of mutex 'n'
+  sleep(2); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+            // expected-note@-1 {{Call to blocking function 'sleep' inside of critical section}}
+  n.unlock();
+}
+
+
+void testNestedMutexes() {
+  std::mutex m, n, k;
+  m.lock(); // expected-note 3{{Entering critical section here}}
+  n.lock(); // expected-note 2{{Entering critical section here}}
+  k.lock(); // expected-note 1{{Entering critical section here}}
+  sleep(1); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+            // expected-note@-1 {{Call to blocking function 'sleep' inside of critical section}}
+  k.unlock();
+  sleep(2); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+            // expected-note@-1 {{Call to blocking function 'sleep' inside of critical section}}
   n.unlock();
   sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+            // expected-note@-1 {{Call to blocking function 'sleep' inside of critical section}}
+  m.unlock();
+  sleep(4); // no-warning
+}
+
+void testNonOverlappingMutexes() {
+  std::mutex m;
+  m.lock(); // There should be no warning here
+  m.unlock();
+  m.lock(); // expected-note {{Entering critical section here}}
+  sleep(1); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+            // expected-note@-1 {{Call to blocking function 'sleep' inside of critical section}}
+  m.unlock();
+}
+
+void testMixedMutexLocksWithIntermittentUnlock() {
+  std::mutex m, n, k;
+  m.lock(); // expected-note {{Entering critical section here}}
+  n.lock(); // the problem is not is this lock's critical section
+  n.unlock();
+  k.lock(); // same as for n.lock()
+  k.unlock();
+  sleep(1); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+            // expected-note@-1 {{Call to blocking function 'sleep' inside of critical section}}
   m.unlock();
-  sleep(3); // no-warning
 }
 
 void f() {
   sleep(1000); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+               // expected-note@-1 {{Call to blocking function 'sleep' inside of critical section}}
 }
 
 void testBlockInCriticalSectionInterProcedural() {
   std::mutex m;
-  m.lock();
-  f();
+  m.lock(); // expected-note {{Entering critical section here}}
+  f(); // expected-note {{Calling 'f'}}
   m.unlock();
 }
 
+void unknown_function_that_may_lock(std::mutex &);
 void testBlockInCriticalSectionUnexpectedUnlock() {
   std::mutex m;
+  unknown_function_that_may_lock(m);
   m.unlock();
   sleep(1); // no-warning
-  m.lock();
-  sleep(1); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+  m.lock(); // expected-note {{Entering critical section here}}
+  sleep(2); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+            // expected-note@-1 {{Call to blocking function 'sleep' inside of critical section}}
 }
 
 void testBlockInCriticalSectionLockGuard() {
@@ -130,12 +268,13 @@ void testBlockInCriticalSectionLockGuard() {
   std::not_real_lock<std::mutex> not_real_lock(g_mutex);
   sleep(1); // no-warning
 
-  std::lock_guard<std::mutex> lock(g_mutex);
+  std::lock_guard<std::mutex> lock(g_mutex); // expected-note {{Entering critical section here}}
   sleep(1); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+            // expected-note@-1 {{Call to blocking function 'sleep' inside of critical section}}
 }
 
 void testBlockInCriticalSectionLockGuardNested() {
-  testBlockInCriticalSectionLockGuard();
+  testBlockInCriticalSectionLockGuard(); // expected-note {{Calling 'testBlockInCriticalSectionLockGuard'}}
   sleep(1); // no-warning
 }
 
@@ -144,11 +283,12 @@ void testBlockInCriticalSectionUniqueLock() {
   std::not_real_lock<std::mutex> not_real_lock(g_mutex);
   sleep(1); // no-warning
 
-  std::unique_lock<std::mutex> lock(g_mutex);
+  std::unique_lock<std::mutex> lock(g_mutex); // expected-note {{Entering critical section here}}
   sleep(1); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+            // expected-note@-1 {{Call to blocking function 'sleep' inside of critical section}}
 }
 
 void testBlockInCriticalSectionUniqueLockNested() {
-  testBlockInCriticalSectionUniqueLock();
+  testBlockInCriticalSectionUniqueLock(); // expected-note {{Calling 'testBlockInCriticalSectionUniqueLock'}}
   sleep(1); // no-warning
 }