diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h index 1a16fb82f9a84..8e26a4d41a957 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h @@ -38,6 +38,20 @@ bool isAssignmentOperatorLifetimeBound(const CXXMethodDecl *CMD); /// method or because it's a normal assignment operator. bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD); +// Returns true if the implicit object argument (this) of a method call should +// be tracked for GSL lifetime analysis. This applies to STL methods that return +// pointers or references that depend on the lifetime of the object, such as +// container iterators (begin, end), data accessors (c_str, data, get), or +// element accessors (operator[], operator*, front, back, at). +bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee); + +// Returns true if the first argument of a free function should be tracked for +// GSL lifetime analysis. This applies to STL free functions that take a pointer +// to a GSL Owner or Pointer and return a pointer or reference that depends on +// the lifetime of the argument, such as std::begin, std::data, std::get, or +// std::any_cast. +bool shouldTrackFirstArgument(const FunctionDecl *FD); + // Tells whether the type is annotated with [[gsl::Pointer]]. bool isGslPointerType(QualType QT); // Tells whether the type is annotated with [[gsl::Owner]]. diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 460be3ad99347..4f05d7ea94aa7 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -14,6 +14,7 @@ #include "clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h" #include "clang/Analysis/Analyses/PostOrderCFGView.h" #include "llvm/Support/Casting.h" +#include "llvm/Support/Debug.h" #include "llvm/Support/Signals.h" #include "llvm/Support/TimeProfiler.h" @@ -387,11 +388,14 @@ void FactsGenerator::handleFunctionCall(const Expr *Call, Method && Method->isInstance()) { if (I == 0) // For the 'this' argument, the attribute is on the method itself. - return implicitObjectParamIsLifetimeBound(Method); + return implicitObjectParamIsLifetimeBound(Method) || + shouldTrackImplicitObjectArg(Method); if ((I - 1) < Method->getNumParams()) // For explicit arguments, find the corresponding parameter // declaration. PVD = Method->getParamDecl(I - 1); + } else if (I == 0 && shouldTrackFirstArgument(FD)) { + return true; } else if (I < FD->getNumParams()) { // For free functions or static methods. PVD = FD->getParamDecl(I); diff --git a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp index 54e343fc2ee5e..860aa5373a32c 100644 --- a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp @@ -71,6 +71,88 @@ bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) { return isNormalAssignmentOperator(FD); } +// Decl::isInStdNamespace will return false for iterators in some STL +// implementations due to them being defined in a namespace outside of the std +// namespace. +static bool isInStlNamespace(const Decl *D) { + const DeclContext *DC = D->getDeclContext(); + if (!DC) + return false; + if (const auto *ND = dyn_cast(DC)) + if (const IdentifierInfo *II = ND->getIdentifier()) { + StringRef Name = II->getName(); + if (Name.size() >= 2 && Name.front() == '_' && + (Name[1] == '_' || isUppercase(Name[1]))) + return true; + } + + return DC->isStdNamespace(); +} + +static bool isPointerLikeType(QualType QT) { + return isGslPointerType(QT) || QT->isPointerType() || QT->isNullPtrType(); +} + +bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) { + if (auto *Conv = dyn_cast_or_null(Callee)) + if (isGslPointerType(Conv->getConversionType()) && + Callee->getParent()->hasAttr()) + return true; + if (!isInStlNamespace(Callee->getParent())) + return false; + if (!isGslPointerType(Callee->getFunctionObjectParameterType()) && + !isGslOwnerType(Callee->getFunctionObjectParameterType())) + return false; + if (isPointerLikeType(Callee->getReturnType())) { + if (!Callee->getIdentifier()) + return false; + return llvm::StringSwitch(Callee->getName()) + .Cases({"begin", "rbegin", "cbegin", "crbegin"}, true) + .Cases({"end", "rend", "cend", "crend"}, true) + .Cases({"c_str", "data", "get"}, true) + // Map and set types. + .Cases({"find", "equal_range", "lower_bound", "upper_bound"}, true) + .Default(false); + } + if (Callee->getReturnType()->isReferenceType()) { + if (!Callee->getIdentifier()) { + auto OO = Callee->getOverloadedOperator(); + if (!Callee->getParent()->hasAttr()) + return false; + return OO == OverloadedOperatorKind::OO_Subscript || + OO == OverloadedOperatorKind::OO_Star; + } + return llvm::StringSwitch(Callee->getName()) + .Cases({"front", "back", "at", "top", "value"}, true) + .Default(false); + } + return false; +} + +bool shouldTrackFirstArgument(const FunctionDecl *FD) { + if (!FD->getIdentifier() || FD->getNumParams() != 1) + return false; + const auto *RD = FD->getParamDecl(0)->getType()->getPointeeCXXRecordDecl(); + if (!FD->isInStdNamespace() || !RD || !RD->isInStdNamespace()) + return false; + if (!RD->hasAttr() && !RD->hasAttr()) + return false; + if (FD->getReturnType()->isPointerType() || + isGslPointerType(FD->getReturnType())) { + return llvm::StringSwitch(FD->getName()) + .Cases({"begin", "rbegin", "cbegin", "crbegin"}, true) + .Cases({"end", "rend", "cend", "crend"}, true) + .Case("data", true) + .Default(false); + } + if (FD->getReturnType()->isReferenceType()) { + return llvm::StringSwitch(FD->getName()) + .Cases({"get", "any_cast"}, true) + .Default(false); + } + return false; +} + template static bool isRecordWithAttr(QualType Type) { auto *RD = Type->getAsCXXRecordDecl(); if (!RD) diff --git a/clang/lib/Analysis/LifetimeSafety/Origins.cpp b/clang/lib/Analysis/LifetimeSafety/Origins.cpp index ac8d8041f600b..0a7d1dc38567a 100644 --- a/clang/lib/Analysis/LifetimeSafety/Origins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Origins.cpp @@ -11,6 +11,7 @@ #include "clang/AST/Attr.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclTemplate.h" +#include "clang/AST/ExprCXX.h" #include "clang/AST/TypeBase.h" #include "clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h" @@ -79,6 +80,9 @@ OriginList *OriginManager::getOrCreateList(const ValueDecl *D) { OriginList *OriginManager::getOrCreateList(const Expr *E, size_t Depth) { if (auto *ParenIgnored = E->IgnoreParens(); ParenIgnored != E) return getOrCreateList(ParenIgnored); + // We do not see CFG stmts for ExprWithCleanups. Simply peel them. + if (auto *EC = dyn_cast(E)) + return getOrCreateList(EC->getSubExpr(), Depth); if (!hasOrigins(E)) return nullptr; diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index c91ca751984c9..26e4d75b1fa49 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -320,66 +320,6 @@ static bool isStdInitializerListOfPointer(const RecordDecl *RD) { return false; } -static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) { - if (auto *Conv = dyn_cast_or_null(Callee)) - if (isGslPointerType(Conv->getConversionType()) && - Callee->getParent()->hasAttr()) - return true; - if (!isInStlNamespace(Callee->getParent())) - return false; - if (!isGslPointerType(Callee->getFunctionObjectParameterType()) && - !isGslOwnerType(Callee->getFunctionObjectParameterType())) - return false; - if (isPointerLikeType(Callee->getReturnType())) { - if (!Callee->getIdentifier()) - return false; - return llvm::StringSwitch(Callee->getName()) - .Cases({"begin", "rbegin", "cbegin", "crbegin"}, true) - .Cases({"end", "rend", "cend", "crend"}, true) - .Cases({"c_str", "data", "get"}, true) - // Map and set types. - .Cases({"find", "equal_range", "lower_bound", "upper_bound"}, true) - .Default(false); - } - if (Callee->getReturnType()->isReferenceType()) { - if (!Callee->getIdentifier()) { - auto OO = Callee->getOverloadedOperator(); - if (!Callee->getParent()->hasAttr()) - return false; - return OO == OverloadedOperatorKind::OO_Subscript || - OO == OverloadedOperatorKind::OO_Star; - } - return llvm::StringSwitch(Callee->getName()) - .Cases({"front", "back", "at", "top", "value"}, true) - .Default(false); - } - return false; -} - -static bool shouldTrackFirstArgument(const FunctionDecl *FD) { - if (!FD->getIdentifier() || FD->getNumParams() != 1) - return false; - const auto *RD = FD->getParamDecl(0)->getType()->getPointeeCXXRecordDecl(); - if (!FD->isInStdNamespace() || !RD || !RD->isInStdNamespace()) - return false; - if (!RD->hasAttr() && !RD->hasAttr()) - return false; - if (FD->getReturnType()->isPointerType() || - isGslPointerType(FD->getReturnType())) { - return llvm::StringSwitch(FD->getName()) - .Cases({"begin", "rbegin", "cbegin", "crbegin"}, true) - .Cases({"end", "rend", "cend", "crend"}, true) - .Case("data", true) - .Default(false); - } - if (FD->getReturnType()->isReferenceType()) { - return llvm::StringSwitch(FD->getName()) - .Cases({"get", "any_cast"}, true) - .Default(false); - } - return false; -} - // Returns true if the given constructor is a copy-like constructor, such as // `Ctor(Owner&&)` or `Ctor(const Owner&)`. static bool isCopyLikeConstructor(const CXXConstructorDecl *Ctor) { @@ -564,7 +504,7 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, VisitLifetimeBoundArg(Callee, ObjectArg); else if (EnableGSLAnalysis) { if (auto *CME = dyn_cast(Callee); - CME && shouldTrackImplicitObjectArg(CME)) + CME && lifetimes::shouldTrackImplicitObjectArg(CME)) VisitGSLPointerArg(Callee, ObjectArg); } } @@ -605,7 +545,7 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, VisitLifetimeBoundArg(CanonCallee->getParamDecl(I), Arg); else if (EnableGSLAnalysis && I == 0) { // Perform GSL analysis for the first argument - if (shouldTrackFirstArgument(CanonCallee)) { + if (lifetimes::shouldTrackFirstArgument(CanonCallee)) { VisitGSLPointerArg(CanonCallee, Arg); } else if (auto *Ctor = dyn_cast(Call); Ctor && shouldTrackFirstArgumentForConstructor(Ctor)) { diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp b/clang/unittests/Analysis/LifetimeSafetyTest.cpp index ddc9cb602fc26..5c76e866cdb65 100644 --- a/clang/unittests/Analysis/LifetimeSafetyTest.cpp +++ b/clang/unittests/Analysis/LifetimeSafetyTest.cpp @@ -1597,5 +1597,185 @@ TEST_F(LifetimeAnalysisTest, TrivialClassDestructorsUAR) { EXPECT_THAT("s", HasLiveLoanAtExpiry("p1")); } +// ========================================================================= // +// Tests for shouldTrackImplicitObjectArg +// ========================================================================= // + +TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_STLBegin) { + SetupTest(R"( + namespace std { + template + struct vector { + struct iterator {}; + iterator begin(); + }; + } + + void target() { + std::vector vec; + auto it = vec.begin(); + POINT(p1); + } + )"); + EXPECT_THAT(Origin("it"), HasLoansTo({"vec"}, "p1")); +} + +TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_OwnerDeref) { + SetupTest(R"( + namespace std { + template + struct optional { + T& operator*(); + }; + } + + void target() { + std::optional opt; + int& r = *opt; + POINT(p1); + } + )"); + EXPECT_THAT(Origin("r"), HasLoansTo({"opt"}, "p1")); +} + +TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_Value) { + SetupTest(R"( + namespace std { + template + struct optional { + T& value(); + }; + } + + void target() { + std::optional opt; + int& r = opt.value(); + POINT(p1); + } + )"); + EXPECT_THAT(Origin("r"), HasLoansTo({"opt"}, "p1")); +} + +TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_UniquePtr_Get) { + SetupTest(R"( + namespace std { + template + struct unique_ptr { + T *get() const; + }; + } + + void target() { + std::unique_ptr up; + int* r = up.get(); + POINT(p1); + } + )"); + EXPECT_THAT(Origin("r"), HasLoansTo({"up"}, "p1")); +} + +TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_ConversionOperator) { + SetupTest(R"( + struct [[gsl::Pointer(int)]] IntPtr { + int& operator*(); + }; + + struct [[gsl::Owner(int)]] OwnerWithConversion { + operator IntPtr(); + }; + + void target() { + OwnerWithConversion owner; + IntPtr ptr = owner; + POINT(p1); + } + )"); + EXPECT_THAT(Origin("ptr"), HasLoansTo({"owner"}, "p1")); +} + +TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_MapFind) { + SetupTest(R"( + namespace std { + template + struct map { + struct iterator {}; + iterator find(const K&); + }; + } + + void target() { + std::map m; + auto it = m.find(42); + POINT(p1); + } + )"); + EXPECT_THAT(Origin("it"), HasLoansTo({"m"}, "p1")); +} + +// ========================================================================= // +// Tests for shouldTrackFirstArgument +// ========================================================================= // + +TEST_F(LifetimeAnalysisTest, TrackFirstArgument_StdBegin) { + SetupTest(R"( + namespace std { + template + struct vector { + struct iterator {}; + iterator begin(); + }; + + template + auto begin(C& c) -> decltype(c.begin()); + } + + void target() { + std::vector vec; + auto it = std::begin(vec); + POINT(p1); + } + )"); + EXPECT_THAT(Origin("it"), HasLoansTo({"vec"}, "p1")); +} + +TEST_F(LifetimeAnalysisTest, TrackFirstArgument_StdData) { + SetupTest(R"( + namespace std { + template + struct vector { + const T* data() const; + }; + + template + auto data(C& c) -> decltype(c.data()); + } + + void target() { + std::vector vec; + const int* p = std::data(vec); + POINT(p1); + } + )"); + EXPECT_THAT(Origin("p"), HasLoansTo({"vec"}, "p1")); +} + +TEST_F(LifetimeAnalysisTest, TrackFirstArgument_StdAnyCast) { + SetupTest(R"( + namespace std { + struct any {}; + + template + T any_cast(const any& op); + } + + void target() { + std::any a; + int& r = std::any_cast(a); + POINT(p1); + } + )"); + EXPECT_THAT(Origin("r"), HasLoansTo({"a"}, "p1")); +} + } // anonymous namespace } // namespace clang::lifetimes::internal