[clang] false-posiitve thread-safety (PR #170843)

[Utkarsh Saxena via cfe-commits]

https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/170843

>From cf5322832037f0fdc98b0ce802708b811babe707 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Fri, 5 Dec 2025 11:58:46 +0000
Subject: [PATCH 1/2] false-posiitve thread-safety

---
 clang/lib/Analysis/ThreadSafety.cpp              |  7 +++++++
 .../test/SemaCXX/warn-thread-safety-analysis.cpp | 16 ++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index a25bd6007d5ed..417a951d9c99f 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -43,6 +43,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Allocator.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/TrailingObjects.h"
 #include "llvm/Support/raw_ostream.h"
@@ -2820,6 +2821,12 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
         case CFGElement::AutomaticObjectDtor: {
           CFGAutomaticObjDtor AD = BI.castAs<CFGAutomaticObjDtor>();
           const auto *DD = AD.getDestructorDecl(AC.getASTContext());
+          // Ignore dtor for parameters as the corresponding constructor does
+          // not happen in the callee context but in the caller context. It is
+          // not possible to know which constuctor was used for its construction
+          // so avoid reading the destructor as well.
+          if (isa_and_nonnull<ParmVarDecl>(AD.getVarDecl()))
+            break;
           if (!DD || !DD->hasAttrs())
             break;
 
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index 0e91639a271c5..d03493cceeb85 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -7622,3 +7622,19 @@ void testLoopConditionalReassignment(Foo *f1, Foo *f2, bool cond) {
   ptr->mu.Unlock(); // expected-warning{{releasing mutex 'ptr->mu' that was not held}}
 } // expected-warning{{mutex 'f1->mu' is still held at the end of function}}
 }  // namespace CapabilityAliases
+
+namespace test_function_param_lock_unlock {
+class A {
+ public:
+  A() EXCLUSIVE_LOCK_FUNCTION(mu_) {
+    mu_.Lock();
+  }
+  ~A() UNLOCK_FUNCTION(mu_) { mu_.Unlock(); }
+ private:
+  Mutex mu_;
+};
+
+int do_something(A a) {
+  return 0;
+}
+} // namespace test_function_param_lock_unlock

>From ac7fddb93be1249f1b91b5d5bcb9fa64ae02f02c Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Fri, 5 Dec 2025 12:11:06 +0000
Subject: [PATCH 2/2] Add more tests

---
 clang/lib/Analysis/ThreadSafety.cpp           |  5 +----
 .../SemaCXX/warn-thread-safety-analysis.cpp   | 19 +++++++++++++------
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index 417a951d9c99f..12bcf2a78821e 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -2821,10 +2821,7 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
         case CFGElement::AutomaticObjectDtor: {
           CFGAutomaticObjDtor AD = BI.castAs<CFGAutomaticObjDtor>();
           const auto *DD = AD.getDestructorDecl(AC.getASTContext());
-          // Ignore dtor for parameters as the corresponding constructor does
-          // not happen in the callee context but in the caller context. It is
-          // not possible to know which constuctor was used for its construction
-          // so avoid reading the destructor as well.
+          // Ignore parameter dtors: their ctors happen in caller context.
           if (isa_and_nonnull<ParmVarDecl>(AD.getVarDecl()))
             break;
           if (!DD || !DD->hasAttrs())
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index d03493cceeb85..203ea424020b6 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -7626,15 +7626,22 @@ void testLoopConditionalReassignment(Foo *f1, Foo *f2, bool cond) {
 namespace test_function_param_lock_unlock {
 class A {
  public:
-  A() EXCLUSIVE_LOCK_FUNCTION(mu_) {
-    mu_.Lock();
-  }
+  A() EXCLUSIVE_LOCK_FUNCTION(mu_) { mu_.Lock(); }
   ~A() UNLOCK_FUNCTION(mu_) { mu_.Unlock(); }
  private:
   Mutex mu_;
 };
+int do_something(A a) { return 0; }
 
-int do_something(A a) {
-  return 0;
-}
+// Unlock in dtor without lock in ctor.
+// FIXME: We cannot detect that we are releasing a lock that was never held!
+class B {
+ public:
+  B() {}
+  B(int) {}
+  ~B() UNLOCK_FUNCTION(mu_) { mu_.Unlock(); }
+ private:
+  Mutex mu_;
+};
+int do_something(B b) { return 0; }
 } // namespace test_function_param_lock_unlock