125 lines
5.4 KiB
Diff
125 lines
5.4 KiB
Diff
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;
|
|
}
|
|
|