https://github.com/aaronpuchert created
https://github.com/llvm/llvm-project/pull/141432
The analysis already excludes functions with a zero-argument acquire or release
attribute. According to the requirements enforced by
-Wthread-safety-attributes, these are methods of a capability class where the
attribute implicitly refers to `this`.
C doesn't have class methods, so the lock/unlock implementations are free
functions. If we disable the analysis for all free functions with attributes,
this would obviously exclude too much. But maybe we can exclude functions where
the attribute directly refers to a parameter. Mutex implementations should
typically do that, and I don't see why other functions should. (Typically,
other functions acquire or release a global mutex or a member of a parameter.)
Fixes #139933.
>From 7df9ba1dca287c8ce1025160d06a04835b43f254 Mon Sep 17 00:00:00 2001
From: Aaron Puchert <aaronpuch...@alice-dsl.net>
Date: Sun, 25 May 2025 23:51:23 +0200
Subject: [PATCH] Thread safety analysis: Skip functions acquiring/releasing
parameters
The analysis already excludes functions with a zero-argument acquire or
release attribute. According to the requirements enforced by
-Wthread-safety-attributes, these are methods of a capability class
where the attribute implicitly refers to `this`.
C doesn't have class methods, so the lock/unlock implementations are
free functions. If we disable the analysis for all free functions with
attributes, this would obviously exclude too much. But maybe we can
exclude functions where the attribute directly refers to a parameter.
Mutex implementations should typically do that, and I don't see why
other functions should. (Typically, other functions acquire or release a
global mutex or a member of a parameter.)
Fixes #139933.
---
clang/lib/Analysis/ThreadSafety.cpp | 18 +++++++++--
.../SemaCXX/warn-thread-safety-analysis.cpp | 30 +++++++++++++++++++
2 files changed, 46 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index 528fd09958578..9f375d700b641 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -2344,6 +2344,20 @@ static bool neverReturns(const CFGBlock *B) {
return false;
}
+static bool attrArgsForImpl(llvm::iterator_range<clang::Expr **> Args) {
+ // An attribute with no arguments implicitly refers to 'this'.
+ if (Args.empty())
+ return true;
+
+ return llvm::all_of(Args, [](const Expr *E) {
+ if (isa<CXXThisExpr>(E))
+ return true;
+ if (const auto* DRE = dyn_cast<DeclRefExpr>(E))
+ return isa<ParmVarDecl>(DRE->getDecl());
+ return false;
+ });
+}
+
/// Check a function's CFG for thread-safety violations.
///
/// We traverse the blocks in the CFG, compute the set of mutexes that are held
@@ -2421,13 +2435,13 @@ void
ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
} else if (const auto *A = dyn_cast<ReleaseCapabilityAttr>(Attr)) {
// UNLOCK_FUNCTION() is used to hide the underlying lock
implementation.
// We must ignore such methods.
- if (A->args_size() == 0)
+ if (attrArgsForImpl(A->args()))
return;
getMutexIDs(A->isShared() ? SharedLocksToAdd : ExclusiveLocksToAdd, A,
nullptr, D);
getMutexIDs(LocksReleased, A, nullptr, D);
} else if (const auto *A = dyn_cast<AcquireCapabilityAttr>(Attr)) {
- if (A->args_size() == 0)
+ if (attrArgsForImpl(A->args()))
return;
getMutexIDs(A->isShared() ? SharedLocksAcquired
: ExclusiveLocksAcquired,
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index 5382023318f28..4b12d68d41ed0 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -2401,6 +2401,36 @@ class LOCKABLE MyLock2 {
} // end namespace SelfLockingTest
+namespace ExcludeImplementation {
+
+class LOCKABLE Mutex2 {
+public:
+ // We don't require EXCLUSIVE_(UN)LOCK_FUNCTION(impl) on these methods.
+ void Lock() EXCLUSIVE_LOCK_FUNCTION() { impl.Lock(); }
+ void Unlock() EXCLUSIVE_UNLOCK_FUNCTION() { impl.Unlock(); }
+
+ void LockAlt() EXCLUSIVE_LOCK_FUNCTION(this) { impl.Lock(); }
+ void UnlockAlt() EXCLUSIVE_UNLOCK_FUNCTION(this) { impl.Unlock(); }
+
+private:
+ Mutex impl;
+};
+
+struct LOCKABLE Mutex3 {
+ Mutex impl;
+};
+
+void Lock(Mutex3* mu) EXCLUSIVE_LOCK_FUNCTION(mu) {
+ mu->impl.Lock();
+}
+
+void Unlock(Mutex3* mu) EXCLUSIVE_UNLOCK_FUNCTION(mu) {
+ mu->impl.Unlock();
+}
+
+} // namespace ExcludeImplementation
+
+
namespace InvalidNonstatic {
// Forward decl here causes bogus "invalid use of non-static data member"
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
