Hi, On Fri, Nov 08 2019, Martin Jambor wrote: > Hi, > > this patch is an attempt to implement my idea from a previous thread > about moving -Wmaybe-uninitialized to -Wextra: > > https://gcc.gnu.org/ml/gcc-patches/2019-02/msg00220.html > > Specifically, it attempts to split -Wmaybe-uninitialized into those that > are about SRA DECLs and those which are not, and move to -Wextra only > the former ones. The main idea is that false -Wmaybe-uninitialized > warings about values that are scalar in user's code are easy to silence > by initializing them to zero or something, as opposed to bits of > aggregates such as a value in std::optional which are not. Therefore, > the warnings about user-scalars can remain in -Wall but warnings about > SRAed pieces should be moved to -Wextra. >
I have not seen any opposition to the idea in principle and so I'd like to propose it as a patch (as opposed to an RFC) while this stage 1 lasts. I have also re-based the patch and tried to bootstrap it... and it failed because of PR 91825. That bug was fixed by adding #pragma GCC diagnostic warning "-Wmaybe-uninitialized" to expmed.c by Jason in r277864 and I of course had to change that to: #pragma GCC diagnostic warning "-Wmaybe-uninitialized-aggregates" This episode only reinforced my opinion that maybe-uninitialized warnings for aggregates are nasty - if even we have to work around them they should not be part of -Wall. I would not oppose moving the whole -Wmaybe-uninitialized as it is now to -Wextra but if we want it to stay for scalars, I think we need something like the below, which is a version that did survive a round of bootstrap and testing. Thanks, Martin 2019-11-15 Martin Jambor <mjam...@suse.cz> * common.opt (Wmaybe-uninitialized-aggregates): New. * tree-ssa-uninit.c (gate_warn_uninitialized): Also run if warn_maybe_uninitialized_aggregates is set. (warn_uninit): Warn for artificial DECLs only if warn_maybe_uninitialized_aggregates is set. * doc/invoke.texi (Warning Options): Add -Wmaybe-uninitialized-aggregates to the list. (-Wextra): Likewise. (-Wmaybe-uninitialized): Document that it only works on scalars. (-Wmaybe-uninitialized-aggregates): Document. * expmed.c: Change GCC diagnostic warning to -Wmaybe-uninitialized-aggregates testsuite/ * gcc.dg/pr45083.c: Add Wmaybe-uninitialized-aggregates to options. * gcc.dg/ubsan/pr81981.c: Likewise. * gfortran.dg/pr25923.f90: Likewise. * g++.dg/warn/pr80635.C: New. --- gcc/common.opt | 4 +++ gcc/doc/invoke.texi | 18 +++++++++-- gcc/expmed.c | 2 +- gcc/testsuite/g++.dg/warn/pr80635.C | 45 +++++++++++++++++++++++++++ gcc/testsuite/gcc.dg/pr45083.c | 2 +- gcc/testsuite/gcc.dg/ubsan/pr81981.c | 2 +- gcc/testsuite/gfortran.dg/pr25923.f90 | 2 +- gcc/tree-ssa-uninit.c | 14 ++++++++- 8 files changed, 82 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/g++.dg/warn/pr80635.C diff --git a/gcc/common.opt b/gcc/common.opt index 12c0083964e..212e55f88c7 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -783,6 +783,10 @@ Wmaybe-uninitialized Common Var(warn_maybe_uninitialized) Warning EnabledBy(Wuninitialized) Warn about maybe uninitialized automatic variables. +Wmaybe-uninitialized-aggregates +Common Var(warn_maybe_uninitialized_aggregates) Warning EnabledBy(Wextra) +Warn about maybe uninitialized automatic parts of aggregates. + Wunreachable-code Common Ignore Warning Does nothing. Preserved for backward compatibility. diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 00eb7e77808..60612271eed 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -328,7 +328,8 @@ Objective-C and Objective-C++ Dialects}. -Wzero-length-bounds @gol -Winvalid-pch -Wlarger-than=@var{byte-size} @gol -Wlogical-op -Wlogical-not-parentheses -Wlong-long @gol --Wmain -Wmaybe-uninitialized -Wmemset-elt-size -Wmemset-transposed-args @gol +-Wmain -Wmaybe-uninitialized -Wmaybe-uninitialized-aggregates @gol +-Wmemset-elt-size -Wmemset-transposed-args @gol -Wmisleading-indentation -Wmissing-attributes -Wmissing-braces @gol -Wmissing-field-initializers -Wmissing-format-attribute @gol -Wmissing-include-dirs -Wmissing-noreturn -Wmissing-profile @gol @@ -4503,6 +4504,7 @@ name is still supported, but the newer name is more descriptive.) -Wempty-body @gol -Wignored-qualifiers @gol -Wimplicit-fallthrough=3 @gol +-Wmaybe-uninitialized-aggregates @gol -Wmissing-field-initializers @gol -Wmissing-parameter-type @r{(C only)} @gol -Wold-style-declaration @r{(C only)} @gol @@ -5695,10 +5697,22 @@ in fact be called at the place that would cause a problem. Some spurious warnings can be avoided if you declare all the functions you use that never return as @code{noreturn}. @xref{Function -Attributes}. +Attributes}. This option only warns about scalar variables, use +@option{-Wmaybe-uninitialized-aggregates} to enable the warnings on parts of +aggregates which the compiler can track. This warning is enabled by @option{-Wall} or @option{-Wextra}. +@item -Wmaybe-uninitialized-aggregates +@opindex Wmaybe-uninitialized-aggregates +@opindex Wmaybe-uninitialized-aggregates + +This option enables the same warning like @option{-Wmaybe-uninitialized} but +for parts of aggregates (i.g.@: structures, arrays or unions) that GCC +optimizers can track. These warnings are only possible in optimizing +compilation, because otherwise GCC does not keep track of the state of +variables. This warning is enabled by @option{-Wextra}. + @item -Wunknown-pragmas @opindex Wunknown-pragmas @opindex Wno-unknown-pragmas diff --git a/gcc/expmed.c b/gcc/expmed.c index ff8554b1562..8f23da6b2d5 100644 --- a/gcc/expmed.c +++ b/gcc/expmed.c @@ -19,7 +19,7 @@ along with GCC; see the file COPYING3. If not see <http://www.gnu.org/licenses/>. */ /* Work around tree-optimization/91825. */ -#pragma GCC diagnostic warning "-Wmaybe-uninitialized" +#pragma GCC diagnostic warning "-Wmaybe-uninitialized-aggregates" #include "config.h" #include "system.h" diff --git a/gcc/testsuite/g++.dg/warn/pr80635.C b/gcc/testsuite/g++.dg/warn/pr80635.C new file mode 100644 index 00000000000..eaf6b34a29d --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/pr80635.C @@ -0,0 +1,45 @@ +// PR C++/80635 +// { dg-options "-std=c++11 -Wuninitialized -O" } + +void* operator new (__SIZE_TYPE__, void *p) { return p; } + +int f (); +int x; + +struct A { + int i; + A (): i (f ()) { } // { dg-bogus "uninitialized" } + ~A () { x = i; } +}; + +struct B { + B (); + ~B (); +}; + + +template <class T> +struct C { + C (): t (), b () { } + ~C () { if (b) t.~T (); } + + void f () { + new (&t) T (); + b = true; + } + + union { + T t; + char x[sizeof (T)]; + }; + bool b; +}; + +void g () +{ + C<A> c1; + C<B> c2; + + c1.f (); + c2.f (); +} diff --git a/gcc/testsuite/gcc.dg/pr45083.c b/gcc/testsuite/gcc.dg/pr45083.c index c9a4dbfe191..bce7d0bd7a6 100644 --- a/gcc/testsuite/gcc.dg/pr45083.c +++ b/gcc/testsuite/gcc.dg/pr45083.c @@ -1,6 +1,6 @@ /* PR tree-optimization/45083 */ /* { dg-do compile } */ -/* { dg-options "-O2 -Wuninitialized" } */ +/* { dg-options "-O2 -Wuninitialized -Wmaybe-uninitialized-aggregates" } */ struct S { char *a; unsigned b; unsigned c; }; extern int foo (const char *); diff --git a/gcc/testsuite/gcc.dg/ubsan/pr81981.c b/gcc/testsuite/gcc.dg/ubsan/pr81981.c index b2636d4c934..6a381f6b180 100644 --- a/gcc/testsuite/gcc.dg/ubsan/pr81981.c +++ b/gcc/testsuite/gcc.dg/ubsan/pr81981.c @@ -1,6 +1,6 @@ /* PR sanitizer/81981 */ /* { dg-do compile } */ -/* { dg-options "-O2 -Wmaybe-uninitialized -fsanitize=undefined -ffat-lto-objects" } */ +/* { dg-options "-O2 -Wmaybe-uninitialized -Wmaybe-uninitialized-aggregates -fsanitize=undefined -ffat-lto-objects" } */ int v; diff --git a/gcc/testsuite/gfortran.dg/pr25923.f90 b/gcc/testsuite/gfortran.dg/pr25923.f90 index 3283ba21f32..2ddc3b92485 100644 --- a/gcc/testsuite/gfortran.dg/pr25923.f90 +++ b/gcc/testsuite/gfortran.dg/pr25923.f90 @@ -1,5 +1,5 @@ ! { dg-do compile } -! { dg-options "-O -Wuninitialized" } +! { dg-options "-O -Wuninitialized -Wmaybe-uninitialized-aggregates" } module foo implicit none diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c index fe8f8f0bc28..362af73e5d2 100644 --- a/gcc/tree-ssa-uninit.c +++ b/gcc/tree-ssa-uninit.c @@ -134,6 +134,16 @@ warn_uninit (enum opt_code wc, tree t, tree expr, tree var, return; if (!has_undefined_value_p (t)) return; + if (wc == OPT_Wmaybe_uninitialized + && SSA_NAME_VAR (t) + && DECL_ARTIFICIAL (SSA_NAME_VAR (t)) + && DECL_HAS_DEBUG_EXPR_P (SSA_NAME_VAR (t))) + { + if (warn_maybe_uninitialized_aggregates) + wc = OPT_Wmaybe_uninitialized_aggregates; + else + return; + } /* Anonymous SSA_NAMEs shouldn't be uninitialized, but ssa_undefined_value_p can return true if the def stmt of anonymous SSA_NAME is COMPLEX_EXPR @@ -2622,7 +2632,9 @@ warn_uninitialized_phi (gphi *phi, vec<gphi *> *worklist, static bool gate_warn_uninitialized (void) { - return warn_uninitialized || warn_maybe_uninitialized; + return (warn_uninitialized + || warn_maybe_uninitialized + || warn_maybe_uninitialized_aggregates); } namespace { -- 2.23.0