https://github.com/ziqingluo-90 updated
https://github.com/llvm/llvm-project/pull/74020
>From 00083bb1573561361ff0782f425ebec2a5218f34 Mon Sep 17 00:00:00 2001
From: ziqingluo-90 <ziq...@udel.edu>
Date: Mon, 4 Dec 2023 14:37:10 -0800
Subject: [PATCH] Thread safety analysis: Fix a bug in handling temporary
constructors
Extends the lifetime of the map `ConstructedObjects` to be of the
whole CFG so that the map can connect temporary Ctor and Dtor in
different CFG blocks.
---
clang/lib/Analysis/ThreadSafety.cpp | 27 +++++++++----------
.../SemaCXX/warn-thread-safety-analysis.cpp | 8 ++++++
2 files changed, 21 insertions(+), 14 deletions(-)
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index 7fdf22c2f3919..b65d4dfab8dea 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1010,6 +1010,8 @@ class ThreadSafetyAnalyzer {
ThreadSafetyHandler &Handler;
const FunctionDecl *CurrentFunction;
LocalVariableMap LocalVarMap;
+ // Maps constructed objects to `this` placeholder prior to initialization.
+ llvm::SmallDenseMap<const Expr *, til::LiteralPtr *> ConstructedObjects;
FactManager FactMan;
std::vector<CFGBlockInfo> BlockInfo;
@@ -1530,7 +1532,6 @@ void ThreadSafetyAnalyzer::getEdgeLockset(FactSet& Result,
}
namespace {
-
/// We use this class to visit different types of expressions in
/// CFGBlocks, and build up the lockset.
/// An expression may cause us to add or remove locks from the lockset, or else
@@ -1543,8 +1544,6 @@ class BuildLockset : public
ConstStmtVisitor<BuildLockset> {
FactSet FSet;
// The fact set for the function on exit.
const FactSet &FunctionExitFSet;
- /// Maps constructed objects to `this` placeholder prior to initialization.
- llvm::SmallDenseMap<const Expr *, til::LiteralPtr *> ConstructedObjects;
LocalVariableMap::Context LVarCtx;
unsigned CtxIndex;
@@ -1808,7 +1807,7 @@ void BuildLockset::handleCall(const Expr *Exp, const
NamedDecl *D,
std::pair<til::LiteralPtr *, StringRef> Placeholder =
Analyzer->SxBuilder.createThisPlaceholder(Exp);
[[maybe_unused]] auto inserted =
- ConstructedObjects.insert({Exp, Placeholder.first});
+ Analyzer->ConstructedObjects.insert({Exp, Placeholder.first});
assert(inserted.second && "Are we visiting the same expression again?");
if (isa<CXXConstructExpr>(Exp))
Self = Placeholder.first;
@@ -2128,10 +2127,10 @@ void BuildLockset::VisitDeclStmt(const DeclStmt *S) {
E = EWC->getSubExpr()->IgnoreParens();
E = UnpackConstruction(E);
- if (auto Object = ConstructedObjects.find(E);
- Object != ConstructedObjects.end()) {
+ if (auto Object = Analyzer->ConstructedObjects.find(E);
+ Object != Analyzer->ConstructedObjects.end()) {
Object->second->setClangDecl(VD);
- ConstructedObjects.erase(Object);
+ Analyzer->ConstructedObjects.erase(Object);
}
}
}
@@ -2140,11 +2139,11 @@ void BuildLockset::VisitDeclStmt(const DeclStmt *S) {
void BuildLockset::VisitMaterializeTemporaryExpr(
const MaterializeTemporaryExpr *Exp) {
if (const ValueDecl *ExtD = Exp->getExtendingDecl()) {
- if (auto Object =
- ConstructedObjects.find(UnpackConstruction(Exp->getSubExpr()));
- Object != ConstructedObjects.end()) {
+ if (auto Object = Analyzer->ConstructedObjects.find(
+ UnpackConstruction(Exp->getSubExpr()));
+ Object != Analyzer->ConstructedObjects.end()) {
Object->second->setClangDecl(ExtD);
- ConstructedObjects.erase(Object);
+ Analyzer->ConstructedObjects.erase(Object);
}
}
}
@@ -2487,15 +2486,15 @@ void
ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
// Clean up constructed object even if there are no attributes to
// keep the number of objects in limbo as small as possible.
- if (auto Object = LocksetBuilder.ConstructedObjects.find(
+ if (auto Object = LocksetBuilder.Analyzer->ConstructedObjects.find(
TD.getBindTemporaryExpr()->getSubExpr());
- Object != LocksetBuilder.ConstructedObjects.end()) {
+ Object != LocksetBuilder.Analyzer->ConstructedObjects.end()) {
const auto *DD = TD.getDestructorDecl(AC.getASTContext());
if (DD->hasAttrs())
// TODO: the location here isn't quite correct.
LocksetBuilder.handleCall(nullptr, DD, Object->second,
TD.getBindTemporaryExpr()->getEndLoc());
- LocksetBuilder.ConstructedObjects.erase(Object);
+ LocksetBuilder.Analyzer->ConstructedObjects.erase(Object);
}
break;
}
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index 205cfa284f6c9..dfb966d3b5902 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -1702,6 +1702,8 @@ struct TestScopedLockable {
bool getBool();
+ bool lock2Bool(MutexLock);
+
void foo1() {
MutexLock mulock(&mu1);
a = 5;
@@ -1718,6 +1720,12 @@ struct TestScopedLockable {
MutexLock{&mu1}, a = 5;
}
+ void temporary_cfg(int x) {
+ // test the case where a pair of temporary Ctor and Dtor is in different
CFG blocks
+ lock2Bool(MutexLock{&mu1}) || x;
+ MutexLock{&mu1}; // no-warn
+ }
+
void lifetime_extension() {
const MutexLock &mulock = MutexLock(&mu1);
a = 5;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
