[clang] [LifetimeSafety] Fix false positives for pointers in loops (PR #182368)
Zhijie Wang via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 13 10:46:06 PDT 2026
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 <yesterda9 at gmail.com>
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 <yesterda9 at gmail.com>
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 <yesterda9 at gmail.com>
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.
More information about the cfe-commits
mailing list