[llvm-branch-commits] [clang] [LifetimeSafety] Add support for GSL Pointer types (PR #154009)
Utkarsh Saxena via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Wed Sep 3 07:34:32 PDT 2025
https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/154009
>From 447c9ba2cc7d4a886a3c5c40d54bb1b3bcab2255 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Sun, 17 Aug 2025 10:10:18 +0000
Subject: [PATCH] [LifetimeSafety] Track gsl::Pointer types
---
clang/lib/Analysis/LifetimeSafety.cpp | 153 ++++++++++++---
clang/test/Sema/warn-lifetime-safety.cpp | 120 +++++++++++-
.../unittests/Analysis/LifetimeSafetyTest.cpp | 182 +++++++++++++++++-
3 files changed, 419 insertions(+), 36 deletions(-)
diff --git a/clang/lib/Analysis/LifetimeSafety.cpp b/clang/lib/Analysis/LifetimeSafety.cpp
index dbbf7f3cc14b1..875e08431fe02 100644
--- a/clang/lib/Analysis/LifetimeSafety.cpp
+++ b/clang/lib/Analysis/LifetimeSafety.cpp
@@ -445,6 +445,31 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
void VisitDeclRefExpr(const DeclRefExpr *DRE) { handleUse(DRE); }
+ void VisitCXXConstructExpr(const CXXConstructExpr *CCE) {
+ if (!isGslPointerType(CCE->getType()))
+ return;
+ if (CCE->getNumArgs() != 1)
+ return;
+ if (hasOrigin(CCE->getArg(0)->getType()))
+ addAssignOriginFact(*CCE, *CCE->getArg(0));
+ else
+ // This could be a new borrow.
+ handleFucntionCall(CCE, CCE->getConstructor(),
+ {CCE->getArgs(), CCE->getNumArgs()});
+ }
+
+ void VisitCXXMemberCallExpr(const CXXMemberCallExpr *MCE) {
+ // Specifically for conversion operators,
+ // like `std::string_view p = std::string{};`
+ if (isGslPointerType(MCE->getType()) &&
+ isa<CXXConversionDecl>(MCE->getCalleeDecl())) {
+ // The argument is the implicit object itself.
+ handleFucntionCall(MCE, MCE->getMethodDecl(),
+ {MCE->getImplicitObjectArgument()});
+ }
+ // FIXME: A more general VisitCallExpr could also be used here.
+ }
+
void VisitCXXNullPtrLiteralExpr(const CXXNullPtrLiteralExpr *N) {
/// TODO: Handle nullptr expr as a special 'null' loan. Uninitialized
/// pointers can use the same type of loan.
@@ -465,18 +490,9 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
void VisitUnaryOperator(const UnaryOperator *UO) {
if (UO->getOpcode() == UO_AddrOf) {
const Expr *SubExpr = UO->getSubExpr();
- if (const auto *DRE = dyn_cast<DeclRefExpr>(SubExpr)) {
- if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
- // Check if it's a local variable.
- if (VD->hasLocalStorage()) {
- OriginID OID = FactMgr.getOriginMgr().getOrCreate(*UO);
- AccessPath AddrOfLocalVarPath(VD);
- const Loan &L =
- FactMgr.getLoanMgr().addLoan(AddrOfLocalVarPath, UO);
- CurrentBlockFacts.push_back(
- FactMgr.createFact<IssueFact>(L.ID, OID));
- }
- }
+ if (const Loan *L = createLoanFrom(SubExpr, UO)) {
+ OriginID OID = FactMgr.getOriginMgr().getOrCreate(*UO);
+ CurrentBlockFacts.push_back(FactMgr.createFact<IssueFact>(L->ID, OID));
}
}
}
@@ -504,22 +520,27 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
void VisitCXXFunctionalCastExpr(const CXXFunctionalCastExpr *FCE) {
// Check if this is a test point marker. If so, we are done with this
// expression.
- if (VisitTestPoint(FCE))
+ if (handleTestPoint(FCE))
return;
- // Visit as normal otherwise.
- Base::VisitCXXFunctionalCastExpr(FCE);
+ if (isGslPointerType(FCE->getType()))
+ addAssignOriginFact(*FCE, *FCE->getSubExpr());
}
-private:
- // Check if a type has an origin.
- bool hasOrigin(QualType QT) { return QT->isPointerOrReferenceType(); }
+ void VisitInitListExpr(const InitListExpr *ILE) {
+ if (!hasOrigin(ILE->getType()))
+ return;
+ // For list initialization with a single element, like `View{...}`, the
+ // origin of the list itself is the origin of its single element.
+ if (ILE->getNumInits() == 1)
+ addAssignOriginFact(*ILE, *ILE->getInit(0));
+ }
- 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 VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *MTE) {
+ if (!hasOrigin(MTE->getType()))
+ return;
+ // A temporary object's origin is the same as the origin of the
+ // expression that initializes it.
+ addAssignOriginFact(*MTE, *MTE->getSubExpr());
}
void handleDestructor(const CFGAutomaticObjDtor &DtorOpt) {
@@ -544,9 +565,91 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
}
}
+private:
+ static bool isGslPointerType(QualType QT) {
+ if (const auto *RD = QT->getAsCXXRecordDecl()) {
+ // We need to check the template definition for specializations.
+ if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD))
+ return CTSD->getSpecializedTemplate()
+ ->getTemplatedDecl()
+ ->hasAttr<PointerAttr>();
+ return RD->hasAttr<PointerAttr>();
+ }
+ return false;
+ }
+
+ // Check if a type has an origin.
+ static bool hasOrigin(QualType QT) {
+ if (QT->isFunctionPointerType())
+ return false;
+ return QT->isPointerOrReferenceType() || isGslPointerType(QT);
+ }
+
+ /// Checks if a call-like expression creates a borrow by passing a value to a
+ /// reference parameter, creating an IssueFact if it does.
+ void handleFucntionCall(const Expr *Call, const FunctionDecl *FD,
+ ArrayRef<const Expr *> Args) {
+ if (!FD)
+ return;
+ auto ParamType = [&](int I) -> QualType {
+ if (FD->isCXXClassMember())
+ I = I - 1;
+ assert(I >= 0);
+ return FD->getParamDecl(I)->getType();
+ };
+ auto IsImplicitObjIndex = [&](int I) {
+ return FD->isCXXClassMember() && I == 0;
+ };
+ for (unsigned I = 0; I < Args.size(); ++I) {
+ const Expr *Arg = Args[I];
+
+ // A loan is created when a value type (with no origin) is passed to a
+ // reference parameter. Implicit 'this' object arg always creates a loan.
+ if ((IsImplicitObjIndex(I) || ParamType(I)->isReferenceType()) &&
+ !hasOrigin(Arg->getType())) {
+ if (const Loan *L = createLoanFrom(Arg, Call)) {
+ OriginID CallOID = FactMgr.getOriginMgr().getOrCreate(*Call);
+ CurrentBlockFacts.push_back(
+ FactMgr.createFact<IssueFact>(L->ID, CallOID));
+ // FIXME: Handle loans to more than one arguments (specially for
+ // params with lifetime annotations).
+ return;
+ }
+ }
+ }
+ }
+
+ /// Attempts to create a loan by analyzing the source expression of a borrow.
+ /// \param Path The expression representing the path to a object being
+ /// borrowed.
+ /// \param IssuedTo The expression that triggers the borrow (e.g., an
+ /// address-of unary operator). The loan is issued to the origin of this
+ /// expression.
+ /// \return The new Loan on success, nullptr otherwise.
+ const Loan *createLoanFrom(const Expr *Path, const Expr *IssuedTo) {
+ // For now, we only handle direct borrows from local variables.
+ // In the future, this can be extended to handle MaterializeTemporaryExpr,
+ // etc.
+ if (const auto *DRE = dyn_cast<DeclRefExpr>(Path->IgnoreParenImpCasts())) {
+ if (const auto *VD = dyn_cast<ValueDecl>(DRE->getDecl())) {
+ AccessPath Path(VD);
+ return &FactMgr.getLoanMgr().addLoan(Path, IssuedTo);
+ }
+ }
+ return nullptr;
+ }
+
+ 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) {
+ bool handleTestPoint(const CXXFunctionalCastExpr *FCE) {
if (!FCE->getType()->isVoidType())
return false;
diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp
index 660b9c9d5e243..bc8a5f3f7150f 100644
--- a/clang/test/Sema/warn-lifetime-safety.cpp
+++ b/clang/test/Sema/warn-lifetime-safety.cpp
@@ -6,6 +6,12 @@ struct MyObj {
MyObj operator+(MyObj);
};
+struct [[gsl::Pointer()]] View {
+ View(const MyObj&); // Borrows from MyObj
+ View();
+ void use() const;
+};
+
//===----------------------------------------------------------------------===//
// Basic Definite Use-After-Free (-W...permissive)
// These are cases where the pointer is guaranteed to be dangling at the use site.
@@ -20,12 +26,31 @@ void definite_simple_case() {
(void)*p; // expected-note {{later used here}}
}
+void definite_simple_case_gsl() {
+ View v;
+ {
+ MyObj s;
+ v = s; // expected-warning {{object whose reference is captured does not live long enough}}
+ } // expected-note {{destroyed here}}
+ v.use(); // expected-note {{later used here}}
+}
+
void no_use_no_error() {
MyObj* p;
{
MyObj s;
p = &s;
}
+ // 'p' is dangling here, but since it is never used, no warning is issued.
+}
+
+void no_use_no_error_gsl() {
+ View v;
+ {
+ MyObj s;
+ v = s;
+ }
+ // 'v' is dangling here, but since it is never used, no warning is issued.
}
void definite_pointer_chain() {
@@ -39,6 +64,16 @@ void definite_pointer_chain() {
(void)*q; // expected-note {{later used here}}
}
+void definite_propagation_gsl() {
+ View v1, v2;
+ {
+ MyObj s;
+ v1 = s; // expected-warning {{object whose reference is captured does not live long enough}}
+ v2 = v1;
+ } // expected-note {{destroyed here}}
+ v2.use(); // expected-note {{later used here}}
+}
+
void definite_multiple_uses_one_warning() {
MyObj* p;
{
@@ -78,6 +113,19 @@ void definite_single_pointer_multiple_loans(bool cond) {
(void)*p; // expected-note 2 {{later used here}}
}
+void definite_single_pointer_multiple_loans_gsl(bool cond) {
+ View v;
+ if (cond){
+ MyObj s;
+ v = s; // expected-warning {{object whose reference is captured does not live long enough}}
+ } // expected-note {{destroyed here}}
+ else {
+ MyObj t;
+ v = t; // expected-warning {{object whose reference is captured does not live long enough}}
+ } // expected-note {{destroyed here}}
+ v.use(); // expected-note 2 {{later used here}}
+}
+
//===----------------------------------------------------------------------===//
// Potential (Maybe) Use-After-Free (-W...strict)
@@ -94,18 +142,14 @@ void potential_if_branch(bool cond) {
(void)*p; // expected-note {{later used here}}
}
-// If all paths lead to a dangle, it becomes a definite error.
-void potential_becomes_definite(bool cond) {
- MyObj* p;
+void potential_if_branch_gsl(bool cond) {
+ MyObj safe;
+ View v = safe;
if (cond) {
- MyObj temp1;
- p = &temp1; // expected-warning {{does not live long enough}}
- } // expected-note {{destroyed here}}
- else {
- MyObj temp2;
- p = &temp2; // expected-warning {{does not live long enough}}
+ MyObj temp;
+ v = temp; // expected-warning {{object whose reference is captured may not live long enough}}
} // expected-note {{destroyed here}}
- (void)*p; // expected-note 2 {{later used here}}
+ v.use(); // expected-note {{later used here}}
}
void definite_potential_together(bool cond) {
@@ -159,6 +203,16 @@ void potential_for_loop_use_after_loop_body(MyObj safe) {
(void)*p; // expected-note {{later used here}}
}
+void potential_for_loop_gsl() {
+ MyObj safe;
+ View v = safe;
+ for (int i = 0; i < 1; ++i) {
+ MyObj s;
+ v = s; // expected-warning {{object whose reference is captured may not live long enough}}
+ } // expected-note {{destroyed here}}
+ v.use(); // expected-note {{later used here}}
+}
+
void potential_for_loop_use_before_loop_body(MyObj safe) {
MyObj* p = &safe;
for (int i = 0; i < 1; ++i) {
@@ -182,6 +236,19 @@ void potential_loop_with_break(bool cond) {
(void)*p; // expected-note {{later used here}}
}
+void potential_loop_with_break_gsl(bool cond) {
+ MyObj safe;
+ View v = safe;
+ for (int i = 0; i < 10; ++i) {
+ if (cond) {
+ MyObj temp;
+ v = temp; // expected-warning {{object whose reference is captured may not live long enough}}
+ break; // expected-note {{destroyed here}}
+ }
+ }
+ v.use(); // expected-note {{later used here}}
+}
+
void potential_multiple_expiry_of_same_loan(bool cond) {
// Choose the last expiry location for the loan.
MyObj safe;
@@ -258,6 +325,28 @@ void definite_switch(int mode) {
(void)*p; // expected-note 3 {{later used here}}
}
+void definite_switch_gsl(int mode) {
+ View v;
+ switch (mode) {
+ case 1: {
+ MyObj temp1;
+ v = temp1; // expected-warning {{object whose reference is captured does not live long enough}}
+ break; // expected-note {{destroyed here}}
+ }
+ case 2: {
+ MyObj temp2;
+ v = temp2; // expected-warning {{object whose reference is captured does not live long enough}}
+ break; // expected-note {{destroyed here}}
+ }
+ default: {
+ MyObj temp3;
+ v = temp3; // expected-warning {{object whose reference is captured does not live long enough}}
+ break; // expected-note {{destroyed here}}
+ }
+ }
+ v.use(); // expected-note 3 {{later used here}}
+}
+
//===----------------------------------------------------------------------===//
// No-Error Cases
//===----------------------------------------------------------------------===//
@@ -271,3 +360,14 @@ void no_error_if_dangle_then_rescue() {
p = &safe; // p is "rescued" before use.
(void)*p; // This is safe.
}
+
+void no_error_if_dangle_then_rescue_gsl() {
+ MyObj safe;
+ View v;
+ {
+ MyObj temp;
+ v = temp; // 'v' is temporarily dangling.
+ }
+ v = safe; // 'v' is "rescued" before use by reassigning to a valid object.
+ v.use(); // This is safe.
+}
diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp b/clang/unittests/Analysis/LifetimeSafetyTest.cpp
index 13e5832d70050..bff5378c0a8a9 100644
--- a/clang/unittests/Analysis/LifetimeSafetyTest.cpp
+++ b/clang/unittests/Analysis/LifetimeSafetyTest.cpp
@@ -11,7 +11,6 @@
#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>
@@ -31,7 +30,13 @@ class LifetimeTestRunner {
LifetimeTestRunner(llvm::StringRef Code) {
std::string FullCode = R"(
#define POINT(name) void("__lifetime_test_point_" #name)
+
struct MyObj { ~MyObj() {} int i; };
+
+ struct [[gsl::Pointer()]] View {
+ View(const MyObj&);
+ View();
+ };
)";
FullCode += Code.str();
@@ -741,5 +746,180 @@ TEST_F(LifetimeAnalysisTest, NoDuplicateLoansForImplicitCastToConst) {
EXPECT_THAT(Helper->getLoansForVar("a"), SizeIs(2));
}
+TEST_F(LifetimeAnalysisTest, GslPointerSimpleLoan) {
+ SetupTest(R"(
+ void target() {
+ MyObj a;
+ View x = a;
+ POINT(p1);
+ }
+ )");
+ EXPECT_THAT(Origin("x"), HasLoansTo({"a"}, "p1"));
+}
+
+TEST_F(LifetimeAnalysisTest, GslPointerConstructFromOwner) {
+ SetupTest(R"(
+ void target() {
+ MyObj al, bl, cl, dl, el, fl;
+ View a = View(al);
+ View b = View{bl};
+ View c = View(View(View(cl)));
+ View d = View{View(View(dl))};
+ View e = View{View{View{el}}};
+ View f = {fl};
+ POINT(p1);
+ }
+ )");
+ EXPECT_THAT(Origin("a"), HasLoansTo({"al"}, "p1"));
+ EXPECT_THAT(Origin("b"), HasLoansTo({"bl"}, "p1"));
+ EXPECT_THAT(Origin("c"), HasLoansTo({"cl"}, "p1"));
+ EXPECT_THAT(Origin("d"), HasLoansTo({"dl"}, "p1"));
+ EXPECT_THAT(Origin("e"), HasLoansTo({"el"}, "p1"));
+ EXPECT_THAT(Origin("f"), HasLoansTo({"fl"}, "p1"));
+}
+
+TEST_F(LifetimeAnalysisTest, GslPointerConstructFromView) {
+ SetupTest(R"(
+ void target() {
+ MyObj a;
+ View x = View(a);
+ View y = View{x};
+ View z = View(View(View(y)));
+ View p = View{View(View(x))};
+ View q = {x};
+ POINT(p1);
+ }
+ )");
+ EXPECT_THAT(Origin("x"), HasLoansTo({"a"}, "p1"));
+ EXPECT_THAT(Origin("y"), HasLoansTo({"a"}, "p1"));
+ EXPECT_THAT(Origin("z"), HasLoansTo({"a"}, "p1"));
+ EXPECT_THAT(Origin("p"), HasLoansTo({"a"}, "p1"));
+ EXPECT_THAT(Origin("q"), HasLoansTo({"a"}, "p1"));
+}
+
+// FIXME: Handle loans in ternary operator!
+TEST_F(LifetimeAnalysisTest, GslPointerInConditionalOperator) {
+ SetupTest(R"(
+ void target(bool cond) {
+ MyObj a, b;
+ View v = cond ? a : b;
+ POINT(p1);
+ }
+ )");
+ EXPECT_THAT(Origin("v"), HasLoansTo({}, "p1"));
+}
+
+// FIXME: Handle temporaries.
+TEST_F(LifetimeAnalysisTest, ViewFromTemporary) {
+ SetupTest(R"(
+ MyObj temporary();
+ void target() {
+ View v = temporary();
+ POINT(p1);
+ }
+ )");
+ EXPECT_THAT(Origin("v"), HasLoansTo({}, "p1"));
+}
+
+TEST_F(LifetimeAnalysisTest, GslPointerWithConstAndAuto) {
+ SetupTest(R"(
+ void target() {
+ MyObj a;
+ const View v1 = a;
+ auto v2 = v1;
+ const auto& v3 = v2;
+ POINT(p1);
+ }
+ )");
+ EXPECT_THAT(Origin("v1"), HasLoansTo({"a"}, "p1"));
+ EXPECT_THAT(Origin("v2"), HasLoansTo({"a"}, "p1"));
+ EXPECT_THAT(Origin("v3"), HasLoansTo({"a"}, "p1"));
+}
+
+TEST_F(LifetimeAnalysisTest, GslPointerPropagation) {
+ SetupTest(R"(
+ void target() {
+ MyObj a;
+ View x = a;
+ POINT(p1);
+
+ View y = x; // Propagation via copy-construction
+ POINT(p2);
+
+ View z;
+ z = x; // Propagation via copy-assignment
+ POINT(p3);
+ }
+ )");
+
+ EXPECT_THAT(Origin("x"), HasLoansTo({"a"}, "p1"));
+ EXPECT_THAT(Origin("y"), HasLoansTo({"a"}, "p2"));
+ EXPECT_THAT(Origin("z"), HasLoansTo({"a"}, "p3"));
+}
+
+TEST_F(LifetimeAnalysisTest, GslPointerLoanExpiration) {
+ SetupTest(R"(
+ void target() {
+ View x;
+ {
+ MyObj a;
+ x = a;
+ POINT(before_expiry);
+ } // `a` is destroyed here.
+ POINT(after_expiry);
+ }
+ )");
+
+ EXPECT_THAT(NoLoans(), AreExpiredAt("before_expiry"));
+ EXPECT_THAT(LoansTo({"a"}), AreExpiredAt("after_expiry"));
+}
+
+TEST_F(LifetimeAnalysisTest, GslPointerReassignment) {
+ SetupTest(R"(
+ void target() {
+ MyObj safe;
+ View v;
+ v = safe;
+ POINT(p1);
+ {
+ MyObj unsafe;
+ v = unsafe;
+ POINT(p2);
+ } // `unsafe` expires here.
+ POINT(p3);
+ }
+ )");
+
+ EXPECT_THAT(Origin("v"), HasLoansTo({"safe"}, "p1"));
+ EXPECT_THAT(Origin("v"), HasLoansTo({"unsafe"}, "p2"));
+ EXPECT_THAT(Origin("v"), HasLoansTo({"unsafe"}, "p3"));
+ EXPECT_THAT(LoansTo({"unsafe"}), AreExpiredAt("p3"));
+}
+
+TEST_F(LifetimeAnalysisTest, GslPointerConversionOperator) {
+ SetupTest(R"(
+ struct String;
+
+ struct [[gsl::Pointer()]] StringView {
+ StringView() = default;
+ };
+
+ struct String {
+ ~String() {}
+ operator StringView() const;
+ };
+
+ void target() {
+ String xl, yl;
+ StringView x = xl;
+ StringView y;
+ y = yl;
+ POINT(p1);
+ }
+ )");
+ EXPECT_THAT(Origin("x"), HasLoansTo({"xl"}, "p1"));
+ EXPECT_THAT(Origin("y"), HasLoansTo({"yl"}, "p1"));
+}
+
} // anonymous namespace
} // namespace clang::lifetimes::internal
More information about the llvm-branch-commits
mailing list