Teach some warnings to respect gsl::Pointer and gsl::Owner attributes
This patch extends some existing warnings to utilize the knowledge about the gsl::Pointer and gsl::Owner attributes. Differential Revision: https://reviews.llvm.org/D64256 llvm-svn: 368072
This commit is contained in:
parent
800618f241
commit
e5e10b526f
|
@ -8107,6 +8107,10 @@ def warn_dangling_member : Warning<
|
|||
"%select{binds to|is}2 a temporary object "
|
||||
"whose lifetime is shorter than the lifetime of the constructed object">,
|
||||
InGroup<DanglingField>;
|
||||
def warn_dangling_lifetime_pointer_member : Warning<
|
||||
"initializing pointer member %0 to point to a temporary object "
|
||||
"whose lifetime is shorter than the lifetime of the constructed object">,
|
||||
InGroup<DanglingField>;
|
||||
def note_lifetime_extending_member_declared_here : Note<
|
||||
"%select{%select{reference|'std::initializer_list'}0 member|"
|
||||
"member with %select{reference|'std::initializer_list'}0 subobject}1 "
|
||||
|
@ -8125,6 +8129,10 @@ def warn_new_dangling_reference : Warning<
|
|||
"temporary bound to reference member of allocated object "
|
||||
"will be destroyed at the end of the full-expression">,
|
||||
InGroup<DanglingField>;
|
||||
def warn_dangling_lifetime_pointer : Warning<
|
||||
"object backing the pointer "
|
||||
"will be destroyed at the end of the full-expression">,
|
||||
InGroup<Dangling>;
|
||||
def warn_new_dangling_initializer_list : Warning<
|
||||
"array backing "
|
||||
"%select{initializer list subobject of the allocated object|"
|
||||
|
|
|
@ -6513,6 +6513,7 @@ struct IndirectLocalPathEntry {
|
|||
VarInit,
|
||||
LValToRVal,
|
||||
LifetimeBoundCall,
|
||||
GslPointerInit
|
||||
} Kind;
|
||||
Expr *E;
|
||||
const Decl *D = nullptr;
|
||||
|
@ -6557,6 +6558,40 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
|
|||
Expr *Init, ReferenceKind RK,
|
||||
LocalVisitor Visit);
|
||||
|
||||
template <typename T> static bool isRecordWithAttr(QualType Type) {
|
||||
if (auto *RD = Type->getAsCXXRecordDecl())
|
||||
return RD->getCanonicalDecl()->hasAttr<T>();
|
||||
return false;
|
||||
}
|
||||
|
||||
static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call,
|
||||
LocalVisitor Visit) {
|
||||
auto VisitPointerArg = [&](const Decl *D, Expr *Arg) {
|
||||
Path.push_back({IndirectLocalPathEntry::GslPointerInit, Arg, D});
|
||||
if (Arg->isGLValue())
|
||||
visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding,
|
||||
Visit);
|
||||
else
|
||||
visitLocalsRetainedByInitializer(Path, Arg, Visit, true);
|
||||
Path.pop_back();
|
||||
};
|
||||
|
||||
if (auto *MCE = dyn_cast<CXXMemberCallExpr>(Call)) {
|
||||
const FunctionDecl *Callee = MCE->getDirectCallee();
|
||||
if (auto *Conv = dyn_cast_or_null<CXXConversionDecl>(Callee))
|
||||
if (isRecordWithAttr<PointerAttr>(Conv->getConversionType()))
|
||||
VisitPointerArg(Callee, MCE->getImplicitObjectArgument());
|
||||
return;
|
||||
}
|
||||
|
||||
if (auto *CCE = dyn_cast<CXXConstructExpr>(Call)) {
|
||||
const auto *Ctor = CCE->getConstructor();
|
||||
const CXXRecordDecl *RD = Ctor->getParent()->getCanonicalDecl();
|
||||
if (CCE->getNumArgs() > 0 && RD->hasAttr<PointerAttr>())
|
||||
VisitPointerArg(Ctor->getParamDecl(0), CCE->getArgs()[0]);
|
||||
}
|
||||
}
|
||||
|
||||
static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) {
|
||||
const TypeSourceInfo *TSI = FD->getTypeSourceInfo();
|
||||
if (!TSI)
|
||||
|
@ -6678,8 +6713,10 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
|
|||
true);
|
||||
}
|
||||
|
||||
if (isa<CallExpr>(Init))
|
||||
if (isa<CallExpr>(Init)) {
|
||||
handleGslAnnotatedTypes(Path, Init, Visit);
|
||||
return visitLifetimeBoundArguments(Path, Init, Visit);
|
||||
}
|
||||
|
||||
switch (Init->getStmtClass()) {
|
||||
case Stmt::DeclRefExprClass: {
|
||||
|
@ -6905,8 +6942,10 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path,
|
|||
}
|
||||
}
|
||||
|
||||
if (isa<CallExpr>(Init) || isa<CXXConstructExpr>(Init))
|
||||
if (isa<CallExpr>(Init) || isa<CXXConstructExpr>(Init)) {
|
||||
handleGslAnnotatedTypes(Path, Init, Visit);
|
||||
return visitLifetimeBoundArguments(Path, Init, Visit);
|
||||
}
|
||||
|
||||
switch (Init->getStmtClass()) {
|
||||
case Stmt::UnaryOperatorClass: {
|
||||
|
@ -6988,6 +7027,7 @@ static SourceRange nextPathEntryRange(const IndirectLocalPath &Path, unsigned I,
|
|||
case IndirectLocalPathEntry::AddressOf:
|
||||
case IndirectLocalPathEntry::LValToRVal:
|
||||
case IndirectLocalPathEntry::LifetimeBoundCall:
|
||||
case IndirectLocalPathEntry::GslPointerInit:
|
||||
// These exist primarily to mark the path as not permitting or
|
||||
// supporting lifetime extension.
|
||||
break;
|
||||
|
@ -7000,6 +7040,11 @@ static SourceRange nextPathEntryRange(const IndirectLocalPath &Path, unsigned I,
|
|||
return E->getSourceRange();
|
||||
}
|
||||
|
||||
static bool pathOnlyInitializesGslPointer(IndirectLocalPath &Path) {
|
||||
return !Path.empty() &&
|
||||
Path.back().Kind == IndirectLocalPathEntry::GslPointerInit;
|
||||
}
|
||||
|
||||
void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
|
||||
Expr *Init) {
|
||||
LifetimeResult LR = getEntityLifetime(&Entity);
|
||||
|
@ -7016,12 +7061,31 @@ void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
|
|||
SourceRange DiagRange = nextPathEntryRange(Path, 0, L);
|
||||
SourceLocation DiagLoc = DiagRange.getBegin();
|
||||
|
||||
auto *MTE = dyn_cast<MaterializeTemporaryExpr>(L);
|
||||
bool IsTempGslOwner = MTE && isRecordWithAttr<OwnerAttr>(MTE->getType());
|
||||
bool IsLocalGslOwner =
|
||||
isa<DeclRefExpr>(L) && isRecordWithAttr<OwnerAttr>(L->getType());
|
||||
|
||||
// Skipping a chain of initializing gsl::Pointer annotated objects.
|
||||
// We are looking only for the final source to find out if it was
|
||||
// a local or temporary owner or the address of a local variable/param. We
|
||||
// do not want to follow the references when returning a pointer originating
|
||||
// from a local owner to avoid the following false positive:
|
||||
// int &p = *localOwner;
|
||||
// someContainer.add(std::move(localOwner));
|
||||
// return p;
|
||||
if (!IsTempGslOwner && pathOnlyInitializesGslPointer(Path) &&
|
||||
!(IsLocalGslOwner && !pathContainsInit(Path)))
|
||||
return true;
|
||||
|
||||
bool IsGslPtrInitWithGslTempOwner =
|
||||
IsTempGslOwner && pathOnlyInitializesGslPointer(Path);
|
||||
|
||||
switch (LK) {
|
||||
case LK_FullExpression:
|
||||
llvm_unreachable("already handled this");
|
||||
|
||||
case LK_Extended: {
|
||||
auto *MTE = dyn_cast<MaterializeTemporaryExpr>(L);
|
||||
if (!MTE) {
|
||||
// The initialized entity has lifetime beyond the full-expression,
|
||||
// and the local entity does too, so don't warn.
|
||||
|
@ -7031,6 +7095,11 @@ void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
|
|||
return false;
|
||||
}
|
||||
|
||||
if (IsGslPtrInitWithGslTempOwner) {
|
||||
Diag(DiagLoc, diag::warn_dangling_lifetime_pointer) << DiagRange;
|
||||
return false;
|
||||
}
|
||||
|
||||
// Lifetime-extend the temporary.
|
||||
if (Path.empty()) {
|
||||
// Update the storage duration of the materialized temporary.
|
||||
|
@ -7072,6 +7141,14 @@ void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
|
|||
// temporary, the program is ill-formed.
|
||||
if (auto *ExtendingDecl =
|
||||
ExtendingEntity ? ExtendingEntity->getDecl() : nullptr) {
|
||||
if (IsGslPtrInitWithGslTempOwner) {
|
||||
Diag(DiagLoc, diag::warn_dangling_lifetime_pointer_member)
|
||||
<< ExtendingDecl << DiagRange;
|
||||
Diag(ExtendingDecl->getLocation(),
|
||||
diag::note_ref_or_ptr_member_declared_here)
|
||||
<< true;
|
||||
return false;
|
||||
}
|
||||
bool IsSubobjectMember = ExtendingEntity != &Entity;
|
||||
Diag(DiagLoc, shouldLifetimeExtendThroughPath(Path)
|
||||
? diag::err_dangling_member
|
||||
|
@ -7112,7 +7189,7 @@ void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
|
|||
|
||||
if (auto *Member =
|
||||
ExtendingEntity ? ExtendingEntity->getDecl() : nullptr) {
|
||||
bool IsPointer = Member->getType()->isAnyPointerType();
|
||||
bool IsPointer = !Member->getType()->isReferenceType();
|
||||
Diag(DiagLoc, IsPointer ? diag::warn_init_ptr_member_to_parameter_addr
|
||||
: diag::warn_bind_ref_member_to_parameter)
|
||||
<< Member << VD << isa<ParmVarDecl>(VD) << DiagRange;
|
||||
|
@ -7126,10 +7203,13 @@ void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
|
|||
|
||||
case LK_New:
|
||||
if (isa<MaterializeTemporaryExpr>(L)) {
|
||||
Diag(DiagLoc, RK == RK_ReferenceBinding
|
||||
? diag::warn_new_dangling_reference
|
||||
: diag::warn_new_dangling_initializer_list)
|
||||
<< !Entity.getParent() << DiagRange;
|
||||
if (IsGslPtrInitWithGslTempOwner)
|
||||
Diag(DiagLoc, diag::warn_dangling_lifetime_pointer) << DiagRange;
|
||||
else
|
||||
Diag(DiagLoc, RK == RK_ReferenceBinding
|
||||
? diag::warn_new_dangling_reference
|
||||
: diag::warn_new_dangling_initializer_list)
|
||||
<< !Entity.getParent() << DiagRange;
|
||||
} else {
|
||||
// We can't determine if the allocation outlives the local declaration.
|
||||
return false;
|
||||
|
@ -7172,7 +7252,8 @@ void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
|
|||
break;
|
||||
|
||||
case IndirectLocalPathEntry::LifetimeBoundCall:
|
||||
// FIXME: Consider adding a note for this.
|
||||
case IndirectLocalPathEntry::GslPointerInit:
|
||||
// FIXME: Consider adding a note for these.
|
||||
break;
|
||||
|
||||
case IndirectLocalPathEntry::DefaultInit: {
|
||||
|
|
|
@ -0,0 +1,135 @@
|
|||
// RUN: %clang_cc1 -fsyntax-only -Wdangling -Wdangling-field -Wreturn-stack-address -verify %s
|
||||
struct [[gsl::Owner(int)]] MyIntOwner {
|
||||
MyIntOwner();
|
||||
int &operator*();
|
||||
int *c_str() const;
|
||||
};
|
||||
|
||||
struct [[gsl::Pointer(int)]] MyIntPointer {
|
||||
MyIntPointer(int *p = nullptr);
|
||||
// Conversion operator and constructor conversion will result in two
|
||||
// different ASTs. The former is tested with another owner and
|
||||
// pointer type.
|
||||
MyIntPointer(const MyIntOwner &);
|
||||
int &operator*();
|
||||
MyIntOwner toOwner();
|
||||
};
|
||||
|
||||
struct [[gsl::Pointer(long)]] MyLongPointerFromConversion {
|
||||
MyLongPointerFromConversion(long *p = nullptr);
|
||||
long &operator*();
|
||||
};
|
||||
|
||||
struct [[gsl::Owner(long)]] MyLongOwnerWithConversion {
|
||||
MyLongOwnerWithConversion();
|
||||
operator MyLongPointerFromConversion();
|
||||
long &operator*();
|
||||
MyIntPointer releaseAsMyPointer();
|
||||
long *releaseAsRawPointer();
|
||||
};
|
||||
|
||||
void danglingHeapObject() {
|
||||
new MyLongPointerFromConversion(MyLongOwnerWithConversion{}); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
|
||||
new MyIntPointer(MyIntOwner{}); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
|
||||
}
|
||||
|
||||
void intentionalFalseNegative() {
|
||||
int i;
|
||||
MyIntPointer p{&i};
|
||||
// In this case we do not have enough information in a statement local
|
||||
// analysis to detect the problem.
|
||||
new MyIntPointer(p);
|
||||
new MyIntPointer(MyIntPointer{p});
|
||||
}
|
||||
|
||||
MyIntPointer ownershipTransferToMyPointer() {
|
||||
MyLongOwnerWithConversion t;
|
||||
return t.releaseAsMyPointer(); // ok
|
||||
}
|
||||
|
||||
long *ownershipTransferToRawPointer() {
|
||||
MyLongOwnerWithConversion t;
|
||||
return t.releaseAsRawPointer(); // ok
|
||||
}
|
||||
|
||||
int *danglingRawPtrFromLocal() {
|
||||
MyIntOwner t;
|
||||
return t.c_str(); // TODO
|
||||
}
|
||||
|
||||
int *danglingRawPtrFromTemp() {
|
||||
MyIntPointer p;
|
||||
return p.toOwner().c_str(); // TODO
|
||||
}
|
||||
|
||||
struct Y {
|
||||
int a[4];
|
||||
};
|
||||
|
||||
void dangligGslPtrFromTemporary() {
|
||||
MyIntPointer p = Y{}.a; // expected-warning {{temporary whose address is used as value of local variable 'p' will be destroyed at the end of the full-expression}}
|
||||
(void)p;
|
||||
}
|
||||
|
||||
struct DanglingGslPtrField {
|
||||
MyIntPointer p; // expected-note 2{{pointer member declared here}}
|
||||
MyLongPointerFromConversion p2; // expected-note {{pointer member declared here}}
|
||||
DanglingGslPtrField(int i) : p(&i) {} // expected-warning {{initializing pointer member 'p' with the stack address of parameter 'i'}}
|
||||
DanglingGslPtrField() : p2(MyLongOwnerWithConversion{}) {} // expected-warning {{initializing pointer member 'p2' to point to a temporary object whose lifetime is shorter than the lifetime of the constructed object}}
|
||||
DanglingGslPtrField(double) : p(MyIntOwner{}) {} // expected-warning {{initializing pointer member 'p' to point to a temporary object whose lifetime is shorter than the lifetime of the constructed object}}
|
||||
};
|
||||
|
||||
MyIntPointer danglingGslPtrFromLocal() {
|
||||
int j;
|
||||
return &j; // expected-warning {{address of stack memory associated with local variable 'j' returned}}
|
||||
}
|
||||
|
||||
MyIntPointer returningLocalPointer() {
|
||||
MyIntPointer localPointer;
|
||||
return localPointer; // ok
|
||||
}
|
||||
|
||||
MyIntPointer daglingGslPtrFromLocalOwner() {
|
||||
MyIntOwner localOwner;
|
||||
return localOwner; // expected-warning {{address of stack memory associated with local variable 'localOwner' returned}}
|
||||
}
|
||||
|
||||
MyLongPointerFromConversion daglingGslPtrFromLocalOwnerConv() {
|
||||
MyLongOwnerWithConversion localOwner;
|
||||
return localOwner; // expected-warning {{address of stack memory associated with local variable 'localOwner' returned}}
|
||||
}
|
||||
|
||||
MyIntPointer danglingGslPtrFromTemporary() {
|
||||
return MyIntOwner{}; // expected-warning {{returning address of local temporary object}}
|
||||
}
|
||||
|
||||
MyLongPointerFromConversion danglingGslPtrFromTemporaryConv() {
|
||||
return MyLongOwnerWithConversion{}; // expected-warning {{returning address of local temporary object}}
|
||||
}
|
||||
|
||||
int *noFalsePositive(MyIntOwner &o) {
|
||||
MyIntPointer p = o;
|
||||
return &*p; // ok
|
||||
}
|
||||
|
||||
MyIntPointer global;
|
||||
MyLongPointerFromConversion global2;
|
||||
|
||||
void initLocalGslPtrWithTempOwner() {
|
||||
MyIntPointer p = MyIntOwner{}; // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
|
||||
p = MyIntOwner{}; // TODO ?
|
||||
global = MyIntOwner{}; // TODO ?
|
||||
MyLongPointerFromConversion p2 = MyLongOwnerWithConversion{}; // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
|
||||
p2 = MyLongOwnerWithConversion{}; // TODO ?
|
||||
global2 = MyLongOwnerWithConversion{}; // TODO ?
|
||||
}
|
||||
|
||||
struct IntVector {
|
||||
int *begin();
|
||||
int *end();
|
||||
};
|
||||
|
||||
void modelIterators() {
|
||||
int *it = IntVector{}.begin(); // TODO ?
|
||||
(void)it;
|
||||
}
|
Loading…
Reference in New Issue