[LifetimeSafety] Suggest lifetime annotations #169767
Open
+268
−33
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang-analysis @llvm/pr-subscribers-clang Author: Kashika Akhouri (kashika0112) ChangesAdd lifetime annotation suggestion in lifetime analysis. This PR introduces a new feature to Clang's lifetime analysis to detect and suggest missing This warning, controlled by the Example: Facts: Sample warning: Full diff: https://github.com/llvm/llvm-project/pull/169767.diff 10 Files Affected:
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;
+}
|
Member
|
@llvm/pr-subscribers-clang-temporal-safety Author: Kashika Akhouri (kashika0112) ChangesAdd lifetime annotation suggestion in lifetime analysis. This PR introduces a new feature to Clang's lifetime analysis to detect and suggest missing This warning, controlled by the Example: Facts: Sample warning: Full diff: https://github.com/llvm/llvm-project/pull/169767.diff 10 Files Affected:
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;
+}
|
🐧 Linux x64 Test Results
|
Xazax-hun
reviewed
Nov 27, 2025
Collaborator
Xazax-hun
left a comment
Xazax-hun
left a comment
There was a problem hiding this comment.
This looks amazing! Thanks for looking into this! Most of my comments are about making sure this change is a strong basis for some future improvements that we might want to make.
|
|
||
| for (const ParmVarDecl *PVD : FD->parameters()) { | ||
| QualType ParamType = PVD->getType(); | ||
| if (PVD->hasAttr<LifetimeBoundAttr>()) |
Collaborator
There was a problem hiding this comment.
We do not necessarily need to address this now, but I think we might want to have loans for the lifetimebound annotated parameters as well. This would help as find invalidation style issues in the future:
int* func(std::vector<int>& v [[clang::lifetimebound]]) {
v.push_back(0);
return v[0];
}
Also, having loans for annotated parameters would help us potentially find erroneous annotations:
int* func(std::vector<int>& v1 [[clang::lifetimebound]], // oops, added to the wrong parameter.
std::vector<int>& v2) {
return v2[0];
}
Contributor
There was a problem hiding this comment.
+1. The reporting at the end should take care of not suggesting already annotated params instead of dropping the placeholder loans.
These loans would also help us improve C++ coverage metric: "Origins with no loans". It seems helpful to differentiate between the case when origins hold placeholders vs origins hold no loans (because of being either unknown of analysis bug). So these loans serve mutliple purposes beyond lifetime suggestions alone.
Contributor
Author
There was a problem hiding this comment.
Okay, have moved this check to the reporting function.
usx95
changed the title
usx95
reviewed
Nov 28, 2025
Contributor
usx95
left a comment
usx95
left a comment
There was a problem hiding this comment.
This looks amazing. I have dropped comments mostly for some structural changes.
|
|
||
| for (const ParmVarDecl *PVD : FD->parameters()) { | ||
| QualType ParamType = PVD->getType(); | ||
| if (PVD->hasAttr<LifetimeBoundAttr>()) |
Contributor
There was a problem hiding this comment.
+1. The reporting at the end should take care of not suggesting already annotated params instead of dropping the placeholder loans.
These loans would also help us improve C++ coverage metric: "Origins with no loans". It seems helpful to differentiate between the case when origins hold placeholders vs origins hold no loans (because of being either unknown of analysis bug). So these loans serve mutliple purposes beyond lifetime suggestions alone.
clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
Outdated
Show resolved
Hide resolved
Xazax-hun
approved these changes
Nov 28, 2025
Collaborator
Xazax-hun
left a comment
Xazax-hun
left a comment
There was a problem hiding this comment.
This looks great, thanks!
There is one significant feature that would be nice to support, detecting missing lifetimebound annotations for the implicit object parameter! I think that is probably better be addressed in a follow-up though and that might be a bit more involved.
usx95
reviewed
Nov 28, 2025
usx95
reviewed
Nov 28, 2025
Contributor
usx95
left a comment
usx95
left a comment
There was a problem hiding this comment.
Thanks. LGTM.
Last few comments:
- I would suggest to build LLVM using this to be sure we do not introduce crashes. Maybe see if we have some suggestions already.
- Filed a bug for implicit this #169941. Please add these tests with a FIXME quoting the bug.
| } | ||
| } | ||
|
|
||
| void issueAnnotationWarnings() { |
Contributor
There was a problem hiding this comment.
nit: rename to suggestAnnotations
usx95
approved these changes
Nov 28, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add lifetime annotation suggestion in lifetime analysis.
This PR introduces a new feature to Clang's lifetime analysis to detect and suggest missing
[[clang::lifetimebound]]annotations on function parameters.It introduces the concept of
placeholder loans. At the entry of a function, a special placeholder loan is created for each pointer or reference parameter. The analysis then tracks these loans usingOriginFlowfacts. If anOriginEscapesFactshows that an origin holding a placeholder loan escapes the function's scope (e.g., via a return statement), a new warning is issued.This warning, controlled by the warning flag
-Wexperimental-lifetime-safety-suggestions, suggests adding the[[clang::lifetimebound]]attribute to the corresponding parameter.Example:
Facts:
Sample warning:
Fixes: #169939