https://github.com/aeft updated https://github.com/llvm/llvm-project/pull/182368
>From 71cc34ae5f68886ef9e70a81abfe62d8d93a2450 Mon Sep 17 00:00:00 2001 From: Alex Wang <[email protected]> Date: Fri, 13 Mar 2026 00:01:21 -0700 Subject: [PATCH] [LifetimeSafety] Fix false positives for pointers in loops --- .../Analysis/Analyses/LifetimeSafety/Facts.h | 11 +++-- .../Analyses/LifetimeSafety/FactsGenerator.h | 2 + clang/lib/Analysis/LifetimeSafety/Facts.cpp | 6 ++- .../LifetimeSafety/FactsGenerator.cpp | 21 +++++++- .../Analysis/LifetimeSafety/LiveOrigins.cpp | 6 +++ .../LifetimeSafety/LoanPropagation.cpp | 6 +++ clang/test/Sema/warn-lifetime-safety.cpp | 48 +++++++++++++++++++ 7 files changed, 95 insertions(+), 5 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index 42a71fb5a50d2..fdcf317c69cbf 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -24,6 +24,7 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/Support/Debug.h" #include <cstdint> +#include <optional> namespace clang::lifetimes::internal { @@ -103,19 +104,23 @@ class IssueFact : public Fact { class ExpireFact : public Fact { LoanID LID; + // Expired origin (e.g., its variable goes out of scope). + std::optional<OriginID> OID; SourceLocation ExpiryLoc; public: static bool classof(const Fact *F) { return F->getKind() == Kind::Expire; } - ExpireFact(LoanID LID, SourceLocation ExpiryLoc) - : Fact(Kind::Expire), LID(LID), ExpiryLoc(ExpiryLoc) {} + ExpireFact(LoanID LID, SourceLocation ExpiryLoc, + std::optional<OriginID> OID = std::nullopt) + : Fact(Kind::Expire), LID(LID), OID(OID), ExpiryLoc(ExpiryLoc) {} LoanID getLoanID() const { return LID; } + std::optional<OriginID> getOriginID() const { return OID; } SourceLocation getExpiryLoc() const { return ExpiryLoc; } void dump(llvm::raw_ostream &OS, const LoanManager &LM, - const OriginManager &) const override; + const OriginManager &OM) const override; }; class OriginFlowFact : public Fact { diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h index ddaa69719b666..4bb2a4198ebcf 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h @@ -112,6 +112,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/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp index 4ffc8b4195949..535da2a014273 100644 --- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp @@ -30,9 +30,13 @@ void IssueFact::dump(llvm::raw_ostream &OS, const LoanManager &LM, } void ExpireFact::dump(llvm::raw_ostream &OS, const LoanManager &LM, - const OriginManager &) const { + const OriginManager &OM) const { OS << "Expire ("; LM.getLoan(getLoanID())->dump(OS); + if (auto OID = getOriginID()) { + OS << ", Origin: "; + OM.dump(*OID, OS); + } OS << ")\n"; } diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 886111ee64e73..e2b297518cb7d 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -470,10 +470,28 @@ void FactsGenerator::VisitLambdaExpr(const LambdaExpr *LE) { } } +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) { const VarDecl *LifetimeEndsVD = LifetimeEnds.getVarDecl(); if (!LifetimeEndsVD) return; + // In loops, the back-edge can make a dead origin appear live at its + // pointee's ExpireFact. Expiring the origin prevents that. + std::optional<OriginID> ExpiredOID; + if (OriginList *List = getOriginsList(*LifetimeEndsVD)) { + OriginID OID = List->getOuterOriginID(); + // Skip if this origin escapes. Its loans are still needed + // for the escape checker. + if (!isEscapingOrigin(OID)) + ExpiredOID = OID; + } // Iterate through all loans to see if any expire. for (const auto *Loan : FactMgr.getLoanMgr().getLoans()) { if (const auto *BL = dyn_cast<PathLoan>(Loan)) { @@ -483,7 +501,8 @@ void FactsGenerator::handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds) { const ValueDecl *Path = AP.getAsValueDecl(); if (Path == LifetimeEndsVD) CurrentBlockFacts.push_back(FactMgr.createFact<ExpireFact>( - BL->getID(), LifetimeEnds.getTriggerStmt()->getEndLoc())); + BL->getID(), LifetimeEnds.getTriggerStmt()->getEndLoc(), + ExpiredOID)); } } } diff --git a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp index fe20d779669c1..bc7494360624e 100644 --- a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp @@ -166,6 +166,12 @@ class AnalysisImpl return Lattice(Factory.remove(In.LiveOrigins, OF.getDestOriginID())); } + Lattice transfer(Lattice In, const ExpireFact &F) { + if (auto OID = F.getOriginID()) + return Lattice(Factory.remove(In.LiveOrigins, *OID)); + return In; + } + LivenessMap getLiveOriginsAt(ProgramPoint P) const { return getState(P).LiveOrigins; } diff --git a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp index 8a020eb829be6..e437fb7d41268 100644 --- a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp @@ -181,6 +181,12 @@ class AnalysisImpl return setLoans(In, DestOID, MergedLoans); } + Lattice transfer(Lattice In, const ExpireFact &F) { + if (auto OID = F.getOriginID()) + return setLoans(In, *OID, LoanSetFactory.getEmptySet()); + return In; + } + 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 7034c8686b315..0651bbe9a9c1c 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -1931,3 +1931,51 @@ auto capture_multilevel_pointer() { return lambda; // expected-note 3 {{returned here}} } } // namespace lambda_captures + +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; + } +} + +// FIXME: False positive. Requires modeling flow-sensitive aliased origins +// to properly expire pp's inner origin when p's lifetime ends. +void multi_level_pointer_in_loop() { + for (int i = 0; i < 10; ++i) { + MyObj obj; + MyObj* p; + MyObj** pp; + if (i > 5) { + p = &obj; // expected-warning {{object whose reference is captured does not live long enough}} + pp = &p; + } + (void)**pp; // expected-note {{later used here}} + } // expected-note {{destroyed here}} +} + +void outer_pointer_outlives_inner_pointee() { + MyObj safe; + MyObj* view = &safe; + for (int i = 0; i < 10; ++i) { + MyObj obj; + view = &obj; // expected-warning {{object whose reference is captured does not live long enough}} + } // expected-note {{destroyed here}} + (void)*view; // expected-note {{later used here}} +} + +} // namespace LoopLocalPointers _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
