mirrors/llvm-project
0
0
Fork 0
mirror of https://github.com/llvm/llvm-project.git synced 2025-04-26 03:16:07 +00:00
Code Issues Projects Releases Wiki Activity

Thread safety analysis: Fix a bug in handling temporary constructors (#74020)

Browse Source 
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.
This commit is contained in:
Ziqing Luo 2023-12-08 10:29:37 -08:00 committed by GitHub
parent 6f9cb9a75c
commit a341e177ce
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 21 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
clang/lib/Analysis/ThreadSafety.cpp
View File

@ -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;
@ -1543,8 +1545,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 +1808,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 +2128,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 +2140,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 +2487,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 = ConstructedObjects.find(
TD.getBindTemporaryExpr()->getSubExpr());
Object != LocksetBuilder.ConstructedObjects.end()) {
Object != 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);
ConstructedObjects.erase(Object);
}
break;
}

8
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
View File

@ -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;