[clang] Field and interior paths (PR #180369)
Utkarsh Saxena via cfe-commits
cfe-commits at lists.llvm.org
Sat Feb 7 14:57:02 PST 2026
https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/180369
>From a177ce4e0cfda6818f6f35948235492d3f1df295 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Fri, 6 Feb 2026 11:55:06 +0000
Subject: [PATCH] Field and interior paths
---
.../Analysis/Analyses/LifetimeSafety/Facts.h | 14 +-
.../Analyses/LifetimeSafety/FactsGenerator.h | 3 +-
.../Analysis/Analyses/LifetimeSafety/Loans.h | 246 +++++++++++-------
clang/lib/Analysis/LifetimeSafety/Checker.cpp | 77 +++---
clang/lib/Analysis/LifetimeSafety/Facts.cpp | 6 +-
.../LifetimeSafety/FactsGenerator.cpp | 88 ++++---
.../LifetimeSafety/LoanPropagation.cpp | 18 +-
clang/lib/Analysis/LifetimeSafety/Loans.cpp | 67 ++++-
.../Analysis/LifetimeSafety/MovedLoans.cpp | 10 +-
.../warn-lifetime-safety-invalidations.cpp | 87 ++++---
.../unittests/Analysis/LifetimeSafetyTest.cpp | 10 +-
11 files changed, 371 insertions(+), 255 deletions(-)
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h
index f9d55991f2e09..99950b6f861dd 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h
@@ -14,15 +14,16 @@
#ifndef LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMESAFETY_FACTS_H
#define LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMESAFETY_FACTS_H
+#include <cstdint>
+#include <optional>
+
#include "clang/Analysis/Analyses/LifetimeSafety/Loans.h"
#include "clang/Analysis/Analyses/LifetimeSafety/Origins.h"
#include "clang/Analysis/Analyses/LifetimeSafety/Utils.h"
#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Analysis/CFG.h"
-#include "llvm/ADT/STLFunctionalExtras.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/Debug.h"
-#include <cstdint>
namespace clang::lifetimes::internal {
@@ -123,19 +124,24 @@ class OriginFlowFact : public Fact {
// True if the destination origin should be killed (i.e., its current loans
// cleared) before the source origin's loans are flowed into it.
bool KillDest;
+ // If set, the source origin's loans are extended by this path element before
+ // flowing into the destination.
+ std::optional<PathElement> Element;
public:
static bool classof(const Fact *F) {
return F->getKind() == Kind::OriginFlow;
}
- OriginFlowFact(OriginID OIDDest, OriginID OIDSrc, bool KillDest)
+ OriginFlowFact(OriginID OIDDest, OriginID OIDSrc, bool KillDest,
+ std::optional<PathElement> Element = std::nullopt)
: Fact(Kind::OriginFlow), OIDDest(OIDDest), OIDSrc(OIDSrc),
- KillDest(KillDest) {}
+ KillDest(KillDest), Element(Element) {}
OriginID getDestOriginID() const { return OIDDest; }
OriginID getSrcOriginID() const { return OIDSrc; }
bool getKillDest() const { return KillDest; }
+ std::optional<PathElement> getPathElement() const { return Element; }
void dump(llvm::raw_ostream &OS, const LoanManager &,
const OriginManager &OM) const override;
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
index fb7d5ad91db79..520e9b94c719a 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
@@ -55,7 +55,8 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> {
OriginList *getOriginsList(const ValueDecl &D);
OriginList *getOriginsList(const Expr &E);
- void flow(OriginList *Dst, OriginList *Src, bool Kill);
+ void flow(OriginList *Dst, OriginList *Src, bool Kill,
+ std::optional<PathElement> AddPath = std::nullopt);
void handleAssignment(const Expr *LHSExpr, const Expr *RHSExpr);
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h
index 9aaf4627ce5ad..49d2ddccf8fab 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h
@@ -18,6 +18,8 @@
#include "clang/AST/DeclCXX.h"
#include "clang/AST/ExprCXX.h"
#include "clang/Analysis/Analyses/LifetimeSafety/Utils.h"
+#include "llvm/ADT/FoldingSet.h"
+#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/raw_ostream.h"
namespace clang::lifetimes::internal {
@@ -27,140 +29,194 @@ inline llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, LoanID ID) {
return OS << ID.Value;
}
-/// 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.
-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.
- const llvm::PointerUnion<const clang::ValueDecl *,
- const clang::MaterializeTemporaryExpr *>
- P;
-
+/// Represents one step in an access path: either a field access or an
+/// interior access (denoted by '*').
+class PathElement {
public:
- AccessPath(const clang::ValueDecl *D) : P(D) {}
- AccessPath(const clang::MaterializeTemporaryExpr *MTE) : P(MTE) {}
+ enum class Kind { Field, Interior };
- const clang::ValueDecl *getAsValueDecl() const {
- return P.dyn_cast<const clang::ValueDecl *>();
+ static PathElement getField(const FieldDecl *FD) {
+ return PathElement(Kind::Field, FD);
}
+ static PathElement getInterior() { return PathElement(Kind::Interior, nullptr); }
- const clang::MaterializeTemporaryExpr *getAsMaterializeTemporaryExpr() const {
- return P.dyn_cast<const clang::MaterializeTemporaryExpr *>();
+ bool isField() const { return K == Kind::Field; }
+ bool isInterior() const { return K == Kind::Interior; }
+ const FieldDecl *getFieldDecl() const {
+ assert(isField());
+ return FD;
}
- bool operator==(const AccessPath &RHS) const { return P == RHS.P; }
-};
+ bool operator==(const PathElement &Other) const {
+ return K == Other.K && FD == Other.FD;
+ }
+ bool operator<(const PathElement &Other) const {
+ if (K != Other.K)
+ return static_cast<int>(K) < static_cast<int>(Other.K);
+ return FD < Other.FD;
+ }
+ bool operator!=(const PathElement &Other) const { return !(*this == Other); }
-/// An abstract base class for a single "Loan" which represents lending a
-/// storage in memory.
-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; }
+ void Profile(llvm::FoldingSetNodeID &IDBuilder) const {
+ IDBuilder.AddInteger(static_cast<char>(K));
+ IDBuilder.AddPointer(FD);
+ }
- virtual void dump(llvm::raw_ostream &OS) const = 0;
+ void dump(llvm::raw_ostream &OS) const {
+ if (isField())
+ OS << "." << FD->getNameAsString();
+ else
+ OS << ".*";
+ }
private:
- const Kind K;
- const LoanID ID;
+ PathElement(Kind K, const FieldDecl *FD) : K(K), FD(FD) {}
+ Kind K;
+ const FieldDecl *FD;
};
-/// 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 Expr *IssueExpr;
+/// Represents the base of a placeholder access path, which is either a
+/// function parameter or the implicit 'this' object of an instance method.
+class PlaceholderBase : public llvm::FoldingSetNode {
+ llvm::PointerUnion<const ParmVarDecl *, const CXXMethodDecl *> ParamOrMethod;
public:
- PathLoan(LoanID ID, AccessPath Path, const Expr *IssueExpr)
- : Loan(Kind::Path, ID), Path(Path), IssueExpr(IssueExpr) {}
+ PlaceholderBase(const ParmVarDecl *PVD) : ParamOrMethod(PVD) {}
+ PlaceholderBase(const CXXMethodDecl *MD) : ParamOrMethod(MD) {}
- const AccessPath &getAccessPath() const { return Path; }
- const Expr *getIssueExpr() const { return IssueExpr; }
+ const ParmVarDecl *getParmVarDecl() const {
+ return ParamOrMethod.dyn_cast<const ParmVarDecl *>();
+ }
- void dump(llvm::raw_ostream &OS) const override;
+ const CXXMethodDecl *getMethodDecl() const {
+ return ParamOrMethod.dyn_cast<const CXXMethodDecl *>();
+ }
- static bool classof(const Loan *L) { return L->getKind() == Kind::Path; }
+ void Profile(llvm::FoldingSetNodeID &ID) const {
+ ID.AddPointer(ParamOrMethod.getOpaqueValue());
+ }
};
-/// 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;
+/// Represents the storage location being borrowed, e.g., a specific stack
+/// variable or a field within it: var.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.
+ // - PlaceholderBase * , to represent a borrow from the caller's scope (e.g. a
+ // parameter).
+ const llvm::PointerUnion<const clang::ValueDecl *,
+ const clang::MaterializeTemporaryExpr *,
+ const PlaceholderBase *>
+ Base;
+ llvm::SmallVector<PathElement> Elements;
public:
- PlaceholderLoan(LoanID ID, const ParmVarDecl *PVD)
- : Loan(Kind::Placeholder, ID), ParamOrMethod(PVD) {}
+ AccessPath(const clang::ValueDecl *D) : Base(D) {}
+ AccessPath(const clang::MaterializeTemporaryExpr *MTE) : Base(MTE) {}
+ AccessPath(const PlaceholderBase *PB) : Base(PB) {}
- PlaceholderLoan(LoanID ID, const CXXMethodDecl *MD)
- : Loan(Kind::Placeholder, ID), ParamOrMethod(MD) {}
+ AccessPath(const AccessPath &Other, PathElement E)
+ : Base(Other.Base), Elements(Other.Elements) {
+ Elements.push_back(E);
+ }
- const ParmVarDecl *getParmVarDecl() const {
- return ParamOrMethod.dyn_cast<const ParmVarDecl *>();
+ const clang::ValueDecl *getAsValueDecl() const {
+ return Base.dyn_cast<const clang::ValueDecl *>();
}
- const CXXMethodDecl *getMethodDecl() const {
- return ParamOrMethod.dyn_cast<const CXXMethodDecl *>();
+ const clang::MaterializeTemporaryExpr *getAsMaterializeTemporaryExpr() const {
+ return Base.dyn_cast<const clang::MaterializeTemporaryExpr *>();
}
- void dump(llvm::raw_ostream &OS) const override;
+ const PlaceholderBase *getAsPlaceholderBase() const {
+ return Base.dyn_cast<const PlaceholderBase *>();
+ }
- static bool classof(const Loan *L) {
- return L->getKind() == Kind::Placeholder;
+ bool operator==(const AccessPath &RHS) const {
+ return Base == RHS.Base && Elements == RHS.Elements;
}
+
+ /// Returns true if this path is a strict prefix of Other.
+ bool isStrictPrefixOf(const AccessPath &Other) const {
+ if (Base != Other.Base)
+ return false;
+ if (Elements.size() >= Other.Elements.size())
+ return false;
+ for (size_t i = 0; i < Elements.size(); ++i) {
+ if (Elements[i] != Other.Elements[i])
+ return false;
+ }
+ return true;
+ }
+
+ /// Returns true if this path is a prefix of Other (or same as Other).
+ bool isPrefixOf(const AccessPath &Other) const {
+ if (Base != Other.Base)
+ return false;
+ if (Elements.size() > Other.Elements.size())
+ return false;
+ for (size_t i = 0; i < Elements.size(); ++i) {
+ if (Elements[i] != Other.Elements[i])
+ return false;
+ }
+ return true;
+ }
+
+ void Profile(llvm::FoldingSetNodeID &IDBuilder) const {
+ IDBuilder.AddPointer(Base.getOpaqueValue());
+ for (const auto &E : Elements)
+ E.Profile(IDBuilder);
+ }
+
+ void dump(llvm::raw_ostream &OS) const;
+};
+
+/// Represents lending a storage location.
+class Loan {
+ const LoanID ID;
+ const AccessPath Path;
+ /// The expression that creates the loan, e.g., &x. Optional for placeholder
+ /// loans.
+ const Expr *IssueExpr;
+
+public:
+ 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;
};
/// Manages the creation, storage and retrieval of loans.
class LoanManager {
+ using ExtensionCacheKey = std::pair<LoanID, PathElement>;
+
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;
}
+ /// Gets or creates a placeholder base for a given parameter or method.
+ const PlaceholderBase *getOrCreatePlaceholderBase(const ParmVarDecl *PVD);
+ const PlaceholderBase *getOrCreatePlaceholderBase(const CXXMethodDecl *MD);
+
+ /// Gets or creates a loan by extending BaseLoanID with Element.
+ /// Caches the result to ensure convergence in LoanPropagation.
+ Loan *getOrCreateExtendedLoan(LoanID BaseLoanID, PathElement Element,
+ const Expr *ContextExpr);
+
const Loan *getLoan(LoanID ID) const {
assert(ID.Value < AllLoans.size());
return AllLoans[ID.Value];
@@ -175,6 +231,8 @@ class LoanManager {
/// optimisation.
llvm::SmallVector<const Loan *> AllLoans;
llvm::BumpPtrAllocator LoanAllocator;
+ llvm::FoldingSet<PlaceholderBase> PlaceholderBases;
+ std::map<ExtensionCacheKey, Loan *> ExtensionCache;
};
} // namespace clang::lifetimes::internal
diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp
index 78c2a6dba3eb6..dc78c649736f8 100644
--- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp
@@ -125,12 +125,12 @@ 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);
}
}
@@ -147,40 +147,34 @@ class LifetimeChecker {
/// liveness. Future enhancements could also consider the confidence of loan
/// propagation (e.g., a loan may only be held on some execution paths).
void checkExpiry(const ExpireFact *EF) {
- LoanID ExpiredLoan = EF->getLoanID();
- const Expr *MovedExpr = nullptr;
- if (auto *ME = MovedLoans.getMovedLoans(EF).lookup(ExpiredLoan))
- MovedExpr = *ME;
+ LoanID ExpiredLoanID = EF->getLoanID();
+ const Loan *ExpiredLoan = FactMgr.getLoanMgr().getLoan(ExpiredLoanID);
LivenessMap Origins = LiveOrigins.getLiveOriginsAt(EF);
- Confidence CurConfidence = Confidence::None;
- // The UseFact or OriginEscapesFact most indicative of a lifetime error,
- // prioritized by earlier source location.
- llvm::PointerUnion<const UseFact *, const OriginEscapesFact *>
- BestCausingFact = nullptr;
for (auto &[OID, LiveInfo] : Origins) {
LoanSet HeldLoans = LoanPropagation.getLoans(OID, EF);
- if (!HeldLoans.contains(ExpiredLoan))
- continue;
- // Loan is defaulted.
- Confidence NewConfidence = livenessKindToConfidence(LiveInfo.Kind);
- if (CurConfidence < NewConfidence) {
- CurConfidence = NewConfidence;
- BestCausingFact = LiveInfo.CausingFact;
+ for (LoanID HeldLoanID : HeldLoans) {
+ const Loan *HeldLoan = FactMgr.getLoanMgr().getLoan(HeldLoanID);
+
+ if (ExpiredLoan->getAccessPath().isPrefixOf(HeldLoan->getAccessPath())) {
+ // HeldLoan is expired because its base or itself is expired.
+ const Expr *MovedExpr = nullptr;
+ if (auto *ME = MovedLoans.getMovedLoans(EF).lookup(HeldLoanID))
+ MovedExpr = *ME;
+
+ Confidence NewConfidence = livenessKindToConfidence(LiveInfo.Kind);
+ Confidence LastConf =
+ FinalWarningsMap.lookup(HeldLoanID).ConfidenceLevel;
+ if (LastConf >= NewConfidence)
+ continue;
+
+ FinalWarningsMap[HeldLoanID] = {EF->getExpiryLoc(),
+ LiveInfo.CausingFact, MovedExpr,
+ nullptr, NewConfidence};
+ }
}
}
- if (!BestCausingFact)
- return;
- // We have a use-after-free.
- Confidence LastConf = FinalWarningsMap.lookup(ExpiredLoan).ConfidenceLevel;
- if (LastConf >= CurConfidence)
- return;
- FinalWarningsMap[ExpiredLoan] = {/*ExpiryLoc=*/EF->getExpiryLoc(),
- /*BestCausingFact=*/BestCausingFact,
- /*MovedExpr=*/MovedExpr,
- /*InvalidatedByExpr=*/nullptr,
- /*ConfidenceLevel=*/CurConfidence};
}
/// Checks for use-after-invalidation errors when a container is modified.
@@ -194,18 +188,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().isStrictPrefixOf(L->getAccessPath()))
return true;
}
return false;
@@ -237,12 +222,10 @@ class LifetimeChecker {
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;
Confidence Confidence = Warning.ConfidenceLevel;
diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp
index c963d9c45fa9d..46c667ce96eaa 100644
--- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp
@@ -9,8 +9,6 @@
#include "clang/Analysis/Analyses/LifetimeSafety/Facts.h"
#include "clang/AST/Decl.h"
#include "clang/Analysis/Analyses/PostOrderCFGView.h"
-#include "clang/Analysis/FlowSensitive/DataflowWorklist.h"
-#include "llvm/ADT/STLFunctionalExtras.h"
namespace clang::lifetimes::internal {
@@ -44,6 +42,10 @@ void OriginFlowFact::dump(llvm::raw_ostream &OS, const LoanManager &,
OS << "\tSrc: ";
OM.dump(getSrcOriginID(), OS);
OS << (getKillDest() ? "" : ", Merge");
+ if (auto Path = getPathElement()) {
+ OS << "\n\tAdd path: ";
+ Path->dump(OS);
+ }
OS << "\n";
}
diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
index b69f69ddbae34..61c1f2b40c378 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -45,7 +45,8 @@ OriginList *FactsGenerator::getOriginsList(const Expr &E) {
/// * Level 1: pp <- p's address
/// * Level 2: (*pp) <- what p points to (i.e., &x)
/// - `View v = obj;` flows origins from `obj` (depth 1) to `v` (depth 1)
-void FactsGenerator::flow(OriginList *Dst, OriginList *Src, bool Kill) {
+void FactsGenerator::flow(OriginList *Dst, OriginList *Src, bool Kill,
+ std::optional<PathElement> AddPath) {
if (!Dst)
return;
assert(Src &&
@@ -55,7 +56,7 @@ void FactsGenerator::flow(OriginList *Dst, OriginList *Src, bool Kill) {
while (Dst && Src) {
CurrentBlockFacts.push_back(FactMgr.createFact<OriginFlowFact>(
- Dst->getOuterOriginID(), Src->getOuterOriginID(), Kill));
+ Dst->getOuterOriginID(), Src->getOuterOriginID(), Kill, AddPath));
Dst = Dst->peelOuterOrigin();
Src = Src->peelOuterOrigin();
}
@@ -65,12 +66,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;
}
@@ -78,10 +78,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,
+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);
}
/// Try to find a CXXBindTemporaryExpr that descends from MTE, stripping away
@@ -225,7 +225,7 @@ void FactsGenerator::VisitCXXMemberCallExpr(const CXXMemberCallExpr *MCE) {
void FactsGenerator::VisitMemberExpr(const MemberExpr *ME) {
auto *MD = ME->getMemberDecl();
- if (isa<FieldDecl>(MD) && doesDeclHaveStorage(MD)) {
+ if (auto *FD = dyn_cast<FieldDecl>(MD); FD && doesDeclHaveStorage(FD)) {
assert(ME->isGLValue() && "Field member should be GL value");
OriginList *Dst = getOriginsList(*ME);
assert(Dst && "Field member should have an origin list as it is GL value");
@@ -235,7 +235,7 @@ void FactsGenerator::VisitMemberExpr(const MemberExpr *ME) {
// expression.
CurrentBlockFacts.push_back(FactMgr.createFact<OriginFlowFact>(
Dst->getOuterOriginID(), Src->getOuterOriginID(),
- /*Kill=*/true));
+ /*Kill=*/true, PathElement::getField(FD)));
}
}
@@ -444,15 +444,13 @@ void FactsGenerator::handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds) {
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 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()));
- }
+ // Check if the loan is for a stack variable and if that variable
+ // is the one being destructed.
+ const AccessPath &AP = Loan->getAccessPath();
+ const ValueDecl *Path = AP.getAsValueDecl();
+ if (Path == LifetimeEndsVD)
+ CurrentBlockFacts.push_back(FactMgr.createFact<ExpireFact>(
+ Loan->getID(), LifetimeEnds.getTriggerStmt()->getEndLoc()));
}
}
@@ -464,17 +462,15 @@ void FactsGenerator::handleTemporaryDtor(
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()));
- }
+ // Check if the loan is for a temporary materialization and if that
+ // storage location is the one being destructed.
+ const AccessPath &AP = Loan->getAccessPath();
+ const MaterializeTemporaryExpr *Path = AP.getAsMaterializeTemporaryExpr();
+ if (!Path)
+ continue;
+ if (ExpiringBTE == getChildBinding(Path)) {
+ CurrentBlockFacts.push_back(FactMgr.createFact<ExpireFact>(
+ Loan->getID(), TemporaryDtor.getBindTemporaryExpr()->getEndLoc()));
}
}
}
@@ -549,6 +545,7 @@ void FactsGenerator::handleInvalidatingCall(const Expr *Call,
const auto *MD = dyn_cast<CXXMethodDecl>(FD);
if (!MD || !MD->isInstance())
return;
+ // TODO: Move this into handleMovedArgsInCall in a separate PR.
// std::unique_ptr::release() transfers ownership.
// Treat it as a move to prevent false-positive warnings when the unique_ptr
// destructor runs after ownership has been transferred.
@@ -562,11 +559,6 @@ void FactsGenerator::handleInvalidatingCall(const Expr *Call,
if (!isContainerInvalidationMethod(*MD))
return;
- // Heuristics to turn-down false positives.
- auto *DRE = dyn_cast<DeclRefExpr>(Args[0]);
- if (!DRE || DRE->getDecl()->getType()->isReferenceType())
- return;
-
OriginList *ThisList = getOriginsList(*Args[0]);
if (ThisList)
CurrentBlockFacts.push_back(FactMgr.createFact<InvalidateOriginFact>(
@@ -626,18 +618,19 @@ void FactsGenerator::handleFunctionCall(const Expr *Call,
continue;
if (IsGslConstruction) {
// TODO: document with code example.
- // std::string_view(const std::string_view& from)
+ // std::string_view(const std::string_view& from);
if (isGslPointerType(Args[I]->getType())) {
assert(!Args[I]->isGLValue() || ArgList->getLength() >= 2);
ArgList = getRValueOrigins(Args[I], ArgList);
}
+ // std::string_view(const std::string& from);
if (isGslOwnerType(Args[I]->getType())) {
// GSL construction creates a view that borrows from arguments.
// This implies flowing origins through the list structure.
- flow(CallList, ArgList, KillSrc);
+ flow(CallList, ArgList, KillSrc, PathElement::getInterior());
KillSrc = false;
}
- } else if (shouldTrackPointerImplicitObjectArg(I)) {
+ } else if (I == 0 && shouldTrackPointerImplicitObjectArg(I)) {
assert(ArgList->getLength() >= 2 &&
"Object arg of pointer type should have atleast two origins");
// See through the GSLPointer reference to see the pointer's value.
@@ -649,8 +642,17 @@ void FactsGenerator::handleFunctionCall(const Expr *Call,
// Lifetimebound on a non-GSL-ctor function means the returned
// pointer/reference itself must not outlive the arguments. This
// only constraints the top-level origin.
+ // TODO: Extract to a function.
+ std::optional<PathElement> Element;
+ if (const auto *Method = dyn_cast<CXXMethodDecl>(FD);
+ Method && Method->isInstance() && I == 0 &&
+ (isGslOwnerType(Args[I]->getType()) ||
+ (Args[I]->getType()->isPointerType() &&
+ isGslOwnerType(Args[I]->getType()->getPointeeType()))))
+ Element = PathElement::getInterior();
CurrentBlockFacts.push_back(FactMgr.createFact<OriginFlowFact>(
- CallList->getOuterOriginID(), ArgList->getOuterOriginID(), KillSrc));
+ CallList->getOuterOriginID(), ArgList->getOuterOriginID(), KillSrc,
+ Element));
KillSrc = false;
}
}
@@ -712,8 +714,9 @@ llvm::SmallVector<Fact *> FactsGenerator::issuePlaceholderLoans() {
llvm::SmallVector<Fact *> PlaceholderLoanFacts;
if (const auto *MD = dyn_cast<CXXMethodDecl>(FD); MD && MD->isInstance()) {
OriginList *List = *FactMgr.getOriginMgr().getThisOrigins();
- const PlaceholderLoan *L =
- FactMgr.getLoanMgr().createLoan<PlaceholderLoan>(MD);
+ const PlaceholderBase *PB =
+ FactMgr.getLoanMgr().getOrCreatePlaceholderBase(MD);
+ const Loan *L = FactMgr.getLoanMgr().createLoan(AccessPath(PB));
PlaceholderLoanFacts.push_back(
FactMgr.createFact<IssueFact>(L->getID(), List->getOuterOriginID()));
}
@@ -721,8 +724,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/LoanPropagation.cpp b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp
index 8a020eb829be6..e0bd544e31b5a 100644
--- a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp
@@ -169,14 +169,28 @@ class AnalysisImpl
/// A flow from source to destination. If `KillDest` is true, this replaces
/// the destination's loans with the source's. Otherwise, the source's loans
/// are merged into the destination's.
+ /// If OriginFlowFact has a PathElement, loans from source are extended
+ /// before being propagated to destination.
Lattice transfer(Lattice In, const OriginFlowFact &F) {
OriginID DestOID = F.getDestOriginID();
OriginID SrcOID = F.getSrcOriginID();
+ LoanSet SrcLoans = getLoans(In, SrcOID);
+ LoanSet FlowLoans = SrcLoans;
+
+ if (auto Element = F.getPathElement()) {
+ FlowLoans = LoanSetFactory.getEmptySet();
+ for (LoanID LID : SrcLoans) {
+ Loan *ExtendedLoan =
+ FactMgr.getLoanMgr().getOrCreateExtendedLoan(LID, *Element,
+ nullptr);
+ FlowLoans = LoanSetFactory.add(FlowLoans, ExtendedLoan->getID());
+ }
+ }
+
LoanSet DestLoans =
F.getKillDest() ? LoanSetFactory.getEmptySet() : getLoans(In, DestOID);
- LoanSet SrcLoans = getLoans(In, SrcOID);
- LoanSet MergedLoans = utils::join(DestLoans, SrcLoans, LoanSetFactory);
+ LoanSet MergedLoans = utils::join(DestLoans, FlowLoans, LoanSetFactory);
return setLoans(In, DestOID, MergedLoans);
}
diff --git a/clang/lib/Analysis/LifetimeSafety/Loans.cpp b/clang/lib/Analysis/LifetimeSafety/Loans.cpp
index 8a2cd2a39322b..9c66b3d2834b1 100644
--- a/clang/lib/Analysis/LifetimeSafety/Loans.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Loans.cpp
@@ -10,21 +10,70 @@
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 << "Param:" << PVD->getNameAsString();
+ else if (const auto *MD = PB->getMethodDecl())
+ OS << "This:" << MD->getNameAsString();
+ } else
+ llvm_unreachable("access path base invalid");
+ for (const auto &E : Elements)
+ E.dump(OS);
+}
+
+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) {
+ llvm::FoldingSetNodeID ID;
+ ID.AddPointer(PVD);
+ void *InsertPos = nullptr;
+ if (PlaceholderBase *Existing = PlaceholderBases.FindNodeOrInsertPos(ID, InsertPos))
+ return Existing;
+
+ void *Mem = LoanAllocator.Allocate<PlaceholderBase>();
+ PlaceholderBase *NewPB = new (Mem) PlaceholderBase(PVD);
+ PlaceholderBases.InsertNode(NewPB, InsertPos);
+ return NewPB;
+}
+
+const PlaceholderBase *
+LoanManager::getOrCreatePlaceholderBase(const CXXMethodDecl *MD) {
+ llvm::FoldingSetNodeID ID;
+ ID.AddPointer(MD);
+ void *InsertPos = nullptr;
+ if (PlaceholderBase *Existing = PlaceholderBases.FindNodeOrInsertPos(ID, InsertPos))
+ return Existing;
+
+ void *Mem = LoanAllocator.Allocate<PlaceholderBase>();
+ PlaceholderBase *NewPB = new (Mem) PlaceholderBase(MD);
+ PlaceholderBases.InsertNode(NewPB, InsertPos);
+ return NewPB;
+}
+
+Loan *LoanManager::getOrCreateExtendedLoan(LoanID BaseLoanID,
+ PathElement Element,
+ const Expr *ContextExpr) {
+ ExtensionCacheKey Key = {BaseLoanID, Element};
+ auto It = ExtensionCache.find(Key);
+ if (It != ExtensionCache.end())
+ return It->second;
+
+ const auto *BaseLoan = getLoan(BaseLoanID);
+ AccessPath ExtendedPath(BaseLoan->getAccessPath(), Element);
+ Loan *ExtendedLoan = createLoan(ExtendedPath, BaseLoan->getIssueExpr());
+ ExtensionCache[Key] = ExtendedLoan;
+ return ExtendedLoan;
}
} // namespace clang::lifetimes::internal
diff --git a/clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp b/clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp
index 95de08d4425b0..1a9ce3e576733 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().isPrefixOf(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-invalidations.cpp b/clang/test/Sema/warn-lifetime-safety-invalidations.cpp
index c9ce0c35c53d2..a0b1159b3be3f 100644
--- a/clang/test/Sema/warn-lifetime-safety-invalidations.cpp
+++ b/clang/test/Sema/warn-lifetime-safety-invalidations.cpp
@@ -4,19 +4,22 @@
bool Bool();
+// TODO: DO NOT SUBMIT. We must have better debug output to print post analysis states!
+// TODO: Print name fof function in CXXMemberCallExpr/CallExpr for Origins.
+
namespace SimpleResize {
void IteratorInvalidAfterResize(int new_size) {
std::vector<int> v;
- auto it = std::begin(v); // expected-warning {{object whose reference is captured is later invalidated}}
- v.resize(new_size); // expected-note {{invalidated here}}
- *it; // expected-note {{later used here}}
+ auto it = v.begin(); // expected-warning {{object whose reference is captured is later invalidated}}
+ v.resize(new_size); // expected-note {{invalidated here}}
+ *it; // expected-note {{later used here}}
}
void IteratorValidAfterResize(int new_size) {
std::vector<int> v;
- auto it = std::begin(v);
+ auto it = v.begin();
v.resize(new_size);
- it = std::begin(v);
+ it = v.begin();
if (it != std::end(v)) {
*it; // ok
}
@@ -48,8 +51,8 @@ void PointerToContainerTest(std::vector<int>* v) {
namespace InvalidateBeforeSwap {
void InvalidateBeforeSwapIterators(std::vector<int> v1, std::vector<int> v2) {
- auto it1 = std::begin(v1); // expected-warning {{object whose reference is captured is later invalidated}}
- auto it2 = std::begin(v2);
+ auto it1 = v1.begin(); // expected-warning {{object whose reference is captured is later invalidated}}
+ auto it2 = v2.begin();
if (it1 == std::end(v1) || it2 == std::end(v2)) return;
*it1 = 0; // ok
*it2 = 0; // ok
@@ -62,8 +65,8 @@ void InvalidateBeforeSwapIterators(std::vector<int> v1, std::vector<int> v2) {
}
void InvalidateBeforeSwapContainers(std::vector<int> v1, std::vector<int> v2) {
- auto it1 = std::begin(v1); // expected-warning {{object whose reference is captured is later invalidated}}
- auto it2 = std::begin(v2);
+ auto it1 = v1.begin(); // expected-warning {{object whose reference is captured is later invalidated}}
+ auto it2 = v2.begin();
if (it1 == std::end(v1) || it2 == std::end(v2)) return;
*it1 = 0; // ok
*it2 = 0; // ok
@@ -119,7 +122,7 @@ void MergeWithDifferentContainerValuesInvalidated() {
namespace InvalidationInLoops {
void IteratorInvalidationInAForLoop(std::vector<int> v) {
- for (auto it = std::begin(v); // expected-warning {{object whose reference is captured is later invalidated}}
+ for (auto it = v.begin(); // expected-warning {{object whose reference is captured is later invalidated}}
it != std::end(v);
++it) { // expected-note {{later used here}}
if (Bool()) {
@@ -129,7 +132,7 @@ void IteratorInvalidationInAForLoop(std::vector<int> v) {
}
void IteratorInvalidationInAWhileLoop(std::vector<int> v) {
- auto it = std::begin(v); // expected-warning {{object whose reference is captured is later invalidated}}
+ auto it = v.begin(); // expected-warning {{object whose reference is captured is later invalidated}}
while (it != std::end(v)) {
if (Bool()) {
v.erase(it); // expected-note {{invalidated here}}
@@ -161,14 +164,14 @@ void StdVectorPopBackInvalid(std::vector<int> v) {
namespace SimpleStdFind {
void IteratorCheckedAfterFind(std::vector<int> v) {
- auto it = std::find(std::begin(v), std::end(v), 3);
+ auto it = std::find(v.begin(), std::end(v), 3);
if (it != std::end(v)) {
*it; // ok
}
}
void IteratorCheckedAfterFindThenErased(std::vector<int> v) {
- auto it = std::find(std::begin(v), std::end(v), 3); // expected-warning {{object whose reference is captured is later invalidated}}
+ auto it = std::find(v.begin(), std::end(v), 3); // expected-warning {{object whose reference is captured is later invalidated}}
if (it != std::end(v)) {
v.erase(it); // expected-note {{invalidated here}}
}
@@ -178,7 +181,7 @@ void IteratorCheckedAfterFindThenErased(std::vector<int> v) {
namespace SimpleInsert {
void UseReturnedIteratorAfterInsert(std::vector<int> v) {
- auto it = std::begin(v);
+ auto it = v.begin();
it = v.insert(it, 10);
if (it != std::end(v)) {
*it; // ok
@@ -186,7 +189,7 @@ void UseReturnedIteratorAfterInsert(std::vector<int> v) {
}
void UseInvalidIteratorAfterInsert(std::vector<int> v) {
- auto it = std::begin(v); // expected-warning {{object whose reference is captured is later invalidated}}
+ auto it = v.begin(); // expected-warning {{object whose reference is captured is later invalidated}}
v.insert(it, 10); // expected-note {{invalidated here}}
if (it != std::end(v)) { // expected-note {{later used here}}
*it;
@@ -196,24 +199,24 @@ void UseInvalidIteratorAfterInsert(std::vector<int> v) {
namespace SimpleStdInsert {
void IteratorValidAfterInsert(std::vector<int> v) {
- auto it = std::begin(v);
+ auto it = v.begin();
v.insert(it, 0);
- it = std::begin(v);
+ it = v.begin();
if (it != std::end(v)) {
*it; // ok
}
}
void IteratorInvalidAfterInsert(std::vector<int> v, int value) {
- auto it = std::begin(v); // expected-warning {{object whose reference is captured is later invalidated}}
- v.insert(it, 0); // expected-note {{invalidated here}}
- *it; // expected-note {{later used here}}
+ auto it = v.begin(); // expected-warning {{object whose reference is captured is later invalidated}}
+ v.insert(it, 0); // expected-note {{invalidated here}}
+ *it; // expected-note {{later used here}}
}
} // namespace SimpleStdInsert
namespace SimpleInvalidIterators {
void IteratorUsedAfterErase(std::vector<int> v) {
- auto it = std::begin(v); // expected-warning {{object whose reference is captured is later invalidated}}
+ auto it = v.begin(); // expected-warning {{object whose reference is captured is later invalidated}}
for (; it != std::end(v); ++it) { // expected-note {{later used here}}
if (*it > 3) {
v.erase(it); // expected-note {{invalidated here}}
@@ -221,17 +224,16 @@ void IteratorUsedAfterErase(std::vector<int> v) {
}
}
-// FIXME: Detect this. We currently skip invalidation through ref/pointers to containers.
-void IteratorUsedAfterPushBackParam(std::vector<int>& v) {
- auto it = std::begin(v);
+void IteratorUsedAfterPushBackParam(std::vector<int>& v) { // expected-warning {{parameter is later invalidated}}
+ auto it = v.begin();
if (it != std::end(v) && *it == 3) {
- v.push_back(4);
+ v.push_back(4); // expected-note {{invalidated here}}
}
- ++it;
+ ++it; // expected-note {{later used here}}
}
void IteratorUsedAfterPushBack(std::vector<int> v) {
- auto it = std::begin(v); // expected-warning {{object whose reference is captured is later invalidated}}
+ auto it = v.begin(); // expected-warning {{object whose reference is captured is later invalidated}}
if (it != std::end(v) && *it == 3) {
v.push_back(4); // expected-note {{invalidated here}}
}
@@ -297,44 +299,51 @@ struct S {
// FIXME: Make Paths more precise to reason at field granularity.
// Currently we only detect invalidations to direct declarations and not members.
void Invalidate1Use1IsInvalid() {
- // FIXME: Detect this.
S s;
- auto it = s.strings1.begin();
- s.strings1.push_back("1");
- *it;
+ auto it = s.strings1.begin(); // expected-warning {{object whose reference is captured is later invalidated}}
+ s.strings1.push_back("1"); // expected-note {{invalidated here}}
+ *it; // expected-note {{later used here}}
}
+
void Invalidate1Use2IsOk() {
S s;
auto it = s.strings1.begin();
s.strings2.push_back("1");
*it;
-}void Invalidate1Use2ViaRefIsOk() {
+}
+
+void Invalidate1Use2ViaRefIsOk() {
S s;
- auto it = s.strings2.begin();
+ auto it1 = s.strings1.begin();
+ auto it2 = s.strings2.begin(); // expected-warning {{object whose reference is captured is later invalidated}}
auto& strings2 = s.strings2;
- strings2.push_back("1");
- *it;
+ strings2.push_back("1"); // expected-note {{invalidated here}}
+ *it1;
+ *it2; // expected-note {{later used here}}
}
+
void Invalidate1UseSIsOk() {
S s;
S* p = &s;
s.strings2.push_back("1");
(void)*p;
}
+
void PointerToContainerIsOk() {
std::vector<std::string> s;
std::vector<std::string>* p = &s;
p->push_back("1");
(void)*p;
}
+
void IteratorFromPointerToContainerIsInvalidated() {
- // FIXME: Detect this.
std::vector<std::string> s;
- std::vector<std::string>* p = &s;
+ std::vector<std::string>* p = &s; // expected-warning {{object whose reference is captured is later invalidated}}
auto it = p->begin();
- p->push_back("1");
- *it;
+ p->push_back("1"); // expected-note {{invalidated here}}
+ *it; // expected-note {{later used here}}
}
+
void ChangingRegionOwnedByContainerIsOk() {
std::vector<std::string> subdirs;
for (std::string& path : subdirs)
diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp b/clang/unittests/Analysis/LifetimeSafetyTest.cpp
index 45611f856b3b2..7453b0f9a3e09 100644
--- a/clang/unittests/Analysis/LifetimeSafetyTest.cpp
+++ b/clang/unittests/Analysis/LifetimeSafetyTest.cpp
@@ -122,9 +122,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 {};
@@ -134,10 +133,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().getAsMaterializeTemporaryExpr() != nullptr;
- }
- return false;
+ return L->getAccessPath().getAsMaterializeTemporaryExpr() != nullptr;
}
// Gets the set of loans that are live at the given program point. A loan is
More information about the cfe-commits
mailing list