https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/180369
>From a67a3ec7d03efd3578366477790ec0644ab740ab Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <[email protected]> Date: Sat, 4 Jul 2026 07:31:45 +0000 Subject: [PATCH] [LifetimeSafety] Support field-sensitive and interior-sensitive AccessPaths This patch improves the precision of the Lifetime Safety Analysis by introducing field and interior paths to track lifetimes at a finer granularity. ### Patch Description: - **Access Paths**: Added support for tracking fields (`.field`) and container interiors (`.*`) in `AccessPath`. This allows the analysis to distinguish between borrowing a container object itself (owner-borrow) vs. borrowing its elements (content-borrow). - **Facts and Loans**: Extended `Fact` and `Loan` representations to handle nested paths. - **`std::begin`/`std::end` support**: Free functions like `std::begin` and `std::data` that return iterators or pointers to container elements are now correctly recognized as returning interior borrows (`.*` path element) instead of a borrow of the container object itself. ### Design Changes / Invalidation Rules: - **Container Invalidation**: Modifying a container (e.g., `v.resize()`) now invalidates loans pointing to its interior (`v.*`), but does not invalidate loans to the container object itself (`v`). This uses a strict prefix check (`isStrictPrefixOf`). - **Memory Freeing (`delete`)**: Freeing memory via `delete p` invalidates the pointer `p` itself (equality) as well as any sub-paths (prefix). This uses a prefix check (`isPrefixOf`) to ensure the pointer is marked as dangling. - **Robustness**: Fixed a compiler crash on qualified null types by checking `getTypePtrOrNull()` instead of `isNull()` in `getPathElementForLifetimeBoundArg`. ### Test Changes: - Updated `clang/test/Sema/LifetimeSafety/invalidations.cpp` to match the new behavior: - Removed duplicate false-positive warnings for self-invalidating container operations (e.g., `mp[2] = mp[1]`). - Added warnings for parenthesized container modifications (e.g. `(v).push_back(42)`) and conditional container/field modifications (e.g., `(flag ? v1 : v2).push_back(42)`). - Resolved several FIXMEs regarding field and container alias invalidations. TAG=agy CONV=2cfd8d00-18d7-4a03-8d78-2aba2f9a8f23 --- .../Analysis/Analyses/LifetimeSafety/Facts.h | 77 +++-- .../Analyses/LifetimeSafety/FactsGenerator.h | 1 + .../Analysis/Analyses/LifetimeSafety/Loans.h | 241 ++++++++++----- clang/lib/Analysis/LifetimeSafety/Checker.cpp | 71 +++-- clang/lib/Analysis/LifetimeSafety/Facts.cpp | 60 +++- .../LifetimeSafety/FactsGenerator.cpp | 74 +++-- .../LifetimeSafety/LifetimeSafety.cpp | 10 +- .../LifetimeSafety/LoanPropagation.cpp | 38 ++- clang/lib/Analysis/LifetimeSafety/Loans.cpp | 99 +++++-- .../Analysis/LifetimeSafety/MovedLoans.cpp | 2 +- .../LifetimeSafety/Inputs/lifetime-analysis.h | 9 +- .../Sema/LifetimeSafety/invalidations.cpp | 228 ++++++++++++--- .../Sema/LifetimeSafety/lifetime-facts.cpp | 64 ++-- .../unittests/Analysis/LifetimeSafetyTest.cpp | 275 ++++++++++++------ .../AMDGPU/vgpr-spill-placement-issue61083.ll | 1 - 15 files changed, 879 insertions(+), 371 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index 5c671a93b149c..e435065584340 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -13,21 +13,21 @@ //===----------------------------------------------------------------------===// #ifndef LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMESAFETY_FACTS_H #define LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMESAFETY_FACTS_H - #include "clang/AST/Decl.h" #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 "llvm/Support/raw_ostream.h" #include <cstdint> #include <optional> namespace clang::lifetimes::internal { +class LoanPropagationAnalysis; + using FactID = utils::ID<struct FactTag>; /// An abstract base class for a single, atomic lifetime-relevant event. @@ -80,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 @@ -101,14 +102,17 @@ 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 associated with this path expire. For example, if `x` expires, then -/// the loan to `x` expires. +/// 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 { - // The access path that expires. + /// The access path that expires (e.g., the variable going out of scope). AccessPath AP; // Expired origin (e.g., its variable goes out of scope). @@ -127,31 +131,41 @@ class ExpireFact : public Fact { SourceLocation getExpiryLoc() const { return ExpiryLoc; } void dump(llvm::raw_ostream &OS, const LoanManager &LM, - const OriginManager &OM) const override; + const OriginManager &OM, + 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. @@ -191,8 +205,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. @@ -211,8 +225,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; }; /// Represents that an origin escapes via assignment to global or static @@ -231,7 +245,8 @@ class GlobalEscapeFact : public OriginEscapesFact { } const VarDecl *getGlobal() const { return Global; }; void dump(llvm::raw_ostream &OS, const LoanManager &, - const OriginManager &OM) const override; + const OriginManager &OM, + const LoanPropagationAnalysis *LPA = nullptr) const override; }; class UseFact : public Fact { @@ -253,8 +268,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 @@ -276,8 +291,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 @@ -297,8 +312,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. @@ -314,8 +329,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; }; /// All loans are cleared from an origin (e.g., assigning a callable without @@ -333,7 +348,8 @@ class KillOriginFact : public Fact { OriginID getKilledOrigin() const { return OID; } void dump(llvm::raw_ostream &OS, const LoanManager &, - const OriginManager &OM) const override; + const OriginManager &OM, + const LoanPropagationAnalysis *LPA = nullptr) const override; }; class FactManager { @@ -363,7 +379,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 9821078ec1d1e..adb73f563fb17 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h @@ -67,6 +67,7 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> { bool hasOrigins(const Expr *E) const; void flow(OriginList *Dst, OriginList *Src, bool Kill, + std::optional<PathElement> AddPath = std::nullopt, const CFGBlock *Block = nullptr); /// Handles assignment for both BinaryOperator and CXXOperatorCallExpr. diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h index cfaae4d574686..a5537ffc62b19 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,126 +31,210 @@ 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 or a field within it: var.field.* -/// -/// An AccessPath consists of a root which is one of: -/// - ValueDecl: a local variable or global -/// - MaterializeTemporaryExpr: a temporary object -/// - ParmVarDecl: a function parameter (placeholder) -/// - CXXMethodDecl: the implicit 'this' object (placeholder) -/// - CXXNewExpr: a heap allocation made by `new` -/// -/// Placeholder paths never expire within the function scope, as they represent -/// storage from the caller's scope. +/// Represents one step in an access path: either a field access or an +/// access to an unnamed interior region (denoted by '*'). /// -/// TODO: Model access paths of other types, e.g. field, array subscript, heap -/// allocation not through `new`, and globals. -class AccessPath { +/// 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: - enum class Kind : uint8_t { - ValueDecl, - MaterializeTemporary, - PlaceholderParam, - PlaceholderThis, - NewAllocation, - }; + enum class Kind { Field, Interior }; + + static PathElement getField(const FieldDecl *FD) { + return PathElement(Kind::Field, FD); + } + static PathElement getInterior() { + return PathElement(Kind::Interior, nullptr); + } + + bool isField() const { return K == Kind::Field; } + bool isInterior() const { return K == Kind::Interior; } + const FieldDecl *getFieldDecl() const { return FD; } + + bool operator==(const PathElement &Other) const { + return K == Other.K && FD == Other.FD; + } + bool operator!=(const PathElement &Other) const { return !(*this == Other); } + + void dump(llvm::raw_ostream &OS) const { + if (isField()) + OS << "." << FD->getNameAsString(); + else + OS << ".*"; + } private: + PathElement(Kind K, const FieldDecl *FD) : K(K), FD(FD) {} Kind K; - llvm::PointerUnion<const Expr *, const Decl *> Root; + const FieldDecl *FD; +}; + +/// 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: - AccessPath(const clang::ValueDecl *D) : K(Kind::ValueDecl), Root(D) {} - AccessPath(const clang::MaterializeTemporaryExpr *MTE) - : K(Kind::MaterializeTemporary), Root(MTE) {} - AccessPath(const CXXNewExpr *New) : K(Kind::NewAllocation), Root(New) {} - static AccessPath Placeholder(const ParmVarDecl *PVD) { - return AccessPath(Kind::PlaceholderParam, PVD); + PlaceholderBase(const ParmVarDecl *PVD) : ParamOrMethod(PVD) {} + PlaceholderBase(const CXXMethodDecl *MD) : ParamOrMethod(MD) {} + + const ParmVarDecl *getParmVarDecl() const { + return ParamOrMethod.dyn_cast<const ParmVarDecl *>(); } - static AccessPath Placeholder(const CXXMethodDecl *MD) { - return AccessPath(Kind::PlaceholderThis, MD); + + const CXXMethodDecl *getMethodDecl() const { + return ParamOrMethod.dyn_cast<const CXXMethodDecl *>(); } - AccessPath(const AccessPath &Other) : K(Other.K), Root(Other.Root) {} - AccessPath &operator=(const AccessPath &) = delete; - Kind getKind() const { return K; } + void Profile(llvm::FoldingSetNodeID &ID) const { + ID.AddPointer(ParamOrMethod.getOpaqueValue()); + } +}; + +/// Represents the storage location being borrowed, e.g., a specific stack +/// variable or a field within it: var.field.* +/// +/// An AccessPath consists of: +/// - A base: either a ValueDecl, MaterializeTemporaryExpr, or PlaceholderBase +/// - A sequence of PathElements representing field accesses or interior +/// regions +/// +/// Examples: +/// - `x` -> Base=x, Elements=[] +/// - `x.field` -> Base=x, Elements=[.field] +/// - `x.*` (e.g., string_view from string) -> Base=x, Elements=[.*] +/// - `x.field.*` -> Base=x, Elements=[.field, .*] +/// - `$param.field` -> Base=$param, Elements=[.field] +/// +/// TODO: Model access paths of other types, e.g. heap and globals. +class AccessPath { + /// The base of the access path: a variable, temporary, or placeholder. + const llvm::PointerUnion<const clang::ValueDecl *, + const clang::MaterializeTemporaryExpr *, + const PlaceholderBase *, + const clang::CXXNewExpr *> + Base; + /// The path elements representing field accesses and access to unnamed + /// interior regions. + llvm::SmallVector<PathElement, 1> Elements; + +public: + AccessPath(const clang::ValueDecl *D) : Base(D) {} + AccessPath(const clang::MaterializeTemporaryExpr *MTE) : Base(MTE) {} + AccessPath(const PlaceholderBase *PB) : Base(PB) {} + AccessPath(const clang::CXXNewExpr *New) : Base(New) {} + + /// 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 clang::ValueDecl *getAsValueDecl() const { - return K == Kind::ValueDecl - ? cast<const clang::ValueDecl>(cast<const clang::Decl *>(Root)) - : nullptr; + return Base.dyn_cast<const clang::ValueDecl *>(); } + const clang::MaterializeTemporaryExpr *getAsMaterializeTemporaryExpr() const { - return K == Kind::MaterializeTemporary - ? cast<const MaterializeTemporaryExpr>( - cast<const clang::Expr *>(Root)) - : nullptr; - } - const ParmVarDecl *getAsPlaceholderParam() const { - return K == Kind::PlaceholderParam - ? cast<const ParmVarDecl>(cast<const clang::Decl *>(Root)) - : nullptr; + return Base.dyn_cast<const clang::MaterializeTemporaryExpr *>(); } - const CXXMethodDecl *getAsPlaceholderThis() const { - return K == Kind::PlaceholderThis - ? cast<const CXXMethodDecl>(cast<const clang::Decl *>(Root)) - : nullptr; + + const PlaceholderBase *getAsPlaceholderBase() const { + return Base.dyn_cast<const PlaceholderBase *>(); } - const CXXNewExpr *getAsNewAllocation() const { - return K == Kind::NewAllocation - ? cast<const CXXNewExpr>(cast<const clang::Expr *>(Root)) - : nullptr; + + const clang::CXXNewExpr *getAsNewAllocation() const { + return Base.dyn_cast<const clang::CXXNewExpr *>(); } bool operator==(const AccessPath &RHS) const { - return K == RHS.K && Root == RHS.Root; + return Base == RHS.Base && Elements == RHS.Elements; } - bool operator!=(const AccessPath &RHS) const { return !(*this == RHS); } - void dump(llvm::raw_ostream &OS) const; -private: - AccessPath(Kind K, const ParmVarDecl *PVD) : K(K), Root(PVD) {} - AccessPath(Kind K, const CXXMethodDecl *MD) : K(K), Root(MD) {} + /// 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; + } + + /// 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(); + } + llvm::ArrayRef<PathElement> getElements() const { return Elements; } + + 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. Null for placeholder + /// The expression that creates the loan, e.g., &x. Optional for placeholder /// loans. - const Expr *IssuingExpr; + const Expr *IssueExpr; public: - Loan(LoanID ID, AccessPath Path, const Expr *IssuingExpr) - : ID(ID), Path(Path), IssuingExpr(IssuingExpr) {} + 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 *getIssuingExpr() const { return IssuingExpr; } + 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; - Loan *createLoan(AccessPath Path, const Expr *IssueExpr) { + 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); + + /// Finds the base loan IDs that could have been extended to produce ExtendedLoanID. + llvm::SmallVector<LoanID, 2> getBaseLoans(LoanID ExtendedLoanID, + PathElement Element) const; + const Loan *getLoan(LoanID ID) const { assert(ID.Value < AllLoans.size()); return AllLoans[ID.Value]; @@ -158,6 +246,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; @@ -165,4 +261,15 @@ class LoanManager { }; } // namespace clang::lifetimes::internal +namespace llvm { +template <> struct DenseMapInfo<clang::lifetimes::internal::PathElement> { + using PathElement = clang::lifetimes::internal::PathElement; + 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 746ebbfb15c39..559cc410abd42 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -164,10 +164,12 @@ class LifetimeChecker { auto MovedAtEscape = MovedLoans.getMovedLoans(OEF); for (LoanID LID : EscapedLoans) { const Loan *L = FactMgr.getLoanMgr().getLoan(LID); - const AccessPath &AP = L->getAccessPath(); - if (const auto *PVD = AP.getAsPlaceholderParam()) + const PlaceholderBase *PB = L->getAccessPath().getAsPlaceholderBase(); + if (!PB) + continue; + if (const auto *PVD = PB->getParmVarDecl()) CheckParam(PVD, /*IsMoved=*/MovedAtEscape.lookup(LID)); - else if (const auto *MD = AP.getAsPlaceholderThis()) + else if (const auto *MD = PB->getMethodDecl()) CheckImplicitThis(MD); } } @@ -175,7 +177,8 @@ class LifetimeChecker { /// Checks for use-after-free & use-after-return errors when an access path /// expires (e.g., a variable goes out of scope). /// - /// When a path expires, all loans having this path expires. + /// 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) { @@ -185,41 +188,50 @@ class LifetimeChecker { LoanSet HeldLoans = LoanPropagation.getLoans(OID, EF); for (LoanID HeldLoanID : HeldLoans) { const Loan *HeldLoan = FactMgr.getLoanMgr().getLoan(HeldLoanID); - if (ExpiredPath != HeldLoan->getAccessPath()) - continue; - // HeldLoan is expired because its AccessPath is expired. - PendingWarning &CurWarning = FinalWarningsMap[HeldLoan->getID()]; - const Expr *MovedExpr = nullptr; - if (auto *ME = MovedLoans.getMovedLoans(EF).lookup(HeldLoanID)) - MovedExpr = *ME; - // Skip if we already have a dominating causing fact. - if (CurWarning.CausingFactDominatesExpiry) - continue; - if (causingFactDominatesExpiry(LiveInfo.Kind)) - CurWarning.CausingFactDominatesExpiry = true; - CurWarning.CausingFact = LiveInfo.CausingFact; - CurWarning.ExpiryLoc = EF->getExpiryLoc(); - CurWarning.MovedExpr = MovedExpr; - CurWarning.InvalidatedByExpr = nullptr; + if (ExpiredPath.isPrefixOf(HeldLoan->getAccessPath())) { + // HeldLoan is expired because its base or itself is expired. + PendingWarning &CurWarning = FinalWarningsMap[HeldLoan->getID()]; + const Expr *MovedExpr = nullptr; + if (auto *ME = MovedLoans.getMovedLoans(EF).lookup(HeldLoanID)) + MovedExpr = *ME; + // Skip if we already have a dominating causing fact. + if (CurWarning.CausingFactDominatesExpiry) + continue; + if (causingFactDominatesExpiry(LiveInfo.Kind)) + CurWarning.CausingFactDominatesExpiry = true; + CurWarning.CausingFact = LiveInfo.CausingFact; + CurWarning.ExpiryLoc = EF->getExpiryLoc(); + CurWarning.MovedExpr = MovedExpr; + CurWarning.InvalidatedByExpr = nullptr; + } } } } /// 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); + bool AllowEquality = + isa_and_nonnull<CXXDeleteExpr>(IOF->getInvalidationExpr()); auto IsInvalidated = [&](const Loan *L) { for (LoanID InvalidID : DirectlyInvalidatedLoans) { const Loan *InvalidL = FactMgr.getLoanMgr().getLoan(InvalidID); - if (InvalidL->getAccessPath() == L->getAccessPath()) - return true; + if (AllowEquality) { + if (InvalidL->getAccessPath().isPrefixOf(L->getAccessPath())) + return true; + } else { + if (InvalidL->getAccessPath().isStrictPrefixOf(L->getAccessPath())) + return true; + } } return false; }; @@ -249,16 +261,19 @@ class LifetimeChecker { return; for (const auto &[LID, Warning] : FinalWarningsMap) { const Loan *L = FactMgr.getLoanMgr().getLoan(LID); - const Expr *IssueExpr = L->getIssuingExpr(); + const Expr *IssueExpr = L->getIssueExpr(); + const ParmVarDecl *InvalidatedPVD = nullptr; + if (const PlaceholderBase *PB = L->getAccessPath().getAsPlaceholderBase()) + InvalidatedPVD = PB->getParmVarDecl(); + llvm::PointerUnion<const UseFact *, const OriginEscapesFact *> CausingFact = Warning.CausingFact; - const ParmVarDecl *InvalidatedPVD = - L->getAccessPath().getAsPlaceholderParam(); const Expr *MovedExpr = Warning.MovedExpr; SourceLocation ExpiryLoc = Warning.ExpiryLoc; if (const auto *UF = CausingFact.dyn_cast<const UseFact *>()) { if (Warning.InvalidatedByExpr) { + // Use-after-invalidation of an object on stack. if (IssueExpr) // Use-after-invalidation of an object on stack. SemaHelper->reportUseAfterInvalidation(IssueExpr, UF->getUseExpr(), diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp index 3d7fbcdacc830..5dfc2c37f8852 100644 --- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp @@ -8,17 +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" 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: "; @@ -27,7 +29,8 @@ void IssueFact::dump(llvm::raw_ostream &OS, const LoanManager &LM, } void ExpireFact::dump(llvm::raw_ostream &OS, const LoanManager &LM, - const OriginManager &OM) const { + const OriginManager &OM, + const LoanPropagationAnalysis *LPA) const { OS << "Expire ("; getAccessPath().dump(OS); if (auto OID = getOriginID()) { @@ -37,48 +40,71 @@ void ExpireFact::dump(llvm::raw_ostream &OS, const LoanManager &LM, 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 GlobalEscapeFact::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 Global)\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; @@ -92,19 +118,22 @@ 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"; } void KillOriginFact::dump(llvm::raw_ostream &OS, const LoanManager &, - const OriginManager &OM) const { + const OriginManager &OM, + const LoanPropagationAnalysis *) const { OS << "KillOrigin ("; OM.dump(getKilledOrigin(), OS); OS << ")\n"; @@ -125,7 +154,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"; @@ -137,7 +167,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 095bee8135c20..09c4ed0bb2787 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -52,6 +52,10 @@ bool FactsGenerator::hasOrigins(const Expr *E) const { /// have the same shape (same depth/structure). This invariant ensures that /// origins flow only between compatible types during expression evaluation. /// +/// If AddPath is specified, loans from the source are extended by the given +/// path element before flowing to the destination. This is used for field +/// accesses and interior borrows. +/// /// Examples: /// - `int* p = &x;` flows origins from `&x` (depth 1) to `p` (depth 1) /// - `int** pp = &p;` flows origins from `&p` (depth 2) to `pp` (depth 2) @@ -63,10 +67,12 @@ bool FactsGenerator::hasOrigins(const Expr *E) const { /// \param Src The source origin list. /// \param Kill If true, the destination's existing loans are killed before /// flowing. +/// \param AddPath Optional. If provided, loans are extended with this path element. /// \param Block Optional. If provided, the generated flow facts are appended to /// this specific CFG block. Otherwise, they are appended to the /// current block being visited. void FactsGenerator::flow(OriginList *Dst, OriginList *Src, bool Kill, + std::optional<PathElement> AddPath, const CFGBlock *Block) { if (!Dst) return; @@ -77,7 +83,7 @@ void FactsGenerator::flow(OriginList *Dst, OriginList *Src, bool Kill, while (Dst && Src) { Fact *F = FactMgr.createFact<OriginFlowFact>(Dst->getOuterOriginID(), - Src->getOuterOriginID(), Kill); + Src->getOuterOriginID(), Kill, AddPath); if (Block) FactMgr.appendBlockFact(Block, F); else @@ -274,17 +280,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))); } } @@ -602,10 +608,11 @@ void FactsGenerator::VisitAbstractConditionalOperator( const Expr *FalseExpr = CO->getFalseExpr(); if (const CFGBlock *TBPred = findPredBlockForExpr(CurrentBlock, TrueExpr)) - flow(getOriginsList(*CO), getOriginsList(*TrueExpr), /*Kill=*/true, TBPred); + flow(getOriginsList(*CO), getOriginsList(*TrueExpr), /*Kill=*/true, + std::nullopt, TBPred); if (const CFGBlock *FBPred = findPredBlockForExpr(CurrentBlock, FalseExpr)) flow(getOriginsList(*CO), getOriginsList(*FalseExpr), /*Kill=*/true, - FBPred); + std::nullopt, FBPred); } void FactsGenerator::VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *OCE) { @@ -954,18 +961,13 @@ void FactsGenerator::handleInvalidatingCall(const Expr *Call, if (!isInvalidationMethod(*MD)) return; - // Heuristics to turn-down false positives. Skip member field expressions for - // now. This is not a perfect filter and will still surface some false - // positives (e.g. `auto& r = s.v`). - if (!isa<DeclRefExpr>(Args[0]->IgnoreImpCasts())) - return; - OriginList *ThisList = getOriginsList(*Args[0]); if (ThisList) CurrentBlockFacts.push_back(FactMgr.createFact<InvalidateOriginFact>( ThisList->getOuterOriginID(), Call)); } + void FactsGenerator::handleDestructiveCall(const Expr *Call, const FunctionDecl *FD, ArrayRef<const Expr *> Args) { @@ -1069,6 +1071,28 @@ void FactsGenerator::handleLifetimeCaptureBy(const FunctionDecl *FD, } } +static std::optional<PathElement> +getPathElementForLifetimeBoundArg(const FunctionDecl *FD, unsigned ArgIndex, + const Expr *ArgExpr) { + if (!ArgExpr) + return std::nullopt; + if (ArgIndex == 0) { + const auto *Method = dyn_cast<CXXMethodDecl>(FD); + bool IsContainerArg = + (Method && Method->isInstance()) || shouldTrackFirstArgument(FD); + if (IsContainerArg) { + QualType ArgType = ArgExpr->getType(); + if (const Type *ArgTypePtr = ArgType.getTypePtrOrNull()) { + if (isGslOwnerType(ArgType) || + (ArgTypePtr->isPointerType() && + isGslOwnerType(ArgTypePtr->getPointeeType()))) + return PathElement::getInterior(); + } + } + } + return std::nullopt; +} + void FactsGenerator::handleFunctionCall(const Expr *Call, const FunctionDecl *FD, ArrayRef<const Expr *> Args, @@ -1142,13 +1166,13 @@ 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())) { - // The constructed gsl::Pointer borrows from the Owner's storage, not - // from what the Owner itself borrows, so only the outermost origin is - // needed. + // GSL construction creates a view that borrows from arguments. + // Only flow the outer origin because inner lengths may mismatch. CurrentBlockFacts.push_back(FactMgr.createFact<OriginFlowFact>( CallList->getOuterOriginID(), ArgList->getOuterOriginID(), - KillSrc)); + KillSrc, PathElement::getInterior())); KillSrc = false; } else if (IsArgLifetimeBound(I)) { // Only flow the outer origin here. For lifetimebound args in @@ -1162,7 +1186,7 @@ void FactsGenerator::handleFunctionCall(const Expr *Call, KillSrc)); KillSrc = false; } - } else if (shouldTrackPointerImplicitObjectArg(I)) { + } else if (I == 0 && shouldTrackPointerImplicitObjectArg(I)) { assert(ArgList->getLength() >= 2 && "Object arg of pointer type should have at least two origins"); // See through the GSLPointer reference to see the pointer's value. @@ -1175,7 +1199,8 @@ void FactsGenerator::handleFunctionCall(const Expr *Call, // pointer/reference itself must not outlive the arguments. This // only constrains 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; } } @@ -1237,9 +1262,9 @@ llvm::SmallVector<Fact *> FactsGenerator::issuePlaceholderLoans() { llvm::SmallVector<Fact *> PlaceholderLoanFacts; if (auto ThisOrigins = FactMgr.getOriginMgr().getThisOrigins()) { OriginList *List = *ThisOrigins; - const Loan *L = FactMgr.getLoanMgr().createLoan( - AccessPath::Placeholder(cast<CXXMethodDecl>(FD)), - /*IssuingExpr=*/nullptr); + const PlaceholderBase *PB = FactMgr.getLoanMgr().getOrCreatePlaceholderBase( + cast<CXXMethodDecl>(FD)); + const Loan *L = FactMgr.getLoanMgr().createLoan(AccessPath(PB)); PlaceholderLoanFacts.push_back( FactMgr.createFact<IssueFact>(L->getID(), List->getOuterOriginID())); } @@ -1247,8 +1272,9 @@ llvm::SmallVector<Fact *> FactsGenerator::issuePlaceholderLoans() { OriginList *List = getOriginsList(*PVD); if (!List) continue; - const Loan *L = FactMgr.getLoanMgr().createLoan( - AccessPath::Placeholder(PVD), /*IssuingExpr=*/nullptr); + 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 680095a54177c..798812327d3ab 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 @@ -101,12 +102,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 a67b1b3c0f826..762042d361a52 100644 --- a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp @@ -176,14 +176,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 LoansToFlow = SrcLoans; + + // Extend loans if a path element is specified (e.g., for field access). + if (auto Element = F.getPathElement()) { + LoansToFlow = LoanSetFactory.getEmptySet(); + for (LoanID LID : SrcLoans) { + Loan *ExtendedLoan = + FactMgr.getLoanMgr().getOrCreateExtendedLoan(LID, *Element); + LoansToFlow = LoanSetFactory.add(LoansToFlow, 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, LoansToFlow, LoanSetFactory); return setLoans(In, DestOID, MergedLoans); } @@ -208,6 +222,7 @@ class AnalysisImpl assert(getLoans(StartOID, StartPoint).contains(TargetLoan) && "TargetLoan must be present in the StartOID at the StartPoint"); + LoanID CurrLoanID = TargetLoan; OriginID CurrOID = StartOID; llvm::SmallVector<OriginID> OriginFlowChain; llvm::ArrayRef<const Fact *> Facts = FactMgr.getBlockContaining(StartPoint); @@ -217,7 +232,7 @@ class AnalysisImpl for (const Fact *F : llvm::reverse(llvm::make_range(Facts.begin(), StartIt))) { if (const auto *IF = F->getAs<IssueFact>()) - if (IF->getLoanID() == TargetLoan) { + if (IF->getLoanID() == CurrLoanID) { assert(IF->getOriginID() == CurrOID); return OriginFlowChain; } @@ -229,8 +244,23 @@ class AnalysisImpl continue; const OriginID SrcOriginID = OFF->getSrcOriginID(); - if (!getLoans(SrcOriginID, OFF).contains(TargetLoan)) + std::optional<LoanID> NextLoanID; + if (auto AddPath = OFF->getPathElement()) { + auto Candidates = + FactMgr.getLoanMgr().getBaseLoans(CurrLoanID, *AddPath); + for (LoanID Candidate : Candidates) { + if (getLoans(SrcOriginID, OFF).contains(Candidate)) { + NextLoanID = Candidate; + break; + } + } + } else { + if (getLoans(SrcOriginID, OFF).contains(CurrLoanID)) + NextLoanID = CurrLoanID; + } + if (!NextLoanID) continue; + CurrLoanID = *NextLoanID; OriginFlowChain.push_back(SrcOriginID); CurrOID = SrcOriginID; } diff --git a/clang/lib/Analysis/LifetimeSafety/Loans.cpp b/clang/lib/Analysis/LifetimeSafety/Loans.cpp index c0c8d0b283f8b..b83bd1eb9e006 100644 --- a/clang/lib/Analysis/LifetimeSafety/Loans.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Loans.cpp @@ -11,28 +11,22 @@ namespace clang::lifetimes::internal { void AccessPath::dump(llvm::raw_ostream &OS) const { - switch (K) { - case Kind::ValueDecl: - if (const clang::ValueDecl *VD = getAsValueDecl()) - OS << VD->getNameAsString(); - break; - case Kind::MaterializeTemporary: - if (const clang::MaterializeTemporaryExpr *MTE = - getAsMaterializeTemporaryExpr()) - OS << "MaterializeTemporaryExpr at " << MTE; - break; - case Kind::PlaceholderParam: - if (const auto *PVD = getAsPlaceholderParam()) + if (const clang::ValueDecl *VD = getAsValueDecl()) + OS << VD->getNameAsString(); + else if (const clang::MaterializeTemporaryExpr *MTE = + getAsMaterializeTemporaryExpr()) + OS << "MaterializeTemporaryExpr at " << MTE; + else if (const PlaceholderBase *PB = getAsPlaceholderBase()) { + if (const auto *PVD = PB->getParmVarDecl()) OS << "$" << PVD->getNameAsString(); - break; - case Kind::PlaceholderThis: - OS << "$this"; - break; - case Kind::NewAllocation: - if (const auto *E = getAsNewAllocation()) - OS << "NewAllocation at " << E; - break; - } + else if (PB->getMethodDecl()) + OS << "$this"; + } else if (const auto *E = getAsNewAllocation()) + OS << "NewAllocation at " << E; + else + llvm_unreachable("access path base invalid"); + for (const auto &E : Elements) + E.dump(OS); } void Loan::dump(llvm::raw_ostream &OS) const { @@ -40,4 +34,67 @@ void Loan::dump(llvm::raw_ostream &OS) const { Path.dump(OS); OS << ")"; } + + +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); + // TODO: Polish comment: Do not add interior access if the base loan path + // already contains that at the end. + if (Element.isInterior() && + !BaseLoan->getAccessPath().getElements().empty() && + BaseLoan->getAccessPath().getElements().back().isInterior()) + return ExtensionCache[Key] = const_cast<Loan *>(BaseLoan); + + AccessPath ExtendedPath(BaseLoan->getAccessPath(), Element); + return ExtensionCache[Key] = + createLoan(ExtendedPath, BaseLoan->getIssueExpr()); +} + +llvm::SmallVector<LoanID, 2> +LoanManager::getBaseLoans(LoanID ExtendedLoanID, PathElement Element) const { + llvm::SmallVector<LoanID, 2> Result; + for (const auto &Entry : ExtensionCache) { + if (Entry.second->getID() == ExtendedLoanID && + Entry.first.second == Element) { + Result.push_back(Entry.first.first); + } + } + return Result; +} } // namespace clang::lifetimes::internal diff --git a/clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp b/clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp index 138704821024a..1a9ce3e576733 100644 --- a/clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp +++ b/clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp @@ -80,7 +80,7 @@ class AnalysisImpl auto IsInvalidated = [&](const AccessPath &Path) { for (LoanID LID : ImmediatelyMovedLoans) { const Loan *MovedLoan = LoanMgr.getLoan(LID); - if (MovedLoan->getAccessPath() == Path) + if (MovedLoan->getAccessPath().isPrefixOf(Path)) return true; } return false; diff --git a/clang/test/Sema/LifetimeSafety/Inputs/lifetime-analysis.h b/clang/test/Sema/LifetimeSafety/Inputs/lifetime-analysis.h index 024c3c2bc51b7..815dcf8b6766d 100644 --- a/clang/test/Sema/LifetimeSafety/Inputs/lifetime-analysis.h +++ b/clang/test/Sema/LifetimeSafety/Inputs/lifetime-analysis.h @@ -28,11 +28,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; @@ -225,6 +225,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*); void push_back(T); diff --git a/clang/test/Sema/LifetimeSafety/invalidations.cpp b/clang/test/Sema/LifetimeSafety/invalidations.cpp index be1acc6bc7fbc..37f2d24ce3263 100644 --- a/clang/test/Sema/LifetimeSafety/invalidations.cpp +++ b/clang/test/Sema/LifetimeSafety/invalidations.cpp @@ -338,34 +338,34 @@ void IteratorInvalidatedThroughPointerParameter(std::vector<int> *v) { // expect void ParenthesizedContainerInvalidatesIterator() { // FIXME: Support invalidation through non-DRE lvalue expressions. std::vector<int> v; - auto it = v.begin(); - (v).push_back(42); - (void)it; + auto it = v.begin(); // expected-warning {{local variable 'v' is later invalidated}} + (v).push_back(42); // expected-note {{local variable 'v' is invalidated here}} + (void)it; // expected-note {{later used here}} } } // namespace InvalidatingThroughContainerAliases namespace ContainerObjectAliases { // FIXME: Distinguish owner-borrow from content-borrow. -void PointerParameterObjectUseIsOk(std::vector<int> *v) { // expected-warning {{parameter 'v' is later invalidated}} - v->push_back(42); // expected-note {{parameter 'v' is invalidated here}} - (void)v; // expected-note {{later used here}} +void PointerParameterObjectUseIsOk(std::vector<int> *v) { + v->push_back(42); + (void)v; } // FIXME: Distinguish owner-borrow from content-borrow. void LocalPointerAliasObjectUseIsOk() { std::vector<int> vv; - std::vector<int> *v = &vv; // expected-warning {{local variable 'vv' is later invalidated}} - v->push_back(42); // expected-note {{local variable 'vv' is invalidated here}} - (void)*v; // expected-note {{later used here}} + std::vector<int> *v = &vv; + v->push_back(42); + (void)*v; } // FIXME: Distinguish owner-borrow from content-borrow. void LocalReferenceAliasObjectUseIsOk() { std::vector<int> vv; - std::vector<int> &v = vv; // expected-warning {{local variable 'vv' is later invalidated}} - v.push_back(42); // expected-note {{local variable 'vv' is invalidated here}} - (void)v; // expected-note {{later used here}} + std::vector<int> &v = vv; + v.push_back(42); + (void)v; } } // namespace ContainerObjectAliases @@ -407,13 +407,8 @@ void SelfInvalidatingMap() { // To resolve this, we need to: // 1. Distinguish owner-borrow (borrowing the container object) from content-borrow (borrowing elements inside the container). // 2. Make AccessPaths more precise to reason at element/field granularity rather than treating the whole container as a single storage location. - mp[1] = "42"; // expected-warning {{local variable 'mp' is later invalidated}} \ - // expected-note {{local variable 'mp' is invalidated here}} \ - // expected-note {{later used here}} + mp[1] = "42"; mp[2] = mp[1]; // expected-warning {{local variable 'mp' is later invalidated}} \ - // expected-warning {{local variable 'mp' is later invalidated}} \ - // expected-note {{local variable 'mp' is invalidated here}} \ - // expected-note {{later used here}} \ // expected-note {{local variable 'mp' is invalidated here}} \ // expected-note {{later used here}} } @@ -461,9 +456,9 @@ struct S { 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 {{local variable 's' is later invalidated}} + s.strings1.push_back("1"); // expected-note {{local variable 's' is invalidated here}} + *it; // expected-note {{later used here}} } void Invalidate2Use1IsOk() { S s; @@ -474,24 +469,24 @@ void Invalidate2Use1IsOk() { void ConditionalContainerInvalidatesIterator(bool flag) { // FIXME: Support invalidation through conditional lvalue expressions. std::vector<int> v1, v2; - auto it = v1.begin(); - (flag ? v1 : v2).push_back(42); - (void)it; + auto it = v1.begin(); // expected-warning {{local variable 'v1' is later invalidated}} + (flag ? v1 : v2).push_back(42); // expected-note {{local variable 'v1' is invalidated here}} + (void)it; // expected-note {{later used here}} } void ConditionalFieldInvalidatesIterator(bool flag) { // FIXME: Support conditional invalidation through field expressions. S s; - auto it = s.strings1.begin(); - (flag ? s.strings1 : s.strings2).push_back("1"); - *it; + auto it = s.strings1.begin(); // expected-warning {{local variable 's' is later invalidated}} + (flag ? s.strings1 : s.strings2).push_back("1"); // expected-note {{local variable 's' is invalidated here}} + *it; // expected-note {{later used here}} } // FIXME: Requires field-sensitive AccessPaths to fix. void Invalidate1Use2ViaRefIsOk() { S s; - auto it = s.strings2.begin(); // expected-warning {{local variable 's' is later invalidated}} + auto it = s.strings2.begin(); auto& strings1 = s.strings1; - strings1.push_back("1"); // expected-note {{local variable 's' is invalidated here}} - *it; // expected-note {{later used here}} + strings1.push_back("1"); + *it; } void Invalidate1UseSIsOk() { S s; @@ -502,9 +497,9 @@ void Invalidate1UseSIsOk() { // FIXME: Distinguish owner-borrow from content-borrow. void PointerToContainerIsOk() { std::vector<std::string> s; - std::vector<std::string>* p = &s; // expected-warning {{local variable 's' is later invalidated}} - p->push_back("1"); // expected-note {{local variable 's' is invalidated here}} - (void)*p; // expected-note {{later used here}} + std::vector<std::string>* p = &s; + p->push_back("1"); + (void)*p; } void IteratorFromPointerToContainerIsInvalidated() { std::vector<std::string> s; @@ -517,8 +512,8 @@ void IteratorFromPointerToContainerIsInvalidated() { // iterators into the outer container. void ChangingRegionOwnedByContainerIsOk() { std::vector<std::string> subdirs; - for (std::string& path : subdirs) // expected-warning {{local variable 'subdirs' is later invalidated}} expected-note {{later used here}} - path = std::string(); // expected-note {{local variable 'subdirs' is invalidated here}} + for (std::string& path : subdirs) + path = std::string(); } } // namespace ContainersAsFields @@ -528,11 +523,11 @@ std::string StableString; // FIXME: Distinguish owner-borrow from interior-borrow. struct SinkOwnerBorrow { - std::string *dest_; // expected-note {{this field dangles}} + std::string *dest_; - SinkOwnerBorrow(std::string *dest, int n) : dest_(dest) { // expected-warning {{parameter 'dest' escapes to the field 'dest_' and is later invalidated}} + SinkOwnerBorrow(std::string *dest, int n) : dest_(dest) { if (n > 0) - dest->clear(); // expected-note {{parameter 'dest' is invalidated here}} + dest->clear(); } }; @@ -755,9 +750,6 @@ void FlatMapSubscriptMultipleCallsInvalidate(std::flat_map<int, int> mp, int a, // the second warning on 'mp' itself is redundant and incorrect. // Resolving this requires distinguishing owner-borrow from content-borrow. PrintMax(mp[a], mp[b]); // expected-warning {{parameter 'mp' is later invalidated}} \ - // expected-warning {{parameter 'mp' is later invalidated}} \ - // expected-note {{parameter 'mp' is invalidated here}} \ - // expected-note {{later used here}} \ // expected-note {{parameter 'mp' is invalidated here}} \ // expected-note {{later used here}} } @@ -897,9 +889,9 @@ struct StringOwner { // FIXME: False-positive void member_destructor_invalidates_pointer() { StringOwner owner = {"42", "43"}; - const char *p = owner.s.data(); // expected-warning {{local variable 'owner' is later invalidated}} - owner.t.~basic_string(); // expected-note {{local variable 'owner' is invalidated here}} - (void)*p; // expected-note {{later used here}} + const char *p = owner.s.data(); + owner.t.~basic_string(); + (void)*p; } } // namespace explicit_destructor @@ -937,3 +929,151 @@ void invalid_after_ternary_reset(bool flag) { } } // namespace unique_ptr_invalidation + +namespace InvalidatingFieldsAndInteriors { + +struct SField { int a; int b;}; +void PointerToVectorElementField() { + std::vector<SField> v = {{1, 2}, {3, 4}}; + int* ptr = &v[0].a; // expected-warning {{local variable 'v' is later invalidated}} + v.resize(100); // expected-note {{local variable 'v' is invalidated here}} + *ptr = 10; // expected-note {{later used here}} +} + +void append_call(std::string str) { + std::string_view view = str; // expected-warning {{parameter 'str' is later invalidated}} + str.append("456"); // expected-note {{parameter 'str' is invalidated here}} + (void)view; // expected-note {{later used here}} +} + +void replace_call(std::string str) { + std::string_view view = str; // expected-warning {{parameter 'str' is later invalidated}} + str.replace(0, 1, "456"); // expected-note {{parameter 'str' is invalidated here}} + (void)view; // expected-note {{later used here}} +} + +struct S { + std::vector<std::string> strings1; + std::vector<std::string> strings2; +}; + +void Invalidate1Use1IsInvalid() { + S s; + auto it = s.strings1.begin(); // expected-warning {{local variable 's' is later invalidated}} + s.strings1.push_back("1"); // expected-note {{local variable 's' is 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() { + S s; + auto it1 = s.strings1.begin(); + auto it2 = s.strings2.begin(); // expected-warning {{local variable 's' is later invalidated}} + auto& strings2 = s.strings2; + strings2.push_back("1"); // expected-note {{local variable 's' is invalidated here}} + *it1; + *it2; // expected-note {{later used here}} +} + +void InvalidateBothInASingleExpression(bool cond) { + S s; + auto it1 = s.strings1.begin(); // expected-warning {{local variable 's' is later invalidated}} + auto it2 = s.strings2.begin(); // expected-warning {{local variable 's' is later invalidated}} + auto& both = cond ? s.strings1 : s.strings2; + both.push_back("1"); // expected-note 2 {{local variable 's' is 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; +}; + +void PointerToContainerIsOk() { + std::vector<std::string> s; + std::vector<std::string>* p = &s; + p->push_back("1"); + (void)*p; +} + +void IteratorFromPointerToContainerIsInvalidated() { + std::vector<std::string> s; + std::vector<std::string>* p = &s; // expected-warning {{local variable 's' is later invalidated}} + auto it = p->begin(); + p->push_back("1"); // expected-note {{local variable 's' is invalidated here}} + *it; // expected-note {{later used here}} +} + +void ChangingRegionOwnedByContainerIsOk() { + std::vector<std::string> subdirs; + for (std::string& path : subdirs) + path = std::string(); +} + +} // namespace InvalidatingFieldsAndInteriors + +namespace MapInvalidations { +void MapOperatorBracket() { + std::unordered_map<int, int> m; + auto it = m.begin(); + m[1]; + *it; +} +} // namespace MapInvalidations + +namespace NestedContainers { +// FIXME: Maybe come up with a access path representation to detect this. +void InnerIteratorInvalidated() { + std::vector<std::vector<int>> v; + v.resize(1); + // We cannot differentiate between v.* and v.*.*. + // An annotation system along with lifetimebound could be helpful to describe the paths returned. + auto it = v[0].begin(); + v[0].push_back(1); + *it; +} +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(); + v[1].push_back(1); + *it; +} + +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/test/Sema/LifetimeSafety/lifetime-facts.cpp b/clang/test/Sema/LifetimeSafety/lifetime-facts.cpp index d655bf7391203..028833902aa94 100644 --- a/clang/test/Sema/LifetimeSafety/lifetime-facts.cpp +++ b/clang/test/Sema/LifetimeSafety/lifetime-facts.cpp @@ -23,7 +23,7 @@ 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 (x) // CHECK: Expire (p, Origin: [[O_P]] (Decl: p, Type : MyObj *)) @@ -37,11 +37,11 @@ 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 (obj) } @@ -53,11 +53,11 @@ 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 (trivial_obj) // CHECK-NEXT: End of Block @@ -79,12 +79,12 @@ 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 (s2) // CHECK: Expire (s1) @@ -106,7 +106,7 @@ 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 (s1) } @@ -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,30 +180,30 @@ 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 (y) // CHECK: Expire (x) @@ -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 (x) } diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp b/clang/unittests/Analysis/LifetimeSafetyTest.cpp index 78b7449958140..ddae1d9e0f5cd 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> @@ -136,11 +137,16 @@ class LifetimeTestHelper { } bool isLoanToATemporary(LoanID LID) { - return Analysis.getFactManager() - .getLoanMgr() - .getLoan(LID) - ->getAccessPath() - .getAsMaterializeTemporaryExpr() != nullptr; + const Loan *L = Analysis.getFactManager().getLoanMgr().getLoan(LID); + 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 @@ -283,11 +289,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) { @@ -303,36 +309,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 }; @@ -797,7 +779,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) { @@ -813,12 +795,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) { @@ -833,11 +815,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) { @@ -848,7 +830,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) { @@ -862,10 +844,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) { @@ -892,8 +874,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")); } @@ -913,9 +895,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) { @@ -934,9 +916,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) { @@ -960,8 +942,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) { @@ -977,10 +959,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) { @@ -997,7 +979,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")); } @@ -1029,11 +1011,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) { @@ -1049,10 +1031,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) { @@ -1068,9 +1050,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) { @@ -1097,7 +1079,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) { @@ -1110,7 +1092,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")); } @@ -1145,12 +1127,31 @@ 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")); + // All views have a single interior path (`.*`) because the implementation + // prevents accumulation of multiple `.*` suffixes (see + // DoNotAddMultipleInteriors test). When `Identity(v1)` returns a `MyObj&` + // with loan `a.*`, constructing `View v2` from it would normally add another + // `.*`, but the implementation actively prevents duplication of '.*'. + 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")); +} - EXPECT_THAT(Origin("v4"), HasLoansTo({"c"}, "p3")); +TEST_F(LifetimeAnalysisTest, DoNotAddMultipleInteriors) { + SetupTest(R"( + const MyObj& Identity(View v [[clang::lifetimebound]]); + void target() { + MyObj a; + View v = a; + for (int i = 0; i < 10; ++i) { + const MyObj& b = Identity(v); + v = Identity(b); + POINT(p1); + } + } + )"); + EXPECT_THAT(Origin("v"), HasLoansTo({"a.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, LifetimeboundTemplateFunctionReturnRef) { @@ -1167,7 +1168,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")); } @@ -1191,7 +1192,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")); @@ -1213,6 +1214,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() { @@ -1678,7 +1759,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) { @@ -1696,7 +1777,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) { @@ -1714,7 +1795,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) { @@ -1732,7 +1813,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) { @@ -1751,7 +1832,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) { @@ -1770,7 +1851,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) { @@ -1816,11 +1897,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")); } // ========================================================================= // @@ -1909,7 +1990,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) { diff --git a/llvm/test/CodeGen/AMDGPU/vgpr-spill-placement-issue61083.ll b/llvm/test/CodeGen/AMDGPU/vgpr-spill-placement-issue61083.ll index 991681d4f2889..0fbe77a12b535 100644 --- a/llvm/test/CodeGen/AMDGPU/vgpr-spill-placement-issue61083.ll +++ b/llvm/test/CodeGen/AMDGPU/vgpr-spill-placement-issue61083.ll @@ -91,7 +91,6 @@ bb202: ; preds = %bb194, %bb170, %bb9 ret void } -declare hidden void @__keep_alive() declare i32 @llvm.amdgcn.workitem.id.x() declare align 4 ptr addrspace(4) @llvm.amdgcn.dispatch.ptr() declare void @llvm.assume(i1 noundef) _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
