https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/180369
>From ac89dbf0a1d839e14d71e5faa64a26e31da1f3c3 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 | 237 +++++++++------ 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 +- clang/test/Sema/Inputs/lifetime-analysis.h | 22 +- .../warn-lifetime-safety-invalidations.cpp | 152 +++++++--- .../unittests/Analysis/LifetimeSafetyTest.cpp | 272 +++++++++++------- 12 files changed, 605 insertions(+), 361 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..a753f0a82ac29 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,185 @@ 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 prefix of Other (or same as Other). + bool isPrefixOf(const AccessPath &Other) const { + if (Base != Other.Base || 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 strict prefix of Other. + bool isStrictPrefixOf(const AccessPath &Other) const { + return isPrefixOf(Other) && Elements.size() < Other.Elements.size(); + } + + 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 +222,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..da223f4dbc244 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 << "$" << PVD->getNameAsString(); + else if (PB->getMethodDecl()) + OS << "$this"; + } 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/Inputs/lifetime-analysis.h b/clang/test/Sema/Inputs/lifetime-analysis.h index f30db1a29b149..f1c84ff0a3f68 100644 --- a/clang/test/Sema/Inputs/lifetime-analysis.h +++ b/clang/test/Sema/Inputs/lifetime-analysis.h @@ -19,11 +19,11 @@ template<typename T> struct remove_reference<T &> { typedef T type; }; template<typename T> struct remove_reference<T &&> { typedef T type; }; template< class InputIt, class T > -InputIt find( InputIt first, InputIt last, const T& value ); +InputIt find(InputIt first, InputIt last, const T& value); template< class ForwardIt1, class ForwardIt2 > -ForwardIt1 search( ForwardIt1 first, ForwardIt1 last, - ForwardIt2 s_first, ForwardIt2 s_last ); +ForwardIt1 search(ForwardIt1 first, ForwardIt1 last, + ForwardIt2 s_first, ForwardIt2 s_last); template<typename T> typename remove_reference<T>::type &&move(T &&t) noexcept; @@ -75,11 +75,6 @@ struct vector { void clear(); }; -template<class Key,class T> -struct unordered_map { - T& operator[](const Key& key); -}; - template<class T> void swap( T& a, T& b ); @@ -89,6 +84,14 @@ struct pair { B second; }; +template<class Key,class T> +struct unordered_map { + using iterator = __gnu_cxx::basic_iterator<std::pair<const Key, T>>; + T& operator[](const Key& key); + iterator begin(); + iterator end(); +}; + template<typename T> struct basic_string_view { basic_string_view(); @@ -116,6 +119,9 @@ struct basic_string { ~basic_string(); basic_string& operator=(const basic_string&); basic_string& operator+=(const basic_string&); + basic_string& append(const basic_string&); + basic_string& replace(unsigned pos, unsigned count, + const basic_string& str); basic_string& operator+=(const T*); const T *c_str() const; operator basic_string_view<T> () const; diff --git a/clang/test/Sema/warn-lifetime-safety-invalidations.cpp b/clang/test/Sema/warn-lifetime-safety-invalidations.cpp index c9ce0c35c53d2..37ae44e04eaa5 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 } @@ -39,17 +42,18 @@ void PointerToContainerTest() { auto it = v->begin(); *it = 0; // not-ok } -void PointerToContainerTest(std::vector<int>* v) { - // FIXME: Handle placeholder loans. + +void PointerToContainerTest(std::vector<int>* v) { // expected-warning {{parameter is later invalidated}} auto it = v->begin(); - *it = 0; // not-ok + v->push_back(42); // expected-note {{invalidated here}} + *it = 0; // expected-note {{later used here}} } } // namespace PointerToContainer 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 +66,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 +123,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 +133,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 +165,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 +182,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 +190,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 +200,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 +225,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}} } @@ -259,6 +262,14 @@ void PointerToVectorElement() { *ptr = 10; // expected-note {{later used here}} } +struct SField { int a; int b;}; +void PointerToVectorElementField() { + std::vector<SField> v = {{1, 2}, {3, 4}}; + int* ptr = &v[0].a; // expected-warning {{object whose reference is captured is later invalidated}} + v.resize(100); // expected-note {{invalidated here}} + *ptr = 10; // expected-note {{later used here}} +} + void SelfInvalidatingMap() { std::unordered_map<int, int> mp; mp[1] = 1; @@ -278,6 +289,18 @@ void reassign(std::string str, std::string str2) { str = str2; // expected-note {{invalidated here}} (void)view; // expected-note {{later used here}} } + +void append_call(std::string str) { + std::string_view view = str; // expected-warning {{object whose reference is captured is later invalidated}} + str.append("456"); // expected-note {{invalidated here}} + (void)view; // expected-note {{later used here}} +} + +void replace_call(std::string str) { + std::string_view view = str; // expected-warning {{object whose reference is captured is later invalidated}} + str.replace(0, 1, "456"); // expected-note {{invalidated here}} + (void)view; // expected-note {{later used here}} +} } // namespace Strings // FIXME: This should be diagnosed as use-after-invalidation but with potential move. @@ -294,47 +317,53 @@ struct S { std::vector<std::string> strings1; std::vector<std::string> strings2; }; -// 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) @@ -342,3 +371,38 @@ void ChangingRegionOwnedByContainerIsOk() { } } // namespace ContainersAsFields + +namespace MapInvalidations { +void MapOperatorBracket() { + std::unordered_map<int, int> m; + auto it = m.begin(); // expected-warning {{object whose reference is captured is later invalidated}} + m[1]; // expected-note {{invalidated here}} + *it; // expected-note {{later used here}} +} +} // namespace MapInvalidations + +namespace NestedContainers { +void InnerIteratorInvalidated() { + std::vector<std::vector<int>> v; + v.resize(1); + auto it = v[0].begin(); // expected-warning {{object whose reference is captured is later invalidated}} + v[0].push_back(1); // expected-note {{invalidated here}} + *it; // expected-note {{later used here}} +} +void InnerIteratorInvalidatedOneUseOtherIsStillBad() { + std::vector<std::vector<int>> v; + v.resize(100); + // We have no way to differentiate between v[0] and v[1] regions. + auto it = v[0].begin(); // expected-warning {{object whose reference is captured is later invalidated}} + v[1].push_back(1); // expected-note {{invalidated here}} + *it; // expected-note {{later used here}} +} + +void OuterIteratorNotInvalidated() { + std::vector<std::vector<int>> v; + v.resize(1); + auto it = v.begin(); + v[0].push_back(1); + it->clear(); // OK +} +} // namespace NestedContainers diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp b/clang/unittests/Analysis/LifetimeSafetyTest.cpp index 45611f856b3b2..61d885a8f8e91 100644 --- a/clang/unittests/Analysis/LifetimeSafetyTest.cpp +++ b/clang/unittests/Analysis/LifetimeSafetyTest.cpp @@ -12,6 +12,7 @@ #include "clang/Analysis/Analyses/LifetimeSafety/Loans.h" #include "clang/Testing/TestAST.h" #include "llvm/ADT/StringMap.h" +#include "llvm/Support/raw_ostream.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include <optional> @@ -122,9 +123,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 +134,15 @@ 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; + } + + std::string getAccessPathString(LoanID LID) { + const Loan *L = Analysis.getFactManager().getLoanMgr().getLoan(LID); + std::string S; + llvm::raw_string_ostream OS(S); + L->getAccessPath().dump(OS); + return S; } // Gets the set of loans that are live at the given program point. A loan is @@ -261,11 +266,11 @@ class OriginsInfo { /// /// This matcher is intended to be used with an \c OriginInfo object. /// -/// \param LoanVars A vector of strings, where each string is the name of a -/// variable expected to be the source of a loan. +/// \param LoanPathStrs A vector of strings, where each string is the +/// string representation of an access path of a loan. /// \param Annotation A string identifying the program point (created with /// POINT()) where the check should be performed. -MATCHER_P2(HasLoansToImpl, LoanVars, Annotation, "") { +MATCHER_P2(HasLoansToImpl, LoanPathStrs, Annotation, "") { const OriginInfo &Info = arg; std::optional<OriginID> OIDOpt = Info.Helper.getOriginForDecl(Info.OriginVar); if (!OIDOpt) { @@ -281,36 +286,12 @@ MATCHER_P2(HasLoansToImpl, LoanVars, Annotation, "") { << Annotation << "'"; return false; } - std::vector<LoanID> ActualLoans(ActualLoansSetOpt->begin(), - ActualLoansSetOpt->end()); - - std::vector<LoanID> ExpectedLoans; - for (const auto &LoanVar : LoanVars) { - std::vector<LoanID> ExpectedLIDs = Info.Helper.getLoansForVar(LoanVar); - if (ExpectedLIDs.empty()) { - *result_listener << "could not find loan for var '" << LoanVar << "'"; - return false; - } - ExpectedLoans.insert(ExpectedLoans.end(), ExpectedLIDs.begin(), - ExpectedLIDs.end()); - } - std::sort(ExpectedLoans.begin(), ExpectedLoans.end()); - std::sort(ActualLoans.begin(), ActualLoans.end()); - if (ExpectedLoans != ActualLoans) { - *result_listener << "Expected: {"; - for (const auto &LoanID : ExpectedLoans) { - *result_listener << LoanID.Value << ", "; - } - *result_listener << "} Actual: {"; - for (const auto &LoanID : ActualLoans) { - *result_listener << LoanID.Value << ", "; - } - *result_listener << "}"; - return false; - } + std::vector<std::string> ActualLoanPaths; + for (LoanID LID : *ActualLoansSetOpt) + ActualLoanPaths.push_back(Info.Helper.getAccessPathString(LID)); - return ExplainMatchResult(UnorderedElementsAreArray(ExpectedLoans), - ActualLoans, result_listener); + return ExplainMatchResult(UnorderedElementsAreArray(LoanPathStrs), + ActualLoanPaths, result_listener); } enum class LivenessKindFilter { Maybe, Must, All }; @@ -775,7 +756,7 @@ TEST_F(LifetimeAnalysisTest, GslPointerSimpleLoan) { POINT(p1); } )"); - EXPECT_THAT(Origin("x"), HasLoansTo({"a"}, "p1")); + EXPECT_THAT(Origin("x"), HasLoansTo({"a.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, GslPointerConstructFromOwner) { @@ -791,12 +772,12 @@ TEST_F(LifetimeAnalysisTest, GslPointerConstructFromOwner) { POINT(p1); } )"); - EXPECT_THAT(Origin("a"), HasLoansTo({"al"}, "p1")); - EXPECT_THAT(Origin("b"), HasLoansTo({"bl"}, "p1")); - EXPECT_THAT(Origin("c"), HasLoansTo({"cl"}, "p1")); - EXPECT_THAT(Origin("d"), HasLoansTo({"dl"}, "p1")); - EXPECT_THAT(Origin("e"), HasLoansTo({"el"}, "p1")); - EXPECT_THAT(Origin("f"), HasLoansTo({"fl"}, "p1")); + EXPECT_THAT(Origin("a"), HasLoansTo({"al.*"}, "p1")); + EXPECT_THAT(Origin("b"), HasLoansTo({"bl.*"}, "p1")); + EXPECT_THAT(Origin("c"), HasLoansTo({"cl.*"}, "p1")); + EXPECT_THAT(Origin("d"), HasLoansTo({"dl.*"}, "p1")); + EXPECT_THAT(Origin("e"), HasLoansTo({"el.*"}, "p1")); + EXPECT_THAT(Origin("f"), HasLoansTo({"fl.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, GslPointerConstructFromView) { @@ -811,11 +792,11 @@ TEST_F(LifetimeAnalysisTest, GslPointerConstructFromView) { POINT(p1); } )"); - EXPECT_THAT(Origin("x"), HasLoansTo({"a"}, "p1")); - EXPECT_THAT(Origin("y"), HasLoansTo({"a"}, "p1")); - EXPECT_THAT(Origin("z"), HasLoansTo({"a"}, "p1")); - EXPECT_THAT(Origin("p"), HasLoansTo({"a"}, "p1")); - EXPECT_THAT(Origin("q"), HasLoansTo({"a"}, "p1")); + EXPECT_THAT(Origin("x"), HasLoansTo({"a.*"}, "p1")); + EXPECT_THAT(Origin("y"), HasLoansTo({"a.*"}, "p1")); + EXPECT_THAT(Origin("z"), HasLoansTo({"a.*"}, "p1")); + EXPECT_THAT(Origin("p"), HasLoansTo({"a.*"}, "p1")); + EXPECT_THAT(Origin("q"), HasLoansTo({"a.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, GslPointerInConditionalOperator) { @@ -826,7 +807,7 @@ TEST_F(LifetimeAnalysisTest, GslPointerInConditionalOperator) { POINT(p1); } )"); - EXPECT_THAT(Origin("v"), HasLoansTo({"a", "b"}, "p1")); + EXPECT_THAT(Origin("v"), HasLoansTo({"a.*", "b.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, ExtraParenthesis) { @@ -840,10 +821,10 @@ TEST_F(LifetimeAnalysisTest, ExtraParenthesis) { POINT(p1); } )"); - EXPECT_THAT(Origin("x"), HasLoansTo({"a"}, "p1")); - EXPECT_THAT(Origin("y"), HasLoansTo({"a"}, "p1")); - EXPECT_THAT(Origin("z"), HasLoansTo({"a"}, "p1")); - EXPECT_THAT(Origin("p"), HasLoansTo({"a"}, "p1")); + EXPECT_THAT(Origin("x"), HasLoansTo({"a.*"}, "p1")); + EXPECT_THAT(Origin("y"), HasLoansTo({"a.*"}, "p1")); + EXPECT_THAT(Origin("z"), HasLoansTo({"a.*"}, "p1")); + EXPECT_THAT(Origin("p"), HasLoansTo({"a.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, ViewFromTemporary) { @@ -870,8 +851,8 @@ TEST_F(LifetimeAnalysisTest, GslPointerWithConstAndAuto) { POINT(p1); } )"); - EXPECT_THAT(Origin("v1"), HasLoansTo({"a"}, "p1")); - EXPECT_THAT(Origin("v2"), HasLoansTo({"a"}, "p1")); + EXPECT_THAT(Origin("v1"), HasLoansTo({"a.*"}, "p1")); + EXPECT_THAT(Origin("v2"), HasLoansTo({"a.*"}, "p1")); EXPECT_THAT(Origin("v3"), HasLoansTo({"v2"}, "p1")); } @@ -891,9 +872,9 @@ TEST_F(LifetimeAnalysisTest, GslPointerPropagation) { } )"); - EXPECT_THAT(Origin("x"), HasLoansTo({"a"}, "p1")); - EXPECT_THAT(Origin("y"), HasLoansTo({"a"}, "p2")); - EXPECT_THAT(Origin("z"), HasLoansTo({"a"}, "p3")); + EXPECT_THAT(Origin("x"), HasLoansTo({"a.*"}, "p1")); + EXPECT_THAT(Origin("y"), HasLoansTo({"a.*"}, "p2")); + EXPECT_THAT(Origin("z"), HasLoansTo({"a.*"}, "p3")); } TEST_F(LifetimeAnalysisTest, GslPointerReassignment) { @@ -912,9 +893,9 @@ TEST_F(LifetimeAnalysisTest, GslPointerReassignment) { } )"); - EXPECT_THAT(Origin("v"), HasLoansTo({"safe"}, "p1")); - EXPECT_THAT(Origin("v"), HasLoansTo({"unsafe"}, "p2")); - EXPECT_THAT(Origin("v"), HasLoansTo({"unsafe"}, "p3")); + EXPECT_THAT(Origin("v"), HasLoansTo({"safe.*"}, "p1")); + EXPECT_THAT(Origin("v"), HasLoansTo({"unsafe.*"}, "p2")); + EXPECT_THAT(Origin("v"), HasLoansTo({"unsafe.*"}, "p3")); } TEST_F(LifetimeAnalysisTest, GslPointerConversionOperator) { @@ -938,8 +919,8 @@ TEST_F(LifetimeAnalysisTest, GslPointerConversionOperator) { POINT(p1); } )"); - EXPECT_THAT(Origin("x"), HasLoansTo({"xl"}, "p1")); - EXPECT_THAT(Origin("y"), HasLoansTo({"yl"}, "p1")); + EXPECT_THAT(Origin("x"), HasLoansTo({"xl.*"}, "p1")); + EXPECT_THAT(Origin("y"), HasLoansTo({"yl.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, LifetimeboundSimple) { @@ -955,10 +936,10 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundSimple) { POINT(p2); } )"); - EXPECT_THAT(Origin("v1"), HasLoansTo({"a"}, "p1")); + EXPECT_THAT(Origin("v1"), HasLoansTo({"a.*"}, "p1")); // The origin of v2 should now contain the loan to 'o' from v1. - EXPECT_THAT(Origin("v2"), HasLoansTo({"a"}, "p2")); - EXPECT_THAT(Origin("v3"), HasLoansTo({"b"}, "p2")); + EXPECT_THAT(Origin("v2"), HasLoansTo({"a.*"}, "p2")); + EXPECT_THAT(Origin("v3"), HasLoansTo({"b.*"}, "p2")); } TEST_F(LifetimeAnalysisTest, LifetimeboundMemberFunctionOfAView) { @@ -975,7 +956,7 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundMemberFunctionOfAView) { POINT(p2); } )"); - EXPECT_THAT(Origin("v1"), HasLoansTo({"o"}, "p1")); + EXPECT_THAT(Origin("v1"), HasLoansTo({"o.*"}, "p1")); // The call v1.pass() is bound to 'v1'. EXPECT_THAT(Origin("v2"), HasLoansTo({"v1"}, "p2")); } @@ -1007,11 +988,11 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundMultipleArgs) { POINT(p2); } )"); - EXPECT_THAT(Origin("v1"), HasLoansTo({"o1"}, "p1")); - EXPECT_THAT(Origin("v2"), HasLoansTo({"o2"}, "p2")); + EXPECT_THAT(Origin("v1"), HasLoansTo({"o1.*"}, "p1")); + EXPECT_THAT(Origin("v2"), HasLoansTo({"o2.*"}, "p2")); // v3 should have loans from both v1 and v2, demonstrating the union of // loans. - EXPECT_THAT(Origin("v3"), HasLoansTo({"o1", "o2"}, "p2")); + EXPECT_THAT(Origin("v3"), HasLoansTo({"o1.*", "o2.*"}, "p2")); } TEST_F(LifetimeAnalysisTest, LifetimeboundMixedArgs) { @@ -1027,10 +1008,10 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundMixedArgs) { POINT(p2); } )"); - EXPECT_THAT(Origin("v1"), HasLoansTo({"o1"}, "p1")); - EXPECT_THAT(Origin("v2"), HasLoansTo({"o2"}, "p1")); + EXPECT_THAT(Origin("v1"), HasLoansTo({"o1.*"}, "p1")); + EXPECT_THAT(Origin("v2"), HasLoansTo({"o2.*"}, "p1")); // v3 should only have loans from v1, as v2 is not lifetimebound. - EXPECT_THAT(Origin("v3"), HasLoansTo({"o1"}, "p2")); + EXPECT_THAT(Origin("v3"), HasLoansTo({"o1.*"}, "p2")); } TEST_F(LifetimeAnalysisTest, LifetimeboundChainOfViews) { @@ -1046,9 +1027,9 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundChainOfViews) { POINT(p2); } )"); - EXPECT_THAT(Origin("v1"), HasLoansTo({"obj"}, "p1")); + EXPECT_THAT(Origin("v1"), HasLoansTo({"obj.*"}, "p1")); // v2 should inherit the loan from v1 through the chain of calls. - EXPECT_THAT(Origin("v2"), HasLoansTo({"obj"}, "p2")); + EXPECT_THAT(Origin("v2"), HasLoansTo({"obj.*"}, "p2")); } TEST_F(LifetimeAnalysisTest, LifetimeboundRawPointerParameter) { @@ -1075,7 +1056,7 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundRawPointerParameter) { EXPECT_THAT(Origin("v"), HasLoansTo({"a"}, "p1")); EXPECT_THAT(Origin("ptr1"), HasLoansTo({"b"}, "p2")); EXPECT_THAT(Origin("ptr2"), HasLoansTo({"b"}, "p2")); - EXPECT_THAT(Origin("v2"), HasLoansTo({"c"}, "p3")); + EXPECT_THAT(Origin("v2"), HasLoansTo({"c.*"}, "p3")); } TEST_F(LifetimeAnalysisTest, LifetimeboundConstRefViewParameter) { @@ -1088,7 +1069,7 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundConstRefViewParameter) { POINT(p1); } )"); - EXPECT_THAT(Origin("v1"), HasLoansTo({"o"}, "p1")); + EXPECT_THAT(Origin("v1"), HasLoansTo({"o.*"}, "p1")); EXPECT_THAT(Origin("v2"), HasLoansTo({"v1"}, "p1")); } @@ -1123,12 +1104,23 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundReturnReference) { POINT(p3); } )"); - EXPECT_THAT(Origin("v1"), HasLoansTo({"a"}, "p1")); - EXPECT_THAT(Origin("v2"), HasLoansTo({"a"}, "p2")); - - EXPECT_THAT(Origin("v3"), HasLoansTo({"a"}, "p2")); - - EXPECT_THAT(Origin("v4"), HasLoansTo({"c"}, "p3")); + // Interior paths (`.*`) accummulate because conversions from `MyObj&` to + // `View` add `.*`. + // + // If `v1` has loan `a.*`, `Identity(v1)` returns `MyObj&` + // which also has loan `a.*` due to `lifetimebound`. Then `View v2 = + // Identity(v1)` constructs `v2` from this `MyObj&`, adding another `.*`, so + // `v2` has loan `a.*.*`. This snowball effect continues with `v3` because + // `Identity(b)` involves an implicit conversion from `MyObj&` to `View` for + // argument `b`. + // + // Ideally, `Identity` would be understood as unwrapping `View`, + // so that if called with a `View` holding `a.*`, it would return a reference + // holding only `a`, but we lack annotations to express this. + EXPECT_THAT(Origin("v1"), HasLoansTo({"a.*"}, "p1")); + EXPECT_THAT(Origin("v2"), HasLoansTo({"a.*.*"}, "p2")); + EXPECT_THAT(Origin("v3"), HasLoansTo({"a.*.*.*"}, "p2")); + EXPECT_THAT(Origin("v4"), HasLoansTo({"c.*.*"}, "p3")); } TEST_F(LifetimeAnalysisTest, LifetimeboundTemplateFunctionReturnRef) { @@ -1145,7 +1137,7 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundTemplateFunctionReturnRef) { POINT(p2); } )"); - EXPECT_THAT(Origin("v1"), HasLoansTo({"a"}, "p1")); + EXPECT_THAT(Origin("v1"), HasLoansTo({"a.*"}, "p1")); EXPECT_THAT(Origin("v2"), HasLoansTo({}, "p2")); EXPECT_THAT(Origin("v3"), HasLoansTo({"v2"}, "p2")); } @@ -1169,7 +1161,7 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundTemplateFunctionReturnVal) { )"); EXPECT_THAT(Origin("v1"), HasLoanToATemporary("p1")); - EXPECT_THAT(Origin("v2"), HasLoansTo({"b"}, "p2")); + EXPECT_THAT(Origin("v2"), HasLoansTo({"b.*"}, "p2")); EXPECT_THAT(Origin("v3"), HasLoansTo({"v2"}, "p2")); // View temporary on RHS is lifetime-extended. EXPECT_THAT(Origin("v4"), HasLoansTo({}, "p2")); @@ -1191,6 +1183,86 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundConversionOperator) { EXPECT_THAT(Origin("v"), HasLoansTo({"owner"}, "p1")); } +TEST_F(LifetimeAnalysisTest, NestedFieldAccess) { + SetupTest(R"( + struct Inner { int val; }; + struct Outer { Inner f; }; + void target() { + Outer o; + Outer *p = &o; + int* p1 = &o.f.val; + POINT(a); + int* p2 = &p->f.val; + POINT(b); + } + )"); + EXPECT_THAT(Origin("p1"), HasLoansTo({"o.f.val"}, "a")); + EXPECT_THAT(Origin("p2"), HasLoansTo({"o.f.val"}, "b")); +} + +TEST_F(LifetimeAnalysisTest, PlaceholderInterior) { + SetupTest(R"( + void target(const MyObj& p) { + View v = p; + POINT(a); + } + )"); + EXPECT_THAT(Origin("v"), HasLoansTo({"$p.*"}, "a")); +} + +TEST_F(LifetimeAnalysisTest, PlaceholderParamField) { + SetupTest(R"( + struct S { int val; }; + void target(S* p) { + int* p1 = &p->val; + POINT(a); + } + )"); + EXPECT_THAT(Origin("p1"), HasLoansTo({"$p.val"}, "a")); +} + +TEST_F(LifetimeAnalysisTest, PlaceholderThisField) { + SetupTest(R"( + struct S { + int f; + void target() { + int* p1 = &f; + POINT(a); + } + }; + )"); + EXPECT_THAT(Origin("p1"), HasLoansTo({"$this.f"}, "a")); +} + +TEST_F(LifetimeAnalysisTest, PlaceholderThisInterior) { + SetupTest(R"( + struct S { + MyObj o; + void target() { + View v = o; + POINT(a); + } + }; + )"); + EXPECT_THAT(Origin("v"), HasLoansTo({"$this.o.*"}, "a")); +} + +TEST_F(LifetimeAnalysisTest, PlaceholderThisNestedField) { + SetupTest(R"( + struct S1 { + int f; + }; + struct S { + S1 s1; + void target() { + int* p1 = &s1.f; + POINT(a); + } + }; + )"); + EXPECT_THAT(Origin("p1"), HasLoansTo({"$this.s1.f"}, "a")); +} + TEST_F(LifetimeAnalysisTest, LivenessDeadPointer) { SetupTest(R"( void target() { @@ -1656,7 +1728,7 @@ TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_STLBegin) { POINT(p1); } )"); - EXPECT_THAT(Origin("it"), HasLoansTo({"vec"}, "p1")); + EXPECT_THAT(Origin("it"), HasLoansTo({"vec.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_OwnerDeref) { @@ -1674,7 +1746,7 @@ TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_OwnerDeref) { POINT(p1); } )"); - EXPECT_THAT(Origin("r"), HasLoansTo({"opt"}, "p1")); + EXPECT_THAT(Origin("r"), HasLoansTo({"opt.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_Value) { @@ -1692,7 +1764,7 @@ TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_Value) { POINT(p1); } )"); - EXPECT_THAT(Origin("r"), HasLoansTo({"opt"}, "p1")); + EXPECT_THAT(Origin("r"), HasLoansTo({"opt.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_UniquePtr_Get) { @@ -1710,7 +1782,7 @@ TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_UniquePtr_Get) { POINT(p1); } )"); - EXPECT_THAT(Origin("r"), HasLoansTo({"up"}, "p1")); + EXPECT_THAT(Origin("r"), HasLoansTo({"up.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_ConversionOperator) { @@ -1729,7 +1801,7 @@ TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_ConversionOperator) { POINT(p1); } )"); - EXPECT_THAT(Origin("ptr"), HasLoansTo({"owner"}, "p1")); + EXPECT_THAT(Origin("ptr"), HasLoansTo({"owner.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_MapFind) { @@ -1748,7 +1820,7 @@ TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_MapFind) { POINT(p1); } )"); - EXPECT_THAT(Origin("it"), HasLoansTo({"m"}, "p1")); + EXPECT_THAT(Origin("it"), HasLoansTo({"m.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_GSLPointerArg) { @@ -1794,11 +1866,11 @@ TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_GSLPointerArg) { POINT(end); } )"); - EXPECT_THAT(Origin("sv1"), HasLoansTo({"s1"}, "end")); - EXPECT_THAT(Origin("sv2"), HasLoansTo({"s2"}, "end")); - EXPECT_THAT(Origin("sv3"), HasLoansTo({"s3"}, "end")); - EXPECT_THAT(Origin("sv4"), HasLoansTo({"s4"}, "end")); - EXPECT_THAT(Origin("sv5"), HasLoansTo({"s5"}, "end")); + EXPECT_THAT(Origin("sv1"), HasLoansTo({"s1.*"}, "end")); + EXPECT_THAT(Origin("sv2"), HasLoansTo({"s2.*"}, "end")); + EXPECT_THAT(Origin("sv3"), HasLoansTo({"s3.*"}, "end")); + EXPECT_THAT(Origin("sv4"), HasLoansTo({"s4.*"}, "end")); + EXPECT_THAT(Origin("sv5"), HasLoansTo({"s5.*"}, "end")); } // ========================================================================= // @@ -1887,7 +1959,7 @@ TEST_F(LifetimeAnalysisTest, DerivedToBaseThisArg) { POINT(p1); } )"); - EXPECT_THAT(Origin("view"), HasLoansTo({"my_obj_or"}, "p1")); + EXPECT_THAT(Origin("view"), HasLoansTo({"my_obj_or.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, DerivedViewWithNoAnnotation) { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
