https://github.com/AbhinavPradeep created https://github.com/llvm/llvm-project/pull/172007
Add support for tracking loans to temporary materializations that require non-trivial destructors This small PR introduces the following changes: 1. AccessPaths can now also represent `CXXBindTemporaryExpr *` via `llvm::PointerUnion` 2. `FactsGenerator::VisitMaterializeTemporaryExpr` now checks to see if the temporary materialization is such that it requires insertion of a non-trivial destructor (by checking for a child `CXXBindTemporaryExpr` node when all implicit casts are stripped away), and if so: creates a Loan whose AccessPath is a pointer to that `CXXBindTemporaryExpr`, and issues it to the origin represented by the `MaterializeTemporaryExpr` node we were called on. 3. `FactsGenerator::handleTemporaryDtor` is called from `FactsGenerator::run` when it encounters a `CFGTemporaryDtor`. It then issues an `ExpireFact` for loan to the corresponding destructed temporary by matching on `CXXBindTemporaryExpr *` 4. Unit tests are extended via the matcher `HasLoanToATemporary` which can check if an origin has a loan to at least 1 temporary. This is done as we are not nicely and reproducibly able to uniquely identify a loan with `AccessPath` `CXXBindTemporaryExpr *`, so we settle for checking the presence of at least a loan to a temporary. 5. A lit test is added for the case where an already destructed temporary is used. Addresses: [#152514](https://github.com/llvm/llvm-project/issues/152514) >From db6fafac81a4e62da5b0dc0f255cb135060f9610 Mon Sep 17 00:00:00 2001 From: Abhinav Pradeep <[email protected]> Date: Fri, 12 Dec 2025 22:47:33 +1000 Subject: [PATCH] Add support for loans to temporary materializations --- .../Analyses/LifetimeSafety/FactsGenerator.h | 2 + .../Analysis/Analyses/LifetimeSafety/Loans.h | 21 +++++++- .../LifetimeSafety/FactsGenerator.cpp | 48 ++++++++++++++++-- clang/lib/Analysis/LifetimeSafety/Loans.cpp | 11 ++++- clang/test/Sema/warn-lifetime-safety.cpp | 10 ++++ .../unittests/Analysis/LifetimeSafetyTest.cpp | 49 +++++++++++++++++-- 6 files changed, 130 insertions(+), 11 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h index a1acd8615afdd..cccb25bd41037 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h @@ -52,6 +52,8 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> { private: 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 2f270b03996f2..ba75bdec3e6f9 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -41,6 +41,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(); @@ -60,6 +66,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()); @@ -218,9 +227,18 @@ void FactsGenerator::VisitMaterializeTemporaryExpr( const MaterializeTemporaryExpr *MTE) { if (!hasOrigin(MTE)) return; - // A temporary object's origin is the same as the origin of the - // expression that initializes it. - killAndFlowOrigin(*MTE, *MTE->getSubExpr()); + const Expr *Child = MTE->getSubExpr()->IgnoreImpCasts(); + if (const CXXBindTemporaryExpr *BTE = + dyn_cast<const CXXBindTemporaryExpr>(Child)) { + // Issue a loan to MTE for the storage location represented by BTE + const Loan *L = createLoan(FactMgr, BTE); + OriginID OID = FactMgr.getOriginMgr().getOrCreate(*MTE); + CurrentBlockFacts.push_back(FactMgr.createFact<IssueFact>(L->getID(), OID)); + } else { + // A temporary object's origin is the same as the origin of the + // expression that initializes it. + killAndFlowOrigin(*MTE, *MTE->getSubExpr()); + } } void FactsGenerator::handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds) { @@ -233,13 +251,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 1191469e23df1..635489581e236 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -16,6 +16,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. @@ -943,3 +946,10 @@ void parentheses(bool cond) { } // expected-note 4 {{destroyed here}} (void)*p; // expected-note 4 {{later used here}} } + +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}} +} \ No newline at end of file diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp b/clang/unittests/Analysis/LifetimeSafetyTest.cpp index fee4e79e27d03..86e5d662880b6 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" @@ -116,7 +117,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."; @@ -125,6 +126,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. @@ -402,6 +411,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: @@ -807,12 +845,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) { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
