Thread Safety Analysis: Support passing scoped locks between functions with appropriate annotations (#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.

1 files changed, 165 insertions, 10 deletions

diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index 5577f45..bfaf1a0 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 @@ public:
   };
 
 private:
+  const FactEntryKind Kind : 8;
+
   /// Exclusive or shared.
   LockKind LKind : 8;
 
@@ -122,13 +126,14 @@ private:
   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 @@ public:
                                 !Cp, LK_Exclusive, UnlockLoc));
     }
   }
+
+  static bool classof(const FactEntry *A) {
+    return A->getFactEntryKind() == Lockable;
+  }
 };
 
 class ScopedLockableFactEntry : public FactEntry {
@@ -903,8 +912,16 @@ private:
   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 @@ public:
       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) {