[libcxx-commits] [clang] [libcxx] [clang][ThreadSafety] Handle mutex scope (PR #157171)
Prabhu Rajasekaran via libcxx-commits
libcxx-commits at lists.llvm.org
Fri Sep 5 16:57:49 PDT 2025
https://github.com/Prabhuk updated https://github.com/llvm/llvm-project/pull/157171
>From e6af544156940a1041a01a44d73dae76ac322111 Mon Sep 17 00:00:00 2001
From: prabhukr <prabhukr at google.com>
Date: Fri, 5 Sep 2025 13:35:10 -0700
Subject: [PATCH 1/7] [clang][ThreadSafety] Handle mutex scope
Before emitting warning about locks being held at the end of function
scope check if the underlying mutex is function scoped or not.
---
clang/lib/Analysis/ThreadSafety.cpp | 16 ++++++++++++++++
.../clang/thread/thread.mutex/lock.verify.cpp | 6 ++++++
2 files changed, 22 insertions(+)
diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index 131170df9976e..b215a6e6d74cc 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -2443,6 +2443,22 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet &EntrySet,
if (join(FactMan[*EntryIt], ExitFact, JoinLoc, EntryLEK))
*EntryIt = Fact;
} else if (!ExitFact.managed() || EntryLEK == LEK_LockedAtEndOfFunction) {
+ if (EntryLEK == LEK_LockedAtEndOfFunction) {
+ const til::SExpr *Sexp = ExitFact.sexpr();
+ const VarDecl *Var = nullptr;
+
+ if (const auto *Proj = dyn_cast<til::Project>(Sexp)) {
+ if (const auto *Base = dyn_cast<til::LiteralPtr>(Proj->record()))
+ Var = dyn_cast_or_null<VarDecl>(Base->clangDecl());
+ } else if (const auto *LP = dyn_cast<til::LiteralPtr>(Sexp)) {
+ Var = dyn_cast_or_null<VarDecl>(LP->clangDecl());
+ }
+
+ if (Var && Var->getStorageDuration() == SD_Automatic &&
+ Var->getDeclContext() == CurrentFunction) {
+ continue;
+ }
+ }
ExitFact.handleRemovalFromIntersection(ExitSet, FactMan, JoinLoc,
EntryLEK, Handler);
}
diff --git a/libcxx/test/extensions/clang/thread/thread.mutex/lock.verify.cpp b/libcxx/test/extensions/clang/thread/thread.mutex/lock.verify.cpp
index 51ffa6962ac83..e0d4ec39ea845 100644
--- a/libcxx/test/extensions/clang/thread/thread.mutex/lock.verify.cpp
+++ b/libcxx/test/extensions/clang/thread/thread.mutex/lock.verify.cpp
@@ -45,3 +45,9 @@ void f3() {
expected-warning {{mutex 'm2' is still held at the end of function}} \
expected-warning {{mutex 'm3' is still held at the end of function}}
#endif
+
+void f4() {
+ std::mutex local_m0;
+ std::mutex local_m1;
+ std::lock(local_m0, local_m1);
+}
>From e9e92e57859901a8247fad62656d1fd68cd6fa53 Mon Sep 17 00:00:00 2001
From: prabhukr <prabhukr at google.com>
Date: Fri, 5 Sep 2025 13:48:07 -0700
Subject: [PATCH 2/7] Use better variable name.
---
clang/lib/Analysis/ThreadSafety.cpp | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index b215a6e6d74cc..56055f1ee6e3e 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -2445,17 +2445,17 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet &EntrySet,
} else if (!ExitFact.managed() || EntryLEK == LEK_LockedAtEndOfFunction) {
if (EntryLEK == LEK_LockedAtEndOfFunction) {
const til::SExpr *Sexp = ExitFact.sexpr();
- const VarDecl *Var = nullptr;
+ const VarDecl *MutexVar = nullptr;
if (const auto *Proj = dyn_cast<til::Project>(Sexp)) {
if (const auto *Base = dyn_cast<til::LiteralPtr>(Proj->record()))
- Var = dyn_cast_or_null<VarDecl>(Base->clangDecl());
+ MutexVar = dyn_cast_or_null<VarDecl>(Base->clangDecl());
} else if (const auto *LP = dyn_cast<til::LiteralPtr>(Sexp)) {
- Var = dyn_cast_or_null<VarDecl>(LP->clangDecl());
+ MutexVar = dyn_cast_or_null<VarDecl>(LP->clangDecl());
}
- if (Var && Var->getStorageDuration() == SD_Automatic &&
- Var->getDeclContext() == CurrentFunction) {
+ if (MutexVar && MutexVar->getStorageDuration() == SD_Automatic &&
+ MutexVar->getDeclContext() == CurrentFunction) {
continue;
}
}
>From a7e28b9317a8539d31e251fcef76c5d97d36a29d Mon Sep 17 00:00:00 2001
From: prabhukr <prabhukr at google.com>
Date: Fri, 5 Sep 2025 14:06:36 -0700
Subject: [PATCH 3/7] Add regression tests.
---
clang/test/PCH/thread-safety-attrs.cpp | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/clang/test/PCH/thread-safety-attrs.cpp b/clang/test/PCH/thread-safety-attrs.cpp
index d33917e518597..67fb87ea16d54 100644
--- a/clang/test/PCH/thread-safety-attrs.cpp
+++ b/clang/test/PCH/thread-safety-attrs.cpp
@@ -316,4 +316,21 @@ void sls_fun_bad_12() {
expected-warning{{releasing mutex 'sls_mu' that was not held}}
}
+
+template <class T, class... Ts>
+void LockMutexes(T &m, Ts &...ms) __attribute__((exclusive_lock_function(m, ms...)));
+
+Mutex m0, m1;
+void non_local_mutex_held() {
+ LockMutexes(m0, m1); // expected-note {{mutex acquired here}} \
+ // expected-note {{mutex acquired here}}
+} // expected-warning{{mutex 'm0' is still held at the end of function}} \
+// expected-warning{{mutex 'm1' is still held at the end of function}}
+
+void no_local_mutex_held_warning() {
+ Mutex local_m0;
+ Mutex local_m1;
+ LockMutexes(local_m0, local_m1);
+} // No warnings expected at end of function scope as the mutexes are function local.
+
#endif
>From e4e193fc114baa8bc3966378974047626d4cca32 Mon Sep 17 00:00:00 2001
From: prabhukr <prabhukr at google.com>
Date: Fri, 5 Sep 2025 14:26:15 -0700
Subject: [PATCH 4/7] Remove libcxx test from this change.
---
.../extensions/clang/thread/thread.mutex/lock.verify.cpp | 6 ------
1 file changed, 6 deletions(-)
diff --git a/libcxx/test/extensions/clang/thread/thread.mutex/lock.verify.cpp b/libcxx/test/extensions/clang/thread/thread.mutex/lock.verify.cpp
index e0d4ec39ea845..51ffa6962ac83 100644
--- a/libcxx/test/extensions/clang/thread/thread.mutex/lock.verify.cpp
+++ b/libcxx/test/extensions/clang/thread/thread.mutex/lock.verify.cpp
@@ -45,9 +45,3 @@ void f3() {
expected-warning {{mutex 'm2' is still held at the end of function}} \
expected-warning {{mutex 'm3' is still held at the end of function}}
#endif
-
-void f4() {
- std::mutex local_m0;
- std::mutex local_m1;
- std::lock(local_m0, local_m1);
-}
>From 78c98c52f6b80915ffe54875784a07b550cfd1fb Mon Sep 17 00:00:00 2001
From: prabhukr <prabhukr at google.com>
Date: Fri, 5 Sep 2025 15:32:57 -0700
Subject: [PATCH 5/7] Add mock unique_lock and unique_lock regression tests.
---
clang/test/PCH/thread-safety-attrs.cpp | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/clang/test/PCH/thread-safety-attrs.cpp b/clang/test/PCH/thread-safety-attrs.cpp
index 67fb87ea16d54..09a351b346f2d 100644
--- a/clang/test/PCH/thread-safety-attrs.cpp
+++ b/clang/test/PCH/thread-safety-attrs.cpp
@@ -60,6 +60,20 @@ class SCOPED_LOCKABLE ReleasableMutexLock {
void Release() UNLOCK_FUNCTION();
};
+template<typename T>
+struct lock_guard {
+ lock_guard<T>(T) {}
+ ~lock_guard<T>() {}
+};
+template<typename T>
+struct unique_lock {
+ unique_lock<T>(T) {}
+ ~unique_lock<T>() {}
+};
+
+template <class T, class... Ts>
+void LockMutexes(T &m, Ts &...ms) __attribute__((exclusive_lock_function(m, ms...)));
+
// The universal lock, written "*", allows checking to be selectively turned
// off for a particular piece of code.
@@ -316,10 +330,6 @@ void sls_fun_bad_12() {
expected-warning{{releasing mutex 'sls_mu' that was not held}}
}
-
-template <class T, class... Ts>
-void LockMutexes(T &m, Ts &...ms) __attribute__((exclusive_lock_function(m, ms...)));
-
Mutex m0, m1;
void non_local_mutex_held() {
LockMutexes(m0, m1); // expected-note {{mutex acquired here}} \
@@ -333,4 +343,10 @@ void no_local_mutex_held_warning() {
LockMutexes(local_m0, local_m1);
} // No warnings expected at end of function scope as the mutexes are function local.
+void no_local_unique_locks_held_warning() {
+ unique_lock<Mutex> ul0(m0);
+ unique_lock<Mutex> ul1(m1);
+ LockMutexes(ul0, ul1);
+} // No warnings expected at end of function scope as the unique_locks held are function local.
+
#endif
>From bd4eb5058bf00f173ef5d818c21f001b189d06bb Mon Sep 17 00:00:00 2001
From: prabhukr <prabhukr at google.com>
Date: Fri, 5 Sep 2025 23:55:47 +0000
Subject: [PATCH 6/7] Extract regression test into a separate file
---
.../Analysis/thread-safety-lock-scope.cpp | 45 +++++++++++++++++++
clang/test/PCH/thread-safety-attrs.cpp | 33 --------------
2 files changed, 45 insertions(+), 33 deletions(-)
create mode 100644 clang/test/Analysis/thread-safety-lock-scope.cpp
diff --git a/clang/test/Analysis/thread-safety-lock-scope.cpp b/clang/test/Analysis/thread-safety-lock-scope.cpp
new file mode 100644
index 0000000000000..4ca87625c3a3a
--- /dev/null
+++ b/clang/test/Analysis/thread-safety-lock-scope.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -Wthread-safety %s
+
+#ifndef HEADER
+#define HEADER
+
+#define LOCKABLE __attribute__ ((lockable))
+#define EXCLUSIVE_LOCK_FUNCTION(...) __attribute__ ((exclusive_lock_function(__VA_ARGS__)))
+
+class LOCKABLE Mutex{};
+
+template<typename T>
+struct lock_guard {
+ lock_guard<T>(T) {}
+ ~lock_guard<T>() {}
+};
+template<typename T>
+struct unique_lock {
+ unique_lock<T>(T) {}
+ ~unique_lock<T>() {}
+};
+
+template <class T, class... Ts>
+void LockMutexes(T &m, Ts &...ms) EXCLUSIVE_LOCK_FUNCTION(m, ms...);
+
+#else
+
+Mutex m0, m1;
+void non_local_mutex_held() {
+ LockMutexes(m0, m1); // expected-note {{mutex acquired here}} \
+ // expected-note {{mutex acquired here}}
+} // expected-warning{{mutex 'm0' is still held at the end of function}} \
+// expected-warning{{mutex 'm1' is still held at the end of function}}
+
+void no_local_mutex_held_warning() {
+ Mutex local_m0;
+ Mutex local_m1;
+ LockMutexes(local_m0, local_m1);
+} // No warnings expected at end of function scope as the mutexes are function local.
+
+void no_local_unique_locks_held_warning() {
+ unique_lock<Mutex> ul0(m0);
+ unique_lock<Mutex> ul1(m1);
+ LockMutexes(ul0, ul1);
+} // No warnings expected at end of function scope as the unique_locks held are function local.
+#endif
diff --git a/clang/test/PCH/thread-safety-attrs.cpp b/clang/test/PCH/thread-safety-attrs.cpp
index 09a351b346f2d..d33917e518597 100644
--- a/clang/test/PCH/thread-safety-attrs.cpp
+++ b/clang/test/PCH/thread-safety-attrs.cpp
@@ -60,20 +60,6 @@ class SCOPED_LOCKABLE ReleasableMutexLock {
void Release() UNLOCK_FUNCTION();
};
-template<typename T>
-struct lock_guard {
- lock_guard<T>(T) {}
- ~lock_guard<T>() {}
-};
-template<typename T>
-struct unique_lock {
- unique_lock<T>(T) {}
- ~unique_lock<T>() {}
-};
-
-template <class T, class... Ts>
-void LockMutexes(T &m, Ts &...ms) __attribute__((exclusive_lock_function(m, ms...)));
-
// The universal lock, written "*", allows checking to be selectively turned
// off for a particular piece of code.
@@ -330,23 +316,4 @@ void sls_fun_bad_12() {
expected-warning{{releasing mutex 'sls_mu' that was not held}}
}
-Mutex m0, m1;
-void non_local_mutex_held() {
- LockMutexes(m0, m1); // expected-note {{mutex acquired here}} \
- // expected-note {{mutex acquired here}}
-} // expected-warning{{mutex 'm0' is still held at the end of function}} \
-// expected-warning{{mutex 'm1' is still held at the end of function}}
-
-void no_local_mutex_held_warning() {
- Mutex local_m0;
- Mutex local_m1;
- LockMutexes(local_m0, local_m1);
-} // No warnings expected at end of function scope as the mutexes are function local.
-
-void no_local_unique_locks_held_warning() {
- unique_lock<Mutex> ul0(m0);
- unique_lock<Mutex> ul1(m1);
- LockMutexes(ul0, ul1);
-} // No warnings expected at end of function scope as the unique_locks held are function local.
-
#endif
>From 0939f0ce6571d511a6d81698e2c32e69497222f1 Mon Sep 17 00:00:00 2001
From: prabhukr <prabhukr at google.com>
Date: Fri, 5 Sep 2025 23:57:34 +0000
Subject: [PATCH 7/7] Fix missing include flag
---
clang/test/Analysis/thread-safety-lock-scope.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/test/Analysis/thread-safety-lock-scope.cpp b/clang/test/Analysis/thread-safety-lock-scope.cpp
index 4ca87625c3a3a..15a547e345275 100644
--- a/clang/test/Analysis/thread-safety-lock-scope.cpp
+++ b/clang/test/Analysis/thread-safety-lock-scope.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify -fsyntax-only -Wthread-safety %s
+// RUN: %clang_cc1 -include %s -verify -fsyntax-only -Wthread-safety %s
#ifndef HEADER
#define HEADER
More information about the libcxx-commits
mailing list