[clang] Adding use-after-return in Lifetime Analysis (PR #165370)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 28 04:07:01 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Kashika Akhouri (kashika0112)
<details>
<summary>Changes</summary>
Adding "use-after-return" in Lifetime Analysis.
---
Patch is 32.46 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/165370.diff
9 Files Affected:
- (modified) clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h (+19)
- (modified) clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h (+1)
- (modified) clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h (+5)
- (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+1)
- (modified) clang/lib/Analysis/LifetimeSafety/Checker.cpp (+74-3)
- (modified) clang/lib/Analysis/LifetimeSafety/Dataflow.h (+3)
- (modified) clang/lib/Analysis/LifetimeSafety/Facts.cpp (+8)
- (modified) clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp (+5-1)
- (modified) clang/lib/Sema/AnalysisBasedWarnings.cpp (+213-202)
``````````diff
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h
index 6a90aeb01e638..712e1d42e2966 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 {
Use,
/// A marker for a specific point in the code, for testing.
TestPoint,
+ OriginEscapes,
+ // An origin that stores a loan escapes the function.
};
private:
@@ -142,6 +144,23 @@ class ReturnOfOriginFact : public Fact {
const OriginManager &OM) const override;
};
+class OriginEscapesFact : public Fact {
+ OriginID OID;
+ const Stmt *EscapeSource;
+
+public:
+ static bool classof(const Fact *F) {
+ return F->getKind() == Kind::OriginEscapes;
+ }
+
+ OriginEscapesFact(OriginID OID, const Stmt *Source)
+ : Fact(Kind::OriginEscapes), OID(OID), EscapeSource(Source) {}
+ OriginID getEscapedOriginID() const { return OID; }
+ const Stmt *getEscapeSource() const { return EscapeSource; }
+ void dump(llvm::raw_ostream &OS, const LoanManager &,
+ const OriginManager &OM) const override;
+};
+
class UseFact : public Fact {
const Expr *UseExpr;
// True if this use is a write operation (e.g., left-hand side of assignment).
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
index 5e58abee2bbb3..ec3c130bb6c66 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
@@ -93,6 +93,7 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> {
FactManager &FactMgr;
AnalysisDeclContext &AC;
llvm::SmallVector<Fact *> CurrentBlockFacts;
+ llvm::SmallVector<Fact *> EscapesInCurrentBlock;
// To distinguish between reads and writes for use-after-free checks, this map
// stores the `UseFact` for each `DeclRefExpr`. We initially identify all
// `DeclRefExpr`s as "read" uses. When an assignment is processed, the use
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
index 91ffbb169f947..f5b1b3d9b6564 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
@@ -42,6 +42,11 @@ class LifetimeSafetyReporter {
virtual void reportUseAfterFree(const Expr *IssueExpr, const Expr *UseExpr,
SourceLocation FreeLoc,
Confidence Confidence) {}
+
+ virtual void reportUseAfterReturn(const Expr *IssueExpr,
+ const Stmt *EscapeStmt,
+ SourceLocation ExpiryLoc,
+ Confidence Confidence) {}
};
/// The main entry point for the analysis.
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 5ff4cc4b833d9..6ea5e34cab291 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10730,6 +10730,7 @@ def warn_lifetime_safety_loan_expires_strict : Warning<
InGroup<LifetimeSafetyStrict>, 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">;
// 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 c443c3a5d4f9b..ae5c62ef04d72 100644
--- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp
@@ -21,6 +21,7 @@
#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Basic/SourceLocation.h"
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallSet.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/TimeProfiler.h"
@@ -61,10 +62,23 @@ class LifetimeChecker {
AnalysisDeclContext &ADC, LifetimeSafetyReporter *Reporter)
: LoanPropagation(LoanPropagation), LiveOrigins(LiveOrigins), FactMgr(FM),
Reporter(Reporter) {
- for (const CFGBlock *B : *ADC.getAnalysis<PostOrderCFGView>())
- for (const Fact *F : FactMgr.getFacts(B))
- if (const auto *EF = F->getAs<ExpireFact>())
+ for (const CFGBlock *B : *ADC.getAnalysis<PostOrderCFGView>()) {
+ llvm::SmallVector<const ExpireFact *> BlockExpires;
+ llvm::SmallVector<const OriginEscapesFact *> BlockEscapes;
+ for (const Fact *F : FactMgr.getFacts(B)) {
+ if (const auto *EF = F->getAs<ExpireFact>()) {
checkExpiry(EF);
+ BlockExpires.push_back(EF);
+ } else if (const auto *OEF = F->getAs<OriginEscapesFact>()) {
+ BlockEscapes.push_back(OEF);
+ }
+ }
+ if (Reporter) {
+ for (const OriginEscapesFact *OEF : BlockEscapes) {
+ checkEscape(OEF, BlockExpires);
+ }
+ }
+ }
issuePendingWarnings();
}
@@ -106,6 +120,63 @@ class LifetimeChecker {
/*ConfidenceLevel=*/CurConfidence};
}
+ void checkEscape(const OriginEscapesFact *OEF,
+ llvm::ArrayRef<const ExpireFact *> BlockExpires) {
+
+ if (!Reporter) {
+ return;
+ }
+
+ OriginID returnedOID = OEF->getEscapedOriginID();
+ ProgramPoint PP = OEF;
+
+ LoanSet HeldLoans = LoanPropagation.getLoans(returnedOID, PP);
+ if (HeldLoans.isEmpty()) {
+ return;
+ }
+
+ llvm::SmallSet<LoanID, 4> ExpiredLoansInBlock;
+ llvm::DenseMap<LoanID, SourceLocation> ExpiryLocs;
+
+ for (const ExpireFact *EF : BlockExpires) {
+ ExpiredLoansInBlock.insert(EF->getLoanID());
+ ExpiryLocs[EF->getLoanID()] = EF->getExpiryLoc();
+ }
+
+ bool hasExpiredDependency = false;
+ bool allHeldLoansExpired = true;
+ LoanID exampleExpiredLoan = LoanID();
+
+ for (LoanID heldLoan : HeldLoans) {
+ if (ExpiredLoansInBlock.count(heldLoan)) {
+ hasExpiredDependency = true;
+ if (exampleExpiredLoan.Value == LoanID().Value) {
+ exampleExpiredLoan = heldLoan;
+ }
+ } else {
+ allHeldLoansExpired = false;
+ }
+ }
+
+ if (!hasExpiredDependency) {
+ return;
+ }
+
+ Confidence FinalConfidence;
+ if (allHeldLoansExpired) {
+ FinalConfidence = Confidence::Definite;
+ } else {
+ FinalConfidence = Confidence::Maybe;
+ }
+
+ const Loan &L = FactMgr.getLoanMgr().getLoan(exampleExpiredLoan);
+ SourceLocation ExpiryLoc = ExpiryLocs[exampleExpiredLoan];
+ const Stmt *EscapeSource = OEF->getEscapeSource();
+
+ Reporter->reportUseAfterReturn(L.IssueExpr, EscapeSource, ExpiryLoc,
+ FinalConfidence);
+ }
+
void issuePendingWarnings() {
if (!Reporter)
return;
diff --git a/clang/lib/Analysis/LifetimeSafety/Dataflow.h b/clang/lib/Analysis/LifetimeSafety/Dataflow.h
index 2f7bcb6e5dc81..37720ffa03618 100644
--- a/clang/lib/Analysis/LifetimeSafety/Dataflow.h
+++ b/clang/lib/Analysis/LifetimeSafety/Dataflow.h
@@ -168,6 +168,8 @@ class DataflowAnalysis {
return D->transfer(In, *F->getAs<OriginFlowFact>());
case Fact::Kind::ReturnOfOrigin:
return D->transfer(In, *F->getAs<ReturnOfOriginFact>());
+ case Fact::Kind::OriginEscapes:
+ return D->transfer(In, *F->getAs<OriginEscapesFact>());
case Fact::Kind::Use:
return D->transfer(In, *F->getAs<UseFact>());
case Fact::Kind::TestPoint:
@@ -181,6 +183,7 @@ class DataflowAnalysis {
Lattice transfer(Lattice In, const ExpireFact &) { return In; }
Lattice transfer(Lattice In, const OriginFlowFact &) { return In; }
Lattice transfer(Lattice In, const ReturnOfOriginFact &) { 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 1aea64f864367..29c75959ba0fe 100644
--- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp
@@ -50,6 +50,14 @@ void ReturnOfOriginFact::dump(llvm::raw_ostream &OS, const LoanManager &,
OS << ")\n";
}
+void OriginEscapesFact::dump(llvm::raw_ostream &OS, const LoanManager &,
+ const OriginManager &OM) const {
+ OS << "OriginEscapes (";
+ OM.dump(getEscapedOriginID(), OS);
+ OS << ")\n";
+}
+
+
void UseFact::dump(llvm::raw_ostream &OS, const LoanManager &,
const OriginManager &OM) const {
OS << "Use (";
diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
index 9b68de107e314..762ed07eb5bb8 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -58,6 +58,7 @@ void FactsGenerator::run() {
// initializations and destructions are processed in the correct sequence.
for (const CFGBlock *Block : *AC.getAnalysis<PostOrderCFGView>()) {
CurrentBlockFacts.clear();
+ EscapesInCurrentBlock.clear();
for (unsigned I = 0; I < Block->size(); ++I) {
const CFGElement &Element = Block->Elements[I];
if (std::optional<CFGStmt> CS = Element.getAs<CFGStmt>())
@@ -66,6 +67,8 @@ void FactsGenerator::run() {
Element.getAs<CFGAutomaticObjDtor>())
handleDestructor(*DtorOpt);
}
+ CurrentBlockFacts.append(EscapesInCurrentBlock.begin(),
+ EscapesInCurrentBlock.end());
FactMgr.addBlockFacts(Block, CurrentBlockFacts);
}
}
@@ -166,7 +169,8 @@ void FactsGenerator::VisitReturnStmt(const ReturnStmt *RS) {
if (const Expr *RetExpr = RS->getRetValue()) {
if (hasOrigin(RetExpr)) {
OriginID OID = FactMgr.getOriginMgr().getOrCreate(*RetExpr);
- CurrentBlockFacts.push_back(FactMgr.createFact<ReturnOfOriginFact>(OID));
+ EscapesInCurrentBlock.push_back(
+ FactMgr.createFact<OriginEscapesFact>(OID, RS));
}
}
}
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 140b709dbb651..879793575e0b4 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -65,61 +65,61 @@ using namespace clang;
//===----------------------------------------------------------------------===//
namespace {
- class UnreachableCodeHandler : public reachable_code::Callback {
- Sema &S;
- SourceRange PreviousSilenceableCondVal;
-
- public:
- UnreachableCodeHandler(Sema &s) : S(s) {}
-
- void HandleUnreachable(reachable_code::UnreachableKind UK, SourceLocation L,
- SourceRange SilenceableCondVal, SourceRange R1,
- SourceRange R2, bool HasFallThroughAttr) override {
- // If the diagnosed code is `[[fallthrough]];` and
- // `-Wunreachable-code-fallthrough` is enabled, suppress `code will never
- // be executed` warning to avoid generating diagnostic twice
- if (HasFallThroughAttr &&
- !S.getDiagnostics().isIgnored(diag::warn_unreachable_fallthrough_attr,
- SourceLocation()))
- return;
+class UnreachableCodeHandler : public reachable_code::Callback {
+ Sema &S;
+ SourceRange PreviousSilenceableCondVal;
- // Avoid reporting multiple unreachable code diagnostics that are
- // triggered by the same conditional value.
+public:
+ UnreachableCodeHandler(Sema &s) : S(s) {}
+
+ void HandleUnreachable(reachable_code::UnreachableKind UK, SourceLocation L,
+ SourceRange SilenceableCondVal, SourceRange R1,
+ SourceRange R2, bool HasFallThroughAttr) override {
+ // If the diagnosed code is `[[fallthrough]];` and
+ // `-Wunreachable-code-fallthrough` is enabled, suppress `code will never
+ // be executed` warning to avoid generating diagnostic twice
+ if (HasFallThroughAttr &&
+ !S.getDiagnostics().isIgnored(diag::warn_unreachable_fallthrough_attr,
+ SourceLocation()))
+ return;
+
+ // Avoid reporting multiple unreachable code diagnostics that are
+ // triggered by the same conditional value.
if (PreviousSilenceableCondVal.isValid() &&
SilenceableCondVal.isValid() &&
- PreviousSilenceableCondVal == SilenceableCondVal)
- return;
- PreviousSilenceableCondVal = SilenceableCondVal;
+ PreviousSilenceableCondVal == SilenceableCondVal)
+ return;
+ PreviousSilenceableCondVal = SilenceableCondVal;
- unsigned diag = diag::warn_unreachable;
- switch (UK) {
- case reachable_code::UK_Break:
- diag = diag::warn_unreachable_break;
- break;
- case reachable_code::UK_Return:
- diag = diag::warn_unreachable_return;
- break;
- case reachable_code::UK_Loop_Increment:
- diag = diag::warn_unreachable_loop_increment;
- break;
- case reachable_code::UK_Other:
- break;
- }
+ unsigned diag = diag::warn_unreachable;
+ switch (UK) {
+ case reachable_code::UK_Break:
+ diag = diag::warn_unreachable_break;
+ break;
+ case reachable_code::UK_Return:
+ diag = diag::warn_unreachable_return;
+ break;
+ case reachable_code::UK_Loop_Increment:
+ diag = diag::warn_unreachable_loop_increment;
+ break;
+ case reachable_code::UK_Other:
+ break;
+ }
- S.Diag(L, diag) << R1 << R2;
+ S.Diag(L, diag) << R1 << R2;
- SourceLocation Open = SilenceableCondVal.getBegin();
- if (Open.isValid()) {
- SourceLocation Close = SilenceableCondVal.getEnd();
- Close = S.getLocForEndOfToken(Close);
- if (Close.isValid()) {
- S.Diag(Open, diag::note_unreachable_silence)
+ SourceLocation Open = SilenceableCondVal.getBegin();
+ if (Open.isValid()) {
+ SourceLocation Close = SilenceableCondVal.getEnd();
+ Close = S.getLocForEndOfToken(Close);
+ if (Close.isValid()) {
+ S.Diag(Open, diag::note_unreachable_silence)
<< FixItHint::CreateInsertion(Open, "/* DISABLES CODE */ (")
<< FixItHint::CreateInsertion(Close, ")");
- }
}
}
- };
+ }
+};
} // anonymous namespace
/// CheckUnreachable - Check for unreachable code.
@@ -388,9 +388,9 @@ static void checkThrowInNonThrowingFunc(Sema &S, const FunctionDecl *FD,
if (BodyCFG->getExit().pred_empty())
return;
visitReachableThrows(BodyCFG, [&](const CXXThrowExpr *Throw, CFGBlock &Block) {
- if (throwEscapes(S, Throw, Block, BodyCFG))
- EmitDiagForCXXThrowInNonThrowingFunc(S, Throw->getThrowLoc(), FD);
- });
+ if (throwEscapes(S, Throw, Block, BodyCFG))
+ EmitDiagForCXXThrowInNonThrowingFunc(S, Throw->getThrowLoc(), FD);
+ });
}
static bool isNoexcept(const FunctionDecl *FD) {
@@ -803,7 +803,7 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body,
}
else if (isa<BlockDecl>(D)) {
if (const FunctionType *FT =
- BlockType->getPointeeType()->getAs<FunctionType>()) {
+ BlockType->getPointeeType()->getAs<FunctionType>()) {
if (FT->getReturnType()->isVoidType())
ReturnsVoid = true;
if (FT->getNoReturnAttr())
@@ -815,7 +815,7 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body,
// Short circuit for compilation speed.
if (CD.checkDiagnostics(Diags, ReturnsVoid, HasNoReturn))
- return;
+ return;
SourceLocation LBrace = Body->getBeginLoc(), RBrace = Body->getEndLoc();
// cpu_dispatch functions permit empty function bodies for ICC compatibility.
@@ -892,7 +892,7 @@ class ContainsReference : public ConstEvaluatedExprVisitor<ContainsReference> {
typedef ConstEvaluatedExprVisitor<ContainsReference> Inherited;
ContainsReference(ASTContext &Context, const DeclRefExpr *Needle)
- : Inherited(Context), FoundReference(false), Needle(Needle) {}
+ : Inherited(Context), FoundReference(false), Needle(Needle) {}
void VisitExpr(const Expr *E) {
// Stop evaluating if we already have a reference.
@@ -1016,7 +1016,7 @@ static void DiagUninitUse(Sema &S, const VarDecl *VD, const UninitUse &Use,
int RemoveDiagKind = -1;
const char *FixitStr =
S.getLangOpts().CPlusPlus ? (I->Output ? "true" : "false")
- : (I->Output ? "1" : "0");
+ : (I->Output ? "1" : "0");
FixItHint Fixit1, Fixit2;
switch (Term ? Term->getStmtClass() : Stmt::DeclStmtClass) {
@@ -1124,7 +1124,7 @@ static void DiagUninitUse(Sema &S, const VarDecl *VD, const UninitUse &Use,
<< IsCapturedByBlock << User->getSourceRange();
if (RemoveDiagKind != -1)
S.Diag(Fixit1.RemoveRange.getBegin(), diag::note_uninit_fixit_remove_cond)
- << RemoveDiagKind << Str << I->Output << Fixit1 << Fixit2;
+ << RemoveDiagKind << Str << I->Output << Fixit1 << Fixit2;
Diagnosed = true;
}
@@ -1294,32 +1294,32 @@ class FallthroughMapper : public DynamicRecursiveASTVisitor {
// Don't care about other unreachable statements.
}
}
- // If there are no unreachable statements, this may be a special
- // case in CFG:
- // case X: {
- // A a; // A has a destructor.
- // break;
- // }
- // // <<<< This place is represented by a 'hanging' CFG block.
- // case Y:
- continue;
+ // If there are no unreachable statements, this may be a special
+ // case in CFG:
+ // case X: {
+ // A a; // A has a destructor.
+ // break;
+ // }
+ // // <<<< This place is represented by a 'hanging' CFG block.
+ // case Y:
+ continue;
}
- const Stmt *LastStmt = getLastStmt(*P);
- if (const AttributedStmt *AS = asFallThroughAttr(LastStmt)) {
- markFallthroughVisited(AS);
- ++AnnotatedCnt;
- continue; // Fallthrough annotation, good.
- }
+ const Stmt *LastStmt = getLastStmt(*P);
+ if (const AttributedStmt *AS = asFallThroughAttr(LastStmt)) {
+ markFallthroughVisited(AS);
+ ++AnnotatedCnt;
+ continue; // Fallthrough annotation, good.
+ }
- if (!LastStmt) { // This block contains no executable statements.
- // Traverse its predecessors.
- std::copy(P->pred_begin(), P->pred_end(),
- std::back_inserter(BlockQueue));
- continue;
- }
+ if (!LastStmt) { // This block contains no executable statements.
+ // Traverse its predecessors.
+ std::copy(P->pred_begin(), P->pred_end(),
+ std::back_inserter(BlockQueue));
+ continue;
+ }
- ++UnannotatedCnt;
+ ++UnannotatedCnt;
}
return !!UnannotatedCnt;
}
@@ -1335,48 +1335,48 @@ class FallthroughMapper : public DynamicRecursiveASTVisitor {
return true;
}
- // We don't want to traverse local type declarations. We analyze their
- // methods separately.
- bool TraverseDecl(Decl *D) override { return true; }
+ // We don't want to traverse local type declarations. We analyze their
+ // methods separately.
+ bool TraverseDecl(Decl *D) override { return true; }
- // We analyze lambda bodies separately. Skip them here.
- bool TraverseLambdaExpr(LambdaExpr *LE) override {
- // Traverse the captures, but not the body.
- for (const auto C : zip(LE->captures(), LE->capture_inits()))
- TraverseLambdaCapture(LE, &std::get<0>(C), std::get<1>(C));
- return true;
- }
+ // We analyze lambda bodies separately. Skip them here.
+ bool TraverseLambdaExpr(LambdaExpr *LE) override {
+ // Trave...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/165370
More information about the cfe-commits
mailing list