[clang][analyzer] Add defer_lock_t modelling to BlockInCriticalSectionChecker

[gamesh411]

Fixes #166573

[llvmbot]

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Endre Fülöp (gamesh411)

Changes

Fixes #166573

---

Full diff: https://github.com/llvm/llvm-project/pull/168338.diff

2 Files Affected:

• (modified) clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp (+19-1)
• (modified) clang/test/Analysis/block-in-critical-section.cpp (+9)

diff --git a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
index 68bee710e5ce5..99137e038811d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
@@ -115,7 +115,25 @@ class RAIIMutexDescriptor {
       return false;
     const IdentifierInfo *II =
         cast<CXXRecordDecl>(C->getDecl()->getParent())->getIdentifier();
-    return II == Guard;
+    if (II != Guard)
+      return false;
+
+    // For unique_lock, check if it's constructed with a ctor that takes the tag
+    // type defer_lock_t. In this case, the lock is not acquired.
+    if constexpr (std::is_same_v<T, CXXConstructorCall>) {
+      if (GuardName == "unique_lock" && C->getNumArgs() >= 2) {
+        const Expr *SecondArg = C->getArgExpr(1);
+        QualType ArgType = SecondArg->getType().getNonReferenceType();
+        QualType UnqualifiedType = ArgType.getUnqualifiedType();
+        if (const auto *RD = UnqualifiedType->getAsRecordDecl();
+            RD && RD->getName() == "defer_lock_t" &&
+            RD->getDeclContext()->isStdNamespace()) {
+          return false;
+        }
+      }
+    }
+
+    return true;
   }
 
 public:
diff --git a/clang/test/Analysis/block-in-critical-section.cpp b/clang/test/Analysis/block-in-critical-section.cpp
index ee9a708f231a8..674a09265faa5 100644
--- a/clang/test/Analysis/block-in-critical-section.cpp
+++ b/clang/test/Analysis/block-in-critical-section.cpp
@@ -16,9 +16,12 @@ struct lock_guard {
   lock_guard<T>(std::mutex) {}
   ~lock_guard<T>() {}
 };
+struct defer_lock_t {};
+constexpr defer_lock_t defer_lock{};
 template<typename T>
 struct unique_lock {
   unique_lock<T>(std::mutex) {}
+  unique_lock<T>(std::mutex, defer_lock_t) {} // defer_lock parameter
   ~unique_lock<T>() {}
 };
 template<typename T>
@@ -309,3 +312,9 @@ void testTrylockCurrentlyFalsePositive(pthread_mutex_t *m) {
                // FIXME: this is a false positive, the lock was not acquired
   }
 }
+
+void testBlockInCriticalSectionUniqueLockWithDeferLock() {
+  std::mutex g_mutex;
+  std::unique_lock<std::mutex> lock(g_mutex, std::defer_lock);
+  sleep(1); // no-warning
+}

[llvmbot]

@llvm/pr-subscribers-clang

Author: Endre Fülöp (gamesh411)

Changes

Fixes #166573

---

Full diff: https://github.com/llvm/llvm-project/pull/168338.diff

2 Files Affected:

• (modified) clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp (+19-1)
• (modified) clang/test/Analysis/block-in-critical-section.cpp (+9)

diff --git a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
index 68bee710e5ce5..99137e038811d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
@@ -115,7 +115,25 @@ class RAIIMutexDescriptor {
       return false;
     const IdentifierInfo *II =
         cast<CXXRecordDecl>(C->getDecl()->getParent())->getIdentifier();
-    return II == Guard;
+    if (II != Guard)
+      return false;
+
+    // For unique_lock, check if it's constructed with a ctor that takes the tag
+    // type defer_lock_t. In this case, the lock is not acquired.
+    if constexpr (std::is_same_v<T, CXXConstructorCall>) {
+      if (GuardName == "unique_lock" && C->getNumArgs() >= 2) {
+        const Expr *SecondArg = C->getArgExpr(1);
+        QualType ArgType = SecondArg->getType().getNonReferenceType();
+        QualType UnqualifiedType = ArgType.getUnqualifiedType();
+        if (const auto *RD = UnqualifiedType->getAsRecordDecl();
+            RD && RD->getName() == "defer_lock_t" &&
+            RD->getDeclContext()->isStdNamespace()) {
+          return false;
+        }
+      }
+    }
+
+    return true;
   }
 
 public:
diff --git a/clang/test/Analysis/block-in-critical-section.cpp b/clang/test/Analysis/block-in-critical-section.cpp
index ee9a708f231a8..674a09265faa5 100644
--- a/clang/test/Analysis/block-in-critical-section.cpp
+++ b/clang/test/Analysis/block-in-critical-section.cpp
@@ -16,9 +16,12 @@ struct lock_guard {
   lock_guard<T>(std::mutex) {}
   ~lock_guard<T>() {}
 };
+struct defer_lock_t {};
+constexpr defer_lock_t defer_lock{};
 template<typename T>
 struct unique_lock {
   unique_lock<T>(std::mutex) {}
+  unique_lock<T>(std::mutex, defer_lock_t) {} // defer_lock parameter
   ~unique_lock<T>() {}
 };
 template<typename T>
@@ -309,3 +312,9 @@ void testTrylockCurrentlyFalsePositive(pthread_mutex_t *m) {
                // FIXME: this is a false positive, the lock was not acquired
   }
 }
+
+void testBlockInCriticalSectionUniqueLockWithDeferLock() {
+  std::mutex g_mutex;
+  std::unique_lock<std::mutex> lock(g_mutex, std::defer_lock);
+  sleep(1); // no-warning
+}

[NagyDonat]

LGTM with a few minor suggestions.

[steakhal]

Thanks for the review. LGTM for me, the raised suggestions look good to me.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[gamesh411]

@NagyDonat I may have overcomplicated that part. Thanks for the suggestions. I have applied them.

[github-actions]

🐧 Linux x64 Test Results

• 111330 tests passed
• 4421 tests skipped