https://github.com/Prabhuk updated https://github.com/llvm/llvm-project/pull/157171
>From e6af544156940a1041a01a44d73dae76ac322111 Mon Sep 17 00:00:00 2001 From: prabhukr <prabh...@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 <prabh...@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 <prabh...@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 <prabh...@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 <prabh...@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 <prabh...@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 <prabh...@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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits