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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
