https://github.com/aeft updated https://github.com/llvm/llvm-project/pull/184295
>From fe7ea93422db01f15d310941cc2e859f1748562f Mon Sep 17 00:00:00 2001 From: Zhijie Wang <[email protected]> Date: Mon, 30 Mar 2026 09:54:06 -0700 Subject: [PATCH 1/4] [LifetimeSafety] Detect use of a reference type as a use of underlying origin --- .../Analysis/Analyses/LifetimeSafety/Facts.h | 24 ++++++++++++++++--- clang/lib/Analysis/LifetimeSafety/Facts.cpp | 14 +++++------ .../LifetimeSafety/FactsGenerator.cpp | 23 ++++++++++++++++-- .../Analysis/LifetimeSafety/LiveOrigins.cpp | 6 ++--- .../LifetimeSafety/LoanPropagation.cpp | 5 ++-- .../warn-lifetime-safety-invalidations.cpp | 15 ++++++++---- clang/test/Sema/warn-lifetime-safety.cpp | 12 ++++++++++ 7 files changed, 75 insertions(+), 24 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index 0f848abd913d3..a986f37e67c05 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -234,7 +234,7 @@ class GlobalEscapeFact : public OriginEscapesFact { class UseFact : public Fact { const Expr *UseExpr; - const OriginList *OList; + std::variant<const OriginList *, OriginID> Origins; // True if this use is a write operation (e.g., left-hand side of assignment). // Write operations are exempted from use-after-free checks. bool IsWritten = false; @@ -243,13 +243,31 @@ class UseFact : public Fact { static bool classof(const Fact *F) { return F->getKind() == Kind::Use; } UseFact(const Expr *UseExpr, const OriginList *OList) - : Fact(Kind::Use), UseExpr(UseExpr), OList(OList) {} + : Fact(Kind::Use), UseExpr(UseExpr), Origins(OList) {} + UseFact(const Expr *UseExpr, OriginID OID) + : Fact(Kind::Use), UseExpr(UseExpr), Origins(OID) {} - const OriginList *getUsedOrigins() const { return OList; } const Expr *getUseExpr() const { return UseExpr; } void markAsWritten() { IsWritten = true; } bool isWritten() const { return IsWritten; } + void setOrigins(OriginID OID) { Origins = OID; } + + const OriginList *getOriginList() const { + if (auto *OList = std::get_if<const OriginList *>(&Origins)) + return *OList; + return nullptr; + } + + template <typename Fn> void forEachOrigin(Fn &&F) const { + if (const OriginList *OList = getOriginList()) { + for (auto *Cur = OList; Cur; Cur = Cur->peelOuterOrigin()) + F(Cur->getOuterOriginID()); + } else { + F(std::get<OriginID>(Origins)); + } + } + void dump(llvm::raw_ostream &OS, const LoanManager &, const OriginManager &OM) const override; }; diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp index 1bc0521a72359..d1dd396bdbe10 100644 --- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp @@ -80,14 +80,14 @@ void GlobalEscapeFact::dump(llvm::raw_ostream &OS, const LoanManager &, void UseFact::dump(llvm::raw_ostream &OS, const LoanManager &, const OriginManager &OM) const { OS << "Use ("; - size_t NumUsedOrigins = getUsedOrigins()->getLength(); - size_t I = 0; - for (const OriginList *Cur = getUsedOrigins(); Cur; - Cur = Cur->peelOuterOrigin(), ++I) { - OM.dump(Cur->getOuterOriginID(), OS); - if (I < NumUsedOrigins - 1) + bool First = true; + forEachOrigin([&](OriginID OID) { + if (!First) OS << ", "; - } + else + First = false; + OM.dump(OID, OS); + }); OS << ", " << (isWritten() ? "Write" : "Read") << ")\n"; } diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 80a73a2bf687e..bf3589314d7f7 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -366,8 +366,27 @@ void FactsGenerator::handleAssignment(const Expr *LHSExpr, // assigned. RHSList = getRValueOrigins(RHSExpr, RHSList); - if (const auto *DRE_LHS = dyn_cast<DeclRefExpr>(LHSExpr)) - markUseAsWrite(DRE_LHS); + if (const auto *DRE_LHS = dyn_cast<DeclRefExpr>(LHSExpr)) { + QualType QT = DRE_LHS->getDecl()->getType(); + if (QT->isReferenceType() && hasOrigins(QT->getPointeeType())) { + // Writing through a reference uses the binding but overwrites the + // pointee. Model this as a Read of the outer origin (keeping the binding + // live) and a Write of the inner origins (killing the pointee's + // liveness). + if (UseFact *UF = UseFacts.lookup(DRE_LHS)) { + const OriginList *FullList = UF->getOriginList(); + assert(FullList); + UF->setOrigins(FullList->getOuterOriginID()); + if (const OriginList *InnerList = FullList->peelOuterOrigin()) { + UseFact *WriteUF = FactMgr.createFact<UseFact>(DRE_LHS, InnerList); + WriteUF->markAsWritten(); + CurrentBlockFacts.push_back(WriteUF); + } + } + } else if (!QT->isReferenceType()) { + markUseAsWrite(DRE_LHS); + } + } // Kill the old loans of the destination origin and flow the new loans // from the source origin. flow(LHSList->peelOuterOrigin(), RHSList, /*Kill=*/true); diff --git a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp index bc7494360624e..de4d7fa833ddf 100644 --- a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp @@ -129,9 +129,7 @@ class AnalysisImpl /// the origin since it overwrites the value. Lattice transfer(Lattice In, const UseFact &UF) { Lattice Out = In; - for (const OriginList *Cur = UF.getUsedOrigins(); Cur; - Cur = Cur->peelOuterOrigin()) { - OriginID OID = Cur->getOuterOriginID(); + UF.forEachOrigin([&](OriginID OID) { // Write kills liveness. if (UF.isWritten()) { Out = Lattice(Factory.remove(Out.LiveOrigins, OID)); @@ -141,7 +139,7 @@ class AnalysisImpl Out = Lattice(Factory.add(Out.LiveOrigins, OID, LivenessInfo(&UF, LivenessKind::Must))); } - } + }); return Out; } diff --git a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp index e437fb7d41268..8960479edb17b 100644 --- a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp @@ -59,9 +59,8 @@ static llvm::BitVector computePersistentOrigins(const FactManager &FactMgr, break; } case Fact::Kind::Use: - for (const OriginList *Cur = F->getAs<UseFact>()->getUsedOrigins(); Cur; - Cur = Cur->peelOuterOrigin()) - CheckOrigin(Cur->getOuterOriginID()); + F->getAs<UseFact>()->forEachOrigin( + [&](OriginID OID) { CheckOrigin(OID); }); break; case Fact::Kind::MovedOrigin: case Fact::Kind::OriginEscapes: diff --git a/clang/test/Sema/warn-lifetime-safety-invalidations.cpp b/clang/test/Sema/warn-lifetime-safety-invalidations.cpp index 486edd7a1a023..c7f920c03736a 100644 --- a/clang/test/Sema/warn-lifetime-safety-invalidations.cpp +++ b/clang/test/Sema/warn-lifetime-safety-invalidations.cpp @@ -259,14 +259,19 @@ namespace ElementReferences { void ReferenceToVectorElement() { std::vector<int> v = {1, 2, 3}; - int& ref = v[0]; - v.push_back(4); - // FIXME: Detect this as a use of 'ref'. - // https://github.com/llvm/llvm-project/issues/180187 - ref = 10; + int& ref = v[0]; // expected-warning {{object whose reference is captured is later invalidated}} + v.push_back(4); // expected-note {{invalidated here}} + ref = 10; // expected-note {{later used here}} (void)ref; } +void PointerRefToVectorElement() { + std::vector<int*> v = {nullptr, nullptr}; + int*& ref = v[0]; // expected-warning {{object whose reference is captured is later invalidated}} + v.push_back(nullptr); // expected-note {{invalidated here}} + ref = nullptr; // expected-note {{later used here}} +} + void PointerToVectorElement() { std::vector<int> v = {1, 2, 3}; int* ptr = &v[0]; // expected-warning {{object whose reference is captured is later invalidated}} diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp index 76d43445f8636..866542303414a 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -752,6 +752,18 @@ void no_error_if_dangle_then_rescue_gsl() { v.use(); // This is safe. } +void no_error_if_dangle_then_rescue_via_ref() { + MyObj safe; + MyObj* p; + MyObj*& ref = p; + { + MyObj temp; + ref = &temp; // p temporarily points to temp via ref. + } + ref = &safe; // p is "rescued" via ref before use. + (void)*ref; // This is safe. +} + void no_error_loan_from_current_iteration(bool cond) { // See https://github.com/llvm/llvm-project/issues/156959. MyObj b; >From 0da29b4d25907a3c6af60f8d0a78f9c598f14d8b Mon Sep 17 00:00:00 2001 From: Zhijie Wang <[email protected]> Date: Mon, 30 Mar 2026 14:05:56 -0700 Subject: [PATCH 2/4] add comment --- clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index a986f37e67c05..f4ff96786070a 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -234,6 +234,9 @@ class GlobalEscapeFact : public OriginEscapesFact { class UseFact : public Fact { const Expr *UseExpr; + // For reference writes, handleAssignment narrows this to a single OriginID + // for the outer origin (a Read of the reference binding) and creates a + // separate Write UseFact for the inner origins. std::variant<const OriginList *, OriginID> Origins; // True if this use is a write operation (e.g., left-hand side of assignment). // Write operations are exempted from use-after-free checks. >From fbcf7d4f058437b107f6285f26fd562cfbdce05a Mon Sep 17 00:00:00 2001 From: Zhijie Wang <[email protected]> Date: Tue, 7 Apr 2026 23:31:02 -0700 Subject: [PATCH 3/4] create an OriginList with a single origin --- .../Analysis/Analyses/LifetimeSafety/Facts.h | 28 +++---------------- .../Analyses/LifetimeSafety/Origins.h | 4 +++ clang/lib/Analysis/LifetimeSafety/Facts.cpp | 14 +++++----- .../LifetimeSafety/FactsGenerator.cpp | 5 ++-- .../Analysis/LifetimeSafety/LiveOrigins.cpp | 6 ++-- .../LifetimeSafety/LoanPropagation.cpp | 5 ++-- clang/lib/Analysis/LifetimeSafety/Origins.cpp | 4 +++ 7 files changed, 29 insertions(+), 37 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index f4ff96786070a..0107c4d04e073 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -234,10 +234,7 @@ class GlobalEscapeFact : public OriginEscapesFact { class UseFact : public Fact { const Expr *UseExpr; - // For reference writes, handleAssignment narrows this to a single OriginID - // for the outer origin (a Read of the reference binding) and creates a - // separate Write UseFact for the inner origins. - std::variant<const OriginList *, OriginID> Origins; + const OriginList *OList; // True if this use is a write operation (e.g., left-hand side of assignment). // Write operations are exempted from use-after-free checks. bool IsWritten = false; @@ -246,31 +243,14 @@ class UseFact : public Fact { static bool classof(const Fact *F) { return F->getKind() == Kind::Use; } UseFact(const Expr *UseExpr, const OriginList *OList) - : Fact(Kind::Use), UseExpr(UseExpr), Origins(OList) {} - UseFact(const Expr *UseExpr, OriginID OID) - : Fact(Kind::Use), UseExpr(UseExpr), Origins(OID) {} + : Fact(Kind::Use), UseExpr(UseExpr), OList(OList) {} + const OriginList *getUsedOrigins() const { return OList; } + void setUsedOrigins(const OriginList *NewList) { OList = NewList; } const Expr *getUseExpr() const { return UseExpr; } void markAsWritten() { IsWritten = true; } bool isWritten() const { return IsWritten; } - void setOrigins(OriginID OID) { Origins = OID; } - - const OriginList *getOriginList() const { - if (auto *OList = std::get_if<const OriginList *>(&Origins)) - return *OList; - return nullptr; - } - - template <typename Fn> void forEachOrigin(Fn &&F) const { - if (const OriginList *OList = getOriginList()) { - for (auto *Cur = OList; Cur; Cur = Cur->peelOuterOrigin()) - F(Cur->getOuterOriginID()); - } else { - F(std::get<OriginID>(Origins)); - } - } - void dump(llvm::raw_ostream &OS, const LoanManager &, const OriginManager &OM) const override; }; diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h index 8c638bdcace3f..a752f9939d482 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h @@ -145,6 +145,10 @@ class OriginManager { /// \returns The OriginList, or nullptr for non-pointer rvalues. OriginList *getOrCreateList(const Expr *E); + /// Wraps an existing OriginID in a new single-element OriginList, so a fact + /// can refer to a single level of an existing OriginList. + OriginList *createSingleOriginList(OriginID OID); + /// Returns the OriginList for the implicit 'this' parameter if the current /// declaration is an instance method. std::optional<OriginList *> getThisOrigins() const { return ThisOrigins; } diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp index d1dd396bdbe10..1bc0521a72359 100644 --- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp @@ -80,14 +80,14 @@ void GlobalEscapeFact::dump(llvm::raw_ostream &OS, const LoanManager &, void UseFact::dump(llvm::raw_ostream &OS, const LoanManager &, const OriginManager &OM) const { OS << "Use ("; - bool First = true; - forEachOrigin([&](OriginID OID) { - if (!First) + size_t NumUsedOrigins = getUsedOrigins()->getLength(); + size_t I = 0; + for (const OriginList *Cur = getUsedOrigins(); Cur; + Cur = Cur->peelOuterOrigin(), ++I) { + OM.dump(Cur->getOuterOriginID(), OS); + if (I < NumUsedOrigins - 1) OS << ", "; - else - First = false; - OM.dump(OID, OS); - }); + } OS << ", " << (isWritten() ? "Write" : "Read") << ")\n"; } diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index bf3589314d7f7..0719915144eea 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -374,9 +374,10 @@ void FactsGenerator::handleAssignment(const Expr *LHSExpr, // live) and a Write of the inner origins (killing the pointee's // liveness). if (UseFact *UF = UseFacts.lookup(DRE_LHS)) { - const OriginList *FullList = UF->getOriginList(); + const OriginList *FullList = UF->getUsedOrigins(); assert(FullList); - UF->setOrigins(FullList->getOuterOriginID()); + UF->setUsedOrigins(FactMgr.getOriginMgr().createSingleOriginList( + FullList->getOuterOriginID())); if (const OriginList *InnerList = FullList->peelOuterOrigin()) { UseFact *WriteUF = FactMgr.createFact<UseFact>(DRE_LHS, InnerList); WriteUF->markAsWritten(); diff --git a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp index de4d7fa833ddf..bc7494360624e 100644 --- a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp @@ -129,7 +129,9 @@ class AnalysisImpl /// the origin since it overwrites the value. Lattice transfer(Lattice In, const UseFact &UF) { Lattice Out = In; - UF.forEachOrigin([&](OriginID OID) { + for (const OriginList *Cur = UF.getUsedOrigins(); Cur; + Cur = Cur->peelOuterOrigin()) { + OriginID OID = Cur->getOuterOriginID(); // Write kills liveness. if (UF.isWritten()) { Out = Lattice(Factory.remove(Out.LiveOrigins, OID)); @@ -139,7 +141,7 @@ class AnalysisImpl Out = Lattice(Factory.add(Out.LiveOrigins, OID, LivenessInfo(&UF, LivenessKind::Must))); } - }); + } return Out; } diff --git a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp index 8960479edb17b..e437fb7d41268 100644 --- a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp @@ -59,8 +59,9 @@ static llvm::BitVector computePersistentOrigins(const FactManager &FactMgr, break; } case Fact::Kind::Use: - F->getAs<UseFact>()->forEachOrigin( - [&](OriginID OID) { CheckOrigin(OID); }); + for (const OriginList *Cur = F->getAs<UseFact>()->getUsedOrigins(); Cur; + Cur = Cur->peelOuterOrigin()) + CheckOrigin(Cur->getOuterOriginID()); break; case Fact::Kind::MovedOrigin: case Fact::Kind::OriginEscapes: diff --git a/clang/lib/Analysis/LifetimeSafety/Origins.cpp b/clang/lib/Analysis/LifetimeSafety/Origins.cpp index 0122f7a734541..df99a346c68e1 100644 --- a/clang/lib/Analysis/LifetimeSafety/Origins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Origins.cpp @@ -123,6 +123,10 @@ OriginList *OriginManager::createNode(const Expr *E, QualType QT) { return new (ListAllocator.Allocate<OriginList>()) OriginList(NewID); } +OriginList *OriginManager::createSingleOriginList(OriginID OID) { + return new (ListAllocator.Allocate<OriginList>()) OriginList(OID); +} + template <typename T> OriginList *OriginManager::buildListForType(QualType QT, const T *Node) { assert(hasOrigins(QT) && "buildListForType called for non-pointer type"); >From f2d2af9df3e1a765c0d11b33ed0b2446759d41d9 Mon Sep 17 00:00:00 2001 From: Zhijie Wang <[email protected]> Date: Wed, 8 Apr 2026 08:22:17 -0700 Subject: [PATCH 4/4] address review comments --- .../LifetimeSafety/FactsGenerator.cpp | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 0719915144eea..0b76428c86731 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -368,25 +368,26 @@ void FactsGenerator::handleAssignment(const Expr *LHSExpr, if (const auto *DRE_LHS = dyn_cast<DeclRefExpr>(LHSExpr)) { QualType QT = DRE_LHS->getDecl()->getType(); - if (QT->isReferenceType() && hasOrigins(QT->getPointeeType())) { - // Writing through a reference uses the binding but overwrites the - // pointee. Model this as a Read of the outer origin (keeping the binding - // live) and a Write of the inner origins (killing the pointee's - // liveness). - if (UseFact *UF = UseFacts.lookup(DRE_LHS)) { - const OriginList *FullList = UF->getUsedOrigins(); - assert(FullList); - UF->setUsedOrigins(FactMgr.getOriginMgr().createSingleOriginList( - FullList->getOuterOriginID())); - if (const OriginList *InnerList = FullList->peelOuterOrigin()) { - UseFact *WriteUF = FactMgr.createFact<UseFact>(DRE_LHS, InnerList); - WriteUF->markAsWritten(); - CurrentBlockFacts.push_back(WriteUF); + if (QT->isReferenceType()) { + if (hasOrigins(QT->getPointeeType())) { + // Writing through a reference uses the binding but overwrites the + // pointee. Model this as a Read of the outer origin (keeping the + // binding live) and a Write of the inner origins (killing the pointee's + // liveness). + if (UseFact *UF = UseFacts.lookup(DRE_LHS)) { + const OriginList *FullList = UF->getUsedOrigins(); + assert(FullList); + UF->setUsedOrigins(FactMgr.getOriginMgr().createSingleOriginList( + FullList->getOuterOriginID())); + if (const OriginList *InnerList = FullList->peelOuterOrigin()) { + UseFact *WriteUF = FactMgr.createFact<UseFact>(DRE_LHS, InnerList); + WriteUF->markAsWritten(); + CurrentBlockFacts.push_back(WriteUF); + } } } - } else if (!QT->isReferenceType()) { + } else markUseAsWrite(DRE_LHS); - } } // Kill the old loans of the destination origin and flow the new loans // from the source origin. _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
