From: Jason Merrill Date: Tue, 29 May 2018 20:04:52 +0000 (-0400) Subject: PR c++/67445 - returning temporary initializer_list. X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=04eb9c55747cc28466875e891ac22acb3ea67644;p=gcc.git PR c++/67445 - returning temporary initializer_list. PR c++/67711 - assigning from temporary initializer_list. PR c++/48562 - new initializer_list. * typeck.c (maybe_warn_about_returning_address_of_local): Also warn about returning local initializer_list. * cp-tree.h (AUTO_TEMP_NAME, TEMP_NAME_P): Remove. * call.c (build_over_call): Warn about assignment from temporary init_list. * init.c (build_new_1): Warn about 'new std::initializer_list'. (find_list_begin, maybe_warn_list_ctor): New. (perform_member_init): Use maybe_warn_list_ctor. From-SVN: r260905 --- diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog index 40565d68fdc..cb6ee1319c7 100644 --- a/gcc/c-family/ChangeLog +++ b/gcc/c-family/ChangeLog @@ -1,3 +1,7 @@ +2018-05-29 Jason Merrill + + * c.opt (Winit-list-lifetime): New flag. + 2018-05-28 Bernd Edlinger * c-lex.c (get_fileinfo): Use splay_tree_compare_strings and diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 5114543c128..6031cc356b0 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -608,6 +608,10 @@ Winit-self C ObjC C++ ObjC++ Var(warn_init_self) Warning LangEnabledBy(C++ ObjC++,Wall) Warn about variables which are initialized to themselves. +Winit-list-lifetime +C++ ObjC++ Var(warn_init_list) Warning Init(1) +Warn about uses of std::initializer_list that can result in dangling pointers. + Wimplicit C ObjC Var(warn_implicit) Warning LangEnabledBy(C ObjC,Wall) Warn about implicit declarations. diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index 9ccdfbe594f..e94615f0ea3 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,17 @@ +2018-05-29 Jason Merrill + + PR c++/67445 - returning temporary initializer_list. + PR c++/67711 - assigning from temporary initializer_list. + PR c++/48562 - new initializer_list. + * typeck.c (maybe_warn_about_returning_address_of_local): Also warn + about returning local initializer_list. + * cp-tree.h (AUTO_TEMP_NAME, TEMP_NAME_P): Remove. + * call.c (build_over_call): Warn about assignment from temporary + init_list. + * init.c (build_new_1): Warn about 'new std::initializer_list'. + (find_list_begin, maybe_warn_list_ctor): New. + (perform_member_init): Use maybe_warn_list_ctor. + 2018-05-29 Marek Polacek PR c++/85883 diff --git a/gcc/cp/call.c b/gcc/cp/call.c index 7aadd642ebb..2bbf9837487 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -8217,6 +8217,7 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain) tree type = TREE_TYPE (to); tree as_base = CLASSTYPE_AS_BASE (type); tree arg = argarray[1]; + location_t loc = EXPR_LOC_OR_LOC (arg, input_location); if (is_really_empty_class (type)) { @@ -8226,6 +8227,11 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain) } else if (tree_int_cst_equal (TYPE_SIZE (type), TYPE_SIZE (as_base))) { + if (is_std_init_list (type) + && conv_binds_ref_to_prvalue (convs[1])) + warning_at (loc, OPT_Winit_list_lifetime, + "assignment from temporary initializer_list does not " + "extend the lifetime of the underlying array"); arg = cp_build_fold_indirect_ref (arg); val = build2 (MODIFY_EXPR, TREE_TYPE (to), to, arg); } diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index e18480b2805..6a97abbe4e3 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -5234,10 +5234,6 @@ extern GTY(()) vec *keyed_classes; #else /* NO_DOLLAR_IN_LABEL */ -#define AUTO_TEMP_NAME "__tmp_" -#define TEMP_NAME_P(ID_NODE) \ - (!strncmp (IDENTIFIER_POINTER (ID_NODE), AUTO_TEMP_NAME, \ - sizeof (AUTO_TEMP_NAME) - 1)) #define VTABLE_NAME "__vt_" #define VTABLE_NAME_P(ID_NODE) \ (!strncmp (IDENTIFIER_POINTER (ID_NODE), VTABLE_NAME, \ @@ -5272,8 +5268,6 @@ extern GTY(()) vec *keyed_classes; && IDENTIFIER_POINTER (ID_NODE)[2] == 't' \ && IDENTIFIER_POINTER (ID_NODE)[3] == JOINER) -#define TEMP_NAME_P(ID_NODE) \ - (!strncmp (IDENTIFIER_POINTER (ID_NODE), AUTO_TEMP_NAME, sizeof (AUTO_TEMP_NAME)-1)) #define VFIELD_NAME_P(ID_NODE) \ (!strncmp (IDENTIFIER_POINTER (ID_NODE), VFIELD_NAME, sizeof(VFIELD_NAME)-1)) @@ -6888,6 +6882,7 @@ extern void finish_label_decl (tree); extern cp_expr finish_parenthesized_expr (cp_expr); extern tree force_paren_expr (tree); extern tree maybe_undo_parenthesized_ref (tree); +extern tree maybe_strip_ref_conversion (tree); extern tree finish_non_static_data_member (tree, tree, tree); extern tree begin_stmt_expr (void); extern tree finish_stmt_expr_expr (tree, tree); diff --git a/gcc/cp/init.c b/gcc/cp/init.c index b925e843432..24119d18802 100644 --- a/gcc/cp/init.c +++ b/gcc/cp/init.c @@ -674,6 +674,64 @@ maybe_reject_flexarray_init (tree member, tree init) return true; } +/* If INIT's value can come from a call to std::initializer_list::begin, + return that function. Otherwise, NULL_TREE. */ + +static tree +find_list_begin (tree init) +{ + STRIP_NOPS (init); + while (TREE_CODE (init) == COMPOUND_EXPR) + init = TREE_OPERAND (init, 1); + STRIP_NOPS (init); + if (TREE_CODE (init) == COND_EXPR) + { + tree left = TREE_OPERAND (init, 1); + if (!left) + left = TREE_OPERAND (init, 0); + left = find_list_begin (left); + if (left) + return left; + return find_list_begin (TREE_OPERAND (init, 2)); + } + if (TREE_CODE (init) == CALL_EXPR) + if (tree fn = get_callee_fndecl (init)) + if (id_equal (DECL_NAME (fn), "begin") + && is_std_init_list (DECL_CONTEXT (fn))) + return fn; + return NULL_TREE; +} + +/* If INIT initializing MEMBER is copying the address of the underlying array + of an initializer_list, warn. */ + +static void +maybe_warn_list_ctor (tree member, tree init) +{ + tree memtype = TREE_TYPE (member); + if (!init || !TYPE_PTR_P (memtype) + || !is_list_ctor (current_function_decl)) + return; + + tree parms = FUNCTION_FIRST_USER_PARMTYPE (current_function_decl); + tree initlist = non_reference (TREE_VALUE (parms)); + tree targs = CLASSTYPE_TI_ARGS (initlist); + tree elttype = TREE_VEC_ELT (targs, 0); + + if (!same_type_ignoring_top_level_qualifiers_p + (TREE_TYPE (memtype), elttype)) + return; + + tree begin = find_list_begin (init); + if (!begin) + return; + + location_t loc = EXPR_LOC_OR_LOC (init, input_location); + warning_at (loc, OPT_Winit_list_lifetime, + "initializing %qD from %qE does not extend the lifetime " + "of the underlying array", member, begin); +} + /* Initialize MEMBER, a FIELD_DECL, with INIT, a TREE_LIST of arguments. If TREE_LIST is void_type_node, an empty initializer list was given; if NULL_TREE no initializer was given. */ @@ -886,6 +944,8 @@ perform_member_init (tree member, tree init) init = build_x_compound_expr_from_list (init, ELK_MEM_INIT, tf_warning_or_error); + maybe_warn_list_ctor (member, init); + /* Reject a member initializer for a flexible array member. */ if (init && !maybe_reject_flexarray_init (member, init)) finish_expr_stmt (cp_build_modify_expr (input_location, decl, @@ -2934,6 +2994,11 @@ build_new_1 (vec **placement, tree type, tree nelts, return error_mark_node; } + if (is_std_init_list (elt_type)) + warning (OPT_Winit_list_lifetime, + "% of initializer_list does not " + "extend the lifetime of the underlying array"); + if (abstract_virtuals_error_sfinae (ACU_NEW, elt_type, complain)) return error_mark_node; diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index 3df043e2ab4..25d11f5c7b6 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -9012,6 +9012,7 @@ maybe_warn_about_returning_address_of_local (tree retval) { tree valtype = TREE_TYPE (DECL_RESULT (current_function_decl)); tree whats_returned = fold_for_warn (retval); + location_t loc = EXPR_LOC_OR_LOC (retval, input_location); for (;;) { @@ -9024,6 +9025,21 @@ maybe_warn_about_returning_address_of_local (tree retval) break; } + if (TREE_CODE (whats_returned) == TARGET_EXPR + && is_std_init_list (TREE_TYPE (whats_returned))) + { + tree init = TARGET_EXPR_INITIAL (whats_returned); + if (TREE_CODE (init) == CONSTRUCTOR) + /* Pull out the array address. */ + whats_returned = CONSTRUCTOR_ELT (init, 0)->value; + else if (TREE_CODE (init) == INDIRECT_REF) + /* The source of a trivial copy looks like *(T*)&var. */ + whats_returned = TREE_OPERAND (init, 0); + else + return false; + STRIP_NOPS (whats_returned); + } + if (TREE_CODE (whats_returned) != ADDR_EXPR) return false; whats_returned = TREE_OPERAND (whats_returned, 0); @@ -9032,21 +9048,17 @@ maybe_warn_about_returning_address_of_local (tree retval) || TREE_CODE (whats_returned) == ARRAY_REF) whats_returned = TREE_OPERAND (whats_returned, 0); - if (TYPE_REF_P (valtype)) + if (TREE_CODE (whats_returned) == AGGR_INIT_EXPR + || TREE_CODE (whats_returned) == TARGET_EXPR) { - if (TREE_CODE (whats_returned) == AGGR_INIT_EXPR - || TREE_CODE (whats_returned) == TARGET_EXPR) - { - warning (OPT_Wreturn_local_addr, "returning reference to temporary"); - return true; - } - if (VAR_P (whats_returned) - && DECL_NAME (whats_returned) - && TEMP_NAME_P (DECL_NAME (whats_returned))) - { - warning (OPT_Wreturn_local_addr, "reference to non-lvalue returned"); - return true; - } + if (TYPE_REF_P (valtype)) + warning_at (loc, OPT_Wreturn_local_addr, + "returning reference to temporary"); + else if (is_std_init_list (valtype)) + warning_at (loc, OPT_Winit_list_lifetime, + "returning temporary initializer_list does not extend " + "the lifetime of the underlying array"); + return true; } if (DECL_P (whats_returned) @@ -9056,19 +9068,27 @@ maybe_warn_about_returning_address_of_local (tree retval) && !(TREE_STATIC (whats_returned) || TREE_PUBLIC (whats_returned))) { + bool w = false; if (TYPE_REF_P (valtype)) - warning_at (DECL_SOURCE_LOCATION (whats_returned), - OPT_Wreturn_local_addr, - "reference to local variable %qD returned", - whats_returned); + w = warning_at (loc, OPT_Wreturn_local_addr, + "reference to local variable %qD returned", + whats_returned); + else if (is_std_init_list (valtype)) + w = warning_at (loc, OPT_Winit_list_lifetime, + "returning local initializer_list variable %qD " + "does not extend the lifetime of the underlying array", + whats_returned); else if (TREE_CODE (whats_returned) == LABEL_DECL) - warning_at (DECL_SOURCE_LOCATION (whats_returned), - OPT_Wreturn_local_addr, "address of label %qD returned", - whats_returned); + w = warning_at (loc, OPT_Wreturn_local_addr, + "address of label %qD returned", + whats_returned); else - warning_at (DECL_SOURCE_LOCATION (whats_returned), - OPT_Wreturn_local_addr, "address of local variable %qD " - "returned", whats_returned); + w = warning_at (loc, OPT_Wreturn_local_addr, + "address of local variable %qD returned", + whats_returned); + if (w) + inform (DECL_SOURCE_LOCATION (whats_returned), + "declared here"); return true; } @@ -9402,7 +9422,8 @@ check_return_expr (tree retval, bool *no_warning) retval = build2 (COMPOUND_EXPR, TREE_TYPE (retval), retval, TREE_OPERAND (retval, 0)); else if (!processing_template_decl - && maybe_warn_about_returning_address_of_local (retval)) + && maybe_warn_about_returning_address_of_local (retval) + && INDIRECT_TYPE_P (valtype)) retval = build2 (COMPOUND_EXPR, TREE_TYPE (retval), retval, build_zero_cst (TREE_TYPE (retval))); } diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 4a459280dea..53ef14cf170 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -2909,6 +2909,51 @@ assignment operator is deprecated if the class has a user-provided copy constructor, copy assignment operator, or destructor, in C++11 and up. This warning is enabled by @option{-Wall}. +@item -Wno-init-list-lifetime @r{(C++ and Objective-C++ only)} +@opindex Winit-list-lifetime +@opindex Wno-init-list-lifetime +Do not warn about uses of @code{std::initializer_list} that are likely +to result in dangling pointers. Since the underlying array for an +@code{initializer_list} is handled like a normal C++ temporary object, +it is easy to inadvertently keep a pointer to the array past the end +of the array's lifetime. For example: + +@itemize @bullet +@item +If a function returns a temporary @code{initializer_list}, or a local +@code{initializer_list} variable, the array's lifetime ends at the end +of the return statement, so the value returned has a dangling pointer. + +@item +If a new-expression creates an @code{initializer_list}, the array only +lives until the end of the enclosing full-expression, so the +@code{initializer_list} in the heap has a dangling pointer. + +@item +When an @code{initializer_list} variable is assigned from a +brace-enclosed initializer list, the temporary array created for the +right side of the assignment only lives until the end of the +full-expression, so at the next statement the @code{initializer_list} +variable has a dangling pointer. + +@smallexample +// li's initial underlying array lives as long as li +std::initializer_list li = @{ 1,2,3 @}; +// assignment changes li to point to a temporary array +li = @{ 4, 5 @}; +// now the temporary is gone and li has a dangling pointer +int i = li.begin()[0] // undefined behavior +@end smallexample + +@item +When a list constructor stores the @code{begin} pointer from the +@code{initializer_list} argument, this doesn't extend the lifetime of +the array, so if a class variable is constructed from a temporary +@code{initializer_list}, the pointer is left dangling by the end of +the variable declaration statement. + +@end itemize + @item -Wliteral-suffix @r{(C++ and Objective-C++ only)} @opindex Wliteral-suffix @opindex Wno-literal-suffix diff --git a/gcc/testsuite/c-c++-common/pr43395.c b/gcc/testsuite/c-c++-common/pr43395.c index d060ae2a968..6f44ac9b110 100644 --- a/gcc/testsuite/c-c++-common/pr43395.c +++ b/gcc/testsuite/c-c++-common/pr43395.c @@ -5,27 +5,24 @@ void * foo (void) { - lab: /* { dg-line foo_lab } */ + lab: return &&lab; -/* { dg-warning "function returns address of label" "" { target c } .-1 } */ -/* { dg-warning "address of label" "" { target c++ } foo_lab } */ +/* { dg-warning "address of label" "" { target *-*-* } .-1 } */ } void * bar (void) { __label__ lab; - lab: /* { dg-line bar_lab } */ + lab: return &&lab; -/* { dg-warning "function returns address of label" "" { target c } .-1 } */ -/* { dg-warning "address of label" "" { target c++ } bar_lab } */ +/* { dg-warning "address of label" "" { target *-*-* } .-1 } */ } void * baz (void) { - int i; /* { dg-line baz_i } */ + int i; return &i; -/* { dg-warning "function returns address of local variable" "" { target c } .-1 } */ -/* { dg-warning "address of local variable" "" { target c++ } baz_i } */ +/* { dg-warning "address of local variable" "" { target *-*-* } .-1 } */ } diff --git a/gcc/testsuite/g++.dg/cpp1y/pr77591.C b/gcc/testsuite/g++.dg/cpp1y/pr77591.C index 8f9e28c8501..42c127aae99 100644 --- a/gcc/testsuite/g++.dg/cpp1y/pr77591.C +++ b/gcc/testsuite/g++.dg/cpp1y/pr77591.C @@ -7,13 +7,13 @@ class A { }; decltype(auto) foo () { - A c; // { dg-warning "reference to local variable 'c' returned" } - return (c); + A c; + return (c); // { dg-warning "reference to local variable 'c' returned" } } decltype(auto) bar () { - A c; // { dg-warning "reference to local variable 'c' returned" } - return 1==1 ? c : c; + A c; + return 1==1 ? c : c; // { dg-warning "reference to local variable 'c' returned" } } diff --git a/gcc/testsuite/g++.dg/warn/Winit-list1.C b/gcc/testsuite/g++.dg/warn/Winit-list1.C new file mode 100644 index 00000000000..bf3cb094e0f --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Winit-list1.C @@ -0,0 +1,15 @@ +// PR c++/67711, 48562 +// { dg-do compile { target c++11 } } + +#include + +using IL = std::initializer_list; +int main() +{ + IL il = { 1,2,3 }; + il = { 4,5,6 }; // { dg-warning "initializer_list" } + // the array is dead, il now points to garbage + il = *new IL{ 7, 8, 9 }; // { dg-warning "initializer_list" } + // the array is dead, il now points to garbage + return *il.begin(); // undefined +} diff --git a/gcc/testsuite/g++.dg/warn/Winit-list2.C b/gcc/testsuite/g++.dg/warn/Winit-list2.C new file mode 100644 index 00000000000..2ba4b37f029 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Winit-list2.C @@ -0,0 +1,32 @@ +// { dg-do compile { target c++11 } } + +#include + +extern "C" int printf (const char *, ...); + +using size_t = decltype(sizeof(0)); + +template class ArrayRef { +public: + using size_type = size_t; + +private: + /// The start of the array, in an external buffer. + const T *Data = nullptr; + + /// The number of elements. + size_type Length = 0; + +public: + /// Construct an ArrayRef from a std::initializer_list. + /*implicit*/ ArrayRef(const std::initializer_list &Vec) + : Data(Vec.begin() == Vec.end() ? (T *)nullptr : Vec.begin()), // { dg-warning initializer_list } + Length(Vec.size()) {} + + const T &operator[](size_t Index) const { return Data[Index]; } +}; + +int main() { + const ArrayRef Foo = {42}; + printf ("Foo %d\n", Foo[0]); +} diff --git a/gcc/testsuite/g++.dg/warn/Winit-list3.C b/gcc/testsuite/g++.dg/warn/Winit-list3.C new file mode 100644 index 00000000000..7736ca46e53 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Winit-list3.C @@ -0,0 +1,34 @@ +// PR c++/67445 +// { dg-do compile { target c++11 } } + +#include + +using SL = std::initializer_list; + +SL retArray(int i) noexcept +{ + if (i == 0) + { + SL l{"Test 1", "Test 2", "Test 3"}; // { dg-message "declared" } + return l; // { dg-warning "initializer_list" } + } + else if (i == 1) + return SL{"Test 1", "Test 2", "Test 3"}; // { dg-warning "initializer_list" } + else if (i == 2) + return {"Test 1", "Test 2", "Test 3"}; // { dg-warning "initializer_list" } + else + { + static SL l{"Test 1", "Test 2", "Test 3"}; + return l; // no warning about returning static. + } +} + +const char *p; +int main(int, char const* const*) +{ + for (auto&& i : retArray(1)) + { + p = i; + } + return 0; +} diff --git a/gcc/testsuite/g++.dg/warn/Wreturn-local-addr.C b/gcc/testsuite/g++.dg/warn/Wreturn-local-addr.C index faa3a345440..642a5767e84 100644 --- a/gcc/testsuite/g++.dg/warn/Wreturn-local-addr.C +++ b/gcc/testsuite/g++.dg/warn/Wreturn-local-addr.C @@ -4,14 +4,14 @@ int& bad1() { - int x = 0; // { dg-error "reference to local variable" } - return x; + int x = 0; + return x; // { dg-error "reference to local variable" } } int* bad2() { - int x = 0; // { dg-error "address of local variable" } - return &x; + int x = 0; + return &x; // { dg-error "address of local variable" } } const int& bad4() diff --git a/gcc/testsuite/g++.dg/warn/return-reference2.C b/gcc/testsuite/g++.dg/warn/return-reference2.C index 190266215a1..e9004f77a76 100644 --- a/gcc/testsuite/g++.dg/warn/return-reference2.C +++ b/gcc/testsuite/g++.dg/warn/return-reference2.C @@ -10,12 +10,12 @@ public: int &f() { - A a; // { dg-warning "local" } - return a.second; + A a; + return a.second; // { dg-warning "local" } } int &g() { - int ar[42]; // { dg-warning "local" } - return ar[20]; + int ar[42]; + return ar[20]; // { dg-warning "local" } } diff --git a/gcc/testsuite/g++.old-deja/g++.bob/array1.C b/gcc/testsuite/g++.old-deja/g++.bob/array1.C index dac0420c83d..2ec84fb0f6b 100644 --- a/gcc/testsuite/g++.old-deja/g++.bob/array1.C +++ b/gcc/testsuite/g++.old-deja/g++.bob/array1.C @@ -1,6 +1,6 @@ // { dg-do assemble } char *stuff() { - char array[10]; // { dg-warning "" } + char array[10]; - return array; + return array; // { dg-warning "" } } diff --git a/gcc/testsuite/g++.old-deja/g++.brendan/crash55.C b/gcc/testsuite/g++.old-deja/g++.brendan/crash55.C index 3faa538253b..fd4d4b65edb 100644 --- a/gcc/testsuite/g++.old-deja/g++.brendan/crash55.C +++ b/gcc/testsuite/g++.old-deja/g++.brendan/crash55.C @@ -4,9 +4,9 @@ int& f(int x) // { dg-error "new declaration" } { - int local; // { dg-warning "reference to local" } + int local; local = x+2; - return local; + return local; // { dg-warning "reference to local" } } diff --git a/libstdc++-v3/testsuite/util/testsuite_random.h b/libstdc++-v3/testsuite/util/testsuite_random.h index 0ce95df3900..f06eb9489bb 100644 --- a/libstdc++-v3/testsuite/util/testsuite_random.h +++ b/libstdc++-v3/testsuite/util/testsuite_random.h @@ -113,7 +113,10 @@ namespace __gnu_test discrete_pdf(int k, std::initializer_list wl) { if (!wl.size()) - wl = { 1.0 }; + { + static std::initializer_list one = { 1.0 }; + wl = one; + } if (k < 0 || (std::size_t)k >= wl.size()) return 0.0;