https://github.com/AbhinavPradeep updated https://github.com/llvm/llvm-project/pull/177985
>From 01e56c3a72f80834217f196f96a151585b42ed7b Mon Sep 17 00:00:00 2001 From: Abhinav Pradeep <[email protected]> Date: Tue, 27 Jan 2026 01:20:55 +1000 Subject: [PATCH 01/10] Modify CFG to have a CFGFullExprCleanup marker. --- clang/include/clang/Analysis/CFG.h | 32 ++++++++++++ clang/lib/Analysis/CFG.cpp | 71 ++++++++++++++++++++++++--- clang/lib/Analysis/PathDiagnostic.cpp | 1 + 3 files changed, 98 insertions(+), 6 deletions(-) diff --git a/clang/include/clang/Analysis/CFG.h b/clang/include/clang/Analysis/CFG.h index e675935d9230a..b4b733bb4d42e 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 { + using MTEVecTy = BumpVector<const MaterializeTemporaryExpr *>; + +public: + explicit CFGFullExprCleanup(const MTEVecTy *vec) + : CFGElement(FullExprCleanup, vec, 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..440429facbc5a 100644 --- a/clang/lib/Analysis/CFG.cpp +++ b/clang/lib/Analysis/CFG.cpp @@ -55,6 +55,7 @@ #include "llvm/Support/TimeProfiler.h" #include "llvm/Support/raw_ostream.h" #include <cassert> +#include <cstddef> #include <memory> #include <optional> #include <string> @@ -699,7 +700,9 @@ class CFGBuilder { TempDtorContext() = default; TempDtorContext(TryResult KnownExecuted) : IsConditional(true), KnownExecuted(KnownExecuted) {} - + TempDtorContext(TryResult KnownExecuted, bool TrackExpiringMTEs) + : IsConditional(true), TrackExpiringMTEs(TrackExpiringMTEs), + 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 +720,16 @@ class CFGBuilder { TerminatorExpr = E; } + void track(const MaterializeTemporaryExpr *MTE) { + // Must only be invoked when TrackMTE is true + assert(TrackExpiringMTEs); + if (MTE) + CollectedMTEs.push_back(MTE); + } + const bool IsConditional = false; + bool TrackExpiringMTEs = false; + SmallVector<const MaterializeTemporaryExpr *, 5> CollectedMTEs; const TryResult KnownExecuted = true; CFGBlock *Succ = nullptr; CXXBindTemporaryExpr *TerminatorExpr = nullptr; @@ -806,6 +818,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, @@ -1827,8 +1840,11 @@ CFGBlock *CFGBuilder::addInitializer(CXXCtorInitializer *I) { if (BuildOpts.AddTemporaryDtors && HasTemporaries) { // Generate destructors for temporaries in initialization expression. TempDtorContext Context; + Context.TrackExpiringMTEs = true; VisitForTemporaryDtors(cast<ExprWithCleanups>(Init)->getSubExpr(), /*ExternallyDestructed=*/false, Context); + + addFullExprCleanupMarker(Context); } } @@ -2076,6 +2092,23 @@ void CFGBuilder::addScopeChangesHandling(LocalScope::const_iterator SrcPos, addAutomaticObjHandling(SrcPos, BasePos, S); } +void CFGBuilder::addFullExprCleanupMarker(TempDtorContext &Context) { + assert(Context.TrackExpiringMTEs); + + using MTEVecTy = BumpVector<const MaterializeTemporaryExpr *>; + MTEVecTy *ExpiringMTEs = nullptr; + BumpVectorContext &BVC = cfg->getBumpVectorContext(); + + size_t NumCollected = Context.CollectedMTEs.size(); + if (NumCollected > 0) { + autoCreateBlock(); + ExpiringMTEs = new (cfg->getAllocator()) 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 @@ -3137,8 +3170,11 @@ CFGBlock *CFGBuilder::VisitDeclSubExpr(DeclStmt *DS) { if (BuildOpts.AddTemporaryDtors && HasTemporaries) { // Generate destructors for temporaries in initialization expression. TempDtorContext Context; + Context.TrackExpiringMTEs = true; VisitForTemporaryDtors(cast<ExprWithCleanups>(Init)->getSubExpr(), /*ExternallyDestructed=*/true, Context); + + addFullExprCleanupMarker(Context); } } @@ -4956,13 +4992,17 @@ CFGBlock *CFGBuilder::VisitCXXForRangeStmt(CXXForRangeStmt *S) { } CFGBlock *CFGBuilder::VisitExprWithCleanups(ExprWithCleanups *E, - AddStmtChoice asc, bool ExternallyDestructed) { + AddStmtChoice asc, + bool ExternallyDestructed) { if (BuildOpts.AddTemporaryDtors) { // If adding implicit destructors visit the full expression for adding // destructors of temporaries. TempDtorContext Context; + Context.TrackExpiringMTEs = true; VisitForTemporaryDtors(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. asc = asc.withAlwaysAdd(true); @@ -5153,6 +5193,8 @@ CFGBlock *CFGBuilder::VisitForTemporaryDtors(Stmt *E, bool ExternallyDestructed, case Stmt::MaterializeTemporaryExprClass: { const MaterializeTemporaryExpr* MTE = cast<MaterializeTemporaryExpr>(E); ExternallyDestructed = (MTE->getStorageDuration() != SD_FullExpression); + if (Context.TrackExpiringMTEs && !ExternallyDestructed) + Context.track(MTE); SmallVector<const Expr *, 2> CommaLHSs; SmallVector<SubobjectAdjustment, 2> Adjustments; // Find the expression whose lifetime needs to be extended. @@ -5244,10 +5286,14 @@ CFGBlock *CFGBuilder::VisitBinaryOperatorForTemporaryDtors( // executed, thus we add a branch node that depends on the temporary // constructor call. TempDtorContext RHSContext( - bothKnownTrue(Context.KnownExecuted, RHSExecuted)); + bothKnownTrue(Context.KnownExecuted, RHSExecuted), + Context.TrackExpiringMTEs); VisitForTemporaryDtors(E->getRHS(), false, RHSContext); InsertTempDtorDecisionBlock(RHSContext); + if (Context.TrackExpiringMTEs) + Context.CollectedMTEs.append(RHSContext.CollectedMTEs); + return Block; } @@ -5338,14 +5384,15 @@ CFGBlock *CFGBuilder::VisitConditionalOperatorForTemporaryDtors( if (NegatedVal.isKnown()) NegatedVal.negate(); TempDtorContext TrueContext( - bothKnownTrue(Context.KnownExecuted, ConditionVal)); + bothKnownTrue(Context.KnownExecuted, ConditionVal), + Context.TrackExpiringMTEs); VisitForTemporaryDtors(E->getTrueExpr(), ExternallyDestructed, TrueContext); CFGBlock *TrueBlock = Block; Block = ConditionBlock; Succ = ConditionSucc; - TempDtorContext FalseContext( - bothKnownTrue(Context.KnownExecuted, NegatedVal)); + TempDtorContext FalseContext(bothKnownTrue(Context.KnownExecuted, NegatedVal), + Context.TrackExpiringMTEs); VisitForTemporaryDtors(E->getFalseExpr(), ExternallyDestructed, FalseContext); if (TrueContext.TerminatorExpr && FalseContext.TerminatorExpr) { @@ -5356,6 +5403,11 @@ CFGBlock *CFGBuilder::VisitConditionalOperatorForTemporaryDtors( } else { InsertTempDtorDecisionBlock(FalseContext); } + if (Context.TrackExpiringMTEs) { + Context.CollectedMTEs.append(TrueContext.CollectedMTEs); + Context.CollectedMTEs.append(FalseContext.CollectedMTEs); + } + return Block; } @@ -6037,6 +6089,13 @@ static void print_elem(raw_ostream &OS, StmtPrinterHelper &Helper, OS << " (Lifetime ends)"; break; + case CFGElement::Kind::FullExprCleanup: + OS << "(FullExprCleanup collected " + << std::to_string( + E.castAs<CFGFullExprCleanup>().getExpiringMTEs().size()) + << " MTEs)"; + break; + case CFGElement::Kind::LoopExit: OS << E.castAs<CFGLoopExit>().getLoopStmt()->getStmtClassName() << " (LoopExit)"; 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!"); } >From 6b9d881a50b942513ff71ebb6c9a3cb25d897fc5 Mon Sep 17 00:00:00 2001 From: Abhinav Pradeep <[email protected]> Date: Wed, 28 Jan 2026 03:12:45 +1000 Subject: [PATCH 02/10] Fixed solver related issues by no-oping new CFGElement --- clang/lib/Analysis/CFG.cpp | 1 + clang/lib/StaticAnalyzer/Core/CoreEngine.cpp | 6 ++++++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 1 + clang/lib/StaticAnalyzer/Core/SymbolManager.cpp | 2 ++ 4 files changed, 10 insertions(+) diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp index 440429facbc5a..28c5a58ee98d6 100644 --- a/clang/lib/Analysis/CFG.cpp +++ b/clang/lib/Analysis/CFG.cpp @@ -5525,6 +5525,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"); diff --git a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp index 1e5a04b8efbe4..b7d5ce9aa82cc 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 47312a53f61ff..bf5f66209b2e9 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; } >From 4e3e832e72168e2a443f04cc05aacee4b0214ea8 Mon Sep 17 00:00:00 2001 From: Abhinav Pradeep <[email protected]> Date: Wed, 4 Feb 2026 00:42:20 +1000 Subject: [PATCH 03/10] Set up analysis to consume the new CFGFullExprCleanup --- .../Analyses/LifetimeSafety/FactsGenerator.h | 3 +- .../LifetimeSafety/FactsGenerator.cpp | 39 +++++++------------ .../Sema/warn-lifetime-analysis-nocfg.cpp | 21 +++++----- clang/test/Sema/warn-lifetime-safety.cpp | 11 +++--- 4 files changed, 33 insertions(+), 41 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h index fb7d5ad91db79..00807eea47339 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h @@ -61,7 +61,8 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> { 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/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index b69f69ddbae34..732203aee1263 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,9 @@ 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 +421,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 +430,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 +447,10 @@ 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) { + ArrayRef<const MaterializeTemporaryExpr *> CollectedMTEs = + FullExprCleanup.getExpiringMTEs(); // 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 +460,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 (is_contained(CollectedMTEs, Path)) { + CurrentBlockFacts.push_back( + FactMgr.createFact<ExpireFact>(PL->getID(), Path->getEndLoc())); } } } diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp index c40c6a671e3ab..c229ffc6c0fb9 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}} + 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}} + 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}} } >From 62e58d5ff09cadf41511700e111026a85f474bd8 Mon Sep 17 00:00:00 2001 From: Abhinav Pradeep <[email protected]> Date: Wed, 4 Feb 2026 20:01:30 +1000 Subject: [PATCH 04/10] Addressed review comments --- clang/include/clang/Analysis/CFG.h | 6 +++--- clang/lib/Analysis/CFG.cpp | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/clang/include/clang/Analysis/CFG.h b/clang/include/clang/Analysis/CFG.h index b4b733bb4d42e..72b7ac013ccc2 100644 --- a/clang/include/clang/Analysis/CFG.h +++ b/clang/include/clang/Analysis/CFG.h @@ -315,11 +315,11 @@ class CFGLifetimeEnds : public CFGElement { }; class CFGFullExprCleanup : public CFGElement { - using MTEVecTy = BumpVector<const MaterializeTemporaryExpr *>; public: - explicit CFGFullExprCleanup(const MTEVecTy *vec) - : CFGElement(FullExprCleanup, vec, nullptr) {} + using MTEVecTy = BumpVector<const MaterializeTemporaryExpr *>; + explicit CFGFullExprCleanup(const MTEVecTy *MTEs) + : CFGElement(FullExprCleanup, MTEs, nullptr) {} ArrayRef<const MaterializeTemporaryExpr *> getExpiringMTEs() const { const MTEVecTy *ExpiringMTEs = diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp index 28c5a58ee98d6..b46ab93d39cb3 100644 --- a/clang/lib/Analysis/CFG.cpp +++ b/clang/lib/Analysis/CFG.cpp @@ -2095,14 +2095,14 @@ void CFGBuilder::addScopeChangesHandling(LocalScope::const_iterator SrcPos, void CFGBuilder::addFullExprCleanupMarker(TempDtorContext &Context) { assert(Context.TrackExpiringMTEs); - using MTEVecTy = BumpVector<const MaterializeTemporaryExpr *>; - MTEVecTy *ExpiringMTEs = nullptr; + CFGFullExprCleanup::MTEVecTy *ExpiringMTEs = nullptr; BumpVectorContext &BVC = cfg->getBumpVectorContext(); size_t NumCollected = Context.CollectedMTEs.size(); if (NumCollected > 0) { autoCreateBlock(); - ExpiringMTEs = new (cfg->getAllocator()) MTEVecTy(BVC, NumCollected); + ExpiringMTEs = new (cfg->getAllocator()) + CFGFullExprCleanup::MTEVecTy(BVC, NumCollected); for (const MaterializeTemporaryExpr *MTE : Context.CollectedMTEs) ExpiringMTEs->push_back(MTE, BVC); Block->appendFullExprCleanup(ExpiringMTEs, BVC); >From eb87447e65efd21e85002385224474a6c31f9acd Mon Sep 17 00:00:00 2001 From: Abhinav Pradeep <[email protected]> Date: Tue, 10 Feb 2026 18:45:21 +1000 Subject: [PATCH 05/10] CFGFullExprCleanup is treated like a lifetime marker instead of a dtor --- clang/lib/Analysis/CFG.cpp | 105 ++++++++++++++++--------------------- 1 file changed, 46 insertions(+), 59 deletions(-) diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp index b46ab93d39cb3..da689f418bead 100644 --- a/clang/lib/Analysis/CFG.cpp +++ b/clang/lib/Analysis/CFG.cpp @@ -700,9 +700,6 @@ class CFGBuilder { TempDtorContext() = default; TempDtorContext(TryResult KnownExecuted) : IsConditional(true), KnownExecuted(KnownExecuted) {} - TempDtorContext(TryResult KnownExecuted, bool TrackExpiringMTEs) - : IsConditional(true), TrackExpiringMTEs(TrackExpiringMTEs), - 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 @@ -721,14 +718,11 @@ class CFGBuilder { } void track(const MaterializeTemporaryExpr *MTE) { - // Must only be invoked when TrackMTE is true - assert(TrackExpiringMTEs); if (MTE) CollectedMTEs.push_back(MTE); } const bool IsConditional = false; - bool TrackExpiringMTEs = false; SmallVector<const MaterializeTemporaryExpr *, 5> CollectedMTEs; const TryResult KnownExecuted = true; CFGBlock *Succ = nullptr; @@ -737,21 +731,21 @@ class CFGBuilder { // Visitors to walk an AST and generate destructors of temporaries in // full expression. - CFGBlock *VisitForTemporaryDtors(Stmt *E, bool ExternallyDestructed, + CFGBlock *VisitForTemporaries(Stmt *E, bool ExternallyDestructed, TempDtorContext &Context); - CFGBlock *VisitChildrenForTemporaryDtors(Stmt *E, bool ExternallyDestructed, + CFGBlock *VisitChildrenForTemporaries(Stmt *E, bool ExternallyDestructed, TempDtorContext &Context); - CFGBlock *VisitBinaryOperatorForTemporaryDtors(BinaryOperator *E, + CFGBlock *VisitBinaryOperatorForTemporaries(BinaryOperator *E, bool ExternallyDestructed, TempDtorContext &Context); CFGBlock *VisitCXXOperatorCallExprForTemporaryDtors(CXXOperatorCallExpr *E, TempDtorContext &Context); CFGBlock *VisitCXXBindTemporaryExprForTemporaryDtors( CXXBindTemporaryExpr *E, bool ExternallyDestructed, TempDtorContext &Context); - CFGBlock *VisitConditionalOperatorForTemporaryDtors( + CFGBlock *VisitConditionalOperatorForTemporaries( AbstractConditionalOperator *E, bool ExternallyDestructed, TempDtorContext &Context); - void InsertTempDtorDecisionBlock(const TempDtorContext &Context, + void InsertTempDecisionBlock(const TempDtorContext &Context, CFGBlock *FalseSucc = nullptr); // NYS == Not Yet Supported @@ -1840,8 +1834,7 @@ CFGBlock *CFGBuilder::addInitializer(CXXCtorInitializer *I) { if (BuildOpts.AddTemporaryDtors && HasTemporaries) { // Generate destructors for temporaries in initialization expression. TempDtorContext Context; - Context.TrackExpiringMTEs = true; - VisitForTemporaryDtors(cast<ExprWithCleanups>(Init)->getSubExpr(), + VisitForTemporaries(cast<ExprWithCleanups>(Init)->getSubExpr(), /*ExternallyDestructed=*/false, Context); addFullExprCleanupMarker(Context); @@ -2093,7 +2086,6 @@ void CFGBuilder::addScopeChangesHandling(LocalScope::const_iterator SrcPos, } void CFGBuilder::addFullExprCleanupMarker(TempDtorContext &Context) { - assert(Context.TrackExpiringMTEs); CFGFullExprCleanup::MTEVecTy *ExpiringMTEs = nullptr; BumpVectorContext &BVC = cfg->getBumpVectorContext(); @@ -3170,8 +3162,7 @@ CFGBlock *CFGBuilder::VisitDeclSubExpr(DeclStmt *DS) { if (BuildOpts.AddTemporaryDtors && HasTemporaries) { // Generate destructors for temporaries in initialization expression. TempDtorContext Context; - Context.TrackExpiringMTEs = true; - VisitForTemporaryDtors(cast<ExprWithCleanups>(Init)->getSubExpr(), + VisitForTemporaries(cast<ExprWithCleanups>(Init)->getSubExpr(), /*ExternallyDestructed=*/true, Context); addFullExprCleanupMarker(Context); @@ -4998,8 +4989,7 @@ CFGBlock *CFGBuilder::VisitExprWithCleanups(ExprWithCleanups *E, // If adding implicit destructors visit the full expression for adding // destructors of temporaries. TempDtorContext Context; - Context.TrackExpiringMTEs = true; - VisitForTemporaryDtors(E->getSubExpr(), ExternallyDestructed, Context); + VisitForTemporaries(E->getSubExpr(), ExternallyDestructed, Context); addFullExprCleanupMarker(Context); @@ -5138,9 +5128,8 @@ CFGBlock *CFGBuilder::VisitIndirectGotoStmt(IndirectGotoStmt *I) { return addStmt(I->getTarget()); } -CFGBlock *CFGBuilder::VisitForTemporaryDtors(Stmt *E, bool ExternallyDestructed, +CFGBlock *CFGBuilder::VisitForTemporaries(Stmt *E, bool ExternallyDestructed, TempDtorContext &Context) { - assert(BuildOpts.AddImplicitDtors && BuildOpts.AddTemporaryDtors); tryAgain: if (!E) { @@ -5149,13 +5138,13 @@ 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), + return VisitBinaryOperatorForTemporaries(cast<BinaryOperator>(E), ExternallyDestructed, Context); @@ -5169,7 +5158,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: @@ -5193,7 +5182,7 @@ CFGBlock *CFGBuilder::VisitForTemporaryDtors(Stmt *E, bool ExternallyDestructed, case Stmt::MaterializeTemporaryExprClass: { const MaterializeTemporaryExpr* MTE = cast<MaterializeTemporaryExpr>(E); ExternallyDestructed = (MTE->getStorageDuration() != SD_FullExpression); - if (Context.TrackExpiringMTEs && !ExternallyDestructed) + if (BuildOpts.AddLifetime && !ExternallyDestructed) Context.track(MTE); SmallVector<const Expr *, 2> CommaLHSs; SmallVector<SubobjectAdjustment, 2> Adjustments; @@ -5204,7 +5193,7 @@ 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), + VisitForTemporaries(const_cast<Expr *>(CommaLHS), /*ExternallyDestructed=*/false, Context); } goto tryAgain; @@ -5222,7 +5211,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; } @@ -5245,7 +5234,7 @@ CFGBlock *CFGBuilder::VisitForTemporaryDtors(Stmt *E, bool ExternallyDestructed, } } -CFGBlock *CFGBuilder::VisitChildrenForTemporaryDtors(Stmt *E, +CFGBlock *CFGBuilder::VisitChildrenForTemporaries(Stmt *E, bool ExternallyDestructed, TempDtorContext &Context) { if (isa<LambdaExpr>(E)) { @@ -5253,31 +5242,31 @@ CFGBlock *CFGBuilder::VisitChildrenForTemporaryDtors(Stmt *E, return Block; } - // When visiting children for destructors we want to visit them in reverse + // 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(); @@ -5286,12 +5275,11 @@ CFGBlock *CFGBuilder::VisitBinaryOperatorForTemporaryDtors( // executed, thus we add a branch node that depends on the temporary // constructor call. TempDtorContext RHSContext( - bothKnownTrue(Context.KnownExecuted, RHSExecuted), - Context.TrackExpiringMTEs); - VisitForTemporaryDtors(E->getRHS(), false, RHSContext); - InsertTempDtorDecisionBlock(RHSContext); + bothKnownTrue(Context.KnownExecuted, RHSExecuted)); + VisitForTemporaries(E->getRHS(), false, RHSContext); + InsertTempDecisionBlock(RHSContext); - if (Context.TrackExpiringMTEs) + if (BuildOpts.AddLifetime) Context.CollectedMTEs.append(RHSContext.CollectedMTEs); return Block; @@ -5300,13 +5288,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( @@ -5325,8 +5313,10 @@ 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. @@ -5351,13 +5341,12 @@ CFGBlock *CFGBuilder::VisitCXXBindTemporaryExprForTemporaryDtors( Context.setDecisionPoint(Succ, E); } appendTemporaryDtor(Block, E); - - B = Block; + return B; } return B; } -void CFGBuilder::InsertTempDtorDecisionBlock(const TempDtorContext &Context, +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. @@ -5373,10 +5362,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()); @@ -5384,26 +5373,24 @@ CFGBlock *CFGBuilder::VisitConditionalOperatorForTemporaryDtors( if (NegatedVal.isKnown()) NegatedVal.negate(); TempDtorContext TrueContext( - bothKnownTrue(Context.KnownExecuted, ConditionVal), - Context.TrackExpiringMTEs); - VisitForTemporaryDtors(E->getTrueExpr(), ExternallyDestructed, TrueContext); + bothKnownTrue(Context.KnownExecuted, ConditionVal)); + VisitForTemporaries(E->getTrueExpr(), ExternallyDestructed, TrueContext); CFGBlock *TrueBlock = Block; Block = ConditionBlock; Succ = ConditionSucc; - TempDtorContext FalseContext(bothKnownTrue(Context.KnownExecuted, NegatedVal), - Context.TrackExpiringMTEs); - VisitForTemporaryDtors(E->getFalseExpr(), ExternallyDestructed, FalseContext); + TempDtorContext FalseContext(bothKnownTrue(Context.KnownExecuted, NegatedVal)); + 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 (Context.TrackExpiringMTEs) { + if (BuildOpts.AddLifetime) { Context.CollectedMTEs.append(TrueContext.CollectedMTEs); Context.CollectedMTEs.append(FalseContext.CollectedMTEs); } >From 4ecce3cd3e161bd06b5413298bb1bfc5be4a513d Mon Sep 17 00:00:00 2001 From: Abhinav Pradeep <[email protected]> Date: Tue, 10 Feb 2026 19:35:35 +1000 Subject: [PATCH 06/10] Ensure correct conditions at call sites of VisitForTemporaries --- clang/lib/Analysis/CFG.cpp | 60 ++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp index da689f418bead..bde22883cbbd8 100644 --- a/clang/lib/Analysis/CFG.cpp +++ b/clang/lib/Analysis/CFG.cpp @@ -732,9 +732,9 @@ class CFGBuilder { // Visitors to walk an AST and generate destructors of temporaries in // full expression. CFGBlock *VisitForTemporaries(Stmt *E, bool ExternallyDestructed, - TempDtorContext &Context); - CFGBlock *VisitChildrenForTemporaries(Stmt *E, bool ExternallyDestructed, - TempDtorContext &Context); + TempDtorContext &Context); + CFGBlock *VisitChildrenForTemporaries(Stmt *E, bool ExternallyDestructed, + TempDtorContext &Context); CFGBlock *VisitBinaryOperatorForTemporaries(BinaryOperator *E, bool ExternallyDestructed, TempDtorContext &Context); @@ -742,11 +742,12 @@ class CFGBuilder { TempDtorContext &Context); CFGBlock *VisitCXXBindTemporaryExprForTemporaryDtors( CXXBindTemporaryExpr *E, bool ExternallyDestructed, TempDtorContext &Context); - CFGBlock *VisitConditionalOperatorForTemporaries( - AbstractConditionalOperator *E, bool ExternallyDestructed, - TempDtorContext &Context); + CFGBlock * + VisitConditionalOperatorForTemporaries(AbstractConditionalOperator *E, + bool ExternallyDestructed, + TempDtorContext &Context); void InsertTempDecisionBlock(const TempDtorContext &Context, - CFGBlock *FalseSucc = nullptr); + CFGBlock *FalseSucc = nullptr); // NYS == Not Yet Supported CFGBlock *NYS() { @@ -1831,11 +1832,12 @@ 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; VisitForTemporaries(cast<ExprWithCleanups>(Init)->getSubExpr(), - /*ExternallyDestructed=*/false, Context); + /*ExternallyDestructed=*/false, Context); addFullExprCleanupMarker(Context); } @@ -3159,11 +3161,12 @@ 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; VisitForTemporaries(cast<ExprWithCleanups>(Init)->getSubExpr(), - /*ExternallyDestructed=*/true, Context); + /*ExternallyDestructed=*/true, Context); addFullExprCleanupMarker(Context); } @@ -4985,7 +4988,7 @@ CFGBlock *CFGBuilder::VisitCXXForRangeStmt(CXXForRangeStmt *S) { CFGBlock *CFGBuilder::VisitExprWithCleanups(ExprWithCleanups *E, AddStmtChoice asc, bool ExternallyDestructed) { - if (BuildOpts.AddTemporaryDtors) { + if (BuildOpts.AddTemporaryDtors || BuildOpts.AddLifetime) { // If adding implicit destructors visit the full expression for adding // destructors of temporaries. TempDtorContext Context; @@ -5129,7 +5132,7 @@ CFGBlock *CFGBuilder::VisitIndirectGotoStmt(IndirectGotoStmt *I) { } CFGBlock *CFGBuilder::VisitForTemporaries(Stmt *E, bool ExternallyDestructed, - TempDtorContext &Context) { + TempDtorContext &Context) { tryAgain: if (!E) { @@ -5145,8 +5148,7 @@ CFGBlock *CFGBuilder::VisitForTemporaries(Stmt *E, bool ExternallyDestructed, case Stmt::BinaryOperatorClass: return VisitBinaryOperatorForTemporaries(cast<BinaryOperator>(E), - ExternallyDestructed, - Context); + ExternallyDestructed, Context); case Stmt::CXXOperatorCallExprClass: return VisitCXXOperatorCallExprForTemporaryDtors( @@ -5194,7 +5196,7 @@ CFGBlock *CFGBuilder::VisitForTemporaries(Stmt *E, bool ExternallyDestructed, // Visit the skipped comma operator left-hand sides for other temporaries. for (const Expr *CommaLHS : CommaLHSs) { VisitForTemporaries(const_cast<Expr *>(CommaLHS), - /*ExternallyDestructed=*/false, Context); + /*ExternallyDestructed=*/false, Context); } goto tryAgain; } @@ -5235,21 +5237,22 @@ CFGBlock *CFGBuilder::VisitForTemporaries(Stmt *E, bool ExternallyDestructed, } CFGBlock *CFGBuilder::VisitChildrenForTemporaries(Stmt *E, - bool ExternallyDestructed, - TempDtorContext &Context) { + 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 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 + // 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 = VisitForTemporaries(Child, ExternallyDestructed, Context)) + if (CFGBlock *R = + VisitForTemporaries(Child, ExternallyDestructed, Context)) B = R; return B; @@ -5261,7 +5264,8 @@ CFGBlock *CFGBuilder::VisitBinaryOperatorForTemporaries( // For the comma operator, the LHS expression is evaluated before the RHS // expression, so prepend temporary destructors for the LHS first. CFGBlock *LHSBlock = VisitForTemporaries(E->getLHS(), false, Context); - CFGBlock *RHSBlock = VisitForTemporaries(E->getRHS(), ExternallyDestructed, Context); + CFGBlock *RHSBlock = + VisitForTemporaries(E->getRHS(), ExternallyDestructed, Context); return RHSBlock ? RHSBlock : LHSBlock; } @@ -5314,9 +5318,8 @@ CFGBlock *CFGBuilder::VisitCXXBindTemporaryExprForTemporaryDtors( // First add destructors for temporaries in subexpression. // Because VisitCXXBindTemporaryExpr calls setDestructed: CFGBlock *B = VisitForTemporaries(E->getSubExpr(), true, Context); - if (!ExternallyDestructed && - BuildOpts.AddImplicitDtors && - BuildOpts.AddTemporaryDtors) { + if (!ExternallyDestructed && BuildOpts.AddImplicitDtors && + BuildOpts.AddTemporaryDtors) { // If lifetime of temporary is not prolonged (by assigning to constant // reference) add destructor for it. @@ -5347,7 +5350,7 @@ CFGBlock *CFGBuilder::VisitCXXBindTemporaryExprForTemporaryDtors( } void CFGBuilder::InsertTempDecisionBlock(const TempDtorContext &Context, - CFGBlock *FalseSucc) { + CFGBlock *FalseSucc) { if (!Context.TerminatorExpr) { // If no temporary was found, we do not need to insert a decision point. return; @@ -5379,7 +5382,8 @@ CFGBlock *CFGBuilder::VisitConditionalOperatorForTemporaries( Block = ConditionBlock; Succ = ConditionSucc; - TempDtorContext FalseContext(bothKnownTrue(Context.KnownExecuted, NegatedVal)); + TempDtorContext FalseContext( + bothKnownTrue(Context.KnownExecuted, NegatedVal)); VisitForTemporaries(E->getFalseExpr(), ExternallyDestructed, FalseContext); if (TrueContext.TerminatorExpr && FalseContext.TerminatorExpr) { >From 107525429b70be3fd9e70ad0dadd699777fb3080 Mon Sep 17 00:00:00 2001 From: Abhinav Pradeep <[email protected]> Date: Fri, 13 Feb 2026 11:49:16 +1000 Subject: [PATCH 07/10] Addressed review comments --- .../Analyses/LifetimeSafety/FactsGenerator.h | 3 +- clang/lib/Analysis/CFG.cpp | 27 ++++++++---- .../LifetimeSafety/FactsGenerator.cpp | 43 +++++++++---------- clang/lib/Sema/AnalysisBasedWarnings.cpp | 2 - .../Sema/warn-lifetime-analysis-nocfg.cpp | 4 +- 5 files changed, 42 insertions(+), 37 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h index 00807eea47339..c7581697da1a2 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h @@ -60,9 +60,8 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> { void handleAssignment(const Expr *LHSExpr, const Expr *RHSExpr); void handleCXXCtorInitializer(const CXXCtorInitializer *CII); - void handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds); - void handleFullExprCleanup(const CFGFullExprCleanup &FullExprCleanup); + void handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds); void handleExitBlock(); diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp index bde22883cbbd8..38827335ccdd6 100644 --- a/clang/lib/Analysis/CFG.cpp +++ b/clang/lib/Analysis/CFG.cpp @@ -54,6 +54,7 @@ #include "llvm/Support/SaveAndRestore.h" #include "llvm/Support/TimeProfiler.h" #include "llvm/Support/raw_ostream.h" +#include <algorithm> #include <cassert> #include <cstddef> #include <memory> @@ -718,8 +719,7 @@ class CFGBuilder { } void track(const MaterializeTemporaryExpr *MTE) { - if (MTE) - CollectedMTEs.push_back(MTE); + CollectedMTEs.push_back(MTE); } const bool IsConditional = false; @@ -2088,7 +2088,6 @@ void CFGBuilder::addScopeChangesHandling(LocalScope::const_iterator SrcPos, } void CFGBuilder::addFullExprCleanupMarker(TempDtorContext &Context) { - CFGFullExprCleanup::MTEVecTy *ExpiringMTEs = nullptr; BumpVectorContext &BVC = cfg->getBumpVectorContext(); @@ -6081,12 +6080,24 @@ static void print_elem(raw_ostream &OS, StmtPrinterHelper &Helper, OS << " (Lifetime ends)"; break; - case CFGElement::Kind::FullExprCleanup: - OS << "(FullExprCleanup collected " - << std::to_string( - E.castAs<CFGFullExprCleanup>().getExpiringMTEs().size()) - << " MTEs)"; + 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)) { + // Pretty print the sub-expresion as a fallback + MTE->printPretty(OS, &Helper, PrintingPolicy(Helper.getLangOpts())); + }; + FirstMTE = false; + } + OS << ")"; break; + } case CFGElement::Kind::LoopExit: OS << E.castAs<CFGLoopExit>().getLoopStmt()->getStmtClassName() diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 732203aee1263..4d7622c8e8fdf 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -110,8 +110,26 @@ void FactsGenerator::run() { Element.getAs<CFGLifetimeEnds>()) handleLifetimeEnds(*LifetimeEnds); else if (std::optional<CFGFullExprCleanup> FullExprCleanup = - Element.getAs<CFGFullExprCleanup>()) - handleFullExprCleanup(*FullExprCleanup); + Element.getAs<CFGFullExprCleanup>()) { + ArrayRef<const MaterializeTemporaryExpr *> CollectedMTEs = + FullExprCleanup->getExpiringMTEs(); + // Iterate through all loans to see if any expire. + for (const auto *Loan : FactMgr.getLoanMgr().getLoans()) { + if (const auto *PL = dyn_cast<PathLoan>(Loan)) { + // Check if the loan is for a temporary materialization and if that + // storage location is the one being destructed. + const AccessPath &AP = PL->getAccessPath(); + const MaterializeTemporaryExpr *Path = + AP.getAsMaterializeTemporaryExpr(); + if (!Path) + continue; + if (llvm::is_contained(CollectedMTEs, Path)) { + CurrentBlockFacts.push_back(FactMgr.createFact<ExpireFact>( + PL->getID(), Path->getEndLoc())); + } + } + } + } } if (Block == &Cfg.getExit()) handleExitBlock(); @@ -447,27 +465,6 @@ void FactsGenerator::handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds) { } } -void FactsGenerator::handleFullExprCleanup( - const CFGFullExprCleanup &FullExprCleanup) { - ArrayRef<const MaterializeTemporaryExpr *> CollectedMTEs = - FullExprCleanup.getExpiringMTEs(); - // Iterate through all loans to see if any expire. - for (const auto *Loan : FactMgr.getLoanMgr().getLoans()) { - if (const auto *PL = dyn_cast<PathLoan>(Loan)) { - // Check if the loan is for a temporary materialization and if that - // storage location is the one being destructed. - const AccessPath &AP = PL->getAccessPath(); - const MaterializeTemporaryExpr *Path = AP.getAsMaterializeTemporaryExpr(); - if (!Path) - continue; - if (is_contained(CollectedMTEs, Path)) { - CurrentBlockFacts.push_back( - FactMgr.createFact<ExpireFact>(PL->getID(), Path->getEndLoc())); - } - } - } -} - void FactsGenerator::handleExitBlock() { // Creates FieldEscapeFacts for all field origins that remain live at exit. for (const Origin &O : FactMgr.getOriginMgr().getOrigins()) diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 8c7bff27718de..86b9807bb8297 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -3016,8 +3016,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/test/Sema/warn-lifetime-analysis-nocfg.cpp b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp index c229ffc6c0fb9..c764d8b6b816c 100644 --- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp +++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp @@ -298,11 +298,11 @@ 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}} - // https://github.com/llvm/llvm-project/issues/175893 + //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}} - // https://github.com/llvm/llvm-project/issues/175893 + //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}} >From e6a43a59836296b3409f792313fb908829364fe7 Mon Sep 17 00:00:00 2001 From: Abhinav Pradeep <[email protected]> Date: Fri, 13 Feb 2026 16:06:22 +1000 Subject: [PATCH 08/10] CFG filecheck test for FullExprCleanup --- .../exprwithcleanups-lifetime-marker-cfg.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 clang/test/Analysis/exprwithcleanups-lifetime-marker-cfg.cpp 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..87089a11cbf5e --- /dev/null +++ b/clang/test/Analysis/exprwithcleanups-lifetime-marker-cfg.cpp @@ -0,0 +1,15 @@ +// 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&); + +// CHECK: (FullExprCleanup collected 2 MTEs: [B1.4], [B1.8]) +void f() { + foo(TrivialDtor(), NonTrivialDtor()); +} \ No newline at end of file >From b6d695a07a4a68b685dcaa3139d6afd3223762d2 Mon Sep 17 00:00:00 2001 From: Abhinav Pradeep <[email protected]> Date: Mon, 16 Feb 2026 19:15:28 +1000 Subject: [PATCH 09/10] Revert mistake and fix formatting in exprwithcleanups-lifetime-marker-cfg.cpp --- clang/lib/Analysis/CFG.cpp | 2 +- clang/test/Analysis/exprwithcleanups-lifetime-marker-cfg.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp index 38827335ccdd6..ce9a7fd3eb54b 100644 --- a/clang/lib/Analysis/CFG.cpp +++ b/clang/lib/Analysis/CFG.cpp @@ -5343,7 +5343,7 @@ CFGBlock *CFGBuilder::VisitCXXBindTemporaryExprForTemporaryDtors( Context.setDecisionPoint(Succ, E); } appendTemporaryDtor(Block, E); - return B; + B = Block; } return B; } diff --git a/clang/test/Analysis/exprwithcleanups-lifetime-marker-cfg.cpp b/clang/test/Analysis/exprwithcleanups-lifetime-marker-cfg.cpp index 87089a11cbf5e..bee1f7fbf324a 100644 --- a/clang/test/Analysis/exprwithcleanups-lifetime-marker-cfg.cpp +++ b/clang/test/Analysis/exprwithcleanups-lifetime-marker-cfg.cpp @@ -12,4 +12,4 @@ void foo(const TrivialDtor&, const NonTrivialDtor&); // CHECK: (FullExprCleanup collected 2 MTEs: [B1.4], [B1.8]) void f() { foo(TrivialDtor(), NonTrivialDtor()); -} \ No newline at end of file +} >From 753da81b15050c17d82a7f73d3b11c864faad039 Mon Sep 17 00:00:00 2001 From: Abhinav Pradeep <[email protected]> Date: Mon, 16 Feb 2026 20:30:17 +1000 Subject: [PATCH 10/10] Fix formatting and issue from rebasing --- clang/lib/Analysis/CFG.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp index ce9a7fd3eb54b..335a5a9732f53 100644 --- a/clang/lib/Analysis/CFG.cpp +++ b/clang/lib/Analysis/CFG.cpp @@ -736,8 +736,8 @@ class CFGBuilder { CFGBlock *VisitChildrenForTemporaries(Stmt *E, bool ExternallyDestructed, TempDtorContext &Context); CFGBlock *VisitBinaryOperatorForTemporaries(BinaryOperator *E, - bool ExternallyDestructed, - TempDtorContext &Context); + bool ExternallyDestructed, + TempDtorContext &Context); CFGBlock *VisitCXXOperatorCallExprForTemporaryDtors(CXXOperatorCallExpr *E, TempDtorContext &Context); CFGBlock *VisitCXXBindTemporaryExprForTemporaryDtors( @@ -5305,11 +5305,11 @@ 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( _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
