Author: Utkarsh Saxena Date: 2026-02-06T14:49:29Z New Revision: 21f3875ffde0ec12bec9e719f5c3fad7adb41667
URL: https://github.com/llvm/llvm-project/commit/21f3875ffde0ec12bec9e719f5c3fad7adb41667 DIFF: https://github.com/llvm/llvm-project/commit/21f3875ffde0ec12bec9e719f5c3fad7adb41667.diff LOG: [LifetimeSafety] Detect use-after-invalidation for STL containers (#179093) Add detection for use-after-invalidation of container references and iterators. This change improves the lifetime safety analysis by detecting a common class of bugs where references or iterators to container elements are used after operations that invalidate them (like `push_back`, `insert`, `resize`, etc.). Example: ```cpp #include <vector> void test() { std::vector<int> v = {1, 2, 3}; int &ref = v[0]; v.push_back(4); // This invalidates references int x = ref; // This should trigger the warning } ``` - Added a new `InvalidateOriginFact` to track when container elements are invalidated - Added a new diagnostic warning for use-after-invalidation scenarios - Implemented detection for container methods that invalidate references/iterators - Added logic to check for uses of references after container invalidation - Added the new warning to the `lifetime-safety-strict` diagnostic group Compile with `-Wlifetime-safety-strict` or `-Wlifetime-safety-invalidation` to see the new warning. Current limitations (not yet detected): * **Field member invalidations** - When invalidation happens on a field member of a struct: ```cpp void Invalidate1Use1IsInvalid() { S s; auto it = s.strings1.begin(); s.strings1.push_back("1"); // Should invalidate but doesn't *it; // Should warn but doesn't } ``` * **Iterator from pointer to container** - When the iterator is obtained through a pointer: ```cpp void IteratorFromPointerToContainerIsInvalidated() { std::vector<std::string> s; std::vector<std::string>* p = &s; auto it = p->begin(); p->push_back("1"); // Should invalidate but doesn't *it; // Should warn but doesn't } ``` We need to distinguish between the loans to `s` and `s.strings` using more granular `Path`. Destruction/Invalidation of a prefix of path destroys/invalidates the complete path. We would also need some mechanism to differentiate the storage of between Containers and the owned buffer. This could be done for `gsl::Owner` and `lifetimebound` member functions. Added: clang/test/Sema/warn-lifetime-safety-invalidations.cpp Modified: clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h clang/include/clang/Basic/DiagnosticGroups.td clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Analysis/LifetimeSafety/Checker.cpp clang/lib/Analysis/LifetimeSafety/Dataflow.h clang/lib/Analysis/LifetimeSafety/Facts.cpp clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp clang/lib/Sema/AnalysisBasedWarnings.cpp clang/test/Sema/Inputs/lifetime-analysis.h Removed: ################################################################################ diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index 0eb5b248fa1f6..f9d55991f2e09 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -51,6 +51,8 @@ class Fact { TestPoint, /// An origin that escapes the function scope (e.g., via return). OriginEscapes, + /// An origin is invalidated (e.g. vector resized). + InvalidateOrigin, }; private: @@ -222,6 +224,29 @@ class UseFact : public Fact { const OriginManager &OM) const override; }; +/// Represents that an origin's storage has been invalidated by a container +/// operation (e.g., vector::push_back may reallocate, invalidating iterators). +/// Created when a container method that may invalidate references/iterators +/// is called on the container. +class InvalidateOriginFact : public Fact { + OriginID OID; + const Expr *InvalidationExpr; + +public: + static bool classof(const Fact *F) { + return F->getKind() == Kind::InvalidateOrigin; + } + + InvalidateOriginFact(OriginID OID, const Expr *InvalidationExpr) + : Fact(Kind::InvalidateOrigin), OID(OID), + InvalidationExpr(InvalidationExpr) {} + + OriginID getInvalidatedOrigin() const { return OID; } + const Expr *getInvalidationExpr() const { return InvalidationExpr; } + void dump(llvm::raw_ostream &OS, const LoanManager &, + const OriginManager &OM) const override; +}; + /// Top-level origin of the expression which was found to be moved, e.g, when /// being used as an argument to an r-value reference parameter. class MovedOriginFact : public Fact { diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h index bffc4d12562bb..fb7d5ad91db79 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h @@ -83,6 +83,11 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> { ArrayRef<const Expr *> Args, bool IsGslConstruction = false); + // Detect container methods that invalidate iterators/references. + // For instance methods, Args[0] is the implicit 'this' pointer. + void handleInvalidatingCall(const Expr *Call, const FunctionDecl *FD, + ArrayRef<const Expr *> Args); + template <typename Destination, typename Source> void flowOrigin(const Destination &D, const Source &S) { flow(getOriginsList(D), getOriginsList(S), /*Kill=*/false); diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h index 3ca7a79d32ee1..ad3a48667cf2a 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h @@ -66,6 +66,10 @@ bool isGslPointerType(QualType QT); // Tells whether the type is annotated with [[gsl::Owner]]. bool isGslOwnerType(QualType QT); +// Returns true if the given method invalidates iterators or references to +// container elements (e.g. vector::push_back). +bool isContainerInvalidationMethod(const CXXMethodDecl &MD); + } // namespace clang::lifetimes #endif // LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMEANNOTATIONS_H diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h index 6e87951c8961d..6148f86091110 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h @@ -20,6 +20,7 @@ #ifndef LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMESAFETY_H #define LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMESAFETY_H +#include "clang/AST/Decl.h" #include "clang/Analysis/Analyses/LifetimeSafety/Facts.h" #include "clang/Analysis/Analyses/LifetimeSafety/LifetimeStats.h" #include "clang/Analysis/Analyses/LifetimeSafety/LiveOrigins.h" @@ -74,6 +75,15 @@ class LifetimeSafetySemaHelper { const Expr *MovedExpr, SourceLocation ExpiryLoc) {} + // Reports when a reference/iterator is used after the container operation + // that invalidated it. + virtual void reportUseAfterInvalidation(const Expr *IssueExpr, + const Expr *UseExpr, + const Expr *InvalidationExpr) {} + virtual void reportUseAfterInvalidation(const ParmVarDecl *PVD, + const Expr *UseExpr, + const Expr *InvalidationExpr) {} + // Suggests lifetime bound annotations for function paramters. virtual void suggestLifetimeboundToParmVar(SuggestionScope Scope, const ParmVarDecl *ParmToAnnotate, diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index f7263e4c8218c..0372cf062ec67 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -547,10 +547,18 @@ def LifetimeSafetyDanglingFieldStrict : DiagGroup<"lifetime-safety-dangling-fiel }]; } + +def LifetimeSafetyInvalidation : DiagGroup<"lifetime-safety-invalidation"> { + code Documentation = [{ + Warning to detect invalidation of references. + }]; +} + def LifetimeSafetyPermissive : DiagGroup<"lifetime-safety-permissive", [LifetimeSafetyDanglingField]>; def LifetimeSafetyStrict : DiagGroup<"lifetime-safety-strict", - [LifetimeSafetyDanglingFieldStrict]>; + [LifetimeSafetyDanglingFieldStrict, + LifetimeSafetyInvalidation]>; def LifetimeSafety : DiagGroup<"lifetime-safety", [LifetimeSafetyPermissive, LifetimeSafetyStrict]> { diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index af96b6cf02195..f999c362307af 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10832,6 +10832,11 @@ def warn_lifetime_safety_return_stack_addr_moved_strict InGroup<LifetimeSafetyStrict>, DefaultIgnore; +def warn_lifetime_safety_invalidation + : Warning<"%select{object whose reference is captured|parameter}0 is later invalidated">, + InGroup<LifetimeSafetyInvalidation>, + DefaultIgnore; + def warn_lifetime_safety_dangling_field : Warning<"address of stack memory escapes to a field">, InGroup<LifetimeSafetyDanglingField>, @@ -10844,6 +10849,7 @@ def warn_lifetime_safety_dangling_field_moved DefaultIgnore; def note_lifetime_safety_used_here : Note<"later used here">; +def note_lifetime_safety_invalidated_here : Note<"invalidated here">; def note_lifetime_safety_destroyed_here : Note<"destroyed here">; def note_lifetime_safety_returned_here : Note<"returned here">; def note_lifetime_safety_moved_here : Note<"potentially moved here">; diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index 59ae665e652a6..78c2a6dba3eb6 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -24,6 +24,7 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/DenseSet.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/TimeProfiler.h" @@ -48,6 +49,7 @@ struct PendingWarning { SourceLocation ExpiryLoc; // Where the loan expired. llvm::PointerUnion<const UseFact *, const OriginEscapesFact *> CausingFact; const Expr *MovedExpr; + const Expr *InvalidatedByExpr; Confidence ConfidenceLevel; }; @@ -80,6 +82,8 @@ class LifetimeChecker { for (const Fact *F : FactMgr.getFacts(B)) if (const auto *EF = F->getAs<ExpireFact>()) checkExpiry(EF); + else if (const auto *IOF = F->getAs<InvalidateOriginFact>()) + checkInvalidation(IOF); else if (const auto *OEF = F->getAs<OriginEscapesFact>()) checkAnnotations(OEF); issuePendingWarnings(); @@ -175,27 +179,90 @@ class LifetimeChecker { FinalWarningsMap[ExpiredLoan] = {/*ExpiryLoc=*/EF->getExpiryLoc(), /*BestCausingFact=*/BestCausingFact, /*MovedExpr=*/MovedExpr, + /*InvalidatedByExpr=*/nullptr, /*ConfidenceLevel=*/CurConfidence}; } + /// Checks for use-after-invalidation errors when a container is modified. + /// + /// This method identifies origins that are live at the point of invalidation + /// and checks if they hold loans that are invalidated by the operation + /// (e.g., iterators into a vector that is being pushed to). + void checkInvalidation(const InvalidateOriginFact *IOF) { + OriginID InvalidatedOrigin = IOF->getInvalidatedOrigin(); + /// Get loans directly pointing to the invalidated container + LoanSet DirectlyInvalidatedLoans = + LoanPropagation.getLoans(InvalidatedOrigin, IOF); + auto IsInvalidated = [&](const Loan *L) { + auto *PathL = dyn_cast<PathLoan>(L); + auto *PlaceholderL = dyn_cast<PlaceholderLoan>(L); + for (LoanID InvalidID : DirectlyInvalidatedLoans) { + const Loan *L = FactMgr.getLoanMgr().getLoan(InvalidID); + auto *InvalidPathL = dyn_cast<PathLoan>(L); + auto *InvalidPlaceholderL = dyn_cast<PlaceholderLoan>(L); + if (PathL && InvalidPathL && + PathL->getAccessPath() == InvalidPathL->getAccessPath()) + return true; + if (PlaceholderL && InvalidPlaceholderL && + PlaceholderL->getParmVarDecl() == + InvalidPlaceholderL->getParmVarDecl()) + return true; + } + return false; + }; + // For each live origin, check if it holds an invalidated loan and report. + LivenessMap Origins = LiveOrigins.getLiveOriginsAt(IOF); + for (auto &[OID, LiveInfo] : Origins) { + LoanSet HeldLoans = LoanPropagation.getLoans(OID, IOF); + for (LoanID LiveLoanID : HeldLoans) + if (IsInvalidated(FactMgr.getLoanMgr().getLoan(LiveLoanID))) { + Confidence CurConfidence = livenessKindToConfidence(LiveInfo.Kind); + Confidence LastConf = + FinalWarningsMap.lookup(LiveLoanID).ConfidenceLevel; + if (LastConf < CurConfidence) { + FinalWarningsMap[LiveLoanID] = { + /*ExpiryLoc=*/{}, + /*CausingFact=*/LiveInfo.CausingFact, + /*MovedExpr=*/nullptr, + /*InvalidatedByExpr=*/IOF->getInvalidationExpr(), + /*ConfidenceLevel=*/CurConfidence}; + } + } + } + } + void issuePendingWarnings() { if (!SemaHelper) return; for (const auto &[LID, Warning] : FinalWarningsMap) { const Loan *L = FactMgr.getLoanMgr().getLoan(LID); - const auto *BL = cast<PathLoan>(L); - const Expr *IssueExpr = BL->getIssueExpr(); + + const Expr *IssueExpr = nullptr; + if (const auto *BL = dyn_cast<PathLoan>(L)) + IssueExpr = BL->getIssueExpr(); + const ParmVarDecl *InvalidatedPVD = nullptr; + if (const auto *PL = dyn_cast<PlaceholderLoan>(L)) + InvalidatedPVD = PL->getParmVarDecl(); llvm::PointerUnion<const UseFact *, const OriginEscapesFact *> CausingFact = Warning.CausingFact; Confidence Confidence = Warning.ConfidenceLevel; const Expr *MovedExpr = Warning.MovedExpr; SourceLocation ExpiryLoc = Warning.ExpiryLoc; - if (const auto *UF = CausingFact.dyn_cast<const UseFact *>()) - SemaHelper->reportUseAfterFree(IssueExpr, UF->getUseExpr(), MovedExpr, - ExpiryLoc, Confidence); - else if (const auto *OEF = - CausingFact.dyn_cast<const OriginEscapesFact *>()) { + if (const auto *UF = CausingFact.dyn_cast<const UseFact *>()) { + if (Warning.InvalidatedByExpr) { + if (IssueExpr) + SemaHelper->reportUseAfterInvalidation(IssueExpr, UF->getUseExpr(), + Warning.InvalidatedByExpr); + if (InvalidatedPVD) + SemaHelper->reportUseAfterInvalidation( + InvalidatedPVD, UF->getUseExpr(), Warning.InvalidatedByExpr); + + } else + SemaHelper->reportUseAfterFree(IssueExpr, UF->getUseExpr(), MovedExpr, + ExpiryLoc, Confidence); + } else if (const auto *OEF = + CausingFact.dyn_cast<const OriginEscapesFact *>()) { if (const auto *RetEscape = dyn_cast<ReturnEscapeFact>(OEF)) SemaHelper->reportUseAfterReturn(IssueExpr, RetEscape->getReturnExpr(), diff --git a/clang/lib/Analysis/LifetimeSafety/Dataflow.h b/clang/lib/Analysis/LifetimeSafety/Dataflow.h index 4de9bbb54af9f..0f64ac8a36ef7 100644 --- a/clang/lib/Analysis/LifetimeSafety/Dataflow.h +++ b/clang/lib/Analysis/LifetimeSafety/Dataflow.h @@ -178,6 +178,8 @@ class DataflowAnalysis { return D->transfer(In, *F->getAs<UseFact>()); case Fact::Kind::TestPoint: return D->transfer(In, *F->getAs<TestPointFact>()); + case Fact::Kind::InvalidateOrigin: + return D->transfer(In, *F->getAs<InvalidateOriginFact>()); } llvm_unreachable("Unknown fact kind"); } @@ -190,6 +192,7 @@ class DataflowAnalysis { Lattice transfer(Lattice In, const OriginEscapesFact &) { return In; } Lattice transfer(Lattice In, const UseFact &) { return In; } Lattice transfer(Lattice In, const TestPointFact &) { return In; } + Lattice transfer(Lattice In, const InvalidateOriginFact &) { return In; } }; } // namespace clang::lifetimes::internal #endif // LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMESAFETY_DATAFLOW_H diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp index e782d6de82ea6..c963d9c45fa9d 100644 --- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp @@ -82,6 +82,13 @@ void UseFact::dump(llvm::raw_ostream &OS, const LoanManager &, OS << ", " << (isWritten() ? "Write" : "Read") << ")\n"; } +void InvalidateOriginFact::dump(llvm::raw_ostream &OS, const LoanManager &, + const OriginManager &OM) const { + OS << "InvalidateOrigin ("; + OM.dump(getInvalidatedOrigin(), OS); + OS << ")\n"; +} + void TestPointFact::dump(llvm::raw_ostream &OS, const LoanManager &, const OriginManager &) const { OS << "TestPoint (Annotation: \"" << getAnnotation() << "\")\n"; diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index bdb48b0c81172..be7886b093bb2 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -10,6 +10,7 @@ #include <string> #include "clang/AST/DeclCXX.h" +#include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/OperationKinds.h" #include "clang/Analysis/Analyses/LifetimeSafety/Facts.h" @@ -542,6 +543,23 @@ void FactsGenerator::handleMovedArgsInCall(const FunctionDecl *FD, } } +void FactsGenerator::handleInvalidatingCall(const Expr *Call, + const FunctionDecl *FD, + ArrayRef<const Expr *> Args) { + const auto *MD = dyn_cast<CXXMethodDecl>(FD); + if (!MD || !MD->isInstance() || !isContainerInvalidationMethod(*MD)) + return; + // Heuristics to turn-down false positives. + auto *DRE = dyn_cast<DeclRefExpr>(Args[0]); + if (!DRE || DRE->getDecl()->getType()->isReferenceType()) + return; + + OriginList *ThisList = getOriginsList(*Args[0]); + if (ThisList) + CurrentBlockFacts.push_back(FactMgr.createFact<InvalidateOriginFact>( + ThisList->getOuterOriginID(), Call)); +} + void FactsGenerator::handleFunctionCall(const Expr *Call, const FunctionDecl *FD, ArrayRef<const Expr *> Args, @@ -551,6 +569,8 @@ void FactsGenerator::handleFunctionCall(const Expr *Call, FD = getDeclWithMergedLifetimeBoundAttrs(FD); if (!FD) return; + + handleInvalidatingCall(Call, FD, Args); handleMovedArgsInCall(FD, Args); if (!CallList) return; diff --git a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp index 385fb2f05ae2a..51196efd3116e 100644 --- a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp @@ -255,4 +255,72 @@ template <typename T> static bool isRecordWithAttr(QualType Type) { bool isGslPointerType(QualType QT) { return isRecordWithAttr<PointerAttr>(QT); } bool isGslOwnerType(QualType QT) { return isRecordWithAttr<OwnerAttr>(QT); } +bool isContainerInvalidationMethod(const CXXMethodDecl &MD) { + const CXXRecordDecl *RD = MD.getParent(); + if (!isInStlNamespace(RD)) + return false; + + StringRef ContainerName; + if (const auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD)) + ContainerName = CTSD->getSpecializedTemplate()->getName(); + else if (RD->getIdentifier()) + ContainerName = RD->getName(); + else + return false; + + static const llvm::StringSet<> Containers = { + // Sequence + "vector", "basic_string", "deque", + // Adaptors + // FIXME: Add queue and stack and check for underlying container (e.g. no + // invalidation for std::list). + "priority_queue", + // Associative + "set", "multiset", "map", "multimap", + // Unordered Associative + "unordered_set", "unordered_multiset", "unordered_map", + "unordered_multimap", + // C++23 Flat + "flat_map", "flat_set", "flat_multimap", "flat_multiset"}; + + if (!Containers.contains(ContainerName)) + return false; + + // Handle Operators via OverloadedOperatorKind + OverloadedOperatorKind OO = MD.getOverloadedOperator(); + if (OO != OO_None) { + switch (OO) { + case OO_Equal: // operator= : Always invalidates (Assignment) + case OO_PlusEqual: // operator+= : Append (String/Vector) + return true; + case OO_Subscript: // operator[] : Invalidation only for Maps + // (Insert-or-access) + { + static const llvm::StringSet<> MapContainers = {"map", "unordered_map", + "flat_map"}; + return MapContainers.contains(ContainerName); + } + default: + return false; + } + } + + if (!MD.getIdentifier()) + return false; + static const llvm::StringSet<> InvalidatingMembers = { + // Basic Insertion/Emplacement + "push_front", "push_back", "emplace_front", "emplace_back", "insert", + "emplace", "push", + // Basic Removal/Clearing + "pop_front", "pop_back", "pop", "erase", "clear", + // Memory Management + "reserve", "resize", "shrink_to_fit", + // Assignment (Named) + "assign", "swap", + // String Specifics + "append", "replace", + // Modern C++ (C++17/23) + "extract", "try_emplace", "insert_range", "append_range", "assign_range"}; + return InvalidatingMembers.contains(MD.getName()); +} } // namespace clang::lifetimes diff --git a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp index 0996d11f0cdeb..8a020eb829be6 100644 --- a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp @@ -67,6 +67,7 @@ static llvm::BitVector computePersistentOrigins(const FactManager &FactMgr, case Fact::Kind::OriginEscapes: case Fact::Kind::Expire: case Fact::Kind::TestPoint: + case Fact::Kind::InvalidateOrigin: break; } } diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 858d5b7f67b58..4f04bc3999a24 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2925,6 +2925,28 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { << DanglingField->getEndLoc(); } + void reportUseAfterInvalidation(const Expr *IssueExpr, const Expr *UseExpr, + const Expr *InvalidationExpr) override { + S.Diag(IssueExpr->getExprLoc(), diag::warn_lifetime_safety_invalidation) + << false << IssueExpr->getSourceRange(); + S.Diag(InvalidationExpr->getExprLoc(), + diag::note_lifetime_safety_invalidated_here) + << InvalidationExpr->getSourceRange(); + S.Diag(UseExpr->getExprLoc(), diag::note_lifetime_safety_used_here) + << UseExpr->getSourceRange(); + } + void reportUseAfterInvalidation(const ParmVarDecl *PVD, const Expr *UseExpr, + const Expr *InvalidationExpr) override { + S.Diag(PVD->getSourceRange().getBegin(), + diag::warn_lifetime_safety_invalidation) + << true << PVD->getSourceRange(); + S.Diag(InvalidationExpr->getExprLoc(), + diag::note_lifetime_safety_invalidated_here) + << InvalidationExpr->getSourceRange(); + S.Diag(UseExpr->getExprLoc(), diag::note_lifetime_safety_used_here) + << UseExpr->getSourceRange(); + } + void suggestLifetimeboundToParmVar(SuggestionScope Scope, const ParmVarDecl *ParmToAnnotate, const Expr *EscapeExpr) override { @@ -3138,6 +3160,8 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings( !Diags.isIgnored( diag::warn_lifetime_safety_return_stack_addr_moved_strict, D->getBeginLoc()) || + !Diags.isIgnored(diag::warn_lifetime_safety_invalidation, + D->getBeginLoc()) || !Diags.isIgnored(diag::warn_lifetime_safety_noescape_escapes, D->getBeginLoc()); bool EnableLifetimeSafetyAnalysis = diff --git a/clang/test/Sema/Inputs/lifetime-analysis.h b/clang/test/Sema/Inputs/lifetime-analysis.h index 987fb012d59c0..38a28e8dcc49c 100644 --- a/clang/test/Sema/Inputs/lifetime-analysis.h +++ b/clang/test/Sema/Inputs/lifetime-analysis.h @@ -7,6 +7,8 @@ struct basic_iterator { T* operator->() const; }; +template<typename T> +bool operator==(basic_iterator<T>, basic_iterator<T>); template<typename T> bool operator!=(basic_iterator<T>, basic_iterator<T>); } @@ -19,6 +21,10 @@ template<typename T> struct remove_reference<T &&> { typedef T type; }; template< class InputIt, class T > 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 ); + template<typename T> typename remove_reference<T>::type &&move(T &&t) noexcept; @@ -27,6 +33,8 @@ auto data(const C &c) -> decltype(c.data()); template <typename C> auto begin(C &c) -> decltype(c.begin()); +template <typename C> +auto end(C &c) -> decltype(c.end()); template<typename T, int N> T *begin(T (&array)[N]); @@ -52,15 +60,29 @@ struct vector { template<typename InputIterator> vector(InputIterator first, InputIterator __last); + T& operator[](unsigned); + T & at(int n) &; T && at(int n) &&; void push_back(const T&); void push_back(T&&); const T& back() const; - void insert(iterator, T&&); + void pop_back(); + iterator insert(iterator, T&&); + void resize(size_t); + void erase(iterator); + void clear(); }; +template<class Key,class T> +struct unordered_map { + T& operator[](const Key& key); +}; + +template<class T> +void swap( T& a, T& b ); + template<typename A, typename B> struct pair { A first; @@ -92,6 +114,9 @@ struct basic_string { basic_string(basic_string<T> &&); basic_string(const T *); ~basic_string(); + basic_string& operator=(const basic_string&); + basic_string& operator+=(const basic_string&); + basic_string& operator+=(const T*); const T *c_str() const; operator basic_string_view<T> () const; using const_iterator = iter<T>; diff --git a/clang/test/Sema/warn-lifetime-safety-invalidations.cpp b/clang/test/Sema/warn-lifetime-safety-invalidations.cpp new file mode 100644 index 0000000000000..c9ce0c35c53d2 --- /dev/null +++ b/clang/test/Sema/warn-lifetime-safety-invalidations.cpp @@ -0,0 +1,344 @@ +// RUN: %clang_cc1 -fsyntax-only -Wlifetime-safety -Wno-dangling -verify %s + +#include "Inputs/lifetime-analysis.h" + +bool Bool(); + +namespace SimpleResize { +void IteratorInvalidAfterResize(int new_size) { + std::vector<int> v; + auto it = std::begin(v); // expected-warning {{object whose reference is captured is later invalidated}} + v.resize(new_size); // expected-note {{invalidated here}} + *it; // expected-note {{later used here}} +} + +void IteratorValidAfterResize(int new_size) { + std::vector<int> v; + auto it = std::begin(v); + v.resize(new_size); + it = std::begin(v); + if (it != std::end(v)) { + *it; // ok + } +} +} // namespace SimpleResize + +namespace CheckModel { +void IteratorValidAfterCheck() { + std::vector<int> v; + auto it = v.begin(); + *it; // ok +} +} // namespace CheckModel + +namespace PointerToContainer { +std::vector<int>* GetContainerPointer(); +void PointerToContainerTest() { + // FIXME: Use opaque loans. + std::vector<int>* v = GetContainerPointer(); + auto it = v->begin(); + *it = 0; // not-ok +} +void PointerToContainerTest(std::vector<int>* v) { + // FIXME: Handle placeholder loans. + auto it = v->begin(); + *it = 0; // not-ok +} +} // namespace PointerToContainer + +namespace InvalidateBeforeSwap { +void InvalidateBeforeSwapIterators(std::vector<int> v1, std::vector<int> v2) { + auto it1 = std::begin(v1); // expected-warning {{object whose reference is captured is later invalidated}} + auto it2 = std::begin(v2); + if (it1 == std::end(v1) || it2 == std::end(v2)) return; + *it1 = 0; // ok + *it2 = 0; // ok + v1.clear(); // expected-note {{invalidated here}} + *it1 = 0; // expected-note {{later used here}} + // FIXME: Handle invalidating functions like std::swap. + std::swap(it1, it2); + *it1 = 0; // ok + *it2 = 0; // not-ok +} + +void InvalidateBeforeSwapContainers(std::vector<int> v1, std::vector<int> v2) { + auto it1 = std::begin(v1); // expected-warning {{object whose reference is captured is later invalidated}} + auto it2 = std::begin(v2); + if (it1 == std::end(v1) || it2 == std::end(v2)) return; + *it1 = 0; // ok + *it2 = 0; // ok + v1.clear(); // expected-note {{invalidated here}} + *it1 = 0; // expected-note {{later used here}} +} +} // namespace InvalidateBeforeSwap + +namespace MergeConditionBasic { +bool A(); +bool B(); +void SameConditionInvalidatesThenValidatesIterator() { + std::vector<int> container; + auto it = container.begin(); // expected-warning {{object whose reference is captured is later invalidated}} + if (it == container.end()) return; + const bool a = A(); + if (a) { + container.clear(); // expected-note {{invalidated here}} + } + if (a) { + it = container.begin(); + if (it == std::end(container)) return; + } + *it = 10; // expected-note {{later used here}} +} +} // namespace MergeConditionBasic + +namespace IteratorWithMultipleContainers { +void MergeWithDifferentContainerValuesIteratorNotInvalidated() { + std::vector<int> v1, v2, v3; + auto it = std::find(v1.begin(), v1.end(), 10); + if (Bool()) { + it = std::find(v2.begin(), v2.end(), 10); + } else { + it = std::find(v3.begin(), v3.end(), 10); + } + v1.clear(); + *it = 20; +} + +void MergeWithDifferentContainerValuesInvalidated() { + std::vector<int> v1, v2, v3; + auto it = std::find(v1.begin(), v1.end(), 10); + if (Bool()) { + it = std::find(v2.begin(), v2.end(), 10); // expected-warning {{object whose reference is captured is later invalidated}} + } else { + it = std::find(v3.begin(), v3.end(), 10); + } + v2.clear(); // expected-note {{invalidated here}} + *it = 20; // expected-note {{later used here}} +} +} // namespace IteratorWithMultipleContainers + +namespace InvalidationInLoops { +void IteratorInvalidationInAForLoop(std::vector<int> v) { + for (auto it = std::begin(v); // expected-warning {{object whose reference is captured is later invalidated}} + it != std::end(v); + ++it) { // expected-note {{later used here}} + if (Bool()) { + v.erase(it); // expected-note {{invalidated here}} + } + } +} + +void IteratorInvalidationInAWhileLoop(std::vector<int> v) { + auto it = std::begin(v); // expected-warning {{object whose reference is captured is later invalidated}} + while (it != std::end(v)) { + if (Bool()) { + v.erase(it); // expected-note {{invalidated here}} + } + ++it; // expected-note {{later used here}} + } +} + +void IteratorInvalidationInAForeachLoop(std::vector<int> v) { + for (int& x : v) { // expected-warning {{object whose reference is captured is later invalidated}} \ + // expected-note {{later used here}} + if (x % 2 == 0) { + v.erase(std::find(v.begin(), v.end(), 1)); // expected-note {{invalidated here}} + } + } +} +} // namespace InvalidationInLoops + +namespace StdVectorPopBack { +void StdVectorPopBackInvalid(std::vector<int> v) { + auto it = v.begin(); // expected-warning {{object whose reference is captured is later invalidated}} + if (it == v.end()) return; + *it; // ok + v.pop_back(); // expected-note {{invalidated here}} + *it; // expected-note {{later used here}} +} +} // namespace StdVectorPopBack + + +namespace SimpleStdFind { +void IteratorCheckedAfterFind(std::vector<int> v) { + auto it = std::find(std::begin(v), std::end(v), 3); + if (it != std::end(v)) { + *it; // ok + } +} + +void IteratorCheckedAfterFindThenErased(std::vector<int> v) { + auto it = std::find(std::begin(v), std::end(v), 3); // expected-warning {{object whose reference is captured is later invalidated}} + if (it != std::end(v)) { + v.erase(it); // expected-note {{invalidated here}} + } + *it; // expected-note {{later used here}} +} +} // namespace SimpleStdFind + +namespace SimpleInsert { +void UseReturnedIteratorAfterInsert(std::vector<int> v) { + auto it = std::begin(v); + it = v.insert(it, 10); + if (it != std::end(v)) { + *it; // ok + } +} + +void UseInvalidIteratorAfterInsert(std::vector<int> v) { + auto it = std::begin(v); // expected-warning {{object whose reference is captured is later invalidated}} + v.insert(it, 10); // expected-note {{invalidated here}} + if (it != std::end(v)) { // expected-note {{later used here}} + *it; + } +} +} // namespace SimpleInsert + +namespace SimpleStdInsert { +void IteratorValidAfterInsert(std::vector<int> v) { + auto it = std::begin(v); + v.insert(it, 0); + it = std::begin(v); + if (it != std::end(v)) { + *it; // ok + } +} + +void IteratorInvalidAfterInsert(std::vector<int> v, int value) { + auto it = std::begin(v); // expected-warning {{object whose reference is captured is later invalidated}} + v.insert(it, 0); // expected-note {{invalidated here}} + *it; // expected-note {{later used here}} +} +} // namespace SimpleStdInsert + +namespace SimpleInvalidIterators { +void IteratorUsedAfterErase(std::vector<int> v) { + auto it = std::begin(v); // expected-warning {{object whose reference is captured is later invalidated}} + for (; it != std::end(v); ++it) { // expected-note {{later used here}} + if (*it > 3) { + v.erase(it); // expected-note {{invalidated here}} + } + } +} + +// FIXME: Detect this. We currently skip invalidation through ref/pointers to containers. +void IteratorUsedAfterPushBackParam(std::vector<int>& v) { + auto it = std::begin(v); + if (it != std::end(v) && *it == 3) { + v.push_back(4); + } + ++it; +} + +void IteratorUsedAfterPushBack(std::vector<int> v) { + auto it = std::begin(v); // expected-warning {{object whose reference is captured is later invalidated}} + if (it != std::end(v) && *it == 3) { + v.push_back(4); // expected-note {{invalidated here}} + } + ++it; // expected-note {{later used here}} +} +} // namespace SimpleInvalidIterators + +namespace ElementReferences { +// Testing raw pointers and references to elements, not just iterators. + +void ReferenceToVectorElement() { + std::vector<int> v = {1, 2, 3}; + int& ref = v[0]; + v.push_back(4); + // FIXME: Detect this as a use of 'ref'. + // https://github.com/llvm/llvm-project/issues/180187 + ref = 10; + (void)ref; +} + +void PointerToVectorElement() { + std::vector<int> v = {1, 2, 3}; + int* ptr = &v[0]; // expected-warning {{object whose reference is captured is later invalidated}} + v.resize(100); // expected-note {{invalidated here}} + *ptr = 10; // expected-note {{later used here}} +} + +void SelfInvalidatingMap() { + std::unordered_map<int, int> mp; + mp[1] = 1; + mp[2] = mp[1]; // FIXME: Detect this. We are mising a UseFact for the assignment params. +} +} // namespace ElementReferences + +namespace Strings { + +void append(std::string str) { + std::string_view view = str; // expected-warning {{object whose reference is captured is later invalidated}} + str += "456"; // expected-note {{invalidated here}} + (void)view; // expected-note {{later used here}} +} +void reassign(std::string str, std::string str2) { + std::string_view view = str; // expected-warning {{object whose reference is captured is later invalidated}} + str = str2; // expected-note {{invalidated here}} + (void)view; // expected-note {{later used here}} +} +} // namespace Strings + +// FIXME: This should be diagnosed as use-after-invalidation but with potential move. +void ReassigningAfterMove(std::string str, std::string str2) { + std::string_view view = str; // expected-warning {{object whose reference is captured is later invalidated}} + std::vector<std::string> someStorage; + someStorage.push_back(std::move(str)); + str = str2; // expected-note {{invalidated here}} + (void)view; // expected-note {{later used here}} +} + +namespace ContainersAsFields { +struct S { + std::vector<std::string> strings1; + std::vector<std::string> strings2; +}; +// FIXME: Make Paths more precise to reason at field granularity. +// Currently we only detect invalidations to direct declarations and not members. +void Invalidate1Use1IsInvalid() { + // FIXME: Detect this. + S s; + auto it = s.strings1.begin(); + s.strings1.push_back("1"); + *it; +} +void Invalidate1Use2IsOk() { + S s; + auto it = s.strings1.begin(); + s.strings2.push_back("1"); + *it; +}void Invalidate1Use2ViaRefIsOk() { + S s; + auto it = s.strings2.begin(); + auto& strings2 = s.strings2; + strings2.push_back("1"); + *it; +} +void Invalidate1UseSIsOk() { + S s; + S* p = &s; + s.strings2.push_back("1"); + (void)*p; +} +void PointerToContainerIsOk() { + std::vector<std::string> s; + std::vector<std::string>* p = &s; + p->push_back("1"); + (void)*p; +} +void IteratorFromPointerToContainerIsInvalidated() { + // FIXME: Detect this. + std::vector<std::string> s; + std::vector<std::string>* p = &s; + auto it = p->begin(); + p->push_back("1"); + *it; +} +void ChangingRegionOwnedByContainerIsOk() { + std::vector<std::string> subdirs; + for (std::string& path : subdirs) + path = std::string(); +} + +} // namespace ContainersAsFields _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
