[clang] [LifetimeSafety] Introduce AccessPath-based expiry (PR #187708)
Utkarsh Saxena via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 20 07:33:11 PDT 2026
https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/187708
>From f2cd3ea2f7b0bddfa2732d75166c9f12c5eb8e4e Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Fri, 20 Mar 2026 14:25:35 +0000
Subject: [PATCH] Expire AccessPaths instead of loans
---
.../Analysis/Analyses/LifetimeSafety/Facts.h | 13 +-
.../Analysis/Analyses/LifetimeSafety/Loans.h | 183 ++++++++----------
clang/lib/Analysis/LifetimeSafety/Checker.cpp | 101 ++++------
clang/lib/Analysis/LifetimeSafety/Facts.cpp | 5 +-
.../LifetimeSafety/FactsGenerator.cpp | 53 ++---
clang/lib/Analysis/LifetimeSafety/Loans.cpp | 41 +++-
.../Analysis/LifetimeSafety/MovedLoans.cpp | 10 +-
.../Sema/warn-lifetime-safety-dataflow.cpp | 20 +-
clang/test/Sema/warn-lifetime-safety.cpp | 16 +-
.../unittests/Analysis/LifetimeSafetyTest.cpp | 18 +-
10 files changed, 206 insertions(+), 254 deletions(-)
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h
index fdcf317c69cbf..0f848abd913d3 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h
@@ -102,8 +102,13 @@ class IssueFact : public Fact {
const OriginManager &OM) const override;
};
+/// When an AccessPath expires (e.g., a variable goes out of scope), all loans
+/// that are associated with this path expire. For example, if `x` expires, then
+/// the loan to `x` expires.
class ExpireFact : public Fact {
- LoanID LID;
+ // The access path that expires.
+ AccessPath AP;
+
// Expired origin (e.g., its variable goes out of scope).
std::optional<OriginID> OID;
SourceLocation ExpiryLoc;
@@ -111,11 +116,11 @@ class ExpireFact : public Fact {
public:
static bool classof(const Fact *F) { return F->getKind() == Kind::Expire; }
- ExpireFact(LoanID LID, SourceLocation ExpiryLoc,
+ ExpireFact(AccessPath AP, SourceLocation ExpiryLoc,
std::optional<OriginID> OID = std::nullopt)
- : Fact(Kind::Expire), LID(LID), OID(OID), ExpiryLoc(ExpiryLoc) {}
+ : Fact(Kind::Expire), AP(AP), OID(OID), ExpiryLoc(ExpiryLoc) {}
- LoanID getLoanID() const { return LID; }
+ const AccessPath &getAccessPath() const { return AP; }
std::optional<OriginID> getOriginID() const { return OID; }
SourceLocation getExpiryLoc() const { return ExpiryLoc; }
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h
index 9aaf4627ce5ad..b1a2241f2fcf6 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h
@@ -27,120 +27,93 @@ inline llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, LoanID ID) {
return OS << ID.Value;
}
+/// Represents the base of a placeholder access path, which is either a
+/// function parameter or the implicit 'this' object of an instance method.
+/// Placeholder paths never expire within the function scope, as they represent
+/// storage from the caller's scope.
+class PlaceholderBase {
+ llvm::PointerUnion<const ParmVarDecl *, const CXXMethodDecl *> ParamOrMethod;
+
+public:
+ PlaceholderBase(const ParmVarDecl *PVD) : ParamOrMethod(PVD) {}
+ PlaceholderBase(const CXXMethodDecl *MD) : ParamOrMethod(MD) {}
+ const ParmVarDecl *getParmVarDecl() const {
+ return ParamOrMethod.dyn_cast<const ParmVarDecl *>();
+ }
+ const CXXMethodDecl *getMethodDecl() const {
+ return ParamOrMethod.dyn_cast<const CXXMethodDecl *>();
+ }
+};
+
/// Represents the storage location being borrowed, e.g., a specific stack
-/// variable.
-/// TODO: Model access paths of other types, e.g., s.field, heap and globals.
+/// variable or a field within it: var.field.*
+///
+/// An AccessPath consists of:
+/// - A base: either a ValueDecl, MaterializeTemporaryExpr, or PlaceholderBase
+/// - A sequence of PathElements representing field accesses or interior
+/// regions
+///
+/// Examples:
+/// - `x` -> Base=x, Elements=[]
+/// - `x.field` -> Base=x, Elements=[.field]
+/// - `x.*` (e.g., string_view from string) -> Base=x, Elements=[.*]
+/// - `x.field.*` -> Base=x, Elements=[.field, .*]
+/// - `$param.field` -> Base=$param, Elements=[.field]
+///
+/// TODO: Model access paths of other types, e.g. heap and globals.
class AccessPath {
- // 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.
+ /// The base of the access path: a variable, temporary, or placeholder.
const llvm::PointerUnion<const clang::ValueDecl *,
- const clang::MaterializeTemporaryExpr *>
- P;
+ const clang::MaterializeTemporaryExpr *,
+ const PlaceholderBase *>
+ Base;
public:
- AccessPath(const clang::ValueDecl *D) : P(D) {}
- AccessPath(const clang::MaterializeTemporaryExpr *MTE) : P(MTE) {}
-
+ AccessPath(const clang::ValueDecl *D) : Base(D) {}
+ AccessPath(const clang::MaterializeTemporaryExpr *MTE) : Base(MTE) {}
+ AccessPath(const PlaceholderBase *PB) : Base(PB) {}
+ /// Creates an extended access path by appending a path element.
+ /// Example: AccessPath(x_path, field) creates path to `x.field`.
+ AccessPath(const AccessPath &Other) : Base(Other.Base) {}
const clang::ValueDecl *getAsValueDecl() const {
- return P.dyn_cast<const clang::ValueDecl *>();
+ return Base.dyn_cast<const clang::ValueDecl *>();
}
-
const clang::MaterializeTemporaryExpr *getAsMaterializeTemporaryExpr() const {
- return P.dyn_cast<const clang::MaterializeTemporaryExpr *>();
+ return Base.dyn_cast<const clang::MaterializeTemporaryExpr *>();
}
-
- bool operator==(const AccessPath &RHS) const { return P == RHS.P; }
+ const PlaceholderBase *getAsPlaceholderBase() const {
+ return Base.dyn_cast<const PlaceholderBase *>();
+ }
+ bool operator==(const AccessPath &RHS) const { return Base == RHS.Base; }
+ bool operator!=(const AccessPath &RHS) const { return !(Base == RHS.Base); }
+ void dump(llvm::raw_ostream &OS) const;
};
-/// An abstract base class for a single "Loan" which represents lending a
-/// storage in memory.
+/// Represents lending a storage location.
+//
+/// A loan tracks the borrowing relationship created by operations like
+/// taking a pointer/reference (&x), creating a view (std::string_view sv = s),
+/// or receiving a parameter.
+///
+/// Examples:
+/// - `int* p = &x;` creates a loan to `x`
+/// - `std::string_view v = s;` creates a loan to `s.*` (interior) [FIXME]
+/// - `int* p = &obj.field;` creates a loan to `obj.field` [FIXME]
+/// - Parameter loans have no IssueExpr (created at function entry)
class Loan {
- /// TODO: Represent opaque loans.
- /// TODO: Represent nullptr: loans to no path. Accessing it UB! Currently it
- /// is represented as empty LoanSet
-public:
- enum class Kind : uint8_t {
- /// A loan with an access path to a storage location.
- Path,
- /// A non-expiring placeholder loan for a parameter, representing a borrow
- /// from the function's caller.
- Placeholder
- };
-
- Loan(Kind K, LoanID ID) : K(K), ID(ID) {}
- virtual ~Loan() = default;
-
- Kind getKind() const { return K; }
- LoanID getID() const { return ID; }
-
- virtual void dump(llvm::raw_ostream &OS) const = 0;
-
-private:
- const Kind K;
const LoanID ID;
-};
-
-/// PathLoan represents lending a storage location that is visible within the
-/// function's scope (e.g., a local variable on stack).
-class PathLoan : public Loan {
- AccessPath Path;
- /// The expression that creates the loan, e.g., &x.
+ const AccessPath Path;
+ /// The expression that creates the loan, e.g., &x. Null for placeholder
+ /// loans.
const Expr *IssueExpr;
public:
- PathLoan(LoanID ID, AccessPath Path, const Expr *IssueExpr)
- : Loan(Kind::Path, ID), Path(Path), IssueExpr(IssueExpr) {}
-
+ Loan(LoanID ID, AccessPath Path, const Expr *IssueExpr = nullptr)
+ : ID(ID), Path(Path), IssueExpr(IssueExpr) {}
+ LoanID getID() const { return ID; }
const AccessPath &getAccessPath() const { return Path; }
const Expr *getIssueExpr() const { return IssueExpr; }
-
- void dump(llvm::raw_ostream &OS) const override;
-
- static bool classof(const Loan *L) { return L->getKind() == Kind::Path; }
-};
-
-/// A placeholder loan held by a function parameter or an implicit 'this'
-/// object, representing a borrow from the caller's scope.
-///
-/// Created at function entry for each pointer or reference parameter or for
-/// the implicit 'this' parameter of instance methods, with an
-/// origin. Unlike PathLoan, placeholder loans:
-/// - Have no IssueExpr (created at function entry, not at a borrow site)
-/// - Have no AccessPath (the borrowed object is not visible to the function)
-/// - Do not currently expire, but may in the future when modeling function
-/// invalidations (e.g., vector::push_back)
-///
-/// When a placeholder loan escapes the function (e.g., via return), it
-/// indicates the parameter or method should be marked [[clang::lifetimebound]],
-/// enabling lifetime annotation suggestions.
-class PlaceholderLoan : public Loan {
- /// The function parameter or method (representing 'this') that holds this
- /// placeholder loan.
- llvm::PointerUnion<const ParmVarDecl *, const CXXMethodDecl *> ParamOrMethod;
-
-public:
- PlaceholderLoan(LoanID ID, const ParmVarDecl *PVD)
- : Loan(Kind::Placeholder, ID), ParamOrMethod(PVD) {}
-
- PlaceholderLoan(LoanID ID, const CXXMethodDecl *MD)
- : Loan(Kind::Placeholder, ID), ParamOrMethod(MD) {}
-
- const ParmVarDecl *getParmVarDecl() const {
- return ParamOrMethod.dyn_cast<const ParmVarDecl *>();
- }
-
- const CXXMethodDecl *getMethodDecl() const {
- return ParamOrMethod.dyn_cast<const CXXMethodDecl *>();
- }
-
- void dump(llvm::raw_ostream &OS) const override;
-
- static bool classof(const Loan *L) {
- return L->getKind() == Kind::Placeholder;
- }
+ void dump(llvm::raw_ostream &OS) const;
};
/// Manages the creation, storage and retrieval of loans.
@@ -148,15 +121,9 @@ class LoanManager {
public:
LoanManager() = default;
- template <typename LoanType, typename... Args>
- LoanType *createLoan(Args &&...args) {
- static_assert(
- std::is_same_v<LoanType, PathLoan> ||
- std::is_same_v<LoanType, PlaceholderLoan>,
- "createLoan can only be used with PathLoan or PlaceholderLoan");
- void *Mem = LoanAllocator.Allocate<LoanType>();
- auto *NewLoan =
- new (Mem) LoanType(getNextLoanID(), std::forward<Args>(args)...);
+ Loan *createLoan(AccessPath Path, const Expr *IssueExpr = nullptr) {
+ void *Mem = LoanAllocator.Allocate<Loan>();
+ auto *NewLoan = new (Mem) Loan(getNextLoanID(), Path, IssueExpr);
AllLoans.push_back(NewLoan);
return NewLoan;
}
@@ -165,6 +132,11 @@ class LoanManager {
assert(ID.Value < AllLoans.size());
return AllLoans[ID.Value];
}
+
+ /// Gets or creates a placeholder base for a given parameter or method.
+ const PlaceholderBase *getOrCreatePlaceholderBase(const ParmVarDecl *PVD);
+ const PlaceholderBase *getOrCreatePlaceholderBase(const CXXMethodDecl *MD);
+
llvm::ArrayRef<const Loan *> getLoans() const { return AllLoans; }
private:
@@ -174,6 +146,7 @@ class LoanManager {
/// TODO(opt): Profile and evaluate the usefullness of small buffer
/// optimisation.
llvm::SmallVector<const Loan *> AllLoans;
+ llvm::DenseMap<const Decl *, const PlaceholderBase *> PlaceholderBases;
llvm::BumpPtrAllocator LoanAllocator;
};
} // namespace clang::lifetimes::internal
diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp
index db87f511a230f..86c7d5c70558f 100644
--- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp
@@ -139,60 +139,47 @@ class LifetimeChecker {
};
for (LoanID LID : EscapedLoans) {
const Loan *L = FactMgr.getLoanMgr().getLoan(LID);
- const auto *PL = dyn_cast<PlaceholderLoan>(L);
- if (!PL)
+ const PlaceholderBase *PB = L->getAccessPath().getAsPlaceholderBase();
+ if (!PB)
continue;
- if (const auto *PVD = PL->getParmVarDecl())
+ if (const auto *PVD = PB->getParmVarDecl())
CheckParam(PVD);
- else if (const auto *MD = PL->getMethodDecl())
+ else if (const auto *MD = PB->getMethodDecl())
CheckImplicitThis(MD);
}
}
- /// Checks for use-after-free & use-after-return errors when a loan expires.
+ /// Checks for use-after-free & use-after-return errors when an access path
+ /// expires (e.g., a variable goes out of scope).
///
- /// This method examines all live origins at the expiry point and determines
- /// if any of them hold the expiring loan. If so, it creates a pending
- /// warning.
- ///
- /// Note: This implementation considers only the confidence of origin
- /// liveness. Future enhancements could also consider the confidence of loan
- /// propagation (e.g., a loan may only be held on some execution paths).
+ /// When a path expires, all loans having this path expires.
+ /// This method examines all live origins and reports warnings for loans they
+ /// hold that are prefixed by the expired path.
void checkExpiry(const ExpireFact *EF) {
- LoanID ExpiredLoan = EF->getLoanID();
- const Expr *MovedExpr = nullptr;
- if (auto *ME = MovedLoans.getMovedLoans(EF).lookup(ExpiredLoan))
- MovedExpr = *ME;
-
+ const AccessPath &ExpiredPath = EF->getAccessPath();
LivenessMap Origins = LiveOrigins.getLiveOriginsAt(EF);
- bool CurDomination = false;
- // The UseFact or OriginEscapesFact most indicative of a lifetime error,
- // prioritized by earlier source location.
- llvm::PointerUnion<const UseFact *, const OriginEscapesFact *> CausingFact =
- nullptr;
-
for (auto &[OID, LiveInfo] : Origins) {
LoanSet HeldLoans = LoanPropagation.getLoans(OID, EF);
- if (!HeldLoans.contains(ExpiredLoan))
- continue;
- // Loan is defaulted.
-
- if (!CurDomination || causingFactDominatesExpiry(LiveInfo.Kind))
- CausingFact = LiveInfo.CausingFact;
+ for (LoanID HeldLoanID : HeldLoans) {
+ const Loan *HeldLoan = FactMgr.getLoanMgr().getLoan(HeldLoanID);
+ if (ExpiredPath != HeldLoan->getAccessPath())
+ continue;
+ // HeldLoan is expired because its AccessPath is expired.
+ PendingWarning &CurWarning = FinalWarningsMap[HeldLoan->getID()];
+ const Expr *MovedExpr = nullptr;
+ if (auto *ME = MovedLoans.getMovedLoans(EF).lookup(HeldLoanID))
+ MovedExpr = *ME;
+ // Skip if we already have a dominating causing fact.
+ if (CurWarning.CausingFactDominatesExpiry)
+ continue;
+ if (causingFactDominatesExpiry(LiveInfo.Kind))
+ CurWarning.CausingFactDominatesExpiry = true;
+ CurWarning.CausingFact = LiveInfo.CausingFact;
+ CurWarning.ExpiryLoc = EF->getExpiryLoc();
+ CurWarning.MovedExpr = MovedExpr;
+ CurWarning.InvalidatedByExpr = nullptr;
+ }
}
- if (!CausingFact)
- return;
-
- bool LastDomination =
- FinalWarningsMap.lookup(ExpiredLoan).CausingFactDominatesExpiry;
- if (LastDomination)
- return;
- FinalWarningsMap[ExpiredLoan] = {
- /*ExpiryLoc=*/EF->getExpiryLoc(),
- /*CausingFact=*/CausingFact,
- /*MovedExpr=*/MovedExpr,
- /*InvalidatedByExpr=*/nullptr,
- /*CausingFactDominatesExpiry=*/CurDomination};
}
/// Checks for use-after-invalidation errors when a container is modified.
@@ -206,18 +193,9 @@ class LifetimeChecker {
LoanSet DirectlyInvalidatedLoans =
LoanPropagation.getLoans(InvalidatedOrigin, IOF);
auto IsInvalidated = [&](const Loan *L) {
- auto *PathL = dyn_cast<PathLoan>(L);
- auto *PlaceholderL = dyn_cast<PlaceholderLoan>(L);
for (LoanID InvalidID : DirectlyInvalidatedLoans) {
- const Loan *L = FactMgr.getLoanMgr().getLoan(InvalidID);
- auto *InvalidPathL = dyn_cast<PathLoan>(L);
- auto *InvalidPlaceholderL = dyn_cast<PlaceholderLoan>(L);
- if (PathL && InvalidPathL &&
- PathL->getAccessPath() == InvalidPathL->getAccessPath())
- return true;
- if (PlaceholderL && InvalidPlaceholderL &&
- PlaceholderL->getParmVarDecl() ==
- InvalidPlaceholderL->getParmVarDecl())
+ const Loan *InvalidL = FactMgr.getLoanMgr().getLoan(InvalidID);
+ if (InvalidL->getAccessPath() == L->getAccessPath())
return true;
}
return false;
@@ -248,13 +226,10 @@ class LifetimeChecker {
return;
for (const auto &[LID, Warning] : FinalWarningsMap) {
const Loan *L = FactMgr.getLoanMgr().getLoan(LID);
-
- const Expr *IssueExpr = nullptr;
- if (const auto *BL = dyn_cast<PathLoan>(L))
- IssueExpr = BL->getIssueExpr();
+ const Expr *IssueExpr = L->getIssueExpr();
const ParmVarDecl *InvalidatedPVD = nullptr;
- if (const auto *PL = dyn_cast<PlaceholderLoan>(L))
- InvalidatedPVD = PL->getParmVarDecl();
+ if (const PlaceholderBase *PB = L->getAccessPath().getAsPlaceholderBase())
+ InvalidatedPVD = PB->getParmVarDecl();
llvm::PointerUnion<const UseFact *, const OriginEscapesFact *>
CausingFact = Warning.CausingFact;
const Expr *MovedExpr = Warning.MovedExpr;
@@ -263,24 +238,30 @@ class LifetimeChecker {
if (const auto *UF = CausingFact.dyn_cast<const UseFact *>()) {
if (Warning.InvalidatedByExpr) {
if (IssueExpr)
+ // Use-after-invalidation of an object on stack.
SemaHelper->reportUseAfterInvalidation(IssueExpr, UF->getUseExpr(),
Warning.InvalidatedByExpr);
- if (InvalidatedPVD)
+ else if (InvalidatedPVD)
+ // Use-after-invalidation of a parameter.
SemaHelper->reportUseAfterInvalidation(
InvalidatedPVD, UF->getUseExpr(), Warning.InvalidatedByExpr);
} else
+ // Scope-based expiry (use-after-scope).
SemaHelper->reportUseAfterFree(IssueExpr, UF->getUseExpr(), MovedExpr,
ExpiryLoc);
} else if (const auto *OEF =
CausingFact.dyn_cast<const OriginEscapesFact *>()) {
if (const auto *RetEscape = dyn_cast<ReturnEscapeFact>(OEF))
+ // Return stack address.
SemaHelper->reportUseAfterReturn(
IssueExpr, RetEscape->getReturnExpr(), MovedExpr, ExpiryLoc);
else if (const auto *FieldEscape = dyn_cast<FieldEscapeFact>(OEF))
+ // Dangling field.
SemaHelper->reportDanglingField(
IssueExpr, FieldEscape->getFieldDecl(), MovedExpr, ExpiryLoc);
else if (const auto *GlobalEscape = dyn_cast<GlobalEscapeFact>(OEF))
+ // Global escape.
SemaHelper->reportDanglingGlobal(IssueExpr, GlobalEscape->getGlobal(),
MovedExpr, ExpiryLoc);
else
diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp
index 535da2a014273..1bc0521a72359 100644
--- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp
@@ -8,10 +8,7 @@
#include "clang/Analysis/Analyses/LifetimeSafety/Facts.h"
#include "clang/AST/Decl.h"
-#include "clang/AST/DeclID.h"
#include "clang/Analysis/Analyses/PostOrderCFGView.h"
-#include "clang/Analysis/FlowSensitive/DataflowWorklist.h"
-#include "llvm/ADT/STLFunctionalExtras.h"
namespace clang::lifetimes::internal {
@@ -32,7 +29,7 @@ void IssueFact::dump(llvm::raw_ostream &OS, const LoanManager &LM,
void ExpireFact::dump(llvm::raw_ostream &OS, const LoanManager &LM,
const OriginManager &OM) const {
OS << "Expire (";
- LM.getLoan(getLoanID())->dump(OS);
+ getAccessPath().dump(OS);
if (auto OID = getOriginID()) {
OS << ", Origin: ";
OM.dump(*OID, OS);
diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
index 3259505584c9f..42cc4d17c39da 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -69,12 +69,11 @@ void FactsGenerator::flow(OriginList *Dst, OriginList *Src, bool Kill) {
/// This function should be called whenever a DeclRefExpr represents a borrow.
/// \param DRE The declaration reference expression that initiates the borrow.
/// \return The new Loan on success, nullptr otherwise.
-static const PathLoan *createLoan(FactManager &FactMgr,
- const DeclRefExpr *DRE) {
+static const Loan *createLoan(FactManager &FactMgr, const DeclRefExpr *DRE) {
if (const auto *VD = dyn_cast<ValueDecl>(DRE->getDecl())) {
AccessPath Path(VD);
// The loan is created at the location of the DeclRefExpr.
- return FactMgr.getLoanMgr().createLoan<PathLoan>(Path, DRE);
+ return FactMgr.getLoanMgr().createLoan(Path, DRE);
}
return nullptr;
}
@@ -82,10 +81,10 @@ static const PathLoan *createLoan(FactManager &FactMgr,
/// 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) {
+static const Loan *createLoan(FactManager &FactMgr,
+ const MaterializeTemporaryExpr *MTE) {
AccessPath Path(MTE);
- return FactMgr.getLoanMgr().createLoan<PathLoan>(Path, MTE);
+ return FactMgr.getLoanMgr().createLoan(Path, MTE);
}
void FactsGenerator::run() {
@@ -506,38 +505,16 @@ void FactsGenerator::handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds) {
if (!escapesViaReturn(OID))
ExpiredOID = OID;
}
- // 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 stack variable and if that variable
- // is the one being destructed.
- const AccessPath AP = BL->getAccessPath();
- const ValueDecl *Path = AP.getAsValueDecl();
- if (Path == LifetimeEndsVD)
- CurrentBlockFacts.push_back(FactMgr.createFact<ExpireFact>(
- BL->getID(), LifetimeEnds.getTriggerStmt()->getEndLoc(),
- ExpiredOID));
- }
- }
+ CurrentBlockFacts.push_back(FactMgr.createFact<ExpireFact>(
+ AccessPath(LifetimeEndsVD), LifetimeEnds.getTriggerStmt()->getEndLoc(),
+ ExpiredOID));
}
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)) {
- // Check if the loan is for a temporary materialization and if that
- // storage location is the one being destructed.
- const AccessPath &AP = PL->getAccessPath();
- const MaterializeTemporaryExpr *Path = AP.getAsMaterializeTemporaryExpr();
- if (!Path)
- continue;
- if (llvm::is_contained(FullExprCleanup.getExpiringMTEs(), Path)) {
- CurrentBlockFacts.push_back(
- FactMgr.createFact<ExpireFact>(PL->getID(), Path->getEndLoc()));
- }
- }
- }
+ for (const auto *MTE : FullExprCleanup.getExpiringMTEs())
+ CurrentBlockFacts.push_back(
+ FactMgr.createFact<ExpireFact>(AccessPath(MTE), MTE->getEndLoc()));
}
void FactsGenerator::handleExitBlock() {
@@ -784,8 +761,9 @@ llvm::SmallVector<Fact *> FactsGenerator::issuePlaceholderLoans() {
llvm::SmallVector<Fact *> PlaceholderLoanFacts;
if (auto ThisOrigins = FactMgr.getOriginMgr().getThisOrigins()) {
OriginList *List = *ThisOrigins;
- const PlaceholderLoan *L = FactMgr.getLoanMgr().createLoan<PlaceholderLoan>(
+ const PlaceholderBase *PB = FactMgr.getLoanMgr().getOrCreatePlaceholderBase(
cast<CXXMethodDecl>(FD));
+ const Loan *L = FactMgr.getLoanMgr().createLoan(AccessPath(PB));
PlaceholderLoanFacts.push_back(
FactMgr.createFact<IssueFact>(L->getID(), List->getOuterOriginID()));
}
@@ -793,8 +771,9 @@ llvm::SmallVector<Fact *> FactsGenerator::issuePlaceholderLoans() {
OriginList *List = getOriginsList(*PVD);
if (!List)
continue;
- const PlaceholderLoan *L =
- FactMgr.getLoanMgr().createLoan<PlaceholderLoan>(PVD);
+ const PlaceholderBase *PB =
+ FactMgr.getLoanMgr().getOrCreatePlaceholderBase(PVD);
+ const Loan *L = FactMgr.getLoanMgr().createLoan(AccessPath(PB));
PlaceholderLoanFacts.push_back(
FactMgr.createFact<IssueFact>(L->getID(), List->getOuterOriginID()));
}
diff --git a/clang/lib/Analysis/LifetimeSafety/Loans.cpp b/clang/lib/Analysis/LifetimeSafety/Loans.cpp
index 8a2cd2a39322b..79c7ae43887be 100644
--- a/clang/lib/Analysis/LifetimeSafety/Loans.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Loans.cpp
@@ -10,21 +10,44 @@
namespace clang::lifetimes::internal {
-void PathLoan::dump(llvm::raw_ostream &OS) const {
- OS << getID() << " (Path: ";
- if (const clang::ValueDecl *VD = Path.getAsValueDecl())
+void AccessPath::dump(llvm::raw_ostream &OS) const {
+ if (const clang::ValueDecl *VD = getAsValueDecl())
OS << VD->getNameAsString();
else if (const clang::MaterializeTemporaryExpr *MTE =
- Path.getAsMaterializeTemporaryExpr())
- // No nice "name" for the temporary, so deferring to LLVM default
+ getAsMaterializeTemporaryExpr())
OS << "MaterializeTemporaryExpr at " << MTE;
- else
- llvm_unreachable("access path is not one of any supported types");
+ else if (const PlaceholderBase *PB = getAsPlaceholderBase()) {
+ if (const auto *PVD = PB->getParmVarDecl())
+ OS << "$" << PVD->getNameAsString();
+ else if (PB->getMethodDecl())
+ OS << "$this";
+ } else
+ llvm_unreachable("access path base invalid");
+}
+
+void Loan::dump(llvm::raw_ostream &OS) const {
+ OS << getID() << " (Path: ";
+ Path.dump(OS);
OS << ")";
}
-void PlaceholderLoan::dump(llvm::raw_ostream &OS) const {
- OS << getID() << " (Placeholder loan)";
+const PlaceholderBase *
+LoanManager::getOrCreatePlaceholderBase(const ParmVarDecl *PVD) {
+ if (auto It = PlaceholderBases.find(PVD); It != PlaceholderBases.end())
+ return It->second;
+ void *Mem = LoanAllocator.Allocate<PlaceholderBase>();
+ PlaceholderBase *NewPB = new (Mem) PlaceholderBase(PVD);
+ PlaceholderBases.insert({PVD, NewPB});
+ return NewPB;
}
+const PlaceholderBase *
+LoanManager::getOrCreatePlaceholderBase(const CXXMethodDecl *MD) {
+ if (auto It = PlaceholderBases.find(MD); It != PlaceholderBases.end())
+ return It->second;
+ void *Mem = LoanAllocator.Allocate<PlaceholderBase>();
+ PlaceholderBase *NewPB = new (Mem) PlaceholderBase(MD);
+ PlaceholderBases.insert({MD, NewPB});
+ return NewPB;
+}
} // namespace clang::lifetimes::internal
diff --git a/clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp b/clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp
index 95de08d4425b0..138704821024a 100644
--- a/clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp
@@ -80,10 +80,7 @@ class AnalysisImpl
auto IsInvalidated = [&](const AccessPath &Path) {
for (LoanID LID : ImmediatelyMovedLoans) {
const Loan *MovedLoan = LoanMgr.getLoan(LID);
- auto *PL = dyn_cast<PathLoan>(MovedLoan);
- if (!PL)
- continue;
- if (PL->getAccessPath() == Path)
+ if (MovedLoan->getAccessPath() == Path)
return true;
}
return false;
@@ -91,10 +88,7 @@ class AnalysisImpl
for (auto [O, _] : LiveOrigins.getLiveOriginsAt(&F))
for (LoanID LiveLoan : LoanPropagation.getLoans(O, &F)) {
const Loan *LiveLoanPtr = LoanMgr.getLoan(LiveLoan);
- auto *PL = dyn_cast<PathLoan>(LiveLoanPtr);
- if (!PL)
- continue;
- if (IsInvalidated(PL->getAccessPath()))
+ if (IsInvalidated(LiveLoanPtr->getAccessPath()))
MovedLoans =
MovedLoansMapFactory.add(MovedLoans, LiveLoan, F.getMoveExpr());
}
diff --git a/clang/test/Sema/warn-lifetime-safety-dataflow.cpp b/clang/test/Sema/warn-lifetime-safety-dataflow.cpp
index be1b77b36a2e0..2cfec824b0866 100644
--- a/clang/test/Sema/warn-lifetime-safety-dataflow.cpp
+++ b/clang/test/Sema/warn-lifetime-safety-dataflow.cpp
@@ -25,8 +25,8 @@ MyObj* return_local_addr() {
// CHECK: OriginFlow:
// CHECK-NEXT: Dest: [[O_RET_VAL:[0-9]+]] (Expr: ImplicitCastExpr, Type : MyObj *)
// CHECK-NEXT: Src: [[O_P]] (Decl: p, Type : MyObj *)
-// CHECK: Expire ([[L_X]] (Path: x))
-// CHECK: Expire ({{[0-9]+}} (Path: p), Origin: [[O_P]] (Decl: p, Type : MyObj *))
+// CHECK: Expire (x)
+// CHECK: Expire (p, Origin: [[O_P]] (Decl: p, Type : MyObj *))
// CHECK: OriginEscapes ([[O_RET_VAL]] (Expr: ImplicitCastExpr, Type : MyObj *), via Return)
}
@@ -43,7 +43,7 @@ void loan_expires_cpp() {
// CHECK: OriginFlow:
// CHECK-NEXT: Dest: {{[0-9]+}} (Decl: pObj, Type : MyObj *)
// CHECK-NEXT: Src: [[O_ADDR_OBJ]] (Expr: UnaryOperator, Type : MyObj *)
-// CHECK: Expire ([[L_OBJ]] (Path: obj))
+// CHECK: Expire (obj)
}
@@ -59,7 +59,7 @@ void loan_expires_trivial() {
// CHECK: OriginFlow:
// CHECK-NEXT: Dest: {{[0-9]+}} (Decl: pTrivialObj, Type : int *)
// CHECK-NEXT: Src: [[O_ADDR_TRIVIAL_OBJ]] (Expr: UnaryOperator, Type : int *)
-// CHECK: Expire ([[L_TRIVIAL_OBJ]] (Path: trivial_obj))
+// CHECK: Expire (trivial_obj)
// CHECK-NEXT: End of Block
}
@@ -86,8 +86,8 @@ void overwrite_origin() {
// CHECK: OriginFlow:
// CHECK-NEXT: Dest: [[O_P]] (Decl: p, Type : MyObj *)
// CHECK-NEXT: Src: [[O_ADDR_S2]] (Expr: UnaryOperator, Type : MyObj *)
-// CHECK: Expire ([[L_S2]] (Path: s2))
-// CHECK: Expire ([[L_S1]] (Path: s1))
+// CHECK: Expire (s2)
+// CHECK: Expire (s1)
}
// CHECK-LABEL: Function: reassign_to_null
@@ -108,7 +108,7 @@ void reassign_to_null() {
// CHECK: OriginFlow:
// CHECK-NEXT: Dest: [[O_P]] (Decl: p, Type : MyObj *)
// CHECK-NEXT: Src: {{[0-9]+}} (Expr: ImplicitCastExpr, Type : MyObj *)
-// CHECK: Expire ([[L_S1]] (Path: s1))
+// CHECK: Expire (s1)
}
// FIXME: Have a better representation for nullptr than just an empty origin.
// It should be a separate loan and origin kind.
@@ -205,8 +205,8 @@ void test_use_lifetimebound_call() {
// CHECK: OriginFlow:
// CHECK-NEXT: Dest: {{[0-9]+}} (Decl: r, Type : MyObj *)
// CHECK-NEXT: Src: [[O_CALL_EXPR]] (Expr: CallExpr, Type : MyObj *)
-// CHECK: Expire ([[L_Y]] (Path: y))
-// CHECK: Expire ([[L_X]] (Path: x))
+// CHECK: Expire (y)
+// CHECK: Expire (x)
}
// CHECK-LABEL: Function: test_reference_variable
@@ -235,5 +235,5 @@ void test_reference_variable() {
// CHECK: OriginFlow:
// CHECK-NEXT: Dest: {{[0-9]+}} (Decl: p, Type : const MyObj *)
// CHECK-NEXT: Src: {{[0-9]+}} (Expr: UnaryOperator, Type : const MyObj *)
-// CHECK: Expire ([[L_X]] (Path: x))
+// CHECK: Expire (x)
}
diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp
index bd09bb70e9a11..cbc4548c3d934 100644
--- a/clang/test/Sema/warn-lifetime-safety.cpp
+++ b/clang/test/Sema/warn-lifetime-safety.cpp
@@ -328,9 +328,9 @@ void multiple_expiry_of_same_loan(bool cond) {
if (cond) {
p = &unsafe; // expected-warning {{does not live long enough}}
if (cond)
- break;
+ break; // expected-note {{destroyed here}}
}
- } // expected-note {{destroyed here}}
+ }
(void)*p; // expected-note {{later used here}}
p = &safe;
@@ -349,8 +349,8 @@ void multiple_expiry_of_same_loan(bool cond) {
if (cond)
p = &unsafe; // expected-warning {{does not live long enough}}
if (cond)
- break;
- } // expected-note {{destroyed here}}
+ break; // expected-note {{destroyed here}}
+ }
(void)*p; // expected-note {{later used here}}
}
@@ -717,12 +717,12 @@ View uar_before_uaf(const MyObj& safe, bool c) {
View p;
{
MyObj local_obj;
- p = local_obj; // expected-warning {{object whose reference is captured does not live long enough}}
+ p = local_obj; // expected-warning {{ddress of stack memory is returned later}}
if (c) {
- return p;
+ return p; // expected-note {{returned here}}
}
- } // expected-note {{destroyed here}}
- p.use(); // expected-note {{later used here}}
+ }
+ p.use();
p = safe;
return p;
}
diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp b/clang/unittests/Analysis/LifetimeSafetyTest.cpp
index 2116f7736c4be..6cf65dd64ef83 100644
--- a/clang/unittests/Analysis/LifetimeSafetyTest.cpp
+++ b/clang/unittests/Analysis/LifetimeSafetyTest.cpp
@@ -125,9 +125,8 @@ 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().getAsValueDecl() == VD)
- LID.push_back(L->getID());
+ if (L->getAccessPath().getAsValueDecl() == VD)
+ LID.push_back(L->getID());
if (LID.empty()) {
ADD_FAILURE() << "Loan for '" << VarName << "' not found.";
return {};
@@ -136,11 +135,11 @@ 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().getAsMaterializeTemporaryExpr() != nullptr;
- }
- return false;
+ return Analysis.getFactManager()
+ .getLoanMgr()
+ .getLoan(LID)
+ ->getAccessPath()
+ .getAsMaterializeTemporaryExpr() != nullptr;
}
// Gets the set of loans that are live at the given program point. A loan is
@@ -169,9 +168,10 @@ class LifetimeTestHelper {
const ExpireFact *
getExpireFactFromAllFacts(const llvm::ArrayRef<const Fact *> &FactsInBlock,
const LoanID &loanID) {
+ const Loan *L = Analysis.getFactManager().getLoanMgr().getLoan(loanID);
for (const Fact *F : FactsInBlock) {
if (auto const *CurrentEF = F->getAs<ExpireFact>())
- if (CurrentEF->getLoanID() == loanID)
+ if (CurrentEF->getAccessPath() == L->getAccessPath())
return CurrentEF;
}
return nullptr;
More information about the cfe-commits
mailing list