https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/180369
>From 0dad2b7a9fb2f7cb942dab966ff7385528af15de 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 | 80 +++-- .../Analyses/LifetimeSafety/FactsGenerator.h | 3 +- .../Analysis/Analyses/LifetimeSafety/Loans.h | 276 ++++++++++++------ clang/lib/Analysis/LifetimeSafety/Checker.cpp | 112 +++---- clang/lib/Analysis/LifetimeSafety/Facts.cpp | 58 +++- .../LifetimeSafety/FactsGenerator.cpp | 98 ++++--- .../LifetimeSafety/LifetimeSafety.cpp | 10 +- .../LifetimeSafety/LoanPropagation.cpp | 18 +- clang/lib/Analysis/LifetimeSafety/Loans.cpp | 68 ++++- .../Analysis/LifetimeSafety/MovedLoans.cpp | 10 +- clang/test/Sema/Inputs/lifetime-analysis.h | 22 +- .../Sema/warn-lifetime-safety-dataflow.cpp | 84 +++--- .../warn-lifetime-safety-invalidations.cpp | 188 +++++++++--- .../unittests/Analysis/LifetimeSafetyTest.cpp | 275 ++++++++++------- 14 files changed, 832 insertions(+), 470 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index f9d55991f2e09..01092d90c2690 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -14,18 +14,22 @@ #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> +#include "llvm/Support/raw_ostream.h" namespace clang::lifetimes::internal { +class LoanPropagationAnalysis; + using FactID = utils::ID<struct FactTag>; /// An abstract base class for a single, atomic lifetime-relevant event. @@ -76,7 +80,8 @@ class Fact { } virtual void dump(llvm::raw_ostream &OS, const LoanManager &, - const OriginManager &) const; + const OriginManager &, + const LoanPropagationAnalysis *LPA = nullptr) const; }; /// A `ProgramPoint` identifies a location in the CFG by pointing to a specific @@ -97,48 +102,64 @@ class IssueFact : public Fact { LoanID getLoanID() const { return LID; } OriginID getOriginID() const { return OID; } void dump(llvm::raw_ostream &OS, const LoanManager &LM, - const OriginManager &OM) const override; + const OriginManager &OM, + const LoanPropagationAnalysis *LPA = nullptr) const override; }; +/// Represents the expiration of loans at a specific storage location. +/// +/// When an AccessPath expires (e.g., a variable goes out of scope), all loans +/// that are prefixed by this path expire. For example, if `x` expires, then +/// loans to `x`, `x.field`, and `x.field.*` all expire. class ExpireFact : public Fact { - LoanID LID; + /// The access path that expires (e.g., the variable going out of scope). + AccessPath AP; SourceLocation ExpiryLoc; public: static bool classof(const Fact *F) { return F->getKind() == Kind::Expire; } - ExpireFact(LoanID LID, SourceLocation ExpiryLoc) - : Fact(Kind::Expire), LID(LID), ExpiryLoc(ExpiryLoc) {} + ExpireFact(AccessPath AP, SourceLocation ExpiryLoc) + : Fact(Kind::Expire), AP(AP), ExpiryLoc(ExpiryLoc) {} - LoanID getLoanID() const { return LID; } + const AccessPath &getAccessPath() const { return AP; } SourceLocation getExpiryLoc() const { return ExpiryLoc; } - void dump(llvm::raw_ostream &OS, const LoanManager &LM, - const OriginManager &) const override; + void dump(llvm::raw_ostream &OS, const LoanManager &LM, const OriginManager &, + const LoanPropagationAnalysis *LPA = nullptr) const override; }; class OriginFlowFact : public Fact { OriginID OIDDest; OriginID OIDSrc; - // True if the destination origin should be killed (i.e., its current loans - // cleared) before the source origin's loans are flowed into it. + /// 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. + /// + /// Example: If source has loan to `x` and Element=field, then destination + /// receives loan to `x.field`. This is used for member expressions like + /// `p = obj.field;` where `p` gets a loan to `obj.field`. + std::optional<PathElement> AddToPath; 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> AddToPath = std::nullopt) : Fact(Kind::OriginFlow), OIDDest(OIDDest), OIDSrc(OIDSrc), - KillDest(KillDest) {} + KillDest(KillDest), AddToPath(AddToPath) {} OriginID getDestOriginID() const { return OIDDest; } OriginID getSrcOriginID() const { return OIDSrc; } bool getKillDest() const { return KillDest; } + std::optional<PathElement> getPathElement() const { return AddToPath; } - void dump(llvm::raw_ostream &OS, const LoanManager &, - const OriginManager &OM) const override; + void dump(llvm::raw_ostream &OS, const LoanManager &, const OriginManager &OM, + const LoanPropagationAnalysis *LPA = nullptr) const override; }; /// Represents that an origin escapes the current scope through various means. @@ -178,8 +199,8 @@ class ReturnEscapeFact : public OriginEscapesFact { EscapeKind::Return; } const Expr *getReturnExpr() const { return ReturnExpr; }; - void dump(llvm::raw_ostream &OS, const LoanManager &, - const OriginManager &OM) const override; + void dump(llvm::raw_ostream &OS, const LoanManager &, const OriginManager &OM, + const LoanPropagationAnalysis *LPA = nullptr) const override; }; /// Represents that an origin escapes via assignment to a field. @@ -198,8 +219,8 @@ class FieldEscapeFact : public OriginEscapesFact { EscapeKind::Field; } const FieldDecl *getFieldDecl() const { return FDecl; }; - void dump(llvm::raw_ostream &OS, const LoanManager &, - const OriginManager &OM) const override; + void dump(llvm::raw_ostream &OS, const LoanManager &, const OriginManager &OM, + const LoanPropagationAnalysis *LPA = nullptr) const override; }; class UseFact : public Fact { @@ -220,8 +241,8 @@ class UseFact : public Fact { void markAsWritten() { IsWritten = true; } bool isWritten() const { return IsWritten; } - void dump(llvm::raw_ostream &OS, const LoanManager &, - const OriginManager &OM) const override; + void dump(llvm::raw_ostream &OS, const LoanManager &, const OriginManager &OM, + const LoanPropagationAnalysis *LPA = nullptr) const override; }; /// Represents that an origin's storage has been invalidated by a container @@ -243,8 +264,8 @@ class InvalidateOriginFact : public Fact { OriginID getInvalidatedOrigin() const { return OID; } const Expr *getInvalidationExpr() const { return InvalidationExpr; } - void dump(llvm::raw_ostream &OS, const LoanManager &, - const OriginManager &OM) const override; + void dump(llvm::raw_ostream &OS, const LoanManager &, const OriginManager &OM, + const LoanPropagationAnalysis *LPA = nullptr) const override; }; /// Top-level origin of the expression which was found to be moved, e.g, when @@ -264,8 +285,8 @@ class MovedOriginFact : public Fact { OriginID getMovedOrigin() const { return MovedOrigin; } const Expr *getMoveExpr() const { return MoveExpr; } - void dump(llvm::raw_ostream &OS, const LoanManager &, - const OriginManager &OM) const override; + void dump(llvm::raw_ostream &OS, const LoanManager &, const OriginManager &OM, + const LoanPropagationAnalysis *LPA = nullptr) const override; }; /// A dummy-fact used to mark a specific point in the code for testing. @@ -281,8 +302,8 @@ class TestPointFact : public Fact { StringRef getAnnotation() const { return Annotation; } - void dump(llvm::raw_ostream &OS, const LoanManager &, - const OriginManager &) const override; + void dump(llvm::raw_ostream &OS, const LoanManager &, const OriginManager &, + const LoanPropagationAnalysis *LPA = nullptr) const override; }; class FactManager { @@ -309,7 +330,8 @@ class FactManager { return Res; } - void dump(const CFG &Cfg, AnalysisDeclContext &AC) const; + void dump(const CFG &Cfg, AnalysisDeclContext &AC, + const LoanPropagationAnalysis *LPA = nullptr) const; /// Retrieves program points that were specially marked in the source code /// for testing. 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..b73077c21e5db 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h @@ -18,6 +18,10 @@ #include "clang/AST/DeclCXX.h" #include "clang/AST/ExprCXX.h" #include "clang/Analysis/Analyses/LifetimeSafety/Utils.h" +#include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/DenseMapInfo.h" +#include "llvm/ADT/FoldingSet.h" +#include "llvm/ADT/SmallVector.h" #include "llvm/Support/raw_ostream.h" namespace clang::lifetimes::internal { @@ -27,140 +31,199 @@ 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 +/// access to an unnamed interior region (denoted by '*'). +/// +/// Examples: +/// - Field access: `obj.field` has PathElement 'field' +/// - Interior access: `owner.*` has '*' +// - In `std::string s; std::string_view v = s;`, v has loan to s.* +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); } - - const clang::MaterializeTemporaryExpr *getAsMaterializeTemporaryExpr() const { - return P.dyn_cast<const clang::MaterializeTemporaryExpr *>(); + static PathElement getInterior() { + return PathElement(Kind::Interior, nullptr); } - bool operator==(const AccessPath &RHS) const { return P == RHS.P; } -}; + bool isField() const { return K == Kind::Field; } + bool isInterior() const { return K == Kind::Interior; } + const FieldDecl *getFieldDecl() const { return FD; } -/// 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; } + bool operator==(const PathElement &Other) const { + return K == Other.K && FD == Other.FD; + } + bool operator!=(const PathElement &Other) const { return !(*this == Other); } - 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. +/// Placeholder paths never expire within the function scope, as they represent +/// storage from the caller's scope. +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. +/// Represents the storage location being borrowed, e.g., a specific stack +/// variable or a field within it: var.field.* /// -/// 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) +/// An AccessPath consists of: +/// - A base: either a ValueDecl, MaterializeTemporaryExpr, or PlaceholderBase +/// - A sequence of PathElements representing field accesses or interior +/// regions /// -/// 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; +/// Examples: +/// - `x` -> Base=x, Elements=[] +/// - `x.field` -> Base=x, Elements=[.field] +/// - `x.*` (e.g., string_view from string) -> Base=x, Elements=[.*] +/// - `x.field.*` -> Base=x, Elements=[.field, .*] +/// - `$param.field` -> Base=$param, Elements=[.field] +/// +/// TODO: Model access paths of other types, e.g. heap and globals. +class AccessPath { + /// The base of the access path: a variable, temporary, or placeholder. + const llvm::PointerUnion<const clang::ValueDecl *, + const clang::MaterializeTemporaryExpr *, + const PlaceholderBase *> + Base; + /// The path elements representing field accesses and access to unnamed + /// interior regions. + llvm::SmallVector<PathElement, 1> 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) {} + /// Creates an extended access path by appending a path element. + /// Example: AccessPath(x_path, field) creates path to `x.field`. + 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 *>(); + } + + 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). + /// Examples: + /// - `x` is a prefix of `x`, `x.field`, `x.field.*` + /// - `x.field` is a prefix of `x.field` and `x.field.nested` + /// - `x.field` is NOT a prefix of `x.other_field` + 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; + } - static bool classof(const Loan *L) { - return L->getKind() == Kind::Placeholder; + /// Returns true if this path is a strict prefix of Other. + /// Example: + /// - `x` is a strict prefix of `x.field` but NOT of `x` + bool isStrictPrefixOf(const AccessPath &Other) const { + return isPrefixOf(Other) && Elements.size() < Other.Elements.size(); } + + void dump(llvm::raw_ostream &OS) const; +}; + +/// Represents lending a storage location. +// +/// A loan tracks the borrowing relationship created by operations like +/// taking a pointer/reference (&x), creating a view (std::string_view sv = s), +/// or receiving a parameter. +/// +/// Examples: +/// - `int* p = &x;` creates a loan to `x` +/// - `std::string_view v = s;` creates a loan to `s.*` (interior) +/// - `int* p = &obj.field;` creates a loan to `obj.field` +/// - Parameter loans have no IssueExpr (created at function entry) +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 Loan *getLoan(LoanID ID) const { assert(ID.Value < AllLoans.size()); return AllLoans[ID.Value]; @@ -171,6 +234,14 @@ class LoanManager { LoanID getNextLoanID() { return NextLoanID++; } LoanID NextLoanID{0}; + + llvm::FoldingSet<PlaceholderBase> PlaceholderBases; + /// Cache for extended loans. Maps (BaseLoanID, PathElement) to the extended + /// loan. Ensures that extending the same loan with the same path element + /// always returns the same loan object, which is necessary for dataflow + /// analysis convergence. + llvm::DenseMap<ExtensionCacheKey, Loan *> ExtensionCache; + /// TODO(opt): Profile and evaluate the usefullness of small buffer /// optimisation. llvm::SmallVector<const Loan *> AllLoans; @@ -178,4 +249,23 @@ class LoanManager { }; } // namespace clang::lifetimes::internal +namespace llvm { +template <> struct DenseMapInfo<clang::lifetimes::internal::PathElement> { + using PathElement = clang::lifetimes::internal::PathElement; + static inline PathElement getEmptyKey() { + return PathElement::getField( + llvm::DenseMapInfo<const clang::FieldDecl *>::getEmptyKey()); + } + static inline PathElement getTombstoneKey() { + return PathElement::getField( + llvm::DenseMapInfo<const clang::FieldDecl *>::getTombstoneKey()); + } + static unsigned getHashValue(const PathElement &Val) { + return llvm::hash_combine(Val.isInterior(), Val.getFieldDecl()); + } + static bool isEqual(const PathElement &LHS, const PathElement &RHS) { + return LHS == RHS; + } +}; +} // namespace llvm #endif // LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMESAFETY_LOANS_H diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index 78c2a6dba3eb6..60097ef863c9f 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -24,7 +24,6 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "llvm/ADT/DenseMap.h" -#include "llvm/ADT/DenseSet.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/TimeProfiler.h" @@ -125,87 +124,68 @@ class LifetimeChecker { }; for (LoanID LID : EscapedLoans) { const Loan *L = FactMgr.getLoanMgr().getLoan(LID); - const auto *PL = dyn_cast<PlaceholderLoan>(L); - if (!PL) + const PlaceholderBase *PB = L->getAccessPath().getAsPlaceholderBase(); + if (!PB) continue; - if (const auto *PVD = PL->getParmVarDecl()) + if (const auto *PVD = PB->getParmVarDecl()) CheckParam(PVD); - else if (const auto *MD = PL->getMethodDecl()) + else if (const auto *MD = PB->getMethodDecl()) CheckImplicitThis(MD); } } - /// Checks for use-after-free & use-after-return errors when a loan expires. + /// Checks for use-after-free & use-after-return errors when an access path + /// expires (e.g., a variable goes out of scope). /// - /// This method examines all live origins at the expiry point and determines - /// if any of them hold the expiring loan. If so, it creates a pending - /// warning with the appropriate confidence level based on the liveness - /// information. The confidence reflects whether the origin is definitely - /// or maybe live at this point. - /// - /// Note: This implementation considers only the confidence of origin - /// liveness. Future enhancements could also consider the confidence of loan - /// propagation (e.g., a loan may only be held on some execution paths). + /// When a path expires, all loans prefixed by that path expire. For example, + /// if `x` expires, loans to `x`, `x.field`, and `x.field.*` all expire. + /// This method examines all live origins and reports warnings for loans they + /// hold that are prefixed by the expired path. void checkExpiry(const ExpireFact *EF) { - LoanID ExpiredLoan = EF->getLoanID(); - const Expr *MovedExpr = nullptr; - if (auto *ME = MovedLoans.getMovedLoans(EF).lookup(ExpiredLoan)) - MovedExpr = *ME; + const AccessPath &ExpiredPath = EF->getAccessPath(); LivenessMap Origins = LiveOrigins.getLiveOriginsAt(EF); - 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 (ExpiredPath.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. /// - /// This method identifies origins that are live at the point of invalidation - /// and checks if they hold loans that are invalidated by the operation - /// (e.g., iterators into a vector that is being pushed to). + /// When a container is invalidated, loans pointing into its interior are + /// invalidated. For example, if container `v` is invalidated, iterators with + /// loans to `v.*` are invalidated. This method finds live origins holding + /// such loans and reports warnings. A loan is invalidated if its path extends + /// an invalidated container's path (e.g., `v.*` extends `v`). void checkInvalidation(const InvalidateOriginFact *IOF) { OriginID InvalidatedOrigin = IOF->getInvalidatedOrigin(); /// Get loans directly pointing to the invalidated container 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 +217,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; @@ -251,22 +229,28 @@ class LifetimeChecker { if (const auto *UF = CausingFact.dyn_cast<const UseFact *>()) { if (Warning.InvalidatedByExpr) { + // Use-after-invalidation of an object on stack. if (IssueExpr) SemaHelper->reportUseAfterInvalidation(IssueExpr, UF->getUseExpr(), Warning.InvalidatedByExpr); - if (InvalidatedPVD) + // Use-after-invalidation of a parameter. + if (InvalidatedPVD) { SemaHelper->reportUseAfterInvalidation( InvalidatedPVD, UF->getUseExpr(), Warning.InvalidatedByExpr); - - } else + } + } else { + // Scope-based expiry (use-after-scope). SemaHelper->reportUseAfterFree(IssueExpr, UF->getUseExpr(), MovedExpr, ExpiryLoc, Confidence); + } } else if (const auto *OEF = CausingFact.dyn_cast<const OriginEscapesFact *>()) { + // Return stack address. if (const auto *RetEscape = dyn_cast<ReturnEscapeFact>(OEF)) SemaHelper->reportUseAfterReturn(IssueExpr, RetEscape->getReturnExpr(), MovedExpr, ExpiryLoc, Confidence); + // Dangling field. else if (const auto *FieldEscape = dyn_cast<FieldEscapeFact>(OEF)) SemaHelper->reportDanglingField( IssueExpr, FieldEscape->getFieldDecl(), MovedExpr, ExpiryLoc); diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp index c963d9c45fa9d..2e677795bbeef 100644 --- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp @@ -8,19 +8,19 @@ #include "clang/Analysis/Analyses/LifetimeSafety/Facts.h" #include "clang/AST/Decl.h" +#include "clang/Analysis/Analyses/LifetimeSafety/LoanPropagation.h" #include "clang/Analysis/Analyses/PostOrderCFGView.h" -#include "clang/Analysis/FlowSensitive/DataflowWorklist.h" -#include "llvm/ADT/STLFunctionalExtras.h" namespace clang::lifetimes::internal { void Fact::dump(llvm::raw_ostream &OS, const LoanManager &, - const OriginManager &) const { + const OriginManager &, const LoanPropagationAnalysis *) const { OS << "Fact (Kind: " << static_cast<int>(K) << ")\n"; } void IssueFact::dump(llvm::raw_ostream &OS, const LoanManager &LM, - const OriginManager &OM) const { + const OriginManager &OM, + const LoanPropagationAnalysis *) const { OS << "Issue ("; LM.getLoan(getLoanID())->dump(OS); OS << ", ToOrigin: "; @@ -29,47 +29,70 @@ void IssueFact::dump(llvm::raw_ostream &OS, const LoanManager &LM, } void ExpireFact::dump(llvm::raw_ostream &OS, const LoanManager &LM, - const OriginManager &) const { + const OriginManager &, + const LoanPropagationAnalysis *) const { OS << "Expire ("; - LM.getLoan(getLoanID())->dump(OS); + getAccessPath().dump(OS); OS << ")\n"; } -void OriginFlowFact::dump(llvm::raw_ostream &OS, const LoanManager &, - const OriginManager &OM) const { +void OriginFlowFact::dump(llvm::raw_ostream &OS, const LoanManager &LM, + const OriginManager &OM, + const LoanPropagationAnalysis *LPA) const { OS << "OriginFlow: \n"; OS << "\tDest: "; OM.dump(getDestOriginID(), OS); + if (LPA) { + LoanSet DestinationLoans = LPA->getLoans(getDestOriginID(), this); + if (DestinationLoans.isEmpty()) + OS << " has no loans"; + else { + OS << " has loans to { "; + for (LoanID LID : DestinationLoans) { + LM.getLoan(LID)->getAccessPath().dump(OS); + OS << " "; + } + OS << "}"; + } + } OS << "\n"; OS << "\tSrc: "; OM.dump(getSrcOriginID(), OS); OS << (getKillDest() ? "" : ", Merge"); + if (auto Path = getPathElement()) { + OS << "\n\tAdd path: "; + Path->dump(OS); + } OS << "\n"; } void MovedOriginFact::dump(llvm::raw_ostream &OS, const LoanManager &, - const OriginManager &OM) const { + const OriginManager &OM, + const LoanPropagationAnalysis *) const { OS << "MovedOrigins ("; OM.dump(getMovedOrigin(), OS); OS << ")\n"; } void ReturnEscapeFact::dump(llvm::raw_ostream &OS, const LoanManager &, - const OriginManager &OM) const { + const OriginManager &OM, + const LoanPropagationAnalysis *) const { OS << "OriginEscapes ("; OM.dump(getEscapedOriginID(), OS); OS << ", via Return)\n"; } void FieldEscapeFact::dump(llvm::raw_ostream &OS, const LoanManager &, - const OriginManager &OM) const { + const OriginManager &OM, + const LoanPropagationAnalysis *) const { OS << "OriginEscapes ("; OM.dump(getEscapedOriginID(), OS); OS << ", via Field)\n"; } void UseFact::dump(llvm::raw_ostream &OS, const LoanManager &, - const OriginManager &OM) const { + const OriginManager &OM, + const LoanPropagationAnalysis *) const { OS << "Use ("; size_t NumUsedOrigins = getUsedOrigins()->getLength(); size_t I = 0; @@ -83,14 +106,16 @@ void UseFact::dump(llvm::raw_ostream &OS, const LoanManager &, } void InvalidateOriginFact::dump(llvm::raw_ostream &OS, const LoanManager &, - const OriginManager &OM) const { + const OriginManager &OM, + const LoanPropagationAnalysis *) const { OS << "InvalidateOrigin ("; OM.dump(getInvalidatedOrigin(), OS); OS << ")\n"; } void TestPointFact::dump(llvm::raw_ostream &OS, const LoanManager &, - const OriginManager &) const { + const OriginManager &, + const LoanPropagationAnalysis *) const { OS << "TestPoint (Annotation: \"" << getAnnotation() << "\")\n"; } @@ -109,7 +134,8 @@ llvm::StringMap<ProgramPoint> FactManager::getTestPoints() const { return AnnotationToPointMap; } -void FactManager::dump(const CFG &Cfg, AnalysisDeclContext &AC) const { +void FactManager::dump(const CFG &Cfg, AnalysisDeclContext &AC, + const LoanPropagationAnalysis *LPA) const { llvm::dbgs() << "==========================================\n"; llvm::dbgs() << " Lifetime Analysis Facts:\n"; llvm::dbgs() << "==========================================\n"; @@ -121,7 +147,7 @@ void FactManager::dump(const CFG &Cfg, AnalysisDeclContext &AC) const { llvm::dbgs() << " Block B" << B->getBlockID() << ":\n"; for (const Fact *F : getFacts(B)) { llvm::dbgs() << " "; - F->dump(llvm::dbgs(), LoanMgr, OriginMgr); + F->dump(llvm::dbgs(), LoanMgr, OriginMgr, LPA); } llvm::dbgs() << " End of Block\n"; } diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index b69f69ddbae34..0b9ae18bfc9b3 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, - const MaterializeTemporaryExpr *MTE) { +static const Loan *createLoan(FactManager &FactMgr, + const MaterializeTemporaryExpr *MTE) { AccessPath Path(MTE); - return FactMgr.getLoanMgr().createLoan<PathLoan>(Path, MTE); + return FactMgr.getLoanMgr().createLoan(Path, MTE); } /// Try to find a CXXBindTemporaryExpr that descends from MTE, stripping away @@ -225,17 +225,17 @@ 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"); OriginList *Src = getOriginsList(*ME->getBase()); assert(Src && "Base expression should be a pointer/reference type"); - // The field's glvalue (outermost origin) holds the same loans as the base - // expression. + // Flow loans from base to field, extending each loan's path with the field. + // E.g., if base has loan to `obj`, field gets loan to `obj.field`. CurrentBlockFacts.push_back(FactMgr.createFact<OriginFlowFact>( Dst->getOuterOriginID(), Src->getOuterOriginID(), - /*Kill=*/true)); + /*Kill=*/true, PathElement::getField(FD))); } } @@ -438,22 +438,11 @@ void FactsGenerator::VisitMaterializeTemporaryExpr( } void FactsGenerator::handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds) { - /// TODO: Handle loans to temporaries. const VarDecl *LifetimeEndsVD = LifetimeEnds.getVarDecl(); if (!LifetimeEndsVD) 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())); - } - } + CurrentBlockFacts.push_back(FactMgr.createFact<ExpireFact>( + AccessPath(LifetimeEndsVD), LifetimeEnds.getTriggerStmt()->getEndLoc())); } void FactsGenerator::handleTemporaryDtor( @@ -462,20 +451,18 @@ void FactsGenerator::handleTemporaryDtor( TemporaryDtor.getBindTemporaryExpr(); if (!ExpiringBTE) return; - // Iterate through all loans to see if any expire. + + // We need to find the MTE that corresponds to this BTE. 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)) { + const AccessPath &AP = Loan->getAccessPath(); + if (const MaterializeTemporaryExpr *MTE = + AP.getAsMaterializeTemporaryExpr()) + if (ExpiringBTE == getChildBinding(MTE)) { CurrentBlockFacts.push_back(FactMgr.createFact<ExpireFact>( - PL->getID(), TemporaryDtor.getBindTemporaryExpr()->getEndLoc())); + AccessPath(MTE), + TemporaryDtor.getBindTemporaryExpr()->getEndLoc())); + return; } - } } } @@ -562,17 +549,28 @@ 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>( ThisList->getOuterOriginID(), Call)); } +/// Returns a PathElement for a lifetime-bound argument if the flow should +/// target an interior origin. This is currently only for the implicit object +/// argument of instance methods where the object is a gsl owner type. +static std::optional<PathElement> +getPathElementForLifetimeBoundArg(const FunctionDecl *FD, unsigned ArgIndex, + const Expr *ArgExpr) { + const auto *Method = dyn_cast<CXXMethodDecl>(FD); + if (Method && Method->isInstance() && ArgIndex == 0) { + QualType ArgType = ArgExpr->getType(); + if (isGslOwnerType(ArgType) || + (ArgType->isPointerType() && isGslOwnerType(ArgType->getPointeeType()))) + return PathElement::getInterior(); + } + return std::nullopt; +} + void FactsGenerator::handleFunctionCall(const Expr *Call, const FunctionDecl *FD, ArrayRef<const Expr *> Args, @@ -631,13 +629,14 @@ void FactsGenerator::handleFunctionCall(const Expr *Call, 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. @@ -650,7 +649,8 @@ void FactsGenerator::handleFunctionCall(const Expr *Call, // pointer/reference itself must not outlive the arguments. This // only constraints the top-level origin. CurrentBlockFacts.push_back(FactMgr.createFact<OriginFlowFact>( - CallList->getOuterOriginID(), ArgList->getOuterOriginID(), KillSrc)); + CallList->getOuterOriginID(), ArgList->getOuterOriginID(), KillSrc, + getPathElementForLifetimeBoundArg(FD, I, Args[I]))); KillSrc = false; } } @@ -712,8 +712,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 +722,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/LifetimeSafety.cpp b/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp index a6bea74c50b49..d22f3d3b7877f 100644 --- a/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp @@ -35,7 +35,8 @@ namespace internal { #ifndef NDEBUG static void DebugOnlyFunction(AnalysisDeclContext &AC, const CFG &Cfg, - FactManager &FactMgr) { + FactManager &FactMgr, + const LoanPropagationAnalysis *LPA) { std::string Name; if (const Decl *D = AC.getDecl()) { if (const auto *ND = dyn_cast<NamedDecl>(D)) @@ -44,7 +45,7 @@ static void DebugOnlyFunction(AnalysisDeclContext &AC, const CFG &Cfg, DEBUG_WITH_TYPE(Name.c_str(), AC.getDecl()->dumpColor()); DEBUG_WITH_TYPE(Name.c_str(), Cfg.dump(AC.getASTContext().getLangOpts(), /*ShowColors=*/true)); - DEBUG_WITH_TYPE(Name.c_str(), FactMgr.dump(Cfg, AC)); + DEBUG_WITH_TYPE(Name.c_str(), FactMgr.dump(Cfg, AC, LPA)); } #endif @@ -89,12 +90,13 @@ void LifetimeSafetyAnalysis::run() { DEBUG_WITH_TYPE("PrintCFG", Cfg.dump(AC.getASTContext().getLangOpts(), /*ShowColors=*/true)); - DEBUG_WITH_TYPE("LifetimeFacts", FactMgr->dump(Cfg, AC)); + DEBUG_WITH_TYPE("LifetimeFacts", + FactMgr->dump(Cfg, AC, LoanPropagation.get())); // Debug print facts for a specific function using // -debug-only=EnableFilterByFunctionName,YourFunctionNameFoo DEBUG_WITH_TYPE("EnableFilterByFunctionName", - DebugOnlyFunction(AC, Cfg, *FactMgr)); + DebugOnlyFunction(AC, Cfg, *FactMgr, LoanPropagation.get())); DEBUG_WITH_TYPE("LiveOrigins", LiveOrigins->dump(llvm::dbgs(), FactMgr->getTestPoints())); } diff --git a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp index 8a020eb829be6..454bcea6451c7 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 propagating (e.g., loan to `x` becomes loan to `x.field`). Lattice transfer(Lattice In, const OriginFlowFact &F) { OriginID DestOID = F.getDestOriginID(); OriginID SrcOID = F.getSrcOriginID(); + LoanSet SrcLoans = getLoans(In, SrcOID); + LoanSet FlowLoans = SrcLoans; + + // Extend loans if a path element is specified (e.g., for field access). + if (auto Element = F.getPathElement()) { + FlowLoans = LoanSetFactory.getEmptySet(); + for (LoanID LID : SrcLoans) { + Loan *ExtendedLoan = + FactMgr.getLoanMgr().getOrCreateExtendedLoan(LID, *Element); + 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..f76a7b83c2d14 100644 --- a/clang/lib/Analysis/LifetimeSafety/Loans.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Loans.cpp @@ -10,21 +10,71 @@ 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) { + 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-dataflow.cpp b/clang/test/Sema/warn-lifetime-safety-dataflow.cpp index 7e2215b8deedc..f64ed35e05db7 100644 --- a/clang/test/Sema/warn-lifetime-safety-dataflow.cpp +++ b/clang/test/Sema/warn-lifetime-safety-dataflow.cpp @@ -23,10 +23,10 @@ MyObj* return_local_addr() { return p; // CHECK: Issue ({{[0-9]+}} (Path: p), ToOrigin: {{[0-9]+}} (Expr: DeclRefExpr, Decl: p)) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: [[O_RET_VAL:[0-9]+]] (Expr: ImplicitCastExpr, Type : MyObj *) +// CHECK-NEXT: Dest: [[O_RET_VAL:[0-9]+]] (Expr: ImplicitCastExpr, Type : MyObj *) has loans to { x } // CHECK-NEXT: Src: [[O_P]] (Decl: p, Type : MyObj *) -// CHECK: Expire ([[L_X]] (Path: x)) -// CHECK: Expire ({{[0-9]+}} (Path: p)) +// CHECK: Expire (x) +// CHECK: Expire (p) // CHECK: OriginEscapes ([[O_RET_VAL]] (Expr: ImplicitCastExpr, Type : MyObj *), via Return) } @@ -37,13 +37,13 @@ void loan_expires_cpp() { // CHECK: Block B{{[0-9]+}}: // CHECK: Issue ([[L_OBJ:[0-9]+]] (Path: obj), ToOrigin: [[O_DRE_OBJ:[0-9]+]] (Expr: DeclRefExpr, Decl: obj)) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: [[O_ADDR_OBJ:[0-9]+]] (Expr: UnaryOperator, Type : MyObj *) +// CHECK-NEXT: Dest: [[O_ADDR_OBJ:[0-9]+]] (Expr: UnaryOperator, Type : MyObj *) has loans to { obj } // CHECK-NEXT: Src: [[O_DRE_OBJ]] (Expr: DeclRefExpr, Decl: obj) MyObj* pObj = &obj; // CHECK: OriginFlow: -// CHECK-NEXT: Dest: {{[0-9]+}} (Decl: pObj, Type : MyObj *) +// CHECK-NEXT: Dest: {{[0-9]+}} (Decl: pObj, Type : MyObj *) has loans to { obj } // CHECK-NEXT: Src: [[O_ADDR_OBJ]] (Expr: UnaryOperator, Type : MyObj *) -// CHECK: Expire ([[L_OBJ]] (Path: obj)) +// CHECK: Expire (obj) } @@ -53,13 +53,13 @@ void loan_expires_trivial() { // CHECK: Block B{{[0-9]+}}: // CHECK: Issue ([[L_TRIVIAL_OBJ:[0-9]+]] (Path: trivial_obj), ToOrigin: [[O_DRE_TRIVIAL:[0-9]+]] (Expr: DeclRefExpr, Decl: trivial_obj)) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: [[O_ADDR_TRIVIAL_OBJ:[0-9]+]] (Expr: UnaryOperator, Type : int *) +// CHECK-NEXT: Dest: [[O_ADDR_TRIVIAL_OBJ:[0-9]+]] (Expr: UnaryOperator, Type : int *) has loans to { trivial_obj } // CHECK-NEXT: Src: [[O_DRE_TRIVIAL]] (Expr: DeclRefExpr, Decl: trivial_obj) int* pTrivialObj = &trivial_obj; // CHECK: OriginFlow: -// CHECK-NEXT: Dest: {{[0-9]+}} (Decl: pTrivialObj, Type : int *) +// CHECK-NEXT: Dest: {{[0-9]+}} (Decl: pTrivialObj, Type : int *) has loans to { trivial_obj } // CHECK-NEXT: Src: [[O_ADDR_TRIVIAL_OBJ]] (Expr: UnaryOperator, Type : int *) -// CHECK: Expire ([[L_TRIVIAL_OBJ]] (Path: trivial_obj)) +// CHECK: Expire (trivial_obj) // CHECK-NEXT: End of Block } @@ -79,15 +79,15 @@ void overwrite_origin() { p = &s2; // CHECK: Issue ([[L_S2:[0-9]+]] (Path: s2), ToOrigin: [[O_DRE_S2:[0-9]+]] (Expr: DeclRefExpr, Decl: s2)) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: [[O_ADDR_S2:[0-9]+]] (Expr: UnaryOperator, Type : MyObj *) +// CHECK-NEXT: Dest: [[O_ADDR_S2:[0-9]+]] (Expr: UnaryOperator, Type : MyObj *) has loans to { s2 } // CHECK-NEXT: Src: [[O_DRE_S2]] (Expr: DeclRefExpr, Decl: s2) // CHECK: Use ([[O_P]] (Decl: p, Type : MyObj *), Write) // CHECK: Issue ({{[0-9]+}} (Path: p), ToOrigin: {{[0-9]+}} (Expr: DeclRefExpr, Decl: p)) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: [[O_P]] (Decl: p, Type : MyObj *) +// CHECK-NEXT: Dest: [[O_P]] (Decl: p, Type : MyObj *) has loans to { s2 } // CHECK-NEXT: Src: [[O_ADDR_S2]] (Expr: UnaryOperator, Type : MyObj *) -// CHECK: Expire ([[L_S2]] (Path: s2)) -// CHECK: Expire ([[L_S1]] (Path: s1)) +// CHECK: Expire (s2) +// CHECK: Expire (s1) } // CHECK-LABEL: Function: reassign_to_null @@ -106,9 +106,9 @@ void reassign_to_null() { // CHECK: Use ([[O_P]] (Decl: p, Type : MyObj *), Write) // CHECK: Issue ({{[0-9]+}} (Path: p), ToOrigin: {{[0-9]+}} (Expr: DeclRefExpr, Decl: p)) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: [[O_P]] (Decl: p, Type : MyObj *) +// CHECK-NEXT: Dest: [[O_P]] (Decl: p, Type : MyObj *) has no loans // CHECK-NEXT: Src: {{[0-9]+}} (Expr: ImplicitCastExpr, Type : MyObj *) -// CHECK: Expire ([[L_S1]] (Path: s1)) +// CHECK: Expire (s1) } // FIXME: Have a better representation for nullptr than just an empty origin. // It should be a separate loan and origin kind. @@ -120,25 +120,25 @@ void pointer_indirection() { // CHECK: Block B{{[0-9]+}}: // CHECK: Issue ([[L_A:[0-9]+]] (Path: a), ToOrigin: [[O_DRE_A:[0-9]+]] (Expr: DeclRefExpr, Decl: a)) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: [[O_ADDR_A:[0-9]+]] (Expr: UnaryOperator, Type : int *) +// CHECK-NEXT: Dest: [[O_ADDR_A:[0-9]+]] (Expr: UnaryOperator, Type : int *) has loans to { a } // CHECK-NEXT: Src: [[O_DRE_A]] (Expr: DeclRefExpr, Decl: a) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: [[O_P:[0-9]+]] (Decl: p, Type : int *) +// CHECK-NEXT: Dest: [[O_P:[0-9]+]] (Decl: p, Type : int *) has loans to { a } // CHECK-NEXT: Src: [[O_ADDR_A]] (Expr: UnaryOperator, Type : int *) int **pp = &p; // CHECK: Use ([[O_P]] (Decl: p, Type : int *), Read) // CHECK: Issue ({{[0-9]+}} (Path: p), ToOrigin: {{[0-9]+}} (Expr: DeclRefExpr, Decl: p)) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: {{[0-9]+}} (Expr: UnaryOperator, Type : int **) +// CHECK-NEXT: Dest: {{[0-9]+}} (Expr: UnaryOperator, Type : int **) has loans to { p } // CHECK-NEXT: Src: {{[0-9]+}} (Expr: DeclRefExpr, Decl: p) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: {{[0-9]+}} (Expr: UnaryOperator, Type : int *) +// CHECK-NEXT: Dest: {{[0-9]+}} (Expr: UnaryOperator, Type : int *) has loans to { a } // CHECK-NEXT: Src: [[O_P]] (Decl: p, Type : int *) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: [[O_PP_OUTER:[0-9]+]] (Decl: pp, Type : int **) +// CHECK-NEXT: Dest: [[O_PP_OUTER:[0-9]+]] (Decl: pp, Type : int **) has loans to { p } // CHECK-NEXT: Src: {{[0-9]+}} (Expr: UnaryOperator, Type : int **) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: [[O_PP_INNER:[0-9]+]] (Decl: pp, Type : int *) +// CHECK-NEXT: Dest: [[O_PP_INNER:[0-9]+]] (Decl: pp, Type : int *) has loans to { a } // CHECK-NEXT: Src: {{[0-9]+}} (Expr: UnaryOperator, Type : int *) // FIXME: Propagate origins across dereference unary operator* @@ -146,22 +146,22 @@ void pointer_indirection() { // CHECK: Use ([[O_PP_OUTER]] (Decl: pp, Type : int **), [[O_PP_INNER]] (Decl: pp, Type : int *), Read) // CHECK: Issue ({{[0-9]+}} (Path: pp), ToOrigin: {{[0-9]+}} (Expr: DeclRefExpr, Decl: pp)) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: {{[0-9]+}} (Expr: ImplicitCastExpr, Type : int **) +// CHECK-NEXT: Dest: {{[0-9]+}} (Expr: ImplicitCastExpr, Type : int **) has loans to { p } // CHECK-NEXT: Src: [[O_PP_OUTER]] (Decl: pp, Type : int **) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: {{[0-9]+}} (Expr: ImplicitCastExpr, Type : int *) +// CHECK-NEXT: Dest: {{[0-9]+}} (Expr: ImplicitCastExpr, Type : int *) has loans to { a } // CHECK-NEXT: Src: [[O_PP_INNER]] (Decl: pp, Type : int *) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: {{[0-9]+}} (Expr: UnaryOperator, Type : int *&) +// CHECK-NEXT: Dest: {{[0-9]+}} (Expr: UnaryOperator, Type : int *&) has loans to { p } // CHECK-NEXT: Src: {{[0-9]+}} (Expr: ImplicitCastExpr, Type : int **) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: {{[0-9]+}} (Expr: UnaryOperator, Type : int *) +// CHECK-NEXT: Dest: {{[0-9]+}} (Expr: UnaryOperator, Type : int *) has loans to { a } // CHECK-NEXT: Src: {{[0-9]+}} (Expr: ImplicitCastExpr, Type : int *) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: {{[0-9]+}} (Expr: ImplicitCastExpr, Type : int *) +// CHECK-NEXT: Dest: {{[0-9]+}} (Expr: ImplicitCastExpr, Type : int *) has loans to { a } // CHECK-NEXT: Src: {{[0-9]+}} (Expr: UnaryOperator, Type : int *) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: {{[0-9]+}} (Decl: q, Type : int *) +// CHECK-NEXT: Dest: {{[0-9]+}} (Decl: q, Type : int *) has loans to { a } // CHECK-NEXT: Src: {{[0-9]+}} (Expr: ImplicitCastExpr, Type : int *) } @@ -180,33 +180,33 @@ void test_use_lifetimebound_call() { MyObj *q = &y; // CHECK: Issue ([[L_Y:[0-9]+]] (Path: y), ToOrigin: [[O_DRE_Y:[0-9]+]] (Expr: DeclRefExpr, Decl: y)) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: [[O_ADDR_Y:[0-9]+]] (Expr: UnaryOperator, Type : MyObj *) +// CHECK-NEXT: Dest: [[O_ADDR_Y:[0-9]+]] (Expr: UnaryOperator, Type : MyObj *) has loans to { y } // CHECK-NEXT: Src: [[O_DRE_Y]] (Expr: DeclRefExpr, Decl: y) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: [[O_Q:[0-9]+]] (Decl: q, Type : MyObj *) +// CHECK-NEXT: Dest: [[O_Q:[0-9]+]] (Decl: q, Type : MyObj *) has loans to { y } // CHECK-NEXT: Src: [[O_ADDR_Y]] (Expr: UnaryOperator, Type : MyObj *) MyObj* r = LifetimeBoundCall(p, q); // CHECK: Use ([[O_P]] (Decl: p, Type : MyObj *), Read) // CHECK: Issue ({{[0-9]+}} (Path: p), ToOrigin: {{[0-9]+}} (Expr: DeclRefExpr, Decl: p)) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: [[O_P_RVAL:[0-9]+]] (Expr: ImplicitCastExpr, Type : MyObj *) +// CHECK-NEXT: Dest: [[O_P_RVAL:[0-9]+]] (Expr: ImplicitCastExpr, Type : MyObj *) has loans to { x } // CHECK-NEXT: Src: [[O_P]] (Decl: p, Type : MyObj *) // CHECK: Use ([[O_Q]] (Decl: q, Type : MyObj *), Read) // CHECK: Issue ({{[0-9]+}} (Path: q), ToOrigin: {{[0-9]+}} (Expr: DeclRefExpr, Decl: q)) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: [[O_Q_RVAL:[0-9]+]] (Expr: ImplicitCastExpr, Type : MyObj *) +// CHECK-NEXT: Dest: [[O_Q_RVAL:[0-9]+]] (Expr: ImplicitCastExpr, Type : MyObj *) has loans to { y } // CHECK-NEXT: Src: [[O_Q]] (Decl: q, Type : MyObj *) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: [[O_CALL_EXPR:[0-9]+]] (Expr: CallExpr, Type : MyObj *) +// CHECK-NEXT: Dest: [[O_CALL_EXPR:[0-9]+]] (Expr: CallExpr, Type : MyObj *) has loans to { x } // CHECK-NEXT: Src: [[O_P_RVAL]] (Expr: ImplicitCastExpr, Type : MyObj *) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: [[O_CALL_EXPR]] (Expr: CallExpr, Type : MyObj *) +// CHECK-NEXT: Dest: [[O_CALL_EXPR]] (Expr: CallExpr, Type : MyObj *) has loans to { x y } // CHECK-NEXT: Src: [[O_Q_RVAL]] (Expr: ImplicitCastExpr, Type : MyObj *), Merge // CHECK: OriginFlow: -// CHECK-NEXT: Dest: {{[0-9]+}} (Decl: r, Type : MyObj *) +// CHECK-NEXT: Dest: {{[0-9]+}} (Decl: r, Type : MyObj *) has loans to { x y } // CHECK-NEXT: Src: [[O_CALL_EXPR]] (Expr: CallExpr, Type : MyObj *) -// CHECK: Expire ([[L_Y]] (Path: y)) -// CHECK: Expire ([[L_X]] (Path: x)) +// CHECK: Expire (y) +// CHECK: Expire (x) } // CHECK-LABEL: Function: test_reference_variable @@ -217,23 +217,23 @@ void test_reference_variable() { // CHECK: Block B{{[0-9]+}}: // CHECK: Issue ([[L_X:[0-9]+]] (Path: x), ToOrigin: [[O_DRE_X:[0-9]+]] (Expr: DeclRefExpr, Decl: x)) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: [[O_CAST_Y:[0-9]+]] (Expr: ImplicitCastExpr, Type : const MyObj &) +// CHECK-NEXT: Dest: [[O_CAST_Y:[0-9]+]] (Expr: ImplicitCastExpr, Type : const MyObj &) has loans to { x } // CHECK-NEXT: Src: [[O_DRE_X]] (Expr: DeclRefExpr, Decl: x) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: [[O_Y:[0-9]+]] (Decl: y, Type : const MyObj &) +// CHECK-NEXT: Dest: [[O_Y:[0-9]+]] (Decl: y, Type : const MyObj &) has loans to { x } // CHECK-NEXT: Src: [[O_CAST_Y]] (Expr: ImplicitCastExpr, Type : const MyObj &) const MyObj& z = y; // CHECK: OriginFlow: -// CHECK-NEXT: Dest: [[O_Z:[0-9]+]] (Decl: z, Type : const MyObj &) +// CHECK-NEXT: Dest: [[O_Z:[0-9]+]] (Decl: z, Type : const MyObj &) has loans to { x } // CHECK-NEXT: Src: [[O_Y]] (Decl: y, Type : const MyObj &) p = &z; // CHECK: OriginFlow: -// CHECK-NEXT: Dest: {{[0-9]+}} (Expr: UnaryOperator, Type : const MyObj *) +// CHECK-NEXT: Dest: {{[0-9]+}} (Expr: UnaryOperator, Type : const MyObj *) has loans to { x } // CHECK-NEXT: Src: [[O_Z]] (Decl: z, Type : const MyObj &) // CHECK: Use ({{[0-9]+}} (Decl: p, Type : const MyObj *), Write) // CHECK: Issue ({{[0-9]+}} (Path: p), ToOrigin: {{[0-9]+}} (Expr: DeclRefExpr, Decl: p)) // CHECK: OriginFlow: -// CHECK-NEXT: Dest: {{[0-9]+}} (Decl: p, Type : const MyObj *) +// CHECK-NEXT: Dest: {{[0-9]+}} (Decl: p, Type : const MyObj *) has loans to { x } // CHECK-NEXT: Src: {{[0-9]+}} (Expr: UnaryOperator, Type : const MyObj *) -// CHECK: Expire ([[L_X]] (Path: x)) +// CHECK: Expire (x) } diff --git a/clang/test/Sema/warn-lifetime-safety-invalidations.cpp b/clang/test/Sema/warn-lifetime-safety-invalidations.cpp index c9ce0c35c53d2..f687692540a4d 100644 --- a/clang/test/Sema/warn-lifetime-safety-invalidations.cpp +++ b/clang/test/Sema/warn-lifetime-safety-invalidations.cpp @@ -7,16 +7,25 @@ bool Bool(); 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) { +void IteratorInvalidAfterResizeNotDetected(int new_size) { std::vector<int> v; + // FIXME: We do not currently handle non-member lifetimebound functions to return an interior path. + // Currently 'it' will have loan with path 'v'. We want it to instead have 'v.*' inorder to be later invalidated. auto it = std::begin(v); v.resize(new_size); - it = std::begin(v); + *it; +} + +void IteratorValidAfterResize(int new_size) { + std::vector<int> v; + auto it = v.begin(); + v.resize(new_size); + it = v.begin(); if (it != std::end(v)) { *it; // ok } @@ -39,17 +48,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 +72,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 +129,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 +139,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 +171,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 +188,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 +196,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 +206,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 +231,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 +268,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 +295,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,51 +323,120 @@ 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 InvalidateBothInASingleExpression(bool cond) { + S s; + auto it1 = s.strings1.begin(); // expected-warning {{object whose reference is captured is later invalidated}} + auto it2 = s.strings2.begin(); // expected-warning {{object whose reference is captured is later invalidated}} + auto& both = cond ? s.strings1 : s.strings2; + both.push_back("1"); // expected-note 2 {{invalidated here}} + *it1; // expected-note {{later used here}} + *it2; // expected-note {{later used here}} } + void Invalidate1UseSIsOk() { S s; S* p = &s; s.strings2.push_back("1"); (void)*p; } + +// FIXME: Detect invalidation of fields. +// https://github.com/llvm/llvm-project/issues/180992 +struct InvalidateMemberFields { + InvalidateMemberFields(); + void invalidateField() { + auto it = container.begin(); + container.push_back("1"); + *it; + } + void invalidateFieldRef() { + auto it = contiainerRef.begin(); + contiainerRef.push_back("1"); + *it; + } +private: + std::vector<std::string> container; + std::vector<std::string>& contiainerRef; +}; +} // namespace ContainersAsFields + 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) path = std::string(); } -} // 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..f8352b2348780 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 @@ -166,9 +171,10 @@ class LifetimeTestHelper { const ExpireFact * getExpireFactFromAllFacts(const llvm::ArrayRef<const Fact *> &FactsInBlock, const LoanID &loanID) { + const Loan *L = Analysis.getFactManager().getLoanMgr().getLoan(loanID); for (const Fact *F : FactsInBlock) { if (auto const *CurrentEF = F->getAs<ExpireFact>()) - if (CurrentEF->getLoanID() == loanID) + if (CurrentEF->getAccessPath() == L->getAccessPath()) return CurrentEF; } return nullptr; @@ -261,11 +267,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 +287,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 +757,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 +773,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 +793,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 +808,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 +822,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 +852,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 +873,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 +894,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 +920,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 +937,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 +957,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 +989,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 +1009,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 +1028,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 +1057,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 +1070,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 +1105,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 +1138,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 +1162,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 +1184,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 +1729,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 +1747,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 +1765,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 +1783,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 +1802,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 +1821,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 +1867,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 +1960,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
