commit cf8e189a99f988398a48148b9ea7901948665ab0

Author: Timm B├Ąder <tbaeder@redhat.com>

Date:   Wed Sep 6 12:19:20 2023 +0200

    [clang][TSA] Thread safety cleanup functions

    Consider cleanup functions in thread safety analysis.

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

diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h

index 9d28325c1ea6..13e37ac2b56b 100644

--- a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h

+++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h

@@ -361,7 +361,7 @@ public:

     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 3107d035254d..3e6ceb7d54c4 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 b8286cef396c..63cc66852a9e 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 355616b73d96..642ea88ec3c9 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@-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;

 }