[clang] Thread Safety Analysis: Support passing scoped locks between functions with appropriate annotations (PR #110523)
Malek Ben Slimane via cfe-commits
cfe-commits at lists.llvm.org
Fri Nov 29 07:30:31 PST 2024
https://github.com/malek203 updated https://github.com/llvm/llvm-project/pull/110523
>From 70b03bd0532f55f25f51adda0604b10d4b2150e0 Mon Sep 17 00:00:00 2001
From: Malek Ben Slimane <malek.ben.slimane at sap.com>
Date: Wed, 11 Sep 2024 22:07:45 +0200
Subject: [PATCH] Thread Safety Analysis: Support passing scoped locks between
functions with appropriate annotations
This is helpful when multiple functions operate on the same
capabilities, but we still want to use scoped lockable types for
readability and exception safety.
- Introduce support for thread safety annotations on function parameters
marked with the 'scoped_lockable' attribute.
- Add semantic checks for annotated function parameters, ensuring
correct usage.
- Enhance the analysis to recognize and handle parameters annotated for
thread safety, extending the scope of analysis to track these across
function boundries.
- Verify that the underlying mutexes of function arguments match the
expectations set by the annotations.
Limitation: This does not work when the attribute arguments are class
members, because attributes on function parameters are parsed
differently from attributes on functions.
---
clang/docs/ReleaseNotes.rst | 34 ++
clang/docs/ThreadSafetyAnalysis.rst | 51 ++-
.../clang/Analysis/Analyses/ThreadSafety.h | 36 ++
clang/include/clang/Basic/Attr.td | 8 +-
.../clang/Basic/DiagnosticSemaKinds.td | 15 +
clang/lib/Analysis/ThreadSafety.cpp | 175 +++++++-
clang/lib/Sema/AnalysisBasedWarnings.cpp | 40 ++
clang/lib/Sema/SemaDeclAttr.cpp | 41 ++
.../SemaCXX/warn-thread-safety-analysis.cpp | 372 ++++++++++++++++++
.../SemaCXX/warn-thread-safety-parsing.cpp | 42 +-
10 files changed, 784 insertions(+), 30 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index e511614fcf2451..dbd98edd731c98 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -336,6 +336,40 @@ Improvements to Clang's diagnostics
local variables passed to function calls using the ``[[clang::musttail]]``
attribute.
+- The Thread Safety Analysis (#GH110523) now supports passing scoped capabilities into functions:
+ an attribute on the scoped capability parameter indicates both the expected associated capabilities and,
+ like in the case of attributes on the function declaration itself, their state before and after the call.
+
+ .. code-block:: c++
+
+ #include "mutex.h"
+
+ class MutexUnlocker {
+ Mutex* mu;
+
+ public:
+ MutexUnlocker(Mutex* m) RELEASE(m) : mu(m) { mu->Unlock(); }
+ ~MutexUnlocker() ACQUIRE(mu) { mu->Lock(); }
+ };
+
+ Mutex mu1, mu2;
+ int a GUARDED_BY(mu1);
+
+ void require(MutexLocker& scope REQUIRES(mu1)) {
+ scope.Unlock();
+ a = 0; // Warning! Requires mu1.
+ scope.Lock();
+ }
+
+ void testParameter() {
+ MutexLocker scope(&mu1);
+ MutexLocker scope2(&mu2);
+ require(scope2); // Warning! Mutex managed by 'scope' is 'mu2' instead of 'mu1'
+ require(scope); // OK.
+ scope.Unlock();
+ require(scope); // Warning! Requires mu1.
+ }
+
Improvements to Clang's time-trace
----------------------------------
diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst
index cc4089b97b4923..a312e8e1f5924b 100644
--- a/clang/docs/ThreadSafetyAnalysis.rst
+++ b/clang/docs/ThreadSafetyAnalysis.rst
@@ -187,10 +187,13 @@ REQUIRES(...), REQUIRES_SHARED(...)
*Previously*: ``EXCLUSIVE_LOCKS_REQUIRED``, ``SHARED_LOCKS_REQUIRED``
-``REQUIRES`` is an attribute on functions or methods, which
+``REQUIRES`` is an attribute on functions, methods or function parameters of
+reference to :ref:`scoped_capability`-annotated type, which
declares that the calling thread must have exclusive access to the given
capabilities. More than one capability may be specified. The capabilities
must be held on entry to the function, *and must still be held on exit*.
+Additionally, if the attribute is on a function parameter, it declares that
+the scoped capability manages the specified capabilities in the given order.
``REQUIRES_SHARED`` is similar, but requires only shared access.
@@ -211,6 +214,21 @@ must be held on entry to the function, *and must still be held on exit*.
mu1.Unlock();
}
+ void require(MutexLocker& scope REQUIRES(mu1)) {
+ scope.Unlock();
+ a = 0; // Warning! Requires mu1.
+ scope.Lock();
+ }
+
+ void testParameter() {
+ MutexLocker scope(&mu1);
+ MutexLocker scope2(&mu2);
+ require(scope2); // Warning! Mutex managed by 'scope' is 'mu2' instead of 'mu1'
+ require(scope); // OK.
+ scope.Unlock();
+ require(scope); // Warning! Requires mu1.
+ }
+
ACQUIRE(...), ACQUIRE_SHARED(...), RELEASE(...), RELEASE_SHARED(...), RELEASE_GENERIC(...)
------------------------------------------------------------------------------------------
@@ -218,10 +236,13 @@ ACQUIRE(...), ACQUIRE_SHARED(...), RELEASE(...), RELEASE_SHARED(...), RELEASE_GE
*Previously*: ``EXCLUSIVE_LOCK_FUNCTION``, ``SHARED_LOCK_FUNCTION``,
``UNLOCK_FUNCTION``
-``ACQUIRE`` and ``ACQUIRE_SHARED`` are attributes on functions or methods
-declaring that the function acquires a capability, but does not release it.
+``ACQUIRE`` and ``ACQUIRE_SHARED`` are attributes on functions, methods
+or function parameters of reference to :ref:`scoped_capability`-annotated type,
+which declare that the function acquires a capability, but does not release it.
The given capability must not be held on entry, and will be held on exit
(exclusively for ``ACQUIRE``, shared for ``ACQUIRE_SHARED``).
+Additionally, if the attribute is on a function parameter, it declares that
+the scoped capability manages the specified capabilities in the given order.
``RELEASE``, ``RELEASE_SHARED``, and ``RELEASE_GENERIC`` declare that the
function releases the given capability. The capability must be held on entry
@@ -249,6 +270,14 @@ shared for ``RELEASE_GENERIC``), and will no longer be held on exit.
myObject.doSomething(); // Warning, mu is not locked.
}
+ void release(MutexLocker& scope RELEASE(mu)) {
+ } // Warning! Need to unlock mu.
+
+ void testParameter() {
+ MutexLocker scope(&mu);
+ release(scope);
+ }
+
If no argument is passed to ``ACQUIRE`` or ``RELEASE``, then the argument is
assumed to be ``this``, and the analysis will not check the body of the
function. This pattern is intended for use by classes which hide locking
@@ -283,10 +312,13 @@ EXCLUDES(...)
*Previously*: ``LOCKS_EXCLUDED``
-``EXCLUDES`` is an attribute on functions or methods, which declares that
+``EXCLUDES`` is an attribute on functions, methods or function parameters
+of reference to :ref:`scoped_capability`-annotated type, which declares that
the caller must *not* hold the given capabilities. This annotation is
used to prevent deadlock. Many mutex implementations are not re-entrant, so
deadlock can occur if the function acquires the mutex a second time.
+Additionally, if the attribute is on a function parameter, it declares that
+the scoped capability manages the specified capabilities in the given order.
.. code-block:: c++
@@ -305,6 +337,16 @@ deadlock can occur if the function acquires the mutex a second time.
mu.Unlock();
}
+ void exclude(MutexLocker& scope LOCKS_EXCLUDED(mu)) {
+ scope.Unlock(); // Warning! mu is not locked.
+ scope.Lock();
+ } // Warning! mu still held at the end of function.
+
+ void testParameter() {
+ MutexLocker scope(&mu);
+ exclude(scope); // Warning, mu is held.
+ }
+
Unlike ``REQUIRES``, ``EXCLUDES`` is optional. The analysis will not issue a
warning if the attribute is missing, which can lead to false negatives in some
cases. This issue is discussed further in :ref:`negative`.
@@ -393,6 +435,7 @@ class can be used as a capability. The string argument specifies the kind of
capability in error messages, e.g. ``"mutex"``. See the ``Container`` example
given above, or the ``Mutex`` class in :ref:`mutexheader`.
+.. _scoped_capability:
SCOPED_CAPABILITY
-----------------
diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafety.h b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
index 0866b09bab2995..0fcf5bed1285a4 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafety.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
@@ -223,6 +223,42 @@ class ThreadSafetyHandler {
virtual void handleFunExcludesLock(StringRef Kind, Name FunName,
Name LockName, SourceLocation Loc) {}
+ /// Warn when an actual underlying mutex of a scoped lockable does not match
+ /// the expected.
+ /// \param Loc -- The location of the call expression.
+ /// \param DLoc -- The location of the function declaration.
+ /// \param ScopeName -- The name of the scope passed to the function.
+ /// \param Kind -- The kind of the expected mutex.
+ /// \param Expected -- The name of the expected mutex.
+ /// \param Actual -- The name of the actual mutex.
+ virtual void handleUnmatchedUnderlyingMutexes(SourceLocation Loc,
+ SourceLocation DLoc,
+ Name ScopeName, StringRef Kind,
+ Name Expected, Name Actual) {}
+
+ /// Warn when we get fewer underlying mutexes than expected.
+ /// \param Loc -- The location of the call expression.
+ /// \param DLoc -- The location of the function declaration.
+ /// \param ScopeName -- The name of the scope passed to the function.
+ /// \param Kind -- The kind of the expected mutex.
+ /// \param Expected -- The name of the expected mutex.
+ virtual void handleExpectMoreUnderlyingMutexes(SourceLocation Loc,
+ SourceLocation DLoc,
+ Name ScopeName, StringRef Kind,
+ Name Expected) {}
+
+ /// Warn when we get more underlying mutexes than expected.
+ /// \param Loc -- The location of the call expression.
+ /// \param DLoc -- The location of the function declaration.
+ /// \param ScopeName -- The name of the scope passed to the function.
+ /// \param Kind -- The kind of the actual mutex.
+ /// \param Actual -- The name of the actual mutex.
+ virtual void handleExpectFewerUnderlyingMutexes(SourceLocation Loc,
+ SourceLocation DLoc,
+ Name ScopeName,
+ StringRef Kind, Name Actual) {
+ }
+
/// Warn that L1 cannot be acquired before L2.
virtual void handleLockAcquiredBefore(StringRef Kind, Name L1Name,
Name L2Name, SourceLocation Loc) {}
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index ce86116680d7a3..c6f5a394829691 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -3685,7 +3685,7 @@ def AcquireCapability : InheritableAttr {
Clang<"acquire_shared_capability", 0>,
GNU<"exclusive_lock_function">,
GNU<"shared_lock_function">];
- let Subjects = SubjectList<[Function]>;
+ let Subjects = SubjectList<[Function, ParmVar]>;
let LateParsed = LateAttrParseStandard;
let TemplateDependent = 1;
let ParseArgumentsAsUnevaluated = 1;
@@ -3717,7 +3717,7 @@ def ReleaseCapability : InheritableAttr {
Clang<"release_shared_capability", 0>,
Clang<"release_generic_capability", 0>,
Clang<"unlock_function", 0>];
- let Subjects = SubjectList<[Function]>;
+ let Subjects = SubjectList<[Function, ParmVar]>;
let LateParsed = LateAttrParseStandard;
let TemplateDependent = 1;
let ParseArgumentsAsUnevaluated = 1;
@@ -3741,7 +3741,7 @@ def RequiresCapability : InheritableAttr {
let TemplateDependent = 1;
let ParseArgumentsAsUnevaluated = 1;
let InheritEvenIfAlreadyPresent = 1;
- let Subjects = SubjectList<[Function]>;
+ let Subjects = SubjectList<[Function, ParmVar]>;
let Accessors = [Accessor<"isShared", [Clang<"requires_shared_capability", 0>,
Clang<"shared_locks_required", 0>]>];
let Documentation = [Undocumented];
@@ -3863,7 +3863,7 @@ def LocksExcluded : InheritableAttr {
let TemplateDependent = 1;
let ParseArgumentsAsUnevaluated = 1;
let InheritEvenIfAlreadyPresent = 1;
- let Subjects = SubjectList<[Function]>;
+ let Subjects = SubjectList<[Function, ParmVar]>;
let Documentation = [Undocumented];
}
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index e4e04bff8b5120..34e7a5ee0afb37 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3984,6 +3984,10 @@ def warn_thread_attribute_decl_not_lockable : Warning<
def warn_thread_attribute_decl_not_pointer : Warning<
"%0 only applies to pointer types; type here is %1">,
InGroup<ThreadSafetyAttributes>, DefaultIgnore;
+def warn_thread_attribute_not_on_scoped_lockable_param : Warning<
+ "%0 attribute applies to function parameters only if their type is a "
+ "reference to a 'scoped_lockable'-annotated type">,
+ InGroup<ThreadSafetyAttributes>, DefaultIgnore;
def err_attribute_argument_out_of_bounds_extra_info : Error<
"%0 attribute parameter %1 is out of bounds: "
"%plural{0:no parameters to index into|"
@@ -4039,6 +4043,17 @@ def warn_acquired_before : Warning<
def warn_acquired_before_after_cycle : Warning<
"cycle in acquired_before/after dependencies, starting with '%0'">,
InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
+def warn_unmatched_underlying_mutexes : Warning<
+ "%0 managed by '%1' is '%3' instead of '%2'">,
+ InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
+def warn_expect_more_underlying_mutexes : Warning<
+ "%0 '%2' not managed by '%1'">,
+ InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
+def warn_expect_fewer_underlying_mutexes : Warning<
+ "did not expect %0 '%2' to be managed by '%1'">,
+ InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
+def note_managed_mismatch_here_for_param : Note<
+ "see attribute on parameter here">;
// Thread safety warnings negative capabilities
diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index 5577f45aa5217f..bfaf1a0e7c7ffc 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -103,6 +103,8 @@ class FactSet;
/// FIXME: this analysis does not currently support re-entrant locking.
class FactEntry : public CapabilityExpr {
public:
+ enum FactEntryKind { Lockable, ScopedLockable };
+
/// Where a fact comes from.
enum SourceKind {
Acquired, ///< The fact has been directly acquired.
@@ -112,6 +114,8 @@ class FactEntry : public CapabilityExpr {
};
private:
+ const FactEntryKind Kind : 8;
+
/// Exclusive or shared.
LockKind LKind : 8;
@@ -122,13 +126,14 @@ class FactEntry : public CapabilityExpr {
SourceLocation AcquireLoc;
public:
- FactEntry(const CapabilityExpr &CE, LockKind LK, SourceLocation Loc,
- SourceKind Src)
- : CapabilityExpr(CE), LKind(LK), Source(Src), AcquireLoc(Loc) {}
+ FactEntry(FactEntryKind FK, const CapabilityExpr &CE, LockKind LK,
+ SourceLocation Loc, SourceKind Src)
+ : CapabilityExpr(CE), Kind(FK), LKind(LK), Source(Src), AcquireLoc(Loc) {}
virtual ~FactEntry() = default;
LockKind kind() const { return LKind; }
SourceLocation loc() const { return AcquireLoc; }
+ FactEntryKind getFactEntryKind() const { return Kind; }
bool asserted() const { return Source == Asserted; }
bool declared() const { return Source == Declared; }
@@ -857,7 +862,7 @@ class LockableFactEntry : public FactEntry {
public:
LockableFactEntry(const CapabilityExpr &CE, LockKind LK, SourceLocation Loc,
SourceKind Src = Acquired)
- : FactEntry(CE, LK, Loc, Src) {}
+ : FactEntry(Lockable, CE, LK, Loc, Src) {}
void
handleRemovalFromIntersection(const FactSet &FSet, FactManager &FactMan,
@@ -885,6 +890,10 @@ class LockableFactEntry : public FactEntry {
!Cp, LK_Exclusive, UnlockLoc));
}
}
+
+ static bool classof(const FactEntry *A) {
+ return A->getFactEntryKind() == Lockable;
+ }
};
class ScopedLockableFactEntry : public FactEntry {
@@ -903,8 +912,16 @@ class ScopedLockableFactEntry : public FactEntry {
SmallVector<UnderlyingCapability, 2> UnderlyingMutexes;
public:
- ScopedLockableFactEntry(const CapabilityExpr &CE, SourceLocation Loc)
- : FactEntry(CE, LK_Exclusive, Loc, Acquired) {}
+ ScopedLockableFactEntry(const CapabilityExpr &CE, SourceLocation Loc,
+ SourceKind Src)
+ : FactEntry(ScopedLockable, CE, LK_Exclusive, Loc, Src) {}
+
+ CapExprSet getUnderlyingMutexes() const {
+ CapExprSet UnderlyingMutexesSet;
+ for (const UnderlyingCapability &UnderlyingMutex : UnderlyingMutexes)
+ UnderlyingMutexesSet.push_back(UnderlyingMutex.Cap);
+ return UnderlyingMutexesSet;
+ }
void addLock(const CapabilityExpr &M) {
UnderlyingMutexes.push_back(UnderlyingCapability{M, UCK_Acquired});
@@ -971,6 +988,10 @@ class ScopedLockableFactEntry : public FactEntry {
FSet.removeLock(FactMan, Cp);
}
+ static bool classof(const FactEntry *A) {
+ return A->getFactEntryKind() == ScopedLockable;
+ }
+
private:
void lock(FactSet &FSet, FactManager &FactMan, const CapabilityExpr &Cp,
LockKind kind, SourceLocation loc,
@@ -1806,7 +1827,7 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
if (Exp) {
assert(!Self);
const auto *TagT = Exp->getType()->getAs<TagType>();
- if (TagT && Exp->isPRValue()) {
+ if (D->hasAttrs() && TagT && Exp->isPRValue()) {
std::pair<til::LiteralPtr *, StringRef> Placeholder =
Analyzer->SxBuilder.createThisPlaceholder(Exp);
[[maybe_unused]] auto inserted =
@@ -1915,6 +1936,101 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
}
}
+ std::optional<CallExpr::const_arg_range> Args;
+ if (Exp) {
+ if (const auto *CE = dyn_cast<CallExpr>(Exp))
+ Args = CE->arguments();
+ else if (const auto *CE = dyn_cast<CXXConstructExpr>(Exp))
+ Args = CE->arguments();
+ else
+ llvm_unreachable("Unknown call kind");
+ }
+ const auto *CalledFunction = dyn_cast<FunctionDecl>(D);
+ if (CalledFunction && Args.has_value()) {
+ for (auto [Param, Arg] : zip(CalledFunction->parameters(), *Args)) {
+ CapExprSet DeclaredLocks;
+ for (const Attr *At : Param->attrs()) {
+ switch (At->getKind()) {
+ case attr::AcquireCapability: {
+ const auto *A = cast<AcquireCapabilityAttr>(At);
+ Analyzer->getMutexIDs(A->isShared() ? SharedLocksToAdd
+ : ExclusiveLocksToAdd,
+ A, Exp, D, Self);
+ Analyzer->getMutexIDs(DeclaredLocks, A, Exp, D, Self);
+ break;
+ }
+
+ case attr::ReleaseCapability: {
+ const auto *A = cast<ReleaseCapabilityAttr>(At);
+ if (A->isGeneric())
+ Analyzer->getMutexIDs(GenericLocksToRemove, A, Exp, D, Self);
+ else if (A->isShared())
+ Analyzer->getMutexIDs(SharedLocksToRemove, A, Exp, D, Self);
+ else
+ Analyzer->getMutexIDs(ExclusiveLocksToRemove, A, Exp, D, Self);
+ Analyzer->getMutexIDs(DeclaredLocks, A, Exp, D, Self);
+ break;
+ }
+
+ case attr::RequiresCapability: {
+ const auto *A = cast<RequiresCapabilityAttr>(At);
+ for (auto *Arg : A->args())
+ Analyzer->warnIfMutexNotHeld(FSet, D, Exp,
+ A->isShared() ? AK_Read : AK_Written,
+ Arg, POK_FunctionCall, Self, Loc);
+ Analyzer->getMutexIDs(DeclaredLocks, A, Exp, D, Self);
+ break;
+ }
+
+ case attr::LocksExcluded: {
+ const auto *A = cast<LocksExcludedAttr>(At);
+ for (auto *Arg : A->args())
+ Analyzer->warnIfMutexHeld(FSet, D, Exp, Arg, Self, Loc);
+ Analyzer->getMutexIDs(DeclaredLocks, A, Exp, D, Self);
+ break;
+ }
+
+ default:
+ break;
+ }
+ }
+ if (DeclaredLocks.empty())
+ continue;
+ CapabilityExpr Cp(Analyzer->SxBuilder.translate(Arg, nullptr),
+ StringRef("mutex"), false);
+ if (const auto *CBTE = dyn_cast<CXXBindTemporaryExpr>(Arg->IgnoreCasts());
+ Cp.isInvalid() && CBTE) {
+ if (auto Object = Analyzer->ConstructedObjects.find(CBTE->getSubExpr());
+ Object != Analyzer->ConstructedObjects.end())
+ Cp = CapabilityExpr(Object->second, StringRef("mutex"), false);
+ }
+ const FactEntry *Fact = FSet.findLock(Analyzer->FactMan, Cp);
+ if (!Fact) {
+ Analyzer->Handler.handleMutexNotHeld(Cp.getKind(), D, POK_FunctionCall,
+ Cp.toString(), LK_Exclusive,
+ Exp->getExprLoc());
+ continue;
+ }
+ const auto *Scope = cast<ScopedLockableFactEntry>(Fact);
+ for (const auto &[a, b] :
+ zip_longest(DeclaredLocks, Scope->getUnderlyingMutexes())) {
+ if (!a.has_value()) {
+ Analyzer->Handler.handleExpectFewerUnderlyingMutexes(
+ Exp->getExprLoc(), D->getLocation(), Scope->toString(),
+ b.value().getKind(), b.value().toString());
+ } else if (!b.has_value()) {
+ Analyzer->Handler.handleExpectMoreUnderlyingMutexes(
+ Exp->getExprLoc(), D->getLocation(), Scope->toString(),
+ a.value().getKind(), a.value().toString());
+ } else if (!a.value().equals(b.value())) {
+ Analyzer->Handler.handleUnmatchedUnderlyingMutexes(
+ Exp->getExprLoc(), D->getLocation(), Scope->toString(),
+ a.value().getKind(), a.value().toString(), b.value().toString());
+ break;
+ }
+ }
+ }
+ }
// Remove locks first to allow lock upgrading/downgrading.
// FIXME -- should only fully remove if the attribute refers to 'this'.
bool Dtor = isa<CXXDestructorDecl>(D);
@@ -1937,7 +2053,8 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
if (!Scp.shouldIgnore()) {
// Add the managing object as a dummy mutex, mapped to the underlying mutex.
- auto ScopedEntry = std::make_unique<ScopedLockableFactEntry>(Scp, Loc);
+ auto ScopedEntry = std::make_unique<ScopedLockableFactEntry>(
+ Scp, Loc, FactEntry::Acquired);
for (const auto &M : ExclusiveLocksToAdd)
ScopedEntry->addLock(M);
for (const auto &M : SharedLocksToAdd)
@@ -2084,7 +2201,7 @@ void BuildLockset::VisitCallExpr(const CallExpr *Exp) {
}
auto *D = dyn_cast_or_null<NamedDecl>(Exp->getCalleeDecl());
- if(!D || !D->hasAttrs())
+ if (!D)
return;
handleCall(Exp, D);
}
@@ -2324,7 +2441,7 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
// Add locks from exclusive_locks_required and shared_locks_required
// to initial lockset. Also turn off checking for lock and unlock functions.
// FIXME: is there a more intelligent way to check lock/unlock functions?
- if (!SortedGraph->empty() && D->hasAttrs()) {
+ if (!SortedGraph->empty()) {
assert(*SortedGraph->begin() == &CFGraph->getEntry());
FactSet &InitialLockset = Initial.EntrySet;
@@ -2362,6 +2479,44 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
return;
}
}
+ ArrayRef<ParmVarDecl *> Params;
+ if (CurrentFunction)
+ Params = CurrentFunction->getCanonicalDecl()->parameters();
+ else if (auto CurrentMethod = dyn_cast<ObjCMethodDecl>(D))
+ Params = CurrentMethod->getCanonicalDecl()->parameters();
+ else
+ llvm_unreachable("Unknown function kind");
+ for (const ParmVarDecl *Param : Params) {
+ CapExprSet UnderlyingLocks;
+ for (const auto *Attr : Param->attrs()) {
+ Loc = Attr->getLocation();
+ if (const auto *A = dyn_cast<ReleaseCapabilityAttr>(Attr)) {
+ getMutexIDs(A->isShared() ? SharedLocksToAdd : ExclusiveLocksToAdd, A,
+ nullptr, Param);
+ getMutexIDs(LocksReleased, A, nullptr, Param);
+ getMutexIDs(UnderlyingLocks, A, nullptr, Param);
+ } else if (const auto *A = dyn_cast<RequiresCapabilityAttr>(Attr)) {
+ getMutexIDs(A->isShared() ? SharedLocksToAdd : ExclusiveLocksToAdd, A,
+ nullptr, Param);
+ getMutexIDs(UnderlyingLocks, A, nullptr, Param);
+ } else if (const auto *A = dyn_cast<AcquireCapabilityAttr>(Attr)) {
+ getMutexIDs(A->isShared() ? SharedLocksAcquired
+ : ExclusiveLocksAcquired,
+ A, nullptr, Param);
+ getMutexIDs(UnderlyingLocks, A, nullptr, Param);
+ } else if (const auto *A = dyn_cast<LocksExcludedAttr>(Attr)) {
+ getMutexIDs(UnderlyingLocks, A, nullptr, Param);
+ }
+ }
+ if (UnderlyingLocks.empty())
+ continue;
+ CapabilityExpr Cp(SxBuilder.createVariable(Param), StringRef(), false);
+ auto ScopedEntry = std::make_unique<ScopedLockableFactEntry>(
+ Cp, Param->getLocation(), FactEntry::Declared);
+ for (const CapabilityExpr &M : UnderlyingLocks)
+ ScopedEntry->addLock(M);
+ addLock(InitialLockset, std::move(ScopedEntry), true);
+ }
// FIXME -- Loc can be wrong here.
for (const auto &Mu : ExclusiveLocksToAdd) {
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 6496a33b8f5a50..d131072aeaf4c2 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -1851,6 +1851,14 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler {
: getNotes();
}
+ OptionalNotes makeManagedMismatchNoteForParam(SourceLocation DeclLoc) {
+ return DeclLoc.isValid()
+ ? getNotes(PartialDiagnosticAt(
+ DeclLoc,
+ S.PDiag(diag::note_managed_mismatch_here_for_param)))
+ : getNotes();
+ }
+
public:
ThreadSafetyReporter(Sema &S, SourceLocation FL, SourceLocation FEL)
: S(S), FunLocation(FL), FunEndLocation(FEL),
@@ -1871,6 +1879,38 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler {
}
}
+ void handleUnmatchedUnderlyingMutexes(SourceLocation Loc, SourceLocation DLoc,
+ Name scopeName, StringRef Kind,
+ Name expected, Name actual) override {
+ PartialDiagnosticAt Warning(Loc,
+ S.PDiag(diag::warn_unmatched_underlying_mutexes)
+ << Kind << scopeName << expected << actual);
+ Warnings.emplace_back(std::move(Warning),
+ makeManagedMismatchNoteForParam(DLoc));
+ }
+
+ void handleExpectMoreUnderlyingMutexes(SourceLocation Loc,
+ SourceLocation DLoc, Name scopeName,
+ StringRef Kind,
+ Name expected) override {
+ PartialDiagnosticAt Warning(
+ Loc, S.PDiag(diag::warn_expect_more_underlying_mutexes)
+ << Kind << scopeName << expected);
+ Warnings.emplace_back(std::move(Warning),
+ makeManagedMismatchNoteForParam(DLoc));
+ }
+
+ void handleExpectFewerUnderlyingMutexes(SourceLocation Loc,
+ SourceLocation DLoc, Name scopeName,
+ StringRef Kind,
+ Name actual) override {
+ PartialDiagnosticAt Warning(
+ Loc, S.PDiag(diag::warn_expect_fewer_underlying_mutexes)
+ << Kind << scopeName << actual);
+ Warnings.emplace_back(std::move(Warning),
+ makeManagedMismatchNoteForParam(DLoc));
+ }
+
void handleInvalidLockExp(SourceLocation Loc) override {
PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_cannot_resolve_lock)
<< Loc);
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 14cc51cf89665a..e3882cf235ee77 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -273,6 +273,19 @@ static bool checkRecordTypeForCapability(Sema &S, QualType Ty) {
return checkRecordDeclForAttr<CapabilityAttr>(RT->getDecl());
}
+static bool checkRecordTypeForScopedCapability(Sema &S, QualType Ty) {
+ const RecordType *RT = getRecordType(Ty);
+
+ if (!RT)
+ return false;
+
+ // Don't check for the capability if the class hasn't been defined yet.
+ if (RT->isIncompleteType())
+ return true;
+
+ return checkRecordDeclForAttr<ScopedLockableAttr>(RT->getDecl());
+}
+
static bool checkTypedefTypeForCapability(QualType Ty) {
const auto *TD = Ty->getAs<TypedefType>();
if (!TD)
@@ -418,6 +431,19 @@ static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D,
}
}
+static bool checkFunParamsAreScopedLockable(Sema &S,
+ const ParmVarDecl *ParamDecl,
+ const ParsedAttr &AL) {
+ QualType ParamType = ParamDecl->getType();
+ if (const auto *RefType = ParamType->getAs<ReferenceType>();
+ RefType &&
+ checkRecordTypeForScopedCapability(S, RefType->getPointeeType()))
+ return true;
+ S.Diag(AL.getLoc(), diag::warn_thread_attribute_not_on_scoped_lockable_param)
+ << AL;
+ return false;
+}
+
//===----------------------------------------------------------------------===//
// Attribute Implementations
//===----------------------------------------------------------------------===//
@@ -645,6 +671,10 @@ static void handleLockReturnedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
}
static void handleLocksExcludedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+ if (const auto *ParmDecl = dyn_cast<ParmVarDecl>(D);
+ ParmDecl && !checkFunParamsAreScopedLockable(S, ParmDecl, AL))
+ return;
+
if (!AL.checkAtLeastNumArgs(S, 1))
return;
@@ -5767,6 +5797,10 @@ static void handleAssertCapabilityAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
static void handleAcquireCapabilityAttr(Sema &S, Decl *D,
const ParsedAttr &AL) {
+ if (const auto *ParmDecl = dyn_cast<ParmVarDecl>(D))
+ if (!checkFunParamsAreScopedLockable(S, ParmDecl, AL))
+ return;
+
SmallVector<Expr*, 1> Args;
if (!checkLockFunAttrCommon(S, D, AL, Args))
return;
@@ -5787,6 +5821,9 @@ static void handleTryAcquireCapabilityAttr(Sema &S, Decl *D,
static void handleReleaseCapabilityAttr(Sema &S, Decl *D,
const ParsedAttr &AL) {
+ if (const auto *ParmDecl = dyn_cast<ParmVarDecl>(D))
+ if (!checkFunParamsAreScopedLockable(S, ParmDecl, AL))
+ return;
// Check that all arguments are lockable objects.
SmallVector<Expr *, 1> Args;
checkAttrArgsAreCapabilityObjs(S, D, AL, Args, 0, true);
@@ -5797,6 +5834,10 @@ static void handleReleaseCapabilityAttr(Sema &S, Decl *D,
static void handleRequiresCapabilityAttr(Sema &S, Decl *D,
const ParsedAttr &AL) {
+ if (const auto *ParmDecl = dyn_cast<ParmVarDecl>(D))
+ if (!checkFunParamsAreScopedLockable(S, ParmDecl, AL))
+ return;
+
if (!AL.checkAtLeastNumArgs(S, 1))
return;
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index 8477200456d985..4a082c872bf4ed 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -3171,6 +3171,378 @@ void lockUnlock() EXCLUSIVE_LOCKS_REQUIRED(mu) {
} // end namespace ScopedUnlock
+namespace PassingScope {
+
+class SCOPED_LOCKABLE RelockableScope {
+public:
+ RelockableScope(Mutex *mu) EXCLUSIVE_LOCK_FUNCTION(mu);
+ void Release() UNLOCK_FUNCTION();
+ void Acquire() EXCLUSIVE_LOCK_FUNCTION();
+ ~RelockableScope() EXCLUSIVE_UNLOCK_FUNCTION();
+};
+
+class SCOPED_LOCKABLE ReaderRelockableScope {
+public:
+ ReaderRelockableScope(Mutex *mu) SHARED_LOCK_FUNCTION(mu);
+ void Release() UNLOCK_FUNCTION();
+ void Acquire() SHARED_LOCK_FUNCTION();
+ ~ReaderRelockableScope() UNLOCK_FUNCTION();
+};
+
+Mutex mu;
+Mutex mu1;
+Mutex mu2;
+int x GUARDED_BY(mu);
+int y GUARDED_BY(mu) GUARDED_BY(mu2);
+void print(int);
+
+// Analysis inside the function with annotated parameters
+
+void sharedRequired(ReleasableMutexLock& scope SHARED_LOCKS_REQUIRED(mu)) {
+ print(x); // OK.
+}
+
+void sharedAcquire(ReaderRelockableScope& scope SHARED_LOCK_FUNCTION(mu)) {
+ scope.Acquire();
+ print(x);
+}
+
+void sharedRelease(RelockableScope& scope SHARED_UNLOCK_FUNCTION(mu)) {
+ print(x);
+ scope.Release();
+ print(x); // expected-warning{{reading variable 'x' requires holding mutex 'mu'}}
+}
+
+void requiredLock(ReleasableMutexLock& scope EXCLUSIVE_LOCKS_REQUIRED(mu)) {
+ x = 1; // OK.
+}
+
+void reacquireRequiredLock(RelockableScope& scope EXCLUSIVE_LOCKS_REQUIRED(mu)) {
+ scope.Release();
+ scope.Acquire();
+ x = 2; // OK.
+}
+
+void releaseSingleMutex(ReleasableMutexLock& scope EXCLUSIVE_UNLOCK_FUNCTION(mu)) {
+ x = 1; // OK.
+ scope.Release();
+ x = 2; // expected-warning{{writing variable 'x' requires holding mutex 'mu' exclusively}}
+}
+
+void releaseMultipleMutexes(ReleasableMutexLock& scope EXCLUSIVE_UNLOCK_FUNCTION(mu, mu2)) {
+ y = 1; // OK.
+ scope.Release();
+ y = 2; // expected-warning{{writing variable 'y' requires holding mutex 'mu' exclusively}}
+ // expected-warning at -1{{writing variable 'y' requires holding mutex 'mu2' exclusively}}
+}
+
+void acquireLock(RelockableScope& scope EXCLUSIVE_LOCK_FUNCTION(mu)) {
+ x = 1; // expected-warning{{writing variable 'x' requires holding mutex 'mu' exclusively}}
+ scope.Acquire();
+ x = 2; // OK.
+}
+
+void acquireMultipleLocks(RelockableScope& scope EXCLUSIVE_LOCK_FUNCTION(mu, mu2)) {
+ y = 1; // expected-warning{{writing variable 'y' requires holding mutex 'mu' exclusively}}
+ // expected-warning at -1{{writing variable 'y' requires holding mutex 'mu2' exclusively}}
+ scope.Acquire();
+ y = 2; // OK.
+}
+
+void excludedLock(ReleasableMutexLock& scope LOCKS_EXCLUDED(mu)) {
+ x = 1; // expected-warning{{writing variable 'x' requires holding mutex 'mu' exclusively}}
+}
+
+void acquireAndReleaseExcludedLocks(RelockableScope& scope LOCKS_EXCLUDED(mu)) {
+ scope.Acquire();
+ scope.Release();
+}
+
+// expected-note at +1{{mutex acquired here}}
+void unreleasedMutex(ReleasableMutexLock& scope EXCLUSIVE_UNLOCK_FUNCTION(mu)) {
+ x = 1; // OK.
+} // expected-warning{{mutex 'mu' is still held at the end of function}}
+
+// expected-note at +1{{mutex acquired here}}
+void acquireAlreadyHeldMutex(RelockableScope& scope EXCLUSIVE_UNLOCK_FUNCTION(mu)) {
+ scope.Acquire(); // expected-warning{{acquiring mutex 'mu' that is already held}}
+ scope.Release();
+}
+
+void reacquireMutex(RelockableScope& scope EXCLUSIVE_UNLOCK_FUNCTION(mu)) {
+ scope.Release();
+ scope.Acquire(); // expected-note{{mutex acquired here}}
+ x = 2; // OK.
+} // expected-warning{{mutex 'mu' is still held at the end of function}}
+
+// expected-note at +1{{mutex acquired here}}
+void requireSingleMutex(ReleasableMutexLock& scope EXCLUSIVE_LOCKS_REQUIRED(mu)) {
+ x = 1; // OK.
+ scope.Release();
+ x = 2; // expected-warning{{writing variable 'x' requires holding mutex 'mu' exclusively}}
+} // expected-warning{{expecting mutex 'mu' to be held at the end of function}}
+
+// expected-note at +1 2{{mutex acquired here}}
+void requireMultipleMutexes(ReleasableMutexLock& scope EXCLUSIVE_LOCKS_REQUIRED(mu, mu2)) {
+ y = 1; // OK.
+ scope.Release();
+ y = 2; // expected-warning{{writing variable 'y' requires holding mutex 'mu' exclusively}}
+ // expected-warning at -1{{writing variable 'y' requires holding mutex 'mu2' exclusively}}
+} // expected-warning{{expecting mutex 'mu' to be held at the end of function}}
+ // expected-warning at -1{{expecting mutex 'mu2' to be held at the end of function}}
+
+// expected-note at +1{{mutex acquired here}}
+void acquireAlreadyHeldLock(RelockableScope& scope EXCLUSIVE_LOCKS_REQUIRED(mu)) {
+ scope.Acquire(); // expected-warning{{acquiring mutex 'mu' that is already held}}
+}
+
+// expected-note at +1 {{mutex acquired here}}
+void releaseWithoutHoldingLock(ReleasableMutexLock& scope EXCLUSIVE_LOCK_FUNCTION(mu)) {
+ scope.Release(); // expected-warning{{releasing mutex 'mu' that was not held}}
+} // expected-warning{{expecting mutex 'mu' to be held at the end of function}}
+
+// expected-note at +1 {{mutex acquired here}}
+void endWithReleasedMutex(RelockableScope& scope EXCLUSIVE_LOCK_FUNCTION(mu)) {
+ scope.Acquire();
+ scope.Release();
+} // expected-warning{{expecting mutex 'mu' to be held at the end of function}}
+
+void acquireExcludedLock(RelockableScope& scope LOCKS_EXCLUDED(mu)) {
+ x = 1; // expected-warning{{writing variable 'x' requires holding mutex 'mu' exclusively}}
+ scope.Acquire(); // expected-note {{mutex acquired here}}
+ x = 2; // OK.
+} // expected-warning{{mutex 'mu' is still held at the end of function}}
+
+void acquireMultipleExcludedLocks(RelockableScope& scope LOCKS_EXCLUDED(mu, mu2)) {
+ y = 1; // expected-warning{{writing variable 'y' requires holding mutex 'mu' exclusively}}
+ // expected-warning at -1{{writing variable 'y' requires holding mutex 'mu2' exclusively}}
+ scope.Acquire(); // expected-note 2{{mutex acquired here}}
+ y = 2; // OK.
+} // expected-warning{{mutex 'mu' is still held at the end of function}}
+ // expected-warning at -1{{mutex 'mu2' is still held at the end of function}}
+
+void reacquireExcludedLocks(RelockableScope& scope LOCKS_EXCLUDED(mu)) {
+ scope.Release(); // expected-warning{{releasing mutex 'mu' that was not held}}
+ scope.Acquire(); // expected-note {{mutex acquired here}}
+ x = 2; // OK.
+} // expected-warning{{mutex 'mu' is still held at the end of function}}
+
+// expected-note at +1{{mutex acquired here}}
+void sharedRequired2(ReleasableMutexLock& scope SHARED_LOCKS_REQUIRED(mu)) {
+ print(x); // OK.
+ scope.Release();
+ print(x); // expected-warning{{reading variable 'x' requires holding mutex 'mu'}}
+} // expected-warning{{expecting mutex 'mu' to be held at the end of function}}
+
+// expected-note at +1{{mutex acquired here}}
+void sharedAcquire2(RelockableScope& scope SHARED_LOCK_FUNCTION(mu)) {
+ print(x); // expected-warning{{reading variable 'x' requires holding mutex 'mu'}}
+ scope.Release(); // expected-warning{{releasing mutex 'mu' that was not held}}
+} // expected-warning{{expecting mutex 'mu' to be held at the end of function}}
+
+// expected-note at +1 2{{mutex acquired here}}
+void sharedRelease2(RelockableScope& scope SHARED_UNLOCK_FUNCTION(mu)) {
+ scope.Acquire(); //expected-warning{{acquiring mutex 'mu' that is already held}}
+} //expected-warning{{mutex 'mu' is still held at the end of function}}
+
+// Handling the call of the function with annotated parameters
+
+// expected-note at +1{{see attribute on parameter here}}
+void release(ReleasableMutexLock& scope EXCLUSIVE_UNLOCK_FUNCTION(mu));
+// expected-note at +1{{see attribute on parameter here}}
+void release_DoubleMutexLock(DoubleMutexLock& scope EXCLUSIVE_UNLOCK_FUNCTION(mu));
+// expected-note at +1 2{{see attribute on parameter here}}
+void release_two(ReleasableMutexLock& scope EXCLUSIVE_UNLOCK_FUNCTION(mu, mu2));
+// expected-note at +1{{see attribute on parameter here}}
+void release_double(DoubleMutexLock& scope EXCLUSIVE_UNLOCK_FUNCTION(mu, mu2));
+void require(ReleasableMutexLock& scope EXCLUSIVE_LOCKS_REQUIRED(mu));
+void acquire(RelockableScope& scope EXCLUSIVE_LOCK_FUNCTION(mu));
+void exclude(RelockableScope& scope LOCKS_EXCLUDED(mu));
+
+void release_shared(ReaderRelockableScope& scope SHARED_UNLOCK_FUNCTION(mu));
+void require_shared(ReleasableMutexLock& scope SHARED_LOCKS_REQUIRED(mu));
+void acquire_shared(ReaderRelockableScope& scope SHARED_LOCK_FUNCTION(mu));
+
+void unlockCall() {
+ ReleasableMutexLock scope(&mu);
+ x = 1; // OK.
+ release(scope);
+ x = 2; // expected-warning{{writing variable 'x' requires holding mutex 'mu' exclusively}}
+}
+
+void unlockSharedCall() {
+ ReaderRelockableScope scope(&mu);
+ print(x); // OK.
+ release_shared(scope);
+ print(x); // expected-warning{{reading variable 'x' requires holding mutex 'mu'}}
+}
+
+void requireCall() {
+ ReleasableMutexLock scope(&mu);
+ x = 1; // OK.
+ require(scope);
+ x = 2; // Ok.
+}
+
+void requireSharedCall() {
+ ReleasableMutexLock scope(&mu);
+ print(x); // OK.
+ require_shared(scope);
+ print(x); // Ok.
+}
+
+void acquireCall() {
+ RelockableScope scope(&mu);
+ scope.Release();
+ acquire(scope);
+ x = 2; // Ok.
+}
+
+void acquireSharedCall() {
+ ReaderRelockableScope scope(&mu);
+ scope.Release();
+ acquire_shared(scope);
+ print(x); // Ok.
+}
+
+void writeAfterExcludeCall() {
+ RelockableScope scope(&mu);
+ scope.Release();
+ exclude(scope);
+ x = 2; // expected-warning{{writing variable 'x' requires holding mutex 'mu' exclusively}}
+}
+
+void unlockCallAfterExplicitRelease() {
+ ReleasableMutexLock scope(&mu);
+ x = 1; // OK.
+ scope.Release(); // expected-note{{mutex released here}}
+ release(scope); // expected-warning{{releasing mutex 'mu' that was not held}}
+ x = 2; // expected-warning{{writing variable 'x' requires holding mutex 'mu' exclusively}}
+}
+
+void unmatchedMutexes() {
+ ReleasableMutexLock scope(&mu2);
+ release(scope); // expected-warning{{mutex managed by 'scope' is 'mu2' instead of 'mu'}}
+ // expected-warning at -1{{releasing mutex 'mu' that was not held}}
+}
+
+void wrongOrder() {
+ DoubleMutexLock scope(&mu2, &mu);
+ release_double(scope); // expected-warning{{mutex managed by 'scope' is 'mu2' instead of 'mu'}}
+}
+
+void differentNumberOfMutexes() {
+ ReleasableMutexLock scope(&mu);
+ release_two(scope); // expected-warning{{mutex 'mu2' not managed by 'scope'}}
+ // expected-warning at -1{{releasing mutex 'mu2' that was not held}}
+}
+
+void differentNumberOfMutexes2() {
+ ReleasableMutexLock scope(&mu2);
+ release_two(scope); // expected-warning{{mutex managed by 'scope' is 'mu2' instead of 'mu'}}
+ // expected-warning at -1{{releasing mutex 'mu' that was not held}}
+}
+
+void differentNumberOfMutexes3() {
+ DoubleMutexLock scope(&mu, &mu2);
+ release_DoubleMutexLock(scope); // expected-warning{{did not expect mutex 'mu2' to be managed by 'scope'}}
+}
+
+void releaseDefault(ReleasableMutexLock& scope EXCLUSIVE_UNLOCK_FUNCTION(mu), int = 0);
+
+void unlockFunctionDefault() {
+ ReleasableMutexLock scope(&mu);
+ x = 1; // OK.
+ releaseDefault(scope);
+ x = 2; // expected-warning{{writing variable 'x' requires holding mutex 'mu' exclusively}}
+}
+
+void requireCallWithReleasedLock() {
+ ReleasableMutexLock scope(&mu);
+ scope.Release();
+ require(scope); // expected-warning{{calling function 'require' requires holding mutex 'mu' exclusively}}
+}
+
+void acquireCallWithAlreadyHeldLock() {
+ RelockableScope scope(&mu); // expected-note{{mutex acquired here}}
+ acquire(scope); // expected-warning{{acquiring mutex 'mu' that is already held}}
+ x = 1;
+}
+
+void excludeCallWithAlreadyHeldLock() {
+ RelockableScope scope(&mu);
+ exclude(scope); // expected-warning{{cannot call function 'exclude' while mutex 'mu' is held}}
+ x = 2; // OK.
+}
+
+void requireConst(const ReleasableMutexLock& scope EXCLUSIVE_LOCKS_REQUIRED(mu));
+void requireConstCall() {
+ requireConst(ReleasableMutexLock(&mu));
+}
+
+void passScopeUndeclared(ReleasableMutexLock &scope) {
+ release(scope); // expected-warning{{calling function 'release' requires holding mutex 'scope' exclusively}}
+ // expected-warning at -1{{releasing mutex 'mu' that was not held}}
+}
+
+class SCOPED_LOCKABLE ScopedWithoutLock {
+public:
+ ScopedWithoutLock();
+
+ ~ScopedWithoutLock() EXCLUSIVE_UNLOCK_FUNCTION();
+};
+
+void require(ScopedWithoutLock &scope EXCLUSIVE_LOCKS_REQUIRED(mu));
+
+void constructWithoutLock() {
+ ScopedWithoutLock scope;
+ require(scope); // expected-warning{{calling function 'require' requires holding mutex 'mu' exclusively}}
+ // expected-warning at -1{{calling function 'require' requires holding mutex 'scope' exclusively}}
+} // expected-warning {{releasing mutex 'scope' that was not held}}
+
+void requireConst(const ScopedWithoutLock& scope EXCLUSIVE_LOCKS_REQUIRED(mu));
+
+void requireCallWithReleasedLock2() {
+ requireConst(ScopedWithoutLock());
+ // expected-warning at -1{{calling function 'requireConst' requires holding mutex '#undefined' exclusively}}
+ // expected-warning at -2{{calling function 'requireConst' requires holding mutex 'mu' exclusively}}
+}
+
+void requireDecl(RelockableScope &scope EXCLUSIVE_LOCKS_REQUIRED(mu));
+void requireDecl(RelockableScope &scope) {
+ scope.Release();
+ scope.Acquire();
+}
+
+struct foo
+{
+ Mutex mu;
+ // expected-note at +1{{see attribute on parameter here}}
+ void require(RelockableScope &scope EXCLUSIVE_LOCKS_REQUIRED(mu));
+ void callRequire(){
+ RelockableScope scope(&mu);
+ // TODO: False positive due to incorrect parsing of parameter attribute arguments.
+ require(scope);
+ // expected-warning at -1{{calling function 'require' requires holding mutex 'mu' exclusively}}
+ // expected-warning at -2{{mutex managed by 'scope' is 'mu' instead of 'mu'}}
+ }
+};
+
+struct ObjectWithMutex {
+ Mutex mu;
+ int x GUARDED_BY(mu);
+};
+void releaseMember(ObjectWithMutex& object, ReleasableMutexLock& scope EXCLUSIVE_UNLOCK_FUNCTION(object.mu)) {
+ object.x = 1;
+ scope.Release();
+}
+void releaseMemberCall() {
+ ObjectWithMutex obj;
+ ReleasableMutexLock lock(&obj.mu);
+ releaseMember(obj, lock);
+}
+
+} // end namespace PassingScope
namespace TrylockFunctionTest {
diff --git a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
index 0c5b0cc85897be..752803e4a05430 100644
--- a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
@@ -62,6 +62,12 @@ class MuDoubleWrapper {
}
};
+class SCOPED_LOCKABLE MutexLock {
+ public:
+ MutexLock(Mutex *mu) EXCLUSIVE_LOCK_FUNCTION(mu);
+ ~MutexLock() UNLOCK_FUNCTION();
+};
+
Mutex mu1;
UnlockableMu umu;
Mutex mu2;
@@ -599,8 +605,11 @@ class EXCLUSIVE_LOCK_FUNCTION() ElfTestClass { // \
// expected-warning {{'exclusive_lock_function' attribute only applies to functions}}
};
-void elf_fun_params(int lvar EXCLUSIVE_LOCK_FUNCTION()); // \
- // expected-warning {{'exclusive_lock_function' attribute only applies to functions}}
+void elf_fun_params1(MutexLock& scope EXCLUSIVE_LOCK_FUNCTION(mu1));
+void elf_fun_params2(int lvar EXCLUSIVE_LOCK_FUNCTION(mu1)); // \
+ // expected-warning{{'exclusive_lock_function' attribute applies to function parameters only if their type is a reference to a 'scoped_lockable'-annotated type}}
+void elf_fun_params3(MutexLock& scope EXCLUSIVE_LOCK_FUNCTION()); // \
+ // expected-warning{{'exclusive_lock_function' attribute without capability arguments can only be applied to non-static methods of a class}}
// Check argument parsing.
@@ -660,8 +669,11 @@ int slf_testfn(int y) {
int slf_test_var SHARED_LOCK_FUNCTION(); // \
// expected-warning {{'shared_lock_function' attribute only applies to functions}}
-void slf_fun_params(int lvar SHARED_LOCK_FUNCTION()); // \
- // expected-warning {{'shared_lock_function' attribute only applies to functions}}
+void slf_fun_params1(MutexLock& scope SHARED_LOCK_FUNCTION(mu1));
+void slf_fun_params2(int lvar SHARED_LOCK_FUNCTION(mu1)); // \
+ // expected-warning {{'shared_lock_function' attribute applies to function parameters only if their type is a reference to a 'scoped_lockable'-annotated type}}
+void slf_fun_params3(MutexLock& scope SHARED_LOCK_FUNCTION()); // \
+ // expected-warning {{'shared_lock_function' attribute without capability arguments can only be applied to non-static methods of a class}}
class SlfFoo {
private:
@@ -903,8 +915,11 @@ class NO_THREAD_SAFETY_ANALYSIS UfTestClass { // \
// expected-warning {{'no_thread_safety_analysis' attribute only applies to functions}}
};
-void uf_fun_params(int lvar UNLOCK_FUNCTION()); // \
- // expected-warning {{'unlock_function' attribute only applies to functions}}
+void uf_fun_params1(MutexLock& scope UNLOCK_FUNCTION(mu1));
+void uf_fun_params2(int lvar UNLOCK_FUNCTION(mu1)); // \
+ // expected-warning {{'unlock_function' attribute applies to function parameters only if their type is a reference to a 'scoped_lockable'-annotated type}}
+void uf_fun_params3(MutexLock& scope UNLOCK_FUNCTION()); // \
+ // expected-warning {{'unlock_function' attribute without capability arguments can only be applied to non-static methods of a class}}
// Check argument parsing.
@@ -1035,8 +1050,9 @@ int le_testfn(int y) {
int le_test_var LOCKS_EXCLUDED(mu1); // \
// expected-warning {{'locks_excluded' attribute only applies to functions}}
-void le_fun_params(int lvar LOCKS_EXCLUDED(mu1)); // \
- // expected-warning {{'locks_excluded' attribute only applies to functions}}
+void le_fun_params1(MutexLock& scope LOCKS_EXCLUDED(mu1));
+void le_fun_params2(int lvar LOCKS_EXCLUDED(mu1)); // \
+ // expected-warning{{'locks_excluded' attribute applies to function parameters only if their type is a reference to a 'scoped_lockable'-annotated type}}
class LeFoo {
private:
@@ -1102,8 +1118,9 @@ int elr_testfn(int y) {
int elr_test_var EXCLUSIVE_LOCKS_REQUIRED(mu1); // \
// expected-warning {{'exclusive_locks_required' attribute only applies to functions}}
-void elr_fun_params(int lvar EXCLUSIVE_LOCKS_REQUIRED(mu1)); // \
- // expected-warning {{'exclusive_locks_required' attribute only applies to functions}}
+void elr_fun_params1(MutexLock& scope EXCLUSIVE_LOCKS_REQUIRED(mu1));
+void elr_fun_params2(int lvar EXCLUSIVE_LOCKS_REQUIRED(mu1)); // \
+ // expected-warning {{'exclusive_locks_required' attribute applies to function parameters only if their type is a reference to a 'scoped_lockable'-annotated type}}
class ElrFoo {
private:
@@ -1170,8 +1187,9 @@ int slr_testfn(int y) {
int slr_test_var SHARED_LOCKS_REQUIRED(mu1); // \
// expected-warning {{'shared_locks_required' attribute only applies to functions}}
-void slr_fun_params(int lvar SHARED_LOCKS_REQUIRED(mu1)); // \
- // expected-warning {{'shared_locks_required' attribute only applies to functions}}
+void slr_fun_params1(MutexLock& scope SHARED_LOCKS_REQUIRED(mu1));
+void slr_fun_params2(int lvar SHARED_LOCKS_REQUIRED(mu1)); // \
+ // expected-warning {{'shared_locks_required' attribute applies to function parameters only if their type is a reference to a 'scoped_lockable'-annotated type}}
class SlrFoo {
private:
More information about the cfe-commits
mailing list