[llvm-branch-commits] [clang] [LifetimeSafety] Prevent duplicate loans and statement visits (PR #153661)
Utkarsh Saxena via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Aug 14 14:20:41 PDT 2025
https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/153661
>From 3bb7fcab300b49458c38e77f069cfce865ed5da1 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Thu, 14 Aug 2025 06:57:44 +0000
Subject: [PATCH] [LifetimeSafety] Track view types/gsl::Pointer.
---
clang/lib/Analysis/LifetimeSafety.cpp | 8 +++
.../unittests/Analysis/LifetimeSafetyTest.cpp | 53 +++++++++++--------
2 files changed, 40 insertions(+), 21 deletions(-)
diff --git a/clang/lib/Analysis/LifetimeSafety.cpp b/clang/lib/Analysis/LifetimeSafety.cpp
index c762f63c45e09..86d9517dde45c 100644
--- a/clang/lib/Analysis/LifetimeSafety.cpp
+++ b/clang/lib/Analysis/LifetimeSafety.cpp
@@ -396,6 +396,7 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
// initializations and destructions are processed in the correct sequence.
for (const CFGBlock *Block : *AC.getAnalysis<PostOrderCFGView>()) {
CurrentBlockFacts.clear();
+ VisitedStmts.clear();
for (unsigned I = 0; I < Block->size(); ++I) {
const CFGElement &Element = Block->Elements[I];
if (std::optional<CFGStmt> CS = Element.getAs<CFGStmt>())
@@ -408,6 +409,12 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
}
}
+ void Visit(const Stmt *S) {
+ if (!S || !VisitedStmts.insert(S).second)
+ return;
+ Base::Visit(S);
+ }
+
void VisitDeclStmt(const DeclStmt *DS) {
for (const Decl *D : DS->decls())
if (const auto *VD = dyn_cast<VarDecl>(D))
@@ -551,6 +558,7 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
FactManager &FactMgr;
AnalysisDeclContext &AC;
llvm::SmallVector<Fact *> CurrentBlockFacts;
+ llvm::DenseSet<const Stmt *> VisitedStmts;
};
// ========================================================================= //
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
More information about the llvm-branch-commits
mailing list