https://github.com/usx95 created https://github.com/llvm/llvm-project/pull/205323
make liveness more precise doc >From 3cec132fb4b35a1a039e3fff822cdee12111f788 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <[email protected]> Date: Tue, 23 Jun 2026 12:03:27 +0000 Subject: [PATCH 1/2] 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 301822f066de8..e1f4c135e7ffe 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 {{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 {{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 {{invalidated here}} \ + // expected-note {{later used here}} \ + // expected-note {{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 {{invalidated here}} \ - // expected-note {{later used here}} + // expected-warning {{parameter 'mp' is later invalidated}} \ + // expected-note {{invalidated here}} \ + // expected-note {{later used here}} \ + // expected-note {{invalidated here}} \ + // expected-note {{later used here}} } } // namespace AssociativeContainers >From 3d5cc4a8e068d81d727ca728c90c51a3521b5954 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <[email protected]> Date: Tue, 23 Jun 2026 12:04:43 +0000 Subject: [PATCH 2/2] 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); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
