https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/207520
>From 50f94378216de227962fbb976be413ac6cd5d58f Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <[email protected]> Date: Sat, 4 Jul 2026 16:25:06 +0000 Subject: [PATCH] [LifetimeSafety] Support field-sensitivity in lifetime tracking This patch enables field-sensitivity when tracking lifetimes of nested objects. - FactsGenerator now generates `PathElement::getField` for `MemberExpr` accesses, mapping fields to loans. - LoanPropagation now propagates field paths along flow facts, appending fields to base loans. - Removes false-positive warnings in `invalidations.cpp` where modifications to one field were incorrectly reported as invalidating iterators/pointers to another field. - Adds comprehensive unit tests checking nested field access and placeholder fields. TAG=agy CONV=2cfd8d00-18d7-4a03-8d78-2aba2f9a8f23 --- .../Analysis/Analyses/LifetimeSafety/Facts.h | 24 +++ .../Analyses/LifetimeSafety/LoanPropagation.h | 3 + .../Analysis/Analyses/LifetimeSafety/Loans.h | 29 ++++ clang/lib/Analysis/LifetimeSafety/Dataflow.h | 8 +- clang/lib/Analysis/LifetimeSafety/Facts.cpp | 26 ++-- .../LifetimeSafety/FactsGenerator.cpp | 11 +- .../LifetimeSafety/LoanPropagation.cpp | 146 +++++++++++++----- clang/lib/Analysis/LifetimeSafety/Loans.cpp | 20 +++ .../Sema/LifetimeSafety/invalidations.cpp | 90 ++++++++++- .../unittests/Analysis/LifetimeSafetyTest.cpp | 102 ++++++++---- 10 files changed, 364 insertions(+), 95 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index 94db2a7f311ae..6ff63513d21be 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -48,6 +48,11 @@ class Fact { /// it. Otherwise, the source's loan set is merged into the destination's /// loan set. OriginFlow, + /// Loans held by the origin are projected (their access paths are + /// extended by a path element). + /// Example: if `obj` holds loan `{x}`, `p = obj.field` projects `{x}` with + /// `field` to `{x.field}`before flowing into `p`. + Projection, /// An origin is used (eg. appears as l-value expression like DeclRefExpr). Use, /// An origin that is moved (e.g., passed to an rvalue reference parameter). @@ -160,6 +165,25 @@ class OriginFlowFact : public Fact { const LoanPropagationAnalysis *LPA = nullptr) const override; }; +class ProjectionFact : public Fact { + OriginID OID; + PathElement Element; + +public: + static bool classof(const Fact *F) { + return F->getKind() == Kind::Projection; + } + + ProjectionFact(OriginID OID, PathElement Element) + : Fact(Kind::Projection), OID(OID), Element(Element) {} + + OriginID getOriginID() const { return OID; } + PathElement getPathElement() const { return Element; } + + 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. /// This is the base class for different escape scenarios. class OriginEscapesFact : public Fact { diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LoanPropagation.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LoanPropagation.h index 838daa024c953..d032bdeaa4e72 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LoanPropagation.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LoanPropagation.h @@ -38,6 +38,9 @@ class LoanPropagationAnalysis { LoanSet getLoans(OriginID OID, ProgramPoint P) const; + void dumpLoans(OriginID OID, ProgramPoint P, llvm::raw_ostream &OS, + const LoanManager &LM) const; + /// Builds the chain of origins through which a loan has propagated. /// /// Starting from the last fact of the block containing StartPoint, this diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h index c20af392a705f..d94c940328426 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h @@ -206,6 +206,7 @@ class Loan { /// Manages the creation, storage and retrieval of loans. class LoanManager { + using ExtensionCacheKey = std::pair<LoanID, PathElement>; public: LoanManager() = default; @@ -221,6 +222,14 @@ class LoanManager { 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 ID that was extended to produce ExtendedLoanID, if + /// any. + std::optional<LoanID> getBaseLoan(LoanID ExtendedLoanID) const; + const Loan *getLoan(LoanID ID) const { assert(ID.Value < AllLoans.size()); return AllLoans[ID.Value]; @@ -234,6 +243,14 @@ class LoanManager { 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; + + /// Maps an extended loan ID back to its base loan ID. + llvm::DenseMap<LoanID, LoanID> BaseLoansMap; /// TODO(opt): Profile and evaluate the usefullness of small buffer /// optimisation. @@ -242,4 +259,16 @@ 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/Dataflow.h b/clang/lib/Analysis/LifetimeSafety/Dataflow.h index fc3049c8bec84..c3a60f9b881f9 100644 --- a/clang/lib/Analysis/LifetimeSafety/Dataflow.h +++ b/clang/lib/Analysis/LifetimeSafety/Dataflow.h @@ -60,9 +60,6 @@ class DataflowAnalysis { using Base = DataflowAnalysis<Derived, Lattice, Dir>; private: - const CFG &Cfg; - AnalysisDeclContext &AC; - /// The dataflow state before a basic block is processed. llvm::DenseMap<const CFGBlock *, Lattice> InStates; /// The dataflow state after a basic block is processed. @@ -75,6 +72,8 @@ class DataflowAnalysis { static constexpr bool isForward() { return Dir == Direction::Forward; } protected: + const CFG &Cfg; + AnalysisDeclContext &AC; FactManager &FactMgr; explicit DataflowAnalysis(const CFG &Cfg, AnalysisDeclContext &AC, @@ -170,6 +169,8 @@ class DataflowAnalysis { return D->transfer(In, *F->getAs<ExpireFact>()); case Fact::Kind::OriginFlow: return D->transfer(In, *F->getAs<OriginFlowFact>()); + case Fact::Kind::Projection: + return D->transfer(In, *F->getAs<ProjectionFact>()); case Fact::Kind::MovedOrigin: return D->transfer(In, *F->getAs<MovedOriginFact>()); case Fact::Kind::OriginEscapes: @@ -190,6 +191,7 @@ class DataflowAnalysis { Lattice transfer(Lattice In, const IssueFact &) { return In; } Lattice transfer(Lattice In, const ExpireFact &) { return In; } Lattice transfer(Lattice In, const OriginFlowFact &) { return In; } + Lattice transfer(Lattice In, const ProjectionFact &) { return In; } Lattice transfer(Lattice In, const MovedOriginFact &) { return In; } Lattice transfer(Lattice In, const OriginEscapesFact &) { return In; } Lattice transfer(Lattice In, const UseFact &) { return In; } diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp index ec2d42e10206a..54d5024784fc6 100644 --- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp @@ -47,17 +47,7 @@ void OriginFlowFact::dump(llvm::raw_ostream &OS, const LoanManager &LM, 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 << "}"; - } + LPA->dumpLoans(getDestOriginID(), this, OS, LM); } OS << "\n"; OS << "\tSrc: "; @@ -66,6 +56,20 @@ void OriginFlowFact::dump(llvm::raw_ostream &OS, const LoanManager &LM, OS << "\n"; } +void ProjectionFact::dump(llvm::raw_ostream &OS, const LoanManager &LM, + const OriginManager &OM, + const LoanPropagationAnalysis *LPA) const { + OS << "Projection: \n"; + OS << "\tOrigin: "; + OM.dump(getOriginID(), OS); + if (LPA) { + LPA->dumpLoans(getOriginID(), this, OS, LM); + } + OS << "\n\tElement: "; + getPathElement().dump(OS); + OS << "\n"; +} + void MovedOriginFact::dump(llvm::raw_ostream &OS, const LoanManager &, const OriginManager &OM, const LoanPropagationAnalysis *) const { diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 95872d32adfb6..d07e7857b209a 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -274,17 +274,18 @@ 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)); + Dst->getOuterOriginID(), Src->getOuterOriginID(), /*KillDest=*/true)); + CurrentBlockFacts.push_back(FactMgr.createFact<ProjectionFact>( + Dst->getOuterOriginID(), PathElement::getField(FD))); } } diff --git a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp index 078892bd48c10..4f622d628c19d 100644 --- a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp @@ -59,6 +59,11 @@ static llvm::BitVector computePersistentOrigins(const FactManager &FactMgr, CheckOrigin(OF->getSrcOriginID()); break; } + case Fact::Kind::Projection: { + const auto *PF = F->getAs<ProjectionFact>(); + CheckOrigin(PF->getOriginID()); + break; + } case Fact::Kind::Use: for (const OriginList *Cur = F->getAs<UseFact>()->getUsedOrigins(); Cur; Cur = Cur->peelOuterOrigin()) @@ -189,6 +194,21 @@ class AnalysisImpl return setLoans(In, DestOID, MergedLoans); } + /// A projection projects the loans currently held by the origin in-place. + Lattice transfer(Lattice In, const ProjectionFact &F) { + OriginID OID = F.getOriginID(); + LoanSet Loans = getLoans(In, OID); + LoanSet ProjectedLoans = LoanSetFactory.getEmptySet(); + PathElement Element = F.getPathElement(); + for (LoanID LID : Loans) { + Loan *ExtendedLoan = + FactMgr.getLoanMgr().getOrCreateExtendedLoan(LID, Element); + ProjectedLoans = + LoanSetFactory.add(ProjectedLoans, ExtendedLoan->getID()); + } + return setLoans(In, OID, ProjectedLoans); + } + Lattice transfer(Lattice In, const KillOriginFact &F) { return setLoans(In, F.getKilledOrigin(), LoanSetFactory.getEmptySet()); } @@ -218,11 +238,12 @@ class AnalysisImpl EndBlock = Block; break; } + assert(EndBlock && "Could not find CFGBlock containing StartPoint"); - // Set up DFS traversal state - // SearchState tracks which block we're in and which origin we're tracing + // Set up DFS traversal state. + // SearchState tracks which block we're in and which origin we're tracing. // Each DFSNode maintains its own OriginFlowChain. - using SearchState = std::pair<const CFGBlock *, OriginID>; + using SearchState = std::tuple<const CFGBlock *, OriginID, LoanID>; struct DFSNode { SearchState CurrState; llvm::SmallVector<OriginID> OriginFlowChain; @@ -230,30 +251,32 @@ class AnalysisImpl llvm::SmallVector<DFSNode> PendingStates; llvm::SmallSet<SearchState, 16> VistedStates; - PendingStates.push_back({{EndBlock, StartOID}, {}}); + PendingStates.push_back({{EndBlock, StartOID, TargetLoan}, {}}); // DFS loop to trace loan backwards through CFG while (!PendingStates.empty()) { DFSNode CurrNode = PendingStates.pop_back_val(); - auto [CurrBlock, CurrOID] = CurrNode.CurrState; - - // Trace origins within the current block - const auto [BuildResult, Complete] = - buildOriginFlowChain(CurrBlock, CurrOID, TargetLoan); - if (!BuildResult.empty()) { - CurrNode.OriginFlowChain.append(BuildResult); - CurrOID = BuildResult.back(); - } - - // If we found the IssueFact, we're done - if (Complete) + auto [CurrBlock, CurrOID, CurrLoanID] = CurrNode.CurrState; + + // Trace origins within the current block. + BlockTraceResult TraceResult = + buildOriginFlowChain(CurrBlock, CurrOID, CurrLoanID); + if (!TraceResult.Chain.empty()) + CurrNode.OriginFlowChain.append(TraceResult.Chain); + CurrOID = TraceResult.OutOID; + CurrLoanID = TraceResult.OutLoanID; + + // If we found the IssueFact, we're done. + if (TraceResult.Complete) return CurrNode.OriginFlowChain; // Only explore predecessor blocks where the target loan is present in the // current origin. for (const CFGBlock *PredBlock : CurrBlock->preds()) { - SearchState NextState = {PredBlock, CurrOID}; - if (getLoans(getOutState(PredBlock), CurrOID).contains(TargetLoan) && + if (!PredBlock) + continue; + SearchState NextState = {PredBlock, CurrOID, CurrLoanID}; + if (getLoans(getOutState(PredBlock), CurrOID).contains(CurrLoanID) && VistedStates.insert(NextState).second) PendingStates.push_back({NextState, CurrNode.OriginFlowChain}); } @@ -297,38 +320,69 @@ class AnalysisImpl return LoanSetFactory.getEmptySet(); } + struct BlockTraceResult { + llvm::SmallVector<OriginID> Chain; + bool Complete; + OriginID OutOID; + LoanID OutLoanID; + }; + /// Builds the chain of origins through which a loan has propagated. /// /// This procedure operates strictly within a single Block. Starting from the /// last fact of the Block, it traces backwards through OriginFlowFacts to /// identify the sequence of origins through which the loan flowed. /// - /// Returns (chain, true) if the target loan origin is found during the - /// traversal, otherwise returns (chain, false). - std::pair<llvm::SmallVector<OriginID>, bool> - buildOriginFlowChain(const CFGBlock *Block, const OriginID StartOID, - const LoanID TargetLoan) const { + /// Returns (chain, true, outOID, outLoanID) if the target loan origin is + /// found during the traversal, otherwise returns (chain, false, outOID, + /// outLoanID). + BlockTraceResult buildOriginFlowChain(const CFGBlock *Block, + const OriginID StartOID, + const LoanID StartLoanID) const { OriginID CurrOID = StartOID; + LoanID CurrLoanID = StartLoanID; llvm::SmallVector<OriginID> OriginFlowChain; - for (const Fact *F : llvm::reverse(FactMgr.getFacts(Block))) { - if (const auto *IF = F->getAs<IssueFact>()) - if (IF->getLoanID() == TargetLoan && IF->getOriginID() == CurrOID) - return {OriginFlowChain, true}; - - const auto *OFF = F->getAs<OriginFlowFact>(); - if (!OFF || OFF->getDestOriginID() != CurrOID) - continue; - - const OriginID SrcOriginID = OFF->getSrcOriginID(); - if (!getLoans(SrcOriginID, OFF).contains(TargetLoan)) - continue; + llvm::ArrayRef<const Fact *> Facts = FactMgr.getFacts(Block); + auto GetStateBefore = [&](const Fact *F) -> Lattice { + const auto *It = llvm::find(Facts, F); + assert(It != Facts.end()); + if (It == Facts.begin()) { + auto InState = getInState(Block); + assert(InState); + return *InState; + } + return getState(*(It - 1)); + }; - OriginFlowChain.push_back(SrcOriginID); - CurrOID = SrcOriginID; + for (const Fact *F : llvm::reverse(FactMgr.getFacts(Block))) { + if (const auto *IF = F->getAs<IssueFact>()) { + // Search is complete. + if (IF->getLoanID() == CurrLoanID && IF->getOriginID() == CurrOID) + return {OriginFlowChain, true, CurrOID, CurrLoanID}; + } else if (const auto *OFF = F->getAs<OriginFlowFact>()) { + // Trace the loan back to its source origin if it flowed from there. + if (OFF->getDestOriginID() != CurrOID) + continue; + OriginID SrcOriginID = OFF->getSrcOriginID(); + if (!getLoans(SrcOriginID, OFF).contains(CurrLoanID)) + continue; + CurrOID = SrcOriginID; + OriginFlowChain.push_back(SrcOriginID); + } else if (const auto *PF = F->getAs<ProjectionFact>()) { + // Step back from a projected field loan to its base loan (e.g., from + // 'obj.field' to 'obj'). + if (PF->getOriginID() != CurrOID) + continue; + std::optional<LoanID> BaseLoanID = + FactMgr.getLoanMgr().getBaseLoan(CurrLoanID); + if (BaseLoanID && + getLoans(GetStateBefore(PF), CurrOID).contains(*BaseLoanID)) + CurrLoanID = *BaseLoanID; + } } - return {OriginFlowChain, false}; + return {OriginFlowChain, false, CurrOID, CurrLoanID}; } OriginLoanMap::Factory &OriginLoanMapFactory; @@ -359,6 +413,22 @@ LoanSet LoanPropagationAnalysis::getLoans(OriginID OID, ProgramPoint P) const { return PImpl->getLoans(OID, P); } +void LoanPropagationAnalysis::dumpLoans(OriginID OID, ProgramPoint P, + llvm::raw_ostream &OS, + const LoanManager &LM) const { + LoanSet Loans = getLoans(OID, P); + if (Loans.isEmpty()) + OS << " has no loans"; + else { + OS << " has loans to { "; + for (LoanID LID : Loans) { + LM.getLoan(LID)->getAccessPath().dump(OS); + OS << " "; + } + OS << "}"; + } +} + llvm::SmallVector<OriginID> LoanPropagationAnalysis::buildOriginFlowChain( ProgramPoint StartPoint, const OriginID StartOID, const LoanID TargetLoan, const CFG *Cfg) const { diff --git a/clang/lib/Analysis/LifetimeSafety/Loans.cpp b/clang/lib/Analysis/LifetimeSafety/Loans.cpp index 3c90ed85831ca..ed1fdfe02225b 100644 --- a/clang/lib/Analysis/LifetimeSafety/Loans.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Loans.cpp @@ -64,4 +64,24 @@ LoanManager::getOrCreatePlaceholderBase(const CXXMethodDecl *MD) { PlaceholderBases.InsertNode(NewPB, InsertPos); return NewPB; } + +Loan *LoanManager::getOrCreateExtendedLoan(LoanID BaseLoanID, + PathElement Element) { + ExtensionCacheKey Key = {BaseLoanID, Element}; + auto It = ExtensionCache.find(Key); + if (It != ExtensionCache.end()) + return It->second; + const auto *BaseLoan = getLoan(BaseLoanID); + AccessPath ExtendedPath(BaseLoan->getAccessPath(), Element); + Loan *NewLoan = createLoan(ExtendedPath, BaseLoan->getIssueExpr()); + BaseLoansMap[NewLoan->getID()] = BaseLoanID; + return ExtensionCache[Key] = NewLoan; +} + +std::optional<LoanID> LoanManager::getBaseLoan(LoanID ExtendedLoanID) const { + auto It = BaseLoansMap.find(ExtendedLoanID); + if (It != BaseLoansMap.end()) + return It->second; + return std::nullopt; +} } // namespace clang::lifetimes::internal diff --git a/clang/test/Sema/LifetimeSafety/invalidations.cpp b/clang/test/Sema/LifetimeSafety/invalidations.cpp index be1acc6bc7fbc..1572e78a27e14 100644 --- a/clang/test/Sema/LifetimeSafety/invalidations.cpp +++ b/clang/test/Sema/LifetimeSafety/invalidations.cpp @@ -485,13 +485,12 @@ void ConditionalFieldInvalidatesIterator(bool flag) { (flag ? s.strings1 : s.strings2).push_back("1"); *it; } -// 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"); // OK + *it; } void Invalidate1UseSIsOk() { S s; @@ -894,12 +893,11 @@ struct StringOwner { std::string s, t; }; -// 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(); // OK + (void)*p; } } // namespace explicit_destructor @@ -937,3 +935,79 @@ void invalid_after_ternary_reset(bool flag) { } } // namespace unique_ptr_invalidation + +namespace DeepFieldNesting { +struct Level3 { + std::vector<std::string> vec; + int x; +}; +struct Level2 { + Level3 inner3_1; + Level3 inner3_2; +}; +struct Level1 { + Level2 inner2_1; + Level2 inner2_2; +}; + +// Modifying sibling at Level 3: OK +void SiblingLevel3Ok() { + Level1 obj; + auto it = obj.inner2_1.inner3_1.vec.begin(); + obj.inner2_1.inner3_2.vec.push_back("1"); + *it; +} + +// Modifying sibling at Level 2: OK +void SiblingLevel2Ok() { + Level1 obj; + auto it = obj.inner2_1.inner3_1.vec.begin(); + obj.inner2_2.inner3_1.vec.push_back("1"); + *it; +} + +// Modifying sibling non-container field at Level 3: OK +void SiblingFieldLevel3Ok() { + Level1 obj; + auto it = obj.inner2_1.inner3_1.vec.begin(); + obj.inner2_1.inner3_1.x = 42; + *it; +} + +// Modifying parent structure after use: OK +void ParentModifiedAfterUseOk() { + Level1 obj; + auto it = obj.inner2_1.inner3_1.vec.begin(); + *it; // Use here + Level3 new_val; + obj.inner2_1.inner3_1 = new_val; // OK, because 'it' is no longer used! +} + +// Pointers with sibling modification: OK +void PointerSiblingLevel3Ok(Level1* ptr) { + auto it = ptr->inner2_1.inner3_1.vec.begin(); + ptr->inner2_1.inner3_2.vec.push_back("1"); // OK + *it; +} + +// References with sibling modification: OK +void ReferenceSiblingLevel3Ok(Level1& ref) { + auto it = ref.inner2_1.inner3_1.vec.begin(); + ref.inner2_1.inner3_2.vec.push_back("1"); // OK + *it; +} +} // namespace DeepFieldNesting + +namespace StructFieldDisambiguation { +struct S { + std::vector<int> v; + int x; +}; + +void TestStructVsField(S& s) { + int* px = &s.x; + s.v.push_back(1); // Invalidates s.v.* (interior), but must NOT invalidate s.x + *px = 42; // OK +} +} // namespace StructFieldDisambiguation + diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp b/clang/unittests/Analysis/LifetimeSafetyTest.cpp index 57cf7068affae..5aef3fd7d5c66 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> @@ -143,6 +144,14 @@ class LifetimeTestHelper { .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 // considered live at point P if there is a live origin which contains this // loan. @@ -287,7 +296,7 @@ class OriginsInfo { /// variable expected to be the source 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 +312,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 }; @@ -1213,6 +1198,63 @@ 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, 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, 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() { _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
