[clang] [llvm] fix-liveness (PR #205740)
Utkarsh Saxena via cfe-commits
cfe-commits at lists.llvm.org
Thu Jun 25 00:32:42 PDT 2026
https://github.com/usx95 created https://github.com/llvm/llvm-project/pull/205740
fix-liveness
Reapply "[LifetimeSafety] Fix liveness propagation for all origin flows (#205323)" (#205687)
This reverts commit d4cf04ba17c833cfbab5a16aa2d21f7185a0c9ae.
>From 11682b558c96366401bd66025edd8bd8e5682753 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Wed, 24 Jun 2026 21:47:53 +0000
Subject: [PATCH 1/2] fix-liveness
---
.../Analysis/Analyses/LifetimeSafety/Facts.h | 5 +
.../LifetimeSafety/FactsGenerator.cpp | 93 +++++++++++++++----
clang/test/Sema/LifetimeSafety/safety.cpp | 13 +++
lifetime_reproduce.cpp | 8 ++
4 files changed, 99 insertions(+), 20 deletions(-)
create mode 100644 lifetime_reproduce.cpp
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h
index 88b509e1b94df..ac12957a1fb2c 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h
@@ -351,6 +351,11 @@ class FactManager {
BlockToFacts[B->getBlockID()].assign(NewFacts.begin(), NewFacts.end());
}
+ void appendBlockFact(const CFGBlock *B, const Fact *F) {
+ BlockToFacts[B->getBlockID()].push_back(F);
+ }
+
+
template <typename FactType, typename... Args>
FactType *createFact(Args &&...args) {
void *Mem = FactAllocator.Allocate<FactType>();
diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
index 50bf79d4c1a38..fcb6912ba746a 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -102,6 +102,32 @@ static const Loan *createLoan(FactManager &FactMgr, const CXXNewExpr *NE) {
return FactMgr.getLoanMgr().createLoan(Path, NE);
}
+static const CFGBlock *findPredBlockForExpr(const CFGBlock *MergeBlock,
+ const Expr *ArmExpr) {
+ if (!ArmExpr)
+ return nullptr;
+ const Expr *Target = ArmExpr->IgnoreParenImpCasts();
+ if (const auto *OVE = dyn_cast<OpaqueValueExpr>(Target))
+ if (const Expr *Src = OVE->getSourceExpr())
+ Target = Src->IgnoreParenImpCasts();
+
+ for (const CFGBlock *Pred : MergeBlock->preds()) {
+ if (!Pred)
+ continue;
+ for (const CFGElement &Elt : *Pred) {
+ if (auto CS = Elt.getAs<CFGStmt>()) {
+ if (const auto *E = dyn_cast<Expr>(CS->getStmt())) {
+ if (E->IgnoreParenImpCasts() == Target) {
+ return Pred;
+ }
+ }
+ }
+ }
+ }
+ return nullptr;
+}
+
+
void FactsGenerator::run() {
llvm::TimeTraceScope TimeProfile("FactGenerator");
const CFG &Cfg = *AC.getCFG();
@@ -546,24 +572,56 @@ void FactsGenerator::VisitAbstractConditionalOperator(
if (!hasOrigins(CO))
return;
- // For the GNU binary conditional `a ?: b`, getTrueExpr() is the
- // OpaqueValueExpr wrapping the common subexpression.
const Expr *TrueExpr = CO->getTrueExpr();
const Expr *FalseExpr = CO->getFalseExpr();
- const auto Preds = CurrentBlock->preds();
+ // Try to find predecessor blocks for the arms.
+ const CFGBlock *TBPred = findPredBlockForExpr(CurrentBlock, TrueExpr);
+ const CFGBlock *FBPred = findPredBlockForExpr(CurrentBlock, FalseExpr);
+
+ // Helper to generate flow facts.
+ auto HandleFlowInBlock = [&](const Expr *E, const CFGBlock *PredBlock, bool Kill) {
+ OriginList *Dst = getOriginsList(*CO);
+ OriginList *Src = getOriginsList(*E);
+ if (!Dst || !Src)
+ return;
+ assert(Dst->getLength() == Src->getLength() &&
+ "Lists must have the same length");
+
+ OriginList *CurDst = Dst;
+ OriginList *CurSrc = Src;
+ while (CurDst && CurSrc) {
+ const Fact *F = FactMgr.createFact<OriginFlowFact>(
+ CurDst->getOuterOriginID(), CurSrc->getOuterOriginID(), Kill);
+ if (PredBlock) {
+ FactMgr.appendBlockFact(PredBlock, F);
+ } else {
+ CurrentBlockFacts.push_back(const_cast<Fact *>(F));
+ }
+ CurDst = CurDst->peelOuterOrigin();
+ CurSrc = CurSrc->peelOuterOrigin();
+ }
+ };
- // Skip origin flow from conditional operator arms that cannot produce the
- // result value: throw arms and calls to noreturn functions.
+ // If we found both predecessor blocks, we can generate the flows in them.
+ // In this case, BOTH can be "Kill" flows because they are on separate paths.
+ if (TBPred && FBPred) {
+ assert(TBPred != FBPred && "Predecessor blocks must be different");
+ HandleFlowInBlock(TrueExpr, TBPred, /*Kill=*/true);
+ HandleFlowInBlock(FalseExpr, FBPred, /*Kill=*/true);
+ return;
+ }
+
+ // Fallback: generate in CurrentBlock (B3).
+ // In this case, we must use Kill for the first one and Merge for the second.
bool TBHasEdge = true;
bool FBHasEdge = true;
+ const auto Preds = CurrentBlock->preds();
switch (CurrentBlock->pred_size()) {
case 0:
return;
case 1: {
- // For `a ?: b`, getTrueExpr() is the OpaqueValueExpr; the common
- // subexpression it wraps is what appears in the predecessor block.
const Expr *TrueArm = TrueExpr->IgnoreParenImpCasts();
if (const auto *OVE = dyn_cast<OpaqueValueExpr>(TrueArm))
if (const Expr *Src = OVE->getSourceExpr())
@@ -588,21 +646,16 @@ void FactsGenerator::VisitAbstractConditionalOperator(
}
bool FirstFlow = true;
- auto HandleFlow = [&](const Expr *E) {
- if (FirstFlow) {
- killAndFlowOrigin(*CO, *E);
- FirstFlow = false;
- } else {
- flowOrigin(*CO, *E);
- }
- };
-
- if (TBHasEdge)
- HandleFlow(TrueExpr);
- if (FBHasEdge)
- HandleFlow(FalseExpr);
+ if (TBHasEdge) {
+ HandleFlowInBlock(TrueExpr, nullptr, /*Kill=*/FirstFlow);
+ FirstFlow = false;
+ }
+ if (FBHasEdge) {
+ HandleFlowInBlock(FalseExpr, nullptr, /*Kill=*/FirstFlow);
+ }
}
+
void FactsGenerator::VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *OCE) {
// Assignment operators have special "kill-then-propagate" semantics
// and are handled separately.
diff --git a/clang/test/Sema/LifetimeSafety/safety.cpp b/clang/test/Sema/LifetimeSafety/safety.cpp
index b59fac191dcfb..3243d8ad4cea5 100644
--- a/clang/test/Sema/LifetimeSafety/safety.cpp
+++ b/clang/test/Sema/LifetimeSafety/safety.cpp
@@ -3955,3 +3955,16 @@ struct [[gsl::Pointer()]] PtrWithInt { int x; };
PtrWithInt f() {
return PtrWithInt{10};
}
+
+// Test case for false positive involving conditional operator in a loop.
+struct LoopCondBindS {
+ int* get() const [[clang::lifetimebound]];
+};
+void consume_loop_cond_bind(int*);
+void test_loop_cond_bind(bool cond) {
+ for (int i = 0; i < 2; i++) {
+ LoopCondBindS s;
+ consume_loop_cond_bind(cond ? s.get() : nullptr); // no-warning
+ }
+}
+
diff --git a/lifetime_reproduce.cpp b/lifetime_reproduce.cpp
new file mode 100644
index 0000000000000..cc869194952db
--- /dev/null
+++ b/lifetime_reproduce.cpp
@@ -0,0 +1,8 @@
+void consume(int* a);
+
+void test(bool cond) {
+ for (int i = 0; i < 2; i++) {
+ int x, y;
+ consume(cond ? &x : &y);
+ }
+}
>From b95d7207f13b000628773ba74fb7442400f2260d Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Thu, 25 Jun 2026 07:30:26 +0000
Subject: [PATCH 2/2] Reapply "[LifetimeSafety] Fix liveness propagation for
all origin flows (#205323)" (#205687)
This reverts commit d4cf04ba17c833cfbab5a16aa2d21f7185a0c9ae.
---
.../Analyses/LifetimeSafety/FactsGenerator.h | 1 +
.../LifetimeSafety/FactsGenerator.cpp | 21 ++++++
.../Analysis/LifetimeSafety/LiveOrigins.cpp | 17 ++++-
.../Sema/LifetimeSafety/invalidations.cpp | 29 ++++++--
clang/test/Sema/LifetimeSafety/safety.cpp | 72 +++++++++++++++++++
5 files changed, 131 insertions(+), 9 deletions(-)
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
index 5ac67263681ac..8dc5213dd8de2 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
@@ -57,6 +57,7 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> {
void VisitArraySubscriptExpr(const ArraySubscriptExpr *ASE);
void VisitCXXNewExpr(const CXXNewExpr *NE);
void VisitCXXDeleteExpr(const CXXDeleteExpr *DE);
+ void VisitStmtExpr(const StmtExpr *SE);
private:
OriginList *getOriginsList(const ValueDecl &D);
diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
index fcb6912ba746a..a9999e7c6147b 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -854,6 +854,21 @@ void FactsGenerator::VisitCXXDeleteExpr(const CXXDeleteExpr *DE) {
FactMgr.createFact<InvalidateOriginFact>(List->getOuterOriginID(), DE));
}
+void FactsGenerator::VisitStmtExpr(const StmtExpr *SE) {
+ // A statement expression (`({ ...; e; })`) yields the value of its final
+ // expression `e`. Flow `e`'s origins into the statement expression's origin
+ // so a borrow `e` carries reaches the value's users.
+ const auto *CS = SE->getSubStmt();
+ if (!CS || CS->body_empty())
+ return;
+ const auto *Last = dyn_cast<Expr>(CS->body_back());
+ if (!Last)
+ return;
+ if (OriginList *Dst = getOriginsList(*SE))
+ if (OriginList *Src = getRValueOrigins(Last, getOriginsList(*Last)))
+ flow(Dst, Src, /*Kill=*/true);
+}
+
bool FactsGenerator::escapesViaReturn(OriginID OID) const {
return llvm::any_of(EscapesInCurrentBlock, [OID](const Fact *F) {
if (const auto *EF = F->getAs<ReturnEscapeFact>())
@@ -961,6 +976,12 @@ void FactsGenerator::handleMovedArgsInCall(const FunctionDecl *FD,
const ParmVarDecl *PVD = FD->getParamDecl(I - IsInstance);
if (!PVD->getType()->isRValueReferenceType())
continue;
+ // Skip lifetime annotated r-value reference parameters. Lifetime annotation
+ // indicates that the parameter is borrowed (not consumed), so it should not
+ // be marked as moved even though it's an r-value reference.
+ if (PVD->hasAttr<LifetimeBoundAttr>() ||
+ PVD->hasAttr<LifetimeCaptureByAttr>())
+ continue;
const Expr *Arg = Args[I];
OriginList *MovedOrigins = getOriginsList(*Arg);
assert(MovedOrigins->getLength() >= 1 &&
diff --git a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp
index cfbcacf04b1b0..69b903c813555 100644
--- a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp
@@ -161,9 +161,20 @@ class AnalysisImpl
/// An OriginFlow kills the liveness of the destination origin if `KillDest`
/// is true. Otherwise, it propagates liveness from destination to source.
Lattice transfer(Lattice In, const OriginFlowFact &OF) {
- if (!OF.getKillDest())
- return In;
- return Lattice(Factory.remove(In.LiveOrigins, OF.getDestOriginID()));
+ 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 (OF.getKillDest())
+ Out = Lattice(Factory.remove(Out.LiveOrigins, Dest));
+ return Out;
}
Lattice transfer(Lattice In, const KillOriginFact &F) {
diff --git a/clang/test/Sema/LifetimeSafety/invalidations.cpp b/clang/test/Sema/LifetimeSafety/invalidations.cpp
index c2ac105855d07..be1acc6bc7fbc 100644
--- a/clang/test/Sema/LifetimeSafety/invalidations.cpp
+++ b/clang/test/Sema/LifetimeSafety/invalidations.cpp
@@ -402,10 +402,20 @@ void SelfInvalidatingMap() {
// Therefore the following is safe in practice.
// On the other hand, std::flat_map (since C++23) does not provide pointer stability on
// insertion and following is unsafe for this container.
- mp[1] = "42";
- mp[2] // expected-note {{local variable 'mp' is invalidated here}}
- =
- mp[1]; // expected-warning {{local variable 'mp' is later invalidated}} expected-note {{later used here}}
+ // FIXME: The warnings below are false positives (self-invalidation of the Owner).
+ // Modifying a container should not invalidate the container object itself.
+ // To resolve this, we need to:
+ // 1. Distinguish owner-borrow (borrowing the container object) from content-borrow (borrowing elements inside the container).
+ // 2. Make AccessPaths more precise to reason at element/field granularity rather than treating the whole container as a single storage location.
+ mp[1] = "42"; // expected-warning {{local variable 'mp' is later invalidated}} \
+ // expected-note {{local variable 'mp' is invalidated here}} \
+ // expected-note {{later used here}}
+ mp[2] = mp[1]; // expected-warning {{local variable 'mp' is later invalidated}} \
+ // expected-warning {{local variable 'mp' is later invalidated}} \
+ // expected-note {{local variable 'mp' is invalidated here}} \
+ // expected-note {{later used here}} \
+ // expected-note {{local variable 'mp' is invalidated here}} \
+ // expected-note {{later used here}}
}
void InvalidateErase() {
@@ -740,9 +750,16 @@ void MapSubscriptMultipleCallsDoesNotInvalidate(std::map<int, int> mp, int a, in
}
void FlatMapSubscriptMultipleCallsInvalidate(std::flat_map<int, int> mp, int a, int b) {
+ // FIXME: The duplicate warning below is a false positive caused by self-invalidation of the Owner 'mp'.
+ // While the warning on the temporary reference returned by mp[a] is a true positive (it dangles),
+ // the second warning on 'mp' itself is redundant and incorrect.
+ // Resolving this requires distinguishing owner-borrow from content-borrow.
PrintMax(mp[a], mp[b]); // expected-warning {{parameter 'mp' is later invalidated}} \
- // expected-note {{parameter 'mp' is invalidated here}} \
- // expected-note {{later used here}}
+ // expected-warning {{parameter 'mp' is later invalidated}} \
+ // expected-note {{parameter 'mp' is invalidated here}} \
+ // expected-note {{later used here}} \
+ // expected-note {{parameter 'mp' is invalidated here}} \
+ // expected-note {{later used here}}
}
} // namespace AssociativeContainers
diff --git a/clang/test/Sema/LifetimeSafety/safety.cpp b/clang/test/Sema/LifetimeSafety/safety.cpp
index 3243d8ad4cea5..aaa4f3bf615b0 100644
--- a/clang/test/Sema/LifetimeSafety/safety.cpp
+++ b/clang/test/Sema/LifetimeSafety/safety.cpp
@@ -3956,6 +3956,7 @@ PtrWithInt f() {
return PtrWithInt{10};
}
+<<<<<<< HEAD
// Test case for false positive involving conditional operator in a loop.
struct LoopCondBindS {
int* get() const [[clang::lifetimebound]];
@@ -3968,3 +3969,74 @@ void test_loop_cond_bind(bool cond) {
}
}
+=======
+// A GNU statement expression (`({ ...; e; })`) yields the value of its final
+// expression `e`. `e`'s origins flow into the statement expression's value, so
+// a borrow `e` carries is tracked: a borrow of a body-local dangles, and a
+// borrow forwarded from an outer object propagates to the value's users.
+namespace statement_expression {
+void use(int *p);
+
+// A borrow of a statement-expression-local escaping via the value.
+void borrow_of_local() {
+ int *p = ({ int x = 7; &x; }); // expected-warning {{local variable 'x' does not live long enough}} expected-note {{local variable 'x' is destroyed here}}
+ use(p); // expected-note {{later used here}}
+}
+
+// An outer borrow forwarded through a statement expression and returned:
+// use-after-return.
+int *return_borrow_of_local() {
+ int local = 0;
+ return ({ (void)0; &local; }); // expected-warning {{stack memory associated with local variable 'local' is returned}} expected-note {{returned here}}
+}
+
+// A view bound to a temporary produced by the statement expression dangles.
+void borrow_temporary() {
+ std::string_view view = ({ std::string x = "long enough heap string!!!!!!"; x; }); // expected-warning {{temporary object does not live long enough}} expected-note {{temporary object is destroyed here}}
+ (void)view; // expected-note {{later used here}}
+}
+
+// Forwarding an outer borrow that dangles.
+void forward_outer_borrow() {
+ int *p;
+ {
+ int local = 0;
+ p = ({ (void)0; &local; }); // expected-warning {{local variable 'local' does not live long enough}}
+ } // expected-note {{local variable 'local' is destroyed here}}
+ use(p); // expected-note {{later used here}}
+}
+
+// The statement-expression result carries the borrow, so a `?:` sibling
+// supplying a valid loan no longer hides it via the merge.
+void masked(bool c) {
+ static int valid;
+ int *keep = &valid;
+ int *r;
+ {
+ int local = 0;
+ r = c ? keep : ({ &local; }); // expected-warning {{local variable 'local' does not live long enough}}
+ } // expected-note {{local variable 'local' is destroyed here}}
+ use(r); // expected-note {{later used here}}
+}
+
+// Both conditional arms are statement expressions returning a borrow of a
+// body-local; each is caught as a returned stack address.
+int *conditional_arms(bool c) {
+ return c ? ({ int x = 7; &x; }) // expected-warning {{stack memory associated with local variable 'x' is returned}} expected-note 2 {{returned here}}
+ : ({ int y = 7; &y; }); // expected-warning {{stack memory associated with local variable 'y' is returned}}
+}
+
+// Negative: a statement expression yielding a long-lived borrow stays silent.
+void ok() {
+ static int s;
+ int *p = ({ int unused = 0; (void)unused; &s; });
+ use(p); // no-warning
+}
+
+// A discarded statement expression's value is not consumed, so a borrow of a
+// body-local in it does not reach any user and is correctly not flagged.
+void discarded_body_local() {
+ (void)({ int x = 7; &x; }); // no-warning
+}
+} // namespace statement_expression
+>>>>>>> parent of d4cf04ba17c8 (Revert "[LifetimeSafety] Fix liveness propagation for all origin flows (#205323)" (#205687))
More information about the cfe-commits
mailing list