diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index 94db2a7f311ae..da214fa64c947 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -20,6 +20,7 @@ #include "clang/Analysis/Analyses/LifetimeSafety/Utils.h" #include "clang/Analysis/AnalysisDeclContext.h" #include "clang/Analysis/CFG.h" +#include "llvm/ADT/BitVector.h" #include "llvm/ADT/STLFunctionalExtras.h" #include "llvm/ADT/SmallVector.h" #include "llvm/Support/Debug.h" @@ -389,6 +390,14 @@ class FactManager { unsigned getNumFacts() const { return NextFactID.Value; } + /// Returns a bit vector, indexed by origin ID, marking the origins that are + /// referenced from more than one basic block. Only those need to survive + /// block boundaries in the dataflow analyses; the rest are block-local. + /// + /// Computed on first use and shared by every analysis, so that they all + /// agree on which origins cross boundaries. + const llvm::BitVector &getPersistentOrigins(const CFG &Cfg); + LoanManager &getLoanMgr() { return LoanMgr; } const LoanManager &getLoanMgr() const { return LoanMgr; } OriginManager &getOriginMgr() { return OriginMgr; } @@ -401,6 +410,7 @@ class FactManager { /// Facts for each CFG block, indexed by block ID. llvm::SmallVector> BlockToFacts; llvm::BumpPtrAllocator FactAllocator; + std::optional PersistentOrigins; }; } // namespace clang::lifetimes::internal diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LiveOrigins.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LiveOrigins.h index 8f998fa83fbf5..d6b5bf74c0f54 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LiveOrigins.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LiveOrigins.h @@ -75,6 +75,19 @@ struct LivenessInfo { using LivenessMap = utils::MapTy; +/// The origins that are live at a program point. +/// +/// Origins confined to a single basic block are tracked separately from those +/// referenced by more than one, so that only the latter take part in the joins +/// at block boundaries. Both halves are live, so consumers must visit both: +/// +/// for (const LivenessMap &Live : {Origins.Persistent, Origins.BlockLocal}) +/// for (auto &[OID, Info] : Live) +struct LiveOriginSet { + LivenessMap Persistent; + LivenessMap BlockLocal; +}; + class LiveOriginsAnalysis { public: LiveOriginsAnalysis(const CFG &C, AnalysisDeclContext &AC, FactManager &F, @@ -83,7 +96,7 @@ class LiveOriginsAnalysis { /// Returns the set of origins that are live at a specific program point, /// along with the the details of the liveness. - LivenessMap getLiveOriginsAt(ProgramPoint P) const; + LiveOriginSet getLiveOriginsAt(ProgramPoint P) const; // Dump liveness values on all test points in the program. void dump(llvm::raw_ostream &OS, diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index 155c6072a33a5..cf101b1d9e046 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -185,29 +185,30 @@ class LifetimeChecker { /// hold that are prefixed by the expired path. void checkExpiry(const ExpireFact *EF) { const AccessPath &ExpiredPath = EF->getAccessPath(); - LivenessMap Origins = LiveOrigins.getLiveOriginsAt(EF); - for (auto &[OID, LiveInfo] : Origins) { - LoanSet HeldLoans = LoanPropagation.getLoans(OID, EF); - for (LoanID HeldLoanID : HeldLoans) { - const Loan *HeldLoan = FactMgr.getLoanMgr().getLoan(HeldLoanID); - if (!ExpiredPath.isPrefixOf(HeldLoan->getAccessPath())) - continue; - // HeldLoan is expired because its base or itself is expired. - PendingWarning &CurWarning = FinalWarningsMap[HeldLoan->getID()]; - const Expr *MovedExpr = nullptr; - if (auto *ME = MovedLoans.getMovedLoans(EF).lookup(HeldLoanID)) - MovedExpr = *ME; - // Skip if we already have a dominating causing fact. - if (CurWarning.CausingFactDominatesExpiry) - continue; - if (causingFactDominatesExpiry(LiveInfo.Kind)) - CurWarning.CausingFactDominatesExpiry = true; - CurWarning.CausingFact = LiveInfo.CausingFact; - CurWarning.ExpiryLoc = EF->getExpiryLoc(); - CurWarning.MovedExpr = MovedExpr; - CurWarning.InvalidatedByExpr = nullptr; + LiveOriginSet Origins = LiveOrigins.getLiveOriginsAt(EF); + for (const LivenessMap &Live : {Origins.Persistent, Origins.BlockLocal}) + for (auto &[OID, LiveInfo] : Live) { + LoanSet HeldLoans = LoanPropagation.getLoans(OID, EF); + for (LoanID HeldLoanID : HeldLoans) { + const Loan *HeldLoan = FactMgr.getLoanMgr().getLoan(HeldLoanID); + if (!ExpiredPath.isPrefixOf(HeldLoan->getAccessPath())) + continue; + // HeldLoan is expired because its base or itself is expired. + PendingWarning &CurWarning = FinalWarningsMap[HeldLoan->getID()]; + const Expr *MovedExpr = nullptr; + if (auto *ME = MovedLoans.getMovedLoans(EF).lookup(HeldLoanID)) + MovedExpr = *ME; + // Skip if we already have a dominating causing fact. + if (CurWarning.CausingFactDominatesExpiry) + continue; + if (causingFactDominatesExpiry(LiveInfo.Kind)) + CurWarning.CausingFactDominatesExpiry = true; + CurWarning.CausingFact = LiveInfo.CausingFact; + CurWarning.ExpiryLoc = EF->getExpiryLoc(); + CurWarning.MovedExpr = MovedExpr; + CurWarning.InvalidatedByExpr = nullptr; + } } - } } /// Checks for use-after-invalidation errors when a container is modified. @@ -231,24 +232,25 @@ class LifetimeChecker { 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))) { - bool CurDomination = causingFactDominatesExpiry(LiveInfo.Kind); - bool LastDomination = - FinalWarningsMap.lookup(LiveLoanID).CausingFactDominatesExpiry; - if (!LastDomination) { - FinalWarningsMap[LiveLoanID] = { - /*ExpiryLoc=*/{}, - /*CausingFact=*/LiveInfo.CausingFact, - /*MovedExpr=*/nullptr, - /*InvalidatedByExpr=*/IOF->getInvalidationExpr(), - /*CausingFactDominatesExpiry=*/CurDomination}; + LiveOriginSet Origins = LiveOrigins.getLiveOriginsAt(IOF); + for (const LivenessMap &Live : {Origins.Persistent, Origins.BlockLocal}) + for (auto &[OID, LiveInfo] : Live) { + LoanSet HeldLoans = LoanPropagation.getLoans(OID, IOF); + for (LoanID LiveLoanID : HeldLoans) + if (IsInvalidated(FactMgr.getLoanMgr().getLoan(LiveLoanID))) { + bool CurDomination = causingFactDominatesExpiry(LiveInfo.Kind); + bool LastDomination = + FinalWarningsMap.lookup(LiveLoanID).CausingFactDominatesExpiry; + if (!LastDomination) { + FinalWarningsMap[LiveLoanID] = { + /*ExpiryLoc=*/{}, + /*CausingFact=*/LiveInfo.CausingFact, + /*MovedExpr=*/nullptr, + /*InvalidatedByExpr=*/IOF->getInvalidationExpr(), + /*CausingFactDominatesExpiry=*/CurDomination}; + } } - } - } + } } void issuePendingWarnings() { diff --git a/clang/lib/Analysis/LifetimeSafety/Dataflow.h b/clang/lib/Analysis/LifetimeSafety/Dataflow.h index fc3049c8bec84..aaa2c32c400c3 100644 --- a/clang/lib/Analysis/LifetimeSafety/Dataflow.h +++ b/clang/lib/Analysis/LifetimeSafety/Dataflow.h @@ -47,6 +47,9 @@ using ProgramPoint = const Fact *; /// lifetime-relevant `Fact` transforms the lattice state. Only overloads /// for facts relevant to the analysis need to be implemented. /// +/// It may additionally override `Lattice exitBlock(Lattice);` to drop state +/// that is not visible outside the block it was computed in. +/// /// \tparam Derived The CRTP derived class that implements the specific /// analysis. /// \tparam LatticeType The dataflow lattice used by the analysis. @@ -157,7 +160,7 @@ class DataflowAnalysis { State = transferFact(State, F); } } - return State; + return static_cast(this)->exitBlock(State); } Lattice transferFact(Lattice In, const Fact *F) { @@ -187,6 +190,8 @@ class DataflowAnalysis { } public: + Lattice exitBlock(Lattice In) { return In; } + Lattice transfer(Lattice In, const IssueFact &) { return In; } Lattice transfer(Lattice In, const ExpireFact &) { return In; } Lattice transfer(Lattice In, const OriginFlowFact &) { return In; } diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp index ec2d42e10206a..19776332af7dc 100644 --- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp @@ -10,9 +10,69 @@ #include "clang/AST/Decl.h" #include "clang/Analysis/Analyses/LifetimeSafety/LoanPropagation.h" #include "clang/Analysis/Analyses/PostOrderCFGView.h" +#include "llvm/Support/TimeProfiler.h" namespace clang::lifetimes::internal { +const llvm::BitVector &FactManager::getPersistentOrigins(const CFG &Cfg) { + if (PersistentOrigins) + return *PersistentOrigins; + llvm::TimeTraceScope TimeProfile("ComputePersistentOrigins"); + + unsigned NumOrigins = OriginMgr.getNumOrigins(); + PersistentOrigins.emplace(NumOrigins); + llvm::SmallVector OriginToFirstSeenBlock(NumOrigins, + nullptr); + for (const CFGBlock *B : Cfg) { + for (const Fact *F : getFacts(B)) { + auto CheckOrigin = [&](OriginID OID) { + if (PersistentOrigins->test(OID.Value)) + return; + auto &FirstSeenBlock = OriginToFirstSeenBlock[OID.Value]; + if (FirstSeenBlock == nullptr) + FirstSeenBlock = B; + if (FirstSeenBlock != B) { + // We saw this origin in more than one block. + PersistentOrigins->set(OID.Value); + } + }; + + switch (F->getKind()) { + case Fact::Kind::Issue: + CheckOrigin(F->getAs()->getOriginID()); + break; + case Fact::Kind::OriginFlow: { + const auto *OF = F->getAs(); + CheckOrigin(OF->getDestOriginID()); + CheckOrigin(OF->getSrcOriginID()); + break; + } + case Fact::Kind::Use: + for (const OriginList *Cur = F->getAs()->getUsedOrigins(); Cur; + Cur = Cur->peelOuterOrigin()) + CheckOrigin(Cur->getOuterOriginID()); + break; + case Fact::Kind::KillOrigin: + CheckOrigin(F->getAs()->getKilledOrigin()); + break; + case Fact::Kind::OriginEscapes: + // An escaping origin is read at the exit block but defined earlier, so + // it spans blocks and must participate in joins. + CheckOrigin(F->getAs()->getEscapedOriginID()); + break; + // `Expire` and `InvalidateOrigin` only ever clear an origin, so + // misclassifying one is harmless: the clear becomes a no-op. + case Fact::Kind::MovedOrigin: + case Fact::Kind::Expire: + case Fact::Kind::TestPoint: + case Fact::Kind::InvalidateOrigin: + break; + } + } + } + return *PersistentOrigins; +} + void Fact::dump(llvm::raw_ostream &OS, const LoanManager &, const OriginManager &, const LoanPropagationAnalysis *) const { OS << "Fact (Kind: " << static_cast(K) << ")\n"; diff --git a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp index 69b903c813555..eb4dc38f7f408 100644 --- a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp @@ -18,39 +18,44 @@ namespace { /// It tracks which origins are live, why they're live (which UseFact), /// and the confidence level of that liveness. struct Lattice { - LivenessMap LiveOrigins; + /// Origins referenced from more than one block. Participates in joins. + LivenessMap Persistent; + /// Origins confined to a single block. Discarded at block boundaries. + LivenessMap BlockLocal; - Lattice() : LiveOrigins(nullptr) {}; + Lattice() : Persistent(nullptr), BlockLocal(nullptr) {}; - explicit Lattice(LivenessMap L) : LiveOrigins(L) {} + Lattice(LivenessMap Persistent, LivenessMap BlockLocal) + : Persistent(Persistent), BlockLocal(BlockLocal) {} bool operator==(const Lattice &Other) const { - return LiveOrigins == Other.LiveOrigins; + return Persistent == Other.Persistent && BlockLocal == Other.BlockLocal; } bool operator!=(const Lattice &Other) const { return !(*this == Other); } void dump(llvm::raw_ostream &OS, const OriginManager &OM) const { - if (LiveOrigins.isEmpty()) + if (Persistent.isEmpty() && BlockLocal.isEmpty()) OS << " \n"; - for (const auto &Entry : LiveOrigins) { - OriginID OID = Entry.first; - const LivenessInfo &Info = Entry.second; - OS << " "; - OM.dump(OID, OS); - OS << " is "; - switch (Info.Kind) { - case LivenessKind::Must: - OS << "definitely"; - break; - case LivenessKind::Maybe: - OS << "maybe"; - break; - case LivenessKind::Dead: - llvm_unreachable("liveness kind of live origins should not be dead."); + for (const LivenessMap &Live : {Persistent, BlockLocal}) + for (const auto &Entry : Live) { + OriginID OID = Entry.first; + const LivenessInfo &Info = Entry.second; + OS << " "; + OM.dump(OID, OS); + OS << " is "; + switch (Info.Kind) { + case LivenessKind::Must: + OS << "definitely"; + break; + case LivenessKind::Maybe: + OS << "maybe"; + break; + case LivenessKind::Dead: + llvm_unreachable("liveness kind of live origins should not be dead."); + } + OS << " live at this point\n"; } - OS << " live at this point\n"; - } } }; @@ -77,18 +82,34 @@ class AnalysisImpl public: AnalysisImpl(const CFG &C, AnalysisDeclContext &AC, FactManager &F, LivenessMap::Factory &SF) - : DataflowAnalysis(C, AC, F), FactMgr(F), Factory(SF) {} + : DataflowAnalysis(C, AC, F), FactMgr(F), Factory(SF), + PersistentOrigins(F.getPersistentOrigins(C)) {} using DataflowAnalysis::transfer; StringRef getAnalysisName() const { return "LiveOrigins"; } - Lattice getInitialState() { return Lattice(Factory.getEmptyMap()); } + Lattice getInitialState() { + return Lattice(Factory.getEmptyMap(), Factory.getEmptyMap()); + } + + /// An origin referenced from a single block need not be live outside it. + /// + /// Loans only ever enter an origin through an `IssueFact` or an + /// `OriginFlowFact` naming it as the destination, and both would make the + /// origin persistent if they lived in another block. So a block-local origin + /// holds no loans anywhere outside its own block, and every consumer of + /// liveness intersects it with the origin's loans. Dropping these here keeps + /// them out of the joins and out of the block-entry state comparison. + Lattice exitBlock(Lattice L) { + return Lattice(L.Persistent, Factory.getEmptyMap()); + } /// Merges two lattices by combining liveness information. /// When the same origin has different confidence levels, we take the lower /// one. Lattice join(Lattice L1, Lattice L2) const { - LivenessMap Merged = L1.LiveOrigins; + assert(L1.BlockLocal.isEmpty() && L2.BlockLocal.isEmpty() && + "block-local origins must not reach a block boundary"); // Take the earliest Fact to make the join hermetic and commutative. auto CombineCausingFact = [](CausingFactType A, CausingFactType B) -> CausingFactType { @@ -117,11 +138,12 @@ class AnalysisImpl return LivenessInfo(CombineCausingFact(L1->CausingFact, L2->CausingFact), CombineLivenessKind(L1->Kind, L2->Kind)); }; - return Lattice(utils::join( - L1.LiveOrigins, L2.LiveOrigins, Factory, CombineLivenessInfo, - // A symmetric join is required here. If an origin is live on one - // branch but not the other, its confidence must be demoted to `Maybe`. - utils::JoinKind::Symmetric)); + // A symmetric join is required here. If an origin is live on one branch but + // not the other, its confidence must be demoted to `Maybe`. + LivenessMap Joined = + utils::join(L1.Persistent, L2.Persistent, Factory, CombineLivenessInfo, + utils::JoinKind::Symmetric); + return Lattice(Joined, Factory.getEmptyMap()); } /// A read operation makes the origin live with definite confidence, as it @@ -133,14 +155,12 @@ class AnalysisImpl Cur = Cur->peelOuterOrigin()) { OriginID OID = Cur->getOuterOriginID(); // Write kills liveness. - if (UF.isWritten()) { - Out = Lattice(Factory.remove(Out.LiveOrigins, OID)); - } else { + if (UF.isWritten()) + Out = removeLive(Out, OID); + else // Read makes origin live with definite confidence (dominates this // point). - Out = Lattice(Factory.add(Out.LiveOrigins, OID, - LivenessInfo(&UF, LivenessKind::Must))); - } + Out = addLive(Out, OID, LivenessInfo(&UF, LivenessKind::Must)); } return Out; } @@ -148,14 +168,13 @@ class AnalysisImpl /// An escaping origin (e.g., via return) makes the origin live with definite /// confidence, as it dominates this program point. Lattice transfer(Lattice In, const OriginEscapesFact &OEF) { - OriginID OID = OEF.getEscapedOriginID(); - return Lattice(Factory.add(In.LiveOrigins, OID, - LivenessInfo(&OEF, LivenessKind::Must))); + return addLive(In, OEF.getEscapedOriginID(), + LivenessInfo(&OEF, LivenessKind::Must)); } /// Issuing a new loan to an origin kills its liveness. Lattice transfer(Lattice In, const IssueFact &IF) { - return Lattice(Factory.remove(In.LiveOrigins, IF.getOriginID())); + return removeLive(In, IF.getOriginID()); } /// An OriginFlow kills the liveness of the destination origin if `KillDest` @@ -163,32 +182,29 @@ class AnalysisImpl Lattice transfer(Lattice In, const OriginFlowFact &OF) { Lattice Out = In; OriginID Dest = OF.getDestOriginID(); - OriginID Src = OF.getSrcOriginID(); // If the destination of the flow is live, the source of the flow must also // be marked live before this point as its value will flow into the // destination. - if (In.LiveOrigins.contains(Dest)) { - const LivenessInfo *DestInfo = In.LiveOrigins.lookup(Dest); - assert(DestInfo); - Out = Lattice(Factory.add(Out.LiveOrigins, Src, *DestInfo)); - } + if (const LivenessInfo *DestInfo = lookupLive(In, Dest)) + Out = addLive(Out, OF.getSrcOriginID(), *DestInfo); if (OF.getKillDest()) - Out = Lattice(Factory.remove(Out.LiveOrigins, Dest)); + Out = removeLive(Out, Dest); return Out; } Lattice transfer(Lattice In, const KillOriginFact &F) { - return Lattice(Factory.remove(In.LiveOrigins, F.getKilledOrigin())); + return removeLive(In, F.getKilledOrigin()); } Lattice transfer(Lattice In, const ExpireFact &F) { if (auto OID = F.getOriginID()) - return Lattice(Factory.remove(In.LiveOrigins, *OID)); + return removeLive(In, *OID); return In; } - LivenessMap getLiveOriginsAt(ProgramPoint P) const { - return getState(P).LiveOrigins; + LiveOriginSet getLiveOriginsAt(ProgramPoint P) const { + Lattice L = getState(P); + return LiveOriginSet{L.Persistent, L.BlockLocal}; } // Dump liveness values on all test points in the program. @@ -204,8 +220,33 @@ class AnalysisImpl } private: + /// Routes an origin to the half of the lattice it belongs to. + bool isPersistent(OriginID OID) const { + return PersistentOrigins.test(OID.Value); + } + + Lattice addLive(Lattice L, OriginID OID, LivenessInfo Info) { + if (isPersistent(OID)) + return Lattice(Factory.add(L.Persistent, OID, Info), L.BlockLocal); + return Lattice(L.Persistent, Factory.add(L.BlockLocal, OID, Info)); + } + + Lattice removeLive(Lattice L, OriginID OID) { + if (isPersistent(OID)) + return Lattice(Factory.remove(L.Persistent, OID), L.BlockLocal); + return Lattice(L.Persistent, Factory.remove(L.BlockLocal, OID)); + } + + const LivenessInfo *lookupLive(const Lattice &L, OriginID OID) const { + return isPersistent(OID) ? L.Persistent.lookup(OID) + : L.BlockLocal.lookup(OID); + } + FactManager &FactMgr; LivenessMap::Factory &Factory; + /// Origins referenced from more than one basic block; see + /// `FactManager::getPersistentOrigins`. + const llvm::BitVector &PersistentOrigins; }; } // namespace @@ -223,7 +264,7 @@ LiveOriginsAnalysis::LiveOriginsAnalysis(const CFG &C, AnalysisDeclContext &AC, LiveOriginsAnalysis::~LiveOriginsAnalysis() = default; -LivenessMap LiveOriginsAnalysis::getLiveOriginsAt(ProgramPoint P) const { +LiveOriginSet LiveOriginsAnalysis::getLiveOriginsAt(ProgramPoint P) const { return PImpl->getLiveOriginsAt(P); } diff --git a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp index 078892bd48c10..ac1e0f47cbd79 100644 --- a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp @@ -20,69 +20,10 @@ #include "llvm/ADT/BitVector.h" #include "llvm/ADT/SmallSet.h" #include "llvm/ADT/SmallVector.h" -#include "llvm/Support/TimeProfiler.h" #include "llvm/Support/raw_ostream.h" namespace clang::lifetimes::internal { -// Prepass to find persistent origins. An origin is persistent if it is -// referenced in more than one basic block. -static llvm::BitVector computePersistentOrigins(const FactManager &FactMgr, - const CFG &C) { - llvm::TimeTraceScope("ComputePersistentOrigins"); - unsigned NumOrigins = FactMgr.getOriginMgr().getNumOrigins(); - llvm::BitVector PersistentOrigins(NumOrigins); - - llvm::SmallVector OriginToFirstSeenBlock(NumOrigins, - nullptr); - for (const CFGBlock *B : C) { - for (const Fact *F : FactMgr.getFacts(B)) { - auto CheckOrigin = [&](OriginID OID) { - if (PersistentOrigins.test(OID.Value)) - return; - auto &FirstSeenBlock = OriginToFirstSeenBlock[OID.Value]; - if (FirstSeenBlock == nullptr) - FirstSeenBlock = B; - if (FirstSeenBlock != B) { - // We saw this origin in more than one block. - PersistentOrigins.set(OID.Value); - } - }; - - switch (F->getKind()) { - case Fact::Kind::Issue: - CheckOrigin(F->getAs()->getOriginID()); - break; - case Fact::Kind::OriginFlow: { - const auto *OF = F->getAs(); - CheckOrigin(OF->getDestOriginID()); - CheckOrigin(OF->getSrcOriginID()); - break; - } - case Fact::Kind::Use: - for (const OriginList *Cur = F->getAs()->getUsedOrigins(); Cur; - Cur = Cur->peelOuterOrigin()) - CheckOrigin(Cur->getOuterOriginID()); - break; - case Fact::Kind::KillOrigin: - CheckOrigin(F->getAs()->getKilledOrigin()); - break; - case Fact::Kind::OriginEscapes: - // An escaping origin is read at the exit block but defined earlier, so - // it spans blocks and must participate in joins. - CheckOrigin(F->getAs()->getEscapedOriginID()); - break; - case Fact::Kind::MovedOrigin: - case Fact::Kind::Expire: - case Fact::Kind::TestPoint: - case Fact::Kind::InvalidateOrigin: - break; - } - } - } - return PersistentOrigins; -} - namespace { /// Represents the dataflow lattice for loan propagation. @@ -139,7 +80,7 @@ class AnalysisImpl LoanSet::Factory &LoanSetFactory) : DataflowAnalysis(C, AC, F), OriginLoanMapFactory(OriginLoanMapFactory), LoanSetFactory(LoanSetFactory), - PersistentOrigins(computePersistentOrigins(F, C)) {} + PersistentOrigins(F.getPersistentOrigins(C)) {} using Base::transfer; @@ -148,8 +89,9 @@ class AnalysisImpl Lattice getInitialState() { return Lattice{}; } /// Merges two lattices by taking the union of loans for each origin. - /// Only persistent origins are joined; block-local origins are discarded. Lattice join(Lattice A, Lattice B) { + assert(A.BlockLocalOrigins.isEmpty() && B.BlockLocalOrigins.isEmpty() && + "block-local origins must not reach a block boundary"); OriginLoanMap JoinedOrigins = utils::join( A.PersistentOrigins, B.PersistentOrigins, OriginLoanMapFactory, [&](const LoanSet *S1, const LoanSet *S2) { @@ -166,6 +108,14 @@ class AnalysisImpl return Lattice(JoinedOrigins, OriginLoanMapFactory.getEmptyMap()); } + /// Block-local origins are not referenced outside the block that computed + /// them, so they are dropped here rather than propagated to adjacent blocks. + /// Dropping them at the boundary (instead of in `join`) also covers edges + /// where `join` is never called, such as blocks with a single predecessor. + Lattice exitBlock(Lattice L) { + return Lattice(L.PersistentOrigins, OriginLoanMapFactory.getEmptyMap()); + } + /// A new loan is issued to the origin. Old loans are erased. Lattice transfer(Lattice In, const IssueFact &F) { OriginID OID = F.getOriginID(); @@ -336,7 +286,7 @@ class AnalysisImpl /// Boolean vector indexed by origin ID. If true, the origin appears in /// multiple basic blocks and must participate in join operations. If false, /// the origin is block-local and can be discarded at block boundaries. - llvm::BitVector PersistentOrigins; + const llvm::BitVector &PersistentOrigins; }; } // namespace diff --git a/clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp b/clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp index 1a9ce3e576733..edddc6069c8c6 100644 --- a/clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp +++ b/clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp @@ -85,13 +85,15 @@ class AnalysisImpl } return false; }; - for (auto [O, _] : LiveOrigins.getLiveOriginsAt(&F)) - for (LoanID LiveLoan : LoanPropagation.getLoans(O, &F)) { - const Loan *LiveLoanPtr = LoanMgr.getLoan(LiveLoan); - if (IsInvalidated(LiveLoanPtr->getAccessPath())) - MovedLoans = - MovedLoansMapFactory.add(MovedLoans, LiveLoan, F.getMoveExpr()); - } + LiveOriginSet Origins = LiveOrigins.getLiveOriginsAt(&F); + for (const LivenessMap &Live : {Origins.Persistent, Origins.BlockLocal}) + for (auto [O, _] : Live) + for (LoanID LiveLoan : LoanPropagation.getLoans(O, &F)) { + const Loan *LiveLoanPtr = LoanMgr.getLoan(LiveLoan); + if (IsInvalidated(LiveLoanPtr->getAccessPath())) + MovedLoans = + MovedLoansMapFactory.add(MovedLoans, LiveLoan, F.getMoveExpr()); + } return Lattice(MovedLoans); } diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp b/clang/unittests/Analysis/LifetimeSafetyTest.cpp index 57cf7068affae..1686e88e740c5 100644 --- a/clang/unittests/Analysis/LifetimeSafetyTest.cpp +++ b/clang/unittests/Analysis/LifetimeSafetyTest.cpp @@ -150,15 +150,17 @@ class LifetimeTestHelper { const auto &LiveOriginsAnalysis = Runner.getAnalysis().getLiveOrigins(); const auto &LoanPropagation = Runner.getAnalysis().getLoanPropagation(); - LivenessMap LiveOriginsMap = LiveOriginsAnalysis.getLiveOriginsAt(P); + LiveOriginSet LiveOrigins = LiveOriginsAnalysis.getLiveOriginsAt(P); LoanSet::Factory F; LoanSet Result = F.getEmptySet(); - for (const auto &[OID, LI] : LiveOriginsMap) { - LoanSet Loans = LoanPropagation.getLoans(OID, P); - Result = clang::lifetimes::internal::utils::join(Result, Loans, F); - } + for (const LivenessMap &Live : + {LiveOrigins.Persistent, LiveOrigins.BlockLocal}) + for (const auto &[OID, LI] : Live) { + LoanSet Loans = LoanPropagation.getLoans(OID, P); + Result = clang::lifetimes::internal::utils::join(Result, Loans, F); + } if (Result.isEmpty()) return std::nullopt; @@ -192,8 +194,11 @@ class LifetimeTestHelper { if (!PP) return std::nullopt; std::vector> Result; - for (auto &[OID, Info] : Analysis.getLiveOrigins().getLiveOriginsAt(PP)) - Result.push_back({OID, Info.Kind}); + LiveOriginSet LiveOrigins = Analysis.getLiveOrigins().getLiveOriginsAt(PP); + for (const LivenessMap &Live : + {LiveOrigins.Persistent, LiveOrigins.BlockLocal}) + for (auto &[OID, Info] : Live) + Result.push_back({OID, Info.Kind}); return Result; }