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}} } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
