[clang] cf8e189 - [clang][TSA] Thread safety cleanup functions

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2023-09-19T16:00:33+02:00
New Revision: cf8e189a99f988398a48148b9ea7901948665ab0

URL: https://github.com/llvm/llvm-project/commit/cf8e189a99f988398a48148b9ea7901948665ab0
DIFF: https://github.com/llvm/llvm-project/commit/cf8e189a99f988398a48148b9ea7901948665ab0.diff

LOG: [clang][TSA] Thread safety cleanup functions

Consider cleanup functions in thread safety analysis.

Differential Revision: https://reviews.llvm.org/D152504

Added: 
    

Modified: 
    clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
    clang/lib/Analysis/ThreadSafety.cpp
    clang/lib/Analysis/ThreadSafetyCommon.cpp
    clang/test/Sema/warn-thread-safety-analysis.c

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
index 9d28325c1ea67af..13e37ac2b56b649 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
@@ -361,7 +361,7 @@ class SExprBuilder {
     unsigned NumArgs = 0;
 
     // Function arguments
-    const Expr *const *FunArgs = nullptr;
+    llvm::PointerUnion<const Expr *const *, til::SExpr *> FunArgs = nullptr;
 
     // is Self referred to with -> or .?
     bool SelfArrow = false;

diff  --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index 3107d035254dde6..3e6ceb7d54c427a 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1773,7 +1773,8 @@ void BuildLockset::checkPtAccess(const Expr *Exp, AccessKind AK,
 ///
 /// \param Exp   The call expression.
 /// \param D     The callee declaration.
-/// \param Self  If \p Exp = nullptr, the implicit this argument.
+/// \param Self  If \p Exp = nullptr, the implicit this argument or the argument
+///              of an implicitly called cleanup function.
 /// \param Loc   If \p Exp = nullptr, the location.
 void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
                               til::LiteralPtr *Self, SourceLocation Loc) {
@@ -2417,6 +2418,15 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
                                     AD.getTriggerStmt()->getEndLoc());
           break;
         }
+
+        case CFGElement::CleanupFunction: {
+          const CFGCleanupFunction &CF = BI.castAs<CFGCleanupFunction>();
+          LocksetBuilder.handleCall(/*Exp=*/nullptr, CF.getFunctionDecl(),
+                                    SxBuilder.createVariable(CF.getVarDecl()),
+                                    CF.getVarDecl()->getLocation());
+          break;
+        }
+
         case CFGElement::TemporaryDtor: {
           auto TD = BI.castAs<CFGTemporaryDtor>();
 

diff  --git a/clang/lib/Analysis/ThreadSafetyCommon.cpp b/clang/lib/Analysis/ThreadSafetyCommon.cpp
index b8286cef396c060..63cc66852a9eb77 100644
--- a/clang/lib/Analysis/ThreadSafetyCommon.cpp
+++ b/clang/lib/Analysis/ThreadSafetyCommon.cpp
@@ -110,7 +110,8 @@ static StringRef ClassifyDiagnostic(QualType VDT) {
 /// \param D       The declaration to which the attribute is attached.
 /// \param DeclExp An expression involving the Decl to which the attribute
 ///                is attached.  E.g. the call to a function.
-/// \param Self    S-expression to substitute for a \ref CXXThisExpr.
+/// \param Self    S-expression to substitute for a \ref CXXThisExpr in a call,
+///                or argument to a cleanup function.
 CapabilityExpr SExprBuilder::translateAttrExpr(const Expr *AttrExp,
                                                const NamedDecl *D,
                                                const Expr *DeclExp,
@@ -144,7 +145,11 @@ CapabilityExpr SExprBuilder::translateAttrExpr(const Expr *AttrExp,
 
   if (Self) {
     assert(!Ctx.SelfArg && "Ambiguous self argument");
-    Ctx.SelfArg = Self;
+    assert(isa<FunctionDecl>(D) && "Self argument requires function");
+    if (isa<CXXMethodDecl>(D))
+      Ctx.SelfArg = Self;
+    else
+      Ctx.FunArgs = Self;
 
     // If the attribute has no arguments, then assume the argument is "this".
     if (!AttrExp)
@@ -312,8 +317,14 @@ til::SExpr *SExprBuilder::translateDeclRefExpr(const DeclRefExpr *DRE,
               ? (cast<FunctionDecl>(D)->getCanonicalDecl() == Canonical)
               : (cast<ObjCMethodDecl>(D)->getCanonicalDecl() == Canonical)) {
         // Substitute call arguments for references to function parameters
-        assert(I < Ctx->NumArgs);
-        return translate(Ctx->FunArgs[I], Ctx->Prev);
+        if (const Expr *const *FunArgs =
+                Ctx->FunArgs.dyn_cast<const Expr *const *>()) {
+          assert(I < Ctx->NumArgs);
+          return translate(FunArgs[I], Ctx->Prev);
+        }
+
+        assert(I == 0);
+        return Ctx->FunArgs.get<til::SExpr *>();
       }
     }
     // Map the param back to the param of the original function declaration

diff  --git a/clang/test/Sema/warn-thread-safety-analysis.c b/clang/test/Sema/warn-thread-safety-analysis.c
index 355616b73d967ec..642ea88ec3c96f7 100644
--- a/clang/test/Sema/warn-thread-safety-analysis.c
+++ b/clang/test/Sema/warn-thread-safety-analysis.c
@@ -72,6 +72,8 @@ int get_value(int *p) SHARED_LOCKS_REQUIRED(foo_.mu_){
   return *p;
 }
 
+void unlock_scope(struct Mutex *const *mu) __attribute__((release_capability(**mu)));
+
 int main(void) {
 
   Foo_fun1(1); // expected-warning{{calling function 'Foo_fun1' requires holding mutex 'mu2'}} \
@@ -127,6 +129,13 @@ int main(void) {
                                 // expected-note at -1{{mutex released here}}
   mutex_shared_unlock(&mu1);    // expected-warning {{releasing mutex 'mu1' that was not held}}
 
+  /// Cleanup functions
+  {
+    struct Mutex* const __attribute__((cleanup(unlock_scope))) scope = &mu1;
+    mutex_exclusive_lock(scope);  // Note that we have to lock through scope, because no alias analysis!
+    // Cleanup happens automatically -> no warning.
+  }
+
   return 0;
 }