https://github.com/suoyuan666 updated https://github.com/llvm/llvm-project/pull/204592
>From 7ef65e032565e5e5589b6fdb5f7d6982ccaedffc Mon Sep 17 00:00:00 2001 From: Yuan Suo <[email protected]> Date: Wed, 17 Jun 2026 18:04:06 +0800 Subject: [PATCH 1/8] [LifetimeSafety] Add multi-block support to buildOriginFlowChain After introducing `buildOriginFlowChain` to use-after-scope diagnostics, it should support multi-block analysis. This also allows it to be reused by other diagnostics. In some loops, UseFact may appear before OriginFlowFact: ```cpp void for_loop_use_before_loop_body(MyObj safe) { MyObj* p = &safe; for (int i = 0; i < 1; ++i) { (void)*p; MyObj s; p = &s; } (void)*p; } ``` So, I remove the `StartPoint` parameter to correctly find the `OriginID`. Signed-off-by: Yuan Suo <[email protected]> --- .../Analysis/Analyses/LifetimeSafety/Facts.h | 1 + .../Analyses/LifetimeSafety/LoanPropagation.h | 7 +- clang/lib/Analysis/LifetimeSafety/Checker.cpp | 6 +- clang/lib/Analysis/LifetimeSafety/Facts.cpp | 15 ++- .../LifetimeSafety/LoanPropagation.cpp | 99 ++++++++++++++----- clang/lib/Sema/SemaLifetimeSafety.h | 4 +- clang/test/Sema/LifetimeSafety/safety-c.c | 4 +- clang/test/Sema/LifetimeSafety/safety.cpp | 33 +++++-- .../unittests/Analysis/LifetimeSafetyTest.cpp | 3 +- 9 files changed, 125 insertions(+), 47 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index 88b509e1b94df..e0ddfe95b00b2 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -374,6 +374,7 @@ class FactManager { /// Retrieves all the facts in the block containing Program Point P. /// \note This is intended for testing only. llvm::ArrayRef<const Fact *> getBlockContaining(ProgramPoint P) const; + std::optional<size_t> getBlockID(ProgramPoint P) const; unsigned getNumFacts() const { return NextFactID.Value; } diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LoanPropagation.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LoanPropagation.h index 724c6eee7d3c2..42d7df0838f35 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LoanPropagation.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LoanPropagation.h @@ -16,6 +16,7 @@ #define LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMESAFETY_LOAN_PROPAGATION_H #include "clang/Analysis/Analyses/LifetimeSafety/Facts.h" +#include "clang/Analysis/Analyses/PostOrderCFGView.h" #include "clang/Analysis/AnalysisDeclContext.h" #include "clang/Analysis/CFG.h" #include "llvm/ADT/ImmutableMap.h" @@ -46,10 +47,12 @@ class LoanPropagationAnalysis { /// where the loan was originally issued. llvm::SmallVector<OriginID> buildOriginFlowChain(ProgramPoint StartPoint, const OriginID StartOID, - const LoanID TargetLoan) const; + const LoanID TargetLoan, + const PostOrderCFGView *POV) const; llvm::SmallVector<OriginID> - buildOriginFlowChain(const UseFact *UF, const LoanID TargetLoan) const; + buildOriginFlowChain(const UseFact *UF, const LoanID TargetLoan, + const PostOrderCFGView *POV) const; private: class Impl; diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index d41d6f43f837b..2fe52585087e4 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -68,6 +68,7 @@ class LifetimeChecker { LifetimeSafetySemaHelper *SemaHelper; ASTContext &AST; const Decl *FD; + const PostOrderCFGView *POV; static SourceLocation GetFactLoc(llvm::PointerUnion<const UseFact *, const OriginEscapesFact *> F) { @@ -90,7 +91,8 @@ class LifetimeChecker { LifetimeSafetySemaHelper *SemaHelper) : LoanPropagation(LoanPropagation), MovedLoans(MovedLoans), LiveOrigins(LiveOrigins), FactMgr(FM), SemaHelper(SemaHelper), - AST(ADC.getASTContext()), FD(ADC.getDecl()) { + AST(ADC.getASTContext()), FD(ADC.getDecl()), + POV(ADC.getAnalysis<PostOrderCFGView>()) { for (const CFGBlock *B : *ADC.getAnalysis<PostOrderCFGView>()) for (const Fact *F : FactMgr.getFacts(B)) if (const auto *EF = F->getAs<ExpireFact>()) @@ -268,7 +270,7 @@ class LifetimeChecker { // Scope-based expiry (use-after-scope). SemaHelper->reportUseAfterScope( IssueExpr, UF->getUseExpr(), MovedExpr, ExpiryLoc, - getExprChain(LoanPropagation.buildOriginFlowChain(UF, LID))); + getExprChain(LoanPropagation.buildOriginFlowChain(UF, LID, POV))); } else if (const auto *OEF = CausingFact.dyn_cast<const OriginEscapesFact *>()) { diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp index 3d7fbcdacc830..823f1f686f904 100644 --- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp @@ -145,12 +145,17 @@ void FactManager::dump(const CFG &Cfg, AnalysisDeclContext &AC) const { llvm::ArrayRef<const Fact *> FactManager::getBlockContaining(ProgramPoint P) const { - for (const auto &BlockToFactsVec : BlockToFacts) { - for (const Fact *F : BlockToFactsVec) - if (F == P) - return BlockToFactsVec; - } + std::optional<size_t> BlockIndex = getBlockID(P); + if (BlockIndex) + return BlockToFacts[BlockIndex.value()]; return {}; } +std::optional<size_t> FactManager::getBlockID(ProgramPoint P) const { + for (size_t i = 0; i < BlockToFacts.size(); ++i) + for (const Fact *F : BlockToFacts[i]) + if (F == P) + return i; + return std::nullopt; +} } // namespace clang::lifetimes::internal diff --git a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp index a67b1b3c0f826..8e2d9a6577427 100644 --- a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp @@ -130,6 +130,11 @@ struct Lattice { } }; +struct BuildOriginFlowChainResult { + const llvm::SmallVector<OriginID> OriginFlowChain; + const bool Complete; +}; + class AnalysisImpl : public DataflowAnalysis<AnalysisImpl, Lattice, Direction::Forward> { public: @@ -204,22 +209,72 @@ class AnalysisImpl llvm::SmallVector<OriginID> buildOriginFlowChain(ProgramPoint StartPoint, const OriginID StartOID, - const LoanID TargetLoan) const { + const LoanID TargetLoan, + const PostOrderCFGView *POV) const { assert(getLoans(StartOID, StartPoint).contains(TargetLoan) && "TargetLoan must be present in the StartOID at the StartPoint"); + std::optional<OriginID> FinalOID; + llvm::DenseMap<OriginID, OriginID> VistedOriginIDs; + + const auto OriginFlowChainFilter = [&VistedOriginIDs](OriginID FinalOID) { + llvm::SmallVector<OriginID> OriginFlowChain; + while (true) { + OriginFlowChain.push_back(FinalOID); + const auto NextOriginID = VistedOriginIDs.find(FinalOID); + if (NextOriginID == VistedOriginIDs.end()) + break; + FinalOID = NextOriginID->second; + } + return OriginFlowChain; + }; + + const auto InsertVistedOriginIDs = + [&VistedOriginIDs, &FinalOID](llvm::ArrayRef<OriginID> OriginFlowChain, + OriginID &StartOID) { + if (!VistedOriginIDs.empty()) + VistedOriginIDs.insert({OriginFlowChain[0], StartOID}); + + for (size_t i = 0; i < OriginFlowChain.size() - 1; ++i) + VistedOriginIDs.insert( + {OriginFlowChain[i + 1], OriginFlowChain[i]}); + + StartOID = OriginFlowChain.back(); + FinalOID = StartOID; + }; + + std::optional<size_t> BlockID = FactMgr.getBlockID(StartPoint); + assert(BlockID.has_value()); + const auto StartIt = llvm::find_if(*POV, [&BlockID](const CFGBlock *Block) { + return Block->getBlockID() == BlockID; + }); + + OriginID CurrOID = StartOID; + for (const CFGBlock *B : + llvm::reverse(llvm::make_range(POV->begin(), StartIt + 1))) { + BuildOriginFlowChainResult BuildResult = + buildOriginFlowChain(B, CurrOID, TargetLoan); + if (!BuildResult.OriginFlowChain.empty()) + InsertVistedOriginIDs(BuildResult.OriginFlowChain, CurrOID); + if (BuildResult.Complete) + return OriginFlowChainFilter(*FinalOID); + } + + llvm_unreachable( + "buildOriginFlowChain should return at BuildResult.Complete"); + } + + BuildOriginFlowChainResult + buildOriginFlowChain(const CFGBlock *Block, const OriginID StartOID, + const LoanID TargetLoan) const { OriginID CurrOID = StartOID; llvm::SmallVector<OriginID> OriginFlowChain; - llvm::ArrayRef<const Fact *> Facts = FactMgr.getBlockContaining(StartPoint); - const auto *StartIt = llvm::find(Facts, StartPoint); - assert(StartIt != Facts.end()); - for (const Fact *F : - llvm::reverse(llvm::make_range(Facts.begin(), StartIt))) { + for (const Fact *F : llvm::reverse(FactMgr.getFacts(Block))) { if (const auto *IF = F->getAs<IssueFact>()) if (IF->getLoanID() == TargetLoan) { assert(IF->getOriginID() == CurrOID); - return OriginFlowChain; + return {OriginFlowChain, true}; } const auto *OFF = F->getAs<OriginFlowFact>(); @@ -235,20 +290,17 @@ class AnalysisImpl CurrOID = SrcOriginID; } - // FIXME: Ideally, this return is unreachable and should be an assert - // because we expect to always finish at an IssueFact. But since current - // traversal is limited to a single CFG block, multi-block OriginFlowChain - // construction might miss the IssueFact. We should add llvm_unreachable - // here once multi-block support is implemented. - return {}; + return {OriginFlowChain, false}; } llvm::SmallVector<OriginID> - buildOriginFlowChain(const UseFact *UF, const LoanID TargetLoan) const { + buildOriginFlowChain(const UseFact *UF, const LoanID TargetLoan, + const PostOrderCFGView *POV) const { for (const OriginList *Cur = UF->getUsedOrigins(); Cur; Cur = Cur->peelOuterOrigin()) if (getLoans(Cur->getOuterOriginID(), UF).contains(TargetLoan)) - return buildOriginFlowChain(UF, Cur->getOuterOriginID(), TargetLoan); + return buildOriginFlowChain(UF, Cur->getOuterOriginID(), TargetLoan, + POV); return {}; } @@ -303,16 +355,15 @@ LoanSet LoanPropagationAnalysis::getLoans(OriginID OID, ProgramPoint P) const { return PImpl->getLoans(OID, P); } -llvm::SmallVector<OriginID> -LoanPropagationAnalysis::buildOriginFlowChain(ProgramPoint StartPoint, - const OriginID StartOID, - const LoanID TargetLoan) const { - return PImpl->buildOriginFlowChain(StartPoint, StartOID, TargetLoan); +llvm::SmallVector<OriginID> LoanPropagationAnalysis::buildOriginFlowChain( + ProgramPoint StartPoint, const OriginID StartOID, const LoanID TargetLoan, + const PostOrderCFGView *POV) const { + return PImpl->buildOriginFlowChain(StartPoint, StartOID, TargetLoan, POV); } -llvm::SmallVector<OriginID> -LoanPropagationAnalysis::buildOriginFlowChain(const UseFact *UF, - const LoanID TargetLoan) const { - return PImpl->buildOriginFlowChain(UF, TargetLoan); +llvm::SmallVector<OriginID> LoanPropagationAnalysis::buildOriginFlowChain( + const UseFact *UF, const LoanID TargetLoan, + const PostOrderCFGView *POV) const { + return PImpl->buildOriginFlowChain(UF, TargetLoan, POV); } } // namespace clang::lifetimes::internal diff --git a/clang/lib/Sema/SemaLifetimeSafety.h b/clang/lib/Sema/SemaLifetimeSafety.h index a8bde363e3397..f4649019ceb1d 100644 --- a/clang/lib/Sema/SemaLifetimeSafety.h +++ b/clang/lib/Sema/SemaLifetimeSafety.h @@ -550,10 +550,10 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { if (OriginExprChain.empty()) return; - const Expr *LastExpr = OriginExprChain.back(); + const Expr *LastExpr = OriginExprChain.front(); std::string IssueStr = getDiagSubjectDescription(LastExpr); - for (const Expr *CurrExpr : reverse(OriginExprChain.drop_back())) { + for (const Expr *CurrExpr : OriginExprChain.drop_front()) { if (!shouldShowInAliasChain(CurrExpr, LastExpr)) continue; S.Diag(CurrExpr->getBeginLoc(), diff --git a/clang/test/Sema/LifetimeSafety/safety-c.c b/clang/test/Sema/LifetimeSafety/safety-c.c index 95c8cf7bb00c7..c87a9b4768f2c 100644 --- a/clang/test/Sema/LifetimeSafety/safety-c.c +++ b/clang/test/Sema/LifetimeSafety/safety-c.c @@ -93,7 +93,9 @@ void conditional_operator_lifetimebound(int cond) { int *p; { int a, b; - p = identity(cond ? &a // expected-warning {{local variable 'a' does not live long enough}} + p = identity(cond ? &a // expected-warning {{local variable 'a' does not live long enough}} \ + // expected-note {{result of call to 'identity' aliases the storage of local variable 'a'}} \ + // expected-note {{result of call to 'identity' aliases the storage of local variable 'b'}} : &b); // expected-warning {{local variable 'b' does not live long enough}} } // expected-note 2 {{destroyed here}} (void)*p; // expected-note 2 {{later used here}} diff --git a/clang/test/Sema/LifetimeSafety/safety.cpp b/clang/test/Sema/LifetimeSafety/safety.cpp index 6fc275b51a9d0..51b042afa8760 100644 --- a/clang/test/Sema/LifetimeSafety/safety.cpp +++ b/clang/test/Sema/LifetimeSafety/safety.cpp @@ -236,7 +236,7 @@ void overrides_potential(bool cond) { { MyObj s; q = &s; // expected-warning {{does not live long enough}} - p = q; + p = q; // expected-note {{local variable 'q' aliases the storage of local variable 's'}} } // expected-note {{destroyed here}} if (cond) { @@ -448,7 +448,7 @@ void loan_from_previous_iteration(MyObj safe, bool condition) { p = &x; // expected-warning {{does not live long enough}} if (condition) - q = p; + q = p; // expected-note {{local variable 'p' aliases the storage of local variable 'x'}} (void)*p; (void)*q; // expected-note {{later used here}} } // expected-note {{destroyed here}} @@ -845,7 +845,8 @@ void lifetimebound_multiple_args_potential(bool cond) { MyObj obj1; if (cond) { MyObj obj2; - v = Choose(true, + v = Choose(true, // expected-note {{result of call to 'Choose' aliases the storage of local variable 'obj1'}} \ + // expected-note {{result of call to 'Choose' aliases the storage of local variable 'obj2'}} obj1, // expected-warning {{local variable 'obj1' does not live long enough}} obj2); // expected-warning {{local variable 'obj2' does not live long enough}} } // expected-note {{destroyed here}} @@ -939,7 +940,7 @@ void lifetimebound_partial_safety(bool cond) { if (cond) { MyObj temp_obj; - v = Choose(true, + v = Choose(true, // expected-note {{result of call to 'Choose' aliases the storage of local variable 'temp_obj'}} safe_obj, temp_obj); // expected-warning {{local variable 'temp_obj' does not live long enough}} } // expected-note {{destroyed here}} @@ -1222,7 +1223,9 @@ void conditional_operator_lifetimebound(bool cond) { MyObj* p; { MyObj a, b; - p = Identity(cond ? &a // expected-warning {{local variable 'a' does not live long enough}} + p = Identity(cond ? &a // expected-warning {{local variable 'a' does not live long enough}} \ + // expected-note {{result of call to 'Identity' aliases the storage of local variable 'a'}} \ + // expected-note {{result of call to 'Identity' aliases the storage of local variable 'b'}} : &b); // expected-warning {{local variable 'b' does not live long enough}} } // expected-note 2 {{destroyed here}} (void)*p; // expected-note 2 {{later used here}} @@ -1232,8 +1235,11 @@ void conditional_operator_lifetimebound_nested(bool cond) { MyObj* p; { MyObj a, b; - p = Identity(cond ? Identity(&a) // expected-warning {{local variable 'a' does not live long enough}} - : Identity(&b)); // expected-warning {{local variable 'b' does not live long enough}} + p = Identity(cond ? Identity(&a) // expected-warning {{local variable 'a' does not live long enough}} \ + // expected-note 2 {{result of call to 'Identity' aliases the storage of local variable 'a'}} \ + // expected-note {{result of call to 'Identity' aliases the storage of local variable 'b'}} + : Identity(&b)); // expected-warning {{local variable 'b' does not live long enough}} \ + // expected-note {{result of call to 'Identity' aliases the storage of local variable 'b'}} } // expected-note 2 {{destroyed here}} (void)*p; // expected-note 2 {{later used here}} } @@ -1242,9 +1248,15 @@ void conditional_operator_lifetimebound_nested_deep(bool cond) { MyObj* p; { MyObj a, b, c, d; - p = Identity(cond ? Identity(cond ? &a // expected-warning {{local variable 'a' does not live long enough}} + p = Identity(cond ? Identity(cond ? &a // expected-warning {{local variable 'a' does not live long enough}} \ + // expected-note 2 {{result of call to 'Identity' aliases the storage of local variable 'a'}} \ + // expected-note 2 {{result of call to 'Identity' aliases the storage of local variable 'b'}} \ + // expected-note {{result of call to 'Identity' aliases the storage of local variable 'c'}} \ + // expected-note {{result of call to 'Identity' aliases the storage of local variable 'd'}} : &b) // expected-warning {{local variable 'b' does not live long enough}} - : Identity(cond ? &c // expected-warning {{local variable 'c' does not live long enough}} + : Identity(cond ? &c // expected-warning {{local variable 'c' does not live long enough}} \ + // expected-note {{result of call to 'Identity' aliases the storage of local variable 'c'}} \ + // expected-note {{result of call to 'Identity' aliases the storage of local variable 'd'}} : &d)); // expected-warning {{local variable 'd' does not live long enough}} } // expected-note 4 {{destroyed here}} (void)*p; // expected-note 4 {{later used here}} @@ -1428,7 +1440,8 @@ void range_based_for_use_after_scope() { View v; { MyObjStorage s; - for (const MyObj &o : s) { // expected-warning {{local variable 's' does not live long enough}} + for (const MyObj &o : s) { // expected-warning {{local variable 's' does not live long enough}} \ + // expected-note {{local variable '__range2' aliases the storage of local variable 's'}} v = o; } } // expected-note {{destroyed here}} diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp b/clang/unittests/Analysis/LifetimeSafetyTest.cpp index 78b7449958140..8d5ed9c88dcff 100644 --- a/clang/unittests/Analysis/LifetimeSafetyTest.cpp +++ b/clang/unittests/Analysis/LifetimeSafetyTest.cpp @@ -215,7 +215,8 @@ class LifetimeTestHelper { for (LoanID LID : EndLoanIDs) { llvm::SmallVector<OriginID> OriginFlowChain = Runner.getAnalysis().getLoanPropagation().buildOriginFlowChain( - getProgramPoint(Annotation), *StartOriginID, LID); + getProgramPoint(Annotation), *StartOriginID, LID, + Runner.getAnalysisContext().getAnalysis<PostOrderCFGView>()); if (!OriginFlowChain.empty()) return OriginFlowChain; } >From 3a7b00c0333abc25f1f7a7e140fae90cafeafbf3 Mon Sep 17 00:00:00 2001 From: Yuan Suo <[email protected]> Date: Fri, 19 Jun 2026 15:25:42 +0800 Subject: [PATCH 2/8] Add return after llvm_unreachable and clean up code Signed-off-by: Yuan Suo <[email protected]> --- .../LifetimeSafety/LoanPropagation.cpp | 104 +++++++----------- clang/lib/Sema/SemaLifetimeSafety.h | 4 +- 2 files changed, 42 insertions(+), 66 deletions(-) diff --git a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp index 8e2d9a6577427..59081522b40f7 100644 --- a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp @@ -214,35 +214,7 @@ class AnalysisImpl assert(getLoans(StartOID, StartPoint).contains(TargetLoan) && "TargetLoan must be present in the StartOID at the StartPoint"); - std::optional<OriginID> FinalOID; - llvm::DenseMap<OriginID, OriginID> VistedOriginIDs; - - const auto OriginFlowChainFilter = [&VistedOriginIDs](OriginID FinalOID) { - llvm::SmallVector<OriginID> OriginFlowChain; - while (true) { - OriginFlowChain.push_back(FinalOID); - const auto NextOriginID = VistedOriginIDs.find(FinalOID); - if (NextOriginID == VistedOriginIDs.end()) - break; - FinalOID = NextOriginID->second; - } - return OriginFlowChain; - }; - - const auto InsertVistedOriginIDs = - [&VistedOriginIDs, &FinalOID](llvm::ArrayRef<OriginID> OriginFlowChain, - OriginID &StartOID) { - if (!VistedOriginIDs.empty()) - VistedOriginIDs.insert({OriginFlowChain[0], StartOID}); - - for (size_t i = 0; i < OriginFlowChain.size() - 1; ++i) - VistedOriginIDs.insert( - {OriginFlowChain[i + 1], OriginFlowChain[i]}); - - StartOID = OriginFlowChain.back(); - FinalOID = StartOID; - }; - + llvm::SmallVector<OriginID> OriginFlowChain; std::optional<size_t> BlockID = FactMgr.getBlockID(StartPoint); assert(BlockID.has_value()); const auto StartIt = llvm::find_if(*POV, [&BlockID](const CFGBlock *Block) { @@ -252,45 +224,20 @@ class AnalysisImpl OriginID CurrOID = StartOID; for (const CFGBlock *B : llvm::reverse(llvm::make_range(POV->begin(), StartIt + 1))) { - BuildOriginFlowChainResult BuildResult = - buildOriginFlowChain(B, CurrOID, TargetLoan); - if (!BuildResult.OriginFlowChain.empty()) - InsertVistedOriginIDs(BuildResult.OriginFlowChain, CurrOID); - if (BuildResult.Complete) - return OriginFlowChainFilter(*FinalOID); - } - - llvm_unreachable( - "buildOriginFlowChain should return at BuildResult.Complete"); - } - - BuildOriginFlowChainResult - buildOriginFlowChain(const CFGBlock *Block, const OriginID StartOID, - const LoanID TargetLoan) const { - OriginID CurrOID = StartOID; - llvm::SmallVector<OriginID> OriginFlowChain; + auto [OFChain, Complete] = buildOriginFlowChain(B, CurrOID, TargetLoan); - for (const Fact *F : llvm::reverse(FactMgr.getFacts(Block))) { - if (const auto *IF = F->getAs<IssueFact>()) - if (IF->getLoanID() == TargetLoan) { - assert(IF->getOriginID() == CurrOID); - return {OriginFlowChain, true}; - } - - const auto *OFF = F->getAs<OriginFlowFact>(); - if (!OFF) - continue; - if (OFF->getDestOriginID() != CurrOID) - continue; + if (!OFChain.empty()) { + OriginFlowChain.append(OFChain.begin(), OFChain.end()); + CurrOID = OFChain.back(); + } - const OriginID SrcOriginID = OFF->getSrcOriginID(); - if (!getLoans(SrcOriginID, OFF).contains(TargetLoan)) - continue; - OriginFlowChain.push_back(SrcOriginID); - CurrOID = SrcOriginID; + if (Complete) + return OriginFlowChain; } - return {OriginFlowChain, false}; + llvm_unreachable( + "buildOriginFlowChain should return at BuildResult.Complete"); + return OriginFlowChain; } llvm::SmallVector<OriginID> @@ -327,6 +274,35 @@ class AnalysisImpl return LoanSetFactory.getEmptySet(); } + BuildOriginFlowChainResult + buildOriginFlowChain(const CFGBlock *Block, const OriginID StartOID, + const LoanID TargetLoan) const { + OriginID CurrOID = StartOID; + llvm::SmallVector<OriginID> OriginFlowChain; + + for (const Fact *F : llvm::reverse(FactMgr.getFacts(Block))) { + if (const auto *IF = F->getAs<IssueFact>()) + if (IF->getLoanID() == TargetLoan) { + assert(IF->getOriginID() == CurrOID); + return {OriginFlowChain, true}; + } + + const auto *OFF = F->getAs<OriginFlowFact>(); + if (!OFF) + continue; + if (OFF->getDestOriginID() != CurrOID) + continue; + + const OriginID SrcOriginID = OFF->getSrcOriginID(); + if (!getLoans(SrcOriginID, OFF).contains(TargetLoan)) + continue; + OriginFlowChain.push_back(SrcOriginID); + CurrOID = SrcOriginID; + } + + return {OriginFlowChain, false}; + } + OriginLoanMap::Factory &OriginLoanMapFactory; LoanSet::Factory &LoanSetFactory; /// Boolean vector indexed by origin ID. If true, the origin appears in diff --git a/clang/lib/Sema/SemaLifetimeSafety.h b/clang/lib/Sema/SemaLifetimeSafety.h index f4649019ceb1d..bbc7c77de4d31 100644 --- a/clang/lib/Sema/SemaLifetimeSafety.h +++ b/clang/lib/Sema/SemaLifetimeSafety.h @@ -550,10 +550,10 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { if (OriginExprChain.empty()) return; - const Expr *LastExpr = OriginExprChain.front(); + const Expr *LastExpr = OriginExprChain.back(); std::string IssueStr = getDiagSubjectDescription(LastExpr); - for (const Expr *CurrExpr : OriginExprChain.drop_front()) { + for (const Expr *CurrExpr : llvm::reverse(OriginExprChain.drop_back())) { if (!shouldShowInAliasChain(CurrExpr, LastExpr)) continue; S.Diag(CurrExpr->getBeginLoc(), >From 04a2a91fc9b604bec0080d847db8926a7eaa9e8f Mon Sep 17 00:00:00 2001 From: Yuan Suo <[email protected]> Date: Fri, 19 Jun 2026 15:49:18 +0800 Subject: [PATCH 3/8] Fix issues from previous commit Signed-off-by: Yuan Suo <[email protected]> --- clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp | 9 ++------- clang/lib/Sema/SemaLifetimeSafety.h | 2 +- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp index 59081522b40f7..d8b41f478b878 100644 --- a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp @@ -130,11 +130,6 @@ struct Lattice { } }; -struct BuildOriginFlowChainResult { - const llvm::SmallVector<OriginID> OriginFlowChain; - const bool Complete; -}; - class AnalysisImpl : public DataflowAnalysis<AnalysisImpl, Lattice, Direction::Forward> { public: @@ -237,7 +232,7 @@ class AnalysisImpl llvm_unreachable( "buildOriginFlowChain should return at BuildResult.Complete"); - return OriginFlowChain; + return {}; } llvm::SmallVector<OriginID> @@ -274,7 +269,7 @@ class AnalysisImpl return LoanSetFactory.getEmptySet(); } - BuildOriginFlowChainResult + std::pair<llvm::SmallVector<OriginID>, bool> buildOriginFlowChain(const CFGBlock *Block, const OriginID StartOID, const LoanID TargetLoan) const { OriginID CurrOID = StartOID; diff --git a/clang/lib/Sema/SemaLifetimeSafety.h b/clang/lib/Sema/SemaLifetimeSafety.h index bbc7c77de4d31..a8bde363e3397 100644 --- a/clang/lib/Sema/SemaLifetimeSafety.h +++ b/clang/lib/Sema/SemaLifetimeSafety.h @@ -553,7 +553,7 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { const Expr *LastExpr = OriginExprChain.back(); std::string IssueStr = getDiagSubjectDescription(LastExpr); - for (const Expr *CurrExpr : llvm::reverse(OriginExprChain.drop_back())) { + for (const Expr *CurrExpr : reverse(OriginExprChain.drop_back())) { if (!shouldShowInAliasChain(CurrExpr, LastExpr)) continue; S.Diag(CurrExpr->getBeginLoc(), >From f89f56512d842fbe7a50422bafa8bb555d43be64 Mon Sep 17 00:00:00 2001 From: Yuan Suo <[email protected]> Date: Fri, 19 Jun 2026 15:52:35 +0800 Subject: [PATCH 4/8] Reanme StartIt to EndBlockIt Signed-off-by: Yuan Suo <[email protected]> --- clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp index d8b41f478b878..3424bbc80a14f 100644 --- a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp @@ -212,13 +212,13 @@ class AnalysisImpl llvm::SmallVector<OriginID> OriginFlowChain; std::optional<size_t> BlockID = FactMgr.getBlockID(StartPoint); assert(BlockID.has_value()); - const auto StartIt = llvm::find_if(*POV, [&BlockID](const CFGBlock *Block) { + const auto EndBlockIt = llvm::find_if(*POV, [&BlockID](const CFGBlock *Block) { return Block->getBlockID() == BlockID; }); OriginID CurrOID = StartOID; for (const CFGBlock *B : - llvm::reverse(llvm::make_range(POV->begin(), StartIt + 1))) { + llvm::reverse(llvm::make_range(POV->begin(), EndBlockIt + 1))) { auto [OFChain, Complete] = buildOriginFlowChain(B, CurrOID, TargetLoan); if (!OFChain.empty()) { >From 36629d5474c1767127b44ced1353ff7ff364e5fc Mon Sep 17 00:00:00 2001 From: Yuan Suo <[email protected]> Date: Fri, 19 Jun 2026 15:55:02 +0800 Subject: [PATCH 5/8] Fix clang-format error Signed-off-by: Yuan Suo <[email protected]> --- clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp index 3424bbc80a14f..f6d620d6520ca 100644 --- a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp @@ -212,9 +212,10 @@ class AnalysisImpl llvm::SmallVector<OriginID> OriginFlowChain; std::optional<size_t> BlockID = FactMgr.getBlockID(StartPoint); assert(BlockID.has_value()); - const auto EndBlockIt = llvm::find_if(*POV, [&BlockID](const CFGBlock *Block) { - return Block->getBlockID() == BlockID; - }); + const auto EndBlockIt = + llvm::find_if(*POV, [&BlockID](const CFGBlock *Block) { + return Block->getBlockID() == BlockID; + }); OriginID CurrOID = StartOID; for (const CFGBlock *B : >From dabbdfbb778b269551dbc42869e22c37983a223d Mon Sep 17 00:00:00 2001 From: Yuan Suo <[email protected]> Date: Sat, 20 Jun 2026 15:29:12 +0800 Subject: [PATCH 6/8] Remove return after llvm_unreachable add llvm_unreachable in getBlockID Also add a unit test for buildOriginFlowChain Signed-off-by: Yuan Suo <[email protected]> --- .../Analysis/Analyses/LifetimeSafety/Facts.h | 2 +- clang/lib/Analysis/LifetimeSafety/Facts.cpp | 9 ++-- .../LifetimeSafety/LoanPropagation.cpp | 6 +-- .../unittests/Analysis/LifetimeSafetyTest.cpp | 44 +++++++++++++++++++ 4 files changed, 50 insertions(+), 11 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index e0ddfe95b00b2..e1380a6cf195c 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -374,7 +374,7 @@ class FactManager { /// Retrieves all the facts in the block containing Program Point P. /// \note This is intended for testing only. llvm::ArrayRef<const Fact *> getBlockContaining(ProgramPoint P) const; - std::optional<size_t> getBlockID(ProgramPoint P) const; + size_t getBlockID(ProgramPoint P) const; unsigned getNumFacts() const { return NextFactID.Value; } diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp index 823f1f686f904..08e7c6cd62345 100644 --- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp @@ -145,17 +145,14 @@ void FactManager::dump(const CFG &Cfg, AnalysisDeclContext &AC) const { llvm::ArrayRef<const Fact *> FactManager::getBlockContaining(ProgramPoint P) const { - std::optional<size_t> BlockIndex = getBlockID(P); - if (BlockIndex) - return BlockToFacts[BlockIndex.value()]; - return {}; + return BlockToFacts[getBlockID(P)]; } -std::optional<size_t> FactManager::getBlockID(ProgramPoint P) const { +size_t FactManager::getBlockID(ProgramPoint P) const { for (size_t i = 0; i < BlockToFacts.size(); ++i) for (const Fact *F : BlockToFacts[i]) if (F == P) return i; - return std::nullopt; + llvm_unreachable("Failed to find BlockID for given ProgramPoint"); } } // namespace clang::lifetimes::internal diff --git a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp index f6d620d6520ca..385bf5f8a192f 100644 --- a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp @@ -210,8 +210,7 @@ class AnalysisImpl "TargetLoan must be present in the StartOID at the StartPoint"); llvm::SmallVector<OriginID> OriginFlowChain; - std::optional<size_t> BlockID = FactMgr.getBlockID(StartPoint); - assert(BlockID.has_value()); + size_t BlockID = FactMgr.getBlockID(StartPoint); const auto EndBlockIt = llvm::find_if(*POV, [&BlockID](const CFGBlock *Block) { return Block->getBlockID() == BlockID; @@ -232,8 +231,7 @@ class AnalysisImpl } llvm_unreachable( - "buildOriginFlowChain should return at BuildResult.Complete"); - return {}; + "buildOriginFlowChain did not reach IssueFact for TargetLoan"); } llvm::SmallVector<OriginID> diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp b/clang/unittests/Analysis/LifetimeSafetyTest.cpp index 8d5ed9c88dcff..153225dcdbf15 100644 --- a/clang/unittests/Analysis/LifetimeSafetyTest.cpp +++ b/clang/unittests/Analysis/LifetimeSafetyTest.cpp @@ -2078,5 +2078,49 @@ TEST_F(LifetimeAnalysisTest, BuildOriginFlowChainWithLifetimeBound) { EXPECT_THAT(ChainForTgtB, Contains(*Helper->getOriginForDecl("result"))); EXPECT_THAT(ChainForTgtB, Not(Contains(*Helper->getOriginForDecl("a")))); } + +TEST_F(LifetimeAnalysisTest, BuildOriginFlowChainInMultiBlock) { + SetupTest(R"( + void target(bool c1, bool c2) { + int *s; + int *a, *b, *c; + + { + int tgta, tgtb, tgtc; + a = &tgta; + b = &tgtb; + c = &tgtc; + } + + if (c1) { + s = c2 ? a : b; + } else { + s = c; + } + + POINT(after_nested_merge); + (void)*s; + } + )"); + + llvm::SmallVector<OriginID> ChainForTgtA = + Helper->buildOriginFlowChainInOneBlock("s", "tgta", "after_nested_merge"); + llvm::SmallVector<OriginID> ChainForTgtB = + Helper->buildOriginFlowChainInOneBlock("s", "tgtb", "after_nested_merge"); + llvm::SmallVector<OriginID> ChainForTgtC = + Helper->buildOriginFlowChainInOneBlock("s", "tgtc", "after_nested_merge"); + + EXPECT_THAT(ChainForTgtA, Contains(*Helper->getOriginForDecl("a"))); + EXPECT_THAT(ChainForTgtA, Not(Contains(*Helper->getOriginForDecl("b")))); + EXPECT_THAT(ChainForTgtA, Not(Contains(*Helper->getOriginForDecl("c")))); + + EXPECT_THAT(ChainForTgtB, Contains(*Helper->getOriginForDecl("b"))); + EXPECT_THAT(ChainForTgtB, Not(Contains(*Helper->getOriginForDecl("a")))); + EXPECT_THAT(ChainForTgtB, Not(Contains(*Helper->getOriginForDecl("c")))); + + EXPECT_THAT(ChainForTgtC, Contains(*Helper->getOriginForDecl("c"))); + EXPECT_THAT(ChainForTgtC, Not(Contains(*Helper->getOriginForDecl("b")))); + EXPECT_THAT(ChainForTgtC, Not(Contains(*Helper->getOriginForDecl("a")))); +} } // anonymous namespace } // namespace clang::lifetimes::internal >From 532222aaa70c200e9895aba5e7b0346987dcd09e Mon Sep 17 00:00:00 2001 From: Yuan Suo <[email protected]> Date: Sat, 20 Jun 2026 16:17:23 +0800 Subject: [PATCH 7/8] Add Decl->isImplicit() in shouldShowInAliasChain Signed-off-by: Yuan Suo <[email protected]> --- clang/lib/Sema/SemaLifetimeSafety.h | 15 ++++++++++++++- clang/test/Sema/LifetimeSafety/safety.cpp | 5 ++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/clang/lib/Sema/SemaLifetimeSafety.h b/clang/lib/Sema/SemaLifetimeSafety.h index a8bde363e3397..8e48574e2512f 100644 --- a/clang/lib/Sema/SemaLifetimeSafety.h +++ b/clang/lib/Sema/SemaLifetimeSafety.h @@ -534,11 +534,24 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { return "expression"; } + bool isInValidExpr(const Expr *E) { + if (const auto *DRE = dyn_cast<DeclRefExpr>(E)) { + return !DRE->getDecl()->isImplicit(); + } + + if (const auto *CE = dyn_cast<CallExpr>(E)) { + if (const auto *FE = CE->getDirectCallee()) + return !FE->isImplicit(); + } + + return false; + } + bool shouldShowInAliasChain(const Expr *CurrExpr, const Expr *LastExpr) { CurrExpr = CurrExpr->IgnoreImpCasts(); LastExpr = LastExpr->IgnoreImpCasts(); - if (!isa<CallExpr, DeclRefExpr>(CurrExpr)) + if (!isInValidExpr(CurrExpr)) return false; // Source ranges can be used to filter out many implicit expressions, // because operations between class objects often involve numerous implicit diff --git a/clang/test/Sema/LifetimeSafety/safety.cpp b/clang/test/Sema/LifetimeSafety/safety.cpp index 51b042afa8760..65b60eb2afb72 100644 --- a/clang/test/Sema/LifetimeSafety/safety.cpp +++ b/clang/test/Sema/LifetimeSafety/safety.cpp @@ -1441,7 +1441,7 @@ void range_based_for_use_after_scope() { { MyObjStorage s; for (const MyObj &o : s) { // expected-warning {{local variable 's' does not live long enough}} \ - // expected-note {{local variable '__range2' aliases the storage of local variable 's'}} + // expected-note {{result of call to 'begin' aliases the storage of local variable 's'}} v = o; } } // expected-note {{destroyed here}} @@ -2529,8 +2529,7 @@ void chained_defaulted_assignment_propagation() { std::string str{"abc"}; S a = getS(str); // expected-warning {{local variable 'str' does not live long enough}} \ // expected-note {{result of call to 'getS' aliases the storage of local variable 'str'}} - c = b = a; // expected-note {{local variable 'a' aliases the storage of local variable 'str'}}\ - // expected-note {{expression aliases the storage of local variable 'str'}} + c = b = a; // expected-note {{local variable 'a' aliases the storage of local variable 'str'}} } // expected-note {{destroyed here}} use(c); // expected-note {{later used here}} } >From 92ae44d0c4726aa3f88c485443ea30350f23f1e3 Mon Sep 17 00:00:00 2001 From: Yuan Suo <[email protected]> Date: Sat, 20 Jun 2026 16:55:16 +0800 Subject: [PATCH 8/8] Fix inconsistent semantics in isInValidExpr Signed-off-by: Yuan Suo <[email protected]> --- clang/lib/Sema/SemaLifetimeSafety.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clang/lib/Sema/SemaLifetimeSafety.h b/clang/lib/Sema/SemaLifetimeSafety.h index 8e48574e2512f..817460408c31d 100644 --- a/clang/lib/Sema/SemaLifetimeSafety.h +++ b/clang/lib/Sema/SemaLifetimeSafety.h @@ -536,22 +536,22 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { bool isInValidExpr(const Expr *E) { if (const auto *DRE = dyn_cast<DeclRefExpr>(E)) { - return !DRE->getDecl()->isImplicit(); + return DRE->getDecl()->isImplicit(); } if (const auto *CE = dyn_cast<CallExpr>(E)) { if (const auto *FE = CE->getDirectCallee()) - return !FE->isImplicit(); + return FE->isImplicit(); } - return false; + return true; } bool shouldShowInAliasChain(const Expr *CurrExpr, const Expr *LastExpr) { CurrExpr = CurrExpr->IgnoreImpCasts(); LastExpr = LastExpr->IgnoreImpCasts(); - if (!isInValidExpr(CurrExpr)) + if (isInValidExpr(CurrExpr)) return false; // Source ranges can be used to filter out many implicit expressions, // because operations between class objects often involve numerous implicit _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
