[clang] 1b1ed4f - [LifetimeSafety] Overhaul CFG and analysis to also work with trivially destructed temporary objects (#177985)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 20 09:00:10 PST 2026
Author: Abhinav Pradeep
Date: 2026-02-20T18:00:05+01:00
New Revision: 1b1ed4f25adb24210b09fe353667d68bba910062
URL: https://github.com/llvm/llvm-project/commit/1b1ed4f25adb24210b09fe353667d68bba910062
DIFF: https://github.com/llvm/llvm-project/commit/1b1ed4f25adb24210b09fe353667d68bba910062.diff
LOG: [LifetimeSafety] Overhaul CFG and analysis to also work with trivially destructed temporary objects (#177985)
Change summary:
Modification to CFG:
1. Added `CFGFullExprCleanup` which has a pointer to `BumpVector<const
MaterializeTemporaryExpr *>` to track all MTE that **might** (in the
sense that we take union on branches) be spawned by an
`ExprWithCleanups`
2. Modified logic in `CFGBuilder` to appropriately insert this marker.
It inserts the marker primarily via `CFGBuilder::VisitExprWithCleanups`,
and also `CFGBuilder::addInitializer` and `CFGBuilder::VisitDeclSubExpr`
as these bypass visiting the `ExprWithCleanups`. The bump vector is
allocated appropriately using the bump allocator of the CFG to respect
its lifetime rules.
3. Visiting to track the temporaries is done in
`CFGBuilder::VisitForTemporaries`. Behaviour is modulated via the
`BuildOpts` so as to enable tracking only when necessary. The
encountered non-lifetime extended MTE are stored in a small vector
member of `TempDtorContext`.
Modification to analysis:
1. Logic to issue the loans remains mostly the same, with the caveat
that we now **always** issue a loan to any MTE that expires at the end
of full expression.
2. Introduced `handleFullExprCleanup` to replace `handleTemporaryDtor`.
It loops through all active loans, expiring them if the path matches an
MTE in the `CFGFullExprCleanup` node's `ExpiringMTEs`.
Fixes:
#175893
#178159
Added:
clang/test/Analysis/exprwithcleanups-lifetime-marker-cfg.cpp
Modified:
clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
clang/include/clang/Analysis/CFG.h
clang/lib/Analysis/CFG.cpp
clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
clang/lib/Analysis/PathDiagnostic.cpp
clang/lib/Sema/AnalysisBasedWarnings.cpp
clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
clang/test/Sema/warn-lifetime-safety.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
index fb7d5ad91db79..5f4ddcb39f84c 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
@@ -60,8 +60,10 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> {
void handleAssignment(const Expr *LHSExpr, const Expr *RHSExpr);
void handleCXXCtorInitializer(const CXXCtorInitializer *CII);
+
void handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds);
- void handleTemporaryDtor(const CFGTemporaryDtor &TemporaryDtor);
+
+ void handleFullExprCleanup(const CFGFullExprCleanup &FullExprCleanup);
void handleExitBlock();
diff --git a/clang/include/clang/Analysis/CFG.h b/clang/include/clang/Analysis/CFG.h
index e675935d9230a..72b7ac013ccc2 100644
--- a/clang/include/clang/Analysis/CFG.h
+++ b/clang/include/clang/Analysis/CFG.h
@@ -62,6 +62,7 @@ class CFGElement {
NewAllocator,
LifetimeEnds,
LoopExit,
+ FullExprCleanup,
// stmt kind
Statement,
Constructor,
@@ -313,6 +314,32 @@ class CFGLifetimeEnds : public CFGElement {
}
};
+class CFGFullExprCleanup : public CFGElement {
+
+public:
+ using MTEVecTy = BumpVector<const MaterializeTemporaryExpr *>;
+ explicit CFGFullExprCleanup(const MTEVecTy *MTEs)
+ : CFGElement(FullExprCleanup, MTEs, nullptr) {}
+
+ ArrayRef<const MaterializeTemporaryExpr *> getExpiringMTEs() const {
+ const MTEVecTy *ExpiringMTEs =
+ static_cast<const MTEVecTy *>(Data1.getPointer());
+ if (!ExpiringMTEs)
+ return {};
+ return ArrayRef<const MaterializeTemporaryExpr *>(ExpiringMTEs->begin(),
+ ExpiringMTEs->end());
+ }
+
+private:
+ friend class CFGElement;
+
+ CFGFullExprCleanup() = default;
+
+ static bool isKind(const CFGElement &elem) {
+ return elem.getKind() == FullExprCleanup;
+ }
+};
+
/// Represents beginning of a scope implicitly generated
/// by the compiler on encountering a CompoundStmt
class CFGScopeBegin : public CFGElement {
@@ -1183,6 +1210,11 @@ class CFGBlock {
Elements.push_back(CFGLifetimeEnds(VD, S), C);
}
+ void appendFullExprCleanup(BumpVector<const MaterializeTemporaryExpr *> *BV,
+ BumpVectorContext &C) {
+ Elements.push_back(CFGFullExprCleanup(BV), C);
+ }
+
void appendLoopExit(const Stmt *LoopStmt, BumpVectorContext &C) {
Elements.push_back(CFGLoopExit(LoopStmt), C);
}
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 10a7c5b2c8344..966399060bbc2 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -54,7 +54,9 @@
#include "llvm/Support/SaveAndRestore.h"
#include "llvm/Support/TimeProfiler.h"
#include "llvm/Support/raw_ostream.h"
+#include <algorithm>
#include <cassert>
+#include <cstddef>
#include <memory>
#include <optional>
#include <string>
@@ -699,7 +701,6 @@ class CFGBuilder {
TempDtorContext() = default;
TempDtorContext(TryResult KnownExecuted)
: IsConditional(true), KnownExecuted(KnownExecuted) {}
-
/// Returns whether we need to start a new branch for a temporary destructor
/// call. This is the case when the temporary destructor is
/// conditionally executed, and it is the first one we encounter while
@@ -717,7 +718,12 @@ class CFGBuilder {
TerminatorExpr = E;
}
+ void track(const MaterializeTemporaryExpr *MTE) {
+ CollectedMTEs.push_back(MTE);
+ }
+
const bool IsConditional = false;
+ SmallVector<const MaterializeTemporaryExpr *, 5> CollectedMTEs;
const TryResult KnownExecuted = true;
CFGBlock *Succ = nullptr;
CXXBindTemporaryExpr *TerminatorExpr = nullptr;
@@ -725,22 +731,23 @@ class CFGBuilder {
// Visitors to walk an AST and generate destructors of temporaries in
// full expression.
- CFGBlock *VisitForTemporaryDtors(Stmt *E, bool ExternallyDestructed,
- TempDtorContext &Context);
- CFGBlock *VisitChildrenForTemporaryDtors(Stmt *E, bool ExternallyDestructed,
- TempDtorContext &Context);
- CFGBlock *VisitBinaryOperatorForTemporaryDtors(BinaryOperator *E,
- bool ExternallyDestructed,
- TempDtorContext &Context);
+ CFGBlock *VisitForTemporaries(Stmt *E, bool ExternallyDestructed,
+ TempDtorContext &Context);
+ CFGBlock *VisitChildrenForTemporaries(Stmt *E, bool ExternallyDestructed,
+ TempDtorContext &Context);
+ CFGBlock *VisitBinaryOperatorForTemporaries(BinaryOperator *E,
+ bool ExternallyDestructed,
+ TempDtorContext &Context);
CFGBlock *VisitCXXOperatorCallExprForTemporaryDtors(CXXOperatorCallExpr *E,
TempDtorContext &Context);
CFGBlock *VisitCXXBindTemporaryExprForTemporaryDtors(
CXXBindTemporaryExpr *E, bool ExternallyDestructed, TempDtorContext &Context);
- CFGBlock *VisitConditionalOperatorForTemporaryDtors(
- AbstractConditionalOperator *E, bool ExternallyDestructed,
- TempDtorContext &Context);
- void InsertTempDtorDecisionBlock(const TempDtorContext &Context,
- CFGBlock *FalseSucc = nullptr);
+ CFGBlock *
+ VisitConditionalOperatorForTemporaries(AbstractConditionalOperator *E,
+ bool ExternallyDestructed,
+ TempDtorContext &Context);
+ void InsertTempDecisionBlock(const TempDtorContext &Context,
+ CFGBlock *FalseSucc = nullptr);
// NYS == Not Yet Supported
CFGBlock *NYS() {
@@ -806,6 +813,7 @@ class CFGBuilder {
void addScopeChangesHandling(LocalScope::const_iterator SrcPos,
LocalScope::const_iterator DstPos,
Stmt *S);
+ void addFullExprCleanupMarker(TempDtorContext &Context);
CFGBlock *createScopeChangesHandlingBlock(LocalScope::const_iterator SrcPos,
CFGBlock *SrcBlk,
LocalScope::const_iterator DstPost,
@@ -1824,11 +1832,14 @@ CFGBlock *CFGBuilder::addInitializer(CXXCtorInitializer *I) {
if (Init) {
HasTemporaries = isa<ExprWithCleanups>(Init);
- if (BuildOpts.AddTemporaryDtors && HasTemporaries) {
+ if (HasTemporaries &&
+ (BuildOpts.AddTemporaryDtors || BuildOpts.AddLifetime)) {
// Generate destructors for temporaries in initialization expression.
TempDtorContext Context;
- VisitForTemporaryDtors(cast<ExprWithCleanups>(Init)->getSubExpr(),
- /*ExternallyDestructed=*/false, Context);
+ VisitForTemporaries(cast<ExprWithCleanups>(Init)->getSubExpr(),
+ /*ExternallyDestructed=*/false, Context);
+
+ addFullExprCleanupMarker(Context);
}
}
@@ -2076,6 +2087,21 @@ void CFGBuilder::addScopeChangesHandling(LocalScope::const_iterator SrcPos,
addAutomaticObjHandling(SrcPos, BasePos, S);
}
+void CFGBuilder::addFullExprCleanupMarker(TempDtorContext &Context) {
+ CFGFullExprCleanup::MTEVecTy *ExpiringMTEs = nullptr;
+ BumpVectorContext &BVC = cfg->getBumpVectorContext();
+
+ size_t NumCollected = Context.CollectedMTEs.size();
+ if (NumCollected > 0) {
+ autoCreateBlock();
+ ExpiringMTEs = new (cfg->getAllocator())
+ CFGFullExprCleanup::MTEVecTy(BVC, NumCollected);
+ for (const MaterializeTemporaryExpr *MTE : Context.CollectedMTEs)
+ ExpiringMTEs->push_back(MTE, BVC);
+ Block->appendFullExprCleanup(ExpiringMTEs, BVC);
+ }
+}
+
/// createScopeChangesHandlingBlock - Creates a block with cfgElements
/// corresponding to changing the scope from the source scope of the GotoStmt,
/// to destination scope. Add destructor, lifetime and cfgScopeEnd
@@ -3134,11 +3160,14 @@ CFGBlock *CFGBuilder::VisitDeclSubExpr(DeclStmt *DS) {
if (Init) {
HasTemporaries = isa<ExprWithCleanups>(Init);
- if (BuildOpts.AddTemporaryDtors && HasTemporaries) {
+ if (HasTemporaries &&
+ (BuildOpts.AddTemporaryDtors || BuildOpts.AddLifetime)) {
// Generate destructors for temporaries in initialization expression.
TempDtorContext Context;
- VisitForTemporaryDtors(cast<ExprWithCleanups>(Init)->getSubExpr(),
- /*ExternallyDestructed=*/true, Context);
+ VisitForTemporaries(cast<ExprWithCleanups>(Init)->getSubExpr(),
+ /*ExternallyDestructed=*/true, Context);
+
+ addFullExprCleanupMarker(Context);
}
}
@@ -4956,12 +4985,15 @@ CFGBlock *CFGBuilder::VisitCXXForRangeStmt(CXXForRangeStmt *S) {
}
CFGBlock *CFGBuilder::VisitExprWithCleanups(ExprWithCleanups *E,
- AddStmtChoice asc, bool ExternallyDestructed) {
- if (BuildOpts.AddTemporaryDtors) {
+ AddStmtChoice asc,
+ bool ExternallyDestructed) {
+ if (BuildOpts.AddTemporaryDtors || BuildOpts.AddLifetime) {
// If adding implicit destructors visit the full expression for adding
// destructors of temporaries.
TempDtorContext Context;
- VisitForTemporaryDtors(E->getSubExpr(), ExternallyDestructed, Context);
+ VisitForTemporaries(E->getSubExpr(), ExternallyDestructed, Context);
+
+ addFullExprCleanupMarker(Context);
// Full expression has to be added as CFGStmt so it will be sequenced
// before destructors of it's temporaries.
@@ -5098,9 +5130,8 @@ CFGBlock *CFGBuilder::VisitIndirectGotoStmt(IndirectGotoStmt *I) {
return addStmt(I->getTarget());
}
-CFGBlock *CFGBuilder::VisitForTemporaryDtors(Stmt *E, bool ExternallyDestructed,
- TempDtorContext &Context) {
- assert(BuildOpts.AddImplicitDtors && BuildOpts.AddTemporaryDtors);
+CFGBlock *CFGBuilder::VisitForTemporaries(Stmt *E, bool ExternallyDestructed,
+ TempDtorContext &Context) {
tryAgain:
if (!E) {
@@ -5109,15 +5140,14 @@ CFGBlock *CFGBuilder::VisitForTemporaryDtors(Stmt *E, bool ExternallyDestructed,
}
switch (E->getStmtClass()) {
default:
- return VisitChildrenForTemporaryDtors(E, false, Context);
+ return VisitChildrenForTemporaries(E, false, Context);
case Stmt::InitListExprClass:
- return VisitChildrenForTemporaryDtors(E, ExternallyDestructed, Context);
+ return VisitChildrenForTemporaries(E, ExternallyDestructed, Context);
case Stmt::BinaryOperatorClass:
- return VisitBinaryOperatorForTemporaryDtors(cast<BinaryOperator>(E),
- ExternallyDestructed,
- Context);
+ return VisitBinaryOperatorForTemporaries(cast<BinaryOperator>(E),
+ ExternallyDestructed, Context);
case Stmt::CXXOperatorCallExprClass:
return VisitCXXOperatorCallExprForTemporaryDtors(
@@ -5129,7 +5159,7 @@ CFGBlock *CFGBuilder::VisitForTemporaryDtors(Stmt *E, bool ExternallyDestructed,
case Stmt::BinaryConditionalOperatorClass:
case Stmt::ConditionalOperatorClass:
- return VisitConditionalOperatorForTemporaryDtors(
+ return VisitConditionalOperatorForTemporaries(
cast<AbstractConditionalOperator>(E), ExternallyDestructed, Context);
case Stmt::ImplicitCastExprClass:
@@ -5153,6 +5183,8 @@ CFGBlock *CFGBuilder::VisitForTemporaryDtors(Stmt *E, bool ExternallyDestructed,
case Stmt::MaterializeTemporaryExprClass: {
const MaterializeTemporaryExpr* MTE = cast<MaterializeTemporaryExpr>(E);
ExternallyDestructed = (MTE->getStorageDuration() != SD_FullExpression);
+ if (BuildOpts.AddLifetime && !ExternallyDestructed)
+ Context.track(MTE);
SmallVector<const Expr *, 2> CommaLHSs;
SmallVector<SubobjectAdjustment, 2> Adjustments;
// Find the expression whose lifetime needs to be extended.
@@ -5162,8 +5194,8 @@ CFGBlock *CFGBuilder::VisitForTemporaryDtors(Stmt *E, bool ExternallyDestructed,
->skipRValueSubobjectAdjustments(CommaLHSs, Adjustments));
// Visit the skipped comma operator left-hand sides for other temporaries.
for (const Expr *CommaLHS : CommaLHSs) {
- VisitForTemporaryDtors(const_cast<Expr *>(CommaLHS),
- /*ExternallyDestructed=*/false, Context);
+ VisitForTemporaries(const_cast<Expr *>(CommaLHS),
+ /*ExternallyDestructed=*/false, Context);
}
goto tryAgain;
}
@@ -5180,7 +5212,7 @@ CFGBlock *CFGBuilder::VisitForTemporaryDtors(Stmt *E, bool ExternallyDestructed,
CFGBlock *B = Block;
for (Expr *Init : LE->capture_inits()) {
if (Init) {
- if (CFGBlock *R = VisitForTemporaryDtors(
+ if (CFGBlock *R = VisitForTemporaries(
Init, /*ExternallyDestructed=*/true, Context))
B = R;
}
@@ -5203,39 +5235,41 @@ CFGBlock *CFGBuilder::VisitForTemporaryDtors(Stmt *E, bool ExternallyDestructed,
}
}
-CFGBlock *CFGBuilder::VisitChildrenForTemporaryDtors(Stmt *E,
- bool ExternallyDestructed,
- TempDtorContext &Context) {
+CFGBlock *CFGBuilder::VisitChildrenForTemporaries(Stmt *E,
+ bool ExternallyDestructed,
+ TempDtorContext &Context) {
if (isa<LambdaExpr>(E)) {
// Do not visit the children of lambdas; they have their own CFGs.
return Block;
}
- // When visiting children for destructors we want to visit them in reverse
- // order that they will appear in the CFG. Because the CFG is built
- // bottom-up, this means we visit them in their natural order, which
+ // When visiting children for destructors or lifetime markers we want to visit
+ // them in reverse order that they will appear in the CFG. Because the CFG is
+ // built bottom-up, this means we visit them in their natural order, which
// reverses them in the CFG.
CFGBlock *B = Block;
for (Stmt *Child : E->children())
if (Child)
- if (CFGBlock *R = VisitForTemporaryDtors(Child, ExternallyDestructed, Context))
+ if (CFGBlock *R =
+ VisitForTemporaries(Child, ExternallyDestructed, Context))
B = R;
return B;
}
-CFGBlock *CFGBuilder::VisitBinaryOperatorForTemporaryDtors(
+CFGBlock *CFGBuilder::VisitBinaryOperatorForTemporaries(
BinaryOperator *E, bool ExternallyDestructed, TempDtorContext &Context) {
if (E->isCommaOp()) {
// For the comma operator, the LHS expression is evaluated before the RHS
// expression, so prepend temporary destructors for the LHS first.
- CFGBlock *LHSBlock = VisitForTemporaryDtors(E->getLHS(), false, Context);
- CFGBlock *RHSBlock = VisitForTemporaryDtors(E->getRHS(), ExternallyDestructed, Context);
+ CFGBlock *LHSBlock = VisitForTemporaries(E->getLHS(), false, Context);
+ CFGBlock *RHSBlock =
+ VisitForTemporaries(E->getRHS(), ExternallyDestructed, Context);
return RHSBlock ? RHSBlock : LHSBlock;
}
if (E->isLogicalOp()) {
- VisitForTemporaryDtors(E->getLHS(), false, Context);
+ VisitForTemporaries(E->getLHS(), false, Context);
TryResult RHSExecuted = tryEvaluateBool(E->getLHS());
if (RHSExecuted.isKnown() && E->getOpcode() == BO_LOr)
RHSExecuted.negate();
@@ -5245,8 +5279,11 @@ CFGBlock *CFGBuilder::VisitBinaryOperatorForTemporaryDtors(
// constructor call.
TempDtorContext RHSContext(
bothKnownTrue(Context.KnownExecuted, RHSExecuted));
- VisitForTemporaryDtors(E->getRHS(), false, RHSContext);
- InsertTempDtorDecisionBlock(RHSContext);
+ VisitForTemporaries(E->getRHS(), false, RHSContext);
+ InsertTempDecisionBlock(RHSContext);
+
+ if (BuildOpts.AddLifetime)
+ Context.CollectedMTEs.append(RHSContext.CollectedMTEs);
return Block;
}
@@ -5254,13 +5291,13 @@ CFGBlock *CFGBuilder::VisitBinaryOperatorForTemporaryDtors(
if (E->isAssignmentOp()) {
// For assignment operators, the RHS expression is evaluated before the LHS
// expression, so prepend temporary destructors for the RHS first.
- CFGBlock *RHSBlock = VisitForTemporaryDtors(E->getRHS(), false, Context);
- CFGBlock *LHSBlock = VisitForTemporaryDtors(E->getLHS(), false, Context);
+ CFGBlock *RHSBlock = VisitForTemporaries(E->getRHS(), false, Context);
+ CFGBlock *LHSBlock = VisitForTemporaries(E->getLHS(), false, Context);
return LHSBlock ? LHSBlock : RHSBlock;
}
// Any other operator is visited normally.
- return VisitChildrenForTemporaryDtors(E, ExternallyDestructed, Context);
+ return VisitChildrenForTemporaries(E, ExternallyDestructed, Context);
}
CFGBlock *CFGBuilder::VisitCXXOperatorCallExprForTemporaryDtors(
@@ -5268,19 +5305,20 @@ CFGBlock *CFGBuilder::VisitCXXOperatorCallExprForTemporaryDtors(
if (E->isAssignmentOp()) {
// For assignment operators, the RHS expression is evaluated before the LHS
// expression, so prepend temporary destructors for the RHS first.
- CFGBlock *RHSBlock = VisitForTemporaryDtors(E->getArg(1), false, Context);
- CFGBlock *LHSBlock = VisitForTemporaryDtors(E->getArg(0), false, Context);
+ CFGBlock *RHSBlock = VisitForTemporaries(E->getArg(1), false, Context);
+ CFGBlock *LHSBlock = VisitForTemporaries(E->getArg(0), false, Context);
return LHSBlock ? LHSBlock : RHSBlock;
}
- return VisitChildrenForTemporaryDtors(E, false, Context);
+ return VisitChildrenForTemporaries(E, false, Context);
}
CFGBlock *CFGBuilder::VisitCXXBindTemporaryExprForTemporaryDtors(
CXXBindTemporaryExpr *E, bool ExternallyDestructed, TempDtorContext &Context) {
// First add destructors for temporaries in subexpression.
// Because VisitCXXBindTemporaryExpr calls setDestructed:
- CFGBlock *B = VisitForTemporaryDtors(E->getSubExpr(), true, Context);
- if (!ExternallyDestructed) {
+ CFGBlock *B = VisitForTemporaries(E->getSubExpr(), true, Context);
+ if (!ExternallyDestructed && BuildOpts.AddImplicitDtors &&
+ BuildOpts.AddTemporaryDtors) {
// If lifetime of temporary is not prolonged (by assigning to constant
// reference) add destructor for it.
@@ -5305,14 +5343,13 @@ CFGBlock *CFGBuilder::VisitCXXBindTemporaryExprForTemporaryDtors(
Context.setDecisionPoint(Succ, E);
}
appendTemporaryDtor(Block, E);
-
B = Block;
}
return B;
}
-void CFGBuilder::InsertTempDtorDecisionBlock(const TempDtorContext &Context,
- CFGBlock *FalseSucc) {
+void CFGBuilder::InsertTempDecisionBlock(const TempDtorContext &Context,
+ CFGBlock *FalseSucc) {
if (!Context.TerminatorExpr) {
// If no temporary was found, we do not need to insert a decision point.
return;
@@ -5327,10 +5364,10 @@ void CFGBuilder::InsertTempDtorDecisionBlock(const TempDtorContext &Context,
Block = Decision;
}
-CFGBlock *CFGBuilder::VisitConditionalOperatorForTemporaryDtors(
+CFGBlock *CFGBuilder::VisitConditionalOperatorForTemporaries(
AbstractConditionalOperator *E, bool ExternallyDestructed,
TempDtorContext &Context) {
- VisitForTemporaryDtors(E->getCond(), false, Context);
+ VisitForTemporaries(E->getCond(), false, Context);
CFGBlock *ConditionBlock = Block;
CFGBlock *ConditionSucc = Succ;
TryResult ConditionVal = tryEvaluateBool(E->getCond());
@@ -5339,23 +5376,28 @@ CFGBlock *CFGBuilder::VisitConditionalOperatorForTemporaryDtors(
TempDtorContext TrueContext(
bothKnownTrue(Context.KnownExecuted, ConditionVal));
- VisitForTemporaryDtors(E->getTrueExpr(), ExternallyDestructed, TrueContext);
+ VisitForTemporaries(E->getTrueExpr(), ExternallyDestructed, TrueContext);
CFGBlock *TrueBlock = Block;
Block = ConditionBlock;
Succ = ConditionSucc;
TempDtorContext FalseContext(
bothKnownTrue(Context.KnownExecuted, NegatedVal));
- VisitForTemporaryDtors(E->getFalseExpr(), ExternallyDestructed, FalseContext);
+ VisitForTemporaries(E->getFalseExpr(), ExternallyDestructed, FalseContext);
if (TrueContext.TerminatorExpr && FalseContext.TerminatorExpr) {
- InsertTempDtorDecisionBlock(FalseContext, TrueBlock);
+ InsertTempDecisionBlock(FalseContext, TrueBlock);
} else if (TrueContext.TerminatorExpr) {
Block = TrueBlock;
- InsertTempDtorDecisionBlock(TrueContext);
+ InsertTempDecisionBlock(TrueContext);
} else {
- InsertTempDtorDecisionBlock(FalseContext);
+ InsertTempDecisionBlock(FalseContext);
}
+ if (BuildOpts.AddLifetime) {
+ Context.CollectedMTEs.append(TrueContext.CollectedMTEs);
+ Context.CollectedMTEs.append(FalseContext.CollectedMTEs);
+ }
+
return Block;
}
@@ -5473,6 +5515,7 @@ CFGImplicitDtor::getDestructorDecl(ASTContext &astContext) const {
case CFGElement::CXXRecordTypedCall:
case CFGElement::ScopeBegin:
case CFGElement::ScopeEnd:
+ case CFGElement::FullExprCleanup:
case CFGElement::CleanupFunction:
llvm_unreachable("getDestructorDecl should only be used with "
"ImplicitDtors");
@@ -6037,6 +6080,27 @@ static void print_elem(raw_ostream &OS, StmtPrinterHelper &Helper,
OS << " (Lifetime ends)";
break;
+ case CFGElement::Kind::FullExprCleanup: {
+ auto MTEs = E.castAs<CFGFullExprCleanup>().getExpiringMTEs();
+ size_t MTECount = MTEs.size();
+ OS << "(FullExprCleanup collected " << MTECount
+ << (MTECount > 1 ? " MTEs: " : " MTE: ");
+ bool FirstMTE = true;
+ for (const MaterializeTemporaryExpr *MTE : MTEs) {
+ if (!FirstMTE)
+ OS << ", ";
+ if (!Helper.handledStmt(MTE->getSubExpr(), OS)) {
+ PrintingPolicy Policy{Helper.getLangOpts()};
+ Policy.IncludeNewlines = false;
+ // Pretty print the sub-expresion as a fallback
+ MTE->printPretty(OS, &Helper, Policy);
+ }
+ FirstMTE = false;
+ }
+ OS << ")";
+ break;
+ }
+
case CFGElement::Kind::LoopExit:
OS << E.castAs<CFGLoopExit>().getLoopStmt()->getStmtClassName()
<< " (LoopExit)";
diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
index 0b6e70212003c..ca89e5ff17305 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -18,6 +18,9 @@
#include "clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h"
#include "clang/Analysis/Analyses/LifetimeSafety/Origins.h"
#include "clang/Analysis/Analyses/PostOrderCFGView.h"
+#include "clang/Analysis/CFG.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/Signals.h"
#include "llvm/Support/TimeProfiler.h"
@@ -84,17 +87,6 @@ static const PathLoan *createLoan(FactManager &FactMgr,
return FactMgr.getLoanMgr().createLoan<PathLoan>(Path, MTE);
}
-/// Try to find a CXXBindTemporaryExpr that descends from MTE, stripping away
-/// any implicit casts.
-/// \param MTE MaterializeTemporaryExpr whose descendants we are interested in.
-/// \return Pointer to descendant CXXBindTemporaryExpr or nullptr when not
-/// found.
-static const CXXBindTemporaryExpr *
-getChildBinding(const MaterializeTemporaryExpr *MTE) {
- const Expr *Child = MTE->getSubExpr()->IgnoreImpCasts();
- return dyn_cast<CXXBindTemporaryExpr>(Child);
-}
-
void FactsGenerator::run() {
llvm::TimeTraceScope TimeProfile("FactGenerator");
const CFG &Cfg = *AC.getCFG();
@@ -117,9 +109,10 @@ void FactsGenerator::run() {
else if (std::optional<CFGLifetimeEnds> LifetimeEnds =
Element.getAs<CFGLifetimeEnds>())
handleLifetimeEnds(*LifetimeEnds);
- else if (std::optional<CFGTemporaryDtor> TemporaryDtor =
- Element.getAs<CFGTemporaryDtor>())
- handleTemporaryDtor(*TemporaryDtor);
+ else if (std::optional<CFGFullExprCleanup> FullExprCleanup =
+ Element.getAs<CFGFullExprCleanup>()) {
+ handleFullExprCleanup(*FullExprCleanup);
+ }
}
if (Block == &Cfg.getExit())
handleExitBlock();
@@ -429,7 +422,7 @@ void FactsGenerator::VisitMaterializeTemporaryExpr(
OriginList *RValMTEList = getRValueOrigins(MTE, MTEList);
flow(RValMTEList, SubExprList, /*Kill=*/true);
OriginID OuterMTEID = MTEList->getOuterOriginID();
- if (getChildBinding(MTE)) {
+ if (MTE->getStorageDuration() == SD_FullExpression) {
// Issue a loan to MTE for the storage location represented by MTE.
const Loan *L = createLoan(FactMgr, MTE);
CurrentBlockFacts.push_back(
@@ -438,7 +431,6 @@ void FactsGenerator::VisitMaterializeTemporaryExpr(
}
void FactsGenerator::handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds) {
- /// TODO: Handle loans to temporaries.
const VarDecl *LifetimeEndsVD = LifetimeEnds.getVarDecl();
if (!LifetimeEndsVD)
return;
@@ -456,12 +448,8 @@ void FactsGenerator::handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds) {
}
}
-void FactsGenerator::handleTemporaryDtor(
- const CFGTemporaryDtor &TemporaryDtor) {
- const CXXBindTemporaryExpr *ExpiringBTE =
- TemporaryDtor.getBindTemporaryExpr();
- if (!ExpiringBTE)
- return;
+void FactsGenerator::handleFullExprCleanup(
+ const CFGFullExprCleanup &FullExprCleanup) {
// Iterate through all loans to see if any expire.
for (const auto *Loan : FactMgr.getLoanMgr().getLoans()) {
if (const auto *PL = dyn_cast<PathLoan>(Loan)) {
@@ -471,9 +459,9 @@ void FactsGenerator::handleTemporaryDtor(
const MaterializeTemporaryExpr *Path = AP.getAsMaterializeTemporaryExpr();
if (!Path)
continue;
- if (ExpiringBTE == getChildBinding(Path)) {
- CurrentBlockFacts.push_back(FactMgr.createFact<ExpireFact>(
- PL->getID(), TemporaryDtor.getBindTemporaryExpr()->getEndLoc()));
+ if (llvm::is_contained(FullExprCleanup.getExpiringMTEs(), Path)) {
+ CurrentBlockFacts.push_back(
+ FactMgr.createFact<ExpireFact>(PL->getID(), Path->getEndLoc()));
}
}
}
diff --git a/clang/lib/Analysis/PathDiagnostic.cpp b/clang/lib/Analysis/PathDiagnostic.cpp
index e42731b93bfb2..5e387e086a5a7 100644
--- a/clang/lib/Analysis/PathDiagnostic.cpp
+++ b/clang/lib/Analysis/PathDiagnostic.cpp
@@ -564,6 +564,7 @@ getLocationForCaller(const StackFrameContext *SFC,
case CFGElement::CleanupFunction:
llvm_unreachable("not yet implemented!");
case CFGElement::LifetimeEnds:
+ case CFGElement::FullExprCleanup:
case CFGElement::LoopExit:
llvm_unreachable("CFGElement kind should not be on callsite!");
}
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 5c49e601840cf..45adab94e07da 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -3036,8 +3036,6 @@ LifetimeSafetyTUAnalysis(Sema &S, TranslationUnitDecl *TU,
AC.getCFGBuildOptions().PruneTriviallyFalseEdges = false;
AC.getCFGBuildOptions().AddLifetime = true;
AC.getCFGBuildOptions().AddParameterLifetimes = true;
- AC.getCFGBuildOptions().AddImplicitDtors = true;
- AC.getCFGBuildOptions().AddTemporaryDtors = true;
AC.getCFGBuildOptions().setAllAlwaysAdd();
if (AC.getCFG())
runLifetimeSafetyAnalysis(AC, &SemaHelper, LSStats, S.CollectStats);
diff --git a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
index 9f0fcc4c3e3a5..354739c537802 100644
--- a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -528,6 +528,12 @@ void CoreEngine::HandlePostStmt(const CFGBlock *B, unsigned StmtIdx,
assert(B);
assert(!B->empty());
+ // We no-op by skipping any FullExprCleanup
+ while (StmtIdx < B->size() &&
+ (*B)[StmtIdx].getKind() == CFGElement::FullExprCleanup) {
+ StmtIdx++;
+ }
+
if (StmtIdx == B->size())
HandleBlockExit(B, Pred);
else {
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 1aa35f2e2078e..76c382eeb6899 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -995,6 +995,7 @@ void ExprEngine::processCFGElement(const CFGElement E, ExplodedNode *Pred,
return;
case CFGElement::LifetimeEnds:
case CFGElement::CleanupFunction:
+ case CFGElement::FullExprCleanup:
case CFGElement::ScopeBegin:
case CFGElement::ScopeEnd:
return;
diff --git a/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp b/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
index d03f47fa301e6..35fbe4d60c3f2 100644
--- a/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
@@ -115,6 +115,8 @@ const Stmt *SymbolConjured::getStmt() const {
return Elem->castAs<CFGTemporaryDtor>().getBindTemporaryExpr();
case CFGElement::CleanupFunction:
return nullptr;
+ case CFGElement::FullExprCleanup:
+ return nullptr;
}
return nullptr;
}
diff --git a/clang/test/Analysis/exprwithcleanups-lifetime-marker-cfg.cpp b/clang/test/Analysis/exprwithcleanups-lifetime-marker-cfg.cpp
new file mode 100644
index 0000000000000..526a1850c8c50
--- /dev/null
+++ b/clang/test/Analysis/exprwithcleanups-lifetime-marker-cfg.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_analyze_cc1 -fcxx-exceptions -fexceptions -analyzer-checker=debug.DumpCFG -analyzer-config cfg-lifetime=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+struct TrivialDtor {};
+
+struct NonTrivialDtor {
+ ~NonTrivialDtor();
+};
+
+void foo(const TrivialDtor&, const NonTrivialDtor&);
+
+void bar(const TrivialDtor& = TrivialDtor());
+
+// CHECK: 4: [B1.3] (ImplicitCastExpr, NoOp, const TrivialDtor)
+// CHECK-NEXT: 5: [B1.4]
+// CHECK: 8: [B1.7] (ImplicitCastExpr, NoOp, const NonTrivialDtor)
+// CHECK-NEXT: 9: [B1.8]
+// CHECK: (FullExprCleanup collected 2 MTEs: [B1.4], [B1.8])
+void f() {
+ foo(TrivialDtor(), NonTrivialDtor());
+}
+
+// CHECK: (FullExprCleanup collected 1 MTE: TrivialDtor())
+void g() {
+ bar();
+}
diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
index c40c6a671e3ab..59edfc238cf6a 100644
--- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -298,19 +298,19 @@ std::string_view danglingRefToOptionalFromTemp4() {
void danglingReferenceFromTempOwner() {
int &&r = *std::optional<int>(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}} \
// cfg-warning {{object whose reference is captured does not live long enough}} cfg-note {{destroyed here}}
- // FIXME: Detect this using the CFG-based lifetime analysis.
- // https://github.com/llvm/llvm-project/issues/175893
- int &&r2 = *std::optional<int>(5); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+ // https://github.com/llvm/llvm-project/issues/175893
+ int &&r2 = *std::optional<int>(5); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}} \
+ // cfg-warning {{object whose reference is captured does not live long enough}} cfg-note {{destroyed here}}
- // FIXME: Detect this using the CFG-based lifetime analysis.
- // https://github.com/llvm/llvm-project/issues/175893
- int &&r3 = std::optional<int>(5).value(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+ // https://github.com/llvm/llvm-project/issues/175893
+ int &&r3 = std::optional<int>(5).value(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}} \
+ // cfg-warning {{object whose reference is captured does not live long enough}} cfg-note {{destroyed here}}
const int &r4 = std::vector<int>().at(3); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}} \
// cfg-warning {{object whose reference is captured does not live long enough}} cfg-note {{destroyed here}}
int &&r5 = std::vector<int>().at(3); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}} \
// cfg-warning {{object whose reference is captured does not live long enough}} cfg-note {{destroyed here}}
- use(r, r2, r3, r4, r5); // cfg-note 3 {{later used here}}
+ use(r, r2, r3, r4, r5); // cfg-note 5 {{later used here}}
std::string_view sv = *getTempOptStr(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}} \
// cfg-warning {{object whose reference is captured does not live long enough}} cfg-note {{destroyed here}}
@@ -385,9 +385,9 @@ void handleGslPtrInitsThroughReference2() {
void handleTernaryOperator(bool cond) {
std::basic_string<char> def;
- // FIXME: Detect this using the CFG-based lifetime analysis.
- std::basic_string_view<char> v = cond ? def : ""; // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
- use(v);
+ std::basic_string_view<char> v = cond ? def : ""; // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}} \
+ // cfg-warning {{object whose reference is captured does not live long enough}} cfg-note {{destroyed here}}
+ use(v); // cfg-note {{later used here}}
}
std::string operator+(std::string_view s1, std::string_view s2);
@@ -851,8 +851,9 @@ class set {
namespace GH118064{
void test() {
- auto y = std::set<int>{}.begin(); // expected-warning {{object backing the pointer}}
- use(y);
+ auto y = std::set<int>{}.begin(); // expected-warning {{object backing the pointer}} \
+ // cfg-warning {{object whose reference is captured does not live long enough}} cfg-note {{destroyed here}}
+ use(y); // cfg-note {{later used here}}
}
} // namespace GH118064
diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp
index 8f52ff27bc6fd..8cca9b3402366 100644
--- a/clang/test/Sema/warn-lifetime-safety.cpp
+++ b/clang/test/Sema/warn-lifetime-safety.cpp
@@ -1105,12 +1105,11 @@ void passing_temporary_to_lifetime_bound_function() {
use(a); // expected-note {{later used here}}
}
-// FIXME: We expect to be warned of use-after-free at use(a), but this is not the
-// case as current analysis does not handle trivially destructed temporaries.
void use_trivial_temporary_after_destruction() {
View a;
- a = trivially_destructed_temporary();
- use(a);
+ a = trivially_destructed_temporary(); // expected-warning {{object whose reference is captured does not live long enough}} \
+ expected-note {{destroyed here}}
+ use(a); // expected-note {{later used here}}
}
namespace GH162834 {
@@ -1486,7 +1485,9 @@ void bar() {
{
S s;
x = s.x(); // expected-warning {{object whose reference is captured does not live long enough}}
- View y = S().x(); // FIXME: Handle temporaries.
+ View y = S().x(); // expected-warning {{object whose reference is captured does not live long enough}} \
+ expected-note {{destroyed here}}
+ (void)y; // expected-note {{used here}}
} // expected-note {{destroyed here}}
(void)x; // expected-note {{used here}}
}
More information about the cfe-commits
mailing list