[clang] c6013a1 - [LifetimeSafety] Add support for tracking non-trivially destructed temporary objects (#172007)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 13 05:30:14 PST 2026
Author: Abhinav Pradeep
Date: 2026-01-13T13:30:10Z
New Revision: c6013a196611f88672537961c8cc90237f353dad
URL: https://github.com/llvm/llvm-project/commit/c6013a196611f88672537961c8cc90237f353dad
DIFF: https://github.com/llvm/llvm-project/commit/c6013a196611f88672537961c8cc90237f353dad.diff
LOG: [LifetimeSafety] Add support for tracking non-trivially destructed temporary objects (#172007)
Add support for tracking loans to temporary materializations that
require non-trivial destructors. We only support non-trivially
destructed temporaries as they have a nice end-of-life marker via the
`CFGTemporaryDtor`.
This small PR introduces the following changes:
1. AccessPaths can now also represent `MaterializeTemporaryExpr *` via
`llvm::PointerUnion`
3. `FactsGenerator::VisitMaterializeTemporaryExpr` now checks to see if
the temporary materialization is such that it requires 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 `MaterializeTemporaryExpr`, and issues
it to the origin represented by the `MaterializeTemporaryExpr` node we
were called on. When we cannot find a child `CXXBindTemporaryExpr`, we
fall-back to an `OriginFlow` as before.
4. `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 *`
5. 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` `MaterializeTemporaryExpr *`, so we settle for
checking the presence of at least a loan to a temporary.
6. 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)
Added:
Modified:
clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h
clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
clang/lib/Analysis/LifetimeSafety/Loans.cpp
clang/lib/Analysis/LifetimeSafety/Origins.cpp
clang/test/Sema/warn-lifetime-safety.cpp
clang/unittests/Analysis/LifetimeSafetyTest.cpp
Removed:
################################################################################
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..6dfe4c8fa6b9c 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;
+ // An access path can be:
+ // - ValueDecl * , to represent the storage location corresponding to the
+ // variable declared in ValueDecl.
+ // - MaterializeTemporaryExpr * , to represent the storage location of the
+ // temporary object materialized via this MaterializeTemporaryExpr.
+ const llvm::PointerUnion<const clang::ValueDecl *,
+ const clang::MaterializeTemporaryExpr *>
+ P;
+
+ AccessPath(const clang::ValueDecl *D) : P(D) {}
+ AccessPath(const clang::MaterializeTemporaryExpr *MTE) : P(MTE) {}
+
+ const clang::ValueDecl *getAsValueDecl() const {
+ return P.dyn_cast<const clang::ValueDecl *>();
+ }
- AccessPath(const clang::ValueDecl *D) : D(D) {}
+ const clang::MaterializeTemporaryExpr *getAsMaterializeTemporaryExpr() const {
+ return P.dyn_cast<const clang::MaterializeTemporaryExpr *>();
+ }
};
/// 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 60b0bf7e1bdac..9c8cc7621a467 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -71,6 +71,26 @@ static const PathLoan *createLoan(FactManager &FactMgr,
return nullptr;
}
+/// Creates a loan for the storage location of a temporary object.
+/// \param MTE The MaterializeTemporaryExpr that represents the temporary
+/// binding. \return The new Loan.
+static const PathLoan *createLoan(FactManager &FactMgr,
+ 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() {
llvm::TimeTraceScope TimeProfile("FactGenerator");
const CFG &Cfg = *AC.getCFG();
@@ -90,6 +110,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());
@@ -336,21 +359,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);
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);
- // TODO: Issue a loan to the MTE.
- flow(MTEList, SubExprList, /*Kill=*/true);
- } else {
- assert(MTE->isXValue());
- flow(MTEList, SubExprList, /*Kill=*/true);
+ 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) {
@@ -363,13 +391,38 @@ 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 *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 *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 (ExpiringBTE == getChildBinding(Path)) {
+ CurrentBlockFacts.push_back(FactMgr.createFact<ExpireFact>(
+ PL->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..8a2cd2a39322b 100644
--- a/clang/lib/Analysis/LifetimeSafety/Loans.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Loans.cpp
@@ -12,7 +12,15 @@ 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::MaterializeTemporaryExpr *MTE =
+ Path.getAsMaterializeTemporaryExpr())
+ // No nice "name" for the temporary, so deferring to LLVM default
+ OS << "MaterializeTemporaryExpr at " << MTE;
+ 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/lib/Analysis/LifetimeSafety/Origins.cpp b/clang/lib/Analysis/LifetimeSafety/Origins.cpp
index f7153f23cbfd5..ca933f612eb08 100644
--- a/clang/lib/Analysis/LifetimeSafety/Origins.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Origins.cpp
@@ -126,8 +126,8 @@ OriginList *OriginManager::getOrCreateList(const Expr *E) {
if (auto *ParenIgnored = E->IgnoreParens(); ParenIgnored != E)
return getOrCreateList(ParenIgnored);
// We do not see CFG stmts for ExprWithCleanups. Simply peel them.
- 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 59cd23e281bfc..774b080425e42 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,6 +25,13 @@ class TriviallyDestructedClass {
View a, b;
};
+MyObj non_trivially_destructed_temporary();
+MyTrivialObj trivially_destructed_temporary();
+View construct_view(const MyObj &obj [[clang::lifetimebound]]) {
+ return View(obj);
+}
+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.
@@ -1064,6 +1076,28 @@ void parentheses(bool cond) {
: &(((cond ? c : d))))); // expected-warning 2 {{object whose reference is captured does not live long enough}}.
} // expected-note 4 {{destroyed here}}
(void)*p; // expected-note 4 {{later used here}}
+
+}
+
+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 {
@@ -1375,7 +1409,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 {{later used here}}
} // expected-note {{destroyed here}}
(void)x; // expected-note {{used here}}
}
diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp b/clang/unittests/Analysis/LifetimeSafetyTest.cpp
index 5c892b656d5c8..4bc7bcf371544 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().getAsMaterializeTemporaryExpr() != 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,34 @@ 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:
@@ -809,16 +846,18 @@ TEST_F(LifetimeAnalysisTest, ExtraParenthesis) {
EXPECT_THAT(Origin("p"), HasLoansTo({"a"}, "p1"));
}
-// FIXME: Handle temporaries.
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) {
@@ -1118,7 +1157,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);
@@ -1129,7 +1167,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"));
More information about the cfe-commits
mailing list