https://github.com/kashika0112 updated https://github.com/llvm/llvm-project/pull/169767
>From 00494b03dd31a4a1e541fb45bd524ee452541208 Mon Sep 17 00:00:00 2001 From: Kashika Akhouri <[email protected]> Date: Thu, 27 Nov 2025 07:13:49 +0000 Subject: [PATCH 1/7] Add lifetime annotation suggestion --- .../Analyses/LifetimeSafety/FactsGenerator.h | 2 + .../Analyses/LifetimeSafety/LifetimeSafety.h | 3 + .../Analysis/Analyses/LifetimeSafety/Loans.h | 14 ++++ clang/include/clang/Basic/DiagnosticGroups.td | 2 + .../clang/Basic/DiagnosticSemaKinds.td | 7 ++ clang/lib/Analysis/LifetimeSafety/Checker.cpp | 29 ++++++++ clang/lib/Analysis/LifetimeSafety/Facts.cpp | 6 +- .../LifetimeSafety/FactsGenerator.cpp | 28 ++++++++ clang/lib/Sema/AnalysisBasedWarnings.cpp | 11 +++ clang/test/Sema/warn-lifetime-safety.cpp | 72 ++++++++++++++++++- 10 files changed, 172 insertions(+), 2 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h index 878cb90b685f9..818133eab261d 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h @@ -91,6 +91,8 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> { void markUseAsWrite(const DeclRefExpr *DRE); + llvm::SmallVector<Fact *> createPlaceholderLoanFacts(); + FactManager &FactMgr; AnalysisDeclContext &AC; llvm::SmallVector<Fact *> CurrentBlockFacts; diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h index b34a7f18b5809..5397263315010 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h @@ -47,6 +47,9 @@ class LifetimeSafetyReporter { const Expr *EscapeExpr, SourceLocation ExpiryLoc, Confidence Confidence) {} + + virtual void reportMissingAnnotations(const ParmVarDecl *PVD, + const Expr *EscapeExpr) {} }; /// The main entry point for the analysis. diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h index 7f5cf03fd3e5f..16d4c834c8071 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h @@ -67,6 +67,15 @@ class LoanManager { } llvm::ArrayRef<Loan> getLoans() const { return AllLoans; } + void addPlaceholderLoan(LoanID LID, const ParmVarDecl *PVD) { + PlaceholderLoans[LID] = PVD; + } + + const llvm::DenseMap<LoanID, const ParmVarDecl *> & + getPlaceholderLoans() const { + return PlaceholderLoans; + } + private: LoanID getNextLoanID() { return NextLoanID++; } @@ -74,6 +83,11 @@ class LoanManager { /// TODO(opt): Profile and evaluate the usefullness of small buffer /// optimisation. llvm::SmallVector<Loan> AllLoans; + /// Represents a map of placeholder LoanID to the function parameter. + /// Placeholder loans are dummy loans created for each pointer or reference + /// parameter to represent a borrow from the function's caller, which the + /// analysis tracks to see if it unsafely escapes the function's scope. + llvm::DenseMap<LoanID, const ParmVarDecl *> PlaceholderLoans; }; } // namespace clang::lifetimes::internal diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 2fff32bbc4d6c..d2537870bfd64 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -541,6 +541,8 @@ def LifetimeSafety : DiagGroup<"experimental-lifetime-safety", Experimental warnings to detect use-after-free and related temporal safety bugs based on lifetime safety analysis. }]; } +def LifetimeSafetySuggestions + : DiagGroup<"experimental-lifetime-safety-suggestions">; def DistributedObjectModifiers : DiagGroup<"distributed-object-modifiers">; def DllexportExplicitInstantiationDecl : DiagGroup<"dllexport-explicit-instantiation-decl">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 4a145fd71eedd..3a4949ac9a5d6 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10778,6 +10778,13 @@ 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 warn_lifetime_param_should_be_lifetimebound + : Warning<"param should be marked [[clang::lifetimebound]]">, + InGroup<LifetimeSafetySuggestions>, + DefaultIgnore; + +def note_lifetime_escapes_here : Note<"param escapes here">; + // For non-floating point, expressions of the form x == x or x != x // should result in a warning, since these always evaluate to a constant. // Array comparisons have similar warnings diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index 1f7c282dadac2..6b1d0f619bb4d 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -50,6 +50,7 @@ struct PendingWarning { class LifetimeChecker { private: llvm::DenseMap<LoanID, PendingWarning> FinalWarningsMap; + llvm::DenseMap<const ParmVarDecl *, const Expr *> AnnotationWarningsMap; const LoanPropagationAnalysis &LoanPropagation; const LiveOriginsAnalysis &LiveOrigins; const FactManager &FactMgr; @@ -65,7 +66,28 @@ class LifetimeChecker { for (const Fact *F : FactMgr.getFacts(B)) if (const auto *EF = F->getAs<ExpireFact>()) checkExpiry(EF); + else if (const auto *OEF = F->getAs<OriginEscapesFact>()) + checkAnnotations(OEF); issuePendingWarnings(); + issueAnnotationWarnings(); + } + + /// Checks if an escaping origin holds a placeholder loan, indicating a + /// missing [[clang::lifetimebound]] annotation. + void checkAnnotations(const OriginEscapesFact *OEF) { + if (!Reporter) + return; + const auto &PlaceholderLoansMap = + FactMgr.getLoanMgr().getPlaceholderLoans(); + if (PlaceholderLoansMap.empty()) + return; + OriginID EscapedOID = OEF->getEscapedOriginID(); + LoanSet EscapedLoans = LoanPropagation.getLoans(EscapedOID, OEF); + for (LoanID LID : EscapedLoans) { + if (auto It = PlaceholderLoansMap.find(LID); + It != PlaceholderLoansMap.end()) + AnnotationWarningsMap.try_emplace(It->second, OEF->getEscapeExpr()); + } } /// Checks for use-after-free & use-after-return errors when a loan expires. @@ -132,6 +154,13 @@ class LifetimeChecker { llvm_unreachable("Unhandled CausingFact type"); } } + + void issueAnnotationWarnings() { + if (!Reporter) + return; + for (const auto &[PVD, EscapeExpr] : AnnotationWarningsMap) + Reporter->reportMissingAnnotations(PVD, EscapeExpr); + } }; } // namespace diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp index 0ae7111c489e8..823c4d19e13b9 100644 --- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp @@ -19,8 +19,12 @@ void Fact::dump(llvm::raw_ostream &OS, const LoanManager &, void IssueFact::dump(llvm::raw_ostream &OS, const LoanManager &LM, const OriginManager &OM) const { + const Loan &L = LM.getLoan(getLoanID()); OS << "Issue ("; - LM.getLoan(getLoanID()).dump(OS); + if (L.IssueExpr == nullptr) + OS << getLoanID() << " (Placeholder loan) "; + else + L.dump(OS); OS << ", ToOrigin: "; OM.dump(getOriginID(), OS); OS << ")\n"; diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 00870c3fd4086..9ff4bdf4b90ff 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -42,11 +42,16 @@ static const Loan *createLoan(FactManager &FactMgr, const DeclRefExpr *DRE) { void FactsGenerator::run() { llvm::TimeTraceScope TimeProfile("FactGenerator"); + const CFG &Cfg = *AC.getCFG(); + llvm::SmallVector<Fact *> PlaceholderLoanFacts = createPlaceholderLoanFacts(); // Iterate through the CFG blocks in reverse post-order to ensure that // initializations and destructions are processed in the correct sequence. for (const CFGBlock *Block : *AC.getAnalysis<PostOrderCFGView>()) { CurrentBlockFacts.clear(); EscapesInCurrentBlock.clear(); + if (Block->getBlockID() == Cfg.getEntry().getBlockID()) + CurrentBlockFacts.append(PlaceholderLoanFacts.begin(), + PlaceholderLoanFacts.end()); for (unsigned I = 0; I < Block->size(); ++I) { const CFGElement &Element = Block->Elements[I]; if (std::optional<CFGStmt> CS = Element.getAs<CFGStmt>()) @@ -342,4 +347,27 @@ void FactsGenerator::markUseAsWrite(const DeclRefExpr *DRE) { UseFacts[DRE]->markAsWritten(); } +// Creates an IssueFact for a new placeholder loan for each pointer or reference +// parameter at the function's entry. +llvm::SmallVector<Fact *> FactsGenerator::createPlaceholderLoanFacts() { + llvm::SmallVector<Fact *> PlaceholderLoanFacts; + const auto *FD = dyn_cast<FunctionDecl>(AC.getDecl()); + if (!FD) + return PlaceholderLoanFacts; + + for (const ParmVarDecl *PVD : FD->parameters()) { + QualType ParamType = PVD->getType(); + if (PVD->hasAttr<LifetimeBoundAttr>()) + continue; + if (ParamType->isPointerType() || ParamType->isReferenceType() || + isGslPointerType(ParamType)) { + Loan &L = FactMgr.getLoanMgr().addLoan({PVD}, /*IssueExpr=*/nullptr); + FactMgr.getLoanMgr().addPlaceholderLoan(L.ID, PVD); + OriginID OID = FactMgr.getOriginMgr().getOrCreate(*PVD); + PlaceholderLoanFacts.push_back(FactMgr.createFact<IssueFact>(L.ID, OID)); + } + } + return PlaceholderLoanFacts; +} + } // namespace clang::lifetimes::internal diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 43d2b9a829545..666f8dabbd4cb 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2884,6 +2884,17 @@ class LifetimeSafetyReporterImpl : public LifetimeSafetyReporter { << EscapeExpr->getEndLoc(); } + void reportMissingAnnotations(const ParmVarDecl *PVD, + const Expr *EscapeExpr) override { + S.Diag(PVD->getLocation(), + diag::warn_lifetime_param_should_be_lifetimebound) + << PVD->getSourceRange() + << FixItHint::CreateInsertion( + PVD->getTypeSourceInfo()->getTypeLoc().getBeginLoc(), + "[[clang::lifetimebound]] "); + S.Diag(EscapeExpr->getBeginLoc(), diag::note_lifetime_escapes_here); + } + private: Sema &S; }; diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp index 1191469e23df1..4318ec4c4cb2f 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only -fexperimental-lifetime-safety -Wexperimental-lifetime-safety -Wno-dangling -verify %s +// RUN: %clang_cc1 -fsyntax-only -fexperimental-lifetime-safety -Wexperimental-lifetime-safety -Wexperimental-lifetime-safety-suggestions -Wno-dangling -verify %s struct MyObj { int id; @@ -943,3 +943,73 @@ void parentheses(bool cond) { } // expected-note 4 {{destroyed here}} (void)*p; // expected-note 4 {{later used here}} } + +//===----------------------------------------------------------------------===// +// Lifetimebound Annotation Suggestion Tests +//===----------------------------------------------------------------------===// + +View return_view_directly (View a // expected-warning {{param should be marked [[clang::lifetimebound]]}}. +) { + return a; // expected-note {{param escapes here}} +} + +View conditional_return_view ( + View a, // expected-warning {{param should be marked [[clang::lifetimebound]]}}. + View b, // expected-warning {{param should be marked [[clang::lifetimebound]]}}. + bool c +) { + View res; + if (c) + res = a; + else + res = b; + return res; // expected-note 2 {{param escapes here}} +} + +// FIXME: Fails to generate lifetime suggestion for reference types as these are not handled currently. +MyObj& return_reference ( + MyObj& a, + MyObj& b, + bool c +) { + if(c) { + return a; + } + return b; +} + +// FIXME: Fails to generate lifetime suggestion for reference types as these are not handled currently. +View return_view_from_reference ( + MyObj& p +) { + return p; +} + +int* return_pointer_directly (int* a // expected-warning {{param should be marked [[clang::lifetimebound]]}}. +) { + return a; // expected-note {{param escapes here}} +} + +MyObj* return_pointer_object (MyObj* a // expected-warning {{param should be marked [[clang::lifetimebound]]}}. +) { + return a; // expected-note {{param escapes here}} +} + +View only_one_paramter_annotated (View a [[clang::lifetimebound]], + View b, // expected-warning {{param should be marked [[clang::lifetimebound]]}}. + bool c +) { + if(c) + return a; + return b; // expected-note {{param escapes here}} +} + +// Safe cases +View already_annotated(View a [[clang::lifetimebound]]) { + return a; +} + +// Safe cases +MyObj return_obj_by_value(MyObj& p) { + return p; +} >From 474aa135a28b98559b242de478c31f816b9eeddd Mon Sep 17 00:00:00 2001 From: Kashika Akhouri <[email protected]> Date: Thu, 27 Nov 2025 09:44:17 +0000 Subject: [PATCH 2/7] Remove access path from placeholder loans --- clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h | 2 +- clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h index 16d4c834c8071..07025e1e80d11 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h @@ -31,7 +31,7 @@ inline llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, LoanID ID) { struct AccessPath { const clang::ValueDecl *D; - AccessPath(const clang::ValueDecl *D) : D(D) {} + AccessPath(const clang::ValueDecl *D = nullptr) : D(D) {} }; /// Information about a single borrow, or "Loan". A loan is created when a diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 9ff4bdf4b90ff..ffd57e00bb722 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -361,7 +361,7 @@ llvm::SmallVector<Fact *> FactsGenerator::createPlaceholderLoanFacts() { continue; if (ParamType->isPointerType() || ParamType->isReferenceType() || isGslPointerType(ParamType)) { - Loan &L = FactMgr.getLoanMgr().addLoan({PVD}, /*IssueExpr=*/nullptr); + Loan &L = FactMgr.getLoanMgr().addLoan({}, /*IssueExpr=*/nullptr); FactMgr.getLoanMgr().addPlaceholderLoan(L.ID, PVD); OriginID OID = FactMgr.getOriginMgr().getOrCreate(*PVD); PlaceholderLoanFacts.push_back(FactMgr.createFact<IssueFact>(L.ID, OID)); >From 0d044d78093be9ebd26d254d6b06a6dff97f8b67 Mon Sep 17 00:00:00 2001 From: Kashika Akhouri <[email protected]> Date: Thu, 27 Nov 2025 09:56:25 +0000 Subject: [PATCH 3/7] Update loan dump to handle nullptr --- clang/lib/Analysis/LifetimeSafety/Facts.cpp | 5 +---- clang/lib/Analysis/LifetimeSafety/Loans.cpp | 4 ++++ 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp index 823c4d19e13b9..086dcf9fb4538 100644 --- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp @@ -21,10 +21,7 @@ void IssueFact::dump(llvm::raw_ostream &OS, const LoanManager &LM, const OriginManager &OM) const { const Loan &L = LM.getLoan(getLoanID()); OS << "Issue ("; - if (L.IssueExpr == nullptr) - OS << getLoanID() << " (Placeholder loan) "; - else - L.dump(OS); + L.dump(OS); OS << ", ToOrigin: "; OM.dump(getOriginID(), OS); OS << ")\n"; diff --git a/clang/lib/Analysis/LifetimeSafety/Loans.cpp b/clang/lib/Analysis/LifetimeSafety/Loans.cpp index 2c85a3c6083f3..a0886cb583e04 100644 --- a/clang/lib/Analysis/LifetimeSafety/Loans.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Loans.cpp @@ -11,6 +11,10 @@ namespace clang::lifetimes::internal { void Loan::dump(llvm::raw_ostream &OS) const { + if (Path.D == nullptr) { + OS << ID << " (Placeholder loan) "; + return; + } OS << ID << " (Path: "; OS << Path.D->getNameAsString() << ")"; } >From a634636c469004506132ecc070ae2597c1237b7c Mon Sep 17 00:00:00 2001 From: Kashika Akhouri <[email protected]> Date: Thu, 27 Nov 2025 10:04:10 +0000 Subject: [PATCH 4/7] Formatting changes --- clang/lib/Analysis/LifetimeSafety/Facts.cpp | 3 +-- clang/lib/Analysis/LifetimeSafety/Loans.cpp | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp index 086dcf9fb4538..0ae7111c489e8 100644 --- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp @@ -19,9 +19,8 @@ void Fact::dump(llvm::raw_ostream &OS, const LoanManager &, void IssueFact::dump(llvm::raw_ostream &OS, const LoanManager &LM, const OriginManager &OM) const { - const Loan &L = LM.getLoan(getLoanID()); OS << "Issue ("; - L.dump(OS); + LM.getLoan(getLoanID()).dump(OS); OS << ", ToOrigin: "; OM.dump(getOriginID(), OS); OS << ")\n"; diff --git a/clang/lib/Analysis/LifetimeSafety/Loans.cpp b/clang/lib/Analysis/LifetimeSafety/Loans.cpp index a0886cb583e04..6d2b335205a57 100644 --- a/clang/lib/Analysis/LifetimeSafety/Loans.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Loans.cpp @@ -12,7 +12,7 @@ namespace clang::lifetimes::internal { void Loan::dump(llvm::raw_ostream &OS) const { if (Path.D == nullptr) { - OS << ID << " (Placeholder loan) "; + OS << ID << " (Placeholder loan)"; return; } OS << ID << " (Path: "; >From 4b38be0d4a5cf2baaad0333147931428e67e5922 Mon Sep 17 00:00:00 2001 From: Kashika Akhouri <[email protected]> Date: Fri, 28 Nov 2025 10:17:19 +0000 Subject: [PATCH 5/7] Refactor Loan struct to class --- .../Analyses/LifetimeSafety/LifetimeSafety.h | 4 +- .../Analysis/Analyses/LifetimeSafety/Loans.h | 96 +++++++++++++------ .../clang/Basic/DiagnosticSemaKinds.td | 4 +- clang/lib/Analysis/LifetimeSafety/Checker.cpp | 26 ++--- clang/lib/Analysis/LifetimeSafety/Facts.cpp | 4 +- .../LifetimeSafety/FactsGenerator.cpp | 35 ++++--- clang/lib/Analysis/LifetimeSafety/Loans.cpp | 12 +-- clang/lib/Sema/AnalysisBasedWarnings.cpp | 18 ++-- .../Sema/warn-lifetime-safety-suggestions.cpp | 81 ++++++++++++++++ clang/test/Sema/warn-lifetime-safety.cpp | 69 ------------- .../unittests/Analysis/LifetimeSafetyTest.cpp | 8 +- 11 files changed, 207 insertions(+), 150 deletions(-) create mode 100644 clang/test/Sema/warn-lifetime-safety-suggestions.cpp diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h index 5397263315010..90d69e766ec78 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h @@ -48,8 +48,8 @@ class LifetimeSafetyReporter { SourceLocation ExpiryLoc, Confidence Confidence) {} - virtual void reportMissingAnnotations(const ParmVarDecl *PVD, - const Expr *EscapeExpr) {} + virtual void suggestAnnotation(const ParmVarDecl *PVD, + const Expr *EscapeExpr) {} }; /// The main entry point for the analysis. diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h index 07025e1e80d11..7dab816241a2f 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h @@ -31,24 +31,71 @@ inline llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, LoanID ID) { struct AccessPath { const clang::ValueDecl *D; - AccessPath(const clang::ValueDecl *D = nullptr) : D(D) {} + AccessPath(const clang::ValueDecl *D) : D(D) {} }; -/// Information about a single borrow, or "Loan". A loan is created when a -/// reference or pointer is created. -struct Loan { +/// An abstract base class for a single borrow, or "Loan". +class Loan { /// TODO: Represent opaque loans. /// TODO: Represent nullptr: loans to no path. Accessing it UB! Currently it /// is represented as empty LoanSet - LoanID ID; +public: + enum class Kind : uint8_t { + /// A regular borrow of a variable within the function that has a path and + /// can expire. + Borrow, + /// A non-expiring placeholder loan for a parameter, representing a borrow + /// from the function's caller. + Placeholder + }; + + Loan(Kind K, LoanID ID) : K(K), ID(ID) {} + virtual ~Loan() = default; + + Kind getKind() const { return K; } + LoanID getID() const { return ID; } + + virtual void dump(llvm::raw_ostream &OS) const = 0; + +private: + const Kind K; + const LoanID ID; +}; + +/// Information about a single borrow, or "Loan". A loan is created when a +/// reference or pointer is created. +class BorrowLoan : public Loan { AccessPath Path; - /// The expression that creates the loan, e.g., &x. const Expr *IssueExpr; - Loan(LoanID id, AccessPath path, const Expr *IssueExpr) - : ID(id), Path(path), IssueExpr(IssueExpr) {} +public: + BorrowLoan(LoanID ID, AccessPath Path, const Expr *IssueExpr) + : Loan(Kind::Borrow, ID), Path(Path), IssueExpr(IssueExpr) {} + + const AccessPath &getAccessPath() const { return Path; } + const Expr *getIssueExpr() const { return IssueExpr; } - void dump(llvm::raw_ostream &OS) const; + void dump(llvm::raw_ostream &OS) const override; + + static bool classof(const Loan *L) { return L->getKind() == Kind::Borrow; } +}; + +/// A concrete loan type for placeholder loans on parameters, representing a +/// borrow from the function's caller. +class ParameterLoan : public Loan { + const ParmVarDecl *PVD; + +public: + ParameterLoan(LoanID ID, const ParmVarDecl *PVD) + : Loan(Kind::Placeholder, ID), PVD(PVD) {} + + const ParmVarDecl *getParmVarDecl() const { return PVD; } + + void dump(llvm::raw_ostream &OS) const override; + + static bool classof(const Loan *L) { + return L->getKind() == Kind::Placeholder; + } }; /// Manages the creation, storage and retrieval of loans. @@ -56,25 +103,20 @@ class LoanManager { public: LoanManager() = default; - Loan &addLoan(AccessPath Path, const Expr *IssueExpr) { - AllLoans.emplace_back(getNextLoanID(), Path, IssueExpr); - return AllLoans.back(); + template <typename LoanType, typename... Args> + LoanType *createLoan(Args &&...args) { + void *Mem = LoanAllocator.Allocate<LoanType>(); + auto *NewLoan = + new (Mem) LoanType(getNextLoanID(), std::forward<Args>(args)...); + AllLoans.push_back(NewLoan); + return NewLoan; } - const Loan &getLoan(LoanID ID) const { + const Loan *getLoan(LoanID ID) const { assert(ID.Value < AllLoans.size()); return AllLoans[ID.Value]; } - llvm::ArrayRef<Loan> getLoans() const { return AllLoans; } - - void addPlaceholderLoan(LoanID LID, const ParmVarDecl *PVD) { - PlaceholderLoans[LID] = PVD; - } - - const llvm::DenseMap<LoanID, const ParmVarDecl *> & - getPlaceholderLoans() const { - return PlaceholderLoans; - } + llvm::ArrayRef<const Loan *> getLoans() const { return AllLoans; } private: LoanID getNextLoanID() { return NextLoanID++; } @@ -82,12 +124,8 @@ class LoanManager { LoanID NextLoanID{0}; /// TODO(opt): Profile and evaluate the usefullness of small buffer /// optimisation. - llvm::SmallVector<Loan> AllLoans; - /// Represents a map of placeholder LoanID to the function parameter. - /// Placeholder loans are dummy loans created for each pointer or reference - /// parameter to represent a borrow from the function's caller, which the - /// analysis tracks to see if it unsafely escapes the function's scope. - llvm::DenseMap<LoanID, const ParmVarDecl *> PlaceholderLoans; + llvm::SmallVector<const Loan *> AllLoans; + llvm::BumpPtrAllocator LoanAllocator; }; } // namespace clang::lifetimes::internal diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 3a4949ac9a5d6..a9e70ba731c87 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10778,12 +10778,12 @@ 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 warn_lifetime_param_should_be_lifetimebound +def warn_lifetime_safety_suggest_lifetimebound : Warning<"param should be marked [[clang::lifetimebound]]">, InGroup<LifetimeSafetySuggestions>, DefaultIgnore; -def note_lifetime_escapes_here : Note<"param escapes here">; +def note_lifetime_safety_suggestion_returned_here : Note<"param returned here">; // For non-floating point, expressions of the form x == x or x != x // should result in a warning, since these always evaluate to a constant. diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index 6b1d0f619bb4d..228d081d29669 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -77,16 +77,19 @@ class LifetimeChecker { void checkAnnotations(const OriginEscapesFact *OEF) { if (!Reporter) return; - const auto &PlaceholderLoansMap = - FactMgr.getLoanMgr().getPlaceholderLoans(); - if (PlaceholderLoansMap.empty()) - return; OriginID EscapedOID = OEF->getEscapedOriginID(); LoanSet EscapedLoans = LoanPropagation.getLoans(EscapedOID, OEF); - for (LoanID LID : EscapedLoans) { - if (auto It = PlaceholderLoansMap.find(LID); - It != PlaceholderLoansMap.end()) - AnnotationWarningsMap.try_emplace(It->second, OEF->getEscapeExpr()); + if (EscapedLoans.isEmpty()) + return; + for (const Loan *L : FactMgr.getLoanMgr().getLoans()) { + if (const auto *PL = dyn_cast<ParameterLoan>(L)) { + if (EscapedLoans.contains(PL->getID())) { + const ParmVarDecl *PVD = PL->getParmVarDecl(); + if (PVD->hasAttr<LifetimeBoundAttr>()) + continue; + AnnotationWarningsMap.try_emplace(PVD, OEF->getEscapeExpr()); + } + } } } @@ -136,8 +139,9 @@ class LifetimeChecker { if (!Reporter) return; for (const auto &[LID, Warning] : FinalWarningsMap) { - const Loan &L = FactMgr.getLoanMgr().getLoan(LID); - const Expr *IssueExpr = L.IssueExpr; + const Loan *L = FactMgr.getLoanMgr().getLoan(LID); + const auto *BL = cast<BorrowLoan>(L); + const Expr *IssueExpr = BL->getIssueExpr(); llvm::PointerUnion<const UseFact *, const OriginEscapesFact *> CausingFact = Warning.CausingFact; Confidence Confidence = Warning.ConfidenceLevel; @@ -159,7 +163,7 @@ class LifetimeChecker { if (!Reporter) return; for (const auto &[PVD, EscapeExpr] : AnnotationWarningsMap) - Reporter->reportMissingAnnotations(PVD, EscapeExpr); + Reporter->suggestAnnotation(PVD, EscapeExpr); } }; } // namespace diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp index 0ae7111c489e8..68317318ff4e2 100644 --- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp @@ -20,7 +20,7 @@ void Fact::dump(llvm::raw_ostream &OS, const LoanManager &, void IssueFact::dump(llvm::raw_ostream &OS, const LoanManager &LM, const OriginManager &OM) const { OS << "Issue ("; - LM.getLoan(getLoanID()).dump(OS); + LM.getLoan(getLoanID())->dump(OS); OS << ", ToOrigin: "; OM.dump(getOriginID(), OS); OS << ")\n"; @@ -29,7 +29,7 @@ void IssueFact::dump(llvm::raw_ostream &OS, const LoanManager &LM, void ExpireFact::dump(llvm::raw_ostream &OS, const LoanManager &LM, const OriginManager &) const { OS << "Expire ("; - LM.getLoan(getLoanID()).dump(OS); + LM.getLoan(getLoanID())->dump(OS); OS << ")\n"; } diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index ffd57e00bb722..6a00cf52610c7 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -31,11 +31,12 @@ static bool hasOrigin(const VarDecl *VD) { /// This function should be called whenever a DeclRefExpr represents a borrow. /// \param DRE The declaration reference expression that initiates the borrow. /// \return The new Loan on success, nullptr otherwise. -static const Loan *createLoan(FactManager &FactMgr, const DeclRefExpr *DRE) { +static const BorrowLoan *createLoan(FactManager &FactMgr, + const DeclRefExpr *DRE) { if (const auto *VD = dyn_cast<ValueDecl>(DRE->getDecl())) { AccessPath Path(VD); // The loan is created at the location of the DeclRefExpr. - return &FactMgr.getLoanMgr().addLoan(Path, DRE); + return FactMgr.getLoanMgr().createLoan<BorrowLoan>(Path, DRE); } return nullptr; } @@ -90,7 +91,7 @@ void FactsGenerator::VisitDeclRefExpr(const DeclRefExpr *DRE) { if (const Loan *L = createLoan(FactMgr, DRE)) { OriginID ExprOID = FactMgr.getOriginMgr().getOrCreate(*DRE); CurrentBlockFacts.push_back( - FactMgr.createFact<IssueFact>(L->ID, ExprOID)); + FactMgr.createFact<IssueFact>(L->getID(), ExprOID)); } } } @@ -228,13 +229,14 @@ void FactsGenerator::handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds) { if (!LifetimeEndsVD) return; // Iterate through all loans to see if any expire. - for (const auto &Loan : FactMgr.getLoanMgr().getLoans()) { - const AccessPath &LoanPath = Loan.Path; - // Check if the loan is for a stack variable and if that variable - // is the one being destructed. - if (LoanPath.D == LifetimeEndsVD) - CurrentBlockFacts.push_back(FactMgr.createFact<ExpireFact>( - Loan.ID, LifetimeEnds.getTriggerStmt()->getEndLoc())); + for (const auto *Loan : FactMgr.getLoanMgr().getLoans()) { + if (const auto *BL = dyn_cast<BorrowLoan>(Loan)) { + // Check if the loan is for a stack variable and if that variable + // is the one being destructed. + if (BL->getAccessPath().D == LifetimeEndsVD) + CurrentBlockFacts.push_back(FactMgr.createFact<ExpireFact>( + BL->getID(), LifetimeEnds.getTriggerStmt()->getEndLoc())); + } } } @@ -356,15 +358,12 @@ llvm::SmallVector<Fact *> FactsGenerator::createPlaceholderLoanFacts() { return PlaceholderLoanFacts; for (const ParmVarDecl *PVD : FD->parameters()) { - QualType ParamType = PVD->getType(); - if (PVD->hasAttr<LifetimeBoundAttr>()) - continue; - if (ParamType->isPointerType() || ParamType->isReferenceType() || - isGslPointerType(ParamType)) { - Loan &L = FactMgr.getLoanMgr().addLoan({}, /*IssueExpr=*/nullptr); - FactMgr.getLoanMgr().addPlaceholderLoan(L.ID, PVD); + if (hasOrigin(PVD)) { + const ParameterLoan *L = + FactMgr.getLoanMgr().createLoan<ParameterLoan>(PVD); OriginID OID = FactMgr.getOriginMgr().getOrCreate(*PVD); - PlaceholderLoanFacts.push_back(FactMgr.createFact<IssueFact>(L.ID, OID)); + PlaceholderLoanFacts.push_back( + FactMgr.createFact<IssueFact>(L->getID(), OID)); } } return PlaceholderLoanFacts; diff --git a/clang/lib/Analysis/LifetimeSafety/Loans.cpp b/clang/lib/Analysis/LifetimeSafety/Loans.cpp index 6d2b335205a57..0a6d33cead794 100644 --- a/clang/lib/Analysis/LifetimeSafety/Loans.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Loans.cpp @@ -10,13 +10,13 @@ namespace clang::lifetimes::internal { -void Loan::dump(llvm::raw_ostream &OS) const { - if (Path.D == nullptr) { - OS << ID << " (Placeholder loan)"; - return; - } - OS << ID << " (Path: "; +void BorrowLoan::dump(llvm::raw_ostream &OS) const { + OS << getID() << " (Path: "; OS << Path.D->getNameAsString() << ")"; } +void ParameterLoan::dump(llvm::raw_ostream &OS) const { + OS << getID() << " (Placeholder loan)"; +} + } // namespace clang::lifetimes::internal diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 666f8dabbd4cb..c952578b79d80 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2884,15 +2884,17 @@ class LifetimeSafetyReporterImpl : public LifetimeSafetyReporter { << EscapeExpr->getEndLoc(); } - void reportMissingAnnotations(const ParmVarDecl *PVD, - const Expr *EscapeExpr) override { - S.Diag(PVD->getLocation(), - diag::warn_lifetime_param_should_be_lifetimebound) + void suggestAnnotation(const ParmVarDecl *PVD, + const Expr *EscapeExpr) override { + SourceLocation InsertionPoint = Lexer::getLocForEndOfToken( + PVD->getEndLoc(), 0, S.getSourceManager(), S.getLangOpts()); + S.Diag(PVD->getBeginLoc(), diag::warn_lifetime_safety_suggest_lifetimebound) << PVD->getSourceRange() - << FixItHint::CreateInsertion( - PVD->getTypeSourceInfo()->getTypeLoc().getBeginLoc(), - "[[clang::lifetimebound]] "); - S.Diag(EscapeExpr->getBeginLoc(), diag::note_lifetime_escapes_here); + << FixItHint::CreateInsertion(InsertionPoint, + " [[clang::lifetimebound]]"); + S.Diag(EscapeExpr->getBeginLoc(), + diag::note_lifetime_safety_suggestion_returned_here) + << EscapeExpr->getSourceRange(); } private: diff --git a/clang/test/Sema/warn-lifetime-safety-suggestions.cpp b/clang/test/Sema/warn-lifetime-safety-suggestions.cpp new file mode 100644 index 0000000000000..00807c880c1d5 --- /dev/null +++ b/clang/test/Sema/warn-lifetime-safety-suggestions.cpp @@ -0,0 +1,81 @@ +// RUN: %clang_cc1 -fsyntax-only -fexperimental-lifetime-safety -Wexperimental-lifetime-safety-suggestions -verify %s + +struct MyObj { + int id; + ~MyObj() {} // Non-trivial destructor + MyObj operator+(MyObj); +}; + +struct [[gsl::Pointer()]] View { + View(const MyObj&); // Borrows from MyObj + View(); + void use() const; +}; + +//===----------------------------------------------------------------------===// +// Lifetimebound Annotation Suggestion Tests +//===----------------------------------------------------------------------===// + +View return_view_directly (View a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}. + return a; // expected-note {{param returned here}} +} + +View conditional_return_view ( + View a, // expected-warning {{param should be marked [[clang::lifetimebound]]}}. + View b, // expected-warning {{param should be marked [[clang::lifetimebound]]}}. + bool c) { + View res; + if (c) + res = a; + else + res = b; + return res; // expected-note 2 {{param returned here}} +} + +// FIXME: Fails to generate lifetime suggestion for reference types as these are not handled currently. +MyObj& return_reference (MyObj& a, MyObj& b, bool c) { + if(c) { + return a; + } + return b; +} + +// FIXME: Fails to generate lifetime suggestion for reference types as these are not handled currently. +View return_view_from_reference (MyObj& p) { + return p; +} + +int* return_pointer_directly (int* a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}. + return a; // expected-note {{param returned here}} +} + +MyObj* return_pointer_object (MyObj* a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}. + return a; // expected-note {{param returned here}} +} + +View only_one_paramter_annotated (View a [[clang::lifetimebound]], + View b, // expected-warning {{param should be marked [[clang::lifetimebound]]}}. + bool c) { + if(c) + return a; + return b; // expected-note {{param returned here}} +} + +//===----------------------------------------------------------------------===// +// Lifetimebound Annotation Negative Test Cases +//===----------------------------------------------------------------------===// + +View already_annotated(View a [[clang::lifetimebound]]) { + return a; +} + +MyObj return_obj_by_value(MyObj& p) { + return p; +} + +MyObj GlobalMyObj; +View Global = GlobalMyObj; +View Reassigned(View a) { + a = Global; + return a; +} \ No newline at end of file diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp index 4318ec4c4cb2f..64d558c7e627d 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -944,72 +944,3 @@ void parentheses(bool cond) { (void)*p; // expected-note 4 {{later used here}} } -//===----------------------------------------------------------------------===// -// Lifetimebound Annotation Suggestion Tests -//===----------------------------------------------------------------------===// - -View return_view_directly (View a // expected-warning {{param should be marked [[clang::lifetimebound]]}}. -) { - return a; // expected-note {{param escapes here}} -} - -View conditional_return_view ( - View a, // expected-warning {{param should be marked [[clang::lifetimebound]]}}. - View b, // expected-warning {{param should be marked [[clang::lifetimebound]]}}. - bool c -) { - View res; - if (c) - res = a; - else - res = b; - return res; // expected-note 2 {{param escapes here}} -} - -// FIXME: Fails to generate lifetime suggestion for reference types as these are not handled currently. -MyObj& return_reference ( - MyObj& a, - MyObj& b, - bool c -) { - if(c) { - return a; - } - return b; -} - -// FIXME: Fails to generate lifetime suggestion for reference types as these are not handled currently. -View return_view_from_reference ( - MyObj& p -) { - return p; -} - -int* return_pointer_directly (int* a // expected-warning {{param should be marked [[clang::lifetimebound]]}}. -) { - return a; // expected-note {{param escapes here}} -} - -MyObj* return_pointer_object (MyObj* a // expected-warning {{param should be marked [[clang::lifetimebound]]}}. -) { - return a; // expected-note {{param escapes here}} -} - -View only_one_paramter_annotated (View a [[clang::lifetimebound]], - View b, // expected-warning {{param should be marked [[clang::lifetimebound]]}}. - bool c -) { - if(c) - return a; - return b; // expected-note {{param escapes here}} -} - -// Safe cases -View already_annotated(View a [[clang::lifetimebound]]) { - return a; -} - -// Safe cases -MyObj return_obj_by_value(MyObj& p) { - return p; -} diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp b/clang/unittests/Analysis/LifetimeSafetyTest.cpp index a895475013c98..22824e4bf05db 100644 --- a/clang/unittests/Analysis/LifetimeSafetyTest.cpp +++ b/clang/unittests/Analysis/LifetimeSafetyTest.cpp @@ -114,9 +114,11 @@ class LifetimeTestHelper { return {}; } std::vector<LoanID> LID; - for (const Loan &L : Analysis.getFactManager().getLoanMgr().getLoans()) - if (L.Path.D == VD) - LID.push_back(L.ID); + for (const Loan *L : Analysis.getFactManager().getLoanMgr().getLoans()) + if (const auto *BL = dyn_cast<BorrowLoan>(L)) { + if (BL->getAccessPath().D == VD) + LID.push_back(L->getID()); + } if (LID.empty()) { ADD_FAILURE() << "Loan for '" << VarName << "' not found."; return {}; >From e5fb5955ddf33239e745607fdb83b058c6c0e729 Mon Sep 17 00:00:00 2001 From: Kashika Akhouri <[email protected]> Date: Fri, 28 Nov 2025 10:22:40 +0000 Subject: [PATCH 6/7] Nit formatting --- .../clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h | 1 - clang/test/Sema/warn-lifetime-safety-suggestions.cpp | 2 +- clang/test/Sema/warn-lifetime-safety.cpp | 3 +-- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h index 818133eab261d..5423d4406b5af 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h @@ -92,7 +92,6 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> { void markUseAsWrite(const DeclRefExpr *DRE); llvm::SmallVector<Fact *> createPlaceholderLoanFacts(); - FactManager &FactMgr; AnalysisDeclContext &AC; llvm::SmallVector<Fact *> CurrentBlockFacts; diff --git a/clang/test/Sema/warn-lifetime-safety-suggestions.cpp b/clang/test/Sema/warn-lifetime-safety-suggestions.cpp index 00807c880c1d5..25ca45c74da9a 100644 --- a/clang/test/Sema/warn-lifetime-safety-suggestions.cpp +++ b/clang/test/Sema/warn-lifetime-safety-suggestions.cpp @@ -78,4 +78,4 @@ View Global = GlobalMyObj; View Reassigned(View a) { a = Global; return a; -} \ No newline at end of file +} diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp index 64d558c7e627d..1191469e23df1 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only -fexperimental-lifetime-safety -Wexperimental-lifetime-safety -Wexperimental-lifetime-safety-suggestions -Wno-dangling -verify %s +// RUN: %clang_cc1 -fsyntax-only -fexperimental-lifetime-safety -Wexperimental-lifetime-safety -Wno-dangling -verify %s struct MyObj { int id; @@ -943,4 +943,3 @@ void parentheses(bool cond) { } // expected-note 4 {{destroyed here}} (void)*p; // expected-note 4 {{later used here}} } - >From 4070e2964a491caabcc4139301dbdbd3b3a5629c Mon Sep 17 00:00:00 2001 From: Kashika Akhouri <[email protected]> Date: Fri, 28 Nov 2025 15:25:51 +0000 Subject: [PATCH 7/7] Add documentation for PlaceholderLoan --- .../Analyses/LifetimeSafety/FactsGenerator.h | 2 +- .../Analysis/Analyses/LifetimeSafety/Loans.h | 20 +++++++++++++++---- clang/lib/Analysis/LifetimeSafety/Checker.cpp | 19 +++++++----------- .../LifetimeSafety/FactsGenerator.cpp | 14 ++++++------- clang/lib/Analysis/LifetimeSafety/Loans.cpp | 2 +- .../Sema/warn-lifetime-safety-suggestions.cpp | 13 +++++++----- 6 files changed, 40 insertions(+), 30 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h index 5423d4406b5af..a1acd8615afdd 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h @@ -91,7 +91,7 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> { void markUseAsWrite(const DeclRefExpr *DRE); - llvm::SmallVector<Fact *> createPlaceholderLoanFacts(); + llvm::SmallVector<Fact *> issuePlaceholderLoans(); FactManager &FactMgr; AnalysisDeclContext &AC; llvm::SmallVector<Fact *> CurrentBlockFacts; diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h index 7dab816241a2f..63628959d7d80 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h @@ -80,13 +80,25 @@ class BorrowLoan : public Loan { static bool classof(const Loan *L) { return L->getKind() == Kind::Borrow; } }; -/// A concrete loan type for placeholder loans on parameters, representing a -/// borrow from the function's caller. -class ParameterLoan : public Loan { +/// A placeholder loan held by a function parameter, representing a borrow from +/// the caller's scope. +/// +/// Created at function entry for each pointer or reference parameter with an +/// origin. Unlike BorrowLoan, placeholder loans: +/// - Have no IssueExpr (created at function entry, not at a borrow site) +/// - Have no AccessPath (the borrowed object is not visible to the function) +/// - Do not currently expire, but may in the future when modeling function +/// invalidations (e.g., vector::push_back) +/// +/// When a placeholder loan escapes the function (e.g., via return), it +/// indicates the parameter should be marked [[clang::lifetimebound]], enabling +/// lifetime annotation suggestions. +class PlaceholderLoan : public Loan { + /// The function parameter that holds this placeholder loan. const ParmVarDecl *PVD; public: - ParameterLoan(LoanID ID, const ParmVarDecl *PVD) + PlaceholderLoan(LoanID ID, const ParmVarDecl *PVD) : Loan(Kind::Placeholder, ID), PVD(PVD) {} const ParmVarDecl *getParmVarDecl() const { return PVD; } diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index 228d081d29669..9faac34f4874c 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -75,20 +75,15 @@ class LifetimeChecker { /// Checks if an escaping origin holds a placeholder loan, indicating a /// missing [[clang::lifetimebound]] annotation. void checkAnnotations(const OriginEscapesFact *OEF) { - if (!Reporter) - return; OriginID EscapedOID = OEF->getEscapedOriginID(); LoanSet EscapedLoans = LoanPropagation.getLoans(EscapedOID, OEF); - if (EscapedLoans.isEmpty()) - return; - for (const Loan *L : FactMgr.getLoanMgr().getLoans()) { - if (const auto *PL = dyn_cast<ParameterLoan>(L)) { - if (EscapedLoans.contains(PL->getID())) { - const ParmVarDecl *PVD = PL->getParmVarDecl(); - if (PVD->hasAttr<LifetimeBoundAttr>()) - continue; - AnnotationWarningsMap.try_emplace(PVD, OEF->getEscapeExpr()); - } + for (LoanID LID : EscapedLoans) { + const Loan *L = FactMgr.getLoanMgr().getLoan(LID); + if (const auto *PL = dyn_cast<PlaceholderLoan>(L)) { + const ParmVarDecl *PVD = PL->getParmVarDecl(); + if (PVD->hasAttr<LifetimeBoundAttr>()) + continue; + AnnotationWarningsMap.try_emplace(PVD, OEF->getEscapeExpr()); } } } diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 6a00cf52610c7..988c99860f5ce 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -44,13 +44,13 @@ static const BorrowLoan *createLoan(FactManager &FactMgr, void FactsGenerator::run() { llvm::TimeTraceScope TimeProfile("FactGenerator"); const CFG &Cfg = *AC.getCFG(); - llvm::SmallVector<Fact *> PlaceholderLoanFacts = createPlaceholderLoanFacts(); + llvm::SmallVector<Fact *> PlaceholderLoanFacts = issuePlaceholderLoans(); // Iterate through the CFG blocks in reverse post-order to ensure that // initializations and destructions are processed in the correct sequence. for (const CFGBlock *Block : *AC.getAnalysis<PostOrderCFGView>()) { CurrentBlockFacts.clear(); EscapesInCurrentBlock.clear(); - if (Block->getBlockID() == Cfg.getEntry().getBlockID()) + if (Block == &Cfg.getEntry()) CurrentBlockFacts.append(PlaceholderLoanFacts.begin(), PlaceholderLoanFacts.end()); for (unsigned I = 0; I < Block->size(); ++I) { @@ -351,16 +351,16 @@ void FactsGenerator::markUseAsWrite(const DeclRefExpr *DRE) { // Creates an IssueFact for a new placeholder loan for each pointer or reference // parameter at the function's entry. -llvm::SmallVector<Fact *> FactsGenerator::createPlaceholderLoanFacts() { - llvm::SmallVector<Fact *> PlaceholderLoanFacts; +llvm::SmallVector<Fact *> FactsGenerator::issuePlaceholderLoans() { const auto *FD = dyn_cast<FunctionDecl>(AC.getDecl()); if (!FD) - return PlaceholderLoanFacts; + return {}; + llvm::SmallVector<Fact *> PlaceholderLoanFacts; for (const ParmVarDecl *PVD : FD->parameters()) { if (hasOrigin(PVD)) { - const ParameterLoan *L = - FactMgr.getLoanMgr().createLoan<ParameterLoan>(PVD); + const PlaceholderLoan *L = + FactMgr.getLoanMgr().createLoan<PlaceholderLoan>(PVD); OriginID OID = FactMgr.getOriginMgr().getOrCreate(*PVD); PlaceholderLoanFacts.push_back( FactMgr.createFact<IssueFact>(L->getID(), OID)); diff --git a/clang/lib/Analysis/LifetimeSafety/Loans.cpp b/clang/lib/Analysis/LifetimeSafety/Loans.cpp index 0a6d33cead794..8398cd790b03c 100644 --- a/clang/lib/Analysis/LifetimeSafety/Loans.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Loans.cpp @@ -15,7 +15,7 @@ void BorrowLoan::dump(llvm::raw_ostream &OS) const { OS << Path.D->getNameAsString() << ")"; } -void ParameterLoan::dump(llvm::raw_ostream &OS) const { +void PlaceholderLoan::dump(llvm::raw_ostream &OS) const { OS << getID() << " (Placeholder loan)"; } diff --git a/clang/test/Sema/warn-lifetime-safety-suggestions.cpp b/clang/test/Sema/warn-lifetime-safety-suggestions.cpp index 25ca45c74da9a..0a2f1e1de9b15 100644 --- a/clang/test/Sema/warn-lifetime-safety-suggestions.cpp +++ b/clang/test/Sema/warn-lifetime-safety-suggestions.cpp @@ -12,10 +12,6 @@ struct [[gsl::Pointer()]] View { void use() const; }; -//===----------------------------------------------------------------------===// -// Lifetimebound Annotation Suggestion Tests -//===----------------------------------------------------------------------===// - View return_view_directly (View a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}. return a; // expected-note {{param returned here}} } @@ -61,8 +57,15 @@ View only_one_paramter_annotated (View a [[clang::lifetimebound]], return b; // expected-note {{param returned here}} } +View reassigned_to_another_parameter ( + View a, + View b) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}. + a = b; + return a; // expected-note {{param returned here}} +} + //===----------------------------------------------------------------------===// -// Lifetimebound Annotation Negative Test Cases +// Negative Test Cases //===----------------------------------------------------------------------===// View already_annotated(View a [[clang::lifetimebound]]) { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
