r337790 - Warn if a local variable's initializer retains a pointer/reference to a
Eric Fiselier via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 25 04:22:43 PDT 2018
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 at 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 at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180725/3a61b615/attachment-0001.html>
More information about the cfe-commits
mailing list