Author: Utkarsh Saxena Date: 2026-06-23T17:09:45Z New Revision: 8d2a578b2130742c8790f3dba5fb414962eafcd5
URL: https://github.com/llvm/llvm-project/commit/8d2a578b2130742c8790f3dba5fb414962eafcd5 DIFF: https://github.com/llvm/llvm-project/commit/8d2a578b2130742c8790f3dba5fb414962eafcd5.diff LOG: [LifetimeSafety] Fix liveness propagation for all origin flows (#205323) Previously, the `transfer` function for `OriginFlowFact` only handled killing the destination origin. It did not propagate liveness backwards from destination to source, meaning that if an origin flowed into another, the source was not marked live even when the destination was. The `transfer` function now propagates liveness from destination to source: if the destination origin is live, the source origin is also marked live with the same `LivenessInfo`, before optionally killing the destination. Additionally, `handleMovedArgsInCall` now skips rvalue reference parameters annotated with `[[clang::lifetimebound]]`, since such parameters should not be treated as moved-from. This introduces some false positives for invalidations as container modifications are not considered self-invalidating. The updated test expectations reflect this, along with `FIXME` comments explaining that they are false positives stemming from the lack of distinction between owner-borrows and content-borrows, which will require more precise `AccessPath` reasoning to resolve. Added: Modified: clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp clang/test/Sema/LifetimeSafety/invalidations.cpp Removed: ################################################################################ diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 50bf79d4c1a38..d8c5679a80b38 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -908,6 +908,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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
