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@-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; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
