[clang] Thread Safety Analysis: Differentiate between lock sets at real join points and expected/actual sets at function end (PR #105526)

[Malek Ben Slimane via cfe-commits]

https://github.com/malek203 created 
https://github.com/llvm/llvm-project/pull/105526

This fixes false positives related to returning a scoped lockable object. At 
the end of a function, we check managed locks instead of scoped locks.

At real join points, we skip checking managed locks because we assume that the 
scope keeps track of its underlying mutexes and will release them at its 
destruction. So, checking for the scopes is sufficient. However, at the end of 
a function, we aim at comparing the expected and the actual lock sets. There, 
we skip checking scoped locks to prevent to get duplicate warnings for the same 
lock.

>From 9d4dbf5c3be29175e23f1881efa8f3b7626de755 Mon Sep 17 00:00:00 2001
From: Malek Ben Slimane 
Date: Tue, 20 Aug 2024 14:53:54 +0200
Subject: [PATCH] Thread Safety Analysis: Differentiate between lock sets at
 real join points and expected/actual sets at function end

This fixes false positives related to returning a scoped lockable
object. At the end of a function, we check managed locks instead of
scoped locks.

At real join points, we skip checking managed locks because we assume
that the scope keeps track of its underlying mutexes and will release
them at its destruction. So, checking for the scopes is sufficient.
However, at the end of a function, we aim at comparing the expected and
the actual lock sets. There, we skip checking scoped locks to prevent to
get duplicate warnings for the same lock.
---
 clang/lib/Analysis/ThreadSafety.cpp   |  8 ++--
 .../SemaCXX/warn-thread-safety-analysis.cpp   | 20 ---
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/clang/lib/Analysis/ThreadSafety.cpp 
b/clang/lib/Analysis/ThreadSafety.cpp
index e25b843c9bf83e..c4a83b069e0792 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -922,6 +922,9 @@ class ScopedLockableFactEntry : public FactEntry {
   handleRemovalFromIntersection(const FactSet &FSet, FactManager &FactMan,
 SourceLocation JoinLoc, LockErrorKind LEK,
 ThreadSafetyHandler &Handler) const override {
+if (LEK == LEK_LockedAtEndOfFunction || LEK == 
LEK_NotLockedAtEndOfFunction)
+  return;
+
 for (const auto &UnderlyingMutex : UnderlyingMutexes) {
   const auto *Entry = FSet.findLock(FactMan, UnderlyingMutex.Cap);
   if ((UnderlyingMutex.Kind == UCK_Acquired && Entry) ||
@@ -2224,7 +2227,7 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet 
&EntrySet,
   if (join(FactMan[*EntryIt], ExitFact,
EntryLEK != LEK_LockedSomeLoopIterations))
 *EntryIt = Fact;
-} else if (!ExitFact.managed()) {
+} else if (!ExitFact.managed() || EntryLEK == LEK_LockedAtEndOfFunction) {
   ExitFact.handleRemovalFromIntersection(ExitSet, FactMan, JoinLoc,
  EntryLEK, Handler);
 }
@@ -2236,7 +2239,8 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet 
&EntrySet,
 const FactEntry *ExitFact = ExitSet.findLock(FactMan, *EntryFact);
 
 if (!ExitFact) {
-  if (!EntryFact->managed() || ExitLEK == LEK_LockedSomeLoopIterations)
+  if (!EntryFact->managed() || ExitLEK == LEK_LockedSomeLoopIterations ||
+  ExitLEK == LEK_NotLockedAtEndOfFunction)
 EntryFact->handleRemovalFromIntersection(EntrySetOrig, FactMan, 
JoinLoc,
  ExitLEK, Handler);
   if (ExitLEK == LEK_LockedSomePredecessors)
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp 
b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index af9254508d8050..8477200456d985 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -6077,24 +6077,20 @@ namespace ReturnScopedLockable {
 class Object {
 public:
   MutexLock lock() EXCLUSIVE_LOCK_FUNCTION(mutex) {
-// TODO: False positive because scoped lock isn't destructed.
-return MutexLock(&mutex); // expected-note {{mutex acquired here}}
-  }   // expected-warning {{mutex 'mutex' is still 
held at the end of function}}
+return MutexLock(&mutex);
+  }
 
   ReaderMutexLock lockShared() SHARED_LOCK_FUNCTION(mutex) {
-// TODO: False positive because scoped lock isn't destructed.
-return ReaderMutexLock(&mutex); // expected-note {{mutex acquired here}}
-  } // expected-warning {{mutex 'mutex' is 
still held at the end of function}}
+return ReaderMutexLock(&mutex);
+  }
 
   MutexLock adopt() EXCLUSIVE_LOCKS_REQUIRED(mutex) {
-// TODO: False positive because scoped lock isn't destructed.
-return MutexLock(&mutex, true); // expected-note {{mutex acquired here}}
-  } // expected-warning {{mutex 'mutex' is 
still held at the end of function}}
+return MutexLock(&mutex, true);
+  }
 
   ReaderMutexLock adoptShared() SHARED_LOCKS_REQUIR

[clang] Thread Safety Analysis: Support passing scoped locks between functions with appropriate annotations (PR #110523)

[Malek Ben Slimane via cfe-commits]

https://github.com/malek203 created 
https://github.com/llvm/llvm-project/pull/110523

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.

>From f669df245c2661ad502c8f4eca2bc446ebc06606 Mon Sep 17 00:00:00 2001
From: Malek Ben Slimane 
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/ThreadSafetyAnalysis.rst   |  51 ++-
 .../clang/Analysis/Analyses/ThreadSafety.h|  35 ++
 clang/include/clang/Basic/Attr.td |   8 +-
 .../clang/Basic/DiagnosticSemaKinds.td|  15 +
 clang/lib/Analysis/ThreadSafety.cpp   | 173 -
 clang/lib/Sema/AnalysisBasedWarnings.cpp  |  39 ++
 clang/lib/Sema/SemaDeclAttr.cpp   |  40 ++
 .../SemaCXX/warn-thread-safety-analysis.cpp   | 366 ++
 .../SemaCXX/warn-thread-safety-parsing.cpp|  42 +-
 9 files changed, 740 insertions(+), 29 deletions(-)

diff --git a/clang/docs/ThreadSafetyAnalysis.rst 
b/clang/docs/ThreadSafetyAnalysis.rst
index cc4089b97b4923..6c513a52277cee 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

[clang] Thread Safety Analysis: Support passing scoped locks between functions with appropriate annotations (PR #110523)

[Malek Ben Slimane via cfe-commits]

https://github.com/malek203 updated 
https://github.com/llvm/llvm-project/pull/110523

>From b66684cb44abcf3b0a191303d1dc01e6aa0d8dec Mon Sep 17 00:00:00 2001
From: Malek Ben Slimane 
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/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   |  40 ++
 .../SemaCXX/warn-thread-safety-analysis.cpp   | 361 ++
 .../SemaCXX/warn-thread-safety-parsing.cpp|  42 +-
 9 files changed, 738 insertions(+), 30 deletions(-)

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
 

[clang] Thread Safety Analysis: Support passing scoped locks between functions with appropriate annotations (PR #110523)

[Malek Ben Slimane via cfe-commits]

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 
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 @

[clang] Thread Safety Analysis: Support passing scoped locks between functions with appropriate annotations (PR #110523)

[Malek Ben Slimane via cfe-commits]

@@ -1915,6 +1936,101 @@ void BuildLockset::handleCall(const Expr *Exp, const 
NamedDecl *D,
 }
   }
 
+  std::optional Args;
+  if (Exp) {
+if (const auto *CE = dyn_cast(Exp))
+  Args = CE->arguments();
+else if (const auto *CE = dyn_cast(Exp))
+  Args = CE->arguments();
+else
+  llvm_unreachable("Unknown call kind");

malek203 wrote:

`CXXMemberCallExpr` inherits from `CallExpr`, so it is already covered.

https://github.com/llvm/llvm-project/pull/110523
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support passing scoped locks between functions with appropriate annotations (PR #110523)

[Malek Ben Slimane via cfe-commits]

@@ -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, 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, DefaultIgnore;

malek203 wrote:

The `DefaultIgnore` setting aligns with the fact that the warnings 
`-Wthread-safety-analysis` and `-Wthread-safety-attributes` are not enabled by 
default.

https://github.com/llvm/llvm-project/pull/110523
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support passing scoped locks between functions with appropriate annotations (PR #110523)

[Malek Ben Slimane via cfe-commits]

https://github.com/malek203 updated 
https://github.com/llvm/llvm-project/pull/110523

>From 07b0285587df72ada43705641f06cdce4280c483 Mon Sep 17 00:00:00 2001
From: Malek Ben Slimane 
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   |  25 ++
 clang/docs/ThreadSafetyAnalysis.rst   |  50 ++-
 .../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, 774 insertions(+), 30 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index e511614fcf2451..e06b0bd31da234 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -336,6 +336,31 @@ Improvements to Clang's diagnostics
   local variables passed to function calls using the ``[[clang::musttail]]``
   attribute.
 
+- The `Thread Safety Analysis 
`_ 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"
+
+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), scope2(&mu2);
+  require(scope2); // Warning! Mutex managed by 'scope2' 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..a5c3f7937dc049 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,20 @@ 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), scope2(&mu2);
+require(scope2); // Warning! Mutex managed by 'scope2' 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 +235,13 @@ ACQUIRE(...), ACQUIRE_SHARED(...), RELEASE(...), 
RELEASE_SHARED(...), RELEASE_GE
 *Previously*: ``EXCLUSIVE_LOCK_FUNCTION``, ``SHARED_LOCK_FUNCTION``,
 ``UNLOCK_FUNCTION``
 

[clang] Thread Safety Analysis: Support passing scoped locks between functions with appropriate annotations (PR #110523)

[Malek Ben Slimane via cfe-commits]

@@ -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'

malek203 wrote:

That's right and I will fix it in the Thread Safety Analysis documentation too.

https://github.com/llvm/llvm-project/pull/110523
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Thread Safety Analysis: Support passing scoped locks between functions with appropriate annotations (PR #110523)

[Malek Ben Slimane via cfe-commits]

https://github.com/malek203 updated 
https://github.com/llvm/llvm-project/pull/110523

>From f5b5d4262318c4a2048c145aea923c1108ba65f2 Mon Sep 17 00:00:00 2001
From: Malek Ben Slimane 
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   |  25 ++
 clang/docs/ThreadSafetyAnalysis.rst   |  50 ++-
 .../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, 774 insertions(+), 30 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index e511614fcf2451..63154451a09cc1 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -336,6 +336,31 @@ Improvements to Clang's diagnostics
   local variables passed to function calls using the ``[[clang::musttail]]``
   attribute.
 
+- The :doc:`ThreadSafetyAnalysis` 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"
+
+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), scope2(&mu2);
+  require(scope2); // Warning! Mutex managed by 'scope2' 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..a5c3f7937dc049 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,20 @@ 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), scope2(&mu2);
+require(scope2); // Warning! Mutex managed by 'scope2' 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 +235,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 f

[clang] Thread Safety Analysis: Check managed capabilities of returned scoped capability (PR #131831)

[Malek Ben Slimane via cfe-commits]

https://github.com/malek203 updated 
https://github.com/llvm/llvm-project/pull/131831

>From 7157d4c42e534c5c8c06b564055aed1030b9f690 Mon Sep 17 00:00:00 2001
From: Malek Ben Slimane 
Date: Wed, 25 Sep 2024 15:21:08 +0200
Subject: [PATCH] Thread Safety Analysis: Check managed capabilities of
 returned scoped capability

Verify that the return value of type scoped lockable manages the
mutexes expected by the function annotations.
---
 .../clang/Analysis/Analyses/ThreadSafety.h| 16 +++-
 .../clang/Basic/DiagnosticSemaKinds.td|  4 +-
 clang/lib/Analysis/ThreadSafety.cpp   | 90 ++-
 clang/lib/Sema/AnalysisBasedWarnings.cpp  | 23 ++---
 .../SemaCXX/warn-thread-safety-analysis.cpp   | 75 +++-
 5 files changed, 165 insertions(+), 43 deletions(-)

diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafety.h 
b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
index 20b75c46593e0..210610f672933 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafety.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
@@ -243,10 +243,13 @@ class ThreadSafetyHandler {
   /// \param Kind -- The kind of the expected mutex.
   /// \param Expected -- The name of the expected mutex.
   /// \param Actual -- The name of the actual mutex.
+  /// \param ForParam -- Indicates whether the note applies to a function
+  /// parameter.
   virtual void handleUnmatchedUnderlyingMutexes(SourceLocation Loc,
 SourceLocation DLoc,
 Name ScopeName, StringRef Kind,
-Name Expected, Name Actual) {}
+Name Expected, Name Actual,
+bool ForParam) {}
 
   /// Warn when we get fewer underlying mutexes than expected.
   /// \param Loc -- The location of the call expression.
@@ -254,10 +257,13 @@ class ThreadSafetyHandler {
   /// \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 ForParam -- Indicates whether the note applies to a function
+  /// parameter.
   virtual void handleExpectMoreUnderlyingMutexes(SourceLocation Loc,
  SourceLocation DLoc,
  Name ScopeName, StringRef 
Kind,
- Name Expected) {}
+ Name Expected, bool ForParam) 
{
+  }
 
   /// Warn when we get more underlying mutexes than expected.
   /// \param Loc -- The location of the call expression.
@@ -265,11 +271,13 @@ class ThreadSafetyHandler {
   /// \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.
+  /// \param ForParam -- Indicates whether the note applies to a function
+  /// parameter.
   virtual void handleExpectFewerUnderlyingMutexes(SourceLocation Loc,
   SourceLocation DLoc,
   Name ScopeName,
-  StringRef Kind, Name Actual) 
{
-  }
+  StringRef Kind, Name Actual,
+  bool ForParam) {}
 
   /// Warn that L1 cannot be acquired before L2.
   virtual void handleLockAcquiredBefore(StringRef Kind, Name L1Name,
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 627bebb31fc8d..82d9cb02960dd 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4117,8 +4117,8 @@ def warn_expect_more_underlying_mutexes : Warning<
 def warn_expect_fewer_underlying_mutexes : Warning<
   "did not expect %0 '%2' to be managed by '%1'">,
   InGroup, DefaultIgnore;
-def note_managed_mismatch_here_for_param : Note<
-  "see attribute on parameter here">;
+def note_managed_mismatch_here : Note<
+  "see attribute on %select{function|parameter}0 here">;
 
 
 // Thread safety warnings negative capabilities
diff --git a/clang/lib/Analysis/ThreadSafety.cpp 
b/clang/lib/Analysis/ThreadSafety.cpp
index 6b5b49377fa08..e019ee9073efd 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1040,6 +1040,7 @@ class ThreadSafetyAnalyzer {
   std::vector BlockInfo;
 
   BeforeSet *GlobalBeforeSet;
+  CapExprSet ExpectedReturnedCapabilities;
 
 public:
   ThreadSafetyAnalyzer(ThreadSafetyHandler &H, BeforeSet* Bset)
@@ -2041,15 +2042,16 @@ void BuildLockset::handleCall(const Expr *Exp, const 
NamedDecl *D,
 if (!a.has_value()) {
   Analyzer->Handler.handleEx