r337790 - Warn if a local variable's initializer retains a pointer/reference to a
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 23 17:55:08 PDT 2018
Author: rsmith
Date: Mon Jul 23 17:55:08 2018
New Revision: 337790
URL: http://llvm.org/viewvc/llvm-project?rev=337790&view=rev
Log:
Warn if a local variable's initializer retains a pointer/reference to a
non-lifetime-extended temporary object.
Added:
cfe/trunk/test/SemaCXX/warn-dangling-local.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/test/CXX/drs/dr16xx.cpp
cfe/trunk/test/SemaCXX/address-of-temporary.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=337790&r1=337789&r2=337790&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Mon Jul 23 17:55:08 2018
@@ -273,6 +273,10 @@ def OverloadedShiftOpParentheses: DiagGr
def DanglingElse: DiagGroup<"dangling-else">;
def DanglingField : DiagGroup<"dangling-field">;
def DanglingInitializerList : DiagGroup<"dangling-initializer-list">;
+def ReturnStackAddress : DiagGroup<"return-stack-address">;
+def Dangling : DiagGroup<"dangling", [DanglingField,
+ DanglingInitializerList,
+ ReturnStackAddress]>;
def DistributedObjectModifiers : DiagGroup<"distributed-object-modifiers">;
def ExpansionToDefined : DiagGroup<"expansion-to-defined">;
def FlagEnum : DiagGroup<"flag-enum">;
@@ -407,7 +411,6 @@ def RedeclaredClassMember : DiagGroup<"r
def GNURedeclaredEnum : DiagGroup<"gnu-redeclared-enum">;
def RedundantMove : DiagGroup<"redundant-move">;
def Register : DiagGroup<"register", [DeprecatedRegister]>;
-def ReturnStackAddress : DiagGroup<"return-stack-address">;
def ReturnTypeCLinkage : DiagGroup<"return-type-c-linkage">;
def ReturnType : DiagGroup<"return-type", [ReturnTypeCLinkage]>;
def BindToTemporaryCopy : DiagGroup<"bind-to-temporary-copy",
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=337790&r1=337789&r2=337790&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Jul 23 17:55:08 2018
@@ -1845,10 +1845,6 @@ def err_reference_bind_failed : Error<
"type $|could not bind to %select{rvalue|lvalue}1 of incompatible type}0,2">;
def err_reference_bind_init_list : Error<
"reference to type %0 cannot bind to an initializer list">;
-def warn_temporary_array_to_pointer_decay : Warning<
- "pointer is initialized by a temporary array, which will be destroyed at the "
- "end of the full-expression">,
- InGroup<DiagGroup<"address-of-array-temporary">>;
def err_init_list_bad_dest_type : Error<
"%select{|non-aggregate }0type %1 cannot be initialized with an initializer "
"list">;
@@ -7876,15 +7872,31 @@ def warn_init_ptr_member_to_parameter_ad
def note_ref_or_ptr_member_declared_here : Note<
"%select{reference|pointer}0 member declared here">;
-def err_bind_ref_member_to_temporary : Error<
+def err_dangling_member : Error<
"%select{reference|backing array for 'std::initializer_list'}2 "
"%select{|subobject of }1member %0 "
"%select{binds to|is}2 a temporary object "
- "whose lifetime would be shorter than the constructed object">;
+ "whose lifetime would be shorter than the lifetime of "
+ "the constructed object">;
+def warn_dangling_member : Warning<
+ "%select{reference|backing array for 'std::initializer_list'}2 "
+ "%select{|subobject of }1member %0 "
+ "%select{binds to|is}2 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 "
"declared here">;
+def warn_dangling_variable : Warning<
+ "%select{temporary %select{whose address is used as value of|bound to}3 "
+ "%select{%select{|reference }3member of local variable|"
+ "local %select{variable|reference}3}1|"
+ "array backing "
+ "%select{initializer list subobject of local variable|"
+ "local initializer list}1}0 "
+ "%2 will be destroyed at the end of the full-expression">,
+ InGroup<Dangling>;
def warn_new_dangling_reference : Warning<
"temporary bound to reference member of allocated object "
"will be destroyed at the end of the full-expression">,
@@ -7895,16 +7907,12 @@ def warn_new_dangling_initializer_list :
"the allocated initializer list}0 "
"will be destroyed at the end of the full-expression">,
InGroup<DanglingInitializerList>;
-def warn_unsupported_temporary_not_extended : Warning<
- "sorry, lifetime extension of temporary created "
- "by aggregate initialization using default member initializer "
- "is not supported; lifetime of temporary "
- "will end at the end of the full-expression">, InGroup<DanglingField>;
-def warn_unsupported_init_list_not_extended : Warning<
- "sorry, lifetime extension of backing array of initializer list created "
+def warn_unsupported_lifetime_extension : Warning<
+ "sorry, lifetime extension of "
+ "%select{temporary|backing array of initializer list}0 created "
"by aggregate initialization using default member initializer "
- "is not supported; lifetime of backing array will end at the end of the "
- "full-expression">, InGroup<DanglingInitializerList>;
+ "is not supported; lifetime of %select{temporary|backing array}0 "
+ "will end at the end of the full-expression">, InGroup<Dangling>;
// For non-floating point, expressions of the form x == x or x != x
// should result in a warning, since these always evaluate to a constant.
Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=337790&r1=337789&r2=337790&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Mon Jul 23 17:55:08 2018
@@ -6167,49 +6167,6 @@ PerformConstructorInitialization(Sema &S
return CurInit;
}
-/// Determine whether the specified InitializedEntity definitely has a lifetime
-/// longer than the current full-expression. Conservatively returns false if
-/// it's unclear.
-static bool
-InitializedEntityOutlivesFullExpression(const InitializedEntity &Entity) {
- const InitializedEntity *Top = &Entity;
- while (Top->getParent())
- Top = Top->getParent();
-
- switch (Top->getKind()) {
- case InitializedEntity::EK_Variable:
- case InitializedEntity::EK_Result:
- case InitializedEntity::EK_StmtExprResult:
- case InitializedEntity::EK_Exception:
- case InitializedEntity::EK_Member:
- case InitializedEntity::EK_Binding:
- case InitializedEntity::EK_New:
- case InitializedEntity::EK_Base:
- case InitializedEntity::EK_Delegating:
- return true;
-
- case InitializedEntity::EK_ArrayElement:
- case InitializedEntity::EK_VectorElement:
- case InitializedEntity::EK_BlockElement:
- case InitializedEntity::EK_LambdaToBlockConversionBlockElement:
- case InitializedEntity::EK_ComplexElement:
- // Could not determine what the full initialization is. Assume it might not
- // outlive the full-expression.
- return false;
-
- case InitializedEntity::EK_Parameter:
- case InitializedEntity::EK_Parameter_CF_Audited:
- case InitializedEntity::EK_Temporary:
- case InitializedEntity::EK_LambdaCapture:
- case InitializedEntity::EK_CompoundLiteralInit:
- case InitializedEntity::EK_RelatedResult:
- // The entity being initialized might not outlive the full-expression.
- return false;
- }
-
- llvm_unreachable("unknown entity kind");
-}
-
namespace {
enum LifetimeKind {
/// The lifetime of a temporary bound to this entity ends at the end of the
@@ -6393,6 +6350,13 @@ static bool isVarOnPath(IndirectLocalPat
return false;
}
+static bool pathContainsInit(IndirectLocalPath &Path) {
+ return std::any_of(Path.begin(), Path.end(), [=](IndirectLocalPathEntry E) {
+ return E.Kind == IndirectLocalPathEntry::DefaultInit ||
+ E.Kind == IndirectLocalPathEntry::VarInit;
+ });
+}
+
static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path,
Expr *Init, LocalVisitor Visit,
bool RevisitSubinits);
@@ -6660,6 +6624,12 @@ static void visitLocalsRetainedByInitial
// If the initializer is the address of a local, we could have a lifetime
// problem.
if (UO->getOpcode() == UO_AddrOf) {
+ // If this is &rvalue, then it's ill-formed and we have already diagnosed
+ // it. Don't produce a redundant warning about the lifetime of the
+ // temporary.
+ if (isa<MaterializeTemporaryExpr>(UO->getSubExpr()))
+ return;
+
Path.push_back({IndirectLocalPathEntry::AddressOf, UO});
visitLocalsRetainedByReferenceBinding(Path, UO->getSubExpr(),
RK_ReferenceBinding, Visit);
@@ -6761,9 +6731,14 @@ void Sema::checkInitializerLifetime(cons
case LK_Extended: {
auto *MTE = dyn_cast<MaterializeTemporaryExpr>(L);
- if (!MTE)
- // FIXME: Warn on this.
+ if (!MTE) {
+ // The initialized entity has lifetime beyond the full-expression,
+ // and the local entity does too, so don't warn.
+ //
+ // FIXME: We should consider warning if a static / thread storage
+ // duration variable retains an automatic storage duration local.
return false;
+ }
// Lifetime-extend the temporary.
if (Path.empty()) {
@@ -6779,17 +6754,21 @@ void Sema::checkInitializerLifetime(cons
// We're supposed to lifetime-extend the temporary along this path (per
// the resolution of DR1815), but we don't support that yet.
//
- // FIXME: Properly handle these situations.
- // For the default member initializer case, perhaps the easiest approach
+ // FIXME: Properly handle this situation. Perhaps the easiest approach
// would be to clone the initializer expression on each use that would
// lifetime extend its temporaries.
- Diag(DiagLoc, RK == RK_ReferenceBinding
- ? diag::warn_unsupported_temporary_not_extended
- : diag::warn_unsupported_init_list_not_extended)
- << DiagRange;
+ Diag(DiagLoc, diag::warn_unsupported_lifetime_extension)
+ << RK << DiagRange;
} else {
- // FIXME: Warn on this.
- return false;
+ // If the path goes through the initialization of a variable or field,
+ // it can't possibly reach a temporary created in this full-expression.
+ // We will have already diagnosed any problems with the initializer.
+ if (pathContainsInit(Path))
+ return false;
+
+ Diag(DiagLoc, diag::warn_dangling_variable)
+ << RK << !Entity.getParent() << ExtendingEntity->getDecl()
+ << Init->isGLValue() << DiagRange;
}
break;
}
@@ -6802,7 +6781,9 @@ void Sema::checkInitializerLifetime(cons
if (auto *ExtendingDecl =
ExtendingEntity ? ExtendingEntity->getDecl() : nullptr) {
bool IsSubobjectMember = ExtendingEntity != &Entity;
- Diag(DiagLoc, diag::err_bind_ref_member_to_temporary)
+ Diag(DiagLoc, shouldLifetimeExtendThroughPath(Path)
+ ? diag::err_dangling_member
+ : diag::warn_dangling_member)
<< ExtendingDecl << IsSubobjectMember << RK << DiagRange;
// Don't bother adding a note pointing to the field if we're inside
// its default member initializer; our primary diagnostic points to
@@ -6826,9 +6807,7 @@ void Sema::checkInitializerLifetime(cons
// Paths via a default initializer can only occur during error recovery
// (there's no other way that a default initializer can refer to a
// local). Don't produce a bogus warning on those cases.
- if (std::any_of(Path.begin(), Path.end(), [](IndirectLocalPathEntry E) {
- return E.Kind == IndirectLocalPathEntry::DefaultInit;
- }))
+ if (pathContainsInit(Path))
return false;
auto *DRE = dyn_cast<DeclRefExpr>(L);
@@ -6858,7 +6837,7 @@ void Sema::checkInitializerLifetime(cons
Diag(DiagLoc, RK == RK_ReferenceBinding
? diag::warn_new_dangling_reference
: diag::warn_new_dangling_initializer_list)
- << (ExtendingEntity != &Entity) << DiagRange;
+ << !Entity.getParent() << DiagRange;
} else {
// We can't determine if the allocation outlives the local declaration.
return false;
@@ -7184,21 +7163,6 @@ InitializationSequence::Perform(Sema &S,
return ExprError();
}
- // Diagnose cases where we initialize a pointer to an array temporary, and the
- // pointer obviously outlives the temporary.
- // FIXME: Fold this into checkInitializerLifetime.
- if (Args.size() == 1 && Args[0]->getType()->isArrayType() &&
- Entity.getType()->isPointerType() &&
- InitializedEntityOutlivesFullExpression(Entity)) {
- const Expr *Init = Args[0]->skipRValueSubobjectAdjustments();
- if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(Init))
- Init = MTE->GetTemporaryExpr();
- Expr::LValueClassification Kind = Init->ClassifyLValue(S.Context);
- if (Kind == Expr::LV_ClassTemporary || Kind == Expr::LV_ArrayTemporary)
- S.Diag(Init->getLocStart(), diag::warn_temporary_array_to_pointer_decay)
- << Init->getSourceRange();
- }
-
QualType DestType = Entity.getType().getNonReferenceType();
// FIXME: Ugly hack around the fact that Entity.getType() is not
// the same as Entity.getDecl()->getType() in cases involving type merging,
Modified: cfe/trunk/test/CXX/drs/dr16xx.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/drs/dr16xx.cpp?rev=337790&r1=337789&r2=337790&view=diff
==============================================================================
--- cfe/trunk/test/CXX/drs/dr16xx.cpp (original)
+++ cfe/trunk/test/CXX/drs/dr16xx.cpp Mon Jul 23 17:55:08 2018
@@ -300,7 +300,7 @@ namespace dr1696 { // dr1696: 7
#if __cplusplus >= 201103L
struct B {
A &&a; // expected-note {{declared here}}
- B() : a{} {} // expected-error {{reference member 'a' binds to a temporary object whose lifetime would be shorter than the constructed object}}
+ B() : a{} {} // expected-error {{reference member 'a' binds to a temporary object whose lifetime would be shorter than the lifetime of the constructed object}}
} b;
#endif
@@ -308,7 +308,7 @@ namespace dr1696 { // dr1696: 7
C();
const A &a; // expected-note {{declared here}}
};
- C::C() : a(A()) {} // expected-error {{reference member 'a' binds to a temporary object whose lifetime would be shorter than the constructed object}}
+ C::C() : a(A()) {} // expected-error {{reference member 'a' binds to a temporary object whose lifetime would be shorter than the lifetime of the constructed object}}
#if __cplusplus >= 201103L
// This is OK in C++14 onwards, per DR1815, though we don't support that yet:
Modified: cfe/trunk/test/SemaCXX/address-of-temporary.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/address-of-temporary.cpp?rev=337790&r1=337789&r2=337790&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/address-of-temporary.cpp (original)
+++ cfe/trunk/test/SemaCXX/address-of-temporary.cpp Mon Jul 23 17:55:08 2018
@@ -26,11 +26,12 @@ namespace PointerToArrayDecay {
template<typename T> void consume(T);
struct S { int *p; };
- void g0() { int *p = Y().a; } // expected-warning{{pointer is initialized by a temporary array}}
- void g1() { int *p = Y{}.a; } // expected-warning{{pointer is initialized by a temporary array}}
- void g2() { int *p = A{}; } // expected-warning{{pointer is initialized by a temporary array}}
- void g3() { int *p = (A){}; } // expected-warning{{pointer is initialized by a temporary array}}
- void g4() { Z *p = AZ{}; } // expected-warning{{pointer is initialized by a temporary array}}
+ void g0() { int *p = Y().a; } // expected-warning{{will be destroyed at the end of the full-expression}}
+ void g1() { int *p = Y{}.a; } // expected-warning{{will be destroyed at the end of the full-expression}}
+ void g2() { int *p = A{}; } // expected-warning{{will be destroyed at the end of the full-expression}}
+ void g3() { int *p = (A){}; } // expected-warning{{will be destroyed at the end of the full-expression}}
+ void g4() { Z *p = AZ{}; } // expected-warning{{will be destroyed at the end of the full-expression}}
+ void g5() { Z *p = &(Z&)(AZ{}[0]); } // expected-warning{{will be destroyed at the end of the full-expression}}
void h0() { consume(Y().a); }
void h1() { consume(Y{}.a); }
@@ -38,10 +39,10 @@ namespace PointerToArrayDecay {
void h3() { consume((A){}); }
void h4() { consume(AZ{}); }
- void i0() { S s = { Y().a }; } // expected-warning{{pointer is initialized by a temporary array}}
- void i1() { S s = { Y{}.a }; } // expected-warning{{pointer is initialized by a temporary array}}
- void i2() { S s = { A{} }; } // expected-warning{{pointer is initialized by a temporary array}}
- void i3() { S s = { (A){} }; } // expected-warning{{pointer is initialized by a temporary array}}
+ void i0() { S s = { Y().a }; } // expected-warning{{will be destroyed at the end of the full-expression}}
+ void i1() { S s = { Y{}.a }; } // expected-warning{{will be destroyed at the end of the full-expression}}
+ void i2() { S s = { A{} }; } // expected-warning{{will be destroyed at the end of the full-expression}}
+ void i3() { S s = { (A){} }; } // expected-warning{{will be destroyed at the end of the full-expression}}
void j0() { (void)S { Y().a }; }
void j1() { (void)S { Y{}.a }; }
Added: cfe/trunk/test/SemaCXX/warn-dangling-local.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-dangling-local.cpp?rev=337790&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-dangling-local.cpp (added)
+++ cfe/trunk/test/SemaCXX/warn-dangling-local.cpp Mon Jul 23 17:55:08 2018
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -verify -std=c++11 %s
+
+using T = int[];
+
+void f() {
+ int *p = &(int&)(int&&)0; // expected-warning {{temporary whose address is used as value of local variable 'p' will be destroyed at the end of the full-expression}}
+
+ int *q = (int *const &)T{1, 2, 3}; // expected-warning {{temporary whose address is used as value of local variable 'q' will be destroyed at the end of the full-expression}}
+
+ // FIXME: We don't warn here because the 'int*' temporary is not const, but
+ // it also can't have actually changed since it was created, so we could
+ // still warn.
+ int *r = (int *&&)T{1, 2, 3};
+
+ // FIXME: The wording of this warning is not quite right. There are two
+ // temporaries here: an 'int* const' temporary that points to the array, and
+ // is lifetime-extended, and an array temporary that the pointer temporary
+ // points to, which doesn't live long enough.
+ int *const &s = (int *const &)T{1, 2, 3}; // expected-warning {{temporary bound to local reference 's' will be destroyed at the end of the full-expression}}
+}
More information about the cfe-commits
mailing list