[clang] [analyzer] Fix false positive for mutexes inheriting mutex_base (PR #106240)

via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 27 08:59:06 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Arseniy Zaostrovnykh (necto)

<details>
<summary>Changes</summary>

If a mutex interface is split in inheritance chain, e.g. struct mutex has `unlock` and inherits `lock` from __mutex_base then calls m.lock() and m.unlock() have different "this" targets: m and the __mutex_base of m, which used to confuse the `ActiveCritSections` list.

Taking base region canonicalizes the region used to identify a critical section and enables search in ActiveCritSections list regardless of which class the callee is the member of.

This possibly fixes #<!-- -->104241

CPP-5541

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


2 Files Affected:

- (modified) clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp (+4-2) 
- (modified) clang/test/Analysis/block-in-critical-section-inheritance.cpp (+10) 


``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
index 4cd2f2802f30cd..52ff639c6b8022 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
@@ -245,8 +245,10 @@ static const MemRegion *getRegion(const CallEvent &Call,
                                   const MutexDescriptor &Descriptor,
                                   bool IsLock) {
   return std::visit(
-      [&Call, IsLock](auto &&Descriptor) {
-        return Descriptor.getRegion(Call, IsLock);
+      [&Call, IsLock](auto &Descr) -> const MemRegion * {
+        if (const MemRegion *Reg = Descr.getRegion(Call, IsLock))
+          return Reg->getBaseRegion();
+        return nullptr;
       },
       Descriptor);
 }
diff --git a/clang/test/Analysis/block-in-critical-section-inheritance.cpp b/clang/test/Analysis/block-in-critical-section-inheritance.cpp
index db20df8c60a5c9..c60ba2632cee25 100644
--- a/clang/test/Analysis/block-in-critical-section-inheritance.cpp
+++ b/clang/test/Analysis/block-in-critical-section-inheritance.cpp
@@ -29,3 +29,13 @@ void gh_99628() {
   // expected-note at -2 {{Call to blocking function 'sleep' inside of critical section}}
   m.unlock();
 }
+
+void no_false_positive_gh_104241() {
+  std::mutex m;
+  m.lock();
+  // If inheritance not handled properly, this unlock might not match the lock
+  // above because technically they act on different memory regions:
+  // __mutex_base and mutex.
+  m.unlock();
+  sleep(10); // no-warning
+}

``````````

</details>


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


More information about the cfe-commits mailing list