llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Alex Wang (aeft) <details> <summary>Changes</summary> When a pointer and its pointee die in the same loop scope, the back-edge makes the pointer appear live at the pointee's `ExpireFact`, causing a spurious use-after-scope warning. `ExpireOriginFact` is emitted at `LifetimeEnds` to clear the origin's loans and liveness, preventing the checker from reporting against an origin whose variable is already dead. Fixes: #<!-- -->169548 --- Full diff: https://github.com/llvm/llvm-project/pull/182368.diff 8 Files Affected: - (modified) clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h (+19) - (modified) clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h (+2) - (modified) clang/lib/Analysis/LifetimeSafety/Dataflow.h (+3) - (modified) clang/lib/Analysis/LifetimeSafety/Facts.cpp (+7) - (modified) clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp (+19) - (modified) clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp (+5) - (modified) clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp (+6) - (modified) clang/test/Sema/warn-lifetime-safety.cpp (+46) ``````````diff diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index f9d55991f2e09..2d1ed3fde0b12 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -53,6 +53,8 @@ class Fact { OriginEscapes, /// An origin is invalidated (e.g. vector resized). InvalidateOrigin, + /// An origin expires (e.g., its variable goes out of scope). + ExpireOrigin, }; private: @@ -247,6 +249,23 @@ class InvalidateOriginFact : public Fact { const OriginManager &OM) const override; }; +/// Emitted when a variable's lifetime ends. Transfer functions use this to +/// clear the origin's loans and liveness, preventing false positives when +/// a pointer and its pointee die in the same scope inside a loop. +class ExpireOriginFact : public Fact { + OriginID OID; + +public: + static bool classof(const Fact *F) { + return F->getKind() == Kind::ExpireOrigin; + } + + ExpireOriginFact(OriginID OID) : Fact(Kind::ExpireOrigin), OID(OID) {} + OriginID getExpiredOriginID() const { return OID; } + 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 fb7d5ad91db79..f495a532dcbc9 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h @@ -109,6 +109,8 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> { void markUseAsWrite(const DeclRefExpr *DRE); + bool isEscapingOrigin(OriginID OID) const; + llvm::SmallVector<Fact *> issuePlaceholderLoans(); FactManager &FactMgr; AnalysisDeclContext &AC; diff --git a/clang/lib/Analysis/LifetimeSafety/Dataflow.h b/clang/lib/Analysis/LifetimeSafety/Dataflow.h index 0f64ac8a36ef7..edeb13392f319 100644 --- a/clang/lib/Analysis/LifetimeSafety/Dataflow.h +++ b/clang/lib/Analysis/LifetimeSafety/Dataflow.h @@ -180,6 +180,8 @@ class DataflowAnalysis { return D->transfer(In, *F->getAs<TestPointFact>()); case Fact::Kind::InvalidateOrigin: return D->transfer(In, *F->getAs<InvalidateOriginFact>()); + case Fact::Kind::ExpireOrigin: + return D->transfer(In, *F->getAs<ExpireOriginFact>()); } llvm_unreachable("Unknown fact kind"); } @@ -193,6 +195,7 @@ class DataflowAnalysis { Lattice transfer(Lattice In, const UseFact &) { return In; } Lattice transfer(Lattice In, const TestPointFact &) { return In; } Lattice transfer(Lattice In, const InvalidateOriginFact &) { return In; } + Lattice transfer(Lattice In, const ExpireOriginFact &) { 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 c963d9c45fa9d..131aef8b78b9a 100644 --- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp @@ -89,6 +89,13 @@ void InvalidateOriginFact::dump(llvm::raw_ostream &OS, const LoanManager &, OS << ")\n"; } +void ExpireOriginFact::dump(llvm::raw_ostream &OS, const LoanManager &, + const OriginManager &OM) const { + OS << "ExpireOrigin ("; + OM.dump(getExpiredOriginID(), 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 0b25a6a401e08..58a30ff5c0918 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -437,6 +437,14 @@ void FactsGenerator::VisitMaterializeTemporaryExpr( } } +bool FactsGenerator::isEscapingOrigin(OriginID OID) const { + return llvm::any_of(EscapesInCurrentBlock, [OID](const Fact *F) { + if (const auto *EF = F->getAs<OriginEscapesFact>()) + return EF->getEscapedOriginID() == OID; + return false; + }); +} + void FactsGenerator::handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds) { /// TODO: Handle loans to temporaries. const VarDecl *LifetimeEndsVD = LifetimeEnds.getVarDecl(); @@ -454,6 +462,17 @@ void FactsGenerator::handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds) { BL->getID(), LifetimeEnds.getTriggerStmt()->getEndLoc())); } } + // In loops, the back-edge can make a dead origin appear live at its + // pointee's ExpireFact. Expiring the origin here prevents that. + if (OriginList *List = getOriginsList(*LifetimeEndsVD)) { + for (OriginList *L = List; L; L = L->peelOuterOrigin()) { + OriginID OID = L->getOuterOriginID(); + // Skip if this origin escapes. Its loans are still needed + // for the escape checker. + if (!isEscapingOrigin(OID)) + CurrentBlockFacts.push_back(FactMgr.createFact<ExpireOriginFact>(OID)); + } + } } void FactsGenerator::handleTemporaryDtor( diff --git a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp index f210fb4d752d4..29b083d51c05b 100644 --- a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp @@ -151,6 +151,11 @@ class AnalysisImpl LivenessInfo(&OEF, LivenessKind::Must))); } + /// Prevents a dead origin's liveness from leaking through loop back-edges. + Lattice transfer(Lattice In, const ExpireOriginFact &F) { + return Lattice(Factory.remove(In.LiveOrigins, F.getExpiredOriginID())); + } + /// 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())); diff --git a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp index 8a020eb829be6..a92845972bce2 100644 --- a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp @@ -68,6 +68,7 @@ static llvm::BitVector computePersistentOrigins(const FactManager &FactMgr, case Fact::Kind::Expire: case Fact::Kind::TestPoint: case Fact::Kind::InvalidateOrigin: + case Fact::Kind::ExpireOrigin: break; } } @@ -181,6 +182,11 @@ class AnalysisImpl return setLoans(In, DestOID, MergedLoans); } + /// Prevents a dead origin from holding stale loans past its lifetime. + Lattice transfer(Lattice In, const ExpireOriginFact &F) { + return setLoans(In, F.getExpiredOriginID(), LoanSetFactory.getEmptySet()); + } + LoanSet getLoans(OriginID OID, ProgramPoint P) const { return getLoans(getState(P), OID); } diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp index 8f52ff27bc6fd..2f9a08409e372 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -1745,3 +1745,49 @@ View test3(std::string a) { return b; // expected-note {{returned here}} } } // namespace non_trivial_views + +namespace LoopLocalPointers { + +void conditional_assignment_in_loop() { + for (int i = 0; i < 10; ++i) { + MyObj obj; + MyObj* view; + if (i > 5) { + view = &obj; + } + (void)*view; + } +} + +void unconditional_assignment_in_loop() { + for (int i = 0; i < 10; ++i) { + MyObj obj; + MyObj* view = &obj; + (void)*view; + } +} + +void multi_level_pointer_in_loop() { + for (int i = 0; i < 10; ++i) { + MyObj obj; + MyObj* p; + MyObj** pp; + if (i > 5) { + p = &obj; + pp = &p; + } + (void)**pp; + } +} + +void outer_pointer_outlives_inner_pointee() { + MyObj safe; + MyObj* view = &safe; + for (int i = 0; i < 10; ++i) { + MyObj obj; + view = &obj; // expected-warning {{may not live long enough}} + } // expected-note {{destroyed here}} + (void)*view; // expected-note {{later used here}} +} + +} // namespace LoopLocalPointers `````````` </details> https://github.com/llvm/llvm-project/pull/182368 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
