https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/154009
>From ab16e9aa72482026ed1ca48cd9e245fd4eaade9a Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <[email protected]> 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 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
