Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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]].
Expand Down
6 changes: 5 additions & 1 deletion clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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);
Expand Down
82 changes: 82 additions & 0 deletions clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: we seem to have this exact function at two locations now. Should we deduplicate this as well?

const DeclContext *DC = D->getDeclContext();
if (!DC)
return false;
if (const auto *ND = dyn_cast<NamespaceDecl>(DC))
if (const IdentifierInfo *II = ND->getIdentifier()) {
StringRef Name = II->getName();
if (Name.size() >= 2 && Name.front() == '_' &&
(Name[1] == '_' || isUppercase(Name[1])))
return true;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably an off-topic comment: the code looks suspicious to me, if a namespace name with the prefix _[_A-Z], we consider it is a STL namespace. The code was introduced in https://reviews.llvm.org/D66152, @Xazax-hun do you still remember the rationale?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think (at least in some version of) libstdc++ there were some types that were not part of the top level std namespace but they were in a separate __gnu_cxx namespace.

}

return DC->isStdNamespace();
}

static bool isPointerLikeType(QualType QT) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for this function, we have two instances.

return isGslPointerType(QT) || QT->isPointerType() || QT->isNullPtrType();
}

bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) {
if (auto *Conv = dyn_cast_or_null<CXXConversionDecl>(Callee))
if (isGslPointerType(Conv->getConversionType()) &&
Callee->getParent()->hasAttr<OwnerAttr>())
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<bool>(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<OwnerAttr>())
return false;
return OO == OverloadedOperatorKind::OO_Subscript ||
OO == OverloadedOperatorKind::OO_Star;
}
return llvm::StringSwitch<bool>(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<PointerAttr>() && !RD->hasAttr<OwnerAttr>())
return false;
if (FD->getReturnType()->isPointerType() ||
isGslPointerType(FD->getReturnType())) {
return llvm::StringSwitch<bool>(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<bool>(FD->getName())
.Cases({"get", "any_cast"}, true)
.Default(false);
}
return false;
}

template <typename T> static bool isRecordWithAttr(QualType Type) {
auto *RD = Type->getAsCXXRecordDecl();
if (!RD)
Expand Down
4 changes: 4 additions & 0 deletions clang/lib/Analysis/LifetimeSafety/Origins.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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<ExprWithCleanups>(E))
return getOrCreateList(EC->getSubExpr(), Depth);

if (!hasOrigins(E))
return nullptr;
Expand Down
64 changes: 2 additions & 62 deletions clang/lib/Sema/CheckExprLifetime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<CXXConversionDecl>(Callee))
if (isGslPointerType(Conv->getConversionType()) &&
Callee->getParent()->hasAttr<OwnerAttr>())
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<bool>(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<OwnerAttr>())
return false;
return OO == OverloadedOperatorKind::OO_Subscript ||
OO == OverloadedOperatorKind::OO_Star;
}
return llvm::StringSwitch<bool>(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<PointerAttr>() && !RD->hasAttr<OwnerAttr>())
return false;
if (FD->getReturnType()->isPointerType() ||
isGslPointerType(FD->getReturnType())) {
return llvm::StringSwitch<bool>(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<bool>(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<U>&&)` or `Ctor(const Owner<U>&)`.
static bool isCopyLikeConstructor(const CXXConstructorDecl *Ctor) {
Expand Down Expand Up @@ -564,7 +504,7 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
VisitLifetimeBoundArg(Callee, ObjectArg);
else if (EnableGSLAnalysis) {
if (auto *CME = dyn_cast<CXXMethodDecl>(Callee);
CME && shouldTrackImplicitObjectArg(CME))
CME && lifetimes::shouldTrackImplicitObjectArg(CME))
VisitGSLPointerArg(Callee, ObjectArg);
}
}
Expand Down Expand Up @@ -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<CXXConstructExpr>(Call);
Ctor && shouldTrackFirstArgumentForConstructor(Ctor)) {
Expand Down
Loading