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 1/3] [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 >From d561767eee9443cc5ea1ad939b1d1670779ccda7 Mon Sep 17 00:00:00 2001 From: Alex Wang <[email protected]> Date: Fri, 13 Mar 2026 00:59:57 -0700 Subject: [PATCH 2/3] fix CI && add test --- clang/test/Sema/warn-lifetime-safety-dataflow.cpp | 2 +- clang/test/Sema/warn-lifetime-safety.cpp | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/clang/test/Sema/warn-lifetime-safety-dataflow.cpp b/clang/test/Sema/warn-lifetime-safety-dataflow.cpp index 7e2215b8deedc..be1b77b36a2e0 100644 --- a/clang/test/Sema/warn-lifetime-safety-dataflow.cpp +++ b/clang/test/Sema/warn-lifetime-safety-dataflow.cpp @@ -26,7 +26,7 @@ MyObj* return_local_addr() { // CHECK-NEXT: Dest: [[O_RET_VAL:[0-9]+]] (Expr: ImplicitCastExpr, Type : MyObj *) // CHECK-NEXT: Src: [[O_P]] (Decl: p, Type : MyObj *) // CHECK: Expire ([[L_X]] (Path: x)) -// CHECK: Expire ({{[0-9]+}} (Path: p)) +// CHECK: Expire ({{[0-9]+}} (Path: p), Origin: [[O_P]] (Decl: p, Type : MyObj *)) // CHECK: OriginEscapes ([[O_RET_VAL]] (Expr: ImplicitCastExpr, Type : MyObj *), via Return) } diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp index 0651bbe9a9c1c..93f777db891d7 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -612,6 +612,14 @@ int return_reference_to_parameter_no_error(int a) { return b; } +MyObj*& return_ref_to_local_ptr_pointing_to_local() { + MyObj local; + MyObj* p = &local; // expected-warning {{address of stack memory is returned later}} + return p; // expected-note {{returned here}} \ + // expected-warning {{address of stack memory is returned later}} \ + // expected-note {{returned here}} +} + const int& reference_via_conditional(int a, int b, bool cond) { const int &c = (cond ? ((a)) : (b)); // expected-warning 2 {{address of stack memory is returned later}} return c; // expected-note 2 {{returned here}} >From c24f79dceb6a89daf99f80d13157c8f12e87dcb7 Mon Sep 17 00:00:00 2001 From: Alex Wang <[email protected]> Date: Fri, 13 Mar 2026 10:45:49 -0700 Subject: [PATCH 3/3] update comment && only check for ReturnEscapeFact --- .../Analyses/LifetimeSafety/FactsGenerator.h | 2 +- .../lib/Analysis/LifetimeSafety/FactsGenerator.cpp | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h index 4bb2a4198ebcf..9bcf5193ef8fc 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h @@ -112,7 +112,7 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> { void markUseAsWrite(const DeclRefExpr *DRE); - bool isEscapingOrigin(OriginID OID) const; + bool escapesViaReturn(OriginID OID) const; llvm::SmallVector<Fact *> issuePlaceholderLoans(); FactManager &FactMgr; diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index e2b297518cb7d..888176f09c9b9 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -470,9 +470,9 @@ void FactsGenerator::VisitLambdaExpr(const LambdaExpr *LE) { } } -bool FactsGenerator::isEscapingOrigin(OriginID OID) const { +bool FactsGenerator::escapesViaReturn(OriginID OID) const { return llvm::any_of(EscapesInCurrentBlock, [OID](const Fact *F) { - if (const auto *EF = F->getAs<OriginEscapesFact>()) + if (const auto *EF = F->getAs<ReturnEscapeFact>()) return EF->getEscapedOriginID() == OID; return false; }); @@ -482,14 +482,14 @@ 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. + // Expire the origin when its variable's lifetime ends to ensure liveness + // doesn't persist through loop back-edges. 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)) + // Skip origins that escape via return; the escape checker needs their loans + // to remain until the return statement is processed. + if (!escapesViaReturn(OID)) ExpiredOID = OID; } // Iterate through all loans to see if any expire. _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
