[clang] fix-liveness (PR #205740)
Utkarsh Saxena via cfe-commits
cfe-commits at lists.llvm.org
Thu Jun 25 04:51:54 PDT 2026
https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/205740
>From 0830684c73818c3fcc6c10ab560731ac49fed062 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 1/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 | 69 +++++++++++++++++++
5 files changed, 128 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 50bf79d4c1a38..a2341ebc8f2ed 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -801,6 +801,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>())
@@ -908,6 +923,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 b59fac191dcfb..9a97edb2bc6f5 100644
--- a/clang/test/Sema/LifetimeSafety/safety.cpp
+++ b/clang/test/Sema/LifetimeSafety/safety.cpp
@@ -3955,3 +3955,72 @@ struct [[gsl::Pointer()]] PtrWithInt { int x; };
PtrWithInt f() {
return PtrWithInt{10};
}
+// 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
>From 5192ab231a77fcd5ed909523181cfdc0138e477d Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Thu, 25 Jun 2026 11:45:26 +0000
Subject: [PATCH 2/2] [LifetimeSafety] Fix loop liveness leakage for
conditional operator
Generate flow facts for conditional operators in their respective
predecessor blocks (branches) instead of the merge block, path-isolating
the flows and preventing liveness from leaking across loop backedges.
Also includes tests, formatting cleanups, and refactoring of the flow propagation.
TAG=agy
CONV=b4614911-a1e1-489f-a395-2f895c423788
---
.../Analysis/Analyses/LifetimeSafety/Facts.h | 5 +
.../Analyses/LifetimeSafety/FactsGenerator.h | 3 +-
.../LifetimeSafety/FactsGenerator.cpp | 103 ++++++++----------
clang/test/Sema/LifetimeSafety/safety.cpp | 17 +++
4 files changed, 72 insertions(+), 56 deletions(-)
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/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
index 8dc5213dd8de2..9821078ec1d1e 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
@@ -66,7 +66,8 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> {
bool hasOrigins(QualType QT) const;
bool hasOrigins(const Expr *E) const;
- void flow(OriginList *Dst, OriginList *Src, bool Kill);
+ void flow(OriginList *Dst, OriginList *Src, bool Kill,
+ const CFGBlock *Block = nullptr);
/// Handles assignment for both BinaryOperator and CXXOperatorCallExpr.
///
diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
index a2341ebc8f2ed..099d49d793107 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -58,7 +58,15 @@ bool FactsGenerator::hasOrigins(const Expr *E) const {
/// * Level 1: pp <- p's address
/// * Level 2: (*pp) <- what p points to (i.e., &x)
/// - `View v = obj;` flows origins from `obj` (depth 1) to `v` (depth 1)
-void FactsGenerator::flow(OriginList *Dst, OriginList *Src, bool Kill) {
+///
+/// \param Dst The destination origin list.
+/// \param Src The source origin list.
+/// \param Kill If true, the destination's existing loans are killed before flowing.
+/// \param Block Optional. If provided, the generated flow facts are appended to
+/// this specific CFG block. Otherwise, they are appended to the
+/// current block being visited.
+void FactsGenerator::flow(OriginList *Dst, OriginList *Src, bool Kill,
+ const CFGBlock *Block) {
if (!Dst)
return;
assert(Src &&
@@ -67,8 +75,12 @@ void FactsGenerator::flow(OriginList *Dst, OriginList *Src, bool Kill) {
"Lists must have the same length");
while (Dst && Src) {
- CurrentBlockFacts.push_back(FactMgr.createFact<OriginFlowFact>(
- Dst->getOuterOriginID(), Src->getOuterOriginID(), Kill));
+ Fact *F = FactMgr.createFact<OriginFlowFact>(Dst->getOuterOriginID(),
+ Src->getOuterOriginID(), Kill);
+ if (Block)
+ FactMgr.appendBlockFact(Block, F);
+ else
+ CurrentBlockFacts.push_back(F);
Dst = Dst->peelOuterOrigin();
Src = Src->peelOuterOrigin();
}
@@ -541,66 +553,47 @@ void FactsGenerator::VisitBinaryOperator(const BinaryOperator *BO) {
// TODO: Handle assignments involving dereference like `*p = q`.
}
+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;
+}
+
+/// Visits conditional operators (e.g., `cond ? a : b`).
+///
+/// To prevent liveness leakage across loop backedges (which causes false
+/// positives like in `while (...) { int x; consume(cond ? &x : nullptr); }`),
+/// we generate the flow facts in the respective predecessor blocks of the arms
+/// rather than in the merge block. This ensures that the liveness of the
+/// temporary origin from one arm does not propagate into the other arm's path.
void FactsGenerator::VisitAbstractConditionalOperator(
const AbstractConditionalOperator *CO) {
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();
-
- // Skip origin flow from conditional operator arms that cannot produce the
- // result value: throw arms and calls to noreturn functions.
- bool TBHasEdge = true;
- bool FBHasEdge = true;
-
- 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())
- TrueArm = Src->IgnoreParenImpCasts();
- TBHasEdge = llvm::any_of(**Preds.begin(), [TrueArm](const CFGElement &Elt) {
- if (auto CS = Elt.getAs<CFGStmt>())
- return CS->getStmt() == TrueArm;
- return false;
- });
- FBHasEdge = !TBHasEdge;
- break;
- }
- case 2: {
- const auto *It = Preds.begin();
- TBHasEdge = It->isReachable();
- FBHasEdge = (++It)->isReachable();
- break;
- }
- default:
- llvm_unreachable("expected at most 2 predecessors");
- return;
- }
-
- 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 (const CFGBlock *TBPred = findPredBlockForExpr(CurrentBlock, TrueExpr))
+ flow(getOriginsList(*CO), getOriginsList(*TrueExpr), /*Kill=*/true, TBPred);
+ if (const CFGBlock *FBPred = findPredBlockForExpr(CurrentBlock, FalseExpr))
+ flow(getOriginsList(*CO), getOriginsList(*FalseExpr), /*Kill=*/true,
+ FBPred);
}
void FactsGenerator::VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *OCE) {
diff --git a/clang/test/Sema/LifetimeSafety/safety.cpp b/clang/test/Sema/LifetimeSafety/safety.cpp
index 9a97edb2bc6f5..4ff03b0e960bc 100644
--- a/clang/test/Sema/LifetimeSafety/safety.cpp
+++ b/clang/test/Sema/LifetimeSafety/safety.cpp
@@ -3955,6 +3955,7 @@ struct [[gsl::Pointer()]] PtrWithInt { int x; };
PtrWithInt f() {
return PtrWithInt{10};
}
+
// 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
@@ -4024,3 +4025,19 @@ void discarded_body_local() {
(void)({ int x = 7; &x; }); // no-warning
}
} // namespace statement_expression
+
+// 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
+ }
+ for (int i = 0; i < 2; i++) {
+ int x, y;
+ consume_loop_cond_bind(cond ? &x : &y); // no-warning
+ }
+}
More information about the cfe-commits
mailing list