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 <[email protected]> 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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
