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.

Author: aaronpuchert

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,

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"