diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index fdcf317c69cbf..0f848abd913d3 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -102,8 +102,13 @@ class IssueFact : public Fact { const OriginManager &OM) const override; }; +/// 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. class ExpireFact : public Fact { - LoanID LID; + // The access path that expires. + AccessPath AP; + // Expired origin (e.g., its variable goes out of scope). std::optional OID; SourceLocation ExpiryLoc; @@ -111,11 +116,11 @@ class ExpireFact : public Fact { public: static bool classof(const Fact *F) { return F->getKind() == Kind::Expire; } - ExpireFact(LoanID LID, SourceLocation ExpiryLoc, + ExpireFact(AccessPath AP, SourceLocation ExpiryLoc, std::optional OID = std::nullopt) - : Fact(Kind::Expire), LID(LID), OID(OID), ExpiryLoc(ExpiryLoc) {} + : Fact(Kind::Expire), AP(AP), OID(OID), ExpiryLoc(ExpiryLoc) {} - LoanID getLoanID() const { return LID; } + const AccessPath &getAccessPath() const { return AP; } std::optional getOriginID() const { return OID; } SourceLocation getExpiryLoc() const { return ExpiryLoc; } diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h index 9aaf4627ce5ad..aee6bf9eb69c9 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h @@ -28,119 +28,101 @@ inline llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, LoanID ID) { } /// Represents the storage location being borrowed, e.g., a specific stack -/// variable. -/// TODO: Model access paths of other types, e.g., s.field, heap and globals. +/// 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) +/// +/// Placeholder paths never expire within the function scope, as they represent +/// storage from the caller's scope. +/// +/// TODO: Model access paths of other types, e.g. field, array subscript, heap +/// and globals. class AccessPath { - // An access path can be: - // - ValueDecl * , to represent the storage location corresponding to the - // variable declared in ValueDecl. - // - MaterializeTemporaryExpr * , to represent the storage location of the - // temporary object materialized via this MaterializeTemporaryExpr. +public: + enum class Kind : uint8_t { + ValueDecl, + MaterializeTemporary, + PlaceholderParam, + PlaceholderThis + }; + +private: + Kind K; const llvm::PointerUnion - P; + const clang::MaterializeTemporaryExpr *, + const ParmVarDecl *, const CXXMethodDecl *> + Root; public: - AccessPath(const clang::ValueDecl *D) : P(D) {} - AccessPath(const clang::MaterializeTemporaryExpr *MTE) : P(MTE) {} + AccessPath(const clang::ValueDecl *D) : K(Kind::ValueDecl), Root(D) {} + AccessPath(const clang::MaterializeTemporaryExpr *MTE) + : K(Kind::MaterializeTemporary), Root(MTE) {} + static AccessPath Placeholder(const ParmVarDecl *PVD) { + return AccessPath(Kind::PlaceholderParam, PVD); + } + static AccessPath Placeholder(const CXXMethodDecl *MD) { + return AccessPath(Kind::PlaceholderThis, MD); + } + AccessPath(const AccessPath &Other) : K(Other.K), Root(Other.Root) {} + + Kind getKind() const { return K; } const clang::ValueDecl *getAsValueDecl() const { - return P.dyn_cast(); + return K == Kind::ValueDecl ? Root.dyn_cast() + : nullptr; } - const clang::MaterializeTemporaryExpr *getAsMaterializeTemporaryExpr() const { - return P.dyn_cast(); + return K == Kind::MaterializeTemporary + ? Root.dyn_cast() + : nullptr; + } + const ParmVarDecl *getAsPlaceholderParam() const { + return K == Kind::PlaceholderParam ? Root.dyn_cast() + : nullptr; + } + const CXXMethodDecl *getAsPlaceholderThis() const { + return K == Kind::PlaceholderThis ? Root.dyn_cast() + : nullptr; } - bool operator==(const AccessPath &RHS) const { return P == RHS.P; } -}; - -/// An abstract base class for a single "Loan" which represents lending a -/// storage in memory. -class Loan { - /// TODO: Represent opaque loans. - /// TODO: Represent nullptr: loans to no path. Accessing it UB! Currently it - /// is represented as empty LoanSet -public: - enum class Kind : uint8_t { - /// A loan with an access path to a storage location. - Path, - /// A non-expiring placeholder loan for a parameter, representing a borrow - /// from the function's caller. - Placeholder - }; - - Loan(Kind K, LoanID ID) : K(K), ID(ID) {} - virtual ~Loan() = default; - - Kind getKind() const { return K; } - LoanID getID() const { return ID; } - - virtual void dump(llvm::raw_ostream &OS) const = 0; + bool operator==(const AccessPath &RHS) const { + return K == RHS.K && Root == RHS.Root; + } + bool operator!=(const AccessPath &RHS) const { return !(*this == RHS); } + void dump(llvm::raw_ostream &OS) const; private: - const Kind K; - const LoanID ID; -}; - -/// PathLoan represents lending a storage location that is visible within the -/// function's scope (e.g., a local variable on stack). -class PathLoan : public Loan { - AccessPath Path; - /// The expression that creates the loan, e.g., &x. - const Expr *IssueExpr; - -public: - PathLoan(LoanID ID, AccessPath Path, const Expr *IssueExpr) - : Loan(Kind::Path, ID), Path(Path), IssueExpr(IssueExpr) {} - - const AccessPath &getAccessPath() const { return Path; } - const Expr *getIssueExpr() const { return IssueExpr; } - - void dump(llvm::raw_ostream &OS) const override; - - static bool classof(const Loan *L) { return L->getKind() == Kind::Path; } + AccessPath(Kind K, const ParmVarDecl *PVD) : K(K), Root(PVD) {} + AccessPath(Kind K, const CXXMethodDecl *MD) : K(K), Root(MD) {} }; -/// A placeholder loan held by a function parameter or an implicit 'this' -/// object, representing a borrow from the caller's scope. +/// Represents lending a storage location. /// -/// Created at function entry for each pointer or reference parameter or for -/// the implicit 'this' parameter of instance methods, with an -/// origin. Unlike PathLoan, placeholder loans: -/// - Have no IssueExpr (created at function entry, not at a borrow site) -/// - Have no AccessPath (the borrowed object is not visible to the function) -/// - Do not currently expire, but may in the future when modeling function -/// invalidations (e.g., vector::push_back) +/// 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. /// -/// When a placeholder loan escapes the function (e.g., via return), it -/// indicates the parameter or method should be marked [[clang::lifetimebound]], -/// enabling lifetime annotation suggestions. -class PlaceholderLoan : public Loan { - /// The function parameter or method (representing 'this') that holds this - /// placeholder loan. - llvm::PointerUnion ParamOrMethod; +/// Examples: +/// - `int* p = &x;` creates a loan to `x` +/// - 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 + /// loans. + const Expr *IssuingExpr; public: - PlaceholderLoan(LoanID ID, const ParmVarDecl *PVD) - : Loan(Kind::Placeholder, ID), ParamOrMethod(PVD) {} - - PlaceholderLoan(LoanID ID, const CXXMethodDecl *MD) - : Loan(Kind::Placeholder, ID), ParamOrMethod(MD) {} - - const ParmVarDecl *getParmVarDecl() const { - return ParamOrMethod.dyn_cast(); - } - - const CXXMethodDecl *getMethodDecl() const { - return ParamOrMethod.dyn_cast(); - } - - void dump(llvm::raw_ostream &OS) const override; - - static bool classof(const Loan *L) { - return L->getKind() == Kind::Placeholder; - } + Loan(LoanID ID, AccessPath Path, const Expr *IssuingExpr) + : ID(ID), Path(Path), IssuingExpr(IssuingExpr) {} + LoanID getID() const { return ID; } + const AccessPath &getAccessPath() const { return Path; } + const Expr *getIssuingExpr() const { return IssuingExpr; } + void dump(llvm::raw_ostream &OS) const; }; /// Manages the creation, storage and retrieval of loans. @@ -148,15 +130,9 @@ class LoanManager { public: LoanManager() = default; - template - LoanType *createLoan(Args &&...args) { - static_assert( - std::is_same_v || - std::is_same_v, - "createLoan can only be used with PathLoan or PlaceholderLoan"); - void *Mem = LoanAllocator.Allocate(); - auto *NewLoan = - new (Mem) LoanType(getNextLoanID(), std::forward(args)...); + Loan *createLoan(AccessPath Path, const Expr *IssueExpr) { + void *Mem = LoanAllocator.Allocate(); + auto *NewLoan = new (Mem) Loan(getNextLoanID(), Path, IssueExpr); AllLoans.push_back(NewLoan); return NewLoan; } @@ -165,6 +141,7 @@ class LoanManager { assert(ID.Value < AllLoans.size()); return AllLoans[ID.Value]; } + llvm::ArrayRef getLoans() const { return AllLoans; } private: diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index db87f511a230f..36477c6f67b52 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -139,60 +139,45 @@ class LifetimeChecker { }; for (LoanID LID : EscapedLoans) { const Loan *L = FactMgr.getLoanMgr().getLoan(LID); - const auto *PL = dyn_cast(L); - if (!PL) - continue; - if (const auto *PVD = PL->getParmVarDecl()) + const AccessPath &AP = L->getAccessPath(); + if (const auto *PVD = AP.getAsPlaceholderParam()) CheckParam(PVD); - else if (const auto *MD = PL->getMethodDecl()) + else if (const auto *MD = AP.getAsPlaceholderThis()) CheckImplicitThis(MD); } } - /// Checks for use-after-free & use-after-return errors when a loan expires. + /// Checks for use-after-free & use-after-return errors when an access path + /// expires (e.g., a variable goes out of scope). /// - /// This method examines all live origins at the expiry point and determines - /// if any of them hold the expiring loan. If so, it creates a pending - /// warning. - /// - /// Note: This implementation considers only the confidence of origin - /// liveness. Future enhancements could also consider the confidence of loan - /// propagation (e.g., a loan may only be held on some execution paths). + /// When a path expires, all loans having this path expires. + /// This method examines all live origins and reports warnings for loans they + /// hold that are prefixed by the expired path. void checkExpiry(const ExpireFact *EF) { - LoanID ExpiredLoan = EF->getLoanID(); - const Expr *MovedExpr = nullptr; - if (auto *ME = MovedLoans.getMovedLoans(EF).lookup(ExpiredLoan)) - MovedExpr = *ME; - + const AccessPath &ExpiredPath = EF->getAccessPath(); LivenessMap Origins = LiveOrigins.getLiveOriginsAt(EF); - bool CurDomination = false; - // The UseFact or OriginEscapesFact most indicative of a lifetime error, - // prioritized by earlier source location. - llvm::PointerUnion CausingFact = - nullptr; - for (auto &[OID, LiveInfo] : Origins) { LoanSet HeldLoans = LoanPropagation.getLoans(OID, EF); - if (!HeldLoans.contains(ExpiredLoan)) - continue; - // Loan is defaulted. - - if (!CurDomination || causingFactDominatesExpiry(LiveInfo.Kind)) - CausingFact = LiveInfo.CausingFact; + 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 (!CausingFact) - return; - - bool LastDomination = - FinalWarningsMap.lookup(ExpiredLoan).CausingFactDominatesExpiry; - if (LastDomination) - return; - FinalWarningsMap[ExpiredLoan] = { - /*ExpiryLoc=*/EF->getExpiryLoc(), - /*CausingFact=*/CausingFact, - /*MovedExpr=*/MovedExpr, - /*InvalidatedByExpr=*/nullptr, - /*CausingFactDominatesExpiry=*/CurDomination}; } /// Checks for use-after-invalidation errors when a container is modified. @@ -206,18 +191,9 @@ class LifetimeChecker { LoanSet DirectlyInvalidatedLoans = LoanPropagation.getLoans(InvalidatedOrigin, IOF); auto IsInvalidated = [&](const Loan *L) { - auto *PathL = dyn_cast(L); - auto *PlaceholderL = dyn_cast(L); for (LoanID InvalidID : DirectlyInvalidatedLoans) { - const Loan *L = FactMgr.getLoanMgr().getLoan(InvalidID); - auto *InvalidPathL = dyn_cast(L); - auto *InvalidPlaceholderL = dyn_cast(L); - if (PathL && InvalidPathL && - PathL->getAccessPath() == InvalidPathL->getAccessPath()) - return true; - if (PlaceholderL && InvalidPlaceholderL && - PlaceholderL->getParmVarDecl() == - InvalidPlaceholderL->getParmVarDecl()) + const Loan *InvalidL = FactMgr.getLoanMgr().getLoan(InvalidID); + if (InvalidL->getAccessPath() == L->getAccessPath()) return true; } return false; @@ -248,39 +224,41 @@ class LifetimeChecker { return; for (const auto &[LID, Warning] : FinalWarningsMap) { const Loan *L = FactMgr.getLoanMgr().getLoan(LID); - - const Expr *IssueExpr = nullptr; - if (const auto *BL = dyn_cast(L)) - IssueExpr = BL->getIssueExpr(); - const ParmVarDecl *InvalidatedPVD = nullptr; - if (const auto *PL = dyn_cast(L)) - InvalidatedPVD = PL->getParmVarDecl(); + const Expr *IssueExpr = L->getIssuingExpr(); llvm::PointerUnion 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()) { if (Warning.InvalidatedByExpr) { if (IssueExpr) + // Use-after-invalidation of an object on stack. SemaHelper->reportUseAfterInvalidation(IssueExpr, UF->getUseExpr(), Warning.InvalidatedByExpr); - if (InvalidatedPVD) + else if (InvalidatedPVD) + // Use-after-invalidation of a parameter. SemaHelper->reportUseAfterInvalidation( InvalidatedPVD, UF->getUseExpr(), Warning.InvalidatedByExpr); } else + // Scope-based expiry (use-after-scope). SemaHelper->reportUseAfterFree(IssueExpr, UF->getUseExpr(), MovedExpr, ExpiryLoc); } else if (const auto *OEF = CausingFact.dyn_cast()) { if (const auto *RetEscape = dyn_cast(OEF)) + // Return stack address. SemaHelper->reportUseAfterReturn( IssueExpr, RetEscape->getReturnExpr(), MovedExpr, ExpiryLoc); else if (const auto *FieldEscape = dyn_cast(OEF)) + // Dangling field. SemaHelper->reportDanglingField( IssueExpr, FieldEscape->getFieldDecl(), MovedExpr, ExpiryLoc); else if (const auto *GlobalEscape = dyn_cast(OEF)) + // Global escape. SemaHelper->reportDanglingGlobal(IssueExpr, GlobalEscape->getGlobal(), MovedExpr, ExpiryLoc); else diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp index 535da2a014273..1bc0521a72359 100644 --- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp @@ -8,10 +8,7 @@ #include "clang/Analysis/Analyses/LifetimeSafety/Facts.h" #include "clang/AST/Decl.h" -#include "clang/AST/DeclID.h" #include "clang/Analysis/Analyses/PostOrderCFGView.h" -#include "clang/Analysis/FlowSensitive/DataflowWorklist.h" -#include "llvm/ADT/STLFunctionalExtras.h" namespace clang::lifetimes::internal { @@ -32,7 +29,7 @@ void IssueFact::dump(llvm::raw_ostream &OS, const LoanManager &LM, void ExpireFact::dump(llvm::raw_ostream &OS, const LoanManager &LM, const OriginManager &OM) const { OS << "Expire ("; - LM.getLoan(getLoanID())->dump(OS); + getAccessPath().dump(OS); if (auto OID = getOriginID()) { OS << ", Origin: "; OM.dump(*OID, OS); diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 3259505584c9f..b1b47249ef902 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -69,12 +69,11 @@ void FactsGenerator::flow(OriginList *Dst, OriginList *Src, bool Kill) { /// This function should be called whenever a DeclRefExpr represents a borrow. /// \param DRE The declaration reference expression that initiates the borrow. /// \return The new Loan on success, nullptr otherwise. -static const PathLoan *createLoan(FactManager &FactMgr, - const DeclRefExpr *DRE) { +static const Loan *createLoan(FactManager &FactMgr, const DeclRefExpr *DRE) { if (const auto *VD = dyn_cast(DRE->getDecl())) { AccessPath Path(VD); // The loan is created at the location of the DeclRefExpr. - return FactMgr.getLoanMgr().createLoan(Path, DRE); + return FactMgr.getLoanMgr().createLoan(Path, DRE); } return nullptr; } @@ -82,10 +81,10 @@ static const PathLoan *createLoan(FactManager &FactMgr, /// Creates a loan for the storage location of a temporary object. /// \param MTE The MaterializeTemporaryExpr that represents the temporary /// binding. \return The new Loan. -static const PathLoan *createLoan(FactManager &FactMgr, - const MaterializeTemporaryExpr *MTE) { +static const Loan *createLoan(FactManager &FactMgr, + const MaterializeTemporaryExpr *MTE) { AccessPath Path(MTE); - return FactMgr.getLoanMgr().createLoan(Path, MTE); + return FactMgr.getLoanMgr().createLoan(Path, MTE); } void FactsGenerator::run() { @@ -506,38 +505,16 @@ void FactsGenerator::handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds) { if (!escapesViaReturn(OID)) ExpiredOID = OID; } - // Iterate through all loans to see if any expire. - for (const auto *Loan : FactMgr.getLoanMgr().getLoans()) { - if (const auto *BL = dyn_cast(Loan)) { - // Check if the loan is for a stack variable and if that variable - // is the one being destructed. - const AccessPath AP = BL->getAccessPath(); - const ValueDecl *Path = AP.getAsValueDecl(); - if (Path == LifetimeEndsVD) - CurrentBlockFacts.push_back(FactMgr.createFact( - BL->getID(), LifetimeEnds.getTriggerStmt()->getEndLoc(), - ExpiredOID)); - } - } + CurrentBlockFacts.push_back(FactMgr.createFact( + AccessPath(LifetimeEndsVD), LifetimeEnds.getTriggerStmt()->getEndLoc(), + ExpiredOID)); } void FactsGenerator::handleFullExprCleanup( const CFGFullExprCleanup &FullExprCleanup) { - // Iterate through all loans to see if any expire. - for (const auto *Loan : FactMgr.getLoanMgr().getLoans()) { - if (const auto *PL = dyn_cast(Loan)) { - // Check if the loan is for a temporary materialization and if that - // storage location is the one being destructed. - const AccessPath &AP = PL->getAccessPath(); - const MaterializeTemporaryExpr *Path = AP.getAsMaterializeTemporaryExpr(); - if (!Path) - continue; - if (llvm::is_contained(FullExprCleanup.getExpiringMTEs(), Path)) { - CurrentBlockFacts.push_back( - FactMgr.createFact(PL->getID(), Path->getEndLoc())); - } - } - } + for (const auto *MTE : FullExprCleanup.getExpiringMTEs()) + CurrentBlockFacts.push_back( + FactMgr.createFact(AccessPath(MTE), MTE->getEndLoc())); } void FactsGenerator::handleExitBlock() { @@ -784,8 +761,9 @@ llvm::SmallVector FactsGenerator::issuePlaceholderLoans() { llvm::SmallVector PlaceholderLoanFacts; if (auto ThisOrigins = FactMgr.getOriginMgr().getThisOrigins()) { OriginList *List = *ThisOrigins; - const PlaceholderLoan *L = FactMgr.getLoanMgr().createLoan( - cast(FD)); + const Loan *L = FactMgr.getLoanMgr().createLoan( + AccessPath::Placeholder(cast(FD)), + /*IssuingExpr=*/nullptr); PlaceholderLoanFacts.push_back( FactMgr.createFact(L->getID(), List->getOuterOriginID())); } @@ -793,8 +771,8 @@ llvm::SmallVector FactsGenerator::issuePlaceholderLoans() { OriginList *List = getOriginsList(*PVD); if (!List) continue; - const PlaceholderLoan *L = - FactMgr.getLoanMgr().createLoan(PVD); + const Loan *L = FactMgr.getLoanMgr().createLoan( + AccessPath::Placeholder(PVD), /*IssuingExpr=*/nullptr); PlaceholderLoanFacts.push_back( FactMgr.createFact(L->getID(), List->getOuterOriginID())); } diff --git a/clang/lib/Analysis/LifetimeSafety/Loans.cpp b/clang/lib/Analysis/LifetimeSafety/Loans.cpp index 8a2cd2a39322b..336331b8f5a27 100644 --- a/clang/lib/Analysis/LifetimeSafety/Loans.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Loans.cpp @@ -10,21 +10,30 @@ namespace clang::lifetimes::internal { -void PathLoan::dump(llvm::raw_ostream &OS) const { - OS << getID() << " (Path: "; - if (const clang::ValueDecl *VD = Path.getAsValueDecl()) - OS << VD->getNameAsString(); - else if (const clang::MaterializeTemporaryExpr *MTE = - Path.getAsMaterializeTemporaryExpr()) - // No nice "name" for the temporary, so deferring to LLVM default - OS << "MaterializeTemporaryExpr at " << MTE; - else - llvm_unreachable("access path is not one of any supported types"); - OS << ")"; +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()) + OS << "$" << PVD->getNameAsString(); + break; + case Kind::PlaceholderThis: + OS << "$this"; + break; + } } -void PlaceholderLoan::dump(llvm::raw_ostream &OS) const { - OS << getID() << " (Placeholder loan)"; +void Loan::dump(llvm::raw_ostream &OS) const { + OS << getID() << " (Path: "; + Path.dump(OS); + OS << ")"; } - } // namespace clang::lifetimes::internal diff --git a/clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp b/clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp index 95de08d4425b0..138704821024a 100644 --- a/clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp +++ b/clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp @@ -80,10 +80,7 @@ class AnalysisImpl auto IsInvalidated = [&](const AccessPath &Path) { for (LoanID LID : ImmediatelyMovedLoans) { const Loan *MovedLoan = LoanMgr.getLoan(LID); - auto *PL = dyn_cast(MovedLoan); - if (!PL) - continue; - if (PL->getAccessPath() == Path) + if (MovedLoan->getAccessPath() == Path) return true; } return false; @@ -91,10 +88,7 @@ class AnalysisImpl for (auto [O, _] : LiveOrigins.getLiveOriginsAt(&F)) for (LoanID LiveLoan : LoanPropagation.getLoans(O, &F)) { const Loan *LiveLoanPtr = LoanMgr.getLoan(LiveLoan); - auto *PL = dyn_cast(LiveLoanPtr); - if (!PL) - continue; - if (IsInvalidated(PL->getAccessPath())) + if (IsInvalidated(LiveLoanPtr->getAccessPath())) MovedLoans = MovedLoansMapFactory.add(MovedLoans, LiveLoan, F.getMoveExpr()); } diff --git a/clang/test/Sema/warn-lifetime-safety-dataflow.cpp b/clang/test/Sema/warn-lifetime-safety-dataflow.cpp index be1b77b36a2e0..2cfec824b0866 100644 --- a/clang/test/Sema/warn-lifetime-safety-dataflow.cpp +++ b/clang/test/Sema/warn-lifetime-safety-dataflow.cpp @@ -25,8 +25,8 @@ MyObj* return_local_addr() { // CHECK: OriginFlow: // CHECK-NEXT: Dest: [[O_RET_VAL:[0-9]+]] (Expr: ImplicitCastExpr, Type : MyObj *) // CHECK-NEXT: Src: [[O_P]] (Decl: p, Type : MyObj *) -// CHECK: Expire ([[L_X]] (Path: x)) -// CHECK: Expire ({{[0-9]+}} (Path: p), Origin: [[O_P]] (Decl: p, Type : MyObj *)) +// CHECK: Expire (x) +// CHECK: Expire (p, Origin: [[O_P]] (Decl: p, Type : MyObj *)) // CHECK: OriginEscapes ([[O_RET_VAL]] (Expr: ImplicitCastExpr, Type : MyObj *), via Return) } @@ -43,7 +43,7 @@ void loan_expires_cpp() { // CHECK: OriginFlow: // CHECK-NEXT: Dest: {{[0-9]+}} (Decl: pObj, Type : MyObj *) // CHECK-NEXT: Src: [[O_ADDR_OBJ]] (Expr: UnaryOperator, Type : MyObj *) -// CHECK: Expire ([[L_OBJ]] (Path: obj)) +// CHECK: Expire (obj) } @@ -59,7 +59,7 @@ void loan_expires_trivial() { // CHECK: OriginFlow: // CHECK-NEXT: Dest: {{[0-9]+}} (Decl: pTrivialObj, Type : int *) // CHECK-NEXT: Src: [[O_ADDR_TRIVIAL_OBJ]] (Expr: UnaryOperator, Type : int *) -// CHECK: Expire ([[L_TRIVIAL_OBJ]] (Path: trivial_obj)) +// CHECK: Expire (trivial_obj) // CHECK-NEXT: End of Block } @@ -86,8 +86,8 @@ void overwrite_origin() { // CHECK: OriginFlow: // CHECK-NEXT: Dest: [[O_P]] (Decl: p, Type : MyObj *) // CHECK-NEXT: Src: [[O_ADDR_S2]] (Expr: UnaryOperator, Type : MyObj *) -// CHECK: Expire ([[L_S2]] (Path: s2)) -// CHECK: Expire ([[L_S1]] (Path: s1)) +// CHECK: Expire (s2) +// CHECK: Expire (s1) } // CHECK-LABEL: Function: reassign_to_null @@ -108,7 +108,7 @@ void reassign_to_null() { // CHECK: OriginFlow: // CHECK-NEXT: Dest: [[O_P]] (Decl: p, Type : MyObj *) // CHECK-NEXT: Src: {{[0-9]+}} (Expr: ImplicitCastExpr, Type : MyObj *) -// CHECK: Expire ([[L_S1]] (Path: s1)) +// CHECK: Expire (s1) } // FIXME: Have a better representation for nullptr than just an empty origin. // It should be a separate loan and origin kind. @@ -205,8 +205,8 @@ void test_use_lifetimebound_call() { // CHECK: OriginFlow: // CHECK-NEXT: Dest: {{[0-9]+}} (Decl: r, Type : MyObj *) // CHECK-NEXT: Src: [[O_CALL_EXPR]] (Expr: CallExpr, Type : MyObj *) -// CHECK: Expire ([[L_Y]] (Path: y)) -// CHECK: Expire ([[L_X]] (Path: x)) +// CHECK: Expire (y) +// CHECK: Expire (x) } // CHECK-LABEL: Function: test_reference_variable @@ -235,5 +235,5 @@ void test_reference_variable() { // CHECK: OriginFlow: // CHECK-NEXT: Dest: {{[0-9]+}} (Decl: p, Type : const MyObj *) // CHECK-NEXT: Src: {{[0-9]+}} (Expr: UnaryOperator, Type : const MyObj *) -// CHECK: Expire ([[L_X]] (Path: x)) +// CHECK: Expire (x) } diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp index bd09bb70e9a11..cbc4548c3d934 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -328,9 +328,9 @@ void multiple_expiry_of_same_loan(bool cond) { if (cond) { p = &unsafe; // expected-warning {{does not live long enough}} if (cond) - break; + break; // expected-note {{destroyed here}} } - } // expected-note {{destroyed here}} + } (void)*p; // expected-note {{later used here}} p = &safe; @@ -349,8 +349,8 @@ void multiple_expiry_of_same_loan(bool cond) { if (cond) p = &unsafe; // expected-warning {{does not live long enough}} if (cond) - break; - } // expected-note {{destroyed here}} + break; // expected-note {{destroyed here}} + } (void)*p; // expected-note {{later used here}} } @@ -717,12 +717,12 @@ View uar_before_uaf(const MyObj& safe, bool c) { View p; { MyObj local_obj; - p = local_obj; // expected-warning {{object whose reference is captured does not live long enough}} + p = local_obj; // expected-warning {{ddress of stack memory is returned later}} if (c) { - return p; + return p; // expected-note {{returned here}} } - } // expected-note {{destroyed here}} - p.use(); // expected-note {{later used here}} + } + p.use(); p = safe; return p; } diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp b/clang/unittests/Analysis/LifetimeSafetyTest.cpp index 2116f7736c4be..6cf65dd64ef83 100644 --- a/clang/unittests/Analysis/LifetimeSafetyTest.cpp +++ b/clang/unittests/Analysis/LifetimeSafetyTest.cpp @@ -125,9 +125,8 @@ class LifetimeTestHelper { } std::vector LID; for (const Loan *L : Analysis.getFactManager().getLoanMgr().getLoans()) - if (const auto *BL = dyn_cast(L)) - if (BL->getAccessPath().getAsValueDecl() == VD) - LID.push_back(L->getID()); + if (L->getAccessPath().getAsValueDecl() == VD) + LID.push_back(L->getID()); if (LID.empty()) { ADD_FAILURE() << "Loan for '" << VarName << "' not found."; return {}; @@ -136,11 +135,11 @@ class LifetimeTestHelper { } bool isLoanToATemporary(LoanID LID) { - const Loan *L = Analysis.getFactManager().getLoanMgr().getLoan(LID); - if (const auto *BL = dyn_cast(L)) { - return BL->getAccessPath().getAsMaterializeTemporaryExpr() != nullptr; - } - return false; + return Analysis.getFactManager() + .getLoanMgr() + .getLoan(LID) + ->getAccessPath() + .getAsMaterializeTemporaryExpr() != nullptr; } // Gets the set of loans that are live at the given program point. A loan is @@ -169,9 +168,10 @@ class LifetimeTestHelper { const ExpireFact * getExpireFactFromAllFacts(const llvm::ArrayRef &FactsInBlock, const LoanID &loanID) { + const Loan *L = Analysis.getFactManager().getLoanMgr().getLoan(loanID); for (const Fact *F : FactsInBlock) { if (auto const *CurrentEF = F->getAs()) - if (CurrentEF->getLoanID() == loanID) + if (CurrentEF->getAccessPath() == L->getAccessPath()) return CurrentEF; } return nullptr;