https://github.com/usx95 updated
https://github.com/llvm/llvm-project/pull/153661
>From 2f329e7c06633b293988d56aea92768d6fe388b5 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <u...@google.com>
Date: Thu, 14 Aug 2025 06:57:44 +0000
Subject: [PATCH 1/2] [LifetimeSafety] Track view types/gsl::Pointer.
---
clang/lib/Analysis/LifetimeSafety.cpp | 109 ++++++++++++------
.../unittests/Analysis/LifetimeSafetyTest.cpp | 53 +++++----
2 files changed, 107 insertions(+), 55 deletions(-)
diff --git a/clang/lib/Analysis/LifetimeSafety.cpp
b/clang/lib/Analysis/LifetimeSafety.cpp
index ba9f7d0f6ee36..a981d246f3d34 100644
--- a/clang/lib/Analysis/LifetimeSafety.cpp
+++ b/clang/lib/Analysis/LifetimeSafety.cpp
@@ -8,6 +8,7 @@
#include "clang/Analysis/Analyses/LifetimeSafety.h"
#include "clang/AST/Decl.h"
#include "clang/AST/Expr.h"
+#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/AST/StmtVisitor.h"
#include "clang/AST/Type.h"
#include "clang/Analysis/Analyses/PostOrderCFGView.h"
@@ -387,25 +388,16 @@ class FactGenerator : public
ConstStmtVisitor<FactGenerator> {
using Base = ConstStmtVisitor<FactGenerator>;
public:
- FactGenerator(FactManager &FactMgr, AnalysisDeclContext &AC)
- : FactMgr(FactMgr), AC(AC) {}
+ FactGenerator(FactManager &FactMgr) : FactMgr(FactMgr) {}
- void run() {
- llvm::TimeTraceScope TimeProfile("FactGenerator");
- // 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();
- for (unsigned I = 0; I < Block->size(); ++I) {
- const CFGElement &Element = Block->Elements[I];
- if (std::optional<CFGStmt> CS = Element.getAs<CFGStmt>())
- Visit(CS->getStmt());
- else if (std::optional<CFGAutomaticObjDtor> DtorOpt =
- Element.getAs<CFGAutomaticObjDtor>())
- handleDestructor(*DtorOpt);
- }
- FactMgr.addBlockFacts(Block, CurrentBlockFacts);
- }
+ void startBlock(const CFGBlock *Block) {
+ CurrentBlock = Block;
+ CurrentBlockFacts.clear();
+ }
+
+ void endBlock() {
+ FactMgr.addBlockFacts(CurrentBlock, CurrentBlockFacts);
+ startBlock(nullptr);
}
void VisitDeclStmt(const DeclStmt *DS) {
@@ -425,7 +417,6 @@ class FactGenerator : public
ConstStmtVisitor<FactGenerator> {
void VisitImplicitCastExpr(const ImplicitCastExpr *ICE) {
if (!hasOrigin(ICE->getType()))
return;
- Visit(ICE->getSubExpr());
// An ImplicitCastExpr node itself gets an origin, which flows from the
// origin of its sub-expression (after stripping its own parens/casts).
// TODO: Consider if this is actually useful in practice. Alternatively, we
@@ -493,18 +484,6 @@ class FactGenerator : public
ConstStmtVisitor<FactGenerator> {
Base::VisitCXXFunctionalCastExpr(FCE);
}
-private:
- // Check if a type has an origin.
- bool hasOrigin(QualType QT) { return QT->isPointerOrReferenceType(); }
-
- template <typename Destination, typename Source>
- void addAssignOriginFact(const Destination &D, const Source &S) {
- OriginID DestOID = FactMgr.getOriginMgr().getOrCreate(D);
- OriginID SrcOID = FactMgr.getOriginMgr().get(S);
- CurrentBlockFacts.push_back(
- FactMgr.createFact<AssignOriginFact>(DestOID, SrcOID));
- }
-
void handleDestructor(const CFGAutomaticObjDtor &DtorOpt) {
/// TODO: Also handle trivial destructors (e.g., for `int`
/// variables) which will never have a CFGAutomaticObjDtor node.
@@ -527,6 +506,18 @@ class FactGenerator : public
ConstStmtVisitor<FactGenerator> {
}
}
+private:
+ // Check if a type has an origin.
+ bool hasOrigin(QualType QT) { return QT->isPointerOrReferenceType(); }
+
+ template <typename Destination, typename Source>
+ void addAssignOriginFact(const Destination &D, const Source &S) {
+ OriginID DestOID = FactMgr.getOriginMgr().getOrCreate(D);
+ OriginID SrcOID = FactMgr.getOriginMgr().get(S);
+ CurrentBlockFacts.push_back(
+ FactMgr.createFact<AssignOriginFact>(DestOID, SrcOID));
+ }
+
/// Checks if the expression is a `void("__lifetime_test_point_...")` cast.
/// If so, creates a `TestPointFact` and returns true.
bool VisitTestPoint(const CXXFunctionalCastExpr *FCE) {
@@ -549,10 +540,59 @@ class FactGenerator : public
ConstStmtVisitor<FactGenerator> {
}
FactManager &FactMgr;
- AnalysisDeclContext &AC;
+ const CFGBlock *CurrentBlock = nullptr;
llvm::SmallVector<Fact *> CurrentBlockFacts;
};
+class FactGeneratorDriver : public RecursiveASTVisitor<FactGeneratorDriver> {
+public:
+ FactGeneratorDriver(FactGenerator &FG, AnalysisDeclContext &AC)
+ : FG(FG), AC(AC) {}
+ bool shouldTraversePostOrder() const { return true; }
+ void run() {
+ llvm::TimeTraceScope TimeProfile("FactGenerator");
+ // 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>()) {
+ FactGeneratorBlockRAII BlockGenerator(FG, Block);
+ for (const CFGElement &Element : *Block) {
+ if (std::optional<CFGStmt> CS = Element.getAs<CFGStmt>())
+ TraverseStmt(const_cast<Stmt *>(CS->getStmt()));
+ else if (std::optional<CFGAutomaticObjDtor> DtorOpt =
+ Element.getAs<CFGAutomaticObjDtor>())
+ FG.handleDestructor(*DtorOpt);
+ }
+ }
+ }
+
+ bool TraverseStmt(Stmt *S) {
+ // Avoid re-visiting nodes to not create duplicate facts.
+ if (!S || !VisitedStmts.insert(S).second)
+ return true;
+ return RecursiveASTVisitor::TraverseStmt(S);
+ }
+
+ bool VisitStmt(Stmt *S) {
+ FG.Visit(S);
+ return true; // Continue traversing to children.
+ }
+
+private:
+ struct FactGeneratorBlockRAII {
+ FactGeneratorBlockRAII(FactGenerator &FG, const CFGBlock *Block) : FG(FG) {
+ FG.startBlock(Block);
+ }
+ ~FactGeneratorBlockRAII() { FG.endBlock(); }
+
+ private:
+ FactGenerator FG;
+ };
+
+ FactGenerator &FG;
+ AnalysisDeclContext &AC;
+ llvm::DenseSet<const Stmt *> VisitedStmts;
+};
+
// ========================================================================= //
// Generic Dataflow Analysis
// ========================================================================= //
@@ -1096,8 +1136,9 @@ void LifetimeSafetyAnalysis::run() {
DEBUG_WITH_TYPE("PrintCFG", Cfg.dump(AC.getASTContext().getLangOpts(),
/*ShowColors=*/true));
- FactGenerator FactGen(*FactMgr, AC);
- FactGen.run();
+ FactGenerator Generator(*FactMgr);
+ FactGeneratorDriver Driver(Generator, AC);
+ Driver.run();
DEBUG_WITH_TYPE("LifetimeFacts", FactMgr->dump(Cfg, AC));
/// TODO(opt): Consider optimizing individual blocks before running the
diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp
b/clang/unittests/Analysis/LifetimeSafetyTest.cpp
index c8d88b4ea2277..13e5832d70050 100644
--- a/clang/unittests/Analysis/LifetimeSafetyTest.cpp
+++ b/clang/unittests/Analysis/LifetimeSafetyTest.cpp
@@ -11,6 +11,7 @@
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Testing/TestAST.h"
#include "llvm/ADT/StringMap.h"
+#include "llvm/Testing/Support/Error.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include <optional>
@@ -20,6 +21,7 @@ namespace clang::lifetimes::internal {
namespace {
using namespace ast_matchers;
+using ::testing::SizeIs;
using ::testing::UnorderedElementsAreArray;
// A helper class to run the full lifetime analysis on a piece of code
@@ -96,21 +98,18 @@ class LifetimeTestHelper {
return OID;
}
- std::optional<LoanID> getLoanForVar(llvm::StringRef VarName) {
+ std::vector<LoanID> getLoansForVar(llvm::StringRef VarName) {
auto *VD = findDecl<VarDecl>(VarName);
- if (!VD)
- return std::nullopt;
+ if (!VD) {
+ ADD_FAILURE() << "No VarDecl found for '" << VarName << "'";
+ return {};
+ }
std::vector<LoanID> LID = Analysis.getLoanIDForVar(VD);
if (LID.empty()) {
ADD_FAILURE() << "Loan for '" << VarName << "' not found.";
- return std::nullopt;
- }
- // TODO: Support retrieving more than one loans to a var.
- if (LID.size() > 1) {
- ADD_FAILURE() << "More than 1 loans found for '" << VarName;
- return std::nullopt;
+ return {};
}
- return LID[0];
+ return LID;
}
std::optional<LoanSet> getLoansAtPoint(OriginID OID,
@@ -121,13 +120,12 @@ class LifetimeTestHelper {
return Analysis.getLoansAtPoint(OID, PP);
}
- std::optional<llvm::DenseSet<LoanID>>
+ std::optional<std::vector<LoanID>>
getExpiredLoansAtPoint(llvm::StringRef Annotation) {
ProgramPoint PP = Runner.getProgramPoint(Annotation);
if (!PP)
return std::nullopt;
- auto Expired = Analysis.getExpiredLoansAtPoint(PP);
- return llvm::DenseSet<LoanID>{Expired.begin(), Expired.end()};
+ return Analysis.getExpiredLoansAtPoint(PP);
}
private:
@@ -197,12 +195,13 @@ MATCHER_P2(HasLoansToImpl, LoanVars, Annotation, "") {
std::vector<LoanID> ExpectedLoans;
for (const auto &LoanVar : LoanVars) {
- std::optional<LoanID> ExpectedLIDOpt = Info.Helper.getLoanForVar(LoanVar);
- if (!ExpectedLIDOpt) {
+ std::vector<LoanID> ExpectedLIDs = Info.Helper.getLoansForVar(LoanVar);
+ if (ExpectedLIDs.empty()) {
*result_listener << "could not find loan for var '" << LoanVar << "'";
return false;
}
- ExpectedLoans.push_back(*ExpectedLIDOpt);
+ ExpectedLoans.insert(ExpectedLoans.end(), ExpectedLIDs.begin(),
+ ExpectedLIDs.end());
}
return ExplainMatchResult(UnorderedElementsAreArray(ExpectedLoans),
@@ -221,17 +220,17 @@ MATCHER_P(AreExpiredAt, Annotation, "") {
<< Annotation << "'";
return false;
}
- std::vector<LoanID> ActualExpiredLoans(ActualExpiredSetOpt->begin(),
- ActualExpiredSetOpt->end());
+ std::vector<LoanID> ActualExpiredLoans = *ActualExpiredSetOpt;
std::vector<LoanID> ExpectedExpiredLoans;
for (const auto &VarName : Info.LoanVars) {
- auto LoanIDOpt = Helper.getLoanForVar(VarName);
- if (!LoanIDOpt) {
+ auto LoanIDs = Helper.getLoansForVar(VarName);
+ if (LoanIDs.empty()) {
*result_listener << "could not find a loan for variable '" << VarName
<< "'";
return false;
}
- ExpectedExpiredLoans.push_back(*LoanIDOpt);
+ ExpectedExpiredLoans.insert(ExpectedExpiredLoans.end(), LoanIDs.begin(),
+ LoanIDs.end());
}
return ExplainMatchResult(UnorderedElementsAreArray(ExpectedExpiredLoans),
ActualExpiredLoans, result_listener);
@@ -730,5 +729,17 @@ TEST_F(LifetimeAnalysisTest,
ReassignedPointerThenOriginalExpires) {
EXPECT_THAT(LoansTo({"s1", "s2"}), AreExpiredAt("p_after_s1_expires"));
}
+TEST_F(LifetimeAnalysisTest, NoDuplicateLoansForImplicitCastToConst) {
+ SetupTest(R"(
+ void target() {
+ MyObj a;
+ const MyObj* p = &a;
+ const MyObj* q = &a;
+ POINT(at_end);
+ }
+ )");
+ EXPECT_THAT(Helper->getLoansForVar("a"), SizeIs(2));
+}
+
} // anonymous namespace
} // namespace clang::lifetimes::internal
>From 2eac6ae61506e1c9b35af0327cd20c9ffc8c9b81 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <u...@google.com>
Date: Mon, 18 Aug 2025 16:09:52 +0000
Subject: [PATCH 2/2] slight refactor and format
---
clang/lib/Analysis/LifetimeSafety.cpp | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/clang/lib/Analysis/LifetimeSafety.cpp
b/clang/lib/Analysis/LifetimeSafety.cpp
index a981d246f3d34..fb2ae690cdd60 100644
--- a/clang/lib/Analysis/LifetimeSafety.cpp
+++ b/clang/lib/Analysis/LifetimeSafety.cpp
@@ -384,11 +384,11 @@ class FactManager {
llvm::BumpPtrAllocator FactAllocator;
};
-class FactGenerator : public ConstStmtVisitor<FactGenerator> {
- using Base = ConstStmtVisitor<FactGenerator>;
+class FactGeneratorVisitor : public ConstStmtVisitor<FactGeneratorVisitor> {
+ using Base = ConstStmtVisitor<FactGeneratorVisitor>;
public:
- FactGenerator(FactManager &FactMgr) : FactMgr(FactMgr) {}
+ FactGeneratorVisitor(FactManager &FactMgr) : FactMgr(FactMgr) {}
void startBlock(const CFGBlock *Block) {
CurrentBlock = Block;
@@ -544,11 +544,13 @@ class FactGenerator : public
ConstStmtVisitor<FactGenerator> {
llvm::SmallVector<Fact *> CurrentBlockFacts;
};
-class FactGeneratorDriver : public RecursiveASTVisitor<FactGeneratorDriver> {
+class FactGenerator : public RecursiveASTVisitor<FactGenerator> {
public:
- FactGeneratorDriver(FactGenerator &FG, AnalysisDeclContext &AC)
- : FG(FG), AC(AC) {}
+ FactGenerator(FactManager &FactMgr, AnalysisDeclContext &AC)
+ : FG(FactMgr), AC(AC) {}
+
bool shouldTraversePostOrder() const { return true; }
+
void run() {
llvm::TimeTraceScope TimeProfile("FactGenerator");
// Iterate through the CFG blocks in reverse post-order to ensure that
@@ -579,16 +581,17 @@ class FactGeneratorDriver : public
RecursiveASTVisitor<FactGeneratorDriver> {
private:
struct FactGeneratorBlockRAII {
- FactGeneratorBlockRAII(FactGenerator &FG, const CFGBlock *Block) : FG(FG) {
+ FactGeneratorBlockRAII(FactGeneratorVisitor &FG, const CFGBlock *Block)
+ : FG(FG) {
FG.startBlock(Block);
}
~FactGeneratorBlockRAII() { FG.endBlock(); }
private:
- FactGenerator FG;
+ FactGeneratorVisitor &FG;
};
- FactGenerator &FG;
+ FactGeneratorVisitor FG;
AnalysisDeclContext &AC;
llvm::DenseSet<const Stmt *> VisitedStmts;
};
@@ -1136,9 +1139,8 @@ void LifetimeSafetyAnalysis::run() {
DEBUG_WITH_TYPE("PrintCFG", Cfg.dump(AC.getASTContext().getLangOpts(),
/*ShowColors=*/true));
- FactGenerator Generator(*FactMgr);
- FactGeneratorDriver Driver(Generator, AC);
- Driver.run();
+ FactGenerator FG(*FactMgr, AC);
+ FG.run();
DEBUG_WITH_TYPE("LifetimeFacts", FactMgr->dump(Cfg, AC));
/// TODO(opt): Consider optimizing individual blocks before running the
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
