https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/178670
>From 5342e96d8151d47dd1156c24020f3a78a024f114 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <[email protected]> Date: Thu, 29 Jan 2026 08:26:16 +0000 Subject: [PATCH 1/2] Revisit handling moved origins --- .../Analyses/LifetimeSafety/Checker.h | 3 +- .../Analysis/Analyses/LifetimeSafety/Facts.h | 23 ++++ .../Analyses/LifetimeSafety/FactsGenerator.h | 19 +-- .../Analyses/LifetimeSafety/LifetimeSafety.h | 8 +- .../Analysis/Analyses/LifetimeSafety/Loans.h | 7 + .../Analyses/LifetimeSafety/MovedLoans.h | 45 +++++++ clang/include/clang/Basic/DiagnosticGroups.td | 10 +- .../clang/Basic/DiagnosticSemaKinds.td | 17 ++- .../Analysis/LifetimeSafety/CMakeLists.txt | 1 + clang/lib/Analysis/LifetimeSafety/Checker.cpp | 57 +++++++-- clang/lib/Analysis/LifetimeSafety/Dataflow.h | 3 + clang/lib/Analysis/LifetimeSafety/Facts.cpp | 7 + .../LifetimeSafety/FactsGenerator.cpp | 56 ++++---- .../LifetimeSafety/LifetimeSafety.cpp | 28 ++-- .../LifetimeSafety/LoanPropagation.cpp | 1 + .../Analysis/LifetimeSafety/MovedLoans.cpp | 121 ++++++++++++++++++ clang/lib/Sema/AnalysisBasedWarnings.cpp | 35 +++-- clang/test/Sema/Inputs/lifetime-analysis.h | 4 + .../Sema/warn-lifetime-analysis-nocfg.cpp | 21 +-- .../warn-lifetime-safety-dangling-field.cpp | 29 +++-- clang/test/Sema/warn-lifetime-safety.cpp | 54 +++++--- 21 files changed, 443 insertions(+), 106 deletions(-) create mode 100644 clang/include/clang/Analysis/Analyses/LifetimeSafety/MovedLoans.h create mode 100644 clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Checker.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Checker.h index 5c631c92c0167..1ad3f9da61f60 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Checker.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Checker.h @@ -26,8 +26,9 @@ namespace clang::lifetimes::internal { /// examining loan expiration points and checking if any live origins hold /// the expired loan. void runLifetimeChecker(const LoanPropagationAnalysis &LoanPropagation, + const MovedLoansAnalysis &MovedLoans, const LiveOriginsAnalysis &LiveOrigins, - const FactManager &FactMgr, AnalysisDeclContext &ADC, + FactManager &FactMgr, AnalysisDeclContext &ADC, LifetimeSafetySemaHelper *SemaHelper); } // namespace clang::lifetimes::internal diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index d948965af34d5..f2c72d6955930 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -44,6 +44,8 @@ class Fact { OriginFlow, /// An origin is used (eg. appears as l-value expression like DeclRefExpr). Use, + /// An origin that is moved (e.g., passed to an rvalue reference parameter). + MovedOrigin, /// A marker for a specific point in the code, for testing. TestPoint, /// An origin that escapes the function scope (e.g., via return). @@ -219,6 +221,27 @@ class UseFact : public Fact { const OriginManager &OM) const override; }; +/// Top-level origin of the expression which was found to be moved, e.g, when +/// being used as an argument to an r-value reference parameter. +class MovedOriginFact : public Fact { + const OriginID MovedOrigin; + const Expr *MoveExpr; + +public: + static bool classof(const Fact *F) { + return F->getKind() == Kind::MovedOrigin; + } + + MovedOriginFact(const Expr *MoveExpr, OriginID MovedOrigin) + : Fact(Kind::MovedOrigin), MovedOrigin(MovedOrigin), MoveExpr(MoveExpr) {} + + OriginID getMovedOrigin() const { return MovedOrigin; } + const Expr *getMoveExpr() const { return MoveExpr; } + + void dump(llvm::raw_ostream &OS, const LoanManager &, + const OriginManager &OM) const override; +}; + /// A dummy-fact used to mark a specific point in the code for testing. /// It is generated by recognizing a `void("__lifetime_test_point_...")` cast. class TestPointFact : public Fact { diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h index 8b45337bee218..b05cd2fdca83f 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h @@ -67,6 +67,16 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> { void handleGSLPointerConstruction(const CXXConstructExpr *CCE); + /// Detects arguments passed to rvalue reference parameters and creates + /// MovedOriginFact for them. This is a flow-insensitive approximation: once + /// an origin is moved anywhere in the function, all loans from that origin + /// are marked as potentially moved everywhere. This can lead to false + /// negatives on control flow paths where the value is not actually moved, but + /// these are considered lower priority as they are anyway surfaced as strict + /// warning. + void handleMovedArgsInCall(const FunctionDecl *FD, + ArrayRef<const Expr *> Args); + /// Checks if a call-like expression creates a borrow by passing a value to a /// reference parameter, creating an IssueFact if it does. /// \param IsGslConstruction True if this is a GSL construction where all @@ -110,15 +120,6 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> { // corresponding to the left-hand side is updated to be a "write", thereby // exempting it from the check. llvm::DenseMap<const DeclRefExpr *, UseFact *> UseFacts; - - // This is a flow-insensitive approximation: once a declaration is moved - // anywhere in the function, it's treated as moved everywhere. This can lead - // to false negatives on control flow paths where the value is not actually - // moved, but these are considered lower priority than the false positives - // this tracking prevents. - // TODO: The ideal solution would be flow-sensitive ownership tracking that - // records where values are moved from and to, but this is more complex. - llvm::DenseSet<const ValueDecl *> MovedDecls; }; } // namespace clang::lifetimes::internal diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h index 9f22db20e79b1..6e87951c8961d 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h @@ -24,8 +24,10 @@ #include "clang/Analysis/Analyses/LifetimeSafety/LifetimeStats.h" #include "clang/Analysis/Analyses/LifetimeSafety/LiveOrigins.h" #include "clang/Analysis/Analyses/LifetimeSafety/LoanPropagation.h" +#include "clang/Analysis/Analyses/LifetimeSafety/MovedLoans.h" #include "clang/Analysis/Analyses/LifetimeSafety/Origins.h" #include "clang/Analysis/AnalysisDeclContext.h" +#include <memory> namespace clang::lifetimes { @@ -58,16 +60,18 @@ class LifetimeSafetySemaHelper { virtual ~LifetimeSafetySemaHelper() = default; virtual void reportUseAfterFree(const Expr *IssueExpr, const Expr *UseExpr, - SourceLocation FreeLoc, + const Expr *MovedExpr, SourceLocation FreeLoc, Confidence Confidence) {} virtual void reportUseAfterReturn(const Expr *IssueExpr, const Expr *ReturnExpr, + const Expr *MovedExpr, SourceLocation ExpiryLoc, Confidence Confidence) {} virtual void reportDanglingField(const Expr *IssueExpr, const FieldDecl *Field, + const Expr *MovedExpr, SourceLocation ExpiryLoc) {} // Suggests lifetime bound annotations for function paramters. @@ -107,6 +111,7 @@ void collectLifetimeStats(AnalysisDeclContext &AC, OriginManager &OM, struct LifetimeFactory { OriginLoanMap::Factory OriginMapFactory{/*canonicalize=*/false}; LoanSet::Factory LoanSetFactory{/*canonicalize=*/false}; + MovedLoansMap::Factory MovedLoansMapFactory{/*canonicalize=*/false}; LivenessMap::Factory LivenessMapFactory{/*canonicalize=*/false}; }; @@ -133,6 +138,7 @@ class LifetimeSafetyAnalysis { std::unique_ptr<FactManager> FactMgr; std::unique_ptr<LiveOriginsAnalysis> LiveOrigins; std::unique_ptr<LoanPropagationAnalysis> LoanPropagation; + std::unique_ptr<MovedLoansAnalysis> MovedLoans; }; } // namespace internal } // namespace clang::lifetimes diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h index a366e3e811cbf..29bf4b374667f 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h @@ -52,6 +52,12 @@ struct AccessPath { const clang::MaterializeTemporaryExpr *getAsMaterializeTemporaryExpr() const { return P.dyn_cast<const clang::MaterializeTemporaryExpr *>(); } + + bool operator==(const AccessPath &RHS) const { + return getAsValueDecl() == RHS.getAsValueDecl() && + getAsMaterializeTemporaryExpr() == + RHS.getAsMaterializeTemporaryExpr(); + } }; /// An abstract base class for a single "Loan" which represents lending a @@ -164,6 +170,7 @@ class LoanManager { assert(ID.Value < AllLoans.size()); return AllLoans[ID.Value]; } + llvm::ArrayRef<const Loan *> getLoans() const { return AllLoans; } private: diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/MovedLoans.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/MovedLoans.h new file mode 100644 index 0000000000000..3d8f5b9e177e2 --- /dev/null +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/MovedLoans.h @@ -0,0 +1,45 @@ +//===- MovedLoans.h - Moved Loans Analysis -----------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This file defines the MovedLoansAnalysis, a forward dataflow analysis that +// tracks which loans have been moved out of their original storage location +// at each program point. +// +//===----------------------------------------------------------------------===// +#ifndef LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMESAFETY_MOVED_LOANS_H +#define LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMESAFETY_MOVED_LOANS_H + +#include "clang/Analysis/Analyses/LifetimeSafety/Facts.h" +#include "clang/Analysis/Analyses/LifetimeSafety/LoanPropagation.h" +#include "clang/Analysis/AnalysisDeclContext.h" +#include "clang/Analysis/CFG.h" + +namespace clang::lifetimes::internal { + +// Map from a loan to an expression responsible for moving the borrowed storage. +using MovedLoansMap = llvm::ImmutableMap<LoanID, const Expr *>; + +class MovedLoansAnalysis { +public: + MovedLoansAnalysis(const CFG &C, AnalysisDeclContext &AC, FactManager &F, + const LoanPropagationAnalysis &LoanPropagation, + const LoanManager &LoanMgr, + LoanSet::Factory &LoanSetFactory, + MovedLoansMap::Factory &MovedLoansMapFactory); + ~MovedLoansAnalysis(); + + MovedLoansMap getMovedLoans(ProgramPoint P) const; + +private: + class Impl; + std::unique_ptr<Impl> PImpl; +}; + +} // namespace clang::lifetimes::internal + +#endif // LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMESAFETY_MOVED_LOANS_H diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index d36ee57fe7651..2f128fda5e31f 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -539,9 +539,17 @@ def LifetimeSafetyDanglingField : DiagGroup<"lifetime-safety-dangling-field"> { }]; } +def LifetimeSafetyDanglingFieldStrict : DiagGroup<"lifetime-safety-dangling-field-strict"> { + code Documentation = [{ + Warning to detect dangling field references. + This may contain false-positives, e.g. when the borrowed storage is potentially moved and is not destroyed at function exit. + }]; +} + def LifetimeSafetyPermissive : DiagGroup<"lifetime-safety-permissive", [LifetimeSafetyDanglingField]>; -def LifetimeSafetyStrict : DiagGroup<"lifetime-safety-strict">; +def LifetimeSafetyStrict : DiagGroup<"lifetime-safety-strict", + [LifetimeSafetyDanglingFieldStrict]>; def LifetimeSafety : DiagGroup<"lifetime-safety", [LifetimeSafetyPermissive, LifetimeSafetyStrict]> { diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 807440c107897..c6484295504a4 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10816,13 +10816,19 @@ def warn_lifetime_safety_loan_expires_permissive : Warning< def warn_lifetime_safety_loan_expires_strict : Warning< "object whose reference is captured may not live long enough">, InGroup<LifetimeSafetyStrict>, DefaultIgnore; +def warn_lifetime_safety_loan_expires_moved_strict : Warning< + "object whose reference is captured may not live long enough. " + "This could be false positive as the storage may have been moved later">, + InGroup<LifetimeSafetyStrict>, DefaultIgnore; def warn_lifetime_safety_return_stack_addr_permissive : Warning<"address of stack memory is returned later">, InGroup<LifetimeSafetyPermissive>, DefaultIgnore; -def warn_lifetime_safety_return_stack_addr_strict - : Warning<"address of stack memory may be returned later">, +def warn_lifetime_safety_return_stack_addr_moved_strict + : Warning<"address of stack memory may be returned later. " + "This could be false positive as the storage may have been moved. " + "Consider moving first and then aliasing later to resolve the issue">, InGroup<LifetimeSafetyStrict>, DefaultIgnore; @@ -10830,10 +10836,17 @@ def warn_lifetime_safety_dangling_field : Warning<"address of stack memory escapes to a field">, InGroup<LifetimeSafetyDanglingField>, DefaultIgnore; +def warn_lifetime_safety_dangling_field_moved + : Warning<"address of stack memory escapes to a field. " + "This could be a false positive as the storage may have been moved. " + "Consider moving first and then aliasing later to resolve the issue">, + InGroup<LifetimeSafetyDanglingFieldStrict>, + DefaultIgnore; def note_lifetime_safety_used_here : Note<"later used here">; def note_lifetime_safety_destroyed_here : Note<"destroyed here">; def note_lifetime_safety_returned_here : Note<"returned here">; +def note_lifetime_safety_moved_here : Note<"potentially moved here">; def note_lifetime_safety_dangling_field_here: Note<"this field dangles">; def note_lifetime_safety_escapes_to_field_here: Note<"escapes to this field">; diff --git a/clang/lib/Analysis/LifetimeSafety/CMakeLists.txt b/clang/lib/Analysis/LifetimeSafety/CMakeLists.txt index e5876e747610a..247377c7256d9 100644 --- a/clang/lib/Analysis/LifetimeSafety/CMakeLists.txt +++ b/clang/lib/Analysis/LifetimeSafety/CMakeLists.txt @@ -8,6 +8,7 @@ add_clang_library(clangAnalysisLifetimeSafety LifetimeStats.cpp Loans.cpp LoanPropagation.cpp + MovedLoans.cpp Origins.cpp LINK_LIBS diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index c954a9b14bcdf..1692b5e461b0b 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -47,6 +47,7 @@ namespace { struct PendingWarning { SourceLocation ExpiryLoc; // Where the loan expired. llvm::PointerUnion<const UseFact *, const OriginEscapesFact *> CausingFact; + const Expr *MovedExpr; Confidence ConfidenceLevel; }; @@ -60,18 +61,21 @@ class LifetimeChecker { llvm::DenseMap<AnnotationTarget, const Expr *> AnnotationWarningsMap; llvm::DenseMap<const ParmVarDecl *, EscapingTarget> NoescapeWarningsMap; const LoanPropagationAnalysis &LoanPropagation; + const MovedLoansAnalysis &MovedLoans; const LiveOriginsAnalysis &LiveOrigins; - const FactManager &FactMgr; + FactManager &FactMgr; LifetimeSafetySemaHelper *SemaHelper; ASTContext &AST; public: LifetimeChecker(const LoanPropagationAnalysis &LoanPropagation, - const LiveOriginsAnalysis &LiveOrigins, const FactManager &FM, + const MovedLoansAnalysis &MovedLoans, + const LiveOriginsAnalysis &LiveOrigins, FactManager &FM, AnalysisDeclContext &ADC, LifetimeSafetySemaHelper *SemaHelper) - : LoanPropagation(LoanPropagation), LiveOrigins(LiveOrigins), FactMgr(FM), - SemaHelper(SemaHelper), AST(ADC.getASTContext()) { + : LoanPropagation(LoanPropagation), MovedLoans(MovedLoans), + LiveOrigins(LiveOrigins), FactMgr(FM), SemaHelper(SemaHelper), + AST(ADC.getASTContext()) { for (const CFGBlock *B : *ADC.getAnalysis<PostOrderCFGView>()) for (const Fact *F : FactMgr.getFacts(B)) if (const auto *EF = F->getAs<ExpireFact>()) @@ -127,6 +131,27 @@ class LifetimeChecker { } } + // void handleMovedOrigins(const MovedOriginFact *MOF) { + // OriginID OID = MOF->getMovedOrigin(); + // // Mark all the access paths of all loans which share the same access + // path + // // with the loans held by the moved origin. + // LoanSet MovedLoans = LoanPropagation.getLoans(OID, MOF); + // for (LoanID MovedLoan : MovedLoans) { + // const Loan *MovedL = FactMgr.getLoanMgr().getLoan(MovedLoan); + // const auto *MovedPL = dyn_cast<PathLoan>(MovedL); + // if (!MovedPL) + // continue; + // for (Loan *L : FactMgr.getLoanMgr().getLoans()) { + // const auto *PL = dyn_cast<PathLoan>(L); + // if (!PL) + // continue; + // if (PL->getAccessPath() == MovedPL->getAccessPath()) + // FactMgr.getLoanMgr().setAsMoved(PL->getID(), MOF->getMoveExpr()); + // } + // } + // } + /// Checks for use-after-free & use-after-return errors when a loan expires. /// /// This method examines all live origins at the expiry point and determines @@ -140,6 +165,10 @@ class LifetimeChecker { /// propagation (e.g., a loan may only be held on some execution paths). void checkExpiry(const ExpireFact *EF) { LoanID ExpiredLoan = EF->getLoanID(); + const Expr *MovedExpr = nullptr; + if (auto *ME = MovedLoans.getMovedLoans(EF).lookup(ExpiredLoan)) + MovedExpr = *ME; + LivenessMap Origins = LiveOrigins.getLiveOriginsAt(EF); Confidence CurConfidence = Confidence::None; // The UseFact or OriginEscapesFact most indicative of a lifetime error, @@ -166,6 +195,7 @@ class LifetimeChecker { return; FinalWarningsMap[ExpiredLoan] = {/*ExpiryLoc=*/EF->getExpiryLoc(), /*BestCausingFact=*/BestCausingFact, + /*MovedExpr=*/MovedExpr, /*ConfidenceLevel=*/CurConfidence}; } @@ -179,19 +209,21 @@ class LifetimeChecker { llvm::PointerUnion<const UseFact *, const OriginEscapesFact *> CausingFact = Warning.CausingFact; Confidence Confidence = Warning.ConfidenceLevel; + const Expr *MovedExpr = Warning.MovedExpr; SourceLocation ExpiryLoc = Warning.ExpiryLoc; if (const auto *UF = CausingFact.dyn_cast<const UseFact *>()) - SemaHelper->reportUseAfterFree(IssueExpr, UF->getUseExpr(), ExpiryLoc, - Confidence); + SemaHelper->reportUseAfterFree(IssueExpr, UF->getUseExpr(), MovedExpr, + ExpiryLoc, Confidence); else if (const auto *OEF = CausingFact.dyn_cast<const OriginEscapesFact *>()) { if (const auto *RetEscape = dyn_cast<ReturnEscapeFact>(OEF)) - SemaHelper->reportUseAfterReturn( - IssueExpr, RetEscape->getReturnExpr(), ExpiryLoc, Confidence); + SemaHelper->reportUseAfterReturn(IssueExpr, + RetEscape->getReturnExpr(), + MovedExpr, ExpiryLoc, Confidence); else if (const auto *FieldEscape = dyn_cast<FieldEscapeFact>(OEF)) SemaHelper->reportDanglingField( - IssueExpr, FieldEscape->getFieldDecl(), ExpiryLoc); + IssueExpr, FieldEscape->getFieldDecl(), MovedExpr, ExpiryLoc); else llvm_unreachable("Unhandled OriginEscapesFact type"); } else @@ -293,11 +325,12 @@ class LifetimeChecker { } // namespace void runLifetimeChecker(const LoanPropagationAnalysis &LP, - const LiveOriginsAnalysis &LO, - const FactManager &FactMgr, AnalysisDeclContext &ADC, + const MovedLoansAnalysis &MovedLoans, + const LiveOriginsAnalysis &LO, FactManager &FactMgr, + AnalysisDeclContext &ADC, LifetimeSafetySemaHelper *SemaHelper) { llvm::TimeTraceScope TimeProfile("LifetimeChecker"); - LifetimeChecker Checker(LP, LO, FactMgr, ADC, SemaHelper); + LifetimeChecker Checker(LP, MovedLoans, LO, FactMgr, ADC, SemaHelper); } } // namespace clang::lifetimes::internal diff --git a/clang/lib/Analysis/LifetimeSafety/Dataflow.h b/clang/lib/Analysis/LifetimeSafety/Dataflow.h index 05c20d6385368..4de9bbb54af9f 100644 --- a/clang/lib/Analysis/LifetimeSafety/Dataflow.h +++ b/clang/lib/Analysis/LifetimeSafety/Dataflow.h @@ -170,6 +170,8 @@ class DataflowAnalysis { return D->transfer(In, *F->getAs<ExpireFact>()); case Fact::Kind::OriginFlow: return D->transfer(In, *F->getAs<OriginFlowFact>()); + case Fact::Kind::MovedOrigin: + return D->transfer(In, *F->getAs<MovedOriginFact>()); case Fact::Kind::OriginEscapes: return D->transfer(In, *F->getAs<OriginEscapesFact>()); case Fact::Kind::Use: @@ -184,6 +186,7 @@ class DataflowAnalysis { Lattice transfer(Lattice In, const IssueFact &) { return In; } Lattice transfer(Lattice In, const ExpireFact &) { return In; } Lattice transfer(Lattice In, const OriginFlowFact &) { return In; } + Lattice transfer(Lattice In, const MovedOriginFact &) { return In; } Lattice transfer(Lattice In, const OriginEscapesFact &) { return In; } Lattice transfer(Lattice In, const UseFact &) { return In; } Lattice transfer(Lattice In, const TestPointFact &) { return In; } diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp index 1fc72aa0a4259..bc4000e9ce3ef 100644 --- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp @@ -45,6 +45,13 @@ void OriginFlowFact::dump(llvm::raw_ostream &OS, const LoanManager &, OS << "\n"; } +void MovedOriginFact::dump(llvm::raw_ostream &OS, const LoanManager &, + const OriginManager &OM) const { + OS << "MovedOrigins ("; + OM.dump(getMovedOrigin(), OS); + OS << ")\n"; +} + void ReturnEscapeFact::dump(llvm::raw_ostream &OS, const LoanManager &, const OriginManager &OM) const { OS << "OriginEscapes ("; diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index fb859eeb856af..bdb48b0c81172 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -9,6 +9,8 @@ #include <cassert> #include <string> +#include "clang/AST/DeclCXX.h" +#include "clang/AST/ExprCXX.h" #include "clang/AST/OperationKinds.h" #include "clang/Analysis/Analyses/LifetimeSafety/Facts.h" #include "clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h" @@ -185,6 +187,9 @@ void FactsGenerator::VisitCXXConstructExpr(const CXXConstructExpr *CCE) { handleGSLPointerConstruction(CCE); return; } + handleFunctionCall(CCE, CCE->getConstructor(), + {CCE->getArgs(), CCE->getNumArgs()}, + /*IsGslConstruction=*/false); } void FactsGenerator::handleCXXCtorInitializer(const CXXCtorInitializer *CII) { @@ -233,23 +238,9 @@ void FactsGenerator::VisitMemberExpr(const MemberExpr *ME) { } } -static bool isStdMove(const FunctionDecl *FD) { - return FD && FD->isInStdNamespace() && FD->getIdentifier() && - FD->getName() == "move"; -} - void FactsGenerator::VisitCallExpr(const CallExpr *CE) { handleFunctionCall(CE, CE->getDirectCallee(), {CE->getArgs(), CE->getNumArgs()}); - // Track declarations that are moved via std::move. - // This is a flow-insensitive approximation: once a declaration is moved - // anywhere in the function, it's treated as moved everywhere. We do not - // generate expire facts for moved decls to avoid false alarms. - if (isStdMove(CE->getDirectCallee())) - if (CE->getNumArgs() == 1) - if (auto *DRE = - dyn_cast<DeclRefExpr>(CE->getArg(0)->IgnoreParenImpCasts())) - MovedDecls.insert(DRE->getDecl()); } void FactsGenerator::VisitCXXNullPtrLiteralExpr( @@ -453,11 +444,6 @@ void FactsGenerator::handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds) { // Iterate through all loans to see if any expire. for (const auto *Loan : FactMgr.getLoanMgr().getLoans()) { if (const auto *BL = dyn_cast<PathLoan>(Loan)) { - // Skip loans for declarations that have been moved. When a value is - // moved, the original owner no longer has ownership and its destruction - // should not cause the loan to expire, preventing false positives. - if (MovedDecls.contains(BL->getAccessPath().getAsValueDecl())) - continue; // Check if the loan is for a stack variable and if that variable // is the one being destructed. const AccessPath AP = BL->getAccessPath(); @@ -533,10 +519,29 @@ void FactsGenerator::handleGSLPointerConstruction(const CXXConstructExpr *CCE) { } } -/// Checks if a call-like expression creates a borrow by passing a value to a -/// reference parameter, creating an IssueFact if it does. -/// \param IsGslConstruction True if this is a GSL construction where all -/// argument origins should flow to the returned origin. +void FactsGenerator::handleMovedArgsInCall(const FunctionDecl *FD, + ArrayRef<const Expr *> Args) { + unsigned IsInstance = 0; + if (const auto *Method = dyn_cast<CXXMethodDecl>(FD); + Method && Method->isInstance() && !isa<CXXConstructorDecl>(FD)) + IsInstance = 1; + + // Skip 'this' arg as it cannot be moved. + for (unsigned I = IsInstance; + I < Args.size() && I < FD->getNumParams() + IsInstance; ++I) { + const ParmVarDecl *PVD = FD->getParamDecl(I - IsInstance); + if (!PVD->getType()->isRValueReferenceType()) + continue; + const Expr *Arg = Args[I]; + OriginList *MovedOrigins = getOriginsList(*Arg); + assert(MovedOrigins->getLength() >= 1 && + "unexpected length for r-value reference param"); + // Arg is being moved to this parameter. Mark the origin as moved. + CurrentBlockFacts.push_back(FactMgr.createFact<MovedOriginFact>( + Arg, MovedOrigins->getOuterOriginID())); + } +} + void FactsGenerator::handleFunctionCall(const Expr *Call, const FunctionDecl *FD, ArrayRef<const Expr *> Args, @@ -544,7 +549,10 @@ void FactsGenerator::handleFunctionCall(const Expr *Call, OriginList *CallList = getOriginsList(*Call); // Ignore functions returning values with no origin. FD = getDeclWithMergedLifetimeBoundAttrs(FD); - if (!FD || !CallList) + if (!FD) + return; + handleMovedArgsInCall(FD, Args); + if (!CallList) return; auto IsArgLifetimeBound = [FD](unsigned I) -> bool { const ParmVarDecl *PVD = nullptr; diff --git a/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp b/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp index bf9a65254e8fa..d3b06d6040f7f 100644 --- a/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp @@ -56,21 +56,12 @@ void LifetimeSafetyAnalysis::run() { llvm::TimeTraceScope TimeProfile("LifetimeSafetyAnalysis"); const CFG &Cfg = *AC.getCFG(); - DEBUG_WITH_TYPE("PrintCFG", Cfg.dump(AC.getASTContext().getLangOpts(), - /*ShowColors=*/true)); FactMgr = std::make_unique<FactManager>(AC, Cfg); FactsGenerator FactGen(*FactMgr, AC); FactGen.run(); - DEBUG_WITH_TYPE("LifetimeFacts", FactMgr->dump(Cfg, AC)); - - // Debug print facts for a specific function using - // -debug-only=EnableFilterByFunctionName,YourFunctionNameFoo - DEBUG_WITH_TYPE("EnableFilterByFunctionName", - DebugOnlyFunction(AC, Cfg, *FactMgr)); - /// TODO(opt): Consider optimizing individual blocks before running the /// dataflow analysis. /// 1. Expression Origins: These are assigned once and read at most once, @@ -85,12 +76,27 @@ void LifetimeSafetyAnalysis::run() { LoanPropagation = std::make_unique<LoanPropagationAnalysis>( Cfg, AC, *FactMgr, Factory.OriginMapFactory, Factory.LoanSetFactory); + MovedLoans = std::make_unique<MovedLoansAnalysis>( + Cfg, AC, *FactMgr, *LoanPropagation, FactMgr->getLoanMgr(), + Factory.LoanSetFactory, Factory.MovedLoansMapFactory); + LiveOrigins = std::make_unique<LiveOriginsAnalysis>( Cfg, AC, *FactMgr, Factory.LivenessMapFactory); + + runLifetimeChecker(*LoanPropagation, *MovedLoans, *LiveOrigins, *FactMgr, AC, + SemaHelper); + + DEBUG_WITH_TYPE("PrintCFG", Cfg.dump(AC.getASTContext().getLangOpts(), + /*ShowColors=*/true)); + + DEBUG_WITH_TYPE("LifetimeFacts", FactMgr->dump(Cfg, AC)); + + // Debug print facts for a specific function using + // -debug-only=EnableFilterByFunctionName,YourFunctionNameFoo + DEBUG_WITH_TYPE("EnableFilterByFunctionName", + DebugOnlyFunction(AC, Cfg, *FactMgr)); DEBUG_WITH_TYPE("LiveOrigins", LiveOrigins->dump(llvm::dbgs(), FactMgr->getTestPoints())); - - runLifetimeChecker(*LoanPropagation, *LiveOrigins, *FactMgr, AC, SemaHelper); } void collectLifetimeStats(AnalysisDeclContext &AC, OriginManager &OM, diff --git a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp index 01a74d30408ea..0996d11f0cdeb 100644 --- a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp @@ -63,6 +63,7 @@ static llvm::BitVector computePersistentOrigins(const FactManager &FactMgr, Cur = Cur->peelOuterOrigin()) CheckOrigin(Cur->getOuterOriginID()); break; + case Fact::Kind::MovedOrigin: case Fact::Kind::OriginEscapes: case Fact::Kind::Expire: case Fact::Kind::TestPoint: diff --git a/clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp b/clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp new file mode 100644 index 0000000000000..2e248b14d54f3 --- /dev/null +++ b/clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp @@ -0,0 +1,121 @@ + +#include "clang/Analysis/Analyses/LifetimeSafety/MovedLoans.h" +#include "Dataflow.h" +#include "clang/Analysis/Analyses/LifetimeSafety/Facts.h" +#include "clang/Analysis/Analyses/LifetimeSafety/LoanPropagation.h" +#include "clang/Analysis/Analyses/LifetimeSafety/Loans.h" +#include "clang/Analysis/Analyses/LifetimeSafety/Utils.h" + +namespace clang::lifetimes::internal { +namespace { +struct Lattice { + LoanSet ReachableLoans = LoanSet(nullptr); + MovedLoansMap MovedLoans = MovedLoansMap(nullptr); + + explicit Lattice(LoanSet ReachableLoans, MovedLoansMap MovedLoans) + : ReachableLoans(ReachableLoans), MovedLoans(MovedLoans) {} + + Lattice() = default; + + bool operator==(const Lattice &Other) const { + return ReachableLoans == Other.ReachableLoans && + MovedLoans == Other.MovedLoans; + } + bool operator!=(const Lattice &Other) const { return !(*this == Other); } +}; + +class AnalysisImpl + : public DataflowAnalysis<AnalysisImpl, Lattice, Direction::Forward> { +public: + AnalysisImpl(const CFG &C, AnalysisDeclContext &AC, FactManager &F, + const LoanPropagationAnalysis &LoanPropagation, + const LoanManager &LoanMgr, LoanSet::Factory &LoanSetFactory, + MovedLoansMap::Factory &MovedLoansMapFactory) + : DataflowAnalysis(C, AC, F), LoanPropagation(LoanPropagation), + LoanMgr(LoanMgr), LoanSetFactory(LoanSetFactory), + MovedLoansMapFactory(MovedLoansMapFactory) {} + + using Base::transfer; + + StringRef getAnalysisName() const { return "MovedLoans"; } + + Lattice getInitialState() { return Lattice{}; } + + // TODO: Doc. + Lattice join(Lattice A, Lattice B) { + LoanSet ReachableLoans = + utils::join(A.ReachableLoans, B.ReachableLoans, LoanSetFactory); + MovedLoansMap MovedLoans = utils::join( + A.MovedLoans, B.MovedLoans, MovedLoansMapFactory, + [](const Expr *const *MoveA, const Expr *const *MoveB) -> const Expr * { + assert(MoveA || MoveB); + if (!MoveA) + return *MoveB; + if (!MoveB) + return *MoveA; + return (*MoveA)->getExprLoc() < (*MoveB)->getExprLoc() ? *MoveA + : *MoveB; + }, + utils::JoinKind::Asymmetric); + return Lattice(ReachableLoans, MovedLoans); + } + + Lattice transfer(Lattice In, const IssueFact &F) { + LoanSet ReachableLoans = + LoanSetFactory.add(In.ReachableLoans, F.getLoanID()); + return Lattice(ReachableLoans, In.MovedLoans); + } + + Lattice transfer(Lattice In, const MovedOriginFact &F) { + LoanSet ReachableLoans = In.ReachableLoans; + MovedLoansMap MovedLoans = In.MovedLoans; + OriginID MovedOrigin = F.getMovedOrigin(); + for (LoanID LID : LoanPropagation.getLoans(MovedOrigin, &F)) { + const Loan *MovedLoan = LoanMgr.getLoan(LID); + auto *PL = dyn_cast<PathLoan>(MovedLoan); + if (!PL) + continue; + for (LoanID ReachableLID : In.ReachableLoans) { + const Loan *ReachableLoan = LoanMgr.getLoan(ReachableLID); + auto *RL = dyn_cast<PathLoan>(ReachableLoan); + if (!RL) + continue; + if (RL->getAccessPath() == PL->getAccessPath()) { + MovedLoans = MovedLoansMapFactory.add(MovedLoans, ReachableLID, + F.getMoveExpr()); + } + } + } + return Lattice(ReachableLoans, MovedLoans); + } + + MovedLoansMap getMovedLoans(ProgramPoint P) { return getState(P).MovedLoans; } + +private: + const LoanPropagationAnalysis &LoanPropagation; + const LoanManager &LoanMgr; + LoanSet::Factory &LoanSetFactory; + MovedLoansMap::Factory &MovedLoansMapFactory; +}; +} // namespace + +class MovedLoansAnalysis::Impl final : public AnalysisImpl { + using AnalysisImpl::AnalysisImpl; +}; + +MovedLoansAnalysis::MovedLoansAnalysis( + const CFG &C, AnalysisDeclContext &AC, FactManager &F, + const LoanPropagationAnalysis &LoanPropagation, const LoanManager &LoanMgr, + LoanSet::Factory &LoanSetFactory, + MovedLoansMap::Factory &MovedLoansMapFactory) + : PImpl(std::make_unique<Impl>(C, AC, F, LoanPropagation, LoanMgr, + LoanSetFactory, MovedLoansMapFactory)) { + PImpl->run(); +} + +MovedLoansAnalysis::~MovedLoansAnalysis() = default; + +MovedLoansMap MovedLoansAnalysis::getMovedLoans(ProgramPoint P) const { + return PImpl->getMovedLoans(P); +} +} // namespace clang::lifetimes::internal diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 0c96b0afef1a7..858d5b7f67b58 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2880,32 +2880,46 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { LifetimeSafetySemaHelperImpl(Sema &S) : S(S) {} void reportUseAfterFree(const Expr *IssueExpr, const Expr *UseExpr, - SourceLocation FreeLoc, Confidence C) override { + const Expr *MovedExpr, SourceLocation FreeLoc, + Confidence C) override { S.Diag(IssueExpr->getExprLoc(), - C == Confidence::Definite + MovedExpr ? diag::warn_lifetime_safety_loan_expires_moved_strict + : C == Confidence::Definite ? diag::warn_lifetime_safety_loan_expires_permissive : diag::warn_lifetime_safety_loan_expires_strict) << IssueExpr->getSourceRange(); + if (MovedExpr) + S.Diag(MovedExpr->getExprLoc(), diag::note_lifetime_safety_moved_here) + << MovedExpr->getSourceRange(); S.Diag(FreeLoc, diag::note_lifetime_safety_destroyed_here); S.Diag(UseExpr->getExprLoc(), diag::note_lifetime_safety_used_here) << UseExpr->getSourceRange(); } void reportUseAfterReturn(const Expr *IssueExpr, const Expr *ReturnExpr, - SourceLocation ExpiryLoc, Confidence C) override { + const Expr *MovedExpr, SourceLocation ExpiryLoc, + Confidence C) override { S.Diag(IssueExpr->getExprLoc(), - C == Confidence::Definite - ? diag::warn_lifetime_safety_return_stack_addr_permissive - : diag::warn_lifetime_safety_return_stack_addr_strict) + MovedExpr ? diag::warn_lifetime_safety_return_stack_addr_moved_strict + : diag::warn_lifetime_safety_return_stack_addr_permissive) << IssueExpr->getSourceRange(); + if (MovedExpr) + S.Diag(MovedExpr->getExprLoc(), diag::note_lifetime_safety_moved_here) + << MovedExpr->getSourceRange(); S.Diag(ReturnExpr->getExprLoc(), diag::note_lifetime_safety_returned_here) << ReturnExpr->getSourceRange(); } void reportDanglingField(const Expr *IssueExpr, const FieldDecl *DanglingField, + const Expr *MovedExpr, SourceLocation ExpiryLoc) override { - S.Diag(IssueExpr->getExprLoc(), diag::warn_lifetime_safety_dangling_field) + S.Diag(IssueExpr->getExprLoc(), + MovedExpr ? diag::warn_lifetime_safety_dangling_field_moved + : diag::warn_lifetime_safety_dangling_field) << IssueExpr->getSourceRange(); + if (MovedExpr) + S.Diag(MovedExpr->getExprLoc(), diag::note_lifetime_safety_moved_here) + << MovedExpr->getSourceRange(); S.Diag(DanglingField->getLocation(), diag::note_lifetime_safety_dangling_field_here) << DanglingField->getEndLoc(); @@ -3117,10 +3131,13 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings( D->getBeginLoc()) || !Diags.isIgnored(diag::warn_lifetime_safety_loan_expires_strict, D->getBeginLoc()) || - !Diags.isIgnored(diag::warn_lifetime_safety_return_stack_addr_permissive, + !Diags.isIgnored(diag::warn_lifetime_safety_loan_expires_moved_strict, D->getBeginLoc()) || - !Diags.isIgnored(diag::warn_lifetime_safety_return_stack_addr_strict, + !Diags.isIgnored(diag::warn_lifetime_safety_return_stack_addr_permissive, D->getBeginLoc()) || + !Diags.isIgnored( + diag::warn_lifetime_safety_return_stack_addr_moved_strict, + D->getBeginLoc()) || !Diags.isIgnored(diag::warn_lifetime_safety_noescape_escapes, D->getBeginLoc()); bool EnableLifetimeSafetyAnalysis = diff --git a/clang/test/Sema/Inputs/lifetime-analysis.h b/clang/test/Sema/Inputs/lifetime-analysis.h index 4f881d463ebcc..8a3139f3db3d3 100644 --- a/clang/test/Sema/Inputs/lifetime-analysis.h +++ b/clang/test/Sema/Inputs/lifetime-analysis.h @@ -85,6 +85,8 @@ template<class _Mystr> struct iter { template<typename T> struct basic_string { basic_string(); + basic_string(const basic_string<T> &); + basic_string(basic_string<T> &&); basic_string(const T *); ~basic_string(); const T *c_str() const; @@ -96,6 +98,8 @@ using string = basic_string<char>; template<typename T> struct unique_ptr { + unique_ptr(); + unique_ptr(unique_ptr<T>&&); ~unique_ptr(); T &operator*(); T *get() const; diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp index c82cf41b07361..d8631e5a7cf74 100644 --- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp +++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp @@ -349,9 +349,11 @@ const char *trackThroughMultiplePointer() { struct X { X(std::unique_ptr<int> up) : - pointee(*up), pointee2(up.get()), pointer(std::move(up)) {} - int &pointee; - int *pointee2; + pointee(*up), // cfg-field-warning {{may have been moved.}} + pointee2(up.get()), // cfg-field-warning {{may have been moved.}} + pointer(std::move(up)) {} // cfg-field-note 2 {{potentially moved here}} + int &pointee; // cfg-field-note {{this field dangles}} + int *pointee2; // cfg-field-note {{this field dangles}} std::unique_ptr<int> pointer; }; @@ -360,11 +362,11 @@ struct [[gsl::Owner]] XOwner { }; struct X2 { // A common usage that moves the passing owner to the class. - // verify no warning on this case. + // verify a strict warning on this case. X2(XOwner owner) : - pointee(owner.get()), - owner(std::move(owner)) {} - int* pointee; + pointee(owner.get()), // cfg-field-warning {{may have been moved.}} + owner(std::move(owner)) {} // cfg-field-note {{potentially moved here}} + int* pointee; // cfg-field-note {{this field dangles}} XOwner owner; }; @@ -1108,9 +1110,8 @@ struct Foo2 { }; struct Test { - Test(Foo2 foo) : bar(foo.bar.get()), // OK \ - // FIXME: cfg-field-warning {{address of stack memory escapes to a field}} - storage(std::move(foo.bar)) {}; + Test(Foo2 foo) : bar(foo.bar.get()), // cfg-field-warning-re {{address of stack memory escapes to a field. {{.*}} may have been moved}} + storage(std::move(foo.bar)) {}; // cfg-field-note {{potentially moved here}} Bar* bar; // cfg-field-note {{this field dangles}} std::unique_ptr<Bar> storage; diff --git a/clang/test/Sema/warn-lifetime-safety-dangling-field.cpp b/clang/test/Sema/warn-lifetime-safety-dangling-field.cpp index dfc43e49906f2..fa45458371012 100644 --- a/clang/test/Sema/warn-lifetime-safety-dangling-field.cpp +++ b/clang/test/Sema/warn-lifetime-safety-dangling-field.cpp @@ -24,15 +24,26 @@ struct CtorInitLifetimeBound { }; struct CtorInitButMoved { - std::string_view view; - CtorInitButMoved(std::string s) : view(s) { takeString(std::move(s)); } + std::string_view view; // expected-note {{this field dangles}} + CtorInitButMoved(std::string s) + : view(s) { // expected-warning-re {{address of stack memory escapes to a field. {{.*}} may have been moved}} + takeString(std::move(s)); // expected-note {{potentially moved here}} + } }; struct CtorInitButMovedOwned { + std::string_view view; // expected-note {{this field dangles}} + std::string owned; + CtorInitButMovedOwned(std::string s) + : view(s), // expected-warning-re {{address of stack memory escapes to a field. {{.*}} may have been moved}} + owned(std::move(s)) {} // expected-note {{potentially moved here}} +}; + +struct CtorInitButMovedOwnedOrderedCorrectly { std::string owned; std::string_view view; - CtorInitButMovedOwned(std::string s) : view(s), owned(std::move(s)) {} - CtorInitButMovedOwned(Dummy<1>, std::string s) : owned(std::move(s)), view(owned) {} + CtorInitButMovedOwnedOrderedCorrectly(std::string s) + :owned(std::move(s)), view(owned) {} }; struct CtorInitMultipleViews { @@ -65,8 +76,8 @@ struct CtorPointerField { }; struct MemberSetters { - std::string_view view; // expected-note 5 {{this field dangles}} - const char* p; // expected-note 5 {{this field dangles}} + std::string_view view; // expected-note 6 {{this field dangles}} + const char* p; // expected-note 6 {{this field dangles}} void setWithParam(std::string s) { view = s; // expected-warning {{address of stack memory escapes to a field}} @@ -98,9 +109,9 @@ struct MemberSetters { void setWithLocalButMoved() { std::string s; - view = s; - p = s.data(); - takeString(std::move(s)); + view = s; // expected-warning-re {{address of stack memory escapes to a field. {{.*}} may have been moved}} + p = s.data(); // expected-warning-re {{address of stack memory escapes to a field. {{.*}} may have been moved}} + takeString(std::move(s)); // expected-note 2 {{potentially moved here}} } void setWithGlobal() { diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp index 2976c809e389c..96de471b649a9 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -10,6 +10,8 @@ struct [[gsl::Owner]] MyObj { MyObj(); MyObj(int); MyObj(const MyObj&); + MyObj(MyObj&&); + MyObj& operator=(MyObj&&); ~MyObj() {} // Non-trivial destructor MyObj operator+(MyObj); @@ -1404,39 +1406,57 @@ void add(int c, MyObj* node) { } } // namespace CppCoverage -namespace do_not_warn_on_std_move { +namespace strict_warn_on_move { void silenced() { MyObj b; View v; { MyObj a; - v = a; - b = std::move(a); // No warning for 'a' being moved. - } - (void)v; + v = a; // expected-warning-re {{object whose reference {{.*}} may have been moved}} + b = std::move(a); // expected-note {{potentially moved here}} + } // expected-note {{destroyed here}} + (void)v; // expected-note {{later used here}} } -void silenced_flow_insensitive(bool c) { - MyObj a; - View v = a; - if (c) { - MyObj b = std::move(a); - } - (void)v; +void flow_sensitive(bool c) { + View v; + { + MyObj a; + if (c) { + MyObj b = std::move(a); + return; + } + v = a; // expected-warning {{object whose reference}} + } // expected-note {{destroyed here}} + (void)v; // expected-note {{later used here}} } -// FIXME: Silence when move arg is not a declref. void take(MyObj&&); -void not_silenced_via_conditional(bool cond) { +void detect_conditional(bool cond) { View v; { MyObj a, b; - v = cond ? a : b; // expected-warning 2 {{object whose reference }} - take(std::move(cond ? a : b)); + v = cond ? a : b; // expected-warning-re 2 {{object whose reference {{.*}} may have been moved}} + take(std::move(cond ? a : b)); // expected-note 2 {{potentially moved here}} } // expected-note 2 {{destroyed here}} (void)v; // expected-note 2 {{later used here}} } -} // namespace do_not_warn_on_std_move + +void wrong_use_of_move_is_permissive() { + View v; + { + MyObj a; + v = std::move(a); // expected-warning {{object whose reference is captured does not live long enough}} + } // expected-note {{destroyed here}} + (void)v; // expected-note {{later used here}} + const int* p; + { + MyObj a; + p = std::move(a).getData(); // expected-warning {{object whose reference is captured does not live long enough}} + } // expected-note {{destroyed here}} + (void)p; // expected-note {{later used here}} +} +} // namespace strict_warn_on_move // Implicit this annotations with redecls. namespace GH172013 { >From 880b4c73cda277bad2b9dadf4b977aadb05abb7b Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <[email protected]> Date: Fri, 30 Jan 2026 15:09:40 +0100 Subject: [PATCH 2/2] Apply suggested changes Apply suggested changes --- .../Analysis/LifetimeSafety/MovedLoans.cpp | 121 ------------------ 1 file changed, 121 deletions(-) delete mode 100644 clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp diff --git a/clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp b/clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp deleted file mode 100644 index 2e248b14d54f3..0000000000000 --- a/clang/lib/Analysis/LifetimeSafety/MovedLoans.cpp +++ /dev/null @@ -1,121 +0,0 @@ - -#include "clang/Analysis/Analyses/LifetimeSafety/MovedLoans.h" -#include "Dataflow.h" -#include "clang/Analysis/Analyses/LifetimeSafety/Facts.h" -#include "clang/Analysis/Analyses/LifetimeSafety/LoanPropagation.h" -#include "clang/Analysis/Analyses/LifetimeSafety/Loans.h" -#include "clang/Analysis/Analyses/LifetimeSafety/Utils.h" - -namespace clang::lifetimes::internal { -namespace { -struct Lattice { - LoanSet ReachableLoans = LoanSet(nullptr); - MovedLoansMap MovedLoans = MovedLoansMap(nullptr); - - explicit Lattice(LoanSet ReachableLoans, MovedLoansMap MovedLoans) - : ReachableLoans(ReachableLoans), MovedLoans(MovedLoans) {} - - Lattice() = default; - - bool operator==(const Lattice &Other) const { - return ReachableLoans == Other.ReachableLoans && - MovedLoans == Other.MovedLoans; - } - bool operator!=(const Lattice &Other) const { return !(*this == Other); } -}; - -class AnalysisImpl - : public DataflowAnalysis<AnalysisImpl, Lattice, Direction::Forward> { -public: - AnalysisImpl(const CFG &C, AnalysisDeclContext &AC, FactManager &F, - const LoanPropagationAnalysis &LoanPropagation, - const LoanManager &LoanMgr, LoanSet::Factory &LoanSetFactory, - MovedLoansMap::Factory &MovedLoansMapFactory) - : DataflowAnalysis(C, AC, F), LoanPropagation(LoanPropagation), - LoanMgr(LoanMgr), LoanSetFactory(LoanSetFactory), - MovedLoansMapFactory(MovedLoansMapFactory) {} - - using Base::transfer; - - StringRef getAnalysisName() const { return "MovedLoans"; } - - Lattice getInitialState() { return Lattice{}; } - - // TODO: Doc. - Lattice join(Lattice A, Lattice B) { - LoanSet ReachableLoans = - utils::join(A.ReachableLoans, B.ReachableLoans, LoanSetFactory); - MovedLoansMap MovedLoans = utils::join( - A.MovedLoans, B.MovedLoans, MovedLoansMapFactory, - [](const Expr *const *MoveA, const Expr *const *MoveB) -> const Expr * { - assert(MoveA || MoveB); - if (!MoveA) - return *MoveB; - if (!MoveB) - return *MoveA; - return (*MoveA)->getExprLoc() < (*MoveB)->getExprLoc() ? *MoveA - : *MoveB; - }, - utils::JoinKind::Asymmetric); - return Lattice(ReachableLoans, MovedLoans); - } - - Lattice transfer(Lattice In, const IssueFact &F) { - LoanSet ReachableLoans = - LoanSetFactory.add(In.ReachableLoans, F.getLoanID()); - return Lattice(ReachableLoans, In.MovedLoans); - } - - Lattice transfer(Lattice In, const MovedOriginFact &F) { - LoanSet ReachableLoans = In.ReachableLoans; - MovedLoansMap MovedLoans = In.MovedLoans; - OriginID MovedOrigin = F.getMovedOrigin(); - for (LoanID LID : LoanPropagation.getLoans(MovedOrigin, &F)) { - const Loan *MovedLoan = LoanMgr.getLoan(LID); - auto *PL = dyn_cast<PathLoan>(MovedLoan); - if (!PL) - continue; - for (LoanID ReachableLID : In.ReachableLoans) { - const Loan *ReachableLoan = LoanMgr.getLoan(ReachableLID); - auto *RL = dyn_cast<PathLoan>(ReachableLoan); - if (!RL) - continue; - if (RL->getAccessPath() == PL->getAccessPath()) { - MovedLoans = MovedLoansMapFactory.add(MovedLoans, ReachableLID, - F.getMoveExpr()); - } - } - } - return Lattice(ReachableLoans, MovedLoans); - } - - MovedLoansMap getMovedLoans(ProgramPoint P) { return getState(P).MovedLoans; } - -private: - const LoanPropagationAnalysis &LoanPropagation; - const LoanManager &LoanMgr; - LoanSet::Factory &LoanSetFactory; - MovedLoansMap::Factory &MovedLoansMapFactory; -}; -} // namespace - -class MovedLoansAnalysis::Impl final : public AnalysisImpl { - using AnalysisImpl::AnalysisImpl; -}; - -MovedLoansAnalysis::MovedLoansAnalysis( - const CFG &C, AnalysisDeclContext &AC, FactManager &F, - const LoanPropagationAnalysis &LoanPropagation, const LoanManager &LoanMgr, - LoanSet::Factory &LoanSetFactory, - MovedLoansMap::Factory &MovedLoansMapFactory) - : PImpl(std::make_unique<Impl>(C, AC, F, LoanPropagation, LoanMgr, - LoanSetFactory, MovedLoansMapFactory)) { - PImpl->run(); -} - -MovedLoansAnalysis::~MovedLoansAnalysis() = default; - -MovedLoansMap MovedLoansAnalysis::getMovedLoans(ProgramPoint P) const { - return PImpl->getMovedLoans(P); -} -} // namespace clang::lifetimes::internal _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
