https://github.com/AbhinavPradeep updated https://github.com/llvm/llvm-project/pull/172007
>From 95204ac62684b625a11f758818fb972148d6c611 Mon Sep 17 00:00:00 2001 From: Abhinav Pradeep <[email protected]> Date: Fri, 12 Dec 2025 22:47:33 +1000 Subject: [PATCH 1/5] Add support for loans to temporary materializations --- .../Analyses/LifetimeSafety/FactsGenerator.h | 2 + .../Analysis/Analyses/LifetimeSafety/Loans.h | 21 +++++++- .../LifetimeSafety/FactsGenerator.cpp | 33 ++++++++++++- clang/lib/Analysis/LifetimeSafety/Loans.cpp | 11 ++++- clang/test/Sema/warn-lifetime-safety.cpp | 12 +++++ .../unittests/Analysis/LifetimeSafetyTest.cpp | 49 +++++++++++++++++-- 6 files changed, 120 insertions(+), 8 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h index e255b04a12684..0e23dc8ea0fc5 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h @@ -59,6 +59,8 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> { void handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds); + void handleTemporaryDtor(const CFGTemporaryDtor &TemporaryDtor); + void handleGSLPointerConstruction(const CXXConstructExpr *CCE); /// Checks if a call-like expression creates a borrow by passing a value to a diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h index e9bccd4773622..96b34e52529e3 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h @@ -15,6 +15,7 @@ #define LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMESAFETY_LOANS_H #include "clang/AST/Decl.h" +#include "clang/AST/ExprCXX.h" #include "clang/Analysis/Analyses/LifetimeSafety/Utils.h" #include "llvm/Support/raw_ostream.h" @@ -29,9 +30,25 @@ inline llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, LoanID ID) { /// variable. /// TODO: Model access paths of other types, e.g., s.field, heap and globals. struct AccessPath { - const clang::ValueDecl *D; + // Currently, an access path can be: + // - ValueDecl * , to represent the storage location corresponding to the + // variable declared in ValueDecl. + // - CXXBindTemporaryExpr * , to represent the storage location of the + // temporary object used for the binding in CXXBindTemporaryExpr. + const llvm::PointerUnion<const clang::ValueDecl *, + const clang::CXXBindTemporaryExpr *> + P; + + AccessPath(const clang::ValueDecl *D) : P(D) {} + AccessPath(const clang::CXXBindTemporaryExpr *BTE) : P(BTE) {} + + const clang::ValueDecl *getAsValueDecl() const { + return P.dyn_cast<const clang::ValueDecl *>(); + } - AccessPath(const clang::ValueDecl *D) : D(D) {} + const clang::CXXBindTemporaryExpr *getAsCXXBindTemporaryExpr() const { + return P.dyn_cast<const clang::CXXBindTemporaryExpr *>(); + } }; /// An abstract base class for a single "Loan" which represents lending a diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index f3993b7e7e261..1582e5328ddf4 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -71,6 +71,12 @@ static const PathLoan *createLoan(FactManager &FactMgr, return nullptr; } +static const PathLoan *createLoan(FactManager &FactMgr, + const CXXBindTemporaryExpr *BTE) { + AccessPath Path(BTE); + return FactMgr.getLoanMgr().createLoan<PathLoan>(Path, BTE); +} + void FactsGenerator::run() { llvm::TimeTraceScope TimeProfile("FactGenerator"); const CFG &Cfg = *AC.getCFG(); @@ -90,6 +96,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); } CurrentBlockFacts.append(EscapesInCurrentBlock.begin(), EscapesInCurrentBlock.end()); @@ -361,13 +370,35 @@ void FactsGenerator::handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds) { if (const auto *BL = dyn_cast<PathLoan>(Loan)) { // Check if the loan is for a stack variable and if that variable // is the one being destructed. - if (BL->getAccessPath().D == LifetimeEndsVD) + const AccessPath AP = BL->getAccessPath(); + const ValueDecl *Path = AP.getAsValueDecl(); + if (Path == LifetimeEndsVD) CurrentBlockFacts.push_back(FactMgr.createFact<ExpireFact>( BL->getID(), LifetimeEnds.getTriggerStmt()->getEndLoc())); } } } +void FactsGenerator::handleTemporaryDtor( + const CFGTemporaryDtor &TemporaryDtor) { + const CXXBindTemporaryExpr *BTE = TemporaryDtor.getBindTemporaryExpr(); + if (!BTE) { + return; + } + // Iterate through all loans to see if any expire. + for (const auto *Loan : FactMgr.getLoanMgr().getLoans()) { + if (const auto *BL = 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 = BL->getAccessPath(); + const CXXBindTemporaryExpr *Path = AP.getAsCXXBindTemporaryExpr(); + if (Path == BTE) + CurrentBlockFacts.push_back(FactMgr.createFact<ExpireFact>( + BL->getID(), TemporaryDtor.getBindTemporaryExpr()->getEndLoc())); + } + } +} + void FactsGenerator::handleGSLPointerConstruction(const CXXConstructExpr *CCE) { assert(isGslPointerType(CCE->getType())); if (CCE->getNumArgs() != 1) diff --git a/clang/lib/Analysis/LifetimeSafety/Loans.cpp b/clang/lib/Analysis/LifetimeSafety/Loans.cpp index fdfdbb40a2a46..5f6ba6809d2fb 100644 --- a/clang/lib/Analysis/LifetimeSafety/Loans.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Loans.cpp @@ -12,7 +12,16 @@ namespace clang::lifetimes::internal { void PathLoan::dump(llvm::raw_ostream &OS) const { OS << getID() << " (Path: "; - OS << Path.D->getNameAsString() << ")"; + if (const clang::ValueDecl *VD = Path.getAsValueDecl()) { + OS << VD->getNameAsString(); + } else if (const clang::CXXBindTemporaryExpr *BTE = + Path.getAsCXXBindTemporaryExpr()) { + // No nice "name" for the temporary, so deferring to LLVM default + OS << "CXXBindTemporaryExpr at " << BTE; + } else { + llvm_unreachable("access path is not one of any supported types"); + } + OS << ")"; } void PlaceholderLoan::dump(llvm::raw_ostream &OS) const { diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp index 5706d195c383b..344b783dcbdee 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -20,6 +20,9 @@ class TriviallyDestructedClass { View a, b; }; +MyObj temporary(); +void use(View); + //===----------------------------------------------------------------------===// // Basic Definite Use-After-Free (-W...permissive) // These are cases where the pointer is guaranteed to be dangling at the use site. @@ -1066,6 +1069,7 @@ void parentheses(bool cond) { (void)*p; // expected-note 4 {{later used here}} } +<<<<<<< HEAD namespace GH162834 { // https://github.com/llvm/llvm-project/issues/162834 template <class T> @@ -1358,3 +1362,11 @@ void add(int c, MyObj* node) { arr[4] = node; } } // namespace CppCoverage +======= +void use_temporary_after_destruction() { + View a; + a = temporary(); // expected-warning {{object whose reference is captured does not live long enough}} \ + expected-note {{destroyed here}} + use(a); // expected-note {{later used here}} +} +>>>>>>> db6fafac81a4 (Add support for loans to temporary materializations) diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp b/clang/unittests/Analysis/LifetimeSafetyTest.cpp index b1f74a7a2c850..f8b19cc435d1a 100644 --- a/clang/unittests/Analysis/LifetimeSafetyTest.cpp +++ b/clang/unittests/Analysis/LifetimeSafetyTest.cpp @@ -9,6 +9,7 @@ #include "clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Analysis/Analyses/LifetimeSafety/Loans.h" #include "clang/Testing/TestAST.h" #include "llvm/ADT/StringMap.h" #include "gmock/gmock.h" @@ -122,7 +123,7 @@ class LifetimeTestHelper { std::vector<LoanID> LID; for (const Loan *L : Analysis.getFactManager().getLoanMgr().getLoans()) if (const auto *BL = dyn_cast<PathLoan>(L)) - if (BL->getAccessPath().D == VD) + if (BL->getAccessPath().getAsValueDecl() == VD) LID.push_back(L->getID()); if (LID.empty()) { ADD_FAILURE() << "Loan for '" << VarName << "' not found."; @@ -131,6 +132,14 @@ class LifetimeTestHelper { return LID; } + bool isLoanToATemporary(LoanID LID) { + const Loan *L = Analysis.getFactManager().getLoanMgr().getLoan(LID); + if (const auto *BL = dyn_cast<PathLoan>(L)) { + return BL->getAccessPath().getAsCXXBindTemporaryExpr() != nullptr; + } + return false; + } + // Gets the set of loans that are live at the given program point. A loan is // considered live at point P if there is a live origin which contains this // loan. @@ -408,6 +417,35 @@ MATCHER_P(AreLiveAt, Annotation, "") { arg, result_listener); } +MATCHER_P(HasLoanToATemporary, Annotation, "") { + const OriginInfo &Info = arg; + auto &Helper = Info.Helper; + std::optional<OriginID> OIDOpt = Helper.getOriginForDecl(Info.OriginVar); + if (!OIDOpt) { + *result_listener << "could not find origin for '" << Info.OriginVar.str() + << "'"; + return false; + } + + std::optional<LoanSet> LoansSetOpt = + Helper.getLoansAtPoint(*OIDOpt, Annotation); + if (!LoansSetOpt) { + *result_listener << "could not get a valid loan set at point '" + << Annotation << "'"; + return false; + } + + std::vector<LoanID> Loans(LoansSetOpt->begin(), LoansSetOpt->end()); + + for (LoanID LID : Loans) { + if (Helper.isLoanToATemporary(LID)) + return true; + } + *result_listener << "could not find loan to a temporary for '" + << Info.OriginVar.str() << "'"; + return false; +} + // Base test fixture to manage the runner and helper. class LifetimeAnalysisTest : public ::testing::Test { protected: @@ -813,12 +851,15 @@ TEST_F(LifetimeAnalysisTest, ExtraParenthesis) { TEST_F(LifetimeAnalysisTest, ViewFromTemporary) { SetupTest(R"( MyObj temporary(); + void use(View); void target() { - View v = temporary(); - POINT(p1); + View a; + a = temporary(); + POINT(p1); + use(a); } )"); - EXPECT_THAT(Origin("v"), HasLoansTo({}, "p1")); + EXPECT_THAT(Origin("a"), HasLoanToATemporary("p1")); } TEST_F(LifetimeAnalysisTest, GslPointerWithConstAndAuto) { >From caa2e25c12f49d106940b03fdbe56299b029d836 Mon Sep 17 00:00:00 2001 From: Abhinav Pradeep <[email protected]> Date: Mon, 15 Dec 2025 19:28:41 +1000 Subject: [PATCH 2/5] Use MaterializeTemporaryExpr to represent temporary storage --- .../Analysis/Analyses/LifetimeSafety/Loans.h | 14 ++++---- .../LifetimeSafety/FactsGenerator.cpp | 34 ++++++++++++++----- clang/lib/Analysis/LifetimeSafety/Loans.cpp | 6 ++-- clang/test/Sema/warn-lifetime-safety.cpp | 3 ++ .../unittests/Analysis/LifetimeSafetyTest.cpp | 3 +- 5 files changed, 40 insertions(+), 20 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h index 96b34e52529e3..6dfe4c8fa6b9c 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h @@ -30,24 +30,24 @@ inline llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, LoanID ID) { /// variable. /// TODO: Model access paths of other types, e.g., s.field, heap and globals. struct AccessPath { - // Currently, an access path can be: + // An access path can be: // - ValueDecl * , to represent the storage location corresponding to the // variable declared in ValueDecl. - // - CXXBindTemporaryExpr * , to represent the storage location of the - // temporary object used for the binding in CXXBindTemporaryExpr. + // - MaterializeTemporaryExpr * , to represent the storage location of the + // temporary object materialized via this MaterializeTemporaryExpr. const llvm::PointerUnion<const clang::ValueDecl *, - const clang::CXXBindTemporaryExpr *> + const clang::MaterializeTemporaryExpr *> P; AccessPath(const clang::ValueDecl *D) : P(D) {} - AccessPath(const clang::CXXBindTemporaryExpr *BTE) : P(BTE) {} + AccessPath(const clang::MaterializeTemporaryExpr *MTE) : P(MTE) {} const clang::ValueDecl *getAsValueDecl() const { return P.dyn_cast<const clang::ValueDecl *>(); } - const clang::CXXBindTemporaryExpr *getAsCXXBindTemporaryExpr() const { - return P.dyn_cast<const clang::CXXBindTemporaryExpr *>(); + const clang::MaterializeTemporaryExpr *getAsMaterializeTemporaryExpr() const { + return P.dyn_cast<const clang::MaterializeTemporaryExpr *>(); } }; diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 1582e5328ddf4..0e94890e8d678 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -71,10 +71,24 @@ static const PathLoan *createLoan(FactManager &FactMgr, return nullptr; } +/// Creates a loan for the storage location of a temporary object. +/// \param MTW The MaterializeTemporaryExpr that represents the temporary +/// binding. \return The new Loan. static const PathLoan *createLoan(FactManager &FactMgr, - const CXXBindTemporaryExpr *BTE) { - AccessPath Path(BTE); - return FactMgr.getLoanMgr().createLoan<PathLoan>(Path, BTE); + const MaterializeTemporaryExpr *MTE) { + AccessPath Path(MTE); + 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() { @@ -355,8 +369,9 @@ void FactsGenerator::VisitMaterializeTemporaryExpr( // TODO: Issue a loan to the MTE. flow(MTEList, SubExprList, /*Kill=*/true); } else { - assert(MTE->isXValue()); - flow(MTEList, SubExprList, /*Kill=*/true); + // A temporary object's origin is the same as the origin of the + // expression that initializes it. + killAndFlowOrigin(*MTE, *MTE->getSubExpr()); } } @@ -390,11 +405,14 @@ void FactsGenerator::handleTemporaryDtor( if (const auto *BL = 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 = BL->getAccessPath(); - const CXXBindTemporaryExpr *Path = AP.getAsCXXBindTemporaryExpr(); - if (Path == BTE) + const AccessPath &AP = BL->getAccessPath(); + const MaterializeTemporaryExpr *Path = AP.getAsMaterializeTemporaryExpr(); + if (!Path) + continue; + if (BTE == getChildBinding(Path)) { CurrentBlockFacts.push_back(FactMgr.createFact<ExpireFact>( BL->getID(), TemporaryDtor.getBindTemporaryExpr()->getEndLoc())); + } } } } diff --git a/clang/lib/Analysis/LifetimeSafety/Loans.cpp b/clang/lib/Analysis/LifetimeSafety/Loans.cpp index 5f6ba6809d2fb..7d105e6a96967 100644 --- a/clang/lib/Analysis/LifetimeSafety/Loans.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Loans.cpp @@ -14,10 +14,10 @@ void PathLoan::dump(llvm::raw_ostream &OS) const { OS << getID() << " (Path: "; if (const clang::ValueDecl *VD = Path.getAsValueDecl()) { OS << VD->getNameAsString(); - } else if (const clang::CXXBindTemporaryExpr *BTE = - Path.getAsCXXBindTemporaryExpr()) { + } else if (const clang::MaterializeTemporaryExpr *MTE = + Path.getAsMaterializeTemporaryExpr()) { // No nice "name" for the temporary, so deferring to LLVM default - OS << "CXXBindTemporaryExpr at " << BTE; + OS << "MaterializeTemporaryExpr at " << MTE; } else { llvm_unreachable("access path is not one of any supported types"); } diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp index 344b783dcbdee..8ffc5d4c86c53 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -1369,4 +1369,7 @@ void use_temporary_after_destruction() { expected-note {{destroyed here}} use(a); // expected-note {{later used here}} } +<<<<<<< HEAD >>>>>>> db6fafac81a4 (Add support for loans to temporary materializations) +======= +>>>>>>> 51a8c9cd6e24 (Use MaterializeTemporaryExpr to represent temporary storage) diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp b/clang/unittests/Analysis/LifetimeSafetyTest.cpp index f8b19cc435d1a..4c6a92836c4b1 100644 --- a/clang/unittests/Analysis/LifetimeSafetyTest.cpp +++ b/clang/unittests/Analysis/LifetimeSafetyTest.cpp @@ -135,7 +135,7 @@ class LifetimeTestHelper { bool isLoanToATemporary(LoanID LID) { const Loan *L = Analysis.getFactManager().getLoanMgr().getLoan(LID); if (const auto *BL = dyn_cast<PathLoan>(L)) { - return BL->getAccessPath().getAsCXXBindTemporaryExpr() != nullptr; + return BL->getAccessPath().getAsMaterializeTemporaryExpr() != nullptr; } return false; } @@ -847,7 +847,6 @@ TEST_F(LifetimeAnalysisTest, ExtraParenthesis) { EXPECT_THAT(Origin("p"), HasLoansTo({"a"}, "p1")); } -// FIXME: Handle temporaries. TEST_F(LifetimeAnalysisTest, ViewFromTemporary) { SetupTest(R"( MyObj temporary(); >From cbe79b151d5407e7d2683ff76068de4f8e945b20 Mon Sep 17 00:00:00 2001 From: Abhinav Pradeep <[email protected]> Date: Fri, 19 Dec 2025 21:15:01 +1000 Subject: [PATCH 3/5] Adapted functionality to multi-origin model, and handled lifetime-extended temporaries --- .../LifetimeSafety/FactsGenerator.cpp | 13 ++++-- clang/lib/Analysis/LifetimeSafety/Origins.cpp | 4 ++ clang/test/Sema/warn-lifetime-safety.cpp | 44 +++++++++++++------ .../unittests/Analysis/LifetimeSafetyTest.cpp | 3 +- 4 files changed, 46 insertions(+), 18 deletions(-) diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 0e94890e8d678..f1c7c1c512ac9 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -72,7 +72,7 @@ static const PathLoan *createLoan(FactManager &FactMgr, } /// Creates a loan for the storage location of a temporary object. -/// \param MTW The MaterializeTemporaryExpr that represents the temporary +/// \param MTE The MaterializeTemporaryExpr that represents the temporary /// binding. \return The new Loan. static const PathLoan *createLoan(FactManager &FactMgr, const MaterializeTemporaryExpr *MTE) { @@ -358,7 +358,8 @@ void FactsGenerator::VisitInitListExpr(const InitListExpr *ILE) { void FactsGenerator::VisitMaterializeTemporaryExpr( const MaterializeTemporaryExpr *MTE) { OriginList *MTEList = getOriginsList(*MTE); - if (!MTEList) + // Here we also defer from handling lifetime extended materializations. + if (!MTEList || MTE->getStorageDuration() != SD_FullExpression) return; OriginList *SubExprList = getOriginsList(*MTE->getSubExpr()); if (MTE->isGLValue()) { @@ -366,7 +367,13 @@ void FactsGenerator::VisitMaterializeTemporaryExpr( MTEList->getLength() == SubExprList->getLength() + 1 && "MTE top level origin should contain a loan to the MTE itself"); MTEList = getRValueOrigins(MTE, MTEList); - // TODO: Issue a loan to the MTE. + if (getChildBinding(MTE)) { + // Issue a loan to MTE for the storage location represented by MTE. + const Loan *L = createLoan(FactMgr, MTE); + OriginList *List = getOriginsList(*MTE); + CurrentBlockFacts.push_back( + FactMgr.createFact<IssueFact>(L->getID(), List->getOuterOriginID())); + } flow(MTEList, SubExprList, /*Kill=*/true); } else { // A temporary object's origin is the same as the origin of the diff --git a/clang/lib/Analysis/LifetimeSafety/Origins.cpp b/clang/lib/Analysis/LifetimeSafety/Origins.cpp index f7153f23cbfd5..9d02831efc98d 100644 --- a/clang/lib/Analysis/LifetimeSafety/Origins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Origins.cpp @@ -14,6 +14,7 @@ #include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/RecursiveASTVisitor.h" +#include "clang/AST/ExprCXX.h" #include "clang/AST/TypeBase.h" #include "clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h" #include "clang/Analysis/Analyses/LifetimeSafety/LifetimeStats.h" @@ -129,6 +130,9 @@ OriginList *OriginManager::getOrCreateList(const Expr *E) { if (auto *EC = dyn_cast<ExprWithCleanups>(E)) return getOrCreateList(EC->getSubExpr()); + if (const ExprWithCleanups *EWC = dyn_cast<ExprWithCleanups>(E)) + return getOrCreateList(EWC->getSubExpr()); + if (!hasOrigins(E)) return nullptr; diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp index 8ffc5d4c86c53..7e7994aa09ec9 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -10,8 +10,13 @@ struct [[gsl::Owner]] MyObj { View getView() const [[clang::lifetimebound]]; }; +struct [[gsl::Owner]] MyTrivialObj { + int id; +}; + struct [[gsl::Pointer()]] View { View(const MyObj&); // Borrows from MyObj + View(const MyTrivialObj &); // Borrows from MyTrivialObj View(); void use() const; }; @@ -20,7 +25,11 @@ class TriviallyDestructedClass { View a, b; }; -MyObj temporary(); +MyObj non_trivially_destructed_temporary(); +MyTrivialObj trivially_destructed_temporary(); +View construct_view(const MyObj &obj [[clang::lifetimebound]]) { + return View(obj); +} void use(View); //===----------------------------------------------------------------------===// @@ -1069,7 +1078,27 @@ void parentheses(bool cond) { (void)*p; // expected-note 4 {{later used here}} } -<<<<<<< HEAD +void use_temporary_after_destruction() { + View a; + a = non_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}} +} + +void passing_temporary_to_lifetime_bound_function() { + View a = construct_view(non_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}} +} + +// 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); +} + namespace GH162834 { // https://github.com/llvm/llvm-project/issues/162834 template <class T> @@ -1362,14 +1391,3 @@ void add(int c, MyObj* node) { arr[4] = node; } } // namespace CppCoverage -======= -void use_temporary_after_destruction() { - View a; - a = temporary(); // expected-warning {{object whose reference is captured does not live long enough}} \ - expected-note {{destroyed here}} - use(a); // expected-note {{later used here}} -} -<<<<<<< HEAD ->>>>>>> db6fafac81a4 (Add support for loans to temporary materializations) -======= ->>>>>>> 51a8c9cd6e24 (Use MaterializeTemporaryExpr to represent temporary storage) diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp b/clang/unittests/Analysis/LifetimeSafetyTest.cpp index 4c6a92836c4b1..1b869803e0f27 100644 --- a/clang/unittests/Analysis/LifetimeSafetyTest.cpp +++ b/clang/unittests/Analysis/LifetimeSafetyTest.cpp @@ -1158,7 +1158,6 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundTemplateFunctionReturnVal) { void target() { MyObj a; - // FIXME: Captures a reference to temporary MyObj returned by Identity. View v1 = Identity(a); POINT(p1); @@ -1169,7 +1168,7 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundTemplateFunctionReturnVal) { POINT(p2); } )"); - EXPECT_THAT(Origin("v1"), HasLoansTo({}, "p1")); + EXPECT_THAT(Origin("v1"), HasLoanToATemporary("p1")); EXPECT_THAT(Origin("v2"), HasLoansTo({"b"}, "p2")); EXPECT_THAT(Origin("v3"), HasLoansTo({"v2"}, "p2")); >From f383ba240f552038f3c7255afaec3d4f349963ed Mon Sep 17 00:00:00 2001 From: Abhinav Pradeep <[email protected]> Date: Fri, 19 Dec 2025 21:45:35 +1000 Subject: [PATCH 4/5] Addressed naming/format comments --- .../lib/Analysis/LifetimeSafety/FactsGenerator.cpp | 14 +++++++------- clang/lib/Analysis/LifetimeSafety/Loans.cpp | 9 ++++----- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index f1c7c1c512ac9..ccb12c719118b 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -403,22 +403,22 @@ void FactsGenerator::handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds) { void FactsGenerator::handleTemporaryDtor( const CFGTemporaryDtor &TemporaryDtor) { - const CXXBindTemporaryExpr *BTE = TemporaryDtor.getBindTemporaryExpr(); - if (!BTE) { + const CXXBindTemporaryExpr *ExpiringBTE = + TemporaryDtor.getBindTemporaryExpr(); + if (!ExpiringBTE) return; - } // Iterate through all loans to see if any expire. for (const auto *Loan : FactMgr.getLoanMgr().getLoans()) { - if (const auto *BL = dyn_cast<PathLoan>(Loan)) { + 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 = BL->getAccessPath(); + const AccessPath &AP = PL->getAccessPath(); const MaterializeTemporaryExpr *Path = AP.getAsMaterializeTemporaryExpr(); if (!Path) continue; - if (BTE == getChildBinding(Path)) { + if (ExpiringBTE == getChildBinding(Path)) { CurrentBlockFacts.push_back(FactMgr.createFact<ExpireFact>( - BL->getID(), TemporaryDtor.getBindTemporaryExpr()->getEndLoc())); + PL->getID(), TemporaryDtor.getBindTemporaryExpr()->getEndLoc())); } } } diff --git a/clang/lib/Analysis/LifetimeSafety/Loans.cpp b/clang/lib/Analysis/LifetimeSafety/Loans.cpp index 7d105e6a96967..8a2cd2a39322b 100644 --- a/clang/lib/Analysis/LifetimeSafety/Loans.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Loans.cpp @@ -12,15 +12,14 @@ namespace clang::lifetimes::internal { void PathLoan::dump(llvm::raw_ostream &OS) const { OS << getID() << " (Path: "; - if (const clang::ValueDecl *VD = Path.getAsValueDecl()) { + if (const clang::ValueDecl *VD = Path.getAsValueDecl()) OS << VD->getNameAsString(); - } else if (const clang::MaterializeTemporaryExpr *MTE = - Path.getAsMaterializeTemporaryExpr()) { + else if (const clang::MaterializeTemporaryExpr *MTE = + Path.getAsMaterializeTemporaryExpr()) // No nice "name" for the temporary, so deferring to LLVM default OS << "MaterializeTemporaryExpr at " << MTE; - } else { + else llvm_unreachable("access path is not one of any supported types"); - } OS << ")"; } >From b15d7efa7a785ca42d3d8a927b18f87b5de1e0a2 Mon Sep 17 00:00:00 2001 From: Abhinav Pradeep <[email protected]> Date: Mon, 12 Jan 2026 23:29:26 +1000 Subject: [PATCH 5/5] Removed dead branch from FactsGenerator::VisitMaterializeTemporaryExpr and addressed formatting comments. --- .../LifetimeSafety/FactsGenerator.cpp | 35 +++++++++---------- clang/lib/Analysis/LifetimeSafety/Origins.cpp | 1 - .../unittests/Analysis/LifetimeSafetyTest.cpp | 3 +- 3 files changed, 17 insertions(+), 22 deletions(-) diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index ccb12c719118b..cbefa7a1b9ad6 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -357,29 +357,26 @@ void FactsGenerator::VisitInitListExpr(const InitListExpr *ILE) { void FactsGenerator::VisitMaterializeTemporaryExpr( const MaterializeTemporaryExpr *MTE) { + assert(MTE->isGLValue()); + // We defer from handling lifetime extended materializations. + if (MTE->getStorageDuration() != SD_FullExpression) + return; OriginList *MTEList = getOriginsList(*MTE); - // Here we also defer from handling lifetime extended materializations. - if (!MTEList || MTE->getStorageDuration() != SD_FullExpression) + if (!MTEList) return; OriginList *SubExprList = getOriginsList(*MTE->getSubExpr()); - if (MTE->isGLValue()) { - assert(!SubExprList || - MTEList->getLength() == SubExprList->getLength() + 1 && - "MTE top level origin should contain a loan to the MTE itself"); - MTEList = getRValueOrigins(MTE, MTEList); - if (getChildBinding(MTE)) { - // Issue a loan to MTE for the storage location represented by MTE. - const Loan *L = createLoan(FactMgr, MTE); - OriginList *List = getOriginsList(*MTE); - CurrentBlockFacts.push_back( - FactMgr.createFact<IssueFact>(L->getID(), List->getOuterOriginID())); - } - flow(MTEList, SubExprList, /*Kill=*/true); - } else { - // A temporary object's origin is the same as the origin of the - // expression that initializes it. - killAndFlowOrigin(*MTE, *MTE->getSubExpr()); + assert(!SubExprList || + MTEList->getLength() == SubExprList->getLength() + 1 && + "MTE top level origin should contain a loan to the MTE itself"); + MTEList = getRValueOrigins(MTE, MTEList); + if (getChildBinding(MTE)) { + // Issue a loan to MTE for the storage location represented by MTE. + const Loan *L = createLoan(FactMgr, MTE); + OriginList *List = getOriginsList(*MTE); + CurrentBlockFacts.push_back( + FactMgr.createFact<IssueFact>(L->getID(), List->getOuterOriginID())); } + flow(MTEList, SubExprList, /*Kill=*/true); } void FactsGenerator::handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds) { diff --git a/clang/lib/Analysis/LifetimeSafety/Origins.cpp b/clang/lib/Analysis/LifetimeSafety/Origins.cpp index 9d02831efc98d..81f4ac056d884 100644 --- a/clang/lib/Analysis/LifetimeSafety/Origins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Origins.cpp @@ -14,7 +14,6 @@ #include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/RecursiveASTVisitor.h" -#include "clang/AST/ExprCXX.h" #include "clang/AST/TypeBase.h" #include "clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h" #include "clang/Analysis/Analyses/LifetimeSafety/LifetimeStats.h" diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp b/clang/unittests/Analysis/LifetimeSafetyTest.cpp index 1b869803e0f27..6b0fb3eb0d24f 100644 --- a/clang/unittests/Analysis/LifetimeSafetyTest.cpp +++ b/clang/unittests/Analysis/LifetimeSafetyTest.cpp @@ -437,10 +437,9 @@ MATCHER_P(HasLoanToATemporary, Annotation, "") { std::vector<LoanID> Loans(LoansSetOpt->begin(), LoansSetOpt->end()); - for (LoanID LID : Loans) { + for (LoanID LID : Loans) if (Helper.isLoanToATemporary(LID)) return true; - } *result_listener << "could not find loan to a temporary for '" << Info.OriginVar.str() << "'"; return false; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
