| Message ID | 20201115224448.1263247-1-polacek@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: Implement -Wuninitialized for mem-initializers [PR19808] | expand |
On 11/15/20 3:44 PM, Marek Polacek via Gcc-patches wrote: > This patch implements the long-desired -Wuninitialized warning for > member initializer lists, so that the front end can detect bugs like > > struct A { > int a; > int b; > A() : b(1), a(b) { } > }; > > where the field 'b' is used uninitialized because the order of member > initializers in the member initializer list is irrelevant; what matters > is the order of declarations in the class definition. > > I've implemented this by keeping a hash set holding fields that are not > initialized yet, so at first it will be {a, b}, and after initializing > 'a' it will be {b} and so on. Then I use walk_tree to walk the > initializer and if we see that an uninitialized object is used, we warn. > Of course, when we use the address of the object, we may not warn: > > struct B { > int &r; > int *p; > int a; > B() : r(a), p(&a), a(1) { } // ok > }; > > Likewise, don't warn in unevaluated contexts such as sizeof. Classes > without an explicit initializer may still be initialized by their > default constructors; whether or not something is considered initialized > is handled in perform_member_init, see member_initialized_p. It is > possible that we might want to tweak that if the current approach turns > out to be flawed. > > And this is where things stop being simple. I had to implement certain > gymnastics to handle cases like > > struct S { > int a, b, c; > S() : a((b = 42)), c(b) { } > }; > > where the initializer for 'a' surreptitiously initializes 'b'. And since > 'b' isn't read in the assignment, we can't warn that it's being used > uninitialized. > > Also, initializer lists. Consider: > > struct A { > int x, y, z; > }; > > struct S { > A a; > S() : a{ 1, 2, a.y } {} // #1 > S(int) : a{ a.y, 2, 3 } {} // #2 > }; > > In #1, a.y refers to an initialized element, so we oughtn't warn. But > in #2, a.y refers to an uninitialized element, so we should warn. To > that end I've added a vector that tracks which elements of an initializer > list are already initialized, so that when we encounter them later in > the list, we don't warn. This is the find_uninit_data->list_inits > thing. It will contain paths to the initialized elements, so e.g. > > ((S *)this)->a.y > > These are edge cases, but some developers live on the edge. > > This patch also handles delegating constructors and initializing base > classes. > > A couple of TODOs: > - the same approach could be used to warn for NSDMIs, but I think it's out > of scope for this patch; > - the diagnostic for anonymous unions/structs should be improved > to say the name of the member instead of "<anonymous>"; > - certain uninitialized warnings in initializer-lists are issued > by the FE as well as the ME. It's unclear to me what we want to do > here; > - using uninitialized members of base classes is not implemented yet. This is a great enhancement! Detecting the misues even in inline functions is especially useful. I installed the patch to see how it handles some of the cases the middle end was recently enhanced to detect. Below is a test case I tried and the outupt. Clearly, the middle end warning doesn't work as intended for C++ ctors, so that's something for me to look into. It would be good to enhance the front end warning to use the same assumptions as the middle end (i.e., that arguments passed by reference to const-qualified parameters are read from). This is also the basis for warning on the call to strlen in the test case. That way both could detect the same problems (the front end even in inline functions). There's a duplicate warning for the first ctor that we might want to try to avoid, probably by tweaking the middle end one. Martin $ cat a.C && gcc -O2 -S -Wall a.C int f (int); int g (const int&); int h (const int*); struct A { A (int(*)[1]); A (int(*)[2]); A (int(*)[3]); A (int(*)[4]); int a, b; char s[32]; }; A::A (int(*)[1]): a {f (b)} { } // duplicate warning A::A (int(*)[2]): a {g (b)} { } // missing warning A::A (int(*)[3]): a {h (&b)} { } // missing warning A::A (int(*)[4]): a (__builtin_strlen (s)) { } void ff (void) { int a; f (a); // warning } void gg (void) { int a; g (a); // warning } void hh (void) { int a; h (&a); // warning } int ii (void) { char s[32]; return __builtin_strlen (s); // warning } a.C: In constructor ‘A::A(int (*)[1])’: a.C:15:25: warning: field ‘A::b’ is used uninitialized [-Wuninitialized] 15 | A::A (int(*)[1]): a {f (b)} { } // duplicate warning | ^ a.C: In constructor ‘A::A(int (*)[1])’: a.C:15:24: warning: ‘*<unknown>.A::b’ is used uninitialized [-Wuninitialized] 15 | A::A (int(*)[1]): a {f (b)} { } // duplicate warning | ~~^~~ a.C: In function ‘void ff()’: a.C:23:5: warning: ‘a’ is used uninitialized [-Wuninitialized] 23 | f (a); // warning | ~~^~~ a.C: In function ‘void gg()’: a.C:29:5: warning: ‘a’ may be used uninitialized [-Wmaybe-uninitialized] 29 | g (a); // warning | ~~^~~ a.C:2:5: note: by argument 1 of type ‘const int&’ to ‘int g(const int&)’ declared here 2 | int g (const int&); | ^ a.C:28:7: note: ‘a’ declared here 28 | int a; | ^ a.C: In function ‘void hh()’: a.C:35:5: warning: ‘a’ may be used uninitialized [-Wmaybe-uninitialized] 35 | h (&a); // warning | ~~^~~~ a.C:3:5: note: by argument 1 of type ‘const int*’ to ‘int h(const int*)’ declared here 3 | int h (const int*); | ^ a.C:34:7: note: ‘a’ declared here 34 | int a; | ^ a.C: In function ‘int ii()’: a.C:41:27: warning: ‘s’ is used uninitialized [-Wuninitialized] 41 | return __builtin_strlen (s); // warning | ~~~~~~~~~~~~~~~~~^~~ <built-in>: note: by argument 1 of type ‘const char*’ to ‘long unsigned int __builtin_strlen(const char*)’ declared here a.C:40:8: note: ‘s’ declared here 40 | char s[32]; | ^ > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > gcc/cp/ChangeLog: > > PR c++/19808 > PR c++/96121 > * init.c (perform_member_init): Remove a forward declaration. > Walk the initializer using find_uninit_fields_r. New parameter > to track uninitialized fields. If a member is initialized, > remove it from the hash set. > (perform_target_ctor): Return the initializer. > (struct find_uninit_data): New class. > (find_uninit_fields_r): New function. > (emit_mem_initializers): Keep and initialize a set holding fields > that are not initialized. When handling delegating constructors, > walk the constructor tree using find_uninit_fields_r. Also when > initializing base clases. Pass uninitialized down to > perform_member_init. > > gcc/ChangeLog: > > PR c++/19808 > PR c++/96121 > * doc/invoke.texi: Update documentation for -Wuninitialized. > * tree.c (stabilize_reference): Set location. > > gcc/testsuite/ChangeLog: > > PR c++/19808 > PR c++/96121 > * g++.dg/warn/Wuninitialized-12.C: New test. > * g++.dg/warn/Wuninitialized-13.C: New test. > * g++.dg/warn/Wuninitialized-14.C: New test. > * g++.dg/warn/Wuninitialized-15.C: New test. > * g++.dg/warn/Wuninitialized-16.C: New test. > * g++.dg/warn/Wuninitialized-17.C: New test. > * g++.dg/warn/Wuninitialized-18.C: New test. > * g++.dg/warn/Wuninitialized-19.C: New test. > * g++.dg/warn/Wuninitialized-20.C: New test. > * g++.dg/warn/Wuninitialized-21.C: New test. > * g++.dg/warn/Wuninitialized-22.C: New test. > * g++.dg/warn/Wuninitialized-23.C: New test. > * g++.dg/warn/Wuninitialized-24.C: New test. > * g++.dg/warn/Wuninitialized-25.C: New test. > * g++.dg/warn/Wuninitialized-26.C: New test. > * g++.dg/warn/Wuninitialized-27.C: New test. > --- > gcc/cp/init.c | 235 +++++++++++++++++- > gcc/doc/invoke.texi | 12 + > gcc/testsuite/g++.dg/warn/Wuninitialized-12.C | 59 +++++ > gcc/testsuite/g++.dg/warn/Wuninitialized-13.C | 49 ++++ > gcc/testsuite/g++.dg/warn/Wuninitialized-14.C | 22 ++ > gcc/testsuite/g++.dg/warn/Wuninitialized-15.C | 118 +++++++++ > gcc/testsuite/g++.dg/warn/Wuninitialized-16.C | 12 + > gcc/testsuite/g++.dg/warn/Wuninitialized-17.C | 146 +++++++++++ > gcc/testsuite/g++.dg/warn/Wuninitialized-18.C | 22 ++ > gcc/testsuite/g++.dg/warn/Wuninitialized-19.C | 50 ++++ > gcc/testsuite/g++.dg/warn/Wuninitialized-20.C | 16 ++ > gcc/testsuite/g++.dg/warn/Wuninitialized-21.C | 20 ++ > gcc/testsuite/g++.dg/warn/Wuninitialized-22.C | 37 +++ > gcc/testsuite/g++.dg/warn/Wuninitialized-23.C | 24 ++ > gcc/testsuite/g++.dg/warn/Wuninitialized-24.C | 14 ++ > gcc/testsuite/g++.dg/warn/Wuninitialized-25.C | 12 + > gcc/testsuite/g++.dg/warn/Wuninitialized-26.C | 22 ++ > gcc/testsuite/g++.dg/warn/Wuninitialized-27.C | 20 ++ > gcc/tree.c | 1 + > 19 files changed, 878 insertions(+), 13 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-12.C > create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-13.C > create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-14.C > create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-15.C > create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-16.C > create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-17.C > create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-18.C > create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-19.C > create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-20.C > create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-21.C > create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-22.C > create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-23.C > create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-24.C > create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-25.C > create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-26.C > create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-27.C > > diff --git a/gcc/cp/init.c b/gcc/cp/init.c > index ffb84ea5b09..92dcdc7bc02 100644 > --- a/gcc/cp/init.c > +++ b/gcc/cp/init.c > @@ -41,7 +41,6 @@ static tree finish_init_stmts (bool, tree, tree); > static void construct_virtual_base (tree, tree); > static void expand_aggr_init_1 (tree, tree, tree, tree, int, tsubst_flags_t); > static void expand_default_init (tree, tree, tree, tree, int, tsubst_flags_t); > -static void perform_member_init (tree, tree); > static int member_init_ok_or_else (tree, tree, tree); > static void expand_virtual_init (tree, tree); > static tree sort_mem_initializers (tree, tree); > @@ -521,19 +520,19 @@ build_value_init_noctor (tree type, tsubst_flags_t complain) > return build_zero_init (type, NULL_TREE, /*static_storage_p=*/false); > } > > -/* Initialize current class with INIT, a TREE_LIST of > - arguments for a target constructor. If TREE_LIST is void_type_node, > - an empty initializer list was given. */ > +/* Initialize current class with INIT, a TREE_LIST of arguments for > + a target constructor. If TREE_LIST is void_type_node, an empty > + initializer list was given. Return the target constructor. */ > > -static void > +static tree > perform_target_ctor (tree init) > { > tree decl = current_class_ref; > tree type = current_class_type; > > - finish_expr_stmt (build_aggr_init (decl, init, > - LOOKUP_NORMAL|LOOKUP_DELEGATING_CONS, > - tf_warning_or_error)); > + init = build_aggr_init (decl, init, LOOKUP_NORMAL|LOOKUP_DELEGATING_CONS, > + tf_warning_or_error); > + finish_expr_stmt (init); > if (type_build_dtor_call (type)) > { > tree expr = build_delete (input_location, > @@ -546,6 +545,7 @@ perform_target_ctor (tree init) > && TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type)) > finish_eh_cleanup (expr); > } > + return init; > } > > /* Return the non-static data initializer for FIELD_DECL MEMBER. */ > @@ -757,12 +757,175 @@ maybe_warn_list_ctor (tree member, tree init) > "of the underlying array", member, begin); > } > > +/* Data structure for find_uninit_fields_r, below. */ > + > +struct find_uninit_data { > + /* The set tracking the yet-uninitialized members. */ > + hash_set<tree> *uninitialized; > + /* The data member we are currently initializing. It can be either > + a type (initializing a base class/delegating constructors), or > + a COMPONENT_REF. */ > + tree member; > + /* A vector to track initialized fields in an initializer-list. */ > + vec<tree, va_gc> *list_inits; > +}; > + > +/* walk_tree callback that warns about using uninitialized data in > + a member-initializer-list. */ > + > +static tree > +find_uninit_fields_r (tree *tp, int *walk_subtrees, void *data) > +{ > + find_uninit_data *d = static_cast<find_uninit_data *>(data); > + hash_set<tree> *uninitialized = d->uninitialized; > + tree init = *tp; > + > + /* No need to look into types. */ > + if (TYPE_P (init)) > + { > + *walk_subtrees = false; > + return NULL_TREE; > + } > + > + switch (TREE_CODE (init)) > + { > + /* If we're just taking the address of an object, it doesn't matter > + if it's been initialized. */ > + case ADDR_EXPR: > + /* Unevaluated operands. */ > + case ALIGNOF_EXPR: > + case SIZEOF_EXPR: > + case NOEXCEPT_EXPR: > + *walk_subtrees = false; > + return NULL_TREE; > + default: > + break; > + } > + > + /* Handle cases like S() : a((b = 42)), c(b) { } where the initializer > + for a surreptitiously initializes b. */ > + bool write_p = false; > + if (TREE_CODE (init) == MODIFY_EXPR) > + { > + /* First recurse on the RHS to detect (b += 10). */ > + cp_walk_tree_without_duplicates (&TREE_OPERAND (init, 1), > + find_uninit_fields_r, data); > + init = TREE_OPERAND (init, 0); > + /* Don't get tricked by a(((c, b) = 42)). */ > + while (TREE_CODE (init) == COMPOUND_EXPR) > + init = TREE_OPERAND (init, 1); > + /* The LHS might be something like this->a.b. Get this->a, so that > + we can remove the member 'a' from the uninitialized set. We treat > + even partial initialization as initializing the whole object. */ > + while (TREE_CODE (init) == COMPONENT_REF) > + if (TREE_CODE (TREE_OPERAND (init, 0)) == COMPONENT_REF) > + init = TREE_OPERAND (init, 0); > + else > + break; > + write_p = true; > + } > + /* Warn about uninitialized 'this'. */ > + else if (TREE_CODE (init) == CALL_EXPR) > + { > + tree fn = get_callee_fndecl (init); > + if (fn && DECL_NONSTATIC_MEMBER_FUNCTION_P (fn)) > + { > + tree op = CALL_EXPR_ARG (init, 0); > + if (TREE_CODE (op) == ADDR_EXPR) > + op = TREE_OPERAND (op, 0); > + temp_override<tree> ovr (d->member, DECL_ARGUMENTS (fn)); > + cp_walk_tree_without_duplicates (&op, find_uninit_fields_r, data); > + } > + } > + else if (BRACE_ENCLOSED_INITIALIZER_P (init)) > + { > + tree ctor = init; > + if (!d->list_inits) > + { > + /* Reshape the outermost { }; this will fill the .index fields. */ > + tree type = TYPE_P (d->member) ? d->member : TREE_TYPE (d->member); > + ctor = reshape_init (type, ctor, tf_none); > + d->list_inits = make_tree_vector (); > + } > + if (TREE_CODE (ctor) == CONSTRUCTOR && CONSTRUCTOR_NELTS (ctor) > 0) > + { > + for (constructor_elt &elt : *CONSTRUCTOR_ELTS (ctor)) > + { > + temp_override<tree> ovr (d->member); > + /* Build up the path to the element we're initializing, > + if we're initializing a class member. */ > + if (TREE_CODE (elt.index) == FIELD_DECL > + && TREE_CODE (d->member) == COMPONENT_REF) > + d->member = build_class_member_access_expr (d->member, > + elt.index, > + NULL_TREE, > + /*ref=*/true, > + tf_none); > + cp_walk_tree_without_duplicates (&elt.value, > + find_uninit_fields_r, data); > + vec_safe_push (d->list_inits, d->member); > + } > + /* We've processed this initializer-list, clean up. */ > + release_tree_vector (d->list_inits); > + d->list_inits = nullptr; > + *walk_subtrees = false; > + return NULL_TREE; > + } > + /* Reshaping might have unwrapped { X } to X; carry on. */ > + } > + > + /* If we find FIELD in the uninitialized set and we're not writing into > + it, we warn. */ > + if (TREE_CODE (init) == COMPONENT_REF) > + { > + unsigned int ix; > + tree t; > + FOR_EACH_VEC_SAFE_ELT (d->list_inits, ix, t) > + if (cp_tree_equal (init, t)) > + { > + /* This field was already initialized by a prior element in the > + initializer-list. */ > + *walk_subtrees = false; > + return NULL_TREE; > + } > + > + tree field = TREE_OPERAND (init, 1); > + tree type = TYPE_P (d->member) ? d->member : TREE_TYPE (d->member); > + > + /* We're initializing a reference member with itself. */ > + if (TYPE_REF_P (type) && cp_tree_equal (d->member, init)) > + warning_at (EXPR_LOCATION (init), OPT_Winit_self, > + "%qD is initialized with itself", field); > + else if (cp_tree_equal (TREE_OPERAND (init, 0), current_class_ref) > + && uninitialized->contains (field)) > + { > + /* If we're not reading but writing into a member in this initializer > + (this is the LHS of an assignment), remember not to warn about this > + member later. */ > + if (write_p) > + uninitialized->remove (field); > + else if (TYPE_REF_P (TREE_TYPE (field))) > + warning_at (EXPR_LOCATION (init), OPT_Wuninitialized, > + "reference %qD is not yet bound to a value when used " > + "here", field); > + else if (!INDIRECT_TYPE_P (type) || is_this_parameter (d->member)) > + warning_at (EXPR_LOCATION (init), OPT_Wuninitialized, > + "field %qD is used uninitialized", field); > + /* We can't clear *walk_subtrees here to handle bogosities such as > + ((c++, b) = 42). */ > + } > + } > + > + return NULL_TREE; > +} > + > /* 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. */ > + list was given; if NULL_TREE no initializer was given. UNINITIALIZED > + is the hash set that tracks uninitialized fields. */ > > static void > -perform_member_init (tree member, tree init) > +perform_member_init (tree member, tree init, hash_set<tree> &uninitialized) > { > tree decl; > tree type = TREE_TYPE (member); > @@ -790,7 +953,9 @@ perform_member_init (tree member, tree init) > if (decl == error_mark_node) > return; > > - if (warn_init_self && init && TREE_CODE (init) == TREE_LIST > + if ((warn_init_self || warn_uninitialized) > + && init > + && TREE_CODE (init) == TREE_LIST > && TREE_CHAIN (init) == NULL_TREE) > { > tree val = TREE_VALUE (init); > @@ -802,6 +967,12 @@ perform_member_init (tree member, tree init) > warning_at (DECL_SOURCE_LOCATION (current_function_decl), > OPT_Winit_self, "%qD is initialized with itself", > member); > + else if (!uninitialized.is_empty ()) > + { > + find_uninit_data data = { &uninitialized, decl, nullptr }; > + cp_walk_tree_without_duplicates (&val, find_uninit_fields_r, > + &data); > + } > } > > if (array_of_unknown_bound_p (type)) > @@ -830,6 +1001,9 @@ perform_member_init (tree member, tree init) > do aggregate-initialization. */ > } > > + /* Assume we are initializing the member. */ > + bool member_initialized_p = true; > + > if (init == void_type_node) > { > /* mem() means value-initialization. */ > @@ -970,6 +1144,9 @@ perform_member_init (tree member, tree init) > diagnose_uninitialized_cst_or_ref_member (core_type, > /*using_new=*/false, > /*complain=*/true); > + > + /* We left the member uninitialized. */ > + member_initialized_p = false; > } > > maybe_warn_list_ctor (member, init); > @@ -980,6 +1157,9 @@ perform_member_init (tree member, tree init) > tf_warning_or_error)); > } > > + if (member_initialized_p && warn_uninitialized) > + uninitialized.remove (member); > + > if (type_build_dtor_call (type)) > { > tree expr; > @@ -1293,13 +1473,31 @@ emit_mem_initializers (tree mem_inits) > if (!COMPLETE_TYPE_P (current_class_type)) > return; > > + /* Keep a set holding fields that are not initialized. */ > + hash_set<tree> uninitialized; > + > + /* Initially that is all of them. */ > + if (warn_uninitialized) > + for (tree field = TYPE_FIELDS (current_class_type); field; > + field = TREE_CHAIN (field)) > + if (TREE_CODE (field) == FIELD_DECL && !DECL_ARTIFICIAL (field)) > + uninitialized.add (field); > + > if (mem_inits > && TYPE_P (TREE_PURPOSE (mem_inits)) > && same_type_p (TREE_PURPOSE (mem_inits), current_class_type)) > { > /* Delegating constructor. */ > gcc_assert (TREE_CHAIN (mem_inits) == NULL_TREE); > - perform_target_ctor (TREE_VALUE (mem_inits)); > + tree ctor = perform_target_ctor (TREE_VALUE (mem_inits)); > + if (!uninitialized.is_empty ()) > + { > + find_uninit_data data = { &uninitialized, current_class_type, > + nullptr }; > + cp_walk_tree_without_duplicates (&ctor, > + find_uninit_fields_r, > + &data); > + } > return; > } > > @@ -1360,6 +1558,15 @@ emit_mem_initializers (tree mem_inits) > flags, > tf_warning_or_error); > expand_cleanup_for_base (subobject, NULL_TREE); > + if (!uninitialized.is_empty () > + && STATEMENT_LIST_TAIL (cur_stmt_list)) > + { > + tree last = STATEMENT_LIST_TAIL (cur_stmt_list)->stmt; > + find_uninit_data data = { &uninitialized, BINFO_TYPE (subobject), > + nullptr }; > + cp_walk_tree_without_duplicates (&last, find_uninit_fields_r, > + &data); > + } > } > else if (!ABSTRACT_CLASS_TYPE_P (current_class_type)) > /* C++14 DR1658 Means we do not have to construct vbases of > @@ -1387,7 +1594,9 @@ emit_mem_initializers (tree mem_inits) > iloc_sentinel ils (EXPR_LOCATION (TREE_TYPE (mem_inits))); > > perform_member_init (TREE_PURPOSE (mem_inits), > - TREE_VALUE (mem_inits)); > + TREE_VALUE (mem_inits), > + uninitialized); > + > mem_inits = TREE_CHAIN (mem_inits); > } > } > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index b3a2c7ce51d..cfd5c7e00a2 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -6688,6 +6688,18 @@ to compute a value that itself is never used, because such > computations may be deleted by data flow analysis before the warnings > are printed. > > +In C++, this warning also warns about using uninitialized objects in > +member-initializer-lists. For example, GCC warns about @code{b} being > +uninitialized in the following snippet: > + > +@smallexample > +struct A @{ > + int a; > + int b; > + A() : a(b) @{ @} > +@}; > +@end smallexample > + > @item -Wno-invalid-memory-model > @opindex Winvalid-memory-model > @opindex Wno-invalid-memory-model > diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-12.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-12.C > new file mode 100644 > index 00000000000..4e51b4b09f2 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-12.C > @@ -0,0 +1,59 @@ > +// PR c++/19808 > +// { dg-do compile { target c++11 } } > +// { dg-options "-Wuninitialized" } > + > +struct S { > + int i, j, k, l; > + S() : i(j), // { dg-warning "field .S::j. is used uninitialized" } > + j(1), > + k(l + 1), // { dg-warning "field .S::l. is used uninitialized" } > + l(2) { } > +}; > + > +struct A { > + int a, b, c; > + A() : a(b // { dg-warning "field .A::b. is used uninitialized" } > + + c) { } // { dg-warning "field .A::c. is used uninitialized" } > +}; > + > +struct B { > + int &r; > + int *p; > + int a; > + B() : r(a), p(&a), a(1) { } > +}; > + > +struct C { > + const int &r1, &r2; > + C () : r1(r2), // { dg-warning "reference .C::r2. is not yet bound to a value when used here" } > + r2(r1) { } > +}; > + > +struct D { > + int a = 1; > + int b = 2; > + D() : a(b + 1), b(a + 1) { } // { dg-warning "field .D::b. is used uninitialized" } > +}; > + > +struct E { > + int a = 1; > + E() : a(a + 1) { } // { dg-warning "field .E::a. is used uninitialized" } > +}; > + > +struct F { > + int a = 1; > + int b; > + F() : b(a + 1) { } > +}; > + > +struct bar { > + bar() {} > + bar(bar&) {} > +}; > + > +class foo { > + bar first; > + bar second; > +public: > + foo() : first(second) {} // { dg-warning "field .foo::second. is used uninitialized" } > +}; > diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-13.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-13.C > new file mode 100644 > index 00000000000..9c631f22fbc > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-13.C > @@ -0,0 +1,49 @@ > +// PR c++/19808 > +// { dg-do compile { target c++11 } } > +// { dg-options "-Wuninitialized -Winit-self" } > + > +int fint(int); > +int fintp(int *); > +int fintr(int &); > +int fintcr(const int &); > + > +int arr[10]; > + > +struct S { > + int x; > + int y; > + const int z = 42; > + int *p; > + > + S(int (*)[1]) : x(x) { } // { dg-warning "initialized with itself" } > + S(int (*)[2]) : x(x + x) { } // { dg-warning "field .S::x. is used uninitialized" } > + S(int (*)[3]) : x(static_cast<int>(y)) { } // { dg-warning "field .S::y. is used uninitialized" } > + S(int (*)[4]) : x(static_cast<int>(x)) { } // { dg-warning "field .S::x. is used uninitialized" } > + S(int (*)[5]) : x(fint(x)) { } // { dg-warning "field .S::x. is used uninitialized" } > + S(int (*)[6]) : x(fint(y)) { } // { dg-warning "field .S::y. is used uninitialized" } > + > + S(int (*)[7]) : x(sizeof(x)) { } > + S(int (*)[8]) : x(sizeof(y)) { } > + S(int (*)[9]) : p(&x) { } > + S(int (*)[10]) : x(fintp(&y)) { } > + S(int (*)[11]) : x(fintr(y)) { } > + S(int (*)[12]) : x(fintcr(y)) { } > + S(int (*)[26]) : x(((void)(__typeof(y)) 1, 1)) { } > + S(int (*)[27]) : x(((void)(decltype(y)) 1, 1)) { } > + S(int (*)[28]) : x(__alignof__(y)) { } > + S(int (*)[29]) : x(noexcept(y)) { } > + > + S(int (*)[13]) : x(0), y(x ? y : y) { } // { dg-warning "field .S::y. is used uninitialized" } > + S(int (*)[14]) : x(0), y(1 + (x ? y : y)) { } // { dg-warning "field .S::y. is used uninitialized" } > + S(int (*)[15]) : x(-y) { } // { dg-warning "field .S::y. is used uninitialized" } > + S(int (*)[16]) : x(1 << y) { } // { dg-warning "field .S::y. is used uninitialized" } > + S(int (*)[17]) : x(this->y) { } // { dg-warning "field .S::y. is used uninitialized" } > + S(int (*)[18]) : x(arr[y]) { } // { dg-warning "field .S::y. is used uninitialized" } > + S(int (*)[19]) : x(0), y(x ? x : y) { } // { dg-warning "field .S::y. is used uninitialized" } > + S(int (*)[20]) : x(0), y(y ? x : y) { } // { dg-warning "field .S::y. is used uninitialized" } > + S(int (*)[21]) : x(0), y(y ? x : x) { } // { dg-warning "field .S::y. is used uninitialized" } > + S(int (*)[22]) : x(0), y((fint(y), x)) { } // { dg-warning "field .S::y. is used uninitialized" } > + S(int (*)[23]) : x(0), y(x += y) { } // { dg-warning "field .S::y. is used uninitialized" } > + S(int (*)[24]) : x(y += 10) { } // { dg-warning "field .S::y. is used uninitialized" } > + S(int (*)[25]) : x(y++) { } // { dg-warning "field .S::y. is used uninitialized" } > +}; > diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-14.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-14.C > new file mode 100644 > index 00000000000..6bef85e4659 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-14.C > @@ -0,0 +1,22 @@ > +// PR c++/19808 > +// { dg-do compile } > +// { dg-options "-Wuninitialized" } > + > +struct A { > + int m; > + int get() const { return m; } > + > + A() { } > + A(int) { } > + A(const A &) { } > + A(A *) { } > +}; > + > +struct S { > + A a, b; > + > + S(int (*)[1]) : a() {} > + S(int (*)[2]) : b(a.get()) {} > + S(int (*)[3]) : b(a) {} > + S(int (*)[4]) : a(&a) {} > +}; > diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-15.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-15.C > new file mode 100644 > index 00000000000..ed62758a984 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-15.C > @@ -0,0 +1,118 @@ > +// PR c++/19808 > +// { dg-do compile { target c++11 } } > +// { dg-options "-Wuninitialized -Winit-self" } > +// Largely copied from clang's test/SemaCXX/uninitialized.cpp. > + > +int x; > +struct U { > + U() : b(a) { } > + int &a = x; > + int &b; > +}; > + > +struct T { > + T() : a(b), b(a) { } // { dg-warning "reference .T::b. is not yet bound" } > + int &a, &b; > +}; > + > +struct S { > + S() : a(a) { } // { dg-warning ".S::a. is initialized with itself" } > + int &a; > +}; > + > +struct A { > + int a; > + int b; > + A() { } > + A(int (*)[1]) : b(a) { } // { dg-warning ".A::a. is used uninitialized" } > + A(int (*)[2]) : a(b) { } // { dg-warning ".A::b. is used uninitialized" } > +}; > + > +struct D { > + int a; > + int &b; > + int &c = a; > + int d = b; > + D() : b(a) { } > +}; > + > +struct E { > + int a; > + int get(); > + static int num(); > + E() { } > + E(int) { } > +}; > + > +struct F { > + int a; > + E e; > + int b; > + F(int (*)[1]) : a(e.get()) { } // { dg-warning "field .F::e. is used uninitialized" } > + F(int (*)[2]) : a(e.num()) { } > + F(int (*)[3]) : e(a) { } // { dg-warning "field .F::a. is used uninitialized" } > + F(int (*)[4]) : a(4), e(a) { } > + F(int (*)[5]) : e(b) { } // { dg-warning "field .F::b. is used uninitialized" } > + F(int (*)[6]) : e(b), b(4) { } // { dg-warning "field .F::b. is used uninitialized" } > +}; > + > +struct G { > + G(const A&) { }; > +}; > + > +struct H { > + A a1; > + G g; > + A a2; > + H() : g(a1) { } > + // ??? clang++ doesn't warn here > + H(int) : g(a2) { } // { dg-warning "field .H::a2. is used uninitialized" } > +}; > + > +struct I { > + I(int *) { } > +}; > + > +struct J : I { > + int *a; > + int *b; > + int c; > + J() : I((a = new int(5))), b(a), c(*a) { } > +}; > + > +struct M { }; > + > +struct N : public M { > + int a; > + int b; > + N() : b(a) { } // { dg-warning "field .N::a. is used uninitialized" } > +}; > + > +struct O { > + int x = 42; > + int get() { return x; } > +}; > + > +struct P { > + O o; > + int x = o.get(); > + P() : x(o.get()) { } > +}; > + > +struct Q { > + int a; > + int b; > + int &c; > + Q() : > + a(c = 5), // { dg-warning "reference .Q::c. is not yet bound" } > + b(c), // { dg-warning "reference .Q::c. is not yet bound" } > + c(a) { } > +}; > + > +struct R { > + int a; > + int b; > + int c; > + int d = a + b + c; > + R() : a(c = 5), b(c), c(a) { } > +}; > diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-16.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-16.C > new file mode 100644 > index 00000000000..bb47a340f18 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-16.C > @@ -0,0 +1,12 @@ > +// PR c++/19808 > +// { dg-do compile { target c++11 } } > +// { dg-options "-Wuninitialized" } > + > +struct S { > + int a; > + int b; > + int c; > + S() : a((b = 42)), c(b) { } > + S(int) : a(((1, b) = 42)), c(b) { } > + S(char) : a(((c++, b) = 42)), c(b) { } // { dg-warning "field .S::c. is used uninitialized" } > +}; > diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-17.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-17.C > new file mode 100644 > index 00000000000..f18ed9e2251 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-17.C > @@ -0,0 +1,146 @@ > +// PR c++/19808 > +// { dg-do compile { target c++11 } } > +// { dg-options "-Wuninitialized -Winit-self" } > +// Test we warn with initializer-lists. > + > +int num = 5; > + > +struct A { > + int a; > + int b; > +}; > + > +struct B { > + A a1; > + A a2; > +}; > + > +struct S { > + A a; > + int x; > + int y; > + S() : a{1, y} { } // { dg-warning "field .S::y. is used uninitialized" } > +}; > + > +struct R { > + A a1; > + A a2; > + A a3; > + A a4; > + R() : > + a1{1, 2}, > + a2{a2.a + 2}, // { dg-warning "field .R::a2. is used uninitialized" } > + a3{a3.a + 2}, // { dg-warning "field .R::a3. is used uninitialized" } > + a4{a4.b + 2} // { dg-warning "field .R::a4. is used uninitialized" } > + { } > +}; > + > +struct Q { > + B b1; > + B b2; > + B b3; > + B b4; > + B b5; > + B b6; > + B b7; > + B b8; > + B b9; > + B b10; > + B b11; > + B b12; > + B b13; > + B b14; > + B b15; > + B b16; > + B b17; > + B b18; > + B b19; > + B b20; > + B b21; > + B b22; > + Q() : > + b1{ {}, {} }, > + b2{ {}, b2.a1 }, > + b3{ b3.a1 }, // { dg-warning "field .Q::b3. is used uninitialized" } > + b4{ {}, b4.a2}, // { dg-warning "field .Q::b4. is used uninitialized" } > + b5{ b5.a2 }, // { dg-warning "field .Q::b5. is used uninitialized" } > + b6{ {b6.a1.a} }, // { dg-warning "field .Q::b6. is used uninitialized" } > + b7{ {0, b7.a1.a} }, > + b8{ {}, {b8.a1.a} }, > + b9{ {}, {0, b9.a1.a} }, > + b10{ {b10.a1.b} }, // { dg-warning "field .Q::b10. is used uninitialized" } > + b11{ {0, b11.a1.b} }, // { dg-warning "field .Q::b11. is used uninitialized" } > + b12{ {}, {b12.a1.b} }, > + b13{ {}, {0, b13.a1.b} }, > + b14{ {b14.a2.a} }, // { dg-warning "field .Q::b14. is used uninitialized" } > + b15{ {0, b15.a2.a} }, // { dg-warning "field .Q::b15. is used uninitialized" } > + b16{ {}, {b16.a2.a} }, // { dg-warning "field .Q::b16. is used uninitialized" } > + b17{ {}, {0, b17.a2.a} }, > + b18{ {b18.a2.b} }, // { dg-warning "field .Q::b18. is used uninitialized" } > + b19{ {0, b19.a2.b} }, // { dg-warning "field .Q::b19. is used uninitialized" } > + b20{ {}, {b20.a2.b} }, // { dg-warning "field .Q::b20. is used uninitialized" } > + b21{ {}, {0, b21.a2.b} }, // { dg-warning "field .Q::b21. is used uninitialized" } > + b22{ {b18.a2.b + 5} } > + { } > +}; > + > +struct C { > + int a; > + int &b; > + int c; > +}; > + > +struct P { > + C c1; > + C c2; > + C c3; > + C c4; > + C c5; > + C c6; > + C c7; > + P() : > + c1{ 0, num, 0 }, > + c2{ 1, num, c2.b }, > + c3{ c3.b, num }, // { dg-warning "field .P::c3. is used uninitialized" } > + c4{ 0, c4.b, 0 }, // { dg-warning ".C::b. is initialized with itself" } > + /* c5.c is binding to a reference, don't warn. */ > + c5{ 0, c5.c, 0 }, > + c6{ c6.b, num, 0 }, // { dg-warning "field .P::c6. is used uninitialized" } > + c7{ 0, c7.a, 0 } > + { } > +}; > + > +struct D { > + int &a; > + int &b; > +}; > + > +struct M { > + D d1; > + D d2; > + D d3; > + M() : > + d1{ num, num }, > + d2{ num, d2.a }, > + // Not a FE warning. > + d3{ d3.b, num } // { dg-warning "is used uninitialized" } > + { } > +}; > + > +M m; > + > +struct E { > + E(); > + E foo(); > + E* operator->(); > +}; > + > +struct F { > + F(E); > +}; > + > +struct G { > + F f; > + E e; > + G() : f{ e->foo() }, e() { } // { dg-warning "field .G::e. is used uninitialized" } > +}; > diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-18.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-18.C > new file mode 100644 > index 00000000000..c05ad42f95e > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-18.C > @@ -0,0 +1,22 @@ > +// PR c++/96121 > +// { dg-do compile { target c++11 } } > +// { dg-options "-Wuninitialized" } > + > +struct A { > + A(); > + int i; > +}; > +struct B { > + B(A); > + int i; > +}; > + > +struct composed2 { > + B b_; > + A a_; > + composed2() : b_(a_) {} // { dg-warning "field .composed2::a_. is used uninitialized" } > +}; > + > +composed2 test() { > + return composed2{}; > +} > diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-19.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-19.C > new file mode 100644 > index 00000000000..c401f8636bf > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-19.C > @@ -0,0 +1,50 @@ > +// PR c++/19808 > +// { dg-do compile { target c++11 } } > +// { dg-options "-Wuninitialized" } > +// Test we warn when initializing a base class. > + > +struct A { > + A(int) { } > +}; > + > +struct B : public A { > + int x; > + B() : A(x) { } // { dg-warning "field .B::x. is used uninitialized" } > +}; > + > +struct C : public A { > + int x; > + int y; > + C() : A(y = 4), x(y) { } > +}; > + > +struct D : public A { > + int x; > + D() : A{x} { } // { dg-warning "field .D::x. is used uninitialized" } > +}; > + > +struct E : public A { > + int x; > + int y; > + E() : A{y = 4}, x(y) { } > +}; > + > +struct F { > + F(int&) { } > +}; > + > +struct G : F { > + int x; > + G() : F(x) { } > +}; > + > +struct H { > + H(int *) { } > +}; > + > +struct I : H { > + int x; > + int arr[2]; > + I() : H(&x) { } > + I(int) : H(arr) { } > +}; > diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-20.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-20.C > new file mode 100644 > index 00000000000..9f367f0fdfd > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-20.C > @@ -0,0 +1,16 @@ > +// PR c++/96121 > +// { dg-do compile { target c++11 } } > +// { dg-options "-Wuninitialized" } > +// Test we warn with delegating constructors. > + > +struct A { > + A(int); > + A(int &, int); > + A(int (*)[1]) : A(x) { } // { dg-warning "21:field .A::x. is used uninitialized" } > + A(int (*)[2]) : A(x, x) { } // { dg-warning "24:field .A::x. is used uninitialized" } > + A(int (*)[3]) : A(x, 0) { } > + A(int (*)[4]) : A{x} { } // { dg-warning "21:field .A::x. is used uninitialized" } > + A(int (*)[5]) : A{x, x} { } // { dg-warning "24:field .A::x. is used uninitialized" } > + A(int (*)[6]) : A{x, 0} { } > + int x; > +}; > diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-21.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-21.C > new file mode 100644 > index 00000000000..57ca00ab042 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-21.C > @@ -0,0 +1,20 @@ > +// PR c++/19808 > +// { dg-do compile } > +// { dg-options "-Wuninitialized" } > + > +struct A { > + int a; > + int b; > + A(int) {} > +}; > + > +struct S { > + A a; > + A a2; > + S() : > + /* We don't warn here, because we consider partial initialization > + as initializing the whole object. */ > + a((a2.a = 42)), > + a2(a2.a) > + { } > +}; > diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-22.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-22.C > new file mode 100644 > index 00000000000..bd980d61118 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-22.C > @@ -0,0 +1,37 @@ > +// PR c++/19808 > +// { dg-do compile { target c++11 } } > +// { dg-options "-Wuninitialized -Winit-self" } > +// Test that we don't warn when initializing a reference, unless it's > +// self-init. > + > +struct R { > + int &r; > +}; > + > +struct S { > + R r; > + int a; > + int &b; > + int c; > +}; > + > +struct X { > + S s; > + X() : s{ { s.a }, 1, s.c, 3} { } > +}; > + > +struct A { > + int &r; > + A() : r{r} { } // { dg-warning ".A::r. is initialized with itself" } > +}; > + > +struct B { > + int &r; > + int a; > + B() : r{a} { } > +}; > + > +struct C { > + R x; > + C() : x{x.r} { } // { dg-warning ".R::r. is initialized with itself" } > +}; > diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-23.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-23.C > new file mode 100644 > index 00000000000..db3778300be > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-23.C > @@ -0,0 +1,24 @@ > +// PR c++/19808 > +// { dg-do compile { target c++11 } } > +// { dg-options "-Wuninitialized" } > +// Test that we don't warn in an uninstantiated template. > + > +struct A { > + int *fn() { return nullptr; } > +}; > + > +template<typename T> > +struct B { > + B() : p(a->fn()) { } > + A *a; > + int *p; > +}; > + > +template<typename T> > +struct C { > + C() : p(a->fn()) { } // { dg-warning "field .C<int>::a. is used uninitialized" } > + A *a; > + int *p; > +}; > + > +C<int> c; > diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-24.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-24.C > new file mode 100644 > index 00000000000..c0a88631fb4 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-24.C > @@ -0,0 +1,14 @@ > +// PR c++/19808 > +// { dg-do compile { target c++11 } } > +// { dg-options "-Wuninitialized" } > + > +struct S { int x, y; }; > + > +struct A { > + S a1[2]; > + int i; > + A(int (*)[1]) : a1{{1}} { } > + A(int (*)[2]) : a1{ {1, 2} } { } > + A(int (*)[3]) : a1{{i}} { } // { dg-warning "field .A::i. is used uninitialized" } > + A(int (*)[4]) : a1{ {1, i} } { } // { dg-warning "field .A::i. is used uninitialized" } > +}; > diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-25.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-25.C > new file mode 100644 > index 00000000000..fb652f989a4 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-25.C > @@ -0,0 +1,12 @@ > +// PR c++/19808 > +// { dg-do compile { target c++11 } } > +// { dg-options "-Wall" } > + > +struct A { > + A *a; > +}; > + > +struct B : A { > + int i; > + B() : A{a} {} > +}; > diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-26.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-26.C > new file mode 100644 > index 00000000000..a887d12e9f9 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-26.C > @@ -0,0 +1,22 @@ > +// PR c++/19808 > +// { dg-do compile { target c++11 } } > +// { dg-options "-Wuninitialized" } > +// Anonymous union/struct. > +// ??? The diagnostic should be improved to say 'b' instead of > +// "<anonymous>". > + > +struct S { > + __extension__ struct { > + int a; > + int b; > + }; > + S() : a(b) { } // { dg-warning "is used uninitialized" } > +}; > + > +struct U { > + union { > + int a; > + int b; > + }; > + U() : a(b) { } // { dg-warning "is used uninitialized" } > +}; > diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-27.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-27.C > new file mode 100644 > index 00000000000..24e6b9b5b48 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-27.C > @@ -0,0 +1,20 @@ > +// PR c++/19808 > +// { dg-do compile } > +// { dg-options "-Wall" } > + > +enum E { red }; > + > +struct C { > + C(int *, unsigned); > +}; > + > +template <unsigned U> struct D : C { > + D(int, int, E) : C(e, U) {} > + int e[2]; > +}; > + > +void > +g () > +{ > + D<1>(0, 0, red); > +} > diff --git a/gcc/tree.c b/gcc/tree.c > index 1ad4ad5a5f7..1ad884120b0 100644 > --- a/gcc/tree.c > +++ b/gcc/tree.c > @@ -4638,6 +4638,7 @@ stabilize_reference (tree ref) > TREE_READONLY (result) = TREE_READONLY (ref); > TREE_SIDE_EFFECTS (result) = TREE_SIDE_EFFECTS (ref); > TREE_THIS_VOLATILE (result) = TREE_THIS_VOLATILE (ref); > + protected_set_expr_location (result, EXPR_LOCATION (ref)); > > return result; > } > > base-commit: 6403e02510fe84c72c515247a9dfdb6e05d977fe >
On 11/16/20 12:42 PM, Martin Sebor wrote: > On 11/15/20 3:44 PM, Marek Polacek via Gcc-patches wrote: >> This patch implements the long-desired -Wuninitialized warning for >> member initializer lists, so that the front end can detect bugs like >> >> struct A { >> int a; >> int b; >> A() : b(1), a(b) { } >> }; >> >> where the field 'b' is used uninitialized because the order of member >> initializers in the member initializer list is irrelevant; what matters >> is the order of declarations in the class definition. >> >> I've implemented this by keeping a hash set holding fields that are not >> initialized yet, so at first it will be {a, b}, and after initializing >> 'a' it will be {b} and so on. Then I use walk_tree to walk the >> initializer and if we see that an uninitialized object is used, we warn. >> Of course, when we use the address of the object, we may not warn: >> >> struct B { >> int &r; >> int *p; >> int a; >> B() : r(a), p(&a), a(1) { } // ok >> }; >> >> Likewise, don't warn in unevaluated contexts such as sizeof. Classes >> without an explicit initializer may still be initialized by their >> default constructors; whether or not something is considered initialized >> is handled in perform_member_init, see member_initialized_p. It is >> possible that we might want to tweak that if the current approach turns >> out to be flawed. >> >> And this is where things stop being simple. I had to implement certain >> gymnastics to handle cases like >> >> struct S { >> int a, b, c; >> S() : a((b = 42)), c(b) { } >> }; >> >> where the initializer for 'a' surreptitiously initializes 'b'. And since >> 'b' isn't read in the assignment, we can't warn that it's being used >> uninitialized. >> >> Also, initializer lists. Consider: >> >> struct A { >> int x, y, z; >> }; >> >> struct S { >> A a; >> S() : a{ 1, 2, a.y } {} // #1 >> S(int) : a{ a.y, 2, 3 } {} // #2 >> }; >> >> In #1, a.y refers to an initialized element, so we oughtn't warn. But >> in #2, a.y refers to an uninitialized element, so we should warn. To >> that end I've added a vector that tracks which elements of an initializer >> list are already initialized, so that when we encounter them later in >> the list, we don't warn. This is the find_uninit_data->list_inits >> thing. It will contain paths to the initialized elements, so e.g. >> >> ((S *)this)->a.y >> >> These are edge cases, but some developers live on the edge. >> >> This patch also handles delegating constructors and initializing base >> classes. >> >> A couple of TODOs: >> - the same approach could be used to warn for NSDMIs, but I think it's >> out >> of scope for this patch; >> - the diagnostic for anonymous unions/structs should be improved >> to say the name of the member instead of "<anonymous>"; >> - certain uninitialized warnings in initializer-lists are issued >> by the FE as well as the ME. It's unclear to me what we want to do >> here; >> - using uninitialized members of base classes is not implemented yet. > > This is a great enhancement! Detecting the misues even in inline > functions is especially useful. Why doesn't the middle-end warning work for inline functions? > I installed the patch to see how > it handles some of the cases the middle end was recently enhanced > to detect. Below is a test case I tried and the outupt. Clearly, > the middle end warning doesn't work as intended for C++ ctors, so > that's something for me to look into. Please. If we can handle these warnings in the middle-end (where we have data flow info), that should be more reliable than the front-end walking the trees. > It would be good to enhance the front end warning to use the same > assumptions as the middle end (i.e., that arguments passed by > reference to const-qualified parameters are read from). This is > also the basis for warning on the call to strlen in the test case. > That way both could detect the same problems (the front end even > in inline functions). > > There's a duplicate warning for the first ctor that we might want > to try to avoid, probably by tweaking the middle end one. > > Martin > > $ cat a.C && gcc -O2 -S -Wall a.C > int f (int); > int g (const int&); > int h (const int*); > > struct A { > A (int(*)[1]); > A (int(*)[2]); > A (int(*)[3]); > A (int(*)[4]); > > int a, b; > char s[32]; > }; > > A::A (int(*)[1]): a {f (b)} { } // duplicate warning > A::A (int(*)[2]): a {g (b)} { } // missing warning > A::A (int(*)[3]): a {h (&b)} { } // missing warning > A::A (int(*)[4]): a (__builtin_strlen (s)) { } > > void ff (void) > { > int a; > f (a); // warning > } > > void gg (void) > { > int a; > g (a); // warning > } > > void hh (void) > { > int a; > h (&a); // warning > } > > int ii (void) > { > char s[32]; > return __builtin_strlen (s); // warning > } > > a.C: In constructor ‘A::A(int (*)[1])’: > a.C:15:25: warning: field ‘A::b’ is used uninitialized [-Wuninitialized] > 15 | A::A (int(*)[1]): a {f (b)} { } // duplicate warning > | ^ > a.C: In constructor ‘A::A(int (*)[1])’: > a.C:15:24: warning: ‘*<unknown>.A::b’ is used uninitialized > [-Wuninitialized] > 15 | A::A (int(*)[1]): a {f (b)} { } // duplicate warning > | ~~^~~ > a.C: In function ‘void ff()’: > a.C:23:5: warning: ‘a’ is used uninitialized [-Wuninitialized] > 23 | f (a); // warning > | ~~^~~ > a.C: In function ‘void gg()’: > a.C:29:5: warning: ‘a’ may be used uninitialized [-Wmaybe-uninitialized] > 29 | g (a); // warning > | ~~^~~ > a.C:2:5: note: by argument 1 of type ‘const int&’ to ‘int g(const int&)’ > declared here > 2 | int g (const int&); > | ^ > a.C:28:7: note: ‘a’ declared here > 28 | int a; > | ^ > a.C: In function ‘void hh()’: > a.C:35:5: warning: ‘a’ may be used uninitialized [-Wmaybe-uninitialized] > 35 | h (&a); // warning > | ~~^~~~ > a.C:3:5: note: by argument 1 of type ‘const int*’ to ‘int h(const int*)’ > declared here > 3 | int h (const int*); > | ^ > a.C:34:7: note: ‘a’ declared here > 34 | int a; > | ^ > a.C: In function ‘int ii()’: > a.C:41:27: warning: ‘s’ is used uninitialized [-Wuninitialized] > 41 | return __builtin_strlen (s); // warning > | ~~~~~~~~~~~~~~~~~^~~ > <built-in>: note: by argument 1 of type ‘const char*’ to ‘long unsigned > int __builtin_strlen(const char*)’ declared here > a.C:40:8: note: ‘s’ declared here > 40 | char s[32]; > | ^ > >> >> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? >> >> gcc/cp/ChangeLog: >> >> PR c++/19808 >> PR c++/96121 >> * init.c (perform_member_init): Remove a forward declaration. >> Walk the initializer using find_uninit_fields_r. New parameter >> to track uninitialized fields. If a member is initialized, >> remove it from the hash set. >> (perform_target_ctor): Return the initializer. >> (struct find_uninit_data): New class. >> (find_uninit_fields_r): New function. >> (emit_mem_initializers): Keep and initialize a set holding fields >> that are not initialized. When handling delegating constructors, >> walk the constructor tree using find_uninit_fields_r. Also when >> initializing base clases. Pass uninitialized down to >> perform_member_init. >> >> gcc/ChangeLog: >> >> PR c++/19808 >> PR c++/96121 >> * doc/invoke.texi: Update documentation for -Wuninitialized. >> * tree.c (stabilize_reference): Set location. >> >> gcc/testsuite/ChangeLog: >> >> PR c++/19808 >> PR c++/96121 >> * g++.dg/warn/Wuninitialized-12.C: New test. >> * g++.dg/warn/Wuninitialized-13.C: New test. >> * g++.dg/warn/Wuninitialized-14.C: New test. >> * g++.dg/warn/Wuninitialized-15.C: New test. >> * g++.dg/warn/Wuninitialized-16.C: New test. >> * g++.dg/warn/Wuninitialized-17.C: New test. >> * g++.dg/warn/Wuninitialized-18.C: New test. >> * g++.dg/warn/Wuninitialized-19.C: New test. >> * g++.dg/warn/Wuninitialized-20.C: New test. >> * g++.dg/warn/Wuninitialized-21.C: New test. >> * g++.dg/warn/Wuninitialized-22.C: New test. >> * g++.dg/warn/Wuninitialized-23.C: New test. >> * g++.dg/warn/Wuninitialized-24.C: New test. >> * g++.dg/warn/Wuninitialized-25.C: New test. >> * g++.dg/warn/Wuninitialized-26.C: New test. >> * g++.dg/warn/Wuninitialized-27.C: New test. >> --- >> gcc/cp/init.c | 235 +++++++++++++++++- >> gcc/doc/invoke.texi | 12 + >> gcc/testsuite/g++.dg/warn/Wuninitialized-12.C | 59 +++++ >> gcc/testsuite/g++.dg/warn/Wuninitialized-13.C | 49 ++++ >> gcc/testsuite/g++.dg/warn/Wuninitialized-14.C | 22 ++ >> gcc/testsuite/g++.dg/warn/Wuninitialized-15.C | 118 +++++++++ >> gcc/testsuite/g++.dg/warn/Wuninitialized-16.C | 12 + >> gcc/testsuite/g++.dg/warn/Wuninitialized-17.C | 146 +++++++++++ >> gcc/testsuite/g++.dg/warn/Wuninitialized-18.C | 22 ++ >> gcc/testsuite/g++.dg/warn/Wuninitialized-19.C | 50 ++++ >> gcc/testsuite/g++.dg/warn/Wuninitialized-20.C | 16 ++ >> gcc/testsuite/g++.dg/warn/Wuninitialized-21.C | 20 ++ >> gcc/testsuite/g++.dg/warn/Wuninitialized-22.C | 37 +++ >> gcc/testsuite/g++.dg/warn/Wuninitialized-23.C | 24 ++ >> gcc/testsuite/g++.dg/warn/Wuninitialized-24.C | 14 ++ >> gcc/testsuite/g++.dg/warn/Wuninitialized-25.C | 12 + >> gcc/testsuite/g++.dg/warn/Wuninitialized-26.C | 22 ++ >> gcc/testsuite/g++.dg/warn/Wuninitialized-27.C | 20 ++ >> gcc/tree.c | 1 + >> 19 files changed, 878 insertions(+), 13 deletions(-) >> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-12.C >> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-13.C >> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-14.C >> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-15.C >> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-16.C >> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-17.C >> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-18.C >> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-19.C >> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-20.C >> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-21.C >> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-22.C >> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-23.C >> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-24.C >> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-25.C >> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-26.C >> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-27.C >> >> diff --git a/gcc/cp/init.c b/gcc/cp/init.c >> index ffb84ea5b09..92dcdc7bc02 100644 >> --- a/gcc/cp/init.c >> +++ b/gcc/cp/init.c >> @@ -41,7 +41,6 @@ static tree finish_init_stmts (bool, tree, tree); >> static void construct_virtual_base (tree, tree); >> static void expand_aggr_init_1 (tree, tree, tree, tree, int, >> tsubst_flags_t); >> static void expand_default_init (tree, tree, tree, tree, int, >> tsubst_flags_t); >> -static void perform_member_init (tree, tree); >> static int member_init_ok_or_else (tree, tree, tree); >> static void expand_virtual_init (tree, tree); >> static tree sort_mem_initializers (tree, tree); >> @@ -521,19 +520,19 @@ build_value_init_noctor (tree type, >> tsubst_flags_t complain) >> return build_zero_init (type, NULL_TREE, /*static_storage_p=*/false); >> } >> -/* Initialize current class with INIT, a TREE_LIST of >> - arguments for a target constructor. If TREE_LIST is void_type_node, >> - an empty initializer list was given. */ >> +/* Initialize current class with INIT, a TREE_LIST of arguments for >> + a target constructor. If TREE_LIST is void_type_node, an empty >> + initializer list was given. Return the target constructor. */ >> -static void >> +static tree >> perform_target_ctor (tree init) >> { >> tree decl = current_class_ref; >> tree type = current_class_type; >> - finish_expr_stmt (build_aggr_init (decl, init, >> - LOOKUP_NORMAL|LOOKUP_DELEGATING_CONS, >> - tf_warning_or_error)); >> + init = build_aggr_init (decl, init, >> LOOKUP_NORMAL|LOOKUP_DELEGATING_CONS, >> + tf_warning_or_error); >> + finish_expr_stmt (init); >> if (type_build_dtor_call (type)) >> { >> tree expr = build_delete (input_location, >> @@ -546,6 +545,7 @@ perform_target_ctor (tree init) >> && TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type)) >> finish_eh_cleanup (expr); >> } >> + return init; >> } >> /* Return the non-static data initializer for FIELD_DECL MEMBER. */ >> @@ -757,12 +757,175 @@ maybe_warn_list_ctor (tree member, tree init) >> "of the underlying array", member, begin); >> } >> +/* Data structure for find_uninit_fields_r, below. */ >> + >> +struct find_uninit_data { >> + /* The set tracking the yet-uninitialized members. */ >> + hash_set<tree> *uninitialized; >> + /* The data member we are currently initializing. It can be either >> + a type (initializing a base class/delegating constructors), or >> + a COMPONENT_REF. */ >> + tree member; >> + /* A vector to track initialized fields in an initializer-list. */ >> + vec<tree, va_gc> *list_inits; >> +}; >> + >> +/* walk_tree callback that warns about using uninitialized data in >> + a member-initializer-list. */ >> + >> +static tree >> +find_uninit_fields_r (tree *tp, int *walk_subtrees, void *data) >> +{ >> + find_uninit_data *d = static_cast<find_uninit_data *>(data); >> + hash_set<tree> *uninitialized = d->uninitialized; >> + tree init = *tp; >> + >> + /* No need to look into types. */ >> + if (TYPE_P (init)) >> + { >> + *walk_subtrees = false; >> + return NULL_TREE; >> + } >> + >> + switch (TREE_CODE (init)) >> + { >> + /* If we're just taking the address of an object, it doesn't matter >> + if it's been initialized. */ >> + case ADDR_EXPR: >> + /* Unevaluated operands. */ >> + case ALIGNOF_EXPR: >> + case SIZEOF_EXPR: >> + case NOEXCEPT_EXPR: >> + *walk_subtrees = false; >> + return NULL_TREE; >> + default: >> + break; >> + } >> + >> + /* Handle cases like S() : a((b = 42)), c(b) { } where the initializer >> + for a surreptitiously initializes b. */ >> + bool write_p = false; >> + if (TREE_CODE (init) == MODIFY_EXPR) >> + { >> + /* First recurse on the RHS to detect (b += 10). */ >> + cp_walk_tree_without_duplicates (&TREE_OPERAND (init, 1), >> + find_uninit_fields_r, data); >> + init = TREE_OPERAND (init, 0); >> + /* Don't get tricked by a(((c, b) = 42)). */ >> + while (TREE_CODE (init) == COMPOUND_EXPR) >> + init = TREE_OPERAND (init, 1); >> + /* The LHS might be something like this->a.b. Get this->a, so >> that >> + we can remove the member 'a' from the uninitialized set. We treat >> + even partial initialization as initializing the whole object. */ >> + while (TREE_CODE (init) == COMPONENT_REF) >> + if (TREE_CODE (TREE_OPERAND (init, 0)) == COMPONENT_REF) >> + init = TREE_OPERAND (init, 0); >> + else >> + break; >> + write_p = true; >> + } >> + /* Warn about uninitialized 'this'. */ >> + else if (TREE_CODE (init) == CALL_EXPR) >> + { >> + tree fn = get_callee_fndecl (init); >> + if (fn && DECL_NONSTATIC_MEMBER_FUNCTION_P (fn)) >> + { >> + tree op = CALL_EXPR_ARG (init, 0); >> + if (TREE_CODE (op) == ADDR_EXPR) >> + op = TREE_OPERAND (op, 0); >> + temp_override<tree> ovr (d->member, DECL_ARGUMENTS (fn)); >> + cp_walk_tree_without_duplicates (&op, find_uninit_fields_r, data); >> + } >> + } >> + else if (BRACE_ENCLOSED_INITIALIZER_P (init)) >> + { >> + tree ctor = init; >> + if (!d->list_inits) >> + { >> + /* Reshape the outermost { }; this will fill the .index >> fields. */ >> + tree type = TYPE_P (d->member) ? d->member : TREE_TYPE >> (d->member); >> + ctor = reshape_init (type, ctor, tf_none); >> + d->list_inits = make_tree_vector (); >> + } >> + if (TREE_CODE (ctor) == CONSTRUCTOR && CONSTRUCTOR_NELTS (ctor) >> > 0) >> + { >> + for (constructor_elt &elt : *CONSTRUCTOR_ELTS (ctor)) >> + { >> + temp_override<tree> ovr (d->member); >> + /* Build up the path to the element we're initializing, >> + if we're initializing a class member. */ >> + if (TREE_CODE (elt.index) == FIELD_DECL >> + && TREE_CODE (d->member) == COMPONENT_REF) >> + d->member = build_class_member_access_expr (d->member, >> + elt.index, >> + NULL_TREE, >> + /*ref=*/true, >> + tf_none); >> + cp_walk_tree_without_duplicates (&elt.value, >> + find_uninit_fields_r, data); >> + vec_safe_push (d->list_inits, d->member); >> + } >> + /* We've processed this initializer-list, clean up. */ >> + release_tree_vector (d->list_inits); >> + d->list_inits = nullptr; >> + *walk_subtrees = false; >> + return NULL_TREE; >> + } >> + /* Reshaping might have unwrapped { X } to X; carry on. */ >> + } >> + >> + /* If we find FIELD in the uninitialized set and we're not writing >> into >> + it, we warn. */ >> + if (TREE_CODE (init) == COMPONENT_REF) >> + { >> + unsigned int ix; >> + tree t; >> + FOR_EACH_VEC_SAFE_ELT (d->list_inits, ix, t) >> + if (cp_tree_equal (init, t)) >> + { >> + /* This field was already initialized by a prior element in the >> + initializer-list. */ >> + *walk_subtrees = false; >> + return NULL_TREE; >> + } >> + >> + tree field = TREE_OPERAND (init, 1); >> + tree type = TYPE_P (d->member) ? d->member : TREE_TYPE >> (d->member); >> + >> + /* We're initializing a reference member with itself. */ >> + if (TYPE_REF_P (type) && cp_tree_equal (d->member, init)) >> + warning_at (EXPR_LOCATION (init), OPT_Winit_self, >> + "%qD is initialized with itself", field); >> + else if (cp_tree_equal (TREE_OPERAND (init, 0), current_class_ref) >> + && uninitialized->contains (field)) >> + { >> + /* If we're not reading but writing into a member in this >> initializer >> + (this is the LHS of an assignment), remember not to warn >> about this >> + member later. */ >> + if (write_p) >> + uninitialized->remove (field); >> + else if (TYPE_REF_P (TREE_TYPE (field))) >> + warning_at (EXPR_LOCATION (init), OPT_Wuninitialized, >> + "reference %qD is not yet bound to a value when used " >> + "here", field); >> + else if (!INDIRECT_TYPE_P (type) || is_this_parameter (d->member)) >> + warning_at (EXPR_LOCATION (init), OPT_Wuninitialized, >> + "field %qD is used uninitialized", field); >> + /* We can't clear *walk_subtrees here to handle bogosities such as >> + ((c++, b) = 42). */ >> + } >> + } >> + >> + return NULL_TREE; >> +} >> + >> /* 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. */ >> + list was given; if NULL_TREE no initializer was given. UNINITIALIZED >> + is the hash set that tracks uninitialized fields. */ >> static void >> -perform_member_init (tree member, tree init) >> +perform_member_init (tree member, tree init, hash_set<tree> >> &uninitialized) >> { >> tree decl; >> tree type = TREE_TYPE (member); >> @@ -790,7 +953,9 @@ perform_member_init (tree member, tree init) >> if (decl == error_mark_node) >> return; >> - if (warn_init_self && init && TREE_CODE (init) == TREE_LIST >> + if ((warn_init_self || warn_uninitialized) >> + && init >> + && TREE_CODE (init) == TREE_LIST >> && TREE_CHAIN (init) == NULL_TREE) >> { >> tree val = TREE_VALUE (init); >> @@ -802,6 +967,12 @@ perform_member_init (tree member, tree init) >> warning_at (DECL_SOURCE_LOCATION (current_function_decl), >> OPT_Winit_self, "%qD is initialized with itself", >> member); >> + else if (!uninitialized.is_empty ()) >> + { >> + find_uninit_data data = { &uninitialized, decl, nullptr }; >> + cp_walk_tree_without_duplicates (&val, find_uninit_fields_r, >> + &data); >> + } >> } >> if (array_of_unknown_bound_p (type)) >> @@ -830,6 +1001,9 @@ perform_member_init (tree member, tree init) >> do aggregate-initialization. */ >> } >> + /* Assume we are initializing the member. */ >> + bool member_initialized_p = true; >> + >> if (init == void_type_node) >> { >> /* mem() means value-initialization. */ >> @@ -970,6 +1144,9 @@ perform_member_init (tree member, tree init) >> diagnose_uninitialized_cst_or_ref_member (core_type, >> /*using_new=*/false, >> /*complain=*/true); >> + >> + /* We left the member uninitialized. */ >> + member_initialized_p = false; >> } >> maybe_warn_list_ctor (member, init); >> @@ -980,6 +1157,9 @@ perform_member_init (tree member, tree init) >> tf_warning_or_error)); >> } >> + if (member_initialized_p && warn_uninitialized) >> + uninitialized.remove (member); >> + >> if (type_build_dtor_call (type)) >> { >> tree expr; >> @@ -1293,13 +1473,31 @@ emit_mem_initializers (tree mem_inits) >> if (!COMPLETE_TYPE_P (current_class_type)) >> return; >> + /* Keep a set holding fields that are not initialized. */ >> + hash_set<tree> uninitialized; >> + >> + /* Initially that is all of them. */ >> + if (warn_uninitialized) >> + for (tree field = TYPE_FIELDS (current_class_type); field; >> + field = TREE_CHAIN (field)) >> + if (TREE_CODE (field) == FIELD_DECL && !DECL_ARTIFICIAL (field)) >> + uninitialized.add (field); >> + >> if (mem_inits >> && TYPE_P (TREE_PURPOSE (mem_inits)) >> && same_type_p (TREE_PURPOSE (mem_inits), current_class_type)) >> { >> /* Delegating constructor. */ >> gcc_assert (TREE_CHAIN (mem_inits) == NULL_TREE); >> - perform_target_ctor (TREE_VALUE (mem_inits)); >> + tree ctor = perform_target_ctor (TREE_VALUE (mem_inits)); >> + if (!uninitialized.is_empty ()) >> + { >> + find_uninit_data data = { &uninitialized, current_class_type, >> + nullptr }; >> + cp_walk_tree_without_duplicates (&ctor, >> + find_uninit_fields_r, >> + &data); >> + } >> return; >> } >> @@ -1360,6 +1558,15 @@ emit_mem_initializers (tree mem_inits) >> flags, >> tf_warning_or_error); >> expand_cleanup_for_base (subobject, NULL_TREE); >> + if (!uninitialized.is_empty () >> + && STATEMENT_LIST_TAIL (cur_stmt_list)) >> + { >> + tree last = STATEMENT_LIST_TAIL (cur_stmt_list)->stmt; >> + find_uninit_data data = { &uninitialized, BINFO_TYPE >> (subobject), >> + nullptr }; >> + cp_walk_tree_without_duplicates (&last, find_uninit_fields_r, >> + &data); >> + } >> } >> else if (!ABSTRACT_CLASS_TYPE_P (current_class_type)) >> /* C++14 DR1658 Means we do not have to construct vbases of >> @@ -1387,7 +1594,9 @@ emit_mem_initializers (tree mem_inits) >> iloc_sentinel ils (EXPR_LOCATION (TREE_TYPE (mem_inits))); >> perform_member_init (TREE_PURPOSE (mem_inits), >> - TREE_VALUE (mem_inits)); >> + TREE_VALUE (mem_inits), >> + uninitialized); >> + >> mem_inits = TREE_CHAIN (mem_inits); >> } >> } >> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi >> index b3a2c7ce51d..cfd5c7e00a2 100644 >> --- a/gcc/doc/invoke.texi >> +++ b/gcc/doc/invoke.texi >> @@ -6688,6 +6688,18 @@ to compute a value that itself is never used, >> because such >> computations may be deleted by data flow analysis before the warnings >> are printed. >> +In C++, this warning also warns about using uninitialized objects in >> +member-initializer-lists. For example, GCC warns about @code{b} being >> +uninitialized in the following snippet: >> + >> +@smallexample >> +struct A @{ >> + int a; >> + int b; >> + A() : a(b) @{ @} >> +@}; >> +@end smallexample >> + >> @item -Wno-invalid-memory-model >> @opindex Winvalid-memory-model >> @opindex Wno-invalid-memory-model >> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-12.C >> b/gcc/testsuite/g++.dg/warn/Wuninitialized-12.C >> new file mode 100644 >> index 00000000000..4e51b4b09f2 >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-12.C >> @@ -0,0 +1,59 @@ >> +// PR c++/19808 >> +// { dg-do compile { target c++11 } } >> +// { dg-options "-Wuninitialized" } >> + >> +struct S { >> + int i, j, k, l; >> + S() : i(j), // { dg-warning "field .S::j. is used uninitialized" } >> + j(1), >> + k(l + 1), // { dg-warning "field .S::l. is used uninitialized" } >> + l(2) { } >> +}; >> + >> +struct A { >> + int a, b, c; >> + A() : a(b // { dg-warning "field .A::b. is used uninitialized" } >> + + c) { } // { dg-warning "field .A::c. is used uninitialized" } >> +}; >> + >> +struct B { >> + int &r; >> + int *p; >> + int a; >> + B() : r(a), p(&a), a(1) { } >> +}; >> + >> +struct C { >> + const int &r1, &r2; >> + C () : r1(r2), // { dg-warning "reference .C::r2. is not yet bound >> to a value when used here" } >> + r2(r1) { } >> +}; >> + >> +struct D { >> + int a = 1; >> + int b = 2; >> + D() : a(b + 1), b(a + 1) { } // { dg-warning "field .D::b. is used >> uninitialized" } >> +}; >> + >> +struct E { >> + int a = 1; >> + E() : a(a + 1) { } // { dg-warning "field .E::a. is used >> uninitialized" } >> +}; >> + >> +struct F { >> + int a = 1; >> + int b; >> + F() : b(a + 1) { } >> +}; >> + >> +struct bar { >> + bar() {} >> + bar(bar&) {} >> +}; >> + >> +class foo { >> + bar first; >> + bar second; >> +public: >> + foo() : first(second) {} // { dg-warning "field .foo::second. is >> used uninitialized" } >> +}; >> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-13.C >> b/gcc/testsuite/g++.dg/warn/Wuninitialized-13.C >> new file mode 100644 >> index 00000000000..9c631f22fbc >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-13.C >> @@ -0,0 +1,49 @@ >> +// PR c++/19808 >> +// { dg-do compile { target c++11 } } >> +// { dg-options "-Wuninitialized -Winit-self" } >> + >> +int fint(int); >> +int fintp(int *); >> +int fintr(int &); >> +int fintcr(const int &); >> + >> +int arr[10]; >> + >> +struct S { >> + int x; >> + int y; >> + const int z = 42; >> + int *p; >> + >> + S(int (*)[1]) : x(x) { } // { dg-warning "initialized with itself" } >> + S(int (*)[2]) : x(x + x) { } // { dg-warning "field .S::x. is used >> uninitialized" } >> + S(int (*)[3]) : x(static_cast<int>(y)) { } // { dg-warning "field >> .S::y. is used uninitialized" } >> + S(int (*)[4]) : x(static_cast<int>(x)) { } // { dg-warning "field >> .S::x. is used uninitialized" } >> + S(int (*)[5]) : x(fint(x)) { } // { dg-warning "field .S::x. is >> used uninitialized" } >> + S(int (*)[6]) : x(fint(y)) { } // { dg-warning "field .S::y. is >> used uninitialized" } >> + >> + S(int (*)[7]) : x(sizeof(x)) { } >> + S(int (*)[8]) : x(sizeof(y)) { } >> + S(int (*)[9]) : p(&x) { } >> + S(int (*)[10]) : x(fintp(&y)) { } >> + S(int (*)[11]) : x(fintr(y)) { } >> + S(int (*)[12]) : x(fintcr(y)) { } >> + S(int (*)[26]) : x(((void)(__typeof(y)) 1, 1)) { } >> + S(int (*)[27]) : x(((void)(decltype(y)) 1, 1)) { } >> + S(int (*)[28]) : x(__alignof__(y)) { } >> + S(int (*)[29]) : x(noexcept(y)) { } >> + >> + S(int (*)[13]) : x(0), y(x ? y : y) { } // { dg-warning "field >> .S::y. is used uninitialized" } >> + S(int (*)[14]) : x(0), y(1 + (x ? y : y)) { } // { dg-warning >> "field .S::y. is used uninitialized" } >> + S(int (*)[15]) : x(-y) { } // { dg-warning "field .S::y. is used >> uninitialized" } >> + S(int (*)[16]) : x(1 << y) { } // { dg-warning "field .S::y. is >> used uninitialized" } >> + S(int (*)[17]) : x(this->y) { } // { dg-warning "field .S::y. is >> used uninitialized" } >> + S(int (*)[18]) : x(arr[y]) { } // { dg-warning "field .S::y. is >> used uninitialized" } >> + S(int (*)[19]) : x(0), y(x ? x : y) { } // { dg-warning "field >> .S::y. is used uninitialized" } >> + S(int (*)[20]) : x(0), y(y ? x : y) { } // { dg-warning "field >> .S::y. is used uninitialized" } >> + S(int (*)[21]) : x(0), y(y ? x : x) { } // { dg-warning "field >> .S::y. is used uninitialized" } >> + S(int (*)[22]) : x(0), y((fint(y), x)) { } // { dg-warning "field >> .S::y. is used uninitialized" } >> + S(int (*)[23]) : x(0), y(x += y) { } // { dg-warning "field .S::y. >> is used uninitialized" } >> + S(int (*)[24]) : x(y += 10) { } // { dg-warning "field .S::y. is >> used uninitialized" } >> + S(int (*)[25]) : x(y++) { } // { dg-warning "field .S::y. is used >> uninitialized" } >> +}; >> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-14.C >> b/gcc/testsuite/g++.dg/warn/Wuninitialized-14.C >> new file mode 100644 >> index 00000000000..6bef85e4659 >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-14.C >> @@ -0,0 +1,22 @@ >> +// PR c++/19808 >> +// { dg-do compile } >> +// { dg-options "-Wuninitialized" } >> + >> +struct A { >> + int m; >> + int get() const { return m; } >> + >> + A() { } >> + A(int) { } >> + A(const A &) { } >> + A(A *) { } >> +}; >> + >> +struct S { >> + A a, b; >> + >> + S(int (*)[1]) : a() {} >> + S(int (*)[2]) : b(a.get()) {} >> + S(int (*)[3]) : b(a) {} >> + S(int (*)[4]) : a(&a) {} >> +}; >> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-15.C >> b/gcc/testsuite/g++.dg/warn/Wuninitialized-15.C >> new file mode 100644 >> index 00000000000..ed62758a984 >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-15.C >> @@ -0,0 +1,118 @@ >> +// PR c++/19808 >> +// { dg-do compile { target c++11 } } >> +// { dg-options "-Wuninitialized -Winit-self" } >> +// Largely copied from clang's test/SemaCXX/uninitialized.cpp. >> + >> +int x; >> +struct U { >> + U() : b(a) { } >> + int &a = x; >> + int &b; >> +}; >> + >> +struct T { >> + T() : a(b), b(a) { } // { dg-warning "reference .T::b. is not yet >> bound" } >> + int &a, &b; >> +}; >> + >> +struct S { >> + S() : a(a) { } // { dg-warning ".S::a. is initialized with itself" } >> + int &a; >> +}; >> + >> +struct A { >> + int a; >> + int b; >> + A() { } >> + A(int (*)[1]) : b(a) { } // { dg-warning ".A::a. is used >> uninitialized" } >> + A(int (*)[2]) : a(b) { } // { dg-warning ".A::b. is used >> uninitialized" } >> +}; >> + >> +struct D { >> + int a; >> + int &b; >> + int &c = a; >> + int d = b; >> + D() : b(a) { } >> +}; >> + >> +struct E { >> + int a; >> + int get(); >> + static int num(); >> + E() { } >> + E(int) { } >> +}; >> + >> +struct F { >> + int a; >> + E e; >> + int b; >> + F(int (*)[1]) : a(e.get()) { } // { dg-warning "field .F::e. is >> used uninitialized" } >> + F(int (*)[2]) : a(e.num()) { } >> + F(int (*)[3]) : e(a) { } // { dg-warning "field .F::a. is used >> uninitialized" } >> + F(int (*)[4]) : a(4), e(a) { } >> + F(int (*)[5]) : e(b) { } // { dg-warning "field .F::b. is used >> uninitialized" } >> + F(int (*)[6]) : e(b), b(4) { } // { dg-warning "field .F::b. is >> used uninitialized" } >> +}; >> + >> +struct G { >> + G(const A&) { }; >> +}; >> + >> +struct H { >> + A a1; >> + G g; >> + A a2; >> + H() : g(a1) { } >> + // ??? clang++ doesn't warn here >> + H(int) : g(a2) { } // { dg-warning "field .H::a2. is used >> uninitialized" } >> +}; >> + >> +struct I { >> + I(int *) { } >> +}; >> + >> +struct J : I { >> + int *a; >> + int *b; >> + int c; >> + J() : I((a = new int(5))), b(a), c(*a) { } >> +}; >> + >> +struct M { }; >> + >> +struct N : public M { >> + int a; >> + int b; >> + N() : b(a) { } // { dg-warning "field .N::a. is used uninitialized" } >> +}; >> + >> +struct O { >> + int x = 42; >> + int get() { return x; } >> +}; >> + >> +struct P { >> + O o; >> + int x = o.get(); >> + P() : x(o.get()) { } >> +}; >> + >> +struct Q { >> + int a; >> + int b; >> + int &c; >> + Q() : >> + a(c = 5), // { dg-warning "reference .Q::c. is not yet bound" } >> + b(c), // { dg-warning "reference .Q::c. is not yet bound" } >> + c(a) { } >> +}; >> + >> +struct R { >> + int a; >> + int b; >> + int c; >> + int d = a + b + c; >> + R() : a(c = 5), b(c), c(a) { } >> +}; >> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-16.C >> b/gcc/testsuite/g++.dg/warn/Wuninitialized-16.C >> new file mode 100644 >> index 00000000000..bb47a340f18 >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-16.C >> @@ -0,0 +1,12 @@ >> +// PR c++/19808 >> +// { dg-do compile { target c++11 } } >> +// { dg-options "-Wuninitialized" } >> + >> +struct S { >> + int a; >> + int b; >> + int c; >> + S() : a((b = 42)), c(b) { } >> + S(int) : a(((1, b) = 42)), c(b) { } >> + S(char) : a(((c++, b) = 42)), c(b) { } // { dg-warning "field >> .S::c. is used uninitialized" } >> +}; >> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-17.C >> b/gcc/testsuite/g++.dg/warn/Wuninitialized-17.C >> new file mode 100644 >> index 00000000000..f18ed9e2251 >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-17.C >> @@ -0,0 +1,146 @@ >> +// PR c++/19808 >> +// { dg-do compile { target c++11 } } >> +// { dg-options "-Wuninitialized -Winit-self" } >> +// Test we warn with initializer-lists. >> + >> +int num = 5; >> + >> +struct A { >> + int a; >> + int b; >> +}; >> + >> +struct B { >> + A a1; >> + A a2; >> +}; >> + >> +struct S { >> + A a; >> + int x; >> + int y; >> + S() : a{1, y} { } // { dg-warning "field .S::y. is used >> uninitialized" } >> +}; >> + >> +struct R { >> + A a1; >> + A a2; >> + A a3; >> + A a4; >> + R() : >> + a1{1, 2}, >> + a2{a2.a + 2}, // { dg-warning "field .R::a2. is used >> uninitialized" } >> + a3{a3.a + 2}, // { dg-warning "field .R::a3. is used >> uninitialized" } >> + a4{a4.b + 2} // { dg-warning "field .R::a4. is used uninitialized" } >> + { } >> +}; >> + >> +struct Q { >> + B b1; >> + B b2; >> + B b3; >> + B b4; >> + B b5; >> + B b6; >> + B b7; >> + B b8; >> + B b9; >> + B b10; >> + B b11; >> + B b12; >> + B b13; >> + B b14; >> + B b15; >> + B b16; >> + B b17; >> + B b18; >> + B b19; >> + B b20; >> + B b21; >> + B b22; >> + Q() : >> + b1{ {}, {} }, >> + b2{ {}, b2.a1 }, >> + b3{ b3.a1 }, // { dg-warning "field .Q::b3. is used uninitialized" } >> + b4{ {}, b4.a2}, // { dg-warning "field .Q::b4. is used >> uninitialized" } >> + b5{ b5.a2 }, // { dg-warning "field .Q::b5. is used uninitialized" } >> + b6{ {b6.a1.a} }, // { dg-warning "field .Q::b6. is used >> uninitialized" } >> + b7{ {0, b7.a1.a} }, >> + b8{ {}, {b8.a1.a} }, >> + b9{ {}, {0, b9.a1.a} }, >> + b10{ {b10.a1.b} }, // { dg-warning "field .Q::b10. is used >> uninitialized" } >> + b11{ {0, b11.a1.b} }, // { dg-warning "field .Q::b11. is used >> uninitialized" } >> + b12{ {}, {b12.a1.b} }, >> + b13{ {}, {0, b13.a1.b} }, >> + b14{ {b14.a2.a} }, // { dg-warning "field .Q::b14. is used >> uninitialized" } >> + b15{ {0, b15.a2.a} }, // { dg-warning "field .Q::b15. is used >> uninitialized" } >> + b16{ {}, {b16.a2.a} }, // { dg-warning "field .Q::b16. is used >> uninitialized" } >> + b17{ {}, {0, b17.a2.a} }, >> + b18{ {b18.a2.b} }, // { dg-warning "field .Q::b18. is used >> uninitialized" } >> + b19{ {0, b19.a2.b} }, // { dg-warning "field .Q::b19. is used >> uninitialized" } >> + b20{ {}, {b20.a2.b} }, // { dg-warning "field .Q::b20. is used >> uninitialized" } >> + b21{ {}, {0, b21.a2.b} }, // { dg-warning "field .Q::b21. is used >> uninitialized" } >> + b22{ {b18.a2.b + 5} } >> + { } >> +}; >> + >> +struct C { >> + int a; >> + int &b; >> + int c; >> +}; >> + >> +struct P { >> + C c1; >> + C c2; >> + C c3; >> + C c4; >> + C c5; >> + C c6; >> + C c7; >> + P() : >> + c1{ 0, num, 0 }, >> + c2{ 1, num, c2.b }, >> + c3{ c3.b, num }, // { dg-warning "field .P::c3. is used >> uninitialized" } >> + c4{ 0, c4.b, 0 }, // { dg-warning ".C::b. is initialized with >> itself" } >> + /* c5.c is binding to a reference, don't warn. */ >> + c5{ 0, c5.c, 0 }, >> + c6{ c6.b, num, 0 }, // { dg-warning "field .P::c6. is used >> uninitialized" } >> + c7{ 0, c7.a, 0 } >> + { } >> +}; >> + >> +struct D { >> + int &a; >> + int &b; >> +}; >> + >> +struct M { >> + D d1; >> + D d2; >> + D d3; >> + M() : >> + d1{ num, num }, >> + d2{ num, d2.a }, >> + // Not a FE warning. >> + d3{ d3.b, num } // { dg-warning "is used uninitialized" } >> + { } >> +}; >> + >> +M m; >> + >> +struct E { >> + E(); >> + E foo(); >> + E* operator->(); >> +}; >> + >> +struct F { >> + F(E); >> +}; >> + >> +struct G { >> + F f; >> + E e; >> + G() : f{ e->foo() }, e() { } // { dg-warning "field .G::e. is used >> uninitialized" } >> +}; >> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-18.C >> b/gcc/testsuite/g++.dg/warn/Wuninitialized-18.C >> new file mode 100644 >> index 00000000000..c05ad42f95e >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-18.C >> @@ -0,0 +1,22 @@ >> +// PR c++/96121 >> +// { dg-do compile { target c++11 } } >> +// { dg-options "-Wuninitialized" } >> + >> +struct A { >> + A(); >> + int i; >> +}; >> +struct B { >> + B(A); >> + int i; >> +}; >> + >> +struct composed2 { >> + B b_; >> + A a_; >> + composed2() : b_(a_) {} // { dg-warning "field .composed2::a_. is >> used uninitialized" } >> +}; >> + >> +composed2 test() { >> + return composed2{}; >> +} >> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-19.C >> b/gcc/testsuite/g++.dg/warn/Wuninitialized-19.C >> new file mode 100644 >> index 00000000000..c401f8636bf >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-19.C >> @@ -0,0 +1,50 @@ >> +// PR c++/19808 >> +// { dg-do compile { target c++11 } } >> +// { dg-options "-Wuninitialized" } >> +// Test we warn when initializing a base class. >> + >> +struct A { >> + A(int) { } >> +}; >> + >> +struct B : public A { >> + int x; >> + B() : A(x) { } // { dg-warning "field .B::x. is used uninitialized" } >> +}; >> + >> +struct C : public A { >> + int x; >> + int y; >> + C() : A(y = 4), x(y) { } >> +}; >> + >> +struct D : public A { >> + int x; >> + D() : A{x} { } // { dg-warning "field .D::x. is used uninitialized" } >> +}; >> + >> +struct E : public A { >> + int x; >> + int y; >> + E() : A{y = 4}, x(y) { } >> +}; >> + >> +struct F { >> + F(int&) { } >> +}; >> + >> +struct G : F { >> + int x; >> + G() : F(x) { } >> +}; >> + >> +struct H { >> + H(int *) { } >> +}; >> + >> +struct I : H { >> + int x; >> + int arr[2]; >> + I() : H(&x) { } >> + I(int) : H(arr) { } >> +}; >> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-20.C >> b/gcc/testsuite/g++.dg/warn/Wuninitialized-20.C >> new file mode 100644 >> index 00000000000..9f367f0fdfd >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-20.C >> @@ -0,0 +1,16 @@ >> +// PR c++/96121 >> +// { dg-do compile { target c++11 } } >> +// { dg-options "-Wuninitialized" } >> +// Test we warn with delegating constructors. >> + >> +struct A { >> + A(int); >> + A(int &, int); >> + A(int (*)[1]) : A(x) { } // { dg-warning "21:field .A::x. is used >> uninitialized" } >> + A(int (*)[2]) : A(x, x) { } // { dg-warning "24:field .A::x. is >> used uninitialized" } >> + A(int (*)[3]) : A(x, 0) { } >> + A(int (*)[4]) : A{x} { } // { dg-warning "21:field .A::x. is used >> uninitialized" } >> + A(int (*)[5]) : A{x, x} { } // { dg-warning "24:field .A::x. is >> used uninitialized" } >> + A(int (*)[6]) : A{x, 0} { } >> + int x; >> +}; >> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-21.C >> b/gcc/testsuite/g++.dg/warn/Wuninitialized-21.C >> new file mode 100644 >> index 00000000000..57ca00ab042 >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-21.C >> @@ -0,0 +1,20 @@ >> +// PR c++/19808 >> +// { dg-do compile } >> +// { dg-options "-Wuninitialized" } >> + >> +struct A { >> + int a; >> + int b; >> + A(int) {} >> +}; >> + >> +struct S { >> + A a; >> + A a2; >> + S() : >> + /* We don't warn here, because we consider partial initialization >> + as initializing the whole object. */ >> + a((a2.a = 42)), >> + a2(a2.a) >> + { } >> +}; >> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-22.C >> b/gcc/testsuite/g++.dg/warn/Wuninitialized-22.C >> new file mode 100644 >> index 00000000000..bd980d61118 >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-22.C >> @@ -0,0 +1,37 @@ >> +// PR c++/19808 >> +// { dg-do compile { target c++11 } } >> +// { dg-options "-Wuninitialized -Winit-self" } >> +// Test that we don't warn when initializing a reference, unless it's >> +// self-init. >> + >> +struct R { >> + int &r; >> +}; >> + >> +struct S { >> + R r; >> + int a; >> + int &b; >> + int c; >> +}; >> + >> +struct X { >> + S s; >> + X() : s{ { s.a }, 1, s.c, 3} { } >> +}; >> + >> +struct A { >> + int &r; >> + A() : r{r} { } // { dg-warning ".A::r. is initialized with itself" } >> +}; >> + >> +struct B { >> + int &r; >> + int a; >> + B() : r{a} { } >> +}; >> + >> +struct C { >> + R x; >> + C() : x{x.r} { } // { dg-warning ".R::r. is initialized with itself" } >> +}; >> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-23.C >> b/gcc/testsuite/g++.dg/warn/Wuninitialized-23.C >> new file mode 100644 >> index 00000000000..db3778300be >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-23.C >> @@ -0,0 +1,24 @@ >> +// PR c++/19808 >> +// { dg-do compile { target c++11 } } >> +// { dg-options "-Wuninitialized" } >> +// Test that we don't warn in an uninstantiated template. >> + >> +struct A { >> + int *fn() { return nullptr; } >> +}; >> + >> +template<typename T> >> +struct B { >> + B() : p(a->fn()) { } >> + A *a; >> + int *p; >> +}; >> + >> +template<typename T> >> +struct C { >> + C() : p(a->fn()) { } // { dg-warning "field .C<int>::a. is used >> uninitialized" } >> + A *a; >> + int *p; >> +}; >> + >> +C<int> c; >> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-24.C >> b/gcc/testsuite/g++.dg/warn/Wuninitialized-24.C >> new file mode 100644 >> index 00000000000..c0a88631fb4 >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-24.C >> @@ -0,0 +1,14 @@ >> +// PR c++/19808 >> +// { dg-do compile { target c++11 } } >> +// { dg-options "-Wuninitialized" } >> + >> +struct S { int x, y; }; >> + >> +struct A { >> + S a1[2]; >> + int i; >> + A(int (*)[1]) : a1{{1}} { } >> + A(int (*)[2]) : a1{ {1, 2} } { } >> + A(int (*)[3]) : a1{{i}} { } // { dg-warning "field .A::i. is used >> uninitialized" } >> + A(int (*)[4]) : a1{ {1, i} } { } // { dg-warning "field .A::i. is >> used uninitialized" } >> +}; >> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-25.C >> b/gcc/testsuite/g++.dg/warn/Wuninitialized-25.C >> new file mode 100644 >> index 00000000000..fb652f989a4 >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-25.C >> @@ -0,0 +1,12 @@ >> +// PR c++/19808 >> +// { dg-do compile { target c++11 } } >> +// { dg-options "-Wall" } >> + >> +struct A { >> + A *a; >> +}; >> + >> +struct B : A { >> + int i; >> + B() : A{a} {} >> +}; >> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-26.C >> b/gcc/testsuite/g++.dg/warn/Wuninitialized-26.C >> new file mode 100644 >> index 00000000000..a887d12e9f9 >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-26.C >> @@ -0,0 +1,22 @@ >> +// PR c++/19808 >> +// { dg-do compile { target c++11 } } >> +// { dg-options "-Wuninitialized" } >> +// Anonymous union/struct. >> +// ??? The diagnostic should be improved to say 'b' instead of >> +// "<anonymous>". >> + >> +struct S { >> + __extension__ struct { >> + int a; >> + int b; >> + }; >> + S() : a(b) { } // { dg-warning "is used uninitialized" } >> +}; >> + >> +struct U { >> + union { >> + int a; >> + int b; >> + }; >> + U() : a(b) { } // { dg-warning "is used uninitialized" } >> +}; >> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-27.C >> b/gcc/testsuite/g++.dg/warn/Wuninitialized-27.C >> new file mode 100644 >> index 00000000000..24e6b9b5b48 >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-27.C >> @@ -0,0 +1,20 @@ >> +// PR c++/19808 >> +// { dg-do compile } >> +// { dg-options "-Wall" } >> + >> +enum E { red }; >> + >> +struct C { >> + C(int *, unsigned); >> +}; >> + >> +template <unsigned U> struct D : C { >> + D(int, int, E) : C(e, U) {} >> + int e[2]; >> +}; >> + >> +void >> +g () >> +{ >> + D<1>(0, 0, red); >> +} >> diff --git a/gcc/tree.c b/gcc/tree.c >> index 1ad4ad5a5f7..1ad884120b0 100644 >> --- a/gcc/tree.c >> +++ b/gcc/tree.c >> @@ -4638,6 +4638,7 @@ stabilize_reference (tree ref) >> TREE_READONLY (result) = TREE_READONLY (ref); >> TREE_SIDE_EFFECTS (result) = TREE_SIDE_EFFECTS (ref); >> TREE_THIS_VOLATILE (result) = TREE_THIS_VOLATILE (ref); >> + protected_set_expr_location (result, EXPR_LOCATION (ref)); >> return result; >> } >> >> base-commit: 6403e02510fe84c72c515247a9dfdb6e05d977fe >> >
On Mon, Nov 16, 2020 at 02:02:00PM -0500, Jason Merrill via Gcc-patches wrote: > On 11/16/20 12:42 PM, Martin Sebor wrote: > > On 11/15/20 3:44 PM, Marek Polacek via Gcc-patches wrote: > > > This patch implements the long-desired -Wuninitialized warning for > > > member initializer lists, so that the front end can detect bugs like > > > > > >   struct A { > > >     int a; > > >     int b; > > >     A() : b(1), a(b) { } > > >   }; > > > > > > where the field 'b' is used uninitialized because the order of member > > > initializers in the member initializer list is irrelevant; what matters > > > is the order of declarations in the class definition. > > > > > > I've implemented this by keeping a hash set holding fields that are not > > > initialized yet, so at first it will be {a, b}, and after initializing > > > 'a' it will be {b} and so on. Then I use walk_tree to walk the > > > initializer and if we see that an uninitialized object is used, we warn. > > > Of course, when we use the address of the object, we may not warn: > > > > > >   struct B { > > >     int &r; > > >     int *p; > > >     int a; > > >     B() : r(a), p(&a), a(1) { } // ok > > >   }; > > > > > > Likewise, don't warn in unevaluated contexts such as sizeof. Classes > > > without an explicit initializer may still be initialized by their > > > default constructors; whether or not something is considered initialized > > > is handled in perform_member_init, see member_initialized_p. It is > > > possible that we might want to tweak that if the current approach turns > > > out to be flawed. > > > > > > And this is where things stop being simple. I had to implement certain > > > gymnastics to handle cases like > > > > > >   struct S { > > >     int a, b, c; > > >     S() : a((b = 42)), c(b) { } > > >   }; > > > > > > where the initializer for 'a' surreptitiously initializes 'b'. And since > > > 'b' isn't read in the assignment, we can't warn that it's being used > > > uninitialized. > > > > > > Also, initializer lists. Consider: > > > > > >   struct A { > > >     int x, y, z; > > >   }; > > > > > >   struct S { > > >     A a; > > >     S() : a{ 1, 2, a.y } {} // #1 > > >     S(int) : a{ a.y, 2, 3 } {} // #2 > > >   }; > > > > > > In #1, a.y refers to an initialized element, so we oughtn't warn. But > > > in #2, a.y refers to an uninitialized element, so we should warn. To > > > that end I've added a vector that tracks which elements of an initializer > > > list are already initialized, so that when we encounter them later in > > > the list, we don't warn. This is the find_uninit_data->list_inits > > > thing. It will contain paths to the initialized elements, so e.g. > > > > > >   ((S *)this)->a.y > > > > > > These are edge cases, but some developers live on the edge. > > > > > > This patch also handles delegating constructors and initializing base > > > classes. > > > > > > A couple of TODOs: > > > - the same approach could be used to warn for NSDMIs, but I think > > > it's out > > >   of scope for this patch; > > > - the diagnostic for anonymous unions/structs should be improved > > >   to say the name of the member instead of "<anonymous>"; > > > - certain uninitialized warnings in initializer-lists are issued > > >   by the FE as well as the ME. It's unclear to me what we want to do > > >   here; > > > - using uninitialized members of base classes is not implemented yet. > > > > This is a great enhancement! Detecting the misues even in inline > > functions is especially useful. > > Why doesn't the middle-end warning work for inline functions? > > > I installed the patch to see how > > it handles some of the cases the middle end was recently enhanced > > to detect. Below is a test case I tried and the outupt. Clearly, > > the middle end warning doesn't work as intended for C++ ctors, so > > that's something for me to look into. > > Please. If we can handle these warnings in the middle-end (where we have > data flow info), that should be more reliable than the front-end walking the > trees. For function bodies, definitely. I don't we should attempt to use walk_tree on them. But for initializers, like in this patch, I think it's okay. Though of course it will still issue false positives for cases like ?: or a || b, or a && b, etc. in the initializer. Marek
On 11/16/20 2:24 PM, Marek Polacek wrote: > On Mon, Nov 16, 2020 at 02:02:00PM -0500, Jason Merrill via Gcc-patches wrote: >> On 11/16/20 12:42 PM, Martin Sebor wrote: >>> On 11/15/20 3:44 PM, Marek Polacek via Gcc-patches wrote: >>>> This patch implements the long-desired -Wuninitialized warning for >>>> member initializer lists, so that the front end can detect bugs like >>>> >>>>   struct A { >>>>     int a; >>>>     int b; >>>>     A() : b(1), a(b) { } >>>>   }; >>>> >>>> where the field 'b' is used uninitialized because the order of member >>>> initializers in the member initializer list is irrelevant; what matters >>>> is the order of declarations in the class definition. >>>> >>>> I've implemented this by keeping a hash set holding fields that are not >>>> initialized yet, so at first it will be {a, b}, and after initializing >>>> 'a' it will be {b} and so on. Then I use walk_tree to walk the >>>> initializer and if we see that an uninitialized object is used, we warn. >>>> Of course, when we use the address of the object, we may not warn: >>>> >>>>   struct B { >>>>     int &r; >>>>     int *p; >>>>     int a; >>>>     B() : r(a), p(&a), a(1) { } // ok >>>>   }; >>>> >>>> Likewise, don't warn in unevaluated contexts such as sizeof. Classes >>>> without an explicit initializer may still be initialized by their >>>> default constructors; whether or not something is considered initialized >>>> is handled in perform_member_init, see member_initialized_p. It is >>>> possible that we might want to tweak that if the current approach turns >>>> out to be flawed. >>>> >>>> And this is where things stop being simple. I had to implement certain >>>> gymnastics to handle cases like >>>> >>>>   struct S { >>>>     int a, b, c; >>>>     S() : a((b = 42)), c(b) { } >>>>   }; >>>> >>>> where the initializer for 'a' surreptitiously initializes 'b'. And since >>>> 'b' isn't read in the assignment, we can't warn that it's being used >>>> uninitialized. >>>> >>>> Also, initializer lists. Consider: >>>> >>>>   struct A { >>>>     int x, y, z; >>>>   }; >>>> >>>>   struct S { >>>>     A a; >>>>     S() : a{ 1, 2, a.y } {} // #1 >>>>     S(int) : a{ a.y, 2, 3 } {} // #2 >>>>   }; >>>> >>>> In #1, a.y refers to an initialized element, so we oughtn't warn. But >>>> in #2, a.y refers to an uninitialized element, so we should warn. To >>>> that end I've added a vector that tracks which elements of an initializer >>>> list are already initialized, so that when we encounter them later in >>>> the list, we don't warn. This is the find_uninit_data->list_inits >>>> thing. It will contain paths to the initialized elements, so e.g. >>>> >>>>   ((S *)this)->a.y >>>> >>>> These are edge cases, but some developers live on the edge. >>>> >>>> This patch also handles delegating constructors and initializing base >>>> classes. >>>> >>>> A couple of TODOs: >>>> - the same approach could be used to warn for NSDMIs, but I think >>>> it's out >>>>   of scope for this patch; >>>> - the diagnostic for anonymous unions/structs should be improved >>>>   to say the name of the member instead of "<anonymous>"; >>>> - certain uninitialized warnings in initializer-lists are issued >>>>   by the FE as well as the ME. It's unclear to me what we want to do >>>>   here; >>>> - using uninitialized members of base classes is not implemented yet. >>> >>> This is a great enhancement! Detecting the misues even in inline >>> functions is especially useful. >> >> Why doesn't the middle-end warning work for inline functions? >> >>> I installed the patch to see how >>> it handles some of the cases the middle end was recently enhanced >>> to detect. Below is a test case I tried and the outupt. Clearly, >>> the middle end warning doesn't work as intended for C++ ctors, so >>> that's something for me to look into. >> >> Please. If we can handle these warnings in the middle-end (where we have >> data flow info), that should be more reliable than the front-end walking the >> trees. > > For function bodies, definitely. I don't we should attempt to use walk_tree > on them. But for initializers, like in this patch, I think it's okay. > Though of course it will still issue false positives for cases like ?: or > a || b, or a && b, etc. in the initializer. I agree it's OK, but may be redundant if the middle-end will provide the same warnings. Jason
On 11/16/20 12:02 PM, Jason Merrill wrote: > On 11/16/20 12:42 PM, Martin Sebor wrote: >> On 11/15/20 3:44 PM, Marek Polacek via Gcc-patches wrote: >>> This patch implements the long-desired -Wuninitialized warning for >>> member initializer lists, so that the front end can detect bugs like >>> >>> struct A { >>> int a; >>> int b; >>> A() : b(1), a(b) { } >>> }; >>> >>> where the field 'b' is used uninitialized because the order of member >>> initializers in the member initializer list is irrelevant; what matters >>> is the order of declarations in the class definition. >>> >>> I've implemented this by keeping a hash set holding fields that are not >>> initialized yet, so at first it will be {a, b}, and after initializing >>> 'a' it will be {b} and so on. Then I use walk_tree to walk the >>> initializer and if we see that an uninitialized object is used, we warn. >>> Of course, when we use the address of the object, we may not warn: >>> >>> struct B { >>> int &r; >>> int *p; >>> int a; >>> B() : r(a), p(&a), a(1) { } // ok >>> }; >>> >>> Likewise, don't warn in unevaluated contexts such as sizeof. Classes >>> without an explicit initializer may still be initialized by their >>> default constructors; whether or not something is considered initialized >>> is handled in perform_member_init, see member_initialized_p. It is >>> possible that we might want to tweak that if the current approach turns >>> out to be flawed. >>> >>> And this is where things stop being simple. I had to implement certain >>> gymnastics to handle cases like >>> >>> struct S { >>> int a, b, c; >>> S() : a((b = 42)), c(b) { } >>> }; >>> >>> where the initializer for 'a' surreptitiously initializes 'b'. And >>> since >>> 'b' isn't read in the assignment, we can't warn that it's being used >>> uninitialized. >>> >>> Also, initializer lists. Consider: >>> >>> struct A { >>> int x, y, z; >>> }; >>> >>> struct S { >>> A a; >>> S() : a{ 1, 2, a.y } {} // #1 >>> S(int) : a{ a.y, 2, 3 } {} // #2 >>> }; >>> >>> In #1, a.y refers to an initialized element, so we oughtn't warn. But >>> in #2, a.y refers to an uninitialized element, so we should warn. To >>> that end I've added a vector that tracks which elements of an >>> initializer >>> list are already initialized, so that when we encounter them later in >>> the list, we don't warn. This is the find_uninit_data->list_inits >>> thing. It will contain paths to the initialized elements, so e.g. >>> >>> ((S *)this)->a.y >>> >>> These are edge cases, but some developers live on the edge. >>> >>> This patch also handles delegating constructors and initializing base >>> classes. >>> >>> A couple of TODOs: >>> - the same approach could be used to warn for NSDMIs, but I think >>> it's out >>> of scope for this patch; >>> - the diagnostic for anonymous unions/structs should be improved >>> to say the name of the member instead of "<anonymous>"; >>> - certain uninitialized warnings in initializer-lists are issued >>> by the FE as well as the ME. It's unclear to me what we want to do >>> here; >>> - using uninitialized members of base classes is not implemented yet. >> >> This is a great enhancement! Detecting the misues even in inline >> functions is especially useful. > > Why doesn't the middle-end warning work for inline functions? It does but only when they're called (and, as usual, also unless the uninitialized use is eliminated). > >> I installed the patch to see how >> it handles some of the cases the middle end was recently enhanced >> to detect. Below is a test case I tried and the outupt. Clearly, >> the middle end warning doesn't work as intended for C++ ctors, so >> that's something for me to look into. > > Please. If we can handle these warnings in the middle-end (where we > have data flow info), that should be more reliable than the front-end > walking the trees. I agree we should minimize overlap with Marek's work and I'll look into it for GCC 11. What I won't be able to do anything about is detecting these problems in inline functions (unless they're called). I think that's what makes Marek's work valuable. I don't know if his approach does anything for templates. It would be very useful to template libraries if it could. Martin > >> It would be good to enhance the front end warning to use the same >> assumptions as the middle end (i.e., that arguments passed by >> reference to const-qualified parameters are read from). This is >> also the basis for warning on the call to strlen in the test case. >> That way both could detect the same problems (the front end even >> in inline functions). >> >> There's a duplicate warning for the first ctor that we might want >> to try to avoid, probably by tweaking the middle end one. >> >> Martin >> >> $ cat a.C && gcc -O2 -S -Wall a.C >> int f (int); >> int g (const int&); >> int h (const int*); >> >> struct A { >> A (int(*)[1]); >> A (int(*)[2]); >> A (int(*)[3]); >> A (int(*)[4]); >> >> int a, b; >> char s[32]; >> }; >> >> A::A (int(*)[1]): a {f (b)} { } // duplicate warning >> A::A (int(*)[2]): a {g (b)} { } // missing warning >> A::A (int(*)[3]): a {h (&b)} { } // missing warning >> A::A (int(*)[4]): a (__builtin_strlen (s)) { } >> >> void ff (void) >> { >> int a; >> f (a); // warning >> } >> >> void gg (void) >> { >> int a; >> g (a); // warning >> } >> >> void hh (void) >> { >> int a; >> h (&a); // warning >> } >> >> int ii (void) >> { >> char s[32]; >> return __builtin_strlen (s); // warning >> } >> >> a.C: In constructor ‘A::A(int (*)[1])’: >> a.C:15:25: warning: field ‘A::b’ is used uninitialized [-Wuninitialized] >> 15 | A::A (int(*)[1]): a {f (b)} { } // duplicate warning >> | ^ >> a.C: In constructor ‘A::A(int (*)[1])’: >> a.C:15:24: warning: ‘*<unknown>.A::b’ is used uninitialized >> [-Wuninitialized] >> 15 | A::A (int(*)[1]): a {f (b)} { } // duplicate warning >> | ~~^~~ >> a.C: In function ‘void ff()’: >> a.C:23:5: warning: ‘a’ is used uninitialized [-Wuninitialized] >> 23 | f (a); // warning >> | ~~^~~ >> a.C: In function ‘void gg()’: >> a.C:29:5: warning: ‘a’ may be used uninitialized [-Wmaybe-uninitialized] >> 29 | g (a); // warning >> | ~~^~~ >> a.C:2:5: note: by argument 1 of type ‘const int&’ to ‘int g(const >> int&)’ declared here >> 2 | int g (const int&); >> | ^ >> a.C:28:7: note: ‘a’ declared here >> 28 | int a; >> | ^ >> a.C: In function ‘void hh()’: >> a.C:35:5: warning: ‘a’ may be used uninitialized [-Wmaybe-uninitialized] >> 35 | h (&a); // warning >> | ~~^~~~ >> a.C:3:5: note: by argument 1 of type ‘const int*’ to ‘int h(const >> int*)’ declared here >> 3 | int h (const int*); >> | ^ >> a.C:34:7: note: ‘a’ declared here >> 34 | int a; >> | ^ >> a.C: In function ‘int ii()’: >> a.C:41:27: warning: ‘s’ is used uninitialized [-Wuninitialized] >> 41 | return __builtin_strlen (s); // warning >> | ~~~~~~~~~~~~~~~~~^~~ >> <built-in>: note: by argument 1 of type ‘const char*’ to ‘long >> unsigned int __builtin_strlen(const char*)’ declared here >> a.C:40:8: note: ‘s’ declared here >> 40 | char s[32]; >> | ^ >> >>> >>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? >>> >>> gcc/cp/ChangeLog: >>> >>> PR c++/19808 >>> PR c++/96121 >>> * init.c (perform_member_init): Remove a forward declaration. >>> Walk the initializer using find_uninit_fields_r. New parameter >>> to track uninitialized fields. If a member is initialized, >>> remove it from the hash set. >>> (perform_target_ctor): Return the initializer. >>> (struct find_uninit_data): New class. >>> (find_uninit_fields_r): New function. >>> (emit_mem_initializers): Keep and initialize a set holding fields >>> that are not initialized. When handling delegating constructors, >>> walk the constructor tree using find_uninit_fields_r. Also when >>> initializing base clases. Pass uninitialized down to >>> perform_member_init. >>> >>> gcc/ChangeLog: >>> >>> PR c++/19808 >>> PR c++/96121 >>> * doc/invoke.texi: Update documentation for -Wuninitialized. >>> * tree.c (stabilize_reference): Set location. >>> >>> gcc/testsuite/ChangeLog: >>> >>> PR c++/19808 >>> PR c++/96121 >>> * g++.dg/warn/Wuninitialized-12.C: New test. >>> * g++.dg/warn/Wuninitialized-13.C: New test. >>> * g++.dg/warn/Wuninitialized-14.C: New test. >>> * g++.dg/warn/Wuninitialized-15.C: New test. >>> * g++.dg/warn/Wuninitialized-16.C: New test. >>> * g++.dg/warn/Wuninitialized-17.C: New test. >>> * g++.dg/warn/Wuninitialized-18.C: New test. >>> * g++.dg/warn/Wuninitialized-19.C: New test. >>> * g++.dg/warn/Wuninitialized-20.C: New test. >>> * g++.dg/warn/Wuninitialized-21.C: New test. >>> * g++.dg/warn/Wuninitialized-22.C: New test. >>> * g++.dg/warn/Wuninitialized-23.C: New test. >>> * g++.dg/warn/Wuninitialized-24.C: New test. >>> * g++.dg/warn/Wuninitialized-25.C: New test. >>> * g++.dg/warn/Wuninitialized-26.C: New test. >>> * g++.dg/warn/Wuninitialized-27.C: New test. >>> --- >>> gcc/cp/init.c | 235 +++++++++++++++++- >>> gcc/doc/invoke.texi | 12 + >>> gcc/testsuite/g++.dg/warn/Wuninitialized-12.C | 59 +++++ >>> gcc/testsuite/g++.dg/warn/Wuninitialized-13.C | 49 ++++ >>> gcc/testsuite/g++.dg/warn/Wuninitialized-14.C | 22 ++ >>> gcc/testsuite/g++.dg/warn/Wuninitialized-15.C | 118 +++++++++ >>> gcc/testsuite/g++.dg/warn/Wuninitialized-16.C | 12 + >>> gcc/testsuite/g++.dg/warn/Wuninitialized-17.C | 146 +++++++++++ >>> gcc/testsuite/g++.dg/warn/Wuninitialized-18.C | 22 ++ >>> gcc/testsuite/g++.dg/warn/Wuninitialized-19.C | 50 ++++ >>> gcc/testsuite/g++.dg/warn/Wuninitialized-20.C | 16 ++ >>> gcc/testsuite/g++.dg/warn/Wuninitialized-21.C | 20 ++ >>> gcc/testsuite/g++.dg/warn/Wuninitialized-22.C | 37 +++ >>> gcc/testsuite/g++.dg/warn/Wuninitialized-23.C | 24 ++ >>> gcc/testsuite/g++.dg/warn/Wuninitialized-24.C | 14 ++ >>> gcc/testsuite/g++.dg/warn/Wuninitialized-25.C | 12 + >>> gcc/testsuite/g++.dg/warn/Wuninitialized-26.C | 22 ++ >>> gcc/testsuite/g++.dg/warn/Wuninitialized-27.C | 20 ++ >>> gcc/tree.c | 1 + >>> 19 files changed, 878 insertions(+), 13 deletions(-) >>> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-12.C >>> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-13.C >>> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-14.C >>> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-15.C >>> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-16.C >>> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-17.C >>> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-18.C >>> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-19.C >>> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-20.C >>> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-21.C >>> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-22.C >>> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-23.C >>> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-24.C >>> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-25.C >>> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-26.C >>> create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-27.C >>> >>> diff --git a/gcc/cp/init.c b/gcc/cp/init.c >>> index ffb84ea5b09..92dcdc7bc02 100644 >>> --- a/gcc/cp/init.c >>> +++ b/gcc/cp/init.c >>> @@ -41,7 +41,6 @@ static tree finish_init_stmts (bool, tree, tree); >>> static void construct_virtual_base (tree, tree); >>> static void expand_aggr_init_1 (tree, tree, tree, tree, int, >>> tsubst_flags_t); >>> static void expand_default_init (tree, tree, tree, tree, int, >>> tsubst_flags_t); >>> -static void perform_member_init (tree, tree); >>> static int member_init_ok_or_else (tree, tree, tree); >>> static void expand_virtual_init (tree, tree); >>> static tree sort_mem_initializers (tree, tree); >>> @@ -521,19 +520,19 @@ build_value_init_noctor (tree type, >>> tsubst_flags_t complain) >>> return build_zero_init (type, NULL_TREE, >>> /*static_storage_p=*/false); >>> } >>> -/* Initialize current class with INIT, a TREE_LIST of >>> - arguments for a target constructor. If TREE_LIST is void_type_node, >>> - an empty initializer list was given. */ >>> +/* Initialize current class with INIT, a TREE_LIST of arguments for >>> + a target constructor. If TREE_LIST is void_type_node, an empty >>> + initializer list was given. Return the target constructor. */ >>> -static void >>> +static tree >>> perform_target_ctor (tree init) >>> { >>> tree decl = current_class_ref; >>> tree type = current_class_type; >>> - finish_expr_stmt (build_aggr_init (decl, init, >>> - LOOKUP_NORMAL|LOOKUP_DELEGATING_CONS, >>> - tf_warning_or_error)); >>> + init = build_aggr_init (decl, init, >>> LOOKUP_NORMAL|LOOKUP_DELEGATING_CONS, >>> + tf_warning_or_error); >>> + finish_expr_stmt (init); >>> if (type_build_dtor_call (type)) >>> { >>> tree expr = build_delete (input_location, >>> @@ -546,6 +545,7 @@ perform_target_ctor (tree init) >>> && TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type)) >>> finish_eh_cleanup (expr); >>> } >>> + return init; >>> } >>> /* Return the non-static data initializer for FIELD_DECL MEMBER. */ >>> @@ -757,12 +757,175 @@ maybe_warn_list_ctor (tree member, tree init) >>> "of the underlying array", member, begin); >>> } >>> +/* Data structure for find_uninit_fields_r, below. */ >>> + >>> +struct find_uninit_data { >>> + /* The set tracking the yet-uninitialized members. */ >>> + hash_set<tree> *uninitialized; >>> + /* The data member we are currently initializing. It can be either >>> + a type (initializing a base class/delegating constructors), or >>> + a COMPONENT_REF. */ >>> + tree member; >>> + /* A vector to track initialized fields in an initializer-list. */ >>> + vec<tree, va_gc> *list_inits; >>> +}; >>> + >>> +/* walk_tree callback that warns about using uninitialized data in >>> + a member-initializer-list. */ >>> + >>> +static tree >>> +find_uninit_fields_r (tree *tp, int *walk_subtrees, void *data) >>> +{ >>> + find_uninit_data *d = static_cast<find_uninit_data *>(data); >>> + hash_set<tree> *uninitialized = d->uninitialized; >>> + tree init = *tp; >>> + >>> + /* No need to look into types. */ >>> + if (TYPE_P (init)) >>> + { >>> + *walk_subtrees = false; >>> + return NULL_TREE; >>> + } >>> + >>> + switch (TREE_CODE (init)) >>> + { >>> + /* If we're just taking the address of an object, it doesn't matter >>> + if it's been initialized. */ >>> + case ADDR_EXPR: >>> + /* Unevaluated operands. */ >>> + case ALIGNOF_EXPR: >>> + case SIZEOF_EXPR: >>> + case NOEXCEPT_EXPR: >>> + *walk_subtrees = false; >>> + return NULL_TREE; >>> + default: >>> + break; >>> + } >>> + >>> + /* Handle cases like S() : a((b = 42)), c(b) { } where the >>> initializer >>> + for a surreptitiously initializes b. */ >>> + bool write_p = false; >>> + if (TREE_CODE (init) == MODIFY_EXPR) >>> + { >>> + /* First recurse on the RHS to detect (b += 10). */ >>> + cp_walk_tree_without_duplicates (&TREE_OPERAND (init, 1), >>> + find_uninit_fields_r, data); >>> + init = TREE_OPERAND (init, 0); >>> + /* Don't get tricked by a(((c, b) = 42)). */ >>> + while (TREE_CODE (init) == COMPOUND_EXPR) >>> + init = TREE_OPERAND (init, 1); >>> + /* The LHS might be something like this->a.b. Get this->a, so >>> that >>> + we can remove the member 'a' from the uninitialized set. We treat >>> + even partial initialization as initializing the whole object. */ >>> + while (TREE_CODE (init) == COMPONENT_REF) >>> + if (TREE_CODE (TREE_OPERAND (init, 0)) == COMPONENT_REF) >>> + init = TREE_OPERAND (init, 0); >>> + else >>> + break; >>> + write_p = true; >>> + } >>> + /* Warn about uninitialized 'this'. */ >>> + else if (TREE_CODE (init) == CALL_EXPR) >>> + { >>> + tree fn = get_callee_fndecl (init); >>> + if (fn && DECL_NONSTATIC_MEMBER_FUNCTION_P (fn)) >>> + { >>> + tree op = CALL_EXPR_ARG (init, 0); >>> + if (TREE_CODE (op) == ADDR_EXPR) >>> + op = TREE_OPERAND (op, 0); >>> + temp_override<tree> ovr (d->member, DECL_ARGUMENTS (fn)); >>> + cp_walk_tree_without_duplicates (&op, find_uninit_fields_r, >>> data); >>> + } >>> + } >>> + else if (BRACE_ENCLOSED_INITIALIZER_P (init)) >>> + { >>> + tree ctor = init; >>> + if (!d->list_inits) >>> + { >>> + /* Reshape the outermost { }; this will fill the .index >>> fields. */ >>> + tree type = TYPE_P (d->member) ? d->member : TREE_TYPE >>> (d->member); >>> + ctor = reshape_init (type, ctor, tf_none); >>> + d->list_inits = make_tree_vector (); >>> + } >>> + if (TREE_CODE (ctor) == CONSTRUCTOR && CONSTRUCTOR_NELTS >>> (ctor) > 0) >>> + { >>> + for (constructor_elt &elt : *CONSTRUCTOR_ELTS (ctor)) >>> + { >>> + temp_override<tree> ovr (d->member); >>> + /* Build up the path to the element we're initializing, >>> + if we're initializing a class member. */ >>> + if (TREE_CODE (elt.index) == FIELD_DECL >>> + && TREE_CODE (d->member) == COMPONENT_REF) >>> + d->member = build_class_member_access_expr (d->member, >>> + elt.index, >>> + NULL_TREE, >>> + /*ref=*/true, >>> + tf_none); >>> + cp_walk_tree_without_duplicates (&elt.value, >>> + find_uninit_fields_r, data); >>> + vec_safe_push (d->list_inits, d->member); >>> + } >>> + /* We've processed this initializer-list, clean up. */ >>> + release_tree_vector (d->list_inits); >>> + d->list_inits = nullptr; >>> + *walk_subtrees = false; >>> + return NULL_TREE; >>> + } >>> + /* Reshaping might have unwrapped { X } to X; carry on. */ >>> + } >>> + >>> + /* If we find FIELD in the uninitialized set and we're not writing >>> into >>> + it, we warn. */ >>> + if (TREE_CODE (init) == COMPONENT_REF) >>> + { >>> + unsigned int ix; >>> + tree t; >>> + FOR_EACH_VEC_SAFE_ELT (d->list_inits, ix, t) >>> + if (cp_tree_equal (init, t)) >>> + { >>> + /* This field was already initialized by a prior element in the >>> + initializer-list. */ >>> + *walk_subtrees = false; >>> + return NULL_TREE; >>> + } >>> + >>> + tree field = TREE_OPERAND (init, 1); >>> + tree type = TYPE_P (d->member) ? d->member : TREE_TYPE >>> (d->member); >>> + >>> + /* We're initializing a reference member with itself. */ >>> + if (TYPE_REF_P (type) && cp_tree_equal (d->member, init)) >>> + warning_at (EXPR_LOCATION (init), OPT_Winit_self, >>> + "%qD is initialized with itself", field); >>> + else if (cp_tree_equal (TREE_OPERAND (init, 0), >>> current_class_ref) >>> + && uninitialized->contains (field)) >>> + { >>> + /* If we're not reading but writing into a member in this >>> initializer >>> + (this is the LHS of an assignment), remember not to warn >>> about this >>> + member later. */ >>> + if (write_p) >>> + uninitialized->remove (field); >>> + else if (TYPE_REF_P (TREE_TYPE (field))) >>> + warning_at (EXPR_LOCATION (init), OPT_Wuninitialized, >>> + "reference %qD is not yet bound to a value when used " >>> + "here", field); >>> + else if (!INDIRECT_TYPE_P (type) || is_this_parameter >>> (d->member)) >>> + warning_at (EXPR_LOCATION (init), OPT_Wuninitialized, >>> + "field %qD is used uninitialized", field); >>> + /* We can't clear *walk_subtrees here to handle bogosities >>> such as >>> + ((c++, b) = 42). */ >>> + } >>> + } >>> + >>> + return NULL_TREE; >>> +} >>> + >>> /* 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. */ >>> + list was given; if NULL_TREE no initializer was given. >>> UNINITIALIZED >>> + is the hash set that tracks uninitialized fields. */ >>> static void >>> -perform_member_init (tree member, tree init) >>> +perform_member_init (tree member, tree init, hash_set<tree> >>> &uninitialized) >>> { >>> tree decl; >>> tree type = TREE_TYPE (member); >>> @@ -790,7 +953,9 @@ perform_member_init (tree member, tree init) >>> if (decl == error_mark_node) >>> return; >>> - if (warn_init_self && init && TREE_CODE (init) == TREE_LIST >>> + if ((warn_init_self || warn_uninitialized) >>> + && init >>> + && TREE_CODE (init) == TREE_LIST >>> && TREE_CHAIN (init) == NULL_TREE) >>> { >>> tree val = TREE_VALUE (init); >>> @@ -802,6 +967,12 @@ perform_member_init (tree member, tree init) >>> warning_at (DECL_SOURCE_LOCATION (current_function_decl), >>> OPT_Winit_self, "%qD is initialized with itself", >>> member); >>> + else if (!uninitialized.is_empty ()) >>> + { >>> + find_uninit_data data = { &uninitialized, decl, nullptr }; >>> + cp_walk_tree_without_duplicates (&val, find_uninit_fields_r, >>> + &data); >>> + } >>> } >>> if (array_of_unknown_bound_p (type)) >>> @@ -830,6 +1001,9 @@ perform_member_init (tree member, tree init) >>> do aggregate-initialization. */ >>> } >>> + /* Assume we are initializing the member. */ >>> + bool member_initialized_p = true; >>> + >>> if (init == void_type_node) >>> { >>> /* mem() means value-initialization. */ >>> @@ -970,6 +1144,9 @@ perform_member_init (tree member, tree init) >>> diagnose_uninitialized_cst_or_ref_member (core_type, >>> /*using_new=*/false, >>> /*complain=*/true); >>> + >>> + /* We left the member uninitialized. */ >>> + member_initialized_p = false; >>> } >>> maybe_warn_list_ctor (member, init); >>> @@ -980,6 +1157,9 @@ perform_member_init (tree member, tree init) >>> tf_warning_or_error)); >>> } >>> + if (member_initialized_p && warn_uninitialized) >>> + uninitialized.remove (member); >>> + >>> if (type_build_dtor_call (type)) >>> { >>> tree expr; >>> @@ -1293,13 +1473,31 @@ emit_mem_initializers (tree mem_inits) >>> if (!COMPLETE_TYPE_P (current_class_type)) >>> return; >>> + /* Keep a set holding fields that are not initialized. */ >>> + hash_set<tree> uninitialized; >>> + >>> + /* Initially that is all of them. */ >>> + if (warn_uninitialized) >>> + for (tree field = TYPE_FIELDS (current_class_type); field; >>> + field = TREE_CHAIN (field)) >>> + if (TREE_CODE (field) == FIELD_DECL && !DECL_ARTIFICIAL (field)) >>> + uninitialized.add (field); >>> + >>> if (mem_inits >>> && TYPE_P (TREE_PURPOSE (mem_inits)) >>> && same_type_p (TREE_PURPOSE (mem_inits), current_class_type)) >>> { >>> /* Delegating constructor. */ >>> gcc_assert (TREE_CHAIN (mem_inits) == NULL_TREE); >>> - perform_target_ctor (TREE_VALUE (mem_inits)); >>> + tree ctor = perform_target_ctor (TREE_VALUE (mem_inits)); >>> + if (!uninitialized.is_empty ()) >>> + { >>> + find_uninit_data data = { &uninitialized, current_class_type, >>> + nullptr }; >>> + cp_walk_tree_without_duplicates (&ctor, >>> + find_uninit_fields_r, >>> + &data); >>> + } >>> return; >>> } >>> @@ -1360,6 +1558,15 @@ emit_mem_initializers (tree mem_inits) >>> flags, >>> tf_warning_or_error); >>> expand_cleanup_for_base (subobject, NULL_TREE); >>> + if (!uninitialized.is_empty () >>> + && STATEMENT_LIST_TAIL (cur_stmt_list)) >>> + { >>> + tree last = STATEMENT_LIST_TAIL (cur_stmt_list)->stmt; >>> + find_uninit_data data = { &uninitialized, BINFO_TYPE >>> (subobject), >>> + nullptr }; >>> + cp_walk_tree_without_duplicates (&last, find_uninit_fields_r, >>> + &data); >>> + } >>> } >>> else if (!ABSTRACT_CLASS_TYPE_P (current_class_type)) >>> /* C++14 DR1658 Means we do not have to construct vbases of >>> @@ -1387,7 +1594,9 @@ emit_mem_initializers (tree mem_inits) >>> iloc_sentinel ils (EXPR_LOCATION (TREE_TYPE (mem_inits))); >>> perform_member_init (TREE_PURPOSE (mem_inits), >>> - TREE_VALUE (mem_inits)); >>> + TREE_VALUE (mem_inits), >>> + uninitialized); >>> + >>> mem_inits = TREE_CHAIN (mem_inits); >>> } >>> } >>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi >>> index b3a2c7ce51d..cfd5c7e00a2 100644 >>> --- a/gcc/doc/invoke.texi >>> +++ b/gcc/doc/invoke.texi >>> @@ -6688,6 +6688,18 @@ to compute a value that itself is never used, >>> because such >>> computations may be deleted by data flow analysis before the warnings >>> are printed. >>> +In C++, this warning also warns about using uninitialized objects in >>> +member-initializer-lists. For example, GCC warns about @code{b} being >>> +uninitialized in the following snippet: >>> + >>> +@smallexample >>> +struct A @{ >>> + int a; >>> + int b; >>> + A() : a(b) @{ @} >>> +@}; >>> +@end smallexample >>> + >>> @item -Wno-invalid-memory-model >>> @opindex Winvalid-memory-model >>> @opindex Wno-invalid-memory-model >>> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-12.C >>> b/gcc/testsuite/g++.dg/warn/Wuninitialized-12.C >>> new file mode 100644 >>> index 00000000000..4e51b4b09f2 >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-12.C >>> @@ -0,0 +1,59 @@ >>> +// PR c++/19808 >>> +// { dg-do compile { target c++11 } } >>> +// { dg-options "-Wuninitialized" } >>> + >>> +struct S { >>> + int i, j, k, l; >>> + S() : i(j), // { dg-warning "field .S::j. is used uninitialized" } >>> + j(1), >>> + k(l + 1), // { dg-warning "field .S::l. is used uninitialized" } >>> + l(2) { } >>> +}; >>> + >>> +struct A { >>> + int a, b, c; >>> + A() : a(b // { dg-warning "field .A::b. is used uninitialized" } >>> + + c) { } // { dg-warning "field .A::c. is used uninitialized" } >>> +}; >>> + >>> +struct B { >>> + int &r; >>> + int *p; >>> + int a; >>> + B() : r(a), p(&a), a(1) { } >>> +}; >>> + >>> +struct C { >>> + const int &r1, &r2; >>> + C () : r1(r2), // { dg-warning "reference .C::r2. is not yet bound >>> to a value when used here" } >>> + r2(r1) { } >>> +}; >>> + >>> +struct D { >>> + int a = 1; >>> + int b = 2; >>> + D() : a(b + 1), b(a + 1) { } // { dg-warning "field .D::b. is used >>> uninitialized" } >>> +}; >>> + >>> +struct E { >>> + int a = 1; >>> + E() : a(a + 1) { } // { dg-warning "field .E::a. is used >>> uninitialized" } >>> +}; >>> + >>> +struct F { >>> + int a = 1; >>> + int b; >>> + F() : b(a + 1) { } >>> +}; >>> + >>> +struct bar { >>> + bar() {} >>> + bar(bar&) {} >>> +}; >>> + >>> +class foo { >>> + bar first; >>> + bar second; >>> +public: >>> + foo() : first(second) {} // { dg-warning "field .foo::second. is >>> used uninitialized" } >>> +}; >>> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-13.C >>> b/gcc/testsuite/g++.dg/warn/Wuninitialized-13.C >>> new file mode 100644 >>> index 00000000000..9c631f22fbc >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-13.C >>> @@ -0,0 +1,49 @@ >>> +// PR c++/19808 >>> +// { dg-do compile { target c++11 } } >>> +// { dg-options "-Wuninitialized -Winit-self" } >>> + >>> +int fint(int); >>> +int fintp(int *); >>> +int fintr(int &); >>> +int fintcr(const int &); >>> + >>> +int arr[10]; >>> + >>> +struct S { >>> + int x; >>> + int y; >>> + const int z = 42; >>> + int *p; >>> + >>> + S(int (*)[1]) : x(x) { } // { dg-warning "initialized with itself" } >>> + S(int (*)[2]) : x(x + x) { } // { dg-warning "field .S::x. is used >>> uninitialized" } >>> + S(int (*)[3]) : x(static_cast<int>(y)) { } // { dg-warning "field >>> .S::y. is used uninitialized" } >>> + S(int (*)[4]) : x(static_cast<int>(x)) { } // { dg-warning "field >>> .S::x. is used uninitialized" } >>> + S(int (*)[5]) : x(fint(x)) { } // { dg-warning "field .S::x. is >>> used uninitialized" } >>> + S(int (*)[6]) : x(fint(y)) { } // { dg-warning "field .S::y. is >>> used uninitialized" } >>> + >>> + S(int (*)[7]) : x(sizeof(x)) { } >>> + S(int (*)[8]) : x(sizeof(y)) { } >>> + S(int (*)[9]) : p(&x) { } >>> + S(int (*)[10]) : x(fintp(&y)) { } >>> + S(int (*)[11]) : x(fintr(y)) { } >>> + S(int (*)[12]) : x(fintcr(y)) { } >>> + S(int (*)[26]) : x(((void)(__typeof(y)) 1, 1)) { } >>> + S(int (*)[27]) : x(((void)(decltype(y)) 1, 1)) { } >>> + S(int (*)[28]) : x(__alignof__(y)) { } >>> + S(int (*)[29]) : x(noexcept(y)) { } >>> + >>> + S(int (*)[13]) : x(0), y(x ? y : y) { } // { dg-warning "field >>> .S::y. is used uninitialized" } >>> + S(int (*)[14]) : x(0), y(1 + (x ? y : y)) { } // { dg-warning >>> "field .S::y. is used uninitialized" } >>> + S(int (*)[15]) : x(-y) { } // { dg-warning "field .S::y. is used >>> uninitialized" } >>> + S(int (*)[16]) : x(1 << y) { } // { dg-warning "field .S::y. is >>> used uninitialized" } >>> + S(int (*)[17]) : x(this->y) { } // { dg-warning "field .S::y. is >>> used uninitialized" } >>> + S(int (*)[18]) : x(arr[y]) { } // { dg-warning "field .S::y. is >>> used uninitialized" } >>> + S(int (*)[19]) : x(0), y(x ? x : y) { } // { dg-warning "field >>> .S::y. is used uninitialized" } >>> + S(int (*)[20]) : x(0), y(y ? x : y) { } // { dg-warning "field >>> .S::y. is used uninitialized" } >>> + S(int (*)[21]) : x(0), y(y ? x : x) { } // { dg-warning "field >>> .S::y. is used uninitialized" } >>> + S(int (*)[22]) : x(0), y((fint(y), x)) { } // { dg-warning "field >>> .S::y. is used uninitialized" } >>> + S(int (*)[23]) : x(0), y(x += y) { } // { dg-warning "field .S::y. >>> is used uninitialized" } >>> + S(int (*)[24]) : x(y += 10) { } // { dg-warning "field .S::y. is >>> used uninitialized" } >>> + S(int (*)[25]) : x(y++) { } // { dg-warning "field .S::y. is used >>> uninitialized" } >>> +}; >>> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-14.C >>> b/gcc/testsuite/g++.dg/warn/Wuninitialized-14.C >>> new file mode 100644 >>> index 00000000000..6bef85e4659 >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-14.C >>> @@ -0,0 +1,22 @@ >>> +// PR c++/19808 >>> +// { dg-do compile } >>> +// { dg-options "-Wuninitialized" } >>> + >>> +struct A { >>> + int m; >>> + int get() const { return m; } >>> + >>> + A() { } >>> + A(int) { } >>> + A(const A &) { } >>> + A(A *) { } >>> +}; >>> + >>> +struct S { >>> + A a, b; >>> + >>> + S(int (*)[1]) : a() {} >>> + S(int (*)[2]) : b(a.get()) {} >>> + S(int (*)[3]) : b(a) {} >>> + S(int (*)[4]) : a(&a) {} >>> +}; >>> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-15.C >>> b/gcc/testsuite/g++.dg/warn/Wuninitialized-15.C >>> new file mode 100644 >>> index 00000000000..ed62758a984 >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-15.C >>> @@ -0,0 +1,118 @@ >>> +// PR c++/19808 >>> +// { dg-do compile { target c++11 } } >>> +// { dg-options "-Wuninitialized -Winit-self" } >>> +// Largely copied from clang's test/SemaCXX/uninitialized.cpp. >>> + >>> +int x; >>> +struct U { >>> + U() : b(a) { } >>> + int &a = x; >>> + int &b; >>> +}; >>> + >>> +struct T { >>> + T() : a(b), b(a) { } // { dg-warning "reference .T::b. is not yet >>> bound" } >>> + int &a, &b; >>> +}; >>> + >>> +struct S { >>> + S() : a(a) { } // { dg-warning ".S::a. is initialized with itself" } >>> + int &a; >>> +}; >>> + >>> +struct A { >>> + int a; >>> + int b; >>> + A() { } >>> + A(int (*)[1]) : b(a) { } // { dg-warning ".A::a. is used >>> uninitialized" } >>> + A(int (*)[2]) : a(b) { } // { dg-warning ".A::b. is used >>> uninitialized" } >>> +}; >>> + >>> +struct D { >>> + int a; >>> + int &b; >>> + int &c = a; >>> + int d = b; >>> + D() : b(a) { } >>> +}; >>> + >>> +struct E { >>> + int a; >>> + int get(); >>> + static int num(); >>> + E() { } >>> + E(int) { } >>> +}; >>> + >>> +struct F { >>> + int a; >>> + E e; >>> + int b; >>> + F(int (*)[1]) : a(e.get()) { } // { dg-warning "field .F::e. is >>> used uninitialized" } >>> + F(int (*)[2]) : a(e.num()) { } >>> + F(int (*)[3]) : e(a) { } // { dg-warning "field .F::a. is used >>> uninitialized" } >>> + F(int (*)[4]) : a(4), e(a) { } >>> + F(int (*)[5]) : e(b) { } // { dg-warning "field .F::b. is used >>> uninitialized" } >>> + F(int (*)[6]) : e(b), b(4) { } // { dg-warning "field .F::b. is >>> used uninitialized" } >>> +}; >>> + >>> +struct G { >>> + G(const A&) { }; >>> +}; >>> + >>> +struct H { >>> + A a1; >>> + G g; >>> + A a2; >>> + H() : g(a1) { } >>> + // ??? clang++ doesn't warn here >>> + H(int) : g(a2) { } // { dg-warning "field .H::a2. is used >>> uninitialized" } >>> +}; >>> + >>> +struct I { >>> + I(int *) { } >>> +}; >>> + >>> +struct J : I { >>> + int *a; >>> + int *b; >>> + int c; >>> + J() : I((a = new int(5))), b(a), c(*a) { } >>> +}; >>> + >>> +struct M { }; >>> + >>> +struct N : public M { >>> + int a; >>> + int b; >>> + N() : b(a) { } // { dg-warning "field .N::a. is used uninitialized" } >>> +}; >>> + >>> +struct O { >>> + int x = 42; >>> + int get() { return x; } >>> +}; >>> + >>> +struct P { >>> + O o; >>> + int x = o.get(); >>> + P() : x(o.get()) { } >>> +}; >>> + >>> +struct Q { >>> + int a; >>> + int b; >>> + int &c; >>> + Q() : >>> + a(c = 5), // { dg-warning "reference .Q::c. is not yet bound" } >>> + b(c), // { dg-warning "reference .Q::c. is not yet bound" } >>> + c(a) { } >>> +}; >>> + >>> +struct R { >>> + int a; >>> + int b; >>> + int c; >>> + int d = a + b + c; >>> + R() : a(c = 5), b(c), c(a) { } >>> +}; >>> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-16.C >>> b/gcc/testsuite/g++.dg/warn/Wuninitialized-16.C >>> new file mode 100644 >>> index 00000000000..bb47a340f18 >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-16.C >>> @@ -0,0 +1,12 @@ >>> +// PR c++/19808 >>> +// { dg-do compile { target c++11 } } >>> +// { dg-options "-Wuninitialized" } >>> + >>> +struct S { >>> + int a; >>> + int b; >>> + int c; >>> + S() : a((b = 42)), c(b) { } >>> + S(int) : a(((1, b) = 42)), c(b) { } >>> + S(char) : a(((c++, b) = 42)), c(b) { } // { dg-warning "field >>> .S::c. is used uninitialized" } >>> +}; >>> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-17.C >>> b/gcc/testsuite/g++.dg/warn/Wuninitialized-17.C >>> new file mode 100644 >>> index 00000000000..f18ed9e2251 >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-17.C >>> @@ -0,0 +1,146 @@ >>> +// PR c++/19808 >>> +// { dg-do compile { target c++11 } } >>> +// { dg-options "-Wuninitialized -Winit-self" } >>> +// Test we warn with initializer-lists. >>> + >>> +int num = 5; >>> + >>> +struct A { >>> + int a; >>> + int b; >>> +}; >>> + >>> +struct B { >>> + A a1; >>> + A a2; >>> +}; >>> + >>> +struct S { >>> + A a; >>> + int x; >>> + int y; >>> + S() : a{1, y} { } // { dg-warning "field .S::y. is used >>> uninitialized" } >>> +}; >>> + >>> +struct R { >>> + A a1; >>> + A a2; >>> + A a3; >>> + A a4; >>> + R() : >>> + a1{1, 2}, >>> + a2{a2.a + 2}, // { dg-warning "field .R::a2. is used >>> uninitialized" } >>> + a3{a3.a + 2}, // { dg-warning "field .R::a3. is used >>> uninitialized" } >>> + a4{a4.b + 2} // { dg-warning "field .R::a4. is used >>> uninitialized" } >>> + { } >>> +}; >>> + >>> +struct Q { >>> + B b1; >>> + B b2; >>> + B b3; >>> + B b4; >>> + B b5; >>> + B b6; >>> + B b7; >>> + B b8; >>> + B b9; >>> + B b10; >>> + B b11; >>> + B b12; >>> + B b13; >>> + B b14; >>> + B b15; >>> + B b16; >>> + B b17; >>> + B b18; >>> + B b19; >>> + B b20; >>> + B b21; >>> + B b22; >>> + Q() : >>> + b1{ {}, {} }, >>> + b2{ {}, b2.a1 }, >>> + b3{ b3.a1 }, // { dg-warning "field .Q::b3. is used >>> uninitialized" } >>> + b4{ {}, b4.a2}, // { dg-warning "field .Q::b4. is used >>> uninitialized" } >>> + b5{ b5.a2 }, // { dg-warning "field .Q::b5. is used >>> uninitialized" } >>> + b6{ {b6.a1.a} }, // { dg-warning "field .Q::b6. is used >>> uninitialized" } >>> + b7{ {0, b7.a1.a} }, >>> + b8{ {}, {b8.a1.a} }, >>> + b9{ {}, {0, b9.a1.a} }, >>> + b10{ {b10.a1.b} }, // { dg-warning "field .Q::b10. is used >>> uninitialized" } >>> + b11{ {0, b11.a1.b} }, // { dg-warning "field .Q::b11. is used >>> uninitialized" } >>> + b12{ {}, {b12.a1.b} }, >>> + b13{ {}, {0, b13.a1.b} }, >>> + b14{ {b14.a2.a} }, // { dg-warning "field .Q::b14. is used >>> uninitialized" } >>> + b15{ {0, b15.a2.a} }, // { dg-warning "field .Q::b15. is used >>> uninitialized" } >>> + b16{ {}, {b16.a2.a} }, // { dg-warning "field .Q::b16. is used >>> uninitialized" } >>> + b17{ {}, {0, b17.a2.a} }, >>> + b18{ {b18.a2.b} }, // { dg-warning "field .Q::b18. is used >>> uninitialized" } >>> + b19{ {0, b19.a2.b} }, // { dg-warning "field .Q::b19. is used >>> uninitialized" } >>> + b20{ {}, {b20.a2.b} }, // { dg-warning "field .Q::b20. is used >>> uninitialized" } >>> + b21{ {}, {0, b21.a2.b} }, // { dg-warning "field .Q::b21. is >>> used uninitialized" } >>> + b22{ {b18.a2.b + 5} } >>> + { } >>> +}; >>> + >>> +struct C { >>> + int a; >>> + int &b; >>> + int c; >>> +}; >>> + >>> +struct P { >>> + C c1; >>> + C c2; >>> + C c3; >>> + C c4; >>> + C c5; >>> + C c6; >>> + C c7; >>> + P() : >>> + c1{ 0, num, 0 }, >>> + c2{ 1, num, c2.b }, >>> + c3{ c3.b, num }, // { dg-warning "field .P::c3. is used >>> uninitialized" } >>> + c4{ 0, c4.b, 0 }, // { dg-warning ".C::b. is initialized with >>> itself" } >>> + /* c5.c is binding to a reference, don't warn. */ >>> + c5{ 0, c5.c, 0 }, >>> + c6{ c6.b, num, 0 }, // { dg-warning "field .P::c6. is used >>> uninitialized" } >>> + c7{ 0, c7.a, 0 } >>> + { } >>> +}; >>> + >>> +struct D { >>> + int &a; >>> + int &b; >>> +}; >>> + >>> +struct M { >>> + D d1; >>> + D d2; >>> + D d3; >>> + M() : >>> + d1{ num, num }, >>> + d2{ num, d2.a }, >>> + // Not a FE warning. >>> + d3{ d3.b, num } // { dg-warning "is used uninitialized" } >>> + { } >>> +}; >>> + >>> +M m; >>> + >>> +struct E { >>> + E(); >>> + E foo(); >>> + E* operator->(); >>> +}; >>> + >>> +struct F { >>> + F(E); >>> +}; >>> + >>> +struct G { >>> + F f; >>> + E e; >>> + G() : f{ e->foo() }, e() { } // { dg-warning "field .G::e. is used >>> uninitialized" } >>> +}; >>> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-18.C >>> b/gcc/testsuite/g++.dg/warn/Wuninitialized-18.C >>> new file mode 100644 >>> index 00000000000..c05ad42f95e >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-18.C >>> @@ -0,0 +1,22 @@ >>> +// PR c++/96121 >>> +// { dg-do compile { target c++11 } } >>> +// { dg-options "-Wuninitialized" } >>> + >>> +struct A { >>> + A(); >>> + int i; >>> +}; >>> +struct B { >>> + B(A); >>> + int i; >>> +}; >>> + >>> +struct composed2 { >>> + B b_; >>> + A a_; >>> + composed2() : b_(a_) {} // { dg-warning "field .composed2::a_. is >>> used uninitialized" } >>> +}; >>> + >>> +composed2 test() { >>> + return composed2{}; >>> +} >>> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-19.C >>> b/gcc/testsuite/g++.dg/warn/Wuninitialized-19.C >>> new file mode 100644 >>> index 00000000000..c401f8636bf >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-19.C >>> @@ -0,0 +1,50 @@ >>> +// PR c++/19808 >>> +// { dg-do compile { target c++11 } } >>> +// { dg-options "-Wuninitialized" } >>> +// Test we warn when initializing a base class. >>> + >>> +struct A { >>> + A(int) { } >>> +}; >>> + >>> +struct B : public A { >>> + int x; >>> + B() : A(x) { } // { dg-warning "field .B::x. is used uninitialized" } >>> +}; >>> + >>> +struct C : public A { >>> + int x; >>> + int y; >>> + C() : A(y = 4), x(y) { } >>> +}; >>> + >>> +struct D : public A { >>> + int x; >>> + D() : A{x} { } // { dg-warning "field .D::x. is used uninitialized" } >>> +}; >>> + >>> +struct E : public A { >>> + int x; >>> + int y; >>> + E() : A{y = 4}, x(y) { } >>> +}; >>> + >>> +struct F { >>> + F(int&) { } >>> +}; >>> + >>> +struct G : F { >>> + int x; >>> + G() : F(x) { } >>> +}; >>> + >>> +struct H { >>> + H(int *) { } >>> +}; >>> + >>> +struct I : H { >>> + int x; >>> + int arr[2]; >>> + I() : H(&x) { } >>> + I(int) : H(arr) { } >>> +}; >>> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-20.C >>> b/gcc/testsuite/g++.dg/warn/Wuninitialized-20.C >>> new file mode 100644 >>> index 00000000000..9f367f0fdfd >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-20.C >>> @@ -0,0 +1,16 @@ >>> +// PR c++/96121 >>> +// { dg-do compile { target c++11 } } >>> +// { dg-options "-Wuninitialized" } >>> +// Test we warn with delegating constructors. >>> + >>> +struct A { >>> + A(int); >>> + A(int &, int); >>> + A(int (*)[1]) : A(x) { } // { dg-warning "21:field .A::x. is used >>> uninitialized" } >>> + A(int (*)[2]) : A(x, x) { } // { dg-warning "24:field .A::x. is >>> used uninitialized" } >>> + A(int (*)[3]) : A(x, 0) { } >>> + A(int (*)[4]) : A{x} { } // { dg-warning "21:field .A::x. is used >>> uninitialized" } >>> + A(int (*)[5]) : A{x, x} { } // { dg-warning "24:field .A::x. is >>> used uninitialized" } >>> + A(int (*)[6]) : A{x, 0} { } >>> + int x; >>> +}; >>> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-21.C >>> b/gcc/testsuite/g++.dg/warn/Wuninitialized-21.C >>> new file mode 100644 >>> index 00000000000..57ca00ab042 >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-21.C >>> @@ -0,0 +1,20 @@ >>> +// PR c++/19808 >>> +// { dg-do compile } >>> +// { dg-options "-Wuninitialized" } >>> + >>> +struct A { >>> + int a; >>> + int b; >>> + A(int) {} >>> +}; >>> + >>> +struct S { >>> + A a; >>> + A a2; >>> + S() : >>> + /* We don't warn here, because we consider partial initialization >>> + as initializing the whole object. */ >>> + a((a2.a = 42)), >>> + a2(a2.a) >>> + { } >>> +}; >>> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-22.C >>> b/gcc/testsuite/g++.dg/warn/Wuninitialized-22.C >>> new file mode 100644 >>> index 00000000000..bd980d61118 >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-22.C >>> @@ -0,0 +1,37 @@ >>> +// PR c++/19808 >>> +// { dg-do compile { target c++11 } } >>> +// { dg-options "-Wuninitialized -Winit-self" } >>> +// Test that we don't warn when initializing a reference, unless it's >>> +// self-init. >>> + >>> +struct R { >>> + int &r; >>> +}; >>> + >>> +struct S { >>> + R r; >>> + int a; >>> + int &b; >>> + int c; >>> +}; >>> + >>> +struct X { >>> + S s; >>> + X() : s{ { s.a }, 1, s.c, 3} { } >>> +}; >>> + >>> +struct A { >>> + int &r; >>> + A() : r{r} { } // { dg-warning ".A::r. is initialized with itself" } >>> +}; >>> + >>> +struct B { >>> + int &r; >>> + int a; >>> + B() : r{a} { } >>> +}; >>> + >>> +struct C { >>> + R x; >>> + C() : x{x.r} { } // { dg-warning ".R::r. is initialized with >>> itself" } >>> +}; >>> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-23.C >>> b/gcc/testsuite/g++.dg/warn/Wuninitialized-23.C >>> new file mode 100644 >>> index 00000000000..db3778300be >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-23.C >>> @@ -0,0 +1,24 @@ >>> +// PR c++/19808 >>> +// { dg-do compile { target c++11 } } >>> +// { dg-options "-Wuninitialized" } >>> +// Test that we don't warn in an uninstantiated template. >>> + >>> +struct A { >>> + int *fn() { return nullptr; } >>> +}; >>> + >>> +template<typename T> >>> +struct B { >>> + B() : p(a->fn()) { } >>> + A *a; >>> + int *p; >>> +}; >>> + >>> +template<typename T> >>> +struct C { >>> + C() : p(a->fn()) { } // { dg-warning "field .C<int>::a. is used >>> uninitialized" } >>> + A *a; >>> + int *p; >>> +}; >>> + >>> +C<int> c; >>> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-24.C >>> b/gcc/testsuite/g++.dg/warn/Wuninitialized-24.C >>> new file mode 100644 >>> index 00000000000..c0a88631fb4 >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-24.C >>> @@ -0,0 +1,14 @@ >>> +// PR c++/19808 >>> +// { dg-do compile { target c++11 } } >>> +// { dg-options "-Wuninitialized" } >>> + >>> +struct S { int x, y; }; >>> + >>> +struct A { >>> + S a1[2]; >>> + int i; >>> + A(int (*)[1]) : a1{{1}} { } >>> + A(int (*)[2]) : a1{ {1, 2} } { } >>> + A(int (*)[3]) : a1{{i}} { } // { dg-warning "field .A::i. is used >>> uninitialized" } >>> + A(int (*)[4]) : a1{ {1, i} } { } // { dg-warning "field .A::i. is >>> used uninitialized" } >>> +}; >>> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-25.C >>> b/gcc/testsuite/g++.dg/warn/Wuninitialized-25.C >>> new file mode 100644 >>> index 00000000000..fb652f989a4 >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-25.C >>> @@ -0,0 +1,12 @@ >>> +// PR c++/19808 >>> +// { dg-do compile { target c++11 } } >>> +// { dg-options "-Wall" } >>> + >>> +struct A { >>> + A *a; >>> +}; >>> + >>> +struct B : A { >>> + int i; >>> + B() : A{a} {} >>> +}; >>> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-26.C >>> b/gcc/testsuite/g++.dg/warn/Wuninitialized-26.C >>> new file mode 100644 >>> index 00000000000..a887d12e9f9 >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-26.C >>> @@ -0,0 +1,22 @@ >>> +// PR c++/19808 >>> +// { dg-do compile { target c++11 } } >>> +// { dg-options "-Wuninitialized" } >>> +// Anonymous union/struct. >>> +// ??? The diagnostic should be improved to say 'b' instead of >>> +// "<anonymous>". >>> + >>> +struct S { >>> + __extension__ struct { >>> + int a; >>> + int b; >>> + }; >>> + S() : a(b) { } // { dg-warning "is used uninitialized" } >>> +}; >>> + >>> +struct U { >>> + union { >>> + int a; >>> + int b; >>> + }; >>> + U() : a(b) { } // { dg-warning "is used uninitialized" } >>> +}; >>> diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-27.C >>> b/gcc/testsuite/g++.dg/warn/Wuninitialized-27.C >>> new file mode 100644 >>> index 00000000000..24e6b9b5b48 >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-27.C >>> @@ -0,0 +1,20 @@ >>> +// PR c++/19808 >>> +// { dg-do compile } >>> +// { dg-options "-Wall" } >>> + >>> +enum E { red }; >>> + >>> +struct C { >>> + C(int *, unsigned); >>> +}; >>> + >>> +template <unsigned U> struct D : C { >>> + D(int, int, E) : C(e, U) {} >>> + int e[2]; >>> +}; >>> + >>> +void >>> +g () >>> +{ >>> + D<1>(0, 0, red); >>> +} >>> diff --git a/gcc/tree.c b/gcc/tree.c >>> index 1ad4ad5a5f7..1ad884120b0 100644 >>> --- a/gcc/tree.c >>> +++ b/gcc/tree.c >>> @@ -4638,6 +4638,7 @@ stabilize_reference (tree ref) >>> TREE_READONLY (result) = TREE_READONLY (ref); >>> TREE_SIDE_EFFECTS (result) = TREE_SIDE_EFFECTS (ref); >>> TREE_THIS_VOLATILE (result) = TREE_THIS_VOLATILE (ref); >>> + protected_set_expr_location (result, EXPR_LOCATION (ref)); >>> return result; >>> } >>> >>> base-commit: 6403e02510fe84c72c515247a9dfdb6e05d977fe >>> >> >
On 11/16/20 10:13 PM, Martin Sebor wrote: > On 11/16/20 12:02 PM, Jason Merrill wrote: >> On 11/16/20 12:42 PM, Martin Sebor wrote: >>> On 11/15/20 3:44 PM, Marek Polacek via Gcc-patches wrote: >>>> This patch implements the long-desired -Wuninitialized warning for >>>> member initializer lists, so that the front end can detect bugs like >>>> >>>> struct A { >>>> int a; >>>> int b; >>>> A() : b(1), a(b) { } >>>> }; >>>> >>>> where the field 'b' is used uninitialized because the order of member >>>> initializers in the member initializer list is irrelevant; what matters >>>> is the order of declarations in the class definition. >>>> >>>> I've implemented this by keeping a hash set holding fields that are not >>>> initialized yet, so at first it will be {a, b}, and after initializing >>>> 'a' it will be {b} and so on. Then I use walk_tree to walk the >>>> initializer and if we see that an uninitialized object is used, we >>>> warn. >>>> Of course, when we use the address of the object, we may not warn: >>>> >>>> struct B { >>>> int &r; >>>> int *p; >>>> int a; >>>> B() : r(a), p(&a), a(1) { } // ok >>>> }; >>>> >>>> Likewise, don't warn in unevaluated contexts such as sizeof. Classes >>>> without an explicit initializer may still be initialized by their >>>> default constructors; whether or not something is considered >>>> initialized >>>> is handled in perform_member_init, see member_initialized_p. It is >>>> possible that we might want to tweak that if the current approach turns >>>> out to be flawed. >>>> >>>> And this is where things stop being simple. I had to implement certain >>>> gymnastics to handle cases like >>>> >>>> struct S { >>>> int a, b, c; >>>> S() : a((b = 42)), c(b) { } >>>> }; >>>> >>>> where the initializer for 'a' surreptitiously initializes 'b'. And >>>> since >>>> 'b' isn't read in the assignment, we can't warn that it's being used >>>> uninitialized. >>>> >>>> Also, initializer lists. Consider: >>>> >>>> struct A { >>>> int x, y, z; >>>> }; >>>> >>>> struct S { >>>> A a; >>>> S() : a{ 1, 2, a.y } {} // #1 >>>> S(int) : a{ a.y, 2, 3 } {} // #2 >>>> }; >>>> >>>> In #1, a.y refers to an initialized element, so we oughtn't warn. But >>>> in #2, a.y refers to an uninitialized element, so we should warn. To >>>> that end I've added a vector that tracks which elements of an >>>> initializer >>>> list are already initialized, so that when we encounter them later in >>>> the list, we don't warn. This is the find_uninit_data->list_inits >>>> thing. It will contain paths to the initialized elements, so e.g. >>>> >>>> ((S *)this)->a.y >>>> >>>> These are edge cases, but some developers live on the edge. >>>> >>>> This patch also handles delegating constructors and initializing base >>>> classes. >>>> >>>> A couple of TODOs: >>>> - the same approach could be used to warn for NSDMIs, but I think >>>> it's out >>>> of scope for this patch; >>>> - the diagnostic for anonymous unions/structs should be improved >>>> to say the name of the member instead of "<anonymous>"; >>>> - certain uninitialized warnings in initializer-lists are issued >>>> by the FE as well as the ME. It's unclear to me what we want to do >>>> here; >>>> - using uninitialized members of base classes is not implemented yet. >>> >>> This is a great enhancement! Detecting the misues even in inline >>> functions is especially useful. >> >> Why doesn't the middle-end warning work for inline functions? > > It does but only when they're called (and, as usual, also unless > the uninitialized use is eliminated). Yes, but why? I assume because we don't bother going through all the phases of compilation for unused inlines, but couldn't we change that when we're asking for (certain) warnings? Jason
On Tue, Nov 17, 2020 at 01:33:48AM -0500, Jason Merrill via Gcc-patches wrote: > > > Why doesn't the middle-end warning work for inline functions? > > > > It does but only when they're called (and, as usual, also unless > > the uninitialized use is eliminated). > > Yes, but why? I assume because we don't bother going through all the phases > of compilation for unused inlines, but couldn't we change that when we're > asking for (certain) warnings? CCing Richard and Honza on this. I think for unused functions we don't even gimplify unused functions, the cgraph code just throws them away. Even trying just to run the first few passes (gimplification up to uninit1) would have several high costs, the tree body of everything unneeded would be unshared, reemitted as GIMPLE, then cfg created for it and turned into SSA form only to get the uninitialized warnings (for that warning we could stop there and throw the bodies away). I think we would need to try that on some larger C++ codebases and see what the compile time memory and time hit would be. Anyway, taking e.g. Marek's Wuninitialized-17.C testcase, if I add S s; at the end to make the S::S() ctor used, uninit1 does warn (guess many of the warnings would be dependent on -flifetime-dse=2 inserted CLOBBERs, thankfully that is the default), but the warnings then look fairly cryptic: Wuninitialized-17.C: In constructor ‘S::S()’: Wuninitialized-17.C:22:14: warning: ‘*<unknown>.S::y’ is used uninitialized [-Wuninitialized] 22 | S() : a{1, y} { } // { dg-warning "field .S::y. is used uninitialized" } | ^ Guess we should at least try to improve this special case's printing for C++, the IL is: *this_3(D) ={v} {CLOBBER}; this_3(D)->a.a = 1; _1 = this_3(D)->y; this_3(D)->a.b = _1; and we could figure out that this is in a METHOD_TYPE fndecl, the MEM_REF has as address the default def of this SSA_NAME where this is the first argument of the method and the PARM_DECL has this name and is DECL_ARTIFICIAL to print it as this->S::y instead of *<unknown>.S::y. I bet that code is in error.c ... If the costs of gimplifying, creating cfg and SSA form for all functions would be too high (I think it would be, but haven't measured it), then perphaps it might be useful for Marek's FE code to handle the easiest cases and punt when it gets more complicated? I mean e.g. the a(b=1) cases, or can't e.g. some member be initialized only in some other function - a (foo (&b)) where foo would store to what the pointer points to (or reference refers to)? Jakub
> On Tue, Nov 17, 2020 at 01:33:48AM -0500, Jason Merrill via Gcc-patches wrote: > > > > Why doesn't the middle-end warning work for inline functions? > > > > > > It does but only when they're called (and, as usual, also unless > > > the uninitialized use is eliminated). > > > > Yes, but why? I assume because we don't bother going through all the phases > > of compilation for unused inlines, but couldn't we change that when we're > > asking for (certain) warnings? > > CCing Richard and Honza on this. > > I think for unused functions we don't even gimplify unused functions, the > cgraph code just throws them away. Even trying just to run the first few > passes (gimplification up to uninit1) would have several high costs, Note that uninit1 is a late pass so it is not just few passes we speak about. Late passes are run only on cocde that really lands in .s file so enabling them would mean splitting the pass queue and running another unreachable code somewhere. That would confuse inliner and other IPA passes since they will have to somehow deal with dead code in their program size estimate and also affect LTO. Even early passes are run only on reachable portion of program, since functions are analyzed by cgraphunit on demand (only if they are analyzed by someone else). Simlar logic is also done be C++ FE to decide what templates. Changling this would also have quite some compile time/memory use impact. There is -fkeep-inline-functions. > the tree body of everything unneeded would be unshared, reemitted as GIMPLE, > then cfg created for it and turned into SSA form only to get the > uninitialized warnings (for that warning we could stop there and throw the > bodies away). > I think we would need to try that on some larger C++ codebases and see what > the compile time memory and time hit would be. > > Anyway, taking e.g. Marek's Wuninitialized-17.C testcase, if I add > S s; > at the end to make the S::S() ctor used, uninit1 does warn > (guess many of the warnings would be dependent on -flifetime-dse=2 > inserted CLOBBERs, thankfully that is the default), but the warnings then > look fairly cryptic: > Wuninitialized-17.C: In constructor ‘S::S()’: > Wuninitialized-17.C:22:14: warning: ‘*<unknown>.S::y’ is used uninitialized [-Wuninitialized] > 22 | S() : a{1, y} { } // { dg-warning "field .S::y. is used uninitialized" } > | ^ > Guess we should at least try to improve this special case's printing for C++, the IL > is: > *this_3(D) ={v} {CLOBBER}; > this_3(D)->a.a = 1; > _1 = this_3(D)->y; > this_3(D)->a.b = _1; > and we could figure out that this is in a METHOD_TYPE fndecl, the MEM_REF > has as address the default def of this SSA_NAME where this is the first > argument of the method and the PARM_DECL has this name and is > DECL_ARTIFICIAL to print it as this->S::y instead of *<unknown>.S::y. > I bet that code is in error.c ... > > If the costs of gimplifying, creating cfg and SSA form for all functions > would be too high (I think it would be, but haven't measured it), then > perphaps it might be useful for Marek's FE code to handle the easiest cases > and punt when it gets more complicated? > I mean e.g. the a(b=1) cases, or can't e.g. some member be initialized only > in some other function - a (foo (&b)) where foo would store to what the > pointer points to (or reference refers to)? > > Jakub >
On Tue, Nov 17, 2020 at 09:44:26AM +0100, Jan Hubicka wrote: > > I think for unused functions we don't even gimplify unused functions, the > > cgraph code just throws them away. Even trying just to run the first few > > passes (gimplification up to uninit1) would have several high costs, > Note that uninit1 is a late pass so it is not just few passes we speak You are speaking about uninit2? The one I'm talking about is: PUSH_INSERT_PASSES_WITHIN (pass_build_ssa_passes) NEXT_PASS (pass_fixup_cfg); NEXT_PASS (pass_build_ssa); NEXT_PASS (pass_warn_nonnull_compare); NEXT_PASS (pass_early_warn_uninitialized); and that is run right after 023t.ssa Jakub
> On Tue, Nov 17, 2020 at 09:44:26AM +0100, Jan Hubicka wrote: > > > I think for unused functions we don't even gimplify unused functions, the > > > cgraph code just throws them away. Even trying just to run the first few > > > passes (gimplification up to uninit1) would have several high costs, > > Note that uninit1 is a late pass so it is not just few passes we speak > > You are speaking about uninit2? > The one I'm talking about is: > PUSH_INSERT_PASSES_WITHIN (pass_build_ssa_passes) > NEXT_PASS (pass_fixup_cfg); > NEXT_PASS (pass_build_ssa); > NEXT_PASS (pass_warn_nonnull_compare); > NEXT_PASS (pass_early_warn_uninitialized); > and that is run right after 023t.ssa that one is called "*early_warn_uninitialized" and thus have no dump file. One with comes with uninit1 dump is the late pass. Honza > > Jakub >
On Tue, 17 Nov 2020, Jan Hubicka wrote: > > On Tue, Nov 17, 2020 at 01:33:48AM -0500, Jason Merrill via Gcc-patches wrote: > > > > > Why doesn't the middle-end warning work for inline functions? > > > > > > > > It does but only when they're called (and, as usual, also unless > > > > the uninitialized use is eliminated). > > > > > > Yes, but why? I assume because we don't bother going through all the phases > > > of compilation for unused inlines, but couldn't we change that when we're > > > asking for (certain) warnings? > > > > CCing Richard and Honza on this. > > > > I think for unused functions we don't even gimplify unused functions, the > > cgraph code just throws them away. Even trying just to run the first few > > passes (gimplification up to uninit1) would have several high costs, > Note that uninit1 is a late pass so it is not just few passes we speak > about. Late passes are run only on cocde that really lands in .s file > so enabling them would mean splitting the pass queue and running another > unreachable code somewhere. That would confuse inliner and other IPA > passes since they will have to somehow deal with dead code in their > program size estimate and also affect LTO. > > Even early passes are run only on reachable portion of program, since > functions are analyzed by cgraphunit on demand (only if they are > analyzed by someone else). Simlar logic is also done be C++ FE to decide > what templates. Changling this would also have quite some compile > time/memory use impact. There's also impact on various IPA heuristics - we'd need to make sure to not account any doings to unreachable functions or effects of them still there (do we want to inline into them? do we want heuristics of inline into all functions affected?) So IMHO not a good idea. GCC isn't a static analysis engine ... Richard. > There is -fkeep-inline-functions. > > > the tree body of everything unneeded would be unshared, reemitted as GIMPLE, > > then cfg created for it and turned into SSA form only to get the > > uninitialized warnings (for that warning we could stop there and throw the > > bodies away). > > I think we would need to try that on some larger C++ codebases and see what > > the compile time memory and time hit would be. > > > > Anyway, taking e.g. Marek's Wuninitialized-17.C testcase, if I add > > S s; > > at the end to make the S::S() ctor used, uninit1 does warn > > (guess many of the warnings would be dependent on -flifetime-dse=2 > > inserted CLOBBERs, thankfully that is the default), but the warnings then > > look fairly cryptic: > > Wuninitialized-17.C: In constructor ?S::S()?: > > Wuninitialized-17.C:22:14: warning: ?*<unknown>.S::y? is used uninitialized [-Wuninitialized] > > 22 | S() : a{1, y} { } // { dg-warning "field .S::y. is used uninitialized" } > > | ^ > > Guess we should at least try to improve this special case's printing for C++, the IL > > is: > > *this_3(D) ={v} {CLOBBER}; > > this_3(D)->a.a = 1; > > _1 = this_3(D)->y; > > this_3(D)->a.b = _1; > > and we could figure out that this is in a METHOD_TYPE fndecl, the MEM_REF > > has as address the default def of this SSA_NAME where this is the first > > argument of the method and the PARM_DECL has this name and is > > DECL_ARTIFICIAL to print it as this->S::y instead of *<unknown>.S::y. > > I bet that code is in error.c ... > > > > If the costs of gimplifying, creating cfg and SSA form for all functions > > would be too high (I think it would be, but haven't measured it), then > > perphaps it might be useful for Marek's FE code to handle the easiest cases > > and punt when it gets more complicated? > > I mean e.g. the a(b=1) cases, or can't e.g. some member be initialized only > > in some other function - a (foo (&b)) where foo would store to what the > > pointer points to (or reference refers to)? > > > > Jakub > > >
On 11/17/20 3:44 AM, Jan Hubicka wrote: >> On Tue, Nov 17, 2020 at 01:33:48AM -0500, Jason Merrill via Gcc-patches wrote: >>>>> Why doesn't the middle-end warning work for inline functions? >>>> >>>> It does but only when they're called (and, as usual, also unless >>>> the uninitialized use is eliminated). >>> >>> Yes, but why? I assume because we don't bother going through all the phases >>> of compilation for unused inlines, but couldn't we change that when we're >>> asking for (certain) warnings? >> >> CCing Richard and Honza on this. >> >> I think for unused functions we don't even gimplify unused functions, the >> cgraph code just throws them away. Even trying just to run the first few >> passes (gimplification up to uninit1) would have several high costs, > Note that uninit1 is a late pass so it is not just few passes we speak > about. Late passes are run only on cocde that really lands in .s file > so enabling them would mean splitting the pass queue and running another > unreachable code somewhere. That would confuse inliner and other IPA > passes since they will have to somehow deal with dead code in their > program size estimate and also affect LTO. > > Even early passes are run only on reachable portion of program, since > functions are analyzed by cgraphunit on demand (only if they are > analyzed by someone else). Simlar logic is also done be C++ FE to decide > what templates. Changling this would also have quite some compile > time/memory use impact. > > There is -fkeep-inline-functions. OK, thanks for the explanation. -fkeep-inline-functions seems like an acceptable answer for people who want a warning audit of their library header inlines. Martin, I notice that the middle-end warning doesn't currently catch this: struct B { int i,j; }; struct A { B b; A(): b({b.i}) { } }; A a; It does warn if B only has one member; adding the second wrongly silences the warning. Jason
On 11/19/20 11:41 AM, Jason Merrill wrote: > On 11/17/20 3:44 AM, Jan Hubicka wrote: >>> On Tue, Nov 17, 2020 at 01:33:48AM -0500, Jason Merrill via >>> Gcc-patches wrote: >>>>>> Why doesn't the middle-end warning work for inline functions? >>>>> >>>>> It does but only when they're called (and, as usual, also unless >>>>> the uninitialized use is eliminated). >>>> >>>> Yes, but why? I assume because we don't bother going through all >>>> the phases >>>> of compilation for unused inlines, but couldn't we change that when >>>> we're >>>> asking for (certain) warnings? >>> >>> CCing Richard and Honza on this. >>> >>> I think for unused functions we don't even gimplify unused functions, >>> the >>> cgraph code just throws them away. Even trying just to run the first >>> few >>> passes (gimplification up to uninit1) would have several high costs, >> Note that uninit1 is a late pass so it is not just few passes we speak >> about. Late passes are run only on cocde that really lands in .s file >> so enabling them would mean splitting the pass queue and running another >> unreachable code somewhere. That would confuse inliner and other IPA >> passes since they will have to somehow deal with dead code in their >> program size estimate and also affect LTO. >> >> Even early passes are run only on reachable portion of program, since >> functions are analyzed by cgraphunit on demand (only if they are >> analyzed by someone else). Simlar logic is also done be C++ FE to decide >> what templates. Changling this would also have quite some compile >> time/memory use impact. >> >> There is -fkeep-inline-functions. > > OK, thanks for the explanation. -fkeep-inline-functions seems like an > acceptable answer for people who want a warning audit of their library > header inlines. > > Martin, I notice that the middle-end warning doesn't currently catch this: > > struct B { int i,j; }; > > struct A > { > B b; > A(): b({b.i}) { } > }; > > A a; > > It does warn if B only has one member; adding the second wrongly > silences the warning. The GIMPLE for A's ctor looks like this: A::A (struct A * const this) { *this = {CLOBBER}; { this->b = {}; _1 = this->b.i; this->b.i = _1; } } so the b member is cleared first before it's read from and there's nothing to warn about. This happens for C structs too: void f (void*); struct A { int i, j; }; void g (void) { struct A a = { a.i }; f (&a); } The uninitialized read is in the original dump (below) so the zero initialization happens in the gimplifier and could be diagnosed there. ;; enabled by -tree-original { struct A a = {.i=a.i}; struct A a = {.i=a.i}; f ((void *) &a); } Martin PS I think the C++ original dump (below) shows the same problem despite the Unknown trees that obscure it: ;; Function A::A() (null) ;; enabled by -tree-original <<cleanup_point <<< Unknown tree: expr_stmt *(struct A *) this = {CLOBBER} >>>>>; { <<cleanup_point <<< Unknown tree: expr_stmt (void) (((struct A *) this)->b = TARGET_EXPR <D.2389, {.i=((struct A *) this)->b.i}>) >>>>>; }
On 11/20/20 6:08 PM, Martin Sebor wrote: > On 11/19/20 11:41 AM, Jason Merrill wrote: >> On 11/17/20 3:44 AM, Jan Hubicka wrote: >>>> On Tue, Nov 17, 2020 at 01:33:48AM -0500, Jason Merrill via >>>> Gcc-patches wrote: >>>>>>> Why doesn't the middle-end warning work for inline functions? >>>>>> >>>>>> It does but only when they're called (and, as usual, also unless >>>>>> the uninitialized use is eliminated). >>>>> >>>>> Yes, but why? I assume because we don't bother going through all >>>>> the phases >>>>> of compilation for unused inlines, but couldn't we change that when >>>>> we're >>>>> asking for (certain) warnings? >>>> >>>> CCing Richard and Honza on this. >>>> >>>> I think for unused functions we don't even gimplify unused >>>> functions, the >>>> cgraph code just throws them away. Even trying just to run the >>>> first few >>>> passes (gimplification up to uninit1) would have several high costs, >>> Note that uninit1 is a late pass so it is not just few passes we speak >>> about. Late passes are run only on cocde that really lands in .s file >>> so enabling them would mean splitting the pass queue and running another >>> unreachable code somewhere. That would confuse inliner and other IPA >>> passes since they will have to somehow deal with dead code in their >>> program size estimate and also affect LTO. >>> >>> Even early passes are run only on reachable portion of program, since >>> functions are analyzed by cgraphunit on demand (only if they are >>> analyzed by someone else). Simlar logic is also done be C++ FE to decide >>> what templates. Changling this would also have quite some compile >>> time/memory use impact. >>> >>> There is -fkeep-inline-functions. >> >> OK, thanks for the explanation. -fkeep-inline-functions seems like an >> acceptable answer for people who want a warning audit of their library >> header inlines. >> >> Martin, I notice that the middle-end warning doesn't currently catch >> this: >> >> struct B { int i,j; }; >> >> struct A >> { >> B b; >> A(): b({b.i}) { } >> }; >> >> A a; >> >> It does warn if B only has one member; adding the second wrongly >> silences the warning. > > The GIMPLE for A's ctor looks like this: > > A::A (struct A * const this) > { > *this = {CLOBBER}; > { > this->b = {}; > _1 = this->b.i; > this->b.i = _1; > } > } > > so the b member is cleared first before it's read from and there's > nothing to warn about. This happens for C structs too: > > void f (void*); > > struct A { int i, j; }; > > void g (void) > { > struct A a = { a.i }; > f (&a); > } > > The uninitialized read is in the original dump (below) so the zero > initialization happens in the gimplifier and could be diagnosed > there. > > ;; enabled by -tree-original > > > { > struct A a = {.i=a.i}; > > struct A a = {.i=a.i}; > f ((void *) &a); > } Ah, yes, because gimplify_init_constructor leaves the first assignment to clear the whole object if there aren't initializers for all the members. Jason
diff --git a/gcc/cp/init.c b/gcc/cp/init.c index ffb84ea5b09..92dcdc7bc02 100644 --- a/gcc/cp/init.c +++ b/gcc/cp/init.c @@ -41,7 +41,6 @@ static tree finish_init_stmts (bool, tree, tree); static void construct_virtual_base (tree, tree); static void expand_aggr_init_1 (tree, tree, tree, tree, int, tsubst_flags_t); static void expand_default_init (tree, tree, tree, tree, int, tsubst_flags_t); -static void perform_member_init (tree, tree); static int member_init_ok_or_else (tree, tree, tree); static void expand_virtual_init (tree, tree); static tree sort_mem_initializers (tree, tree); @@ -521,19 +520,19 @@ build_value_init_noctor (tree type, tsubst_flags_t complain) return build_zero_init (type, NULL_TREE, /*static_storage_p=*/false); } -/* Initialize current class with INIT, a TREE_LIST of - arguments for a target constructor. If TREE_LIST is void_type_node, - an empty initializer list was given. */ +/* Initialize current class with INIT, a TREE_LIST of arguments for + a target constructor. If TREE_LIST is void_type_node, an empty + initializer list was given. Return the target constructor. */ -static void +static tree perform_target_ctor (tree init) { tree decl = current_class_ref; tree type = current_class_type; - finish_expr_stmt (build_aggr_init (decl, init, - LOOKUP_NORMAL|LOOKUP_DELEGATING_CONS, - tf_warning_or_error)); + init = build_aggr_init (decl, init, LOOKUP_NORMAL|LOOKUP_DELEGATING_CONS, + tf_warning_or_error); + finish_expr_stmt (init); if (type_build_dtor_call (type)) { tree expr = build_delete (input_location, @@ -546,6 +545,7 @@ perform_target_ctor (tree init) && TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type)) finish_eh_cleanup (expr); } + return init; } /* Return the non-static data initializer for FIELD_DECL MEMBER. */ @@ -757,12 +757,175 @@ maybe_warn_list_ctor (tree member, tree init) "of the underlying array", member, begin); } +/* Data structure for find_uninit_fields_r, below. */ + +struct find_uninit_data { + /* The set tracking the yet-uninitialized members. */ + hash_set<tree> *uninitialized; + /* The data member we are currently initializing. It can be either + a type (initializing a base class/delegating constructors), or + a COMPONENT_REF. */ + tree member; + /* A vector to track initialized fields in an initializer-list. */ + vec<tree, va_gc> *list_inits; +}; + +/* walk_tree callback that warns about using uninitialized data in + a member-initializer-list. */ + +static tree +find_uninit_fields_r (tree *tp, int *walk_subtrees, void *data) +{ + find_uninit_data *d = static_cast<find_uninit_data *>(data); + hash_set<tree> *uninitialized = d->uninitialized; + tree init = *tp; + + /* No need to look into types. */ + if (TYPE_P (init)) + { + *walk_subtrees = false; + return NULL_TREE; + } + + switch (TREE_CODE (init)) + { + /* If we're just taking the address of an object, it doesn't matter + if it's been initialized. */ + case ADDR_EXPR: + /* Unevaluated operands. */ + case ALIGNOF_EXPR: + case SIZEOF_EXPR: + case NOEXCEPT_EXPR: + *walk_subtrees = false; + return NULL_TREE; + default: + break; + } + + /* Handle cases like S() : a((b = 42)), c(b) { } where the initializer + for a surreptitiously initializes b. */ + bool write_p = false; + if (TREE_CODE (init) == MODIFY_EXPR) + { + /* First recurse on the RHS to detect (b += 10). */ + cp_walk_tree_without_duplicates (&TREE_OPERAND (init, 1), + find_uninit_fields_r, data); + init = TREE_OPERAND (init, 0); + /* Don't get tricked by a(((c, b) = 42)). */ + while (TREE_CODE (init) == COMPOUND_EXPR) + init = TREE_OPERAND (init, 1); + /* The LHS might be something like this->a.b. Get this->a, so that + we can remove the member 'a' from the uninitialized set. We treat + even partial initialization as initializing the whole object. */ + while (TREE_CODE (init) == COMPONENT_REF) + if (TREE_CODE (TREE_OPERAND (init, 0)) == COMPONENT_REF) + init = TREE_OPERAND (init, 0); + else + break; + write_p = true; + } + /* Warn about uninitialized 'this'. */ + else if (TREE_CODE (init) == CALL_EXPR) + { + tree fn = get_callee_fndecl (init); + if (fn && DECL_NONSTATIC_MEMBER_FUNCTION_P (fn)) + { + tree op = CALL_EXPR_ARG (init, 0); + if (TREE_CODE (op) == ADDR_EXPR) + op = TREE_OPERAND (op, 0); + temp_override<tree> ovr (d->member, DECL_ARGUMENTS (fn)); + cp_walk_tree_without_duplicates (&op, find_uninit_fields_r, data); + } + } + else if (BRACE_ENCLOSED_INITIALIZER_P (init)) + { + tree ctor = init; + if (!d->list_inits) + { + /* Reshape the outermost { }; this will fill the .index fields. */ + tree type = TYPE_P (d->member) ? d->member : TREE_TYPE (d->member); + ctor = reshape_init (type, ctor, tf_none); + d->list_inits = make_tree_vector (); + } + if (TREE_CODE (ctor) == CONSTRUCTOR && CONSTRUCTOR_NELTS (ctor) > 0) + { + for (constructor_elt &elt : *CONSTRUCTOR_ELTS (ctor)) + { + temp_override<tree> ovr (d->member); + /* Build up the path to the element we're initializing, + if we're initializing a class member. */ + if (TREE_CODE (elt.index) == FIELD_DECL + && TREE_CODE (d->member) == COMPONENT_REF) + d->member = build_class_member_access_expr (d->member, + elt.index, + NULL_TREE, + /*ref=*/true, + tf_none); + cp_walk_tree_without_duplicates (&elt.value, + find_uninit_fields_r, data); + vec_safe_push (d->list_inits, d->member); + } + /* We've processed this initializer-list, clean up. */ + release_tree_vector (d->list_inits); + d->list_inits = nullptr; + *walk_subtrees = false; + return NULL_TREE; + } + /* Reshaping might have unwrapped { X } to X; carry on. */ + } + + /* If we find FIELD in the uninitialized set and we're not writing into + it, we warn. */ + if (TREE_CODE (init) == COMPONENT_REF) + { + unsigned int ix; + tree t; + FOR_EACH_VEC_SAFE_ELT (d->list_inits, ix, t) + if (cp_tree_equal (init, t)) + { + /* This field was already initialized by a prior element in the + initializer-list. */ + *walk_subtrees = false; + return NULL_TREE; + } + + tree field = TREE_OPERAND (init, 1); + tree type = TYPE_P (d->member) ? d->member : TREE_TYPE (d->member); + + /* We're initializing a reference member with itself. */ + if (TYPE_REF_P (type) && cp_tree_equal (d->member, init)) + warning_at (EXPR_LOCATION (init), OPT_Winit_self, + "%qD is initialized with itself", field); + else if (cp_tree_equal (TREE_OPERAND (init, 0), current_class_ref) + && uninitialized->contains (field)) + { + /* If we're not reading but writing into a member in this initializer + (this is the LHS of an assignment), remember not to warn about this + member later. */ + if (write_p) + uninitialized->remove (field); + else if (TYPE_REF_P (TREE_TYPE (field))) + warning_at (EXPR_LOCATION (init), OPT_Wuninitialized, + "reference %qD is not yet bound to a value when used " + "here", field); + else if (!INDIRECT_TYPE_P (type) || is_this_parameter (d->member)) + warning_at (EXPR_LOCATION (init), OPT_Wuninitialized, + "field %qD is used uninitialized", field); + /* We can't clear *walk_subtrees here to handle bogosities such as + ((c++, b) = 42). */ + } + } + + return NULL_TREE; +} + /* 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. */ + list was given; if NULL_TREE no initializer was given. UNINITIALIZED + is the hash set that tracks uninitialized fields. */ static void -perform_member_init (tree member, tree init) +perform_member_init (tree member, tree init, hash_set<tree> &uninitialized) { tree decl; tree type = TREE_TYPE (member); @@ -790,7 +953,9 @@ perform_member_init (tree member, tree init) if (decl == error_mark_node) return; - if (warn_init_self && init && TREE_CODE (init) == TREE_LIST + if ((warn_init_self || warn_uninitialized) + && init + && TREE_CODE (init) == TREE_LIST && TREE_CHAIN (init) == NULL_TREE) { tree val = TREE_VALUE (init); @@ -802,6 +967,12 @@ perform_member_init (tree member, tree init) warning_at (DECL_SOURCE_LOCATION (current_function_decl), OPT_Winit_self, "%qD is initialized with itself", member); + else if (!uninitialized.is_empty ()) + { + find_uninit_data data = { &uninitialized, decl, nullptr }; + cp_walk_tree_without_duplicates (&val, find_uninit_fields_r, + &data); + } } if (array_of_unknown_bound_p (type)) @@ -830,6 +1001,9 @@ perform_member_init (tree member, tree init) do aggregate-initialization. */ } + /* Assume we are initializing the member. */ + bool member_initialized_p = true; + if (init == void_type_node) { /* mem() means value-initialization. */ @@ -970,6 +1144,9 @@ perform_member_init (tree member, tree init) diagnose_uninitialized_cst_or_ref_member (core_type, /*using_new=*/false, /*complain=*/true); + + /* We left the member uninitialized. */ + member_initialized_p = false; } maybe_warn_list_ctor (member, init); @@ -980,6 +1157,9 @@ perform_member_init (tree member, tree init) tf_warning_or_error)); } + if (member_initialized_p && warn_uninitialized) + uninitialized.remove (member); + if (type_build_dtor_call (type)) { tree expr; @@ -1293,13 +1473,31 @@ emit_mem_initializers (tree mem_inits) if (!COMPLETE_TYPE_P (current_class_type)) return; + /* Keep a set holding fields that are not initialized. */ + hash_set<tree> uninitialized; + + /* Initially that is all of them. */ + if (warn_uninitialized) + for (tree field = TYPE_FIELDS (current_class_type); field; + field = TREE_CHAIN (field)) + if (TREE_CODE (field) == FIELD_DECL && !DECL_ARTIFICIAL (field)) + uninitialized.add (field); + if (mem_inits && TYPE_P (TREE_PURPOSE (mem_inits)) && same_type_p (TREE_PURPOSE (mem_inits), current_class_type)) { /* Delegating constructor. */ gcc_assert (TREE_CHAIN (mem_inits) == NULL_TREE); - perform_target_ctor (TREE_VALUE (mem_inits)); + tree ctor = perform_target_ctor (TREE_VALUE (mem_inits)); + if (!uninitialized.is_empty ()) + { + find_uninit_data data = { &uninitialized, current_class_type, + nullptr }; + cp_walk_tree_without_duplicates (&ctor, + find_uninit_fields_r, + &data); + } return; } @@ -1360,6 +1558,15 @@ emit_mem_initializers (tree mem_inits) flags, tf_warning_or_error); expand_cleanup_for_base (subobject, NULL_TREE); + if (!uninitialized.is_empty () + && STATEMENT_LIST_TAIL (cur_stmt_list)) + { + tree last = STATEMENT_LIST_TAIL (cur_stmt_list)->stmt; + find_uninit_data data = { &uninitialized, BINFO_TYPE (subobject), + nullptr }; + cp_walk_tree_without_duplicates (&last, find_uninit_fields_r, + &data); + } } else if (!ABSTRACT_CLASS_TYPE_P (current_class_type)) /* C++14 DR1658 Means we do not have to construct vbases of @@ -1387,7 +1594,9 @@ emit_mem_initializers (tree mem_inits) iloc_sentinel ils (EXPR_LOCATION (TREE_TYPE (mem_inits))); perform_member_init (TREE_PURPOSE (mem_inits), - TREE_VALUE (mem_inits)); + TREE_VALUE (mem_inits), + uninitialized); + mem_inits = TREE_CHAIN (mem_inits); } } diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index b3a2c7ce51d..cfd5c7e00a2 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -6688,6 +6688,18 @@ to compute a value that itself is never used, because such computations may be deleted by data flow analysis before the warnings are printed. +In C++, this warning also warns about using uninitialized objects in +member-initializer-lists. For example, GCC warns about @code{b} being +uninitialized in the following snippet: + +@smallexample +struct A @{ + int a; + int b; + A() : a(b) @{ @} +@}; +@end smallexample + @item -Wno-invalid-memory-model @opindex Winvalid-memory-model @opindex Wno-invalid-memory-model diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-12.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-12.C new file mode 100644 index 00000000000..4e51b4b09f2 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-12.C @@ -0,0 +1,59 @@ +// PR c++/19808 +// { dg-do compile { target c++11 } } +// { dg-options "-Wuninitialized" } + +struct S { + int i, j, k, l; + S() : i(j), // { dg-warning "field .S::j. is used uninitialized" } + j(1), + k(l + 1), // { dg-warning "field .S::l. is used uninitialized" } + l(2) { } +}; + +struct A { + int a, b, c; + A() : a(b // { dg-warning "field .A::b. is used uninitialized" } + + c) { } // { dg-warning "field .A::c. is used uninitialized" } +}; + +struct B { + int &r; + int *p; + int a; + B() : r(a), p(&a), a(1) { } +}; + +struct C { + const int &r1, &r2; + C () : r1(r2), // { dg-warning "reference .C::r2. is not yet bound to a value when used here" } + r2(r1) { } +}; + +struct D { + int a = 1; + int b = 2; + D() : a(b + 1), b(a + 1) { } // { dg-warning "field .D::b. is used uninitialized" } +}; + +struct E { + int a = 1; + E() : a(a + 1) { } // { dg-warning "field .E::a. is used uninitialized" } +}; + +struct F { + int a = 1; + int b; + F() : b(a + 1) { } +}; + +struct bar { + bar() {} + bar(bar&) {} +}; + +class foo { + bar first; + bar second; +public: + foo() : first(second) {} // { dg-warning "field .foo::second. is used uninitialized" } +}; diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-13.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-13.C new file mode 100644 index 00000000000..9c631f22fbc --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-13.C @@ -0,0 +1,49 @@ +// PR c++/19808 +// { dg-do compile { target c++11 } } +// { dg-options "-Wuninitialized -Winit-self" } + +int fint(int); +int fintp(int *); +int fintr(int &); +int fintcr(const int &); + +int arr[10]; + +struct S { + int x; + int y; + const int z = 42; + int *p; + + S(int (*)[1]) : x(x) { } // { dg-warning "initialized with itself" } + S(int (*)[2]) : x(x + x) { } // { dg-warning "field .S::x. is used uninitialized" } + S(int (*)[3]) : x(static_cast<int>(y)) { } // { dg-warning "field .S::y. is used uninitialized" } + S(int (*)[4]) : x(static_cast<int>(x)) { } // { dg-warning "field .S::x. is used uninitialized" } + S(int (*)[5]) : x(fint(x)) { } // { dg-warning "field .S::x. is used uninitialized" } + S(int (*)[6]) : x(fint(y)) { } // { dg-warning "field .S::y. is used uninitialized" } + + S(int (*)[7]) : x(sizeof(x)) { } + S(int (*)[8]) : x(sizeof(y)) { } + S(int (*)[9]) : p(&x) { } + S(int (*)[10]) : x(fintp(&y)) { } + S(int (*)[11]) : x(fintr(y)) { } + S(int (*)[12]) : x(fintcr(y)) { } + S(int (*)[26]) : x(((void)(__typeof(y)) 1, 1)) { } + S(int (*)[27]) : x(((void)(decltype(y)) 1, 1)) { } + S(int (*)[28]) : x(__alignof__(y)) { } + S(int (*)[29]) : x(noexcept(y)) { } + + S(int (*)[13]) : x(0), y(x ? y : y) { } // { dg-warning "field .S::y. is used uninitialized" } + S(int (*)[14]) : x(0), y(1 + (x ? y : y)) { } // { dg-warning "field .S::y. is used uninitialized" } + S(int (*)[15]) : x(-y) { } // { dg-warning "field .S::y. is used uninitialized" } + S(int (*)[16]) : x(1 << y) { } // { dg-warning "field .S::y. is used uninitialized" } + S(int (*)[17]) : x(this->y) { } // { dg-warning "field .S::y. is used uninitialized" } + S(int (*)[18]) : x(arr[y]) { } // { dg-warning "field .S::y. is used uninitialized" } + S(int (*)[19]) : x(0), y(x ? x : y) { } // { dg-warning "field .S::y. is used uninitialized" } + S(int (*)[20]) : x(0), y(y ? x : y) { } // { dg-warning "field .S::y. is used uninitialized" } + S(int (*)[21]) : x(0), y(y ? x : x) { } // { dg-warning "field .S::y. is used uninitialized" } + S(int (*)[22]) : x(0), y((fint(y), x)) { } // { dg-warning "field .S::y. is used uninitialized" } + S(int (*)[23]) : x(0), y(x += y) { } // { dg-warning "field .S::y. is used uninitialized" } + S(int (*)[24]) : x(y += 10) { } // { dg-warning "field .S::y. is used uninitialized" } + S(int (*)[25]) : x(y++) { } // { dg-warning "field .S::y. is used uninitialized" } +}; diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-14.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-14.C new file mode 100644 index 00000000000..6bef85e4659 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-14.C @@ -0,0 +1,22 @@ +// PR c++/19808 +// { dg-do compile } +// { dg-options "-Wuninitialized" } + +struct A { + int m; + int get() const { return m; } + + A() { } + A(int) { } + A(const A &) { } + A(A *) { } +}; + +struct S { + A a, b; + + S(int (*)[1]) : a() {} + S(int (*)[2]) : b(a.get()) {} + S(int (*)[3]) : b(a) {} + S(int (*)[4]) : a(&a) {} +}; diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-15.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-15.C new file mode 100644 index 00000000000..ed62758a984 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-15.C @@ -0,0 +1,118 @@ +// PR c++/19808 +// { dg-do compile { target c++11 } } +// { dg-options "-Wuninitialized -Winit-self" } +// Largely copied from clang's test/SemaCXX/uninitialized.cpp. + +int x; +struct U { + U() : b(a) { } + int &a = x; + int &b; +}; + +struct T { + T() : a(b), b(a) { } // { dg-warning "reference .T::b. is not yet bound" } + int &a, &b; +}; + +struct S { + S() : a(a) { } // { dg-warning ".S::a. is initialized with itself" } + int &a; +}; + +struct A { + int a; + int b; + A() { } + A(int (*)[1]) : b(a) { } // { dg-warning ".A::a. is used uninitialized" } + A(int (*)[2]) : a(b) { } // { dg-warning ".A::b. is used uninitialized" } +}; + +struct D { + int a; + int &b; + int &c = a; + int d = b; + D() : b(a) { } +}; + +struct E { + int a; + int get(); + static int num(); + E() { } + E(int) { } +}; + +struct F { + int a; + E e; + int b; + F(int (*)[1]) : a(e.get()) { } // { dg-warning "field .F::e. is used uninitialized" } + F(int (*)[2]) : a(e.num()) { } + F(int (*)[3]) : e(a) { } // { dg-warning "field .F::a. is used uninitialized" } + F(int (*)[4]) : a(4), e(a) { } + F(int (*)[5]) : e(b) { } // { dg-warning "field .F::b. is used uninitialized" } + F(int (*)[6]) : e(b), b(4) { } // { dg-warning "field .F::b. is used uninitialized" } +}; + +struct G { + G(const A&) { }; +}; + +struct H { + A a1; + G g; + A a2; + H() : g(a1) { } + // ??? clang++ doesn't warn here + H(int) : g(a2) { } // { dg-warning "field .H::a2. is used uninitialized" } +}; + +struct I { + I(int *) { } +}; + +struct J : I { + int *a; + int *b; + int c; + J() : I((a = new int(5))), b(a), c(*a) { } +}; + +struct M { }; + +struct N : public M { + int a; + int b; + N() : b(a) { } // { dg-warning "field .N::a. is used uninitialized" } +}; + +struct O { + int x = 42; + int get() { return x; } +}; + +struct P { + O o; + int x = o.get(); + P() : x(o.get()) { } +}; + +struct Q { + int a; + int b; + int &c; + Q() : + a(c = 5), // { dg-warning "reference .Q::c. is not yet bound" } + b(c), // { dg-warning "reference .Q::c. is not yet bound" } + c(a) { } +}; + +struct R { + int a; + int b; + int c; + int d = a + b + c; + R() : a(c = 5), b(c), c(a) { } +}; diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-16.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-16.C new file mode 100644 index 00000000000..bb47a340f18 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-16.C @@ -0,0 +1,12 @@ +// PR c++/19808 +// { dg-do compile { target c++11 } } +// { dg-options "-Wuninitialized" } + +struct S { + int a; + int b; + int c; + S() : a((b = 42)), c(b) { } + S(int) : a(((1, b) = 42)), c(b) { } + S(char) : a(((c++, b) = 42)), c(b) { } // { dg-warning "field .S::c. is used uninitialized" } +}; diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-17.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-17.C new file mode 100644 index 00000000000..f18ed9e2251 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-17.C @@ -0,0 +1,146 @@ +// PR c++/19808 +// { dg-do compile { target c++11 } } +// { dg-options "-Wuninitialized -Winit-self" } +// Test we warn with initializer-lists. + +int num = 5; + +struct A { + int a; + int b; +}; + +struct B { + A a1; + A a2; +}; + +struct S { + A a; + int x; + int y; + S() : a{1, y} { } // { dg-warning "field .S::y. is used uninitialized" } +}; + +struct R { + A a1; + A a2; + A a3; + A a4; + R() : + a1{1, 2}, + a2{a2.a + 2}, // { dg-warning "field .R::a2. is used uninitialized" } + a3{a3.a + 2}, // { dg-warning "field .R::a3. is used uninitialized" } + a4{a4.b + 2} // { dg-warning "field .R::a4. is used uninitialized" } + { } +}; + +struct Q { + B b1; + B b2; + B b3; + B b4; + B b5; + B b6; + B b7; + B b8; + B b9; + B b10; + B b11; + B b12; + B b13; + B b14; + B b15; + B b16; + B b17; + B b18; + B b19; + B b20; + B b21; + B b22; + Q() : + b1{ {}, {} }, + b2{ {}, b2.a1 }, + b3{ b3.a1 }, // { dg-warning "field .Q::b3. is used uninitialized" } + b4{ {}, b4.a2}, // { dg-warning "field .Q::b4. is used uninitialized" } + b5{ b5.a2 }, // { dg-warning "field .Q::b5. is used uninitialized" } + b6{ {b6.a1.a} }, // { dg-warning "field .Q::b6. is used uninitialized" } + b7{ {0, b7.a1.a} }, + b8{ {}, {b8.a1.a} }, + b9{ {}, {0, b9.a1.a} }, + b10{ {b10.a1.b} }, // { dg-warning "field .Q::b10. is used uninitialized" } + b11{ {0, b11.a1.b} }, // { dg-warning "field .Q::b11. is used uninitialized" } + b12{ {}, {b12.a1.b} }, + b13{ {}, {0, b13.a1.b} }, + b14{ {b14.a2.a} }, // { dg-warning "field .Q::b14. is used uninitialized" } + b15{ {0, b15.a2.a} }, // { dg-warning "field .Q::b15. is used uninitialized" } + b16{ {}, {b16.a2.a} }, // { dg-warning "field .Q::b16. is used uninitialized" } + b17{ {}, {0, b17.a2.a} }, + b18{ {b18.a2.b} }, // { dg-warning "field .Q::b18. is used uninitialized" } + b19{ {0, b19.a2.b} }, // { dg-warning "field .Q::b19. is used uninitialized" } + b20{ {}, {b20.a2.b} }, // { dg-warning "field .Q::b20. is used uninitialized" } + b21{ {}, {0, b21.a2.b} }, // { dg-warning "field .Q::b21. is used uninitialized" } + b22{ {b18.a2.b + 5} } + { } +}; + +struct C { + int a; + int &b; + int c; +}; + +struct P { + C c1; + C c2; + C c3; + C c4; + C c5; + C c6; + C c7; + P() : + c1{ 0, num, 0 }, + c2{ 1, num, c2.b }, + c3{ c3.b, num }, // { dg-warning "field .P::c3. is used uninitialized" } + c4{ 0, c4.b, 0 }, // { dg-warning ".C::b. is initialized with itself" } + /* c5.c is binding to a reference, don't warn. */ + c5{ 0, c5.c, 0 }, + c6{ c6.b, num, 0 }, // { dg-warning "field .P::c6. is used uninitialized" } + c7{ 0, c7.a, 0 } + { } +}; + +struct D { + int &a; + int &b; +}; + +struct M { + D d1; + D d2; + D d3; + M() : + d1{ num, num }, + d2{ num, d2.a }, + // Not a FE warning. + d3{ d3.b, num } // { dg-warning "is used uninitialized" } + { } +}; + +M m; + +struct E { + E(); + E foo(); + E* operator->(); +}; + +struct F { + F(E); +}; + +struct G { + F f; + E e; + G() : f{ e->foo() }, e() { } // { dg-warning "field .G::e. is used uninitialized" } +}; diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-18.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-18.C new file mode 100644 index 00000000000..c05ad42f95e --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-18.C @@ -0,0 +1,22 @@ +// PR c++/96121 +// { dg-do compile { target c++11 } } +// { dg-options "-Wuninitialized" } + +struct A { + A(); + int i; +}; +struct B { + B(A); + int i; +}; + +struct composed2 { + B b_; + A a_; + composed2() : b_(a_) {} // { dg-warning "field .composed2::a_. is used uninitialized" } +}; + +composed2 test() { + return composed2{}; +} diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-19.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-19.C new file mode 100644 index 00000000000..c401f8636bf --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-19.C @@ -0,0 +1,50 @@ +// PR c++/19808 +// { dg-do compile { target c++11 } } +// { dg-options "-Wuninitialized" } +// Test we warn when initializing a base class. + +struct A { + A(int) { } +}; + +struct B : public A { + int x; + B() : A(x) { } // { dg-warning "field .B::x. is used uninitialized" } +}; + +struct C : public A { + int x; + int y; + C() : A(y = 4), x(y) { } +}; + +struct D : public A { + int x; + D() : A{x} { } // { dg-warning "field .D::x. is used uninitialized" } +}; + +struct E : public A { + int x; + int y; + E() : A{y = 4}, x(y) { } +}; + +struct F { + F(int&) { } +}; + +struct G : F { + int x; + G() : F(x) { } +}; + +struct H { + H(int *) { } +}; + +struct I : H { + int x; + int arr[2]; + I() : H(&x) { } + I(int) : H(arr) { } +}; diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-20.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-20.C new file mode 100644 index 00000000000..9f367f0fdfd --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-20.C @@ -0,0 +1,16 @@ +// PR c++/96121 +// { dg-do compile { target c++11 } } +// { dg-options "-Wuninitialized" } +// Test we warn with delegating constructors. + +struct A { + A(int); + A(int &, int); + A(int (*)[1]) : A(x) { } // { dg-warning "21:field .A::x. is used uninitialized" } + A(int (*)[2]) : A(x, x) { } // { dg-warning "24:field .A::x. is used uninitialized" } + A(int (*)[3]) : A(x, 0) { } + A(int (*)[4]) : A{x} { } // { dg-warning "21:field .A::x. is used uninitialized" } + A(int (*)[5]) : A{x, x} { } // { dg-warning "24:field .A::x. is used uninitialized" } + A(int (*)[6]) : A{x, 0} { } + int x; +}; diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-21.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-21.C new file mode 100644 index 00000000000..57ca00ab042 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-21.C @@ -0,0 +1,20 @@ +// PR c++/19808 +// { dg-do compile } +// { dg-options "-Wuninitialized" } + +struct A { + int a; + int b; + A(int) {} +}; + +struct S { + A a; + A a2; + S() : + /* We don't warn here, because we consider partial initialization + as initializing the whole object. */ + a((a2.a = 42)), + a2(a2.a) + { } +}; diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-22.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-22.C new file mode 100644 index 00000000000..bd980d61118 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-22.C @@ -0,0 +1,37 @@ +// PR c++/19808 +// { dg-do compile { target c++11 } } +// { dg-options "-Wuninitialized -Winit-self" } +// Test that we don't warn when initializing a reference, unless it's +// self-init. + +struct R { + int &r; +}; + +struct S { + R r; + int a; + int &b; + int c; +}; + +struct X { + S s; + X() : s{ { s.a }, 1, s.c, 3} { } +}; + +struct A { + int &r; + A() : r{r} { } // { dg-warning ".A::r. is initialized with itself" } +}; + +struct B { + int &r; + int a; + B() : r{a} { } +}; + +struct C { + R x; + C() : x{x.r} { } // { dg-warning ".R::r. is initialized with itself" } +}; diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-23.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-23.C new file mode 100644 index 00000000000..db3778300be --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-23.C @@ -0,0 +1,24 @@ +// PR c++/19808 +// { dg-do compile { target c++11 } } +// { dg-options "-Wuninitialized" } +// Test that we don't warn in an uninstantiated template. + +struct A { + int *fn() { return nullptr; } +}; + +template<typename T> +struct B { + B() : p(a->fn()) { } + A *a; + int *p; +}; + +template<typename T> +struct C { + C() : p(a->fn()) { } // { dg-warning "field .C<int>::a. is used uninitialized" } + A *a; + int *p; +}; + +C<int> c; diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-24.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-24.C new file mode 100644 index 00000000000..c0a88631fb4 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-24.C @@ -0,0 +1,14 @@ +// PR c++/19808 +// { dg-do compile { target c++11 } } +// { dg-options "-Wuninitialized" } + +struct S { int x, y; }; + +struct A { + S a1[2]; + int i; + A(int (*)[1]) : a1{{1}} { } + A(int (*)[2]) : a1{ {1, 2} } { } + A(int (*)[3]) : a1{{i}} { } // { dg-warning "field .A::i. is used uninitialized" } + A(int (*)[4]) : a1{ {1, i} } { } // { dg-warning "field .A::i. is used uninitialized" } +}; diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-25.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-25.C new file mode 100644 index 00000000000..fb652f989a4 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-25.C @@ -0,0 +1,12 @@ +// PR c++/19808 +// { dg-do compile { target c++11 } } +// { dg-options "-Wall" } + +struct A { + A *a; +}; + +struct B : A { + int i; + B() : A{a} {} +}; diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-26.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-26.C new file mode 100644 index 00000000000..a887d12e9f9 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-26.C @@ -0,0 +1,22 @@ +// PR c++/19808 +// { dg-do compile { target c++11 } } +// { dg-options "-Wuninitialized" } +// Anonymous union/struct. +// ??? The diagnostic should be improved to say 'b' instead of +// "<anonymous>". + +struct S { + __extension__ struct { + int a; + int b; + }; + S() : a(b) { } // { dg-warning "is used uninitialized" } +}; + +struct U { + union { + int a; + int b; + }; + U() : a(b) { } // { dg-warning "is used uninitialized" } +}; diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-27.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-27.C new file mode 100644 index 00000000000..24e6b9b5b48 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-27.C @@ -0,0 +1,20 @@ +// PR c++/19808 +// { dg-do compile } +// { dg-options "-Wall" } + +enum E { red }; + +struct C { + C(int *, unsigned); +}; + +template <unsigned U> struct D : C { + D(int, int, E) : C(e, U) {} + int e[2]; +}; + +void +g () +{ + D<1>(0, 0, red); +} diff --git a/gcc/tree.c b/gcc/tree.c index 1ad4ad5a5f7..1ad884120b0 100644 --- a/gcc/tree.c +++ b/gcc/tree.c @@ -4638,6 +4638,7 @@ stabilize_reference (tree ref) TREE_READONLY (result) = TREE_READONLY (ref); TREE_SIDE_EFFECTS (result) = TREE_SIDE_EFFECTS (ref); TREE_THIS_VOLATILE (result) = TREE_THIS_VOLATILE (ref); + protected_set_expr_location (result, EXPR_LOCATION (ref)); return result; }
This patch implements the long-desired -Wuninitialized warning for member initializer lists, so that the front end can detect bugs like struct A { int a; int b; A() : b(1), a(b) { } }; where the field 'b' is used uninitialized because the order of member initializers in the member initializer list is irrelevant; what matters is the order of declarations in the class definition. I've implemented this by keeping a hash set holding fields that are not initialized yet, so at first it will be {a, b}, and after initializing 'a' it will be {b} and so on. Then I use walk_tree to walk the initializer and if we see that an uninitialized object is used, we warn. Of course, when we use the address of the object, we may not warn: struct B { int &r; int *p; int a; B() : r(a), p(&a), a(1) { } // ok }; Likewise, don't warn in unevaluated contexts such as sizeof. Classes without an explicit initializer may still be initialized by their default constructors; whether or not something is considered initialized is handled in perform_member_init, see member_initialized_p. It is possible that we might want to tweak that if the current approach turns out to be flawed. And this is where things stop being simple. I had to implement certain gymnastics to handle cases like struct S { int a, b, c; S() : a((b = 42)), c(b) { } }; where the initializer for 'a' surreptitiously initializes 'b'. And since 'b' isn't read in the assignment, we can't warn that it's being used uninitialized. Also, initializer lists. Consider: struct A { int x, y, z; }; struct S { A a; S() : a{ 1, 2, a.y } {} // #1 S(int) : a{ a.y, 2, 3 } {} // #2 }; In #1, a.y refers to an initialized element, so we oughtn't warn. But in #2, a.y refers to an uninitialized element, so we should warn. To that end I've added a vector that tracks which elements of an initializer list are already initialized, so that when we encounter them later in the list, we don't warn. This is the find_uninit_data->list_inits thing. It will contain paths to the initialized elements, so e.g. ((S *)this)->a.y These are edge cases, but some developers live on the edge. This patch also handles delegating constructors and initializing base classes. A couple of TODOs: - the same approach could be used to warn for NSDMIs, but I think it's out of scope for this patch; - the diagnostic for anonymous unions/structs should be improved to say the name of the member instead of "<anonymous>"; - certain uninitialized warnings in initializer-lists are issued by the FE as well as the ME. It's unclear to me what we want to do here; - using uninitialized members of base classes is not implemented yet. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? gcc/cp/ChangeLog: PR c++/19808 PR c++/96121 * init.c (perform_member_init): Remove a forward declaration. Walk the initializer using find_uninit_fields_r. New parameter to track uninitialized fields. If a member is initialized, remove it from the hash set. (perform_target_ctor): Return the initializer. (struct find_uninit_data): New class. (find_uninit_fields_r): New function. (emit_mem_initializers): Keep and initialize a set holding fields that are not initialized. When handling delegating constructors, walk the constructor tree using find_uninit_fields_r. Also when initializing base clases. Pass uninitialized down to perform_member_init. gcc/ChangeLog: PR c++/19808 PR c++/96121 * doc/invoke.texi: Update documentation for -Wuninitialized. * tree.c (stabilize_reference): Set location. gcc/testsuite/ChangeLog: PR c++/19808 PR c++/96121 * g++.dg/warn/Wuninitialized-12.C: New test. * g++.dg/warn/Wuninitialized-13.C: New test. * g++.dg/warn/Wuninitialized-14.C: New test. * g++.dg/warn/Wuninitialized-15.C: New test. * g++.dg/warn/Wuninitialized-16.C: New test. * g++.dg/warn/Wuninitialized-17.C: New test. * g++.dg/warn/Wuninitialized-18.C: New test. * g++.dg/warn/Wuninitialized-19.C: New test. * g++.dg/warn/Wuninitialized-20.C: New test. * g++.dg/warn/Wuninitialized-21.C: New test. * g++.dg/warn/Wuninitialized-22.C: New test. * g++.dg/warn/Wuninitialized-23.C: New test. * g++.dg/warn/Wuninitialized-24.C: New test. * g++.dg/warn/Wuninitialized-25.C: New test. * g++.dg/warn/Wuninitialized-26.C: New test. * g++.dg/warn/Wuninitialized-27.C: New test. --- gcc/cp/init.c | 235 +++++++++++++++++- gcc/doc/invoke.texi | 12 + gcc/testsuite/g++.dg/warn/Wuninitialized-12.C | 59 +++++ gcc/testsuite/g++.dg/warn/Wuninitialized-13.C | 49 ++++ gcc/testsuite/g++.dg/warn/Wuninitialized-14.C | 22 ++ gcc/testsuite/g++.dg/warn/Wuninitialized-15.C | 118 +++++++++ gcc/testsuite/g++.dg/warn/Wuninitialized-16.C | 12 + gcc/testsuite/g++.dg/warn/Wuninitialized-17.C | 146 +++++++++++ gcc/testsuite/g++.dg/warn/Wuninitialized-18.C | 22 ++ gcc/testsuite/g++.dg/warn/Wuninitialized-19.C | 50 ++++ gcc/testsuite/g++.dg/warn/Wuninitialized-20.C | 16 ++ gcc/testsuite/g++.dg/warn/Wuninitialized-21.C | 20 ++ gcc/testsuite/g++.dg/warn/Wuninitialized-22.C | 37 +++ gcc/testsuite/g++.dg/warn/Wuninitialized-23.C | 24 ++ gcc/testsuite/g++.dg/warn/Wuninitialized-24.C | 14 ++ gcc/testsuite/g++.dg/warn/Wuninitialized-25.C | 12 + gcc/testsuite/g++.dg/warn/Wuninitialized-26.C | 22 ++ gcc/testsuite/g++.dg/warn/Wuninitialized-27.C | 20 ++ gcc/tree.c | 1 + 19 files changed, 878 insertions(+), 13 deletions(-) create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-12.C create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-13.C create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-14.C create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-15.C create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-16.C create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-17.C create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-18.C create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-19.C create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-20.C create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-21.C create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-22.C create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-23.C create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-24.C create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-25.C create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-26.C create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-27.C base-commit: 6403e02510fe84c72c515247a9dfdb6e05d977fe