Nice! This found one bug in the libc++abi tests (r337906), and started diagnosing the dangling tuple reference case that libc++ worked hard to diagnose manually (r337905).
/Eric On Mon, Jul 23, 2018 at 6:55 PM Richard Smith via cfe-commits < cfe-commits@lists.llvm.org> wrote: > 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}} > +} > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
