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

[Endre Fülöp via cfe-commits]

================
@@ -70,73 +202,121 @@ class BlockInCriticalSectionChecker : public Checker<check::PostCall> {
 
 } // end anonymous namespace
 
-REGISTER_TRAIT_WITH_PROGRAMSTATE(MutexCounter, unsigned)
+REGISTER_LIST_WITH_PROGRAMSTATE(ActiveCritSections, CritSectionMarker)
 
-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;
-  }
-}
+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
 
-bool BlockInCriticalSectionChecker::isBlockingFunction(const CallEvent &Call) const {
-  return matchesAny(Call, SleepFn, GetcFn, FgetsFn, ReadFn, RecvFn);
+std::optional<MutexDescriptor>
+BlockInCriticalSectionChecker::checkLock(const CallEvent &Call,
+                                         CheckerContext &C) const {
+  const auto *LockDescriptor =
+      llvm::find_if(MutexDescriptors, [&Call](auto &&LockFn) {
+        return std::visit(
+            [&Call](auto &&Descriptor) { return Descriptor.matchesLock(Call); },
+            LockFn);
+      });
+  if (LockDescriptor != MutexDescriptors.end())
+    return *LockDescriptor;
+  return std::nullopt;
 }
 
-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 auto *MutexRegion = std::visit(
+      [&Call](auto &&Descriptor) { return Descriptor.getLockRegion(Call); },
+      LockDescriptor);
+  if (!MutexRegion)
+    return;
+
+  const auto MarkToAdd = CritSectionMarker{Call.getOriginExpr(), MutexRegion};
+  ProgramStateRef StateWithLockEvent =
+      C.getState()->add<ActiveCritSections>(MarkToAdd);
+  C.addTransition(StateWithLockEvent, createCritSectionNote(MarkToAdd, C));
+}
 
-  return matchesAny(Call, LockFn, PthreadLockFn, PthreadTryLockFn, MtxLock,
-                    MtxTimedLock, MtxTryLock);
+std::optional<MutexDescriptor>
+BlockInCriticalSectionChecker::checkUnlock(const CallEvent &Call,
+                                           CheckerContext &C) const {
+  const auto *UnlockDescriptor =
+      llvm::find_if(MutexDescriptors, [&Call](auto &&UnlockFn) {
+        return std::visit(
+            [&Call](auto &&Descriptor) {
+              return Descriptor.matchesUnlock(Call);
+            },
+            UnlockFn);
+      });
+  if (UnlockDescriptor != MutexDescriptors.end())
+    return *UnlockDescriptor;
+  return std::nullopt;
----------------
gamesh411 wrote:

I think I have fixed most of the code duplication, thanks for the suggestions 👍 

https://github.com/llvm/llvm-project/pull/80029