[clang] [LifetimeSafety] Fix liveness propagation for all origin flows (PR #205323)
Utkarsh Saxena via cfe-commits
cfe-commits at lists.llvm.org
Tue Jun 23 06:29:43 PDT 2026
https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/205323
>From d7bf3bd12dbf4ded1438f041c7f734571b16dc2f Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Tue, 23 Jun 2026 12:03:27 +0000
Subject: [PATCH 1/5] make liveness more precise
---
.../Analyses/LifetimeSafety/FactsGenerator.h | 1 +
.../LifetimeSafety/FactsGenerator.cpp | 11 +++++++
.../Analysis/LifetimeSafety/LiveOrigins.cpp | 17 +++++++++--
.../Sema/LifetimeSafety/invalidations.cpp | 29 +++++++++++++++----
4 files changed, 49 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 3861117005752..ee4873882f2dc 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -780,6 +780,15 @@ void FactsGenerator::VisitCXXDeleteExpr(const CXXDeleteExpr *DE) {
FactMgr.createFact<InvalidateOriginFact>(List->getOuterOriginID(), DE));
}
+void FactsGenerator::VisitStmtExpr(const StmtExpr *SE) {
+ if (const CompoundStmt *CS = SE->getSubStmt(); CS && !CS->body_empty()) {
+ if (const auto *Last = dyn_cast<Expr>(CS->body_back())) {
+ // Flow the value from the last expression to the statement expression itself.
+ killAndFlowOrigin(*SE, *Last);
+ }
+ }
+}
+
bool FactsGenerator::escapesViaReturn(OriginID OID) const {
return llvm::any_of(EscapesInCurrentBlock, [OID](const Fact *F) {
if (const auto *EF = F->getAs<ReturnEscapeFact>())
@@ -887,6 +896,8 @@ void FactsGenerator::handleMovedArgsInCall(const FunctionDecl *FD,
const ParmVarDecl *PVD = FD->getParamDecl(I - IsInstance);
if (!PVD->getType()->isRValueReferenceType())
continue;
+ if (PVD->hasAttr<LifetimeBoundAttr>())
+ 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..eabf7f1373daf 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();
+ // Since this is a backward analysis, if the destination of the flow is live
+ // (needed in the future), 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
>From 5a366da9c9ade958c2fb0a811ae4bd4a23cdf55c Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Tue, 23 Jun 2026 12:04:43 +0000
Subject: [PATCH 2/5] doc
---
clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp
index eabf7f1373daf..69b903c813555 100644
--- a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp
@@ -164,9 +164,9 @@ class AnalysisImpl
Lattice Out = In;
OriginID Dest = OF.getDestOriginID();
OriginID Src = OF.getSrcOriginID();
- // Since this is a backward analysis, if the destination of the flow is live
- // (needed in the future), the source of the flow must also be marked live
- // before this point (as its value will flow into the destination).
+ // 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);
>From dc4e682b1b8230e7f22bbdcf21b54cd91b4d54ac Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Tue, 23 Jun 2026 12:07:15 +0000
Subject: [PATCH 3/5] old changes
---
.../Analysis/Analyses/LifetimeSafety/FactsGenerator.h | 1 -
clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp | 9 ---------
2 files changed, 10 deletions(-)
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
index 8dc5213dd8de2..5ac67263681ac 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
@@ -57,7 +57,6 @@ 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 ee4873882f2dc..784df2b1f9ab8 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -780,15 +780,6 @@ void FactsGenerator::VisitCXXDeleteExpr(const CXXDeleteExpr *DE) {
FactMgr.createFact<InvalidateOriginFact>(List->getOuterOriginID(), DE));
}
-void FactsGenerator::VisitStmtExpr(const StmtExpr *SE) {
- if (const CompoundStmt *CS = SE->getSubStmt(); CS && !CS->body_empty()) {
- if (const auto *Last = dyn_cast<Expr>(CS->body_back())) {
- // Flow the value from the last expression to the statement expression itself.
- killAndFlowOrigin(*SE, *Last);
- }
- }
-}
-
bool FactsGenerator::escapesViaReturn(OriginID OID) const {
return llvm::any_of(EscapesInCurrentBlock, [OID](const Fact *F) {
if (const auto *EF = F->getAs<ReturnEscapeFact>())
>From 72d229e6f149e9623c2f4d46f6c688da65b8ba2e Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Tue, 23 Jun 2026 12:12:53 +0000
Subject: [PATCH 4/5] doc for lifetimebound change
---
clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp | 3 +++
1 file changed, 3 insertions(+)
diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
index 784df2b1f9ab8..238b25824a4ce 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -887,6 +887,9 @@ void FactsGenerator::handleMovedArgsInCall(const FunctionDecl *FD,
const ParmVarDecl *PVD = FD->getParamDecl(I - IsInstance);
if (!PVD->getType()->isRValueReferenceType())
continue;
+ // Skip lifetimebound r-value reference parameters. Lifetimebound 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>())
continue;
const Expr *Arg = Args[I];
>From 58b068185febc24ba7c7b5e6213c60e01b25e47b Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Tue, 23 Jun 2026 13:24:07 +0000
Subject: [PATCH 5/5] LifetimeCaptureByAttr
---
clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
index 238b25824a4ce..4400c2eb5f6bb 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -9,6 +9,7 @@
#include <cassert>
#include <string>
+#include "clang/AST/Attrs.inc"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/Expr.h"
@@ -887,10 +888,11 @@ void FactsGenerator::handleMovedArgsInCall(const FunctionDecl *FD,
const ParmVarDecl *PVD = FD->getParamDecl(I - IsInstance);
if (!PVD->getType()->isRValueReferenceType())
continue;
- // Skip lifetimebound r-value reference parameters. Lifetimebound 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>())
+ // 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);
More information about the cfe-commits
mailing list