Move -Wmaybe-uninitialized to -Wextra

Move -Wmaybe-uninitialized to -Wextra

[Marc Glisse]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 6849 bytes --]

Hello,

first, I expect this to be controversial, so feel free to complain.

The description of -Wall says "This enables all the warnings about
constructions that some users consider questionable, and that are easy
to avoid (or modify to prevent the warning), even in conjunction with
macros."

And the description of -Wmaybe-uninitialized "For an automatic variable,
if there exists a path from the function entry to a use of the variable
that is initialized, but there exist some other paths for which the
variable is not initialized, the compiler emits a warning if it cannot
prove the uninitialized paths are not executed at run time. These
warnings are made optional because GCC is not smart enough to see all
the reasons why the code might be correct in spite of appearing to have
an error."

-Wmaybe-uninitialized generates false positives, we can tweak the compiler 
to reduce them, but there will always be some, that's in the nature of 
this warning.

These false positives are not easy to avoid, as required to be part of 
-Wall. Avoiding them, when it is possible at all, requires not just a 
syntactic tweak, like adding parentheses, but a semantic change that can 
make the code worse. Initializing something that does not need it is extra 
code (increases code size and running time). It also prevents better tools 
from detecting true uninitialized uses, either static analyzers or runtime 
checkers (sanitizer, valgrind).

This message concentrates on the negatives, but that doesn't mean I 
consider -Wmaybe-uninitialized as useless. It can find true uninitialized 
uses. And even some false positives can point at places where we can help 
the compiler generate better code (say with a default 
__builtin_unreachable case in a switch). I did in the past contribute 
patches to make it warn more often, and I might do so again in the future.

My opinion is that -Wmaybe-uninitialized would serve its purpose better as 
part of -Wextra. People tend to use -Wall with -Werror (either explicitly 
or implicitly by modifying the code until all warnings are gone). What I 
see currently in projects where I participate is that 
-Wmaybe-uninitialized is making things worse. People don't investigate 
deeply the cause of the warning, they just go for whatever "quick-fix" 
makes the compiler shut up. Quite often, this involves extra code that is 
less readable and performs worse, while it didn't even "fix" what caused 
the warning, it just randomly ended up with code where the compiler 
doesn't warn (possibly because the function got bigger, which changed 
inlining decisions...).

If the warning is not enabled by default so much but only when people are 
ready to investigate any warning thoroughly, the quickfix mentality is 
less likely to be present. People using -Wmaybe-uninitialized need to be 
willing to ignore false positives, or disable them with pragmas.

Note that similar arguments may apply to some other warnings that somehow 
made their way into -Wall when they shouldn't have, but for now I am only 
proposing to move -Wmaybe-uninitialized. Some people tend to consider that 
if a warning is not part of -Wall, it might as well not exist. Obviously I 
disagree with that.

-------

Now the actual patch. Surprisingly, the middle-end puts both 
Wuninitialized and Wmaybe-uninitialized in Wextra, it is the C-family of 
front-ends that puts them in Wall. It also makes Wuninitialized enable 
Wmaybe-uninitialized, which is backwards (it made sense historically), 
Wuninitialized has much fewer false positives, and if we are willing to be 
warned about possibly uninitialized uses, we certainly also want warnings 
about uninitialized uses that are certain. So I am switching the enabling 
relation between those 2, and enabling only Wuninitialized at Wall.

If the patch gets in, this will of course require a mention in the release 
notes.

I changed a set of tests based on a mix of grep and seeing what failed 
make check. The exact list may not be optimal.

gcc/ChangeLog:

2019-02-01  Marc Glisse  <marc.glisse@inria.fr>

 	* common.opt (Wuninitialized): Enable with Wmaybe-uninitialized.
 	(Wmaybe-uninitialized): Enable with Wextra.
 	* doc/invoke.texi: Update implications between Wuninitialized,
 	Wmaybe-uninitialized, Wall and Wextra.

gcc/c-family/ChangeLog:

2019-02-01  Marc Glisse  <marc.glisse@inria.fr>

 	* c.opt (Wmaybe-uninitialized): Enable with Wextra.

gcc/testsuite/ChangeLog:

2019-02-01  Marc Glisse  <marc.glisse@inria.fr>

 	* c-c++-common/pr69543-1.c: Use -Wmaybe-uninitialized.
 	* c-c++-common/pr69543-2.c: Likewise.
 	* c-c++-common/pr69543-3.c: Likewise.
 	* c-c++-common/pr69543-4.c: Likewise.
 	* c-c++-common/uninit-17.c: Likewise.
 	* g++.dg/pr48484.C: Likewise.
 	* g++.dg/uninit-pred-1_b.C: Likewise.
 	* g++.dg/uninit-pred-2_b.C: Likewise.
 	* g++.dg/uninit-pred-3_b.C: Likewise.
 	* g++.dg/warn/Wuninitialized-5.C: Likewise.
 	* g++.dg/warn/Wuninitialized-6.C: Likewise.
 	* g++.dg/warn/Wuninitialized-9.C: Likewise.
 	* gcc.dg/gomp/pr72781.c: Likewise.
 	* gcc.dg/pr18501.c: Likewise.
 	* gcc.dg/pr39666-2.c: Likewise.
 	* gcc.dg/pr45083.c: Likewise.
 	* gcc.dg/pr57149.c: Likewise.
 	* gcc.dg/pr59924.c: Likewise.
 	* gcc.dg/pr69543.c: Likewise.
 	* gcc.dg/uninit-11-O0.c: Likewise.
 	* gcc.dg/uninit-11.c: Likewise.
 	* gcc.dg/uninit-16.c: Likewise.
 	* gcc.dg/uninit-17.c: Likewise.
 	* gcc.dg/uninit-18.c: Likewise.
 	* gcc.dg/uninit-19.c: Likewise.
 	* gcc.dg/uninit-6-O0.c: Likewise.
 	* gcc.dg/uninit-pr19430-2.c: Likewise.
 	* gcc.dg/uninit-pr19430-O0.c: Likewise.
 	* gcc.dg/uninit-pr19430.c: Likewise.
 	* gcc.dg/uninit-pr20644-O0.c: Likewise.
 	* gcc.dg/uninit-pred-2_a.c: Likewise.
 	* gcc.dg/uninit-pred-2_b.c: Likewise.
 	* gcc.dg/uninit-pred-2_c.c: Likewise.
 	* gcc.dg/uninit-pred-3_a.c: Likewise.
 	* gcc.dg/uninit-pred-3_b.c: Likewise.
 	* gcc.dg/uninit-pred-3_c.c: Likewise.
 	* gcc.dg/uninit-pred-3_d.c: Likewise.
 	* gcc.dg/uninit-pred-3_e.c: Likewise.
 	* gcc.dg/uninit-pred-4_a.c: Likewise.
 	* gcc.dg/uninit-pred-4_b.c: Likewise.
 	* gcc.dg/uninit-pred-5_a.c: Likewise.
 	* gcc.dg/uninit-pred-5_b.c: Likewise.
 	* gcc.dg/uninit-pred-6_a.c: Likewise.
 	* gcc.dg/uninit-pred-6_b.c: Likewise.
 	* gcc.dg/uninit-pred-6_c.c: Likewise.
 	* gcc.dg/uninit-pred-6_d.c: Likewise.
 	* gcc.dg/uninit-pred-6_e.c: Likewise.
 	* gcc.dg/uninit-pred-7_a.c: Likewise.
 	* gcc.dg/uninit-pred-7_b.c: Likewise.
 	* gcc.dg/uninit-pred-7_c.c: Likewise.
 	* gcc.dg/uninit-pred-7_d.c: Likewise.
 	* gcc.dg/uninit-pred-8_a.c: Likewise.
 	* gcc.dg/uninit-pred-8_b.c: Likewise.
 	* gcc.dg/uninit-pred-8_c.c: Likewise.
 	* gcc.dg/uninit-pred-8_d.c: Likewise.
 	* gcc.dg/uninit-pred-9_a.c: Likewise.
 	* gcc.dg/uninit-pred-9_b.c: Likewise.
 	* gcc.dg/uninit-suppress_2.c: Likewise.
 	* gfortran.dg/pr25923.f90: Likewise.
 	* gfortran.dg/pr39666-2.f90: Likewise.

-- 
Marc Glisse

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: TEXT/x-diff; name=uninit.diff, Size: 40708 bytes --]

Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 268425)
+++ gcc/c-family/c.opt	(working copy)
@@ -1136,21 +1136,21 @@ Warn about @selector()s without previous
 
 Wundef
 C ObjC C++ ObjC++ CPP(warn_undef) CppReason(CPP_W_UNDEF) Var(cpp_warn_undef) Init(0) Warning
 Warn if an undefined macro is used in an #if directive.
 
 Wuninitialized
 C ObjC C++ ObjC++ LTO LangEnabledBy(C ObjC C++ ObjC++ LTO,Wall)
 ;
 
 Wmaybe-uninitialized
-C ObjC C++ ObjC++ LTO LangEnabledBy(C ObjC C++ ObjC++ LTO,Wall)
+C ObjC C++ ObjC++ LTO LangEnabledBy(C ObjC C++ ObjC++ LTO,Wextra)
 ;
 
 Wunknown-pragmas
 C ObjC C++ ObjC++ Warning Var(warn_unknown_pragmas) LangEnabledBy(C ObjC C++ ObjC++,Wall, 1, 0)
 Warn about unrecognized pragmas.
 
 Wunsuffixed-float-constants
 C ObjC Var(warn_unsuffixed_float_constants) Warning
 Warn about unsuffixed float constants.
 
Index: gcc/common.opt
===================================================================
--- gcc/common.opt	(revision 268425)
+++ gcc/common.opt	(working copy)
@@ -769,25 +769,25 @@ Do not suppress warnings from system hea
 
 Wtrampolines
 Common Var(warn_trampolines) Warning
 Warn whenever a trampoline is generated.
 
 Wtype-limits
 Common Var(warn_type_limits) Warning EnabledBy(Wextra)
 Warn if a comparison is always true or always false due to the limited range of the data type.
 
 Wuninitialized
-Common Var(warn_uninitialized) Warning EnabledBy(Wextra)
+Common Var(warn_uninitialized) Warning EnabledBy(Wmaybe-uninitialized)
 Warn about uninitialized automatic variables.
 
 Wmaybe-uninitialized
-Common Var(warn_maybe_uninitialized) Warning EnabledBy(Wuninitialized)
+Common Var(warn_maybe_uninitialized) Warning EnabledBy(Wextra)
 Warn about maybe uninitialized automatic variables.
 
 Wunreachable-code
 Common Ignore Warning
 Does nothing. Preserved for backward compatibility.
 
 Wunused
 Common Var(warn_unused) Init(0) Warning
 Enable all -Wunused- warnings.
 
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 268425)
+++ gcc/doc/invoke.texi	(working copy)
@@ -4376,21 +4376,20 @@ Options} and @ref{Objective-C and Object
 -Wduplicate-decl-specifier @r{(C and Objective-C only)} @gol
 -Wenum-compare @r{(in C/ObjC; this is on by default in C++)} @gol
 -Wformat   @gol
 -Wint-in-bool-context  @gol
 -Wimplicit @r{(C and Objective-C only)} @gol
 -Wimplicit-int @r{(C and Objective-C only)} @gol
 -Wimplicit-function-declaration @r{(C and Objective-C only)} @gol
 -Winit-self @r{(only for C++)} @gol
 -Wlogical-not-parentheses @gol
 -Wmain @r{(only for C/ObjC and unless} @option{-ffreestanding}@r{)}  @gol
--Wmaybe-uninitialized @gol
 -Wmemset-elt-size @gol
 -Wmemset-transposed-args @gol
 -Wmisleading-indentation @r{(only for C/C++)} @gol
 -Wmissing-attributes @gol
 -Wmissing-braces @r{(only for C/ObjC)} @gol
 -Wmultistatement-macros  @gol
 -Wnarrowing @r{(only for C++)}  @gol
 -Wnonnull  @gol
 -Wnonnull-compare  @gol
 -Wopenmp-simd @gol
@@ -4432,28 +4431,28 @@ them must be enabled individually.
 This enables some extra warning flags that are not enabled by
 @option{-Wall}. (This option used to be called @option{-W}.  The older
 name is still supported, but the newer name is more descriptive.)
 
 @gccoptlist{-Wclobbered  @gol
 -Wcast-function-type  @gol
 -Wdeprecated-copy @r{(C++ only)} @gol
 -Wempty-body  @gol
 -Wignored-qualifiers @gol
 -Wimplicit-fallthrough=3 @gol
+-Wmaybe-uninitialized @gol
 -Wmissing-field-initializers  @gol
 -Wmissing-parameter-type @r{(C only)}  @gol
 -Wold-style-declaration @r{(C only)}  @gol
 -Woverride-init  @gol
 -Wsign-compare @r{(C only)} @gol
 -Wredundant-move @r{(only for C++)}  @gol
 -Wtype-limits  @gol
--Wuninitialized  @gol
 -Wshift-negative-value @r{(in C++03 and in C99 and newer)}  @gol
 -Wunused-parameter @r{(only with} @option{-Wunused} @r{or} @option{-Wall}@r{)} @gol
 -Wunused-but-set-parameter @r{(only with} @option{-Wunused} @r{or} @option{-Wall}@r{)}}
 
 
 The option @option{-Wextra} also prints warning messages for the
 following cases:
 
 @itemize @bullet
 
@@ -5527,20 +5526,22 @@ variables that are uninitialized or clob
 not occur for variables or elements declared @code{volatile}.  Because
 these warnings depend on optimization, the exact variables or elements
 for which there are warnings depends on the precise optimization
 options and version of GCC used.
 
 Note that there may be no warning about a variable that is used only
 to compute a value that itself is never used, because such
 computations may be deleted by data flow analysis before the warnings
 are printed.
 
+This warning is enabled by @option{-Wmaybe-uninitialized}.
+
 @item -Winvalid-memory-model
 @opindex Winvalid-memory-model
 @opindex Wno-invalid-memory-model
 Warn for invocations of @ref{__atomic Builtins}, @ref{__sync Builtins},
 and the C11 atomic generic functions with a memory consistency argument
 that is either invalid for the operation or outside the range of values
 of the @code{memory_order} enumeration.  For example, since the
 @code{__atomic_store} and @code{__atomic_store_n} built-ins are only
 defined for the relaxed, release, and sequentially consistent memory
 orders the following code is diagnosed:
@@ -5599,21 +5600,21 @@ changed by a call to @code{longjmp}.
 The compiler sees only the calls to @code{setjmp}.  It cannot know
 where @code{longjmp} will be called; in fact, a signal handler could
 call it at any point in the code.  As a result, you may get a warning
 even when there is in fact no problem because @code{longjmp} cannot
 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}.
 
-This warning is enabled by @option{-Wall} or @option{-Wextra}.
+This warning is enabled by @option{-Wextra}.
 
 @item -Wunknown-pragmas
 @opindex Wunknown-pragmas
 @opindex Wno-unknown-pragmas
 @cindex warning for unknown pragmas
 @cindex unknown pragmas, warning
 @cindex pragmas, warning of unknown
 Warn when a @code{#pragma} directive is encountered that is not understood by 
 GCC@.  If this command-line option is used, warnings are even issued
 for unknown pragmas in system header files.  This is not the case if
Index: gcc/testsuite/c-c++-common/pr69543-1.c
===================================================================
--- gcc/testsuite/c-c++-common/pr69543-1.c	(revision 268425)
+++ gcc/testsuite/c-c++-common/pr69543-1.c	(working copy)
@@ -1,11 +1,11 @@
-/* { dg-options "-Wuninitialized" } */
+/* { dg-options "-Wmaybe-uninitialized -Wuninitialized" } */
 
 /* Verify disabling a warning, where the _Pragma is within
    a macro, but the affected code is *not* in a macro.  */
 
 # define YY_IGNORE_MAYBE_UNINITIALIZED_BEGIN	\
     _Pragma ("GCC diagnostic push") \
     _Pragma ("GCC diagnostic ignored \"-Wuninitialized\"")\
     _Pragma ("GCC diagnostic ignored \"-Wmaybe-uninitialized\"")
 # define YY_IGNORE_MAYBE_UNINITIALIZED_END \
     _Pragma ("GCC diagnostic pop")
Index: gcc/testsuite/c-c++-common/pr69543-2.c
===================================================================
--- gcc/testsuite/c-c++-common/pr69543-2.c	(revision 268425)
+++ gcc/testsuite/c-c++-common/pr69543-2.c	(working copy)
@@ -1,11 +1,11 @@
-/* { dg-options "-Wuninitialized" } */
+/* { dg-options "-Wmaybe-uninitialized -Wuninitialized" } */
 
 /* Verify disabling a warning, where both the _Pragma and the
    affected code are *not* in a macro.  */
 
 void test (char yylval)
 {
   char *yyvsp;
   _Pragma ("GCC diagnostic push")
   _Pragma ("GCC diagnostic ignored \"-Wuninitialized\"")
   _Pragma ("GCC diagnostic ignored \"-Wmaybe-uninitialized\"")
Index: gcc/testsuite/c-c++-common/pr69543-3.c
===================================================================
--- gcc/testsuite/c-c++-common/pr69543-3.c	(revision 268425)
+++ gcc/testsuite/c-c++-common/pr69543-3.c	(working copy)
@@ -1,11 +1,11 @@
-/* { dg-options "-Wuninitialized" } */
+/* { dg-options "-Wmaybe-uninitialized -Wuninitialized" } */
 
 /* Verify disabling a warning, where the _Pragma is in regular code,
    but the affected code is within a macro.  */
 
 /* TODO: XFAIL: both C and C++ erroneously fail to suppress the warning
    The warning is reported at the macro definition location, rather than
    the macro expansion location.  */
 
 #define WARNABLE_CODE *++yyvsp = yylval; /* { dg-bogus "used uninitialized" "" { xfail *-*-* } } */
 
Index: gcc/testsuite/c-c++-common/pr69543-4.c
===================================================================
--- gcc/testsuite/c-c++-common/pr69543-4.c	(revision 268425)
+++ gcc/testsuite/c-c++-common/pr69543-4.c	(working copy)
@@ -1,11 +1,11 @@
-/* { dg-options "-Wuninitialized" } */
+/* { dg-options "-Wmaybe-uninitialized -Wuninitialized" } */
 
 /* Verify disabling a warning, where both the _Pragma and the
    affected code are within (different) macros.  */
 
 /* TODO: XFAIL: both C and C++ erroneously fail to suppress the warning
    The warning is reported at the macro definition location, rather than
    the macro expansion location.  */
 
 # define YY_IGNORE_MAYBE_UNINITIALIZED_BEGIN	\
     _Pragma ("GCC diagnostic push") \
Index: gcc/testsuite/c-c++-common/uninit-17.c
===================================================================
--- gcc/testsuite/c-c++-common/uninit-17.c	(revision 268425)
+++ gcc/testsuite/c-c++-common/uninit-17.c	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -Wuninitialized -fno-ivopts" } */
+/* { dg-options "-O2 -Wmaybe-uninitialized -fno-ivopts" } */
 
 inline int foo(int x)
 {
   return x;
 }
 static void bar(int a, int *ptr)
 {
   do
   {
     int b;   /* { dg-message "declared" } */
Index: gcc/testsuite/g++.dg/pr48484.C
===================================================================
--- gcc/testsuite/g++.dg/pr48484.C	(revision 268425)
+++ gcc/testsuite/g++.dg/pr48484.C	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile } */
-/* { dg-options "-O -finline-functions -finline-small-functions -Wuninitialized" }  */
+/* { dg-options "-O -finline-functions -finline-small-functions -Wmaybe-uninitialized" }  */
 /* { dg-add-options bind_pic_locally } */
 
 struct SQObjectPtr
 {
   int _type;
   SQObjectPtr operator = (long);
 };
 struct SQObjectPtrVec
 {
   SQObjectPtr fff (unsigned);
Index: gcc/testsuite/g++.dg/uninit-pred-1_b.C
===================================================================
--- gcc/testsuite/g++.dg/uninit-pred-1_b.C	(revision 268425)
+++ gcc/testsuite/g++.dg/uninit-pred-1_b.C	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -O2" } */
 
 typedef long long int64;
 void incr ();
 bool is_valid (int);
 int  get_time ();
 
 class A 
 {
 public:
   A ();
Index: gcc/testsuite/g++.dg/uninit-pred-2_b.C
===================================================================
--- gcc/testsuite/g++.dg/uninit-pred-2_b.C	(revision 268425)
+++ gcc/testsuite/g++.dg/uninit-pred-2_b.C	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -O2" } */
 
 typedef long long int64;
 void incr ();
 bool is_valid (int);
 int  get_time ();
 
 class A 
 {
 public:
   A ();
Index: gcc/testsuite/g++.dg/uninit-pred-3_b.C
===================================================================
--- gcc/testsuite/g++.dg/uninit-pred-3_b.C	(revision 268425)
+++ gcc/testsuite/g++.dg/uninit-pred-3_b.C	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -O2" } */
 
 /* Multiple initialization paths.  */
 
 typedef long long int64;
 void incr ();
 bool is_valid (int);
 int  get_time ();
 
 class A
 {
Index: gcc/testsuite/g++.dg/warn/Wuninitialized-5.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wuninitialized-5.C	(revision 268425)
+++ gcc/testsuite/g++.dg/warn/Wuninitialized-5.C	(working copy)
@@ -1,13 +1,13 @@
 // PR middle-end/39666
 // { dg-do compile }
-// { dg-options "-O2 -Wuninitialized" }
+// { dg-options "-O2 -Wmaybe-uninitialized" }
 
 int
 foo (int i)
 {
   int j;
   switch (i)
     {
     case -__INT_MAX__ - 1 ... -1:
       j = 6;
       break;
Index: gcc/testsuite/g++.dg/warn/Wuninitialized-6.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wuninitialized-6.C	(revision 268425)
+++ gcc/testsuite/g++.dg/warn/Wuninitialized-6.C	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -Wuninitialized -O2" } */
 
 namespace std {
      typedef __SIZE_TYPE__ size_t;
    }
     extern "C++" {
    inline void* operator new(std::size_t, void* __p) throw() {
   return __p;
   }
    }
     namespace boost{
Index: gcc/testsuite/g++.dg/warn/Wuninitialized-9.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wuninitialized-9.C	(revision 268425)
+++ gcc/testsuite/g++.dg/warn/Wuninitialized-9.C	(working copy)
@@ -1,13 +1,13 @@
 // PR c++/80119
 // { dg-do compile { target c++11 } }
-// { dg-options "-Wuninitialized" }
+// { dg-options "-Wmaybe-uninitialized" }
 
 #include <type_traits>
 
 template <bool b>
 void failing_function(std::integral_constant<bool, b>)
 {
    int i;
    if (b && (i = 4)) {
       ++i; // { dg-bogus "may be used uninitialized" }
    }
Index: gcc/testsuite/gcc.dg/gomp/pr72781.c
===================================================================
--- gcc/testsuite/gcc.dg/gomp/pr72781.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/gomp/pr72781.c	(working copy)
@@ -1,13 +1,13 @@
 /* PR middle-end/72781 */
 /* { dg-do compile } */
-/* { dg-additional-options "-O2 -Wuninitialized" } */
+/* { dg-additional-options "-O2 -Wmaybe-uninitialized" } */
 
 int u;
 
 void
 foo (int *p)
 {
   int i;
   #pragma omp for simd lastprivate(u) schedule (static, 32)	/* { dg-bogus "may be used uninitialized in this function" } */
   for (i = 0; i < 1024; i++)
     u = p[i];
Index: gcc/testsuite/gcc.dg/pr18501.c
===================================================================
--- gcc/testsuite/gcc.dg/pr18501.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/pr18501.c	(working copy)
@@ -1,14 +1,14 @@
 /* Expected uninitialized variable warning.  */
 
 /* { dg-do compile } */
-/* { dg-options "-O -Wuninitialized" } */
+/* { dg-options "-O -Wmaybe-uninitialized" } */
 
 unsigned bmp_iter_set ();
 int something (void);
 
 void
 bitmap_print_value_set (void)
 {
   unsigned first;	/* { dg-warning "may be used" "conditional in loop" { xfail *-*-* } } */
   
   for (; bmp_iter_set (); )
Index: gcc/testsuite/gcc.dg/pr39666-2.c
===================================================================
--- gcc/testsuite/gcc.dg/pr39666-2.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/pr39666-2.c	(working copy)
@@ -1,13 +1,13 @@
 /* PR middle-end/39666 */
 /* { dg-do compile } */
-/* { dg-options "-O2 -Wuninitialized" } */
+/* { dg-options "-O2 -Wmaybe-uninitialized" } */
 
 int
 foo (int i)
 {
   int j;
   switch (i)
     {
     case -__INT_MAX__ - 1 ... -1:
       j = 6;
       break;
Index: gcc/testsuite/gcc.dg/pr45083.c
===================================================================
--- gcc/testsuite/gcc.dg/pr45083.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/pr45083.c	(working copy)
@@ -1,13 +1,13 @@
 /* PR tree-optimization/45083 */
 /* { dg-do compile } */
-/* { dg-options "-O2 -Wuninitialized" } */
+/* { dg-options "-O2 -Wuninitialized -Wmaybe-uninitialized" } */
 
 struct S { char *a; unsigned b; unsigned c; };
 extern int foo (const char *);
 extern void bar (int, int);
 
 static void
 baz (void)
 {
   struct S cs[1];	/* { dg-message "was declared here" } */
   switch (cs->b)	/* { dg-warning "cs\[^\n\r\]*\\.b\[^\n\r\]*is used uninitialized" } */
Index: gcc/testsuite/gcc.dg/pr57149.c
===================================================================
--- gcc/testsuite/gcc.dg/pr57149.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/pr57149.c	(working copy)
@@ -1,13 +1,13 @@
 /* PR tree-optimization/57149 */
 /* { dg-do compile } */
-/* { dg-options "-Os -Wuninitialized" } */
+/* { dg-options "-Os -Wmaybe-uninitialized" } */
 
 struct A { struct A *a, *b; };
 struct D { struct A e; };
 struct E { unsigned char f; struct { struct A e; } g; };
 struct F { struct E i[32]; };
 
 extern int fn0 (void);
 extern int fn1 (struct E *, struct D *);
 
 static inline __attribute__ ((always_inline)) int
Index: gcc/testsuite/gcc.dg/pr59924.c
===================================================================
--- gcc/testsuite/gcc.dg/pr59924.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/pr59924.c	(working copy)
@@ -1,13 +1,13 @@
 /* PR tree-optimization/59924 */
 /* { dg-do compile } */
-/* { dg-options "-O1 -Wall" } */
+/* { dg-options "-O1 -Wmaybe-uninitialized" } */
 
 struct S { struct T *a; double b; struct S *c; };
 struct T { struct S *d; };
 extern void bar (double);
 
 void
 foo (struct S * x, int y, int z, int w)
 {
   int e;
   struct S *f;
Index: gcc/testsuite/gcc.dg/pr69543.c
===================================================================
--- gcc/testsuite/gcc.dg/pr69543.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/pr69543.c	(working copy)
@@ -1,13 +1,13 @@
 /* PR preprocessor/69543 */
 /* { dg-do compile } */
-/* { dg-options "-O2 -Wuninitialized" } */
+/* { dg-options "-O2 -Wuninitialized -Wmaybe-uninitialized" } */
 
 # define YY_IGNORE_MAYBE_UNINITIALIZED_BEGIN \
     _Pragma ("GCC diagnostic push") \
     _Pragma ("GCC diagnostic ignored \"-Wuninitialized\"")\
     _Pragma ("GCC diagnostic ignored \"-Wmaybe-uninitialized\"")
 # define YY_IGNORE_MAYBE_UNINITIALIZED_END \
     _Pragma ("GCC diagnostic pop")
 
 void test (char yylval)
 {
Index: gcc/testsuite/gcc.dg/uninit-11-O0.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-11-O0.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-11-O0.c	(working copy)
@@ -1,13 +1,13 @@
 /* Positive test for uninitialized variables.  */
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized" } */
+/* { dg-options "-Wmaybe-uninitialized -Wuninitialized" } */
 
 int sink;
 
 void f1(int parm)	/* { dg-bogus "uninitialized" "parameter" } */
 {
   sink = parm;		/* { dg-bogus "uninitialized" "parameter" } */
 }
 
 void f2(void)
 {
Index: gcc/testsuite/gcc.dg/uninit-11.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-11.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-11.c	(working copy)
@@ -1,13 +1,13 @@
 /* Positive test for uninitialized variables.  */
 /* { dg-do compile } */
-/* { dg-options "-O -Wuninitialized" } */
+/* { dg-options "-O -Wmaybe-uninitialized -Wuninitialized" } */
 
 int sink;
 
 void f1(int parm)	/* { dg-bogus "uninitialized" "parameter" } */
 {
   sink = parm;		/* { dg-bogus "uninitialized" "parameter" } */
 }
 
 void f2(void)
 {
Index: gcc/testsuite/gcc.dg/uninit-16.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-16.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-16.c	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -Wuninitialized" } */
+/* { dg-options "-O2 -Wmaybe-uninitialized" } */
 
 int foo, bar;
 
 static
 void decode_reloc(int reloc, int *is_alt)
 {
   if (reloc >= 20)
       *is_alt = 1;
   else if (reloc >= 10)
       *is_alt = 0;
Index: gcc/testsuite/gcc.dg/uninit-17.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-17.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-17.c	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile } */
-/* { dg-options "-O -Wuninitialized" } */
+/* { dg-options "-O -Wmaybe-uninitialized" } */
 
 typedef _Complex float C;
 C foo(int cond)
 {
   C f;
   __imag__ f = 0;
   if (cond)
     {
       __real__ f = 1;
       return f;
Index: gcc/testsuite/gcc.dg/uninit-18.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-18.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-18.c	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile }  */
-/* { dg-options "-O -Wuninitialized" } */
+/* { dg-options "-O -Wmaybe-uninitialized" } */
 
 char *foo(int bar, char *baz)
 {
   char *tmp;
 
   if (bar & 3)
     tmp = baz;
 
   switch (bar) {
   case 1:
Index: gcc/testsuite/gcc.dg/uninit-19.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-19.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-19.c	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile } */
-/* { dg-options "-O -Wuninitialized" } */
+/* { dg-options "-O -Wmaybe-uninitialized" } */
 /* { dg-additional-options "-finline-small-functions" { target avr-*-* } } */
 
 int a, l, m;
 float *b;
 float c, d, e, g, h;
 unsigned char i, k;
 void
 fn1 (int p1, float *f1, float *f2, float *f3, unsigned char *c1, float *f4,
      unsigned char *c2, float *p10)
 {
Index: gcc/testsuite/gcc.dg/uninit-6-O0.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-6-O0.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-6-O0.c	(working copy)
@@ -1,15 +1,15 @@
 /* Spurious uninitialized variable warnings.
    This one inspired by java/class.c:build_utf8_ref.  */
 
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -ftrack-macro-expansion=2" } */
+/* { dg-options "-Wmaybe-uninitialized -ftrack-macro-expansion=2" } */
 
 #include <stddef.h>
 
 struct tree
 {
     struct tree *car;
     struct tree *cdr;
     int type, data;
 };
 
Index: gcc/testsuite/gcc.dg/uninit-pr19430-2.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pr19430-2.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pr19430-2.c	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile } */
-/* { dg-options "-O -Wuninitialized" } */
+/* { dg-options "-O -Wmaybe-uninitialized" } */
 
 int  *p, *q;
 
 int foo (int b)
 {
   int i, j = 0;
   int *x;
   p = &i;
   q = &j;
   if (b)
Index: gcc/testsuite/gcc.dg/uninit-pr19430-O0.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pr19430-O0.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pr19430-O0.c	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile } */
-/* { dg-options "-O0 -Wuninitialized" } */
+/* { dg-options "-O0 -Wuninitialized -Wmaybe-uninitialized" } */
 extern int bar (int);
 extern void baz (int *);
 
 int
 foo (int i)
 {
   int j; /* { dg-warning "'j' may be used uninitialized in this function" "uninitialized" { xfail *-*-* } } */
 
   if (bar (i)) {
     baz (&j);
Index: gcc/testsuite/gcc.dg/uninit-pr19430.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pr19430.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pr19430.c	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile } */
-/* { dg-options "-O -Wuninitialized" } */
+/* { dg-options "-O -Wmaybe-uninitialized -Wuninitialized" } */
 extern int bar (int);
 extern void baz (int *);
 int
 foo (int i)
 {
   int j; /* { dg-warning "'j' may be used uninitialized in this function" "uninitialized" { xfail *-*-* } } */
 
   if (bar (i)) { 
     baz (&j);
   } else {
Index: gcc/testsuite/gcc.dg/uninit-pr20644-O0.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pr20644-O0.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pr20644-O0.c	(working copy)
@@ -1,13 +1,13 @@
 /* PR 20644 */
 /* { dg-do compile } */
-/* { dg-options "-O0 -Wuninitialized" } */
+/* { dg-options "-O0 -Wmaybe-uninitialized" } */
 int foo ()
 {
   int i = 0;
   int j;
 
   if (1 == i)
     return j; /* { dg-bogus "uninitialized" "uninitialized" { xfail *-*-* } } */
 
   return 0;
 }
Index: gcc/testsuite/gcc.dg/uninit-pred-2_a.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-2_a.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-2_a.c	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -O2" } */
 
 int g;
 void bar (void);
 void blah (int);
 
 int foo (int n, int m, int r)
 {
   int flag = 0;
   int v;
 
Index: gcc/testsuite/gcc.dg/uninit-pred-2_b.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-2_b.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-2_b.c	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -O2" } */
 
 int g;
 void bar (void);
 void blah (int);
 
 int foo (int n, int m, int r)
 {
   int flag = 0;
   int v; 
 
Index: gcc/testsuite/gcc.dg/uninit-pred-2_c.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-2_c.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-2_c.c	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2 -fno-tree-tail-merge" } */
+/* { dg-options "-Wmaybe-uninitialized -O2 -fno-tree-tail-merge" } */
 
 int g;
 void bar (void);
 void blah (int);
 
 int foo (int n, int m, int r)
 {
   int flag = 0;
   int v; 
 
Index: gcc/testsuite/gcc.dg/uninit-pred-3_a.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-3_a.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-3_a.c	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -O2" } */
 
 int g;
 void bar();
 void blah(int);
 
 int foo (int n, int m, int r)
 {
   int flag = 0;
   int v;
 
Index: gcc/testsuite/gcc.dg/uninit-pred-3_b.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-3_b.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-3_b.c	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -O2" } */
 
 int g;
 void bar();
 void blah(int);
 
 int foo (int n, int m, int r)
 {
   int flag = 0;
   int v;
 
Index: gcc/testsuite/gcc.dg/uninit-pred-3_c.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-3_c.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-3_c.c	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -O2" } */
 
 int g;
 void bar();
 void blah(int);
 
 int foo (int n, int m, int r)
 {
   int flag = 0;
   int v;
 
Index: gcc/testsuite/gcc.dg/uninit-pred-3_d.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-3_d.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-3_d.c	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -O2" } */
 
 int g;
 void bar();
 void blah(int);
 
 int foo (int n, int m, int r)
 {
   int flag = 0;
   int v;
 
Index: gcc/testsuite/gcc.dg/uninit-pred-3_e.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-3_e.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-3_e.c	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -O2" } */
 
 int g;
 void bar();
 void blah(int);
 
 int foo (int n, int m, int r)
 {
   int flag = 0;
   int v;
 
Index: gcc/testsuite/gcc.dg/uninit-pred-4_a.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-4_a.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-4_a.c	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -O2" } */
 
 int g;
 void bar();
 void blah(int);
 int foo (int n, int m, int r, int t)
 {
   int flag = 0;
   int v;
 
   if (t)
Index: gcc/testsuite/gcc.dg/uninit-pred-4_b.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-4_b.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-4_b.c	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -O2" } */
 
 int g;
 void bar();
 void blah(int);
 int foo (int n, int m, int r, int t)
 {
   int flag = 0;
   int v;
 
   if (t)
Index: gcc/testsuite/gcc.dg/uninit-pred-5_a.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-5_a.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-5_a.c	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -Wno-attributes -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -Wno-attributes -O2" } */
 
 int g;
 int bar();
 int blah(int);
 void t(int);
 
 static int
 __attribute__((always_inline)) 
 foo (int n, int* v, int r)
 {
Index: gcc/testsuite/gcc.dg/uninit-pred-5_b.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-5_b.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-5_b.c	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -Wno-attributes -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -Wno-attributes -O2" } */
 
 int g;
 int bar();
 int blah(int);
 void t(int);
 
 static int
 __attribute__((always_inline)) 
 foo (int n, int* v, int r)
 {
Index: gcc/testsuite/gcc.dg/uninit-pred-6_a.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-6_a.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-6_a.c	(working copy)
@@ -1,13 +1,13 @@
 
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -O2" } */
 
 int g;
 void bar();
 void blah(int);
 
 int foo (int n, int l, int m, int r)
 {
   int v;
 
   if (n && l)
Index: gcc/testsuite/gcc.dg/uninit-pred-6_b.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-6_b.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-6_b.c	(working copy)
@@ -1,13 +1,13 @@
 
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -O2" } */
 
 int g;
 void bar();
 void blah(int);
 
 int foo (int n, int l, int m, int r)
 {
   int v;
 
   if (n)
Index: gcc/testsuite/gcc.dg/uninit-pred-6_c.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-6_c.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-6_c.c	(working copy)
@@ -1,13 +1,13 @@
 
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -O2" } */
 
 int g;
 void bar();
 void blah(int);
 
 int foo (int n, int l, int m, int r)
 {
   int v;
 
   if (n > 10)
Index: gcc/testsuite/gcc.dg/uninit-pred-6_d.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-6_d.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-6_d.c	(working copy)
@@ -1,13 +1,13 @@
 
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -O2" } */
 
 int g;
 void bar();
 void blah(int);
 
 int foo (int n, int l, int m, int r)
 {
   int v;
 
   if (n)
Index: gcc/testsuite/gcc.dg/uninit-pred-6_e.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-6_e.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-6_e.c	(working copy)
@@ -1,13 +1,13 @@
 
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -O2" } */
 
 int g;
 void bar();
 void blah(int);
 
 int foo (int n, int l, int m, int r)
 {
   int v;
 
   if (n > 10)
Index: gcc/testsuite/gcc.dg/uninit-pred-7_a.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-7_a.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-7_a.c	(working copy)
@@ -1,13 +1,13 @@
 
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -O2" } */
 
 int g;
 void bar();
 void blah(int);
 
 int foo (int n, int l, int m, int r)
 {
   int v;
 
   if (n || l)
Index: gcc/testsuite/gcc.dg/uninit-pred-7_b.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-7_b.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-7_b.c	(working copy)
@@ -1,13 +1,13 @@
 
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -O2" } */
 
 int g;
 void bar();
 void blah(int);
 
 int foo (int n, int l, int m, int r)
 {
   int v;
 
   if (n > 10)
Index: gcc/testsuite/gcc.dg/uninit-pred-7_c.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-7_c.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-7_c.c	(working copy)
@@ -1,13 +1,13 @@
 
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -O2" } */
 
 int g;
 void bar();
 void blah(int);
 
 int foo (int n, int l, int m, int r)
 {
   int v;
 
   if (n)
Index: gcc/testsuite/gcc.dg/uninit-pred-7_d.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-7_d.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-7_d.c	(working copy)
@@ -1,13 +1,13 @@
 
 /* { dg-do compile  { target i?86-*-* x86_64-*-* } } */
-/* { dg-options "-Wuninitialized -O2 -mbranch-cost=0" } */
+/* { dg-options "-Wmaybe-uninitialized -O2 -mbranch-cost=0" } */
 
 int g;
 void bar();
 void blah(int);
 
 int foo (int n, int l, int m, int r)
 {
   int v;
 
   if (n || l)
Index: gcc/testsuite/gcc.dg/uninit-pred-8_a.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-8_a.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-8_a.c	(working copy)
@@ -1,13 +1,13 @@
 
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -O2" } */
 /* Pick a particular tuning to pin down BRANCH_COST.  */
 /* { dg-additional-options "-mtune=cortex-a15" { target arm*-*-* } } */
 
 int g;
 void bar();
 void blah(int);
 
 int foo (int n, int l, int m, int r)
 {
   int v;
Index: gcc/testsuite/gcc.dg/uninit-pred-8_b.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-8_b.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-8_b.c	(working copy)
@@ -1,13 +1,13 @@
 
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -O2" } */
 
 int g;
 void bar();
 void blah(int);
 
 int foo (int n, int l, int m, int r)
 {
   int v;
 
   if (n < 10 || m > 100 || r < 20 || l)
Index: gcc/testsuite/gcc.dg/uninit-pred-8_c.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-8_c.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-8_c.c	(working copy)
@@ -1,13 +1,13 @@
 
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -O2" } */
 
 int g;
 void bar();
 void blah(int);
 
 int foo (int n, int l, int m, int r)
 {
   int v;
 
   if (n < 10 && m > 100  && r < 20 )
Index: gcc/testsuite/gcc.dg/uninit-pred-8_d.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-8_d.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-8_d.c	(working copy)
@@ -1,13 +1,13 @@
 
 /* { dg-do compile  { target i?86-*-* x86_64-*-* } } */
-/* { dg-options "-Wuninitialized -O2 -mbranch-cost=0" } */
+/* { dg-options "-Wmaybe-uninitialized -O2 -mbranch-cost=0" } */
 
 int g;
 void bar();
 void blah(int);
 
 int foo (int n, int l, int m, int r)
 {
   int v;
 
   if (n || m || r || l)
Index: gcc/testsuite/gcc.dg/uninit-pred-9_a.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-9_a.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-9_a.c	(working copy)
@@ -1,13 +1,13 @@
 
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -O2" } */
 
 int g;
 void bar();
 void blah(int);
 
 int foo (int n, int l, int m, int r)
 {
   int v;
 
   if ( (n < 10) && (m == l)  && (r < 20) )
Index: gcc/testsuite/gcc.dg/uninit-pred-9_b.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-pred-9_b.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-pred-9_b.c	(working copy)
@@ -1,13 +1,13 @@
 
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* { dg-options "-Wmaybe-uninitialized -O2" } */
 
 int g;
 void bar();
 void blah(int);
 
 int foo (int n, int l, int m, int r)
 {
   int v;
 
   if ( (n < 10) && (m != 100)  && (r < 20) )
Index: gcc/testsuite/gcc.dg/uninit-suppress_2.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-suppress_2.c	(revision 268425)
+++ gcc/testsuite/gcc.dg/uninit-suppress_2.c	(working copy)
@@ -1,12 +1,12 @@
 /* { dg-do compile } */
-/* { dg-options "-fno-tree-dominator-opts -fno-tree-ccp -fno-tree-vrp -fno-tree-fre -fno-tree-pre -fno-code-hoisting -O2 -Wuninitialized -Werror=uninitialized -Wno-error=maybe-uninitialized" } */
+/* { dg-options "-fno-tree-dominator-opts -fno-tree-ccp -fno-tree-vrp -fno-tree-fre -fno-tree-pre -fno-code-hoisting -O2 -Wuninitialized -Wmaybe-uninitialized -Werror=uninitialized -Wno-error=maybe-uninitialized" } */
 void blah();
 void bar (int);
 int gflag;
 
 void foo()
 {
    int v;
    if (gflag)
      v = 10;
 
Index: gcc/testsuite/gfortran.dg/pr25923.f90
===================================================================
--- gcc/testsuite/gfortran.dg/pr25923.f90	(revision 268425)
+++ gcc/testsuite/gfortran.dg/pr25923.f90	(working copy)
@@ -1,12 +1,12 @@
 ! { dg-do compile }
-! { dg-options "-O -Wuninitialized" }
+! { dg-options "-O -Wmaybe-uninitialized" }
 
 module foo
 implicit none
 
   type bar
     integer :: yr
   end type
 
 contains
 
Index: gcc/testsuite/gfortran.dg/pr39666-2.f90
===================================================================
--- gcc/testsuite/gfortran.dg/pr39666-2.f90	(revision 268425)
+++ gcc/testsuite/gfortran.dg/pr39666-2.f90	(working copy)
@@ -1,13 +1,13 @@
 ! PR middle-end/39666
 ! { dg-do compile }
-! { dg-options "-O2 -Wuninitialized" }
+! { dg-options "-O2 -Wmaybe-uninitialized" }
 
 FUNCTION f(n)
   INTEGER, INTENT(in) :: n
   REAL                :: f
 
   SELECT CASE (n)
     CASE (:-1); f = -1.0
     CASE (0);   f =  0.0
     CASE (2:);  f =  1.0
   END SELECT

Re: Move -Wmaybe-uninitialized to -Wextra

[Segher Boessenkool]

Hi Marc,

On Fri, Feb 01, 2019 at 12:32:45PM +0100, Marc Glisse wrote:
> -Wmaybe-uninitialized generates false positives, we can tweak the compiler 
> to reduce them, but there will always be some, that's in the nature of 
> this warning.

That is true for *every* warning; if not, it should be an error, not a
warning.

> My opinion is that -Wmaybe-uninitialized would serve its purpose better as 
> part of -Wextra.

+1

> People tend to use -Wall with -Werror (either explicitly 
> or implicitly by modifying the code until all warnings are gone). What I 
> see currently in projects where I participate is that 
> -Wmaybe-uninitialized is making things worse. People don't investigate 
> deeply the cause of the warning, they just go for whatever "quick-fix" 
> makes the compiler shut up. Quite often, this involves extra code that is 
> less readable and performs worse, while it didn't even "fix" what caused 
> the warning, it just randomly ended up with code where the compiler 
> doesn't warn (possibly because the function got bigger, which changed 
> inlining decisions...).

Yes, using -Werror is usually a terrible idea.

> Note that similar arguments may apply to some other warnings that somehow 
> made their way into -Wall when they shouldn't have, but for now I am only 
> proposing to move -Wmaybe-uninitialized. Some people tend to consider that 
> if a warning is not part of -Wall, it might as well not exist. Obviously I 
> disagree with that.

If it is not part of -Wall and not of -W, and not special purpose, then it
might as well not exist.


Segher

Re: Move -Wmaybe-uninitialized to -Wextra

[Marek Polacek]

On Fri, Feb 01, 2019 at 07:19:25AM -0600, Segher Boessenkool wrote:
> Hi Marc,
> 
> On Fri, Feb 01, 2019 at 12:32:45PM +0100, Marc Glisse wrote:
> > -Wmaybe-uninitialized generates false positives, we can tweak the compiler 
> > to reduce them, but there will always be some, that's in the nature of 
> > this warning.
> 
> That is true for *every* warning; if not, it should be an error, not a
> warning.
> 
> > My opinion is that -Wmaybe-uninitialized would serve its purpose better as 
> > part of -Wextra.
> 
> +1

+1 from me too.

> > People tend to use -Wall with -Werror (either explicitly 
> > or implicitly by modifying the code until all warnings are gone). What I 
> > see currently in projects where I participate is that 
> > -Wmaybe-uninitialized is making things worse. People don't investigate 
> > deeply the cause of the warning, they just go for whatever "quick-fix" 
> > makes the compiler shut up. Quite often, this involves extra code that is 
> > less readable and performs worse, while it didn't even "fix" what caused 
> > the warning, it just randomly ended up with code where the compiler 
> > doesn't warn (possibly because the function got bigger, which changed 
> > inlining decisions...).
> 
> Yes, using -Werror is usually a terrible idea.
> 
> > Note that similar arguments may apply to some other warnings that somehow 
> > made their way into -Wall when they shouldn't have, but for now I am only 
> > proposing to move -Wmaybe-uninitialized. Some people tend to consider that 
> > if a warning is not part of -Wall, it might as well not exist. Obviously I 
> > disagree with that.
> 
> If it is not part of -Wall and not of -W, and not special purpose, then it
> might as well not exist.

There are warnings that *do* make sense, but have issues e.g. with macro
expansion, so will be outside -Wall/-Wextra unless that's fixed.  E.g.
-Wlogical-op, -Wduplicated-conds, or a warning I posted to some PR
called -Wsame-arguments I think, etc.

Marek

Re: Move -Wmaybe-uninitialized to -Wextra

[Segher Boessenkool]

On Fri, Feb 01, 2019 at 09:01:53AM -0500, Marek Polacek wrote:
> On Fri, Feb 01, 2019 at 07:19:25AM -0600, Segher Boessenkool wrote:
> > > Some people tend to consider that 
> > > if a warning is not part of -Wall, it might as well not exist. Obviously I 
> > > disagree with that.
> > 
> > If it is not part of -Wall and not of -W, and not special purpose, then it
> > might as well not exist.
> 
> There are warnings that *do* make sense, but have issues e.g. with macro
> expansion, so will be outside -Wall/-Wextra unless that's fixed.  E.g.
> -Wlogical-op, -Wduplicated-conds, or a warning I posted to some PR
> called -Wsame-arguments I think, etc.

Yes, we agree on that.  I'm just saying such general-purpose warnings are
not used much until they are part of -Wall or -W.


Segher

Re: Move -Wmaybe-uninitialized to -Wextra

[Jeff Law]

On 2/1/19 7:01 AM, Marek Polacek wrote:
> On Fri, Feb 01, 2019 at 07:19:25AM -0600, Segher Boessenkool wrote:
>> Hi Marc,
>>
>> On Fri, Feb 01, 2019 at 12:32:45PM +0100, Marc Glisse wrote:
>>> -Wmaybe-uninitialized generates false positives, we can tweak the compiler 
>>> to reduce them, but there will always be some, that's in the nature of 
>>> this warning.
>>
>> That is true for *every* warning; if not, it should be an error, not a
>> warning.
>>
>>> My opinion is that -Wmaybe-uninitialized would serve its purpose better as 
>>> part of -Wextra.
>>
>> +1
> 
> +1 from me too.
I disagree strongly.  If we move it to Wextra it's going to see a lot
less usage in real world codebases and potentially lead to the
re-introduction of a class of bugs that we've largely helped stomp out.

It's also the case that maybe uninitialized vs is uninitialized is
really just a function of CFG shape.  Give me any "maybe uninitialized"
case and I can turn it into a "is uninitialized" with simple block
duplication of the forms done by jump threading, path isolation,
superblock formation, etc.


> 
>>> People tend to use -Wall with -Werror (either explicitly 
>>> or implicitly by modifying the code until all warnings are gone). What I 
>>> see currently in projects where I participate is that 
>>> -Wmaybe-uninitialized is making things worse. People don't investigate 
>>> deeply the cause of the warning, they just go for whatever "quick-fix" 
>>> makes the compiler shut up. Quite often, this involves extra code that is 
>>> less readable and performs worse, while it didn't even "fix" what caused 
>>> the warning, it just randomly ended up with code where the compiler 
>>> doesn't warn (possibly because the function got bigger, which changed 
>>> inlining decisions...).
>>
>> Yes, using -Werror is usually a terrible idea.
Generally agreed in released versions of any code.  -Werror *may* be
appropriate in development versions depending on the project's policies,
procedures and quality of codebase.


Jeff

Re: Move -Wmaybe-uninitialized to -Wextra

[Marc Glisse]

On Fri, 1 Feb 2019, Jeff Law wrote:

> On 2/1/19 7:01 AM, Marek Polacek wrote:
>> On Fri, Feb 01, 2019 at 07:19:25AM -0600, Segher Boessenkool wrote:
>>> Hi Marc,
>>>
>>> On Fri, Feb 01, 2019 at 12:32:45PM +0100, Marc Glisse wrote:
>>>> -Wmaybe-uninitialized generates false positives, we can tweak the compiler
>>>> to reduce them, but there will always be some, that's in the nature of
>>>> this warning.
>>>
>>> That is true for *every* warning; if not, it should be an error, not a
>>> warning.
>>>
>>>> My opinion is that -Wmaybe-uninitialized would serve its purpose better as
>>>> part of -Wextra.
>>>
>>> +1
>>
>> +1 from me too.
> I disagree strongly.

I am not surprised, but I had to at least start the conversation. Would 
you mind providing a patch that changes the definition of -Wall, since the 
current one doesn't quite match reality anymore? Also, what do you 
recommend people do when they hit false positives?

> If we move it to Wextra it's going to see a lot less usage in real world 
> codebases

I am very tempted by the strawman: should we deprecate -Wextra since 
nobody uses it? (Sorry)

Ideally serious projects would use (parts of) -Wextra, at least 
occasionally, and with circumspection. But some warnings like 
-Wmaybe-uninitialized are dangerous tools in the hands of quickfix 
developers, and I am indeed trying to keep it out of their hands...

> and potentially lead to the re-introduction of a class of bugs that 
> we've largely helped stomp out.

That's very optimistic. -Wmaybe-uninitialized only detects a very small 
proportion of uninitialized uses. Also, my experience is that people have 
stomped out the warning, not the bugs. In some cases they even introduced 
bugs to stomp out false warnings, or made it harder to detect real bugs in 
the future, so the warning did more harm than good. I am mostly working on 
large C++ header-only template-heavy scientific libraries, it is quite 
possible that people who handle different types of code bases have a 
different experience, and -Wmaybe-uninitialized may have had a more 
positive impact on other projects.

> It's also the case that maybe uninitialized vs is uninitialized is
> really just a function of CFG shape.  Give me any "maybe uninitialized"
> case and I can turn it into a "is uninitialized" with simple block
> duplication of the forms done by jump threading, path isolation,
> superblock formation, etc.

Hmm, you know those things better than me, so you are probably right, but 
I am not seeing it. We omit "maybe" if, starting from the entry of the 
function, and barring exceptions, we know the statement will always be 
executed. If you take a false positive for maybe-uninitialized, i.e. a 
statement in a dead branch, I don't see how block duplication can make it 
so the statement is now always executed. The natural way to remove "maybe" 
is through function cloning or outlining. Then you can create functions 
that are never called, and any warning about those is a false positive.

There is a matter of statistics. In practice maybe-uninitialized has way 
more false positives than uninitialized, which makes it more problematic. 
But if you prefer to move both to -Wextra (this is the current default 
when front-ends don't override it), that's ok with me ;-)

-- 
Marc Glisse

Re: Move -Wmaybe-uninitialized to -Wextra

[Jeff Law]

On 2/1/19 1:31 PM, Marc Glisse wrote:
> 
> I am not surprised, but I had to at least start the conversation. Would
> you mind providing a patch that changes the definition of -Wall, since
> the current one doesn't quite match reality anymore? Also, what do you
> recommend people do when they hit false positives?
So I'm not sure how I'd change the definition of -Wall.

> 
>> If we move it to Wextra it's going to see a lot less usage in real
>> world codebases
> 
> I am very tempted by the strawman: should we deprecate -Wextra since
> nobody uses it? (Sorry)
:-)  I don't see a lot of use of -Wextra and I'm always disappointed
when we have to relegate useful warnings to it precisely because it gets
used so rarely.

But as I've indicated earlier on this thread, it may make sense to split
out uninitialized warnings on aggregates/addressables and relegate those
to -Wextra until we can really beef up analysis in that space.



> 
> Ideally serious projects would use (parts of) -Wextra, at least
> occasionally, and with circumspection. But some warnings like
> -Wmaybe-uninitialized are dangerous tools in the hands of quickfix
> developers, and I am indeed trying to keep it out of their hands...
Well, that's a different problem.  If the developers are doing quick,
dumb hacks to avoid a warning, then the developers need to be retrained.
 We shouldn't cater the warning set to make that class of developer
happy or less dangerous.

> 
>> and potentially lead to the re-introduction of a class of bugs that
>> we've largely helped stomp out.
> 
> That's very optimistic. -Wmaybe-uninitialized only detects a very small
> proportion of uninitialized uses. Also, my experience is that people
> have stomped out the warning, not the bugs. In some cases they even
> introduced bugs to stomp out false warnings, or made it harder to detect
> real bugs in the future, so the warning did more harm than good. I am
> mostly working on large C++ header-only template-heavy scientific
> libraries, it is quite possible that people who handle different types
> of code bases have a different experience, and -Wmaybe-uninitialized may
> have had a more positive impact on other projects.
I see things differently.  Specifically we've done a good job at making
the analysis good for simple objects (anything that's an SSA_NAME).
False positives happen, but are relatively rare and developers have
through the decades addressed most of these problems (we've had this
stuff in -Wall for 3 decades).

Are there cases where developers have screwed things up in the process,
certainly, but I suspect that's the exception rather than the rule.

The simple fact is that if we throw an uninitialized warning on a scalar
you *really* have to look at the code to figure out if it's a false
positive or not.  And you have to really think hard about how to
initialize the value if that's in fact what you end up doing -- there's
no guaranteed safe value and that's why I've resisted calls to have
"initialize everything to XXX" flags through the years.

On the addressable/aggregate side things are totally different.  We miss
tons of warnings, our ability to analyze things to avoid false positives
is poor at best, etc.


> 
>> It's also the case that maybe uninitialized vs is uninitialized is
>> really just a function of CFG shape.  Give me any "maybe uninitialized"
>> case and I can turn it into a "is uninitialized" with simple block
>> duplication of the forms done by jump threading, path isolation,
>> superblock formation, etc.
> 
> Hmm, you know those things better than me, so you are probably right,
> but I am not seeing it. We omit "maybe" if, starting from the entry of
> the function, and barring exceptions, we know the statement will always
> be executed. If you take a false positive for maybe-uninitialized, i.e.
> a statement in a dead branch, I don't see how block duplication can make
> it so the statement is now always executed. The natural way to remove
> "maybe" is through function cloning or outlining. Then you can create
> functions that are never called, and any warning about those is a false
> positive.
Well, that's the problem.  Right now the maybe vs always is based on if
the value flows through a PHI.   But I can avoid having the value flow
through a PHI with simple block copying and light CFG manipulation.  But
in both cases there's still a control dependency path to get to the
point where the uninitialized use happens.  And you also have the issue
of proving that a particular function even executes to begin with.

That's why I think the distinction simply doesn't make any sense.  We
should just call them all maybe-uninitialized uses.

Jeff

Re: Move -Wmaybe-uninitialized to -Wextra

[Eric Gallager]

On 2/20/19, Jeff Law <law@redhat.com> wrote:
> On 2/1/19 1:31 PM, Marc Glisse wrote:
>>
>> I am not surprised, but I had to at least start the conversation. Would
>> you mind providing a patch that changes the definition of -Wall, since
>> the current one doesn't quite match reality anymore? Also, what do you
>> recommend people do when they hit false positives?
> So I'm not sure how I'd change the definition of -Wall.
>
>>
>>> If we move it to Wextra it's going to see a lot less usage in real
>>> world codebases
>>
>> I am very tempted by the strawman: should we deprecate -Wextra since
>> nobody uses it? (Sorry)
> :-)  I don't see a lot of use of -Wextra and I'm always disappointed
> when we have to relegate useful warnings to it precisely because it gets
> used so rarely.

Part of that could be because they're still using the old name of "-W"
for it. Since the documentation says the name "-Wextra" is preferred,
maybe it's time to deprecate the old "-W" alias to get people to spell
it just one way; that would make it easier to count its usage.

>
> But as I've indicated earlier on this thread, it may make sense to split
> out uninitialized warnings on aggregates/addressables and relegate those
> to -Wextra until we can really beef up analysis in that space.
>
>
>
>>
>> Ideally serious projects would use (parts of) -Wextra, at least
>> occasionally, and with circumspection. But some warnings like
>> -Wmaybe-uninitialized are dangerous tools in the hands of quickfix
>> developers, and I am indeed trying to keep it out of their hands...
> Well, that's a different problem.  If the developers are doing quick,
> dumb hacks to avoid a warning, then the developers need to be retrained.
>  We shouldn't cater the warning set to make that class of developer
> happy or less dangerous.
>
>>
>>> and potentially lead to the re-introduction of a class of bugs that
>>> we've largely helped stomp out.
>>
>> That's very optimistic. -Wmaybe-uninitialized only detects a very small
>> proportion of uninitialized uses. Also, my experience is that people
>> have stomped out the warning, not the bugs. In some cases they even
>> introduced bugs to stomp out false warnings, or made it harder to detect
>> real bugs in the future, so the warning did more harm than good. I am
>> mostly working on large C++ header-only template-heavy scientific
>> libraries, it is quite possible that people who handle different types
>> of code bases have a different experience, and -Wmaybe-uninitialized may
>> have had a more positive impact on other projects.
> I see things differently.  Specifically we've done a good job at making
> the analysis good for simple objects (anything that's an SSA_NAME).
> False positives happen, but are relatively rare and developers have
> through the decades addressed most of these problems (we've had this
> stuff in -Wall for 3 decades).
>
> Are there cases where developers have screwed things up in the process,
> certainly, but I suspect that's the exception rather than the rule.
>
> The simple fact is that if we throw an uninitialized warning on a scalar
> you *really* have to look at the code to figure out if it's a false
> positive or not.  And you have to really think hard about how to
> initialize the value if that's in fact what you end up doing -- there's
> no guaranteed safe value and that's why I've resisted calls to have
> "initialize everything to XXX" flags through the years.
>
> On the addressable/aggregate side things are totally different.  We miss
> tons of warnings, our ability to analyze things to avoid false positives
> is poor at best, etc.
>
>
>>
>>> It's also the case that maybe uninitialized vs is uninitialized is
>>> really just a function of CFG shape.  Give me any "maybe uninitialized"
>>> case and I can turn it into a "is uninitialized" with simple block
>>> duplication of the forms done by jump threading, path isolation,
>>> superblock formation, etc.
>>
>> Hmm, you know those things better than me, so you are probably right,
>> but I am not seeing it. We omit "maybe" if, starting from the entry of
>> the function, and barring exceptions, we know the statement will always
>> be executed. If you take a false positive for maybe-uninitialized, i.e.
>> a statement in a dead branch, I don't see how block duplication can make
>> it so the statement is now always executed. The natural way to remove
>> "maybe" is through function cloning or outlining. Then you can create
>> functions that are never called, and any warning about those is a false
>> positive.
> Well, that's the problem.  Right now the maybe vs always is based on if
> the value flows through a PHI.   But I can avoid having the value flow
> through a PHI with simple block copying and light CFG manipulation.  But
> in both cases there's still a control dependency path to get to the
> point where the uninitialized use happens.  And you also have the issue
> of proving that a particular function even executes to begin with.
>
> That's why I think the distinction simply doesn't make any sense.  We
> should just call them all maybe-uninitialized uses.
>
> Jeff
>

Re: Move -Wmaybe-uninitialized to -Wextra

[Segher Boessenkool]

On Fri, Feb 01, 2019 at 12:27:57PM -0700, Jeff Law wrote:
> On 2/1/19 7:01 AM, Marek Polacek wrote:
> > On Fri, Feb 01, 2019 at 07:19:25AM -0600, Segher Boessenkool wrote:
> >> On Fri, Feb 01, 2019 at 12:32:45PM +0100, Marc Glisse wrote:
> >>> My opinion is that -Wmaybe-uninitialized would serve its purpose better as 
> >>> part of -Wextra.
> >>
> >> +1
> > 
> > +1 from me too.
> I disagree strongly.  If we move it to Wextra it's going to see a lot
> less usage in real world codebases and potentially lead to the
> re-introduction of a class of bugs that we've largely helped stomp out.

The usual workaround, especially for programs that build with multiple
(i.e. older) versions of GCC, is to initialise any such variable (to an
either or not useful value) early.  This doesn't fix the actual problem
usually (which is that your control flow is too complex).

> It's also the case that maybe uninitialized vs is uninitialized is
> really just a function of CFG shape.  Give me any "maybe uninitialized"
> case and I can turn it into a "is uninitialized" with simple block
> duplication of the forms done by jump threading, path isolation,
> superblock formation, etc.

Are you saying that -Wmaybe-uninitialized should be included in
-Wuninitialized, since it has no extra false positives?  That wasn't true
at all historically: -Wuninitialized has false positives on paths that
cannot be executed because of function preconditions, but
-Wmaybe-uninitialized used to warn for things that can be locally proven to
never execute, like
  if (a)
    b = 42;
  ...
  if (a)
    f(b);

> >> Yes, using -Werror is usually a terrible idea.
> Generally agreed in released versions of any code.  -Werror *may* be
> appropriate in development versions depending on the project's policies,
> procedures and quality of codebase.

IMO it is more useful it is much more useful if you make your build system
less noisy so that problems are more obvious, instead of breaking the build
for no reason all the time (see PR89162, etc. etc.)


Segher

Re: Move -Wmaybe-uninitialized to -Wextra

[Richard Biener]

On Sat, Feb 2, 2019 at 9:20 PM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Fri, Feb 01, 2019 at 12:27:57PM -0700, Jeff Law wrote:
> > On 2/1/19 7:01 AM, Marek Polacek wrote:
> > > On Fri, Feb 01, 2019 at 07:19:25AM -0600, Segher Boessenkool wrote:
> > >> On Fri, Feb 01, 2019 at 12:32:45PM +0100, Marc Glisse wrote:
> > >>> My opinion is that -Wmaybe-uninitialized would serve its purpose better as
> > >>> part of -Wextra.
> > >>
> > >> +1
> > >
> > > +1 from me too.
> > I disagree strongly.  If we move it to Wextra it's going to see a lot
> > less usage in real world codebases and potentially lead to the
> > re-introduction of a class of bugs that we've largely helped stomp out.
>
> The usual workaround, especially for programs that build with multiple
> (i.e. older) versions of GCC, is to initialise any such variable (to an
> either or not useful value) early.  This doesn't fix the actual problem
> usually (which is that your control flow is too complex).
>
> > It's also the case that maybe uninitialized vs is uninitialized is
> > really just a function of CFG shape.  Give me any "maybe uninitialized"
> > case and I can turn it into a "is uninitialized" with simple block
> > duplication of the forms done by jump threading, path isolation,
> > superblock formation, etc.
>
> Are you saying that -Wmaybe-uninitialized should be included in
> -Wuninitialized, since it has no extra false positives?  That wasn't true
> at all historically: -Wuninitialized has false positives on paths that
> cannot be executed because of function preconditions, but
> -Wmaybe-uninitialized used to warn for things that can be locally proven to
> never execute, like
>   if (a)
>     b = 42;
>   ...
>   if (a)
>     f(b);

IMHO the main reason of most false positives is that we rely on SSA
default-defs in PHIs which are spurious when definition and use are
not in domination relation as in
  if (p)
    a = 1;
  if (p)
    ... = a;
the uninit pass tries to catch those cases but that's obviously hard.

> > >> Yes, using -Werror is usually a terrible idea.
> > Generally agreed in released versions of any code.  -Werror *may* be
> > appropriate in development versions depending on the project's policies,
> > procedures and quality of codebase.
>
> IMO it is more useful it is much more useful if you make your build system
> less noisy so that problems are more obvious, instead of breaking the build
> for no reason all the time (see PR89162, etc. etc.)
>
>
> Segher

Re: Move -Wmaybe-uninitialized to -Wextra

[Jeff Law]

On 2/2/19 1:20 PM, Segher Boessenkool wrote:
> On Fri, Feb 01, 2019 at 12:27:57PM -0700, Jeff Law wrote:
>> On 2/1/19 7:01 AM, Marek Polacek wrote:
>>> On Fri, Feb 01, 2019 at 07:19:25AM -0600, Segher Boessenkool wrote:
>>>> On Fri, Feb 01, 2019 at 12:32:45PM +0100, Marc Glisse wrote:
>>>>> My opinion is that -Wmaybe-uninitialized would serve its purpose better as 
>>>>> part of -Wextra.
>>>>
>>>> +1
>>>
>>> +1 from me too.
>> I disagree strongly.  If we move it to Wextra it's going to see a lot
>> less usage in real world codebases and potentially lead to the
>> re-introduction of a class of bugs that we've largely helped stomp out.
> 
> The usual workaround, especially for programs that build with multiple
> (i.e. older) versions of GCC, is to initialise any such variable (to an
> either or not useful value) early.  This doesn't fix the actual problem
> usually (which is that your control flow is too complex).
> 
>> It's also the case that maybe uninitialized vs is uninitialized is
>> really just a function of CFG shape.  Give me any "maybe uninitialized"
>> case and I can turn it into a "is uninitialized" with simple block
>> duplication of the forms done by jump threading, path isolation,
>> superblock formation, etc.
> 
> Are you saying that -Wmaybe-uninitialized should be included in
> -Wuninitialized, since it has no extra false positives?  That wasn't true
> at all historically: -Wuninitialized has false positives on paths that
> cannot be executed because of function preconditions, but
> -Wmaybe-uninitialized used to warn for things that can be locally proven to
> never execute, like
>   if (a)
>     b = 42;
>   ...
>   if (a)
>     f(b);
No, I'm saying the distinction between maybe and always uninitialized is
a false distinction.  Code duplication can easily take something that
triggers a "maybe" warning and turn it into a "always" warning.  The
distinction between them is just bogus.



To take your example

if (a)
  b = 42
...
if (a)
  f(b);

Can be rewritten as

if (a) {
  b = 42
  [ copy of ...]
  f (b);
} else {
  ...
  f (b);
}

But more importantly consider when the value flows by way of a PHI
argument.  We consider those "maybe" warnings (and it's not just the
uninitialized warning analysis).  But fairly simple code duplication can
eliminate the flow via PHI and suddenly we go from a "maybe" to an
"always" warning.

Let's consider this...


x4 = PHI (x0(D), x1, x2, x3)
use x4;

That's a maybe uninitialized warning by way of the value flow of x0
through the PHI into x4.

But we can duplicate the block creating something like this:

BB:
x5 = PHI (x0(D))
use x5
goto join

BB:
x4 = PHI (x1, x2, x3)
use x4
goto join


Which we then simplify into

BB:
use x0(D)
goto join

BB:
x4 = PHI (x1, x2, x3)
use x4
goto join


That changes the warning from a maybe-uninitialized into an always
uninitialized.  Note that we still may or may not reach the use at
runtime.   THe distinction between maybe and always is just bogus.

Jeff

Re: Move -Wmaybe-uninitialized to -Wextra

[Michael Matz]

Hi,

On Wed, 20 Feb 2019, Jeff Law wrote:

> No, I'm saying the distinction between maybe and always uninitialized is
> a false distinction.  Code duplication can easily take something that
> triggers a "maybe" warning and turn it into a "always" warning.  The
> distinction between them is just bogus.
> 
> To take your example
> 
> if (a)
>   b = 42
> ...
> if (a)
>   f(b);
> 
> Can be rewritten as
> 
> if (a) {
>   b = 42
>   [ copy of ...]
>   f (b);
> } else {
>   ...
>   f (b);   // xxx
> }

No, it cannot.  It would introduce a call to f(b) in a path (at xxx) where 
it wasn't before.  Any transformation that transforms maybe-uninit into 
must-uninit is either wrong or introduces more-obviously-dead-than-before 
code, like in this variant of yours that is correct:

if (a) {
  b = 42
  [ copy of ...]
  f (b);
} else {
  ...
  if (a)     // clearly false, as dominated by if (!a)
    f (b);   // hence more obviously dead than in original code
}

> x4 = PHI (x0(D), x1, x2, x3)
> use x4;
> 
> That's a maybe uninitialized warning by way of the value flow of x0
> through the PHI into x4.

So clearly the use of x4 when reached via edge using x0(D) must be dead 
(or we have a real uninit use) but we can't differ because there's only 
one use of x4 statically ... nevertheless it's unobviously-dead.

> Which we then simplify into
> 
> BB:
> use x0(D)
> goto join
> 
> BB:
> x4 = PHI (x1, x2, x3)
> use x4
> goto join

And this makes the uninit use of x0(D) more obviously dead because now the 
problematic and non-problematic uses are separated.  So, the 
transformation introduced more-obviously-dead code, which it just as well 
could have removed/not generated, getting rid of the must-uninit 
warning.

I guess what I'm saying is that whenever a transformation is about to 
introduce a must-uninit from a maybe-uninit that instead all paths leading 
to that must-uninit should be removed instead.

> That changes the warning from a maybe-uninitialized into an always
> uninitialized.  Note that we still may or may not reach the use at
> runtime.   THe distinction between maybe and always is just bogus.

I think as stated that's a bit extreme :)  On user code before 
optimization the distinction is useful.  The problem stems from 
us generating warnings somewhen in the middle of the optimization 
pipeline.  As long as we continue doing that we will always have these 
different opinions about what is or isn't rightfully a maybe-uninit or not 
:-/


Ciao,
Michael.

Re: Move -Wmaybe-uninitialized to -Wextra

[Richard Biener]

On Fri, Feb 1, 2019 at 8:28 PM Jeff Law <law@redhat.com> wrote:
>
> On 2/1/19 7:01 AM, Marek Polacek wrote:
> > On Fri, Feb 01, 2019 at 07:19:25AM -0600, Segher Boessenkool wrote:
> >> Hi Marc,
> >>
> >> On Fri, Feb 01, 2019 at 12:32:45PM +0100, Marc Glisse wrote:
> >>> -Wmaybe-uninitialized generates false positives, we can tweak the compiler
> >>> to reduce them, but there will always be some, that's in the nature of
> >>> this warning.
> >>
> >> That is true for *every* warning; if not, it should be an error, not a
> >> warning.
> >>
> >>> My opinion is that -Wmaybe-uninitialized would serve its purpose better as
> >>> part of -Wextra.
> >>
> >> +1
> >
> > +1 from me too.
> I disagree strongly.  If we move it to Wextra it's going to see a lot
> less usage in real world codebases and potentially lead to the
> re-introduction of a class of bugs that we've largely helped stomp out.
>
> It's also the case that maybe uninitialized vs is uninitialized is
> really just a function of CFG shape.  Give me any "maybe uninitialized"
> case and I can turn it into a "is uninitialized" with simple block
> duplication of the forms done by jump threading, path isolation,
> superblock formation, etc.

Agreed.  Also note that we consider everything executed when function
entry is executed as not "maybe" even though functions can be called
conditionally.  So there's the artificial function boundary issue when
distinguishing maybe from must-be stuff as well.

Richard.

>
> >
> >>> People tend to use -Wall with -Werror (either explicitly
> >>> or implicitly by modifying the code until all warnings are gone). What I
> >>> see currently in projects where I participate is that
> >>> -Wmaybe-uninitialized is making things worse. People don't investigate
> >>> deeply the cause of the warning, they just go for whatever "quick-fix"
> >>> makes the compiler shut up. Quite often, this involves extra code that is
> >>> less readable and performs worse, while it didn't even "fix" what caused
> >>> the warning, it just randomly ended up with code where the compiler
> >>> doesn't warn (possibly because the function got bigger, which changed
> >>> inlining decisions...).
> >>
> >> Yes, using -Werror is usually a terrible idea.
> Generally agreed in released versions of any code.  -Werror *may* be
> appropriate in development versions depending on the project's policies,
> procedures and quality of codebase.
>
>
> Jeff

Re: Move -Wmaybe-uninitialized to -Wextra

[Martin Sebor]

On 2/1/19 4:32 AM, Marc Glisse wrote:
> Hello,
> 
> first, I expect this to be controversial, so feel free to complain.

I don't feel too strongly about whether -Wmaybe-uninitialized should
be in -Wall or in -Wextra, and I might even be somewhat more inclined
to expect to find it in the latter.  But since you sound like you are
gearing up for proposing other changes in the same vein down the line
where I might have stronger opinions, I should comment.

[It's a bit of a long-winded response because I've been thinking about
this topic a lot lately.]

In general, I think a discussion of warning groupings is worth having
(even needed) at the right time, but stage 4 doesn't strike me as
the most opportune occasion.

Specifically for -Wmaybe-uninitialized, the option has been in -Wall
for many releases, and no major changes to it have been made recently
that I know.  So what I'm missing in this proposal is: why now?  What
has changed to make this a pressing issue now?  Has its rate of false
positives gone up significantly?  If so, by how much and why?

> The description of -Wall says "This enables all the warnings about
> constructions that some users consider questionable, and that are easy
> to avoid (or modify to prevent the warning), even in conjunction with
> macros."
> 
> And the description of -Wmaybe-uninitialized "For an automatic variable,
> if there exists a path from the function entry to a use of the variable
> that is initialized, but there exist some other paths for which the
> variable is not initialized, the compiler emits a warning if it cannot
> prove the uninitialized paths are not executed at run time. These
> warnings are made optional because GCC is not smart enough to see all
> the reasons why the code might be correct in spite of appearing to have
> an error."
> 
> -Wmaybe-uninitialized generates false positives, we can tweak the 
> compiler to reduce them, but there will always be some, that's in the 
> nature of this warning.

Please be clear about what you mean by false positives.  Is it that
the warning triggers contrary to the documentation ("a path exists
where the variable is uninitialized along with one where it is"),
or that the path to the use of the variable does exist but we
(though not GCC) can tell from the context that it cannot be reached?
(The latter wouldn't qualify as a false positive as the term is defined
in literature; i.e., the warning works as designed, we just don't like
or agree with it.)

In practice, false positives (and negatives) of both kinds, whether
they fit the formal definition or the informal one, are the nature
of virtually all non-trivial static diagnostics, certainly all those
that depend on control or data flow analysis.  Some are due to bugs
or limitations in the implementation of the warning.  Others are
inherent in the technology.  Is this warning more prone to one kind
than others?  If so, is it because it's implemented poorly, or that
its implementation hasn't kept up with the improvements to
the optimizers, or has the infrastructure it depends on become
ill-suited for it to avoid some of the false positives (as formally
defined), or is it something else?

> These false positives are not easy to avoid, as required to be part of 
> -Wall. Avoiding them, when it is possible at all, requires not just a 
> syntactic tweak, like adding parentheses, but a semantic change that can 
> make the code worse. Initializing something that does not need it is 
> extra code (increases code size and running time). It also prevents 
> better tools from detecting true uninitialized uses, either static 
> analyzers or runtime checkers (sanitizer, valgrind).

I don't find the argument very compelling that diagnosing potential
bugs should be avoided because the fix (or the suppression in
the case of a false positive) could be wrong.   The risk exists with
all diagnostics.

I also take issue with the suggestion that dynamic analysis is
necessarily a superior mechanism for detecting bugs.  They each have
their strengths and weaknesses.  They are not an either-or proposition
but rather complementary solutions.

Lastly, in the case of uninitialized variables, the usual solution
of initializing them is trivial and always safe (some coding styles
even require it).   Initializing scalars has a negligible performance
impact in practice, and, if it's thought to be necessary, can easily
be done conditionally (as in, based on some macro) so that
the uninitialized accesses can still be detected by dynamic analysis.

> This message concentrates on the negatives, but that doesn't mean I 
> consider -Wmaybe-uninitialized as useless. It can find true 
> uninitialized uses. And even some false positives can point at places 
> where we can help the compiler generate better code (say with a default 
> __builtin_unreachable case in a switch). I did in the past contribute 
> patches to make it warn more often, and I might do so again in the future.

This paragraph makes me think you equate false positives from this
warning with those from -Wuninitialized.  They are (obviously) not
the same.  "May be unitialized" doesn't preclude the possibility that
the variable is not used uninitialized.  I'm not just nit-picking
here either.  It's important to use the same vocabulary.

> My opinion is that -Wmaybe-uninitialized would serve its purpose better 
> as part of -Wextra. People tend to use -Wall with -Werror (either 
> explicitly or implicitly by modifying the code until all warnings are 
> gone). What I see currently in projects where I participate is that 
> -Wmaybe-uninitialized is making things worse. People don't investigate 
> deeply the cause of the warning, they just go for whatever "quick-fix" 
> makes the compiler shut up. Quite often, this involves extra code that 
> is less readable and performs worse, while it didn't even "fix" what 
> caused the warning, it just randomly ended up with code where the 
> compiler doesn't warn (possibly because the function got bigger, which 
> changed inlining decisions...).

I agree that we need to be mindful of warnings leading to unnecessary
and perhaps inappropriate code changes (as the author of a few warnings
with this potential it has been on mind a lot).

At the same time, not taking the time to properly analyze diagnostics
and come up with the appropriate solutions seems like a problem that's
orthogonal to the warnings themselves.  Disabling warnings in hopes of
solving what's really a problem with the development process or
a culture of a subset of projects doesn't seem like an optimal solution.

> If the warning is not enabled by default so much but only when people 
> are ready to investigate any warning thoroughly, the quickfix mentality 
> is less likely to be present. People using -Wmaybe-uninitialized need to 
> be willing to ignore false positives, or disable them with pragmas.
> 
> Note that similar arguments may apply to some other warnings that 
> somehow made their way into -Wall when they shouldn't have, but for now 
> I am only proposing to move -Wmaybe-uninitialized. Some people tend to 
> consider that if a warning is not part of -Wall, it might as well not 
> exist. Obviously I disagree with that.

The description of -Wall is vague enough that arguments about which
warnings should or shouldn't be enabled by it are unavoidably subject
to individual biases and unlikely to lead to a consensus, either among
ourselves or, even less likely, among users.  A shared definition of
a false positive should be one of the very first steps to coming closer
to a consensus.  Real world (as opposed to anecdotal) data on the rates
of actual rates of false positives and negatives vs true positives would
be also most helpful, as would some consensus of the severity of
the bugs the true positives expose, as well as some objective measure
of the ease of suppression.  There probably are others but these would
be a start.

But one size never fits all, so even if we do manage to come to
a consensus among ourselves, it will not make everyone happy(*).
I wonder of we need a better solution than just two (or even four,
f you count enabled-by-default and disabled- by-default) more or
less arbitrarily selected buckets of warnings.

Martin

[*] There are nearly 30 open bugs in Bugzilla about options people
think should or shouldn't be enabled by -Wall or -Weextra, including
bug 89072 that request to enable -Wall and -Werror by default.

> -------
> 
> Now the actual patch. Surprisingly, the middle-end puts both 
> Wuninitialized and Wmaybe-uninitialized in Wextra, it is the C-family of 
> front-ends that puts them in Wall. It also makes Wuninitialized enable 
> Wmaybe-uninitialized, which is backwards (it made sense historically), 
> Wuninitialized has much fewer false positives, and if we are willing to 
> be warned about possibly uninitialized uses, we certainly also want 
> warnings about uninitialized uses that are certain. So I am switching 
> the enabling relation between those 2, and enabling only Wuninitialized 
> at Wall.
> 
> If the patch gets in, this will of course require a mention in the 
> release notes.
> 
> I changed a set of tests based on a mix of grep and seeing what failed 
> make check. The exact list may not be optimal.
> 
> gcc/ChangeLog:
> 
> 2019-02-01  Marc Glisse  <marc.glisse@inria.fr>
> 
>      * common.opt (Wuninitialized): Enable with Wmaybe-uninitialized.
>      (Wmaybe-uninitialized): Enable with Wextra.
>      * doc/invoke.texi: Update implications between Wuninitialized,
>      Wmaybe-uninitialized, Wall and Wextra.
> 
> gcc/c-family/ChangeLog:
> 
> 2019-02-01  Marc Glisse  <marc.glisse@inria.fr>
> 
>      * c.opt (Wmaybe-uninitialized): Enable with Wextra.
> 
> gcc/testsuite/ChangeLog:
> 
> 2019-02-01  Marc Glisse  <marc.glisse@inria.fr>
> 
>      * c-c++-common/pr69543-1.c: Use -Wmaybe-uninitialized.
>      * c-c++-common/pr69543-2.c: Likewise.
>      * c-c++-common/pr69543-3.c: Likewise.
>      * c-c++-common/pr69543-4.c: Likewise.
>      * c-c++-common/uninit-17.c: Likewise.
>      * g++.dg/pr48484.C: Likewise.
>      * g++.dg/uninit-pred-1_b.C: Likewise.
>      * g++.dg/uninit-pred-2_b.C: Likewise.
>      * g++.dg/uninit-pred-3_b.C: Likewise.
>      * g++.dg/warn/Wuninitialized-5.C: Likewise.
>      * g++.dg/warn/Wuninitialized-6.C: Likewise.
>      * g++.dg/warn/Wuninitialized-9.C: Likewise.
>      * gcc.dg/gomp/pr72781.c: Likewise.
>      * gcc.dg/pr18501.c: Likewise.
>      * gcc.dg/pr39666-2.c: Likewise.
>      * gcc.dg/pr45083.c: Likewise.
>      * gcc.dg/pr57149.c: Likewise.
>      * gcc.dg/pr59924.c: Likewise.
>      * gcc.dg/pr69543.c: Likewise.
>      * gcc.dg/uninit-11-O0.c: Likewise.
>      * gcc.dg/uninit-11.c: Likewise.
>      * gcc.dg/uninit-16.c: Likewise.
>      * gcc.dg/uninit-17.c: Likewise.
>      * gcc.dg/uninit-18.c: Likewise.
>      * gcc.dg/uninit-19.c: Likewise.
>      * gcc.dg/uninit-6-O0.c: Likewise.
>      * gcc.dg/uninit-pr19430-2.c: Likewise.
>      * gcc.dg/uninit-pr19430-O0.c: Likewise.
>      * gcc.dg/uninit-pr19430.c: Likewise.
>      * gcc.dg/uninit-pr20644-O0.c: Likewise.
>      * gcc.dg/uninit-pred-2_a.c: Likewise.
>      * gcc.dg/uninit-pred-2_b.c: Likewise.
>      * gcc.dg/uninit-pred-2_c.c: Likewise.
>      * gcc.dg/uninit-pred-3_a.c: Likewise.
>      * gcc.dg/uninit-pred-3_b.c: Likewise.
>      * gcc.dg/uninit-pred-3_c.c: Likewise.
>      * gcc.dg/uninit-pred-3_d.c: Likewise.
>      * gcc.dg/uninit-pred-3_e.c: Likewise.
>      * gcc.dg/uninit-pred-4_a.c: Likewise.
>      * gcc.dg/uninit-pred-4_b.c: Likewise.
>      * gcc.dg/uninit-pred-5_a.c: Likewise.
>      * gcc.dg/uninit-pred-5_b.c: Likewise.
>      * gcc.dg/uninit-pred-6_a.c: Likewise.
>      * gcc.dg/uninit-pred-6_b.c: Likewise.
>      * gcc.dg/uninit-pred-6_c.c: Likewise.
>      * gcc.dg/uninit-pred-6_d.c: Likewise.
>      * gcc.dg/uninit-pred-6_e.c: Likewise.
>      * gcc.dg/uninit-pred-7_a.c: Likewise.
>      * gcc.dg/uninit-pred-7_b.c: Likewise.
>      * gcc.dg/uninit-pred-7_c.c: Likewise.
>      * gcc.dg/uninit-pred-7_d.c: Likewise.
>      * gcc.dg/uninit-pred-8_a.c: Likewise.
>      * gcc.dg/uninit-pred-8_b.c: Likewise.
>      * gcc.dg/uninit-pred-8_c.c: Likewise.
>      * gcc.dg/uninit-pred-8_d.c: Likewise.
>      * gcc.dg/uninit-pred-9_a.c: Likewise.
>      * gcc.dg/uninit-pred-9_b.c: Likewise.
>      * gcc.dg/uninit-suppress_2.c: Likewise.
>      * gfortran.dg/pr25923.f90: Likewise.
>      * gfortran.dg/pr39666-2.f90: Likewise.
> 

Re: Move -Wmaybe-uninitialized to -Wextra

[Marc Glisse]

On Sat, 2 Feb 2019, Martin Sebor wrote:

> I don't feel too strongly about whether -Wmaybe-uninitialized should
> be in -Wall or in -Wextra, and I might even be somewhat more inclined
> to expect to find it in the latter.  But since you sound like you are
> gearing up for proposing other changes in the same vein down the line
> where I might have stronger opinions, I should comment.
>
> [It's a bit of a long-winded response because I've been thinking about
> this topic a lot lately.]

Thank you for taking the time to write this down.

> In general, I think a discussion of warning groupings is worth having
> (even needed) at the right time, but stage 4 doesn't strike me as
> the most opportune occasion.
>
> Specifically for -Wmaybe-uninitialized, the option has been in -Wall
> for many releases, and no major changes to it have been made recently
> that I know.  So what I'm missing in this proposal is: why now?  What
> has changed to make this a pressing issue now?  Has its rate of false
> positives gone up significantly?  If so, by how much and why?

I've been wanting to post this for years, but I only got motivated enough 
recently after seeing negative effects of -Wmaybe-uninitialized in two 
projects within a few days. I don't think the rate of false positives has 
gone up significantly in gcc-9, they just randomly appear or disappear 
from one release to the next. In some sense, for projects that support 
multiple compiler versions, each new gcc release makes the number of false 
positives (on at least one version) go up, but that doesn't count as the 
warning getting worse.

The discussion and/or the change don't need to happen now, but if I didn't 
post this patch while motivated, it might be years before I actually do 
it, so I ignored stage 4.

> Please be clear about what you mean by false positives.  Is it that
> the warning triggers contrary to the documentation ("a path exists
> where the variable is uninitialized along with one where it is"),
> or that the path to the use of the variable does exist but we
> (though not GCC) can tell from the context that it cannot be reached?

The latter.

> (The latter wouldn't qualify as a false positive as the term is defined
> in literature; i.e., the warning works as designed, we just don't like
> or agree with it.)

Indeed the definition of false positive is important. In some sense, no 
warning is ever a false positive, when its definition is "the compiler 
decided to warn". But that makes the warning useless. 
-Wmaybe-uninitialized does not say there is an uninitialized use, it says 
the control flow is too complicated or the compiler not clever enough (or 
there is indeed an uninitialized use for some input, and only this subcase 
is a bug). Another warning, -Wstrict-overflow (after it moved to the 
middle-end), was more of an optimization note than a true warning: 
"warning: assuming your code is valid when generating code", Duh! Was I 
supposed to make it invalid to quiet the warning?

> In practice, false positives (and negatives) of both kinds, whether
> they fit the formal definition or the informal one, are the nature
> of virtually all non-trivial static diagnostics, certainly all those
> that depend on control or data flow analysis.  Some are due to bugs
> or limitations in the implementation of the warning.  Others are
> inherent in the technology.

Yes, and I argue that these warnings belong in a different "level" of 
warnings than the trivial warnings.

> Is this warning more prone to one kind than others?  If so, is it 
> because it's implemented poorly, or that its implementation hasn't kept 
> up with the improvements to the optimizers, or has the infrastructure it 
> depends on become ill-suited for it to avoid some of the false positives 
> (as formally defined), or is it something else?

I am mostly looking at what I see in practice. I regularly see false 
positives of -Wstrict-overflow (less now that I neutered parts of it) and 
-Wmaybe-uninitialized. And when I have strange crashes that later turn out 
to be caused by uninitialized memory uses, it is very seldom that 
-Wmaybe-uninitialized helps detect them. IIRC, last time, I had slightly 
more success with -Wreturn-local-addr -O3 -fkeep-inline-functions 
-fkeep-static-functions.

I think several of the new middle-end warnings you added are about string 
manipulation. I don't do any of that, which may explain why I am not 
seeing them. Also, we are not using gcc-9 much yet.

>> These false positives are not easy to avoid, as required to be part of 
>> -Wall. Avoiding them, when it is possible at all, requires not just a 
>> syntactic tweak, like adding parentheses, but a semantic change that can 
>> make the code worse. Initializing something that does not need it is extra 
>> code (increases code size and running time). It also prevents better tools 
>> from detecting true uninitialized uses, either static analyzers or runtime 
>> checkers (sanitizer, valgrind).
>
> I don't find the argument very compelling that diagnosing potential
> bugs should be avoided because the fix (or the suppression in
> the case of a false positive) could be wrong.   The risk exists with
> all diagnostics.

Yes, but in practice I haven't seen people get the parentheses wrong after 
a warning from -Wparentheses.

> I also take issue with the suggestion that dynamic analysis is
> necessarily a superior mechanism for detecting bugs.  They each have
> their strengths and weaknesses.  They are not an either-or proposition
> but rather complementary solutions.

I didn't claim they were necessarily superior, and I started my 
enumeration of potential better tools with static analyzers, i.e. like 
gcc's -Wmaybe-uninitialized, but really focused on this and not on 
optimization. Dynamic analysis has the advantage that it avoids dead 
branches and thus a whole class of false positives, but indeed it is also 
a weakness that it misses a lot of possible paths not covered during 
testing. I agree 100% that they are complementary.

> Lastly, in the case of uninitialized variables, the usual solution
> of initializing them is trivial and always safe (some coding styles
> even require it).

Here it shows that we don't work with the same type of code at all. If I 
am using a boost::optional, i.e. a class with a buffer and a boolean that 
says if the buffer is initialized, how do I initialize the (private) 
buffer? Or should boost itself zero out the buffer whenever the boolean is 
set to false? The variables can easily be hidden behind dozens of levels 
of abstraction that make them hard to initialize, and there may be nothing 
meaningful to initialize them with. Uninitialized also includes clobbered 
(out-of-scope for instance) in gcc, where it isn't clear what you are 
supposed to initialize to quiet the warning.

By the way, we usually try to avoid imposing a coding style, although 
that's not always true.

> Initializing scalars has a negligible performance
> impact in practice, and, if it's thought to be necessary, can easily
> be done conditionally (as in, based on some macro) so that
> the uninitialized accesses can still be detected by dynamic analysis.

I'd rather avoid increasing yet again the uses of macros, but yes, when 
initializing to quiet what has been identified as a false positive, that 
seems desirable. And it can be arrays instead of scalars.

>> This message concentrates on the negatives, but that doesn't mean I 
>> consider -Wmaybe-uninitialized as useless. It can find true uninitialized 
>> uses. And even some false positives can point at places where we can help 
>> the compiler generate better code (say with a default __builtin_unreachable 
>> case in a switch). I did in the past contribute patches to make it warn 
>> more often, and I might do so again in the future.
>
> This paragraph makes me think you equate false positives from this
> warning with those from -Wuninitialized.

Uh? No, I don't know what gave you this impression.

> "May be unitialized" doesn't preclude the possibility that the variable 
> is not used uninitialized.

But if the variable can never be used uninitialized and gcc is simply 
unable to prove it, the warning is bad. It may not be a "false positive" 
depending on your definition, but it is undesirable.

>> My opinion is that -Wmaybe-uninitialized would serve its purpose better as 
>> part of -Wextra. People tend to use -Wall with -Werror (either explicitly 
>> or implicitly by modifying the code until all warnings are gone). What I 
>> see currently in projects where I participate is that -Wmaybe-uninitialized 
>> is making things worse. People don't investigate deeply the cause of the 
>> warning, they just go for whatever "quick-fix" makes the compiler shut up. 
>> Quite often, this involves extra code that is less readable and performs 
>> worse, while it didn't even "fix" what caused the warning, it just randomly 
>> ended up with code where the compiler doesn't warn (possibly because the 
>> function got bigger, which changed inlining decisions...).
>
> I agree that we need to be mindful of warnings leading to unnecessary
> and perhaps inappropriate code changes (as the author of a few warnings
> with this potential it has been on mind a lot).
>
> At the same time, not taking the time to properly analyze diagnostics
> and come up with the appropriate solutions seems like a problem that's
> orthogonal to the warnings themselves.  Disabling warnings in hopes of
> solving what's really a problem with the development process or
> a culture of a subset of projects doesn't seem like an optimal solution.

I agree with that. But if I complain about the mindless quickfixes, the 
reply is that customers ask for a library that does not warn, and the 
developers do not have infinite time, so they do the minimum to avoid 
warnings. This is strongly linked to the warning levels.

One could recommend that customers include the library with -isystem to 
disable the warnings inside the library, bug that's fragile (though it has 
gotten quite good), and most importantly it also disables some warnings 
that are relevant to the user.

>> If the warning is not enabled by default so much but only when people are 
>> ready to investigate any warning thoroughly, the quickfix mentality is less 
>> likely to be present. People using -Wmaybe-uninitialized need to be willing 
>> to ignore false positives, or disable them with pragmas.
>> 
>> Note that similar arguments may apply to some other warnings that somehow 
>> made their way into -Wall when they shouldn't have, but for now I am only 
>> proposing to move -Wmaybe-uninitialized. Some people tend to consider that 
>> if a warning is not part of -Wall, it might as well not exist. Obviously I 
>> disagree with that.
>
> The description of -Wall is vague enough that arguments about which
> warnings should or shouldn't be enabled by it are unavoidably subject
> to individual biases and unlikely to lead to a consensus, either among
> ourselves or, even less likely, among users.

True. To me, one of the key documented properties of -Wall is being "easy 
to avoid". -Wall -Werror should be a usable combination. Anything not easy 
to avoid belongs in a different class, say -Wextra. We can rename that to 
W1/W2/... if Wall is too charged with historical meaning.

One issue is that, especially in a large, long-lived code base, people 
want to know when a change (or a new compiler) introduces a new warning. 
However, they do not want to have to go through hundreds or pre-existing, 
already analysed warnings. The easiest way to do that is to make sure 
there are no warnings at all (-Werror). For speculative warnings, that's 
really not convenient. I don't think there is a good solution to this 
(marking with a pragma the known false positives may be hard when 
instantiating some class somewhere causes a warning in some almost 
unrelated place, and you want the marking to be narrow enough that it 
won't also disable other warnings), and it is independent on whether the 
warning is in -Wall or -Wextra. I just have a feeling that splitting the 
easy and the not-easy warnings may help developers handle them 
differently.

> A shared definition of a false positive should be one of the very first 
> steps to coming closer to a consensus.  Real world (as opposed to 
> anecdotal) data on the rates of actual rates of false positives and 
> negatives vs true positives would be also most helpful, as would some 
> consensus of the severity of the bugs the true positives expose, as well 
> as some objective measure of the ease of suppression.  There probably 
> are others but these would be a start.

This data is going to be super hard to get. Most projects have been 
compiling for years and tweaking their code to avoid some warnings. We do 
not get to see the code that people originally write, we can only see what 
they commit.

-- 
Marc Glisse

Re: Move -Wmaybe-uninitialized to -Wextra

[Martin Sebor]

>> In practice, false positives (and negatives) of both kinds, whether
>> they fit the formal definition or the informal one, are the nature
>> of virtually all non-trivial static diagnostics, certainly all those
>> that depend on control or data flow analysis.  Some are due to bugs
>> or limitations in the implementation of the warning.  Others are
>> inherent in the technology.
> 
> Yes, and I argue that these warnings belong in a different "level" of 
> warnings than the trivial warnings.

Introducing more levels sounds fine to me.  I wouldn't want to see
a more permissive default; my preference would be the opposite,
leaving it to projects to adjust.

> 
>> Lastly, in the case of uninitialized variables, the usual solution
>> of initializing them is trivial and always safe (some coding styles
>> even require it).
> 
> Here it shows that we don't work with the same type of code at all. If I 
> am using a boost::optional, i.e. a class with a buffer and a boolean 
> that says if the buffer is initialized, how do I initialize the 
> (private) buffer? Or should boost itself zero out the buffer whenever 
> the boolean is set to false? The variables can easily be hidden behind 
> dozens of levels of abstraction that make them hard to initialize, and 
> there may be nothing meaningful to initialize them with. Uninitialized 
> also includes clobbered (out-of-scope for instance) in gcc, where it 
> isn't clear what you are supposed to initialize to quiet the warning.

You're right that this is hard to imagine without first hand experience
with the problem.  If this is a common pattern with the warning in C++
class templates in general, a representative test case would help get
a better appreciation of the problem and might also give us an idea
of a better solution.  (If there is one in Bugzilla please point me
at it.)

>>> This message concentrates on the negatives, but that doesn't mean I 
>>> consider -Wmaybe-uninitialized as useless. It can find true 
>>> uninitialized uses. And even some false positives can point at places 
>>> where we can help the compiler generate better code (say with a 
>>> default __builtin_unreachable case in a switch). I did in the past 
>>> contribute patches to make it warn more often, and I might do so 
>>> again in the future.
>>
>> This paragraph makes me think you equate false positives from this
>> warning with those from -Wuninitialized.
> 
> Uh? No, I don't know what gave you this impression.

It's common to view as a false positive (or "bad" as you write below)
every instance of a warning designed to detect or prevent bugs that 
doesn't indicate one (as opposed to warnings designed to help write
better/clearer code like -Wparentheses).

>> "May be unitialized" doesn't preclude the possibility that the 
>> variable is not used uninitialized.
> 
> But if the variable can never be used uninitialized and gcc is simply 
> unable to prove it, the warning is bad. It may not be a "false positive" 
> depending on your definition, but it is undesirable.

Ideally each instance of every warning designed to find bugs would
point out one.  But 100% accuracy or a zero rate of the undesirable
instances is not attainable in general, and reducing their rate at
the expense of the helpful ones also isn't the best tradeoff either.
The challenge is striking the right balance between their ratios.
(Only if that can't done then it might be time to consider
disabling/demoting the warning.  I don't have the impression
we are at that point with -Wmaybe-uninitialized but I haven't
done any research.)

>>> My opinion is that -Wmaybe-uninitialized would serve its purpose 
>>> better as part of -Wextra. People tend to use -Wall with -Werror 
>>> (either explicitly or implicitly by modifying the code until all 
>>> warnings are gone). What I see currently in projects where I 
>>> participate is that -Wmaybe-uninitialized is making things worse. 
>>> People don't investigate deeply the cause of the warning, they just 
>>> go for whatever "quick-fix" makes the compiler shut up. Quite often, 
>>> this involves extra code that is less readable and performs worse, 
>>> while it didn't even "fix" what caused the warning, it just randomly 
>>> ended up with code where the compiler doesn't warn (possibly because 
>>> the function got bigger, which changed inlining decisions...).
>>
>> I agree that we need to be mindful of warnings leading to unnecessary
>> and perhaps inappropriate code changes (as the author of a few warnings
>> with this potential it has been on mind a lot).
>>
>> At the same time, not taking the time to properly analyze diagnostics
>> and come up with the appropriate solutions seems like a problem that's
>> orthogonal to the warnings themselves.  Disabling warnings in hopes of
>> solving what's really a problem with the development process or
>> a culture of a subset of projects doesn't seem like an optimal solution.
> 
> I agree with that. But if I complain about the mindless quickfixes, the 
> reply is that customers ask for a library that does not warn, and the 
> developers do not have infinite time, so they do the minimum to avoid 
> warnings. This is strongly linked to the warning levels.
> 
> One could recommend that customers include the library with -isystem to 
> disable the warnings inside the library, bug that's fragile (though it 
> has gotten quite good), and most importantly it also disables some 
> warnings that are relevant to the user.

Right, that wouldn't be much better than disabling the warning on
the command line (or us removing it from -Wall and perhaps also
from -Wextra).

So since mindless quick fixes aren't the way to go and assuming we
agree that warnings have value even with some noise, is demoting
them to lower levels because they're not always used properly
the best solution?  No predetermined system of warning levels is
going to make everyone happy.  Different projects have different
constraints and tolerances for noise, or even resources to fix
real bugs -- GCC with over 400 wrong-code bugs in Open status
being a case in point, so a level that works for one, like
a library, may be overly pedantic for an application.

More warning levels might help.  Warning profiles (say one for
system libraries or freestanding code like the kernel, another
for C++ class libraries, yet another for applications) might
be another approach.  Others might be worth brainstorming about.

>>> If the warning is not enabled by default so much but only when people 
>>> are ready to investigate any warning thoroughly, the quickfix 
>>> mentality is less likely to be present. People using 
>>> -Wmaybe-uninitialized need to be willing to ignore false positives, 
>>> or disable them with pragmas.
>>>
>>> Note that similar arguments may apply to some other warnings that 
>>> somehow made their way into -Wall when they shouldn't have, but for 
>>> now I am only proposing to move -Wmaybe-uninitialized. Some people 
>>> tend to consider that if a warning is not part of -Wall, it might as 
>>> well not exist. Obviously I disagree with that.
>>
>> The description of -Wall is vague enough that arguments about which
>> warnings should or shouldn't be enabled by it are unavoidably subject
>> to individual biases and unlikely to lead to a consensus, either among
>> ourselves or, even less likely, among users.
> 
> True. To me, one of the key documented properties of -Wall is being 
> "easy to avoid". -Wall -Werror should be a usable combination. Anything 
> not easy to avoid belongs in a different class, say -Wextra. We can 
> rename that to W1/W2/... if Wall is too charged with historical meaning.

I would have expected dealing with -Wmaybe-uninitialized to always
be easy.  Your comment about std::optional suggests there are cases
when it isn't.  Can we look at some of those cases and see if there
is something we can do to make it easier?

>> A shared definition of a false positive should be one of the very 
>> first steps to coming closer to a consensus.  Real world (as opposed 
>> to anecdotal) data on the rates of actual rates of false positives and 
>> negatives vs true positives would be also most helpful, as would some 
>> consensus of the severity of the bugs the true positives expose, as 
>> well as some objective measure of the ease of suppression.  There 
>> probably are others but these would be a start.
> 
> This data is going to be super hard to get. Most projects have been 
> compiling for years and tweaking their code to avoid some warnings. We 
> do not get to see the code that people originally write, we can only see 
> what they commit.

It's not perfect but we should be able to get a rough idea based
on bug reports in Bugzilla.  Many distros rebuild the world before
a GCC release and new instances of build-breaking warnings tend to
get filed there.  Meta-bugs can be helpful as the first step.
A finer-grained classification by search terms (e.g., "bogus"
"spurious", etc.) helps narrow it down even more.

Martin

Re: Move -Wmaybe-uninitialized to -Wextra

[Marc Glisse]

On Mon, 4 Feb 2019, Martin Sebor wrote:

>>> In practice, false positives (and negatives) of both kinds, whether
>>> they fit the formal definition or the informal one, are the nature
>>> of virtually all non-trivial static diagnostics, certainly all those
>>> that depend on control or data flow analysis.  Some are due to bugs
>>> or limitations in the implementation of the warning.  Others are
>>> inherent in the technology.
>> 
>> Yes, and I argue that these warnings belong in a different "level" of 
>> warnings than the trivial warnings.
>
> Introducing more levels sounds fine to me.  I wouldn't want to see
> a more permissive default; my preference would be the opposite,
> leaving it to projects to adjust.

And I'd rather suggest as default something that can be combined with 
-Werror (not that you should do it, just that you could), leaving the 
non-trivial warnings that require motivation to investigate properly to 
people who have at least the motivation to enable extra flags. We can only 
agree to disagree...

>>> Lastly, in the case of uninitialized variables, the usual solution
>>> of initializing them is trivial and always safe (some coding styles
>>> even require it).
>> 
>> Here it shows that we don't work with the same type of code at all. If I am 
>> using a boost::optional, i.e. a class with a buffer and a boolean that says 
>> if the buffer is initialized, how do I initialize the (private) buffer? Or 
>> should boost itself zero out the buffer whenever the boolean is set to 
>> false? The variables can easily be hidden behind dozens of levels of 
>> abstraction that make them hard to initialize, and there may be nothing 
>> meaningful to initialize them with. Uninitialized also includes clobbered 
>> (out-of-scope for instance) in gcc, where it isn't clear what you are 
>> supposed to initialize to quiet the warning.
>
> You're right that this is hard to imagine without first hand experience
> with the problem.  If this is a common pattern with the warning in C++
> class templates in general, a representative test case would help get
> a better appreciation of the problem and might also give us an idea
> of a better solution.  (If there is one in Bugzilla please point me
> at it.)

Looking for "optional" and "-Wmaybe-uninitialized" shows
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78044
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

Google also gives
https://www.boost.org/doc/libs/1_69_0/libs/optional/doc/html/boost_optional/tutorial/gotchas/false_positive_with__wmaybe_uninitialized.html
https://sourceware.org/ml/gdb-patches/2017-05/msg00130.html
etc

And that's just for using a type called 'optional' (3 implementations of 
it).

> It's common to view as a false positive (or "bad" as you write below)
> every instance of a warning designed to detect or prevent bugs that doesn't 
> indicate one (as opposed to warnings designed to help write
> better/clearer code like -Wparentheses).

Well, warnings can't be completely arbitrary, they have to serve a 
purpose. Preventing some control flows where the use is not dominated by 
the initialization clearly enough could be one, but in my experience, on 
average, this does not lead to better programs for any metric besides the 
number of warnings. Again, experience can vary.

> Ideally each instance of every warning designed to find bugs would
> point out one.  But 100% accuracy or a zero rate of the undesirable
> instances is not attainable in general, and reducing their rate at
> the expense of the helpful ones also isn't the best tradeoff either.
> The challenge is striking the right balance between their ratios.

Yes.

> (Only if that can't done then it might be time to consider
> disabling/demoting the warning.  I don't have the impression
> we are at that point with -Wmaybe-uninitialized but I haven't
> done any research.)

It seems to depend on the type of code base, unsurprisingly.

> So since mindless quick fixes aren't the way to go and assuming we
> agree that warnings have value even with some noise, is demoting
> them to lower levels because they're not always used properly
> the best solution?  No predetermined system of warning levels is
> going to make everyone happy.  Different projects have different
> constraints and tolerances for noise, or even resources to fix
> real bugs -- GCC with over 400 wrong-code bugs in Open status
> being a case in point, so a level that works for one, like
> a library, may be overly pedantic for an application.

True. The main point is that there is a feeling that "gcc recommends using 
-Wall and fixing **all** those warnings **easily**" that does not exist 
(to the same level) with -Wextra. I guess that's not such a strong 
argument (you can probably feel that I've already given up).


Coincidentally, yesterday, I had to debug a crash in one of my programs. 
No warning, whatever I tried. At first, -fsanitize=address didn't give 
anything either. Then I found out about 
ASAN_OPTIONS=detect_stack_use_after_return=1, which pointed out super 
precisely which local variable I was accessing from where, which was then 
trivial to understand. So there are cases where I wish we were more 
verbose by default. Unless I misunderstand, this is disabled because it 
slows things down a bit. However the slowdown does not seem that big, 
people who care could always tweak it and are unlikely to use the defaults 
anyway. But I have almost no experience with it yet.

-- 
Marc Glisse

Re: Move -Wmaybe-uninitialized to -Wextra

[Martin Jambor]

Hi,

On Mon, Feb 04 2019, Marc Glisse wrote:
> On Mon, 4 Feb 2019, Martin Sebor wrote:
>>

...

>> You're right that this is hard to imagine without first hand experience
>> with the problem.  If this is a common pattern with the warning in C++
>> class templates in general, a representative test case would help get
>> a better appreciation of the problem and might also give us an idea
>> of a better solution.  (If there is one in Bugzilla please point me
>> at it.)
>
> Looking for "optional" and "-Wmaybe-uninitialized" shows
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78044
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635
>
> Google also gives
> https://www.boost.org/doc/libs/1_69_0/libs/optional/doc/html/boost_optional/tutorial/gotchas/false_positive_with__wmaybe_uninitialized.html
> https://sourceware.org/ml/gdb-patches/2017-05/msg00130.html
> etc
>
> And that's just for using a type called 'optional' (3 implementations of 
> it).

from my very quick reading of the first googled testcase, I assume the
instance of the optional class got SRAed and a warning was generated for
what originally was a class member, which indeed is not easy to
initialize on its own in order to avoid the warning.

Would it perhaps make sense to split the -Wmaybe-uninitialized warning
into two, one for scalars that are scalars in the original code and one
for SRA-created scalars and move only the latter to -Wextra?

Martin

Re: Move -Wmaybe-uninitialized to -Wextra

[Eric Gallager]

On 2/4/19, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> On Mon, Feb 04 2019, Marc Glisse wrote:
>> On Mon, 4 Feb 2019, Martin Sebor wrote:
>>>
>
> ...
>
>>> You're right that this is hard to imagine without first hand experience
>>> with the problem.  If this is a common pattern with the warning in C++
>>> class templates in general, a representative test case would help get
>>> a better appreciation of the problem and might also give us an idea
>>> of a better solution.  (If there is one in Bugzilla please point me
>>> at it.)
>>
>> Looking for "optional" and "-Wmaybe-uninitialized" shows
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78044
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635
>>
>> Google also gives
>> https://www.boost.org/doc/libs/1_69_0/libs/optional/doc/html/boost_optional/tutorial/gotchas/false_positive_with__wmaybe_uninitialized.html
>> https://sourceware.org/ml/gdb-patches/2017-05/msg00130.html
>> etc
>>
>> And that's just for using a type called 'optional' (3 implementations of
>> it).
>
> from my very quick reading of the first googled testcase, I assume the
> instance of the optional class got SRAed and a warning was generated for
> what originally was a class member, which indeed is not easy to
> initialize on its own in order to avoid the warning.
>
> Would it perhaps make sense to split the -Wmaybe-uninitialized warning
> into two, one for scalars that are scalars in the original code and one
> for SRA-created scalars and move only the latter to -Wextra?

What would the new warning names be once split?

>
> Martin
>

Re: Move -Wmaybe-uninitialized to -Wextra

[Marc Glisse]

On Mon, 4 Feb 2019, Martin Jambor wrote:

>> Looking for "optional" and "-Wmaybe-uninitialized" shows
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78044
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635
>>
>> Google also gives
>> https://www.boost.org/doc/libs/1_69_0/libs/optional/doc/html/boost_optional/tutorial/gotchas/false_positive_with__wmaybe_uninitialized.html
>> https://sourceware.org/ml/gdb-patches/2017-05/msg00130.html
>> etc
>>
>> And that's just for using a type called 'optional' (3 implementations of
>> it).
>
> from my very quick reading of the first googled testcase, I assume the
> instance of the optional class got SRAed and a warning was generated for
> what originally was a class member, which indeed is not easy to
> initialize on its own in order to avoid the warning.

Hmm, SRA and -Wmaybe-uninitialized, I saw that recently in 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66459

> Would it perhaps make sense to split the -Wmaybe-uninitialized warning
> into two, one for scalars that are scalars in the original code and one
> for SRA-created scalars and move only the latter to -Wextra?

If SRA is really the main source of false positives (I don't know about 
that), maybe. But I am afraid we will also miss a significant proportion 
of the already rare true positives that -Wmaybe-uninitialized currently 
finds. I really have no idea if such a split would work great or badly 
(sorry, I am not being very useful).

-- 
Marc Glisse

Re: Move -Wmaybe-uninitialized to -Wextra

[Jeff Law]

On 2/4/19 11:24 PM, Marc Glisse wrote:
> On Mon, 4 Feb 2019, Martin Jambor wrote:
> 
>>> Looking for "optional" and "-Wmaybe-uninitialized" shows
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78044
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635
>>>
>>> Google also gives
>>> https://www.boost.org/doc/libs/1_69_0/libs/optional/doc/html/boost_optional/tutorial/gotchas/false_positive_with__wmaybe_uninitialized.html
>>>
>>> https://sourceware.org/ml/gdb-patches/2017-05/msg00130.html
>>> etc
>>>
>>> And that's just for using a type called 'optional' (3 implementations of
>>> it).
>>
>> from my very quick reading of the first googled testcase, I assume the
>> instance of the optional class got SRAed and a warning was generated for
>> what originally was a class member, which indeed is not easy to
>> initialize on its own in order to avoid the warning.
> 
> Hmm, SRA and -Wmaybe-uninitialized, I saw that recently in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66459
> 
>> Would it perhaps make sense to split the -Wmaybe-uninitialized warning
>> into two, one for scalars that are scalars in the original code and one
>> for SRA-created scalars and move only the latter to -Wextra?
> 
> If SRA is really the main source of false positives (I don't know about
> that), maybe. But I am afraid we will also miss a significant proportion
> of the already rare true positives that -Wmaybe-uninitialized currently
> finds. I really have no idea if such a split would work great or badly
> (sorry, I am not being very useful).
I've had some discussions with Kees and others in this space.  The
general consensus is that for pure scalars that everyone does a pretty
good job at generating good maybe-uninitialized warnings.  LLVM and GCC
take different approaches, both are valid and both have been useful at
giving developers actionable warnings.

However, the general consensus is that for aggregates or anything living
in memory is that most compilers aren't doing a particularly good job
across the board in this space.  Not enough objects are being thoroughly
analyzed and those that are analyzed give too many false positives, etc.
 On the Microsoft side, they've chosen to focus on improving DSE and
just initializing everything in memory (presumably flag controlled).
It's not clear where LLVM is going to go in this space.  ISTM that most
of the analysis to do a good job at DSE-ing away the redundant
initializer code should play directly into doing a good job at
maybe-uninit warnings for objects in memory.  However, I don't really
have the time to explore here.

I would generally support some experimentation in the overall space,
that might include allowing for different default settings for uninit
warnings of pure scalars vs aggregates/addressables.

FWIW, I've been doing Fedora rawhide builds with gcc-9 since early Jan.
 Not everything builds with -Wall -Werror, but lots of packages do.
I've only seen one maybe-uninit warning cause problems -- it was
spurious and for a memory object.  I didn't dig into it at all.

In contrast things like the missing NUL termination warnings, buffer
overflow warnings, NULL strings to *printf* warnings, etc all caught
numerous (dozens) of real bugs.  I mention it because it's one of the
ways I know packages are building with -Wall -Werror :-)

Jeff

Re: Move -Wmaybe-uninitialized to -Wextra

[Pedro Alves]

On 02/05/2019 05:07 PM, Jeff Law wrote:
> FWIW, I've been doing Fedora rawhide builds with gcc-9 since early Jan.
>  Not everything builds with -Wall -Werror, but lots of packages do.
> I've only seen one maybe-uninit warning cause problems -- it was
> spurious and for a memory object.  I didn't dig into it at all.

Do you have insight into how many Fedora packages explicitly disable
maybe-uninit?  I really don't know the answer to that, but I'd at least
keep that in mind as a possible reason for the warning seemingly
not causing trouble often.

GDB disables it (or rather, removes it from -Werror), and I wouldn't be
surprised if other C++ programs do the same, when they start using
std::optional more.  Since boost::optional also triggers the same warnings,
set may be even larger than C++11-or-later programs.

I don't know how to search the Fedora codebase, but a github search
for "-Wno-maybe-uninitialized" shows > 100k hits:

 https://github.com/search?q=-Wno-maybe-uninitialized&type=Code

and a search for -Wno-error=maybe-uninitialized shows >10k:

 https://github.com/search?q=-Wno-error%3Dmaybe-uninitialized&type=Code

That's a much higher hit rate than say -Wno-array-bounds or 
-Wno-format-overflow, or -Wno-stringop-overflow, which were some
searches I tried.

> 
> In contrast things like the missing NUL termination warnings, buffer
> overflow warnings, NULL strings to *printf* warnings, etc all caught
> numerous (dozens) of real bugs.  I mention it because it's one of the
> ways I know packages are building with -Wall -Werror :-)

Thanks,
Pedro Alves

Re: Move -Wmaybe-uninitialized to -Wextra

[Jeff Law]

On 2/20/19 5:36 AM, Pedro Alves wrote:
> On 02/05/2019 05:07 PM, Jeff Law wrote:
>> FWIW, I've been doing Fedora rawhide builds with gcc-9 since early Jan.
>>  Not everything builds with -Wall -Werror, but lots of packages do.
>> I've only seen one maybe-uninit warning cause problems -- it was
>> spurious and for a memory object.  I didn't dig into it at all.
> 
> Do you have insight into how many Fedora packages explicitly disable
> maybe-uninit?  I really don't know the answer to that, but I'd at least
> keep that in mind as a possible reason for the warning seemingly
> not causing trouble often.
Unfortunately not.  And my tester only saves failed log files, so I
can't just grep the log files for it.


> 
> GDB disables it (or rather, removes it from -Werror), and I wouldn't be
> surprised if other C++ programs do the same, when they start using
> std::optional more.  Since boost::optional also triggers the same warnings,
> set may be even larger than C++11-or-later programs.
Well, one of the proposals is to distinguish between scalars and
memory/addressables.  That should (in theory) make it possible to deal
with this more sanely.

jeff

Re: Move -Wmaybe-uninitialized to -Wextra

[Martin Sebor]

On 2/20/19 7:25 AM, Jeff Law wrote:
> On 2/20/19 5:36 AM, Pedro Alves wrote:
>> On 02/05/2019 05:07 PM, Jeff Law wrote:
>>> FWIW, I've been doing Fedora rawhide builds with gcc-9 since early Jan.
>>>   Not everything builds with -Wall -Werror, but lots of packages do.
>>> I've only seen one maybe-uninit warning cause problems -- it was
>>> spurious and for a memory object.  I didn't dig into it at all.
>>
>> Do you have insight into how many Fedora packages explicitly disable
>> maybe-uninit?  I really don't know the answer to that, but I'd at least
>> keep that in mind as a possible reason for the warning seemingly
>> not causing trouble often.
> Unfortunately not.  And my tester only saves failed log files, so I
> can't just grep the log files for it.

Would it difficult to add a pass over the build logs to scan for
these sorts of things?  E.g., count the number of instances of
each warning in builds that don't use -Werror?

Martin

> 
> 
>>
>> GDB disables it (or rather, removes it from -Werror), and I wouldn't be
>> surprised if other C++ programs do the same, when they start using
>> std::optional more.  Since boost::optional also triggers the same warnings,
>> set may be even larger than C++11-or-later programs.
> Well, one of the proposals is to distinguish between scalars and
> memory/addressables.  That should (in theory) make it possible to deal
> with this more sanely.
> 
> jeff
> 

Re: Move -Wmaybe-uninitialized to -Wextra

[Jeff Law]

On 2/20/19 8:11 AM, Martin Sebor wrote:
> On 2/20/19 7:25 AM, Jeff Law wrote:
>> On 2/20/19 5:36 AM, Pedro Alves wrote:
>>> On 02/05/2019 05:07 PM, Jeff Law wrote:
>>>> FWIW, I've been doing Fedora rawhide builds with gcc-9 since early Jan.
>>>>   Not everything builds with -Wall -Werror, but lots of packages do.
>>>> I've only seen one maybe-uninit warning cause problems -- it was
>>>> spurious and for a memory object.  I didn't dig into it at all.
>>>
>>> Do you have insight into how many Fedora packages explicitly disable
>>> maybe-uninit?  I really don't know the answer to that, but I'd at least
>>> keep that in mind as a possible reason for the warning seemingly
>>> not causing trouble often.
>> Unfortunately not.  And my tester only saves failed log files, so I
>> can't just grep the log files for it.
> 
> Would it difficult to add a pass over the build logs to scan for
> these sorts of things?  E.g., count the number of instances of
> each warning in builds that don't use -Werror?
It's essentially the same as saving the successful logs :-)

Everything is built with fedpkg which buries the log files.  For a
failed build the scripts dig them out of the filesystem and dump them to
stdout.  Doing the same for a successful build wouldn't be hard.

Once that's in place we can scan for warnings or whatever else, even in
a successful build.

I'll try to get that added for the next mass test which would start
Sunday after the next gcc-9 snapshot.

jeff

Re: Move -Wmaybe-uninitialized to -Wextra

[Jeff Law]

On 2/4/19 3:52 PM, Martin Jambor wrote:
> Hi,
> 
> On Mon, Feb 04 2019, Marc Glisse wrote:
>> On Mon, 4 Feb 2019, Martin Sebor wrote:
>>>
> 
> ...
> 
>>> You're right that this is hard to imagine without first hand experience
>>> with the problem.  If this is a common pattern with the warning in C++
>>> class templates in general, a representative test case would help get
>>> a better appreciation of the problem and might also give us an idea
>>> of a better solution.  (If there is one in Bugzilla please point me
>>> at it.)
>>
>> Looking for "optional" and "-Wmaybe-uninitialized" shows
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78044
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635
>>
>> Google also gives
>> https://www.boost.org/doc/libs/1_69_0/libs/optional/doc/html/boost_optional/tutorial/gotchas/false_positive_with__wmaybe_uninitialized.html
>> https://sourceware.org/ml/gdb-patches/2017-05/msg00130.html
>> etc
>>
>> And that's just for using a type called 'optional' (3 implementations of 
>> it).
> 
> from my very quick reading of the first googled testcase, I assume the
> instance of the optional class got SRAed and a warning was generated for
> what originally was a class member, which indeed is not easy to
> initialize on its own in order to avoid the warning.
> 
> Would it perhaps make sense to split the -Wmaybe-uninitialized warning
> into two, one for scalars that are scalars in the original code and one
> for SRA-created scalars and move only the latter to -Wextra?
I could support that.   It fits in with the general sense that we're not
handling aggregates and addressables as well as we could.

JEff

Re: Move -Wmaybe-uninitialized to -Wextra

[Eric Gallager]

On 2/4/19, Martin Sebor <msebor@gmail.com> wrote:
>>> In practice, false positives (and negatives) of both kinds, whether
>>> they fit the formal definition or the informal one, are the nature
>>> of virtually all non-trivial static diagnostics, certainly all those
>>> that depend on control or data flow analysis.  Some are due to bugs
>>> or limitations in the implementation of the warning.  Others are
>>> inherent in the technology.
>>
>> Yes, and I argue that these warnings belong in a different "level" of
>> warnings than the trivial warnings.
>
> Introducing more levels sounds fine to me.  I wouldn't want to see
> a more permissive default; my preference would be the opposite,
> leaving it to projects to adjust.

For reference, besides -Wuninitialized and -Wmaybe-uninitialized,
clang also has its uninitialized warnings split further into
-Wsometimes-uninitialized and -Wconditional-uninitialized.
-Wsometimes-uninitialized relies on the integration of their static
analyzer into their FE and gets stuff that gcc puts under
-Wjump-misses-init and -Wswitch-unreachable instead.
-Wconditional-uninitialized is their super-aggressive variant and
warns about just about everything. It intentionally includes false
positives as described in the comments on this bug:
https://bugs.llvm.org/show_bug.cgi?id=38856
If gcc wants to introduce more levels to -Wuninitialized, I'd start by
considering the additional flags clang supports.

>
>>
>>> Lastly, in the case of uninitialized variables, the usual solution
>>> of initializing them is trivial and always safe (some coding styles
>>> even require it).
>>
>> Here it shows that we don't work with the same type of code at all. If I
>> am using a boost::optional, i.e. a class with a buffer and a boolean
>> that says if the buffer is initialized, how do I initialize the
>> (private) buffer? Or should boost itself zero out the buffer whenever
>> the boolean is set to false? The variables can easily be hidden behind
>> dozens of levels of abstraction that make them hard to initialize, and
>> there may be nothing meaningful to initialize them with. Uninitialized
>> also includes clobbered (out-of-scope for instance) in gcc, where it
>> isn't clear what you are supposed to initialize to quiet the warning.
>
> You're right that this is hard to imagine without first hand experience
> with the problem.  If this is a common pattern with the warning in C++
> class templates in general, a representative test case would help get
> a better appreciation of the problem and might also give us an idea
> of a better solution.  (If there is one in Bugzilla please point me
> at it.)
>
>>>> This message concentrates on the negatives, but that doesn't mean I
>>>> consider -Wmaybe-uninitialized as useless. It can find true
>>>> uninitialized uses. And even some false positives can point at places
>>>> where we can help the compiler generate better code (say with a
>>>> default __builtin_unreachable case in a switch). I did in the past
>>>> contribute patches to make it warn more often, and I might do so
>>>> again in the future.
>>>
>>> This paragraph makes me think you equate false positives from this
>>> warning with those from -Wuninitialized.
>>
>> Uh? No, I don't know what gave you this impression.
>
> It's common to view as a false positive (or "bad" as you write below)
> every instance of a warning designed to detect or prevent bugs that
> doesn't indicate one (as opposed to warnings designed to help write
> better/clearer code like -Wparentheses).
>
>>> "May be unitialized" doesn't preclude the possibility that the
>>> variable is not used uninitialized.
>>
>> But if the variable can never be used uninitialized and gcc is simply
>> unable to prove it, the warning is bad. It may not be a "false positive"
>> depending on your definition, but it is undesirable.
>
> Ideally each instance of every warning designed to find bugs would
> point out one.  But 100% accuracy or a zero rate of the undesirable
> instances is not attainable in general, and reducing their rate at
> the expense of the helpful ones also isn't the best tradeoff either.
> The challenge is striking the right balance between their ratios.
> (Only if that can't done then it might be time to consider
> disabling/demoting the warning.  I don't have the impression
> we are at that point with -Wmaybe-uninitialized but I haven't
> done any research.)
>
>>>> My opinion is that -Wmaybe-uninitialized would serve its purpose
>>>> better as part of -Wextra. People tend to use -Wall with -Werror
>>>> (either explicitly or implicitly by modifying the code until all
>>>> warnings are gone). What I see currently in projects where I
>>>> participate is that -Wmaybe-uninitialized is making things worse.
>>>> People don't investigate deeply the cause of the warning, they just
>>>> go for whatever "quick-fix" makes the compiler shut up. Quite often,
>>>> this involves extra code that is less readable and performs worse,
>>>> while it didn't even "fix" what caused the warning, it just randomly
>>>> ended up with code where the compiler doesn't warn (possibly because
>>>> the function got bigger, which changed inlining decisions...).
>>>
>>> I agree that we need to be mindful of warnings leading to unnecessary
>>> and perhaps inappropriate code changes (as the author of a few warnings
>>> with this potential it has been on mind a lot).
>>>
>>> At the same time, not taking the time to properly analyze diagnostics
>>> and come up with the appropriate solutions seems like a problem that's
>>> orthogonal to the warnings themselves.  Disabling warnings in hopes of
>>> solving what's really a problem with the development process or
>>> a culture of a subset of projects doesn't seem like an optimal solution.
>>
>> I agree with that. But if I complain about the mindless quickfixes, the
>> reply is that customers ask for a library that does not warn, and the
>> developers do not have infinite time, so they do the minimum to avoid
>> warnings. This is strongly linked to the warning levels.
>>
>> One could recommend that customers include the library with -isystem to
>> disable the warnings inside the library, bug that's fragile (though it
>> has gotten quite good), and most importantly it also disables some
>> warnings that are relevant to the user.
>
> Right, that wouldn't be much better than disabling the warning on
> the command line (or us removing it from -Wall and perhaps also
> from -Wextra).
>
> So since mindless quick fixes aren't the way to go and assuming we
> agree that warnings have value even with some noise, is demoting
> them to lower levels because they're not always used properly
> the best solution?  No predetermined system of warning levels is
> going to make everyone happy.  Different projects have different
> constraints and tolerances for noise, or even resources to fix
> real bugs -- GCC with over 400 wrong-code bugs in Open status
> being a case in point, so a level that works for one, like
> a library, may be overly pedantic for an application.
>
> More warning levels might help.  Warning profiles (say one for
> system libraries or freestanding code like the kernel, another
> for C++ class libraries, yet another for applications) might
> be another approach.  Others might be worth brainstorming about.
>
>>>> If the warning is not enabled by default so much but only when people
>>>> are ready to investigate any warning thoroughly, the quickfix
>>>> mentality is less likely to be present. People using
>>>> -Wmaybe-uninitialized need to be willing to ignore false positives,
>>>> or disable them with pragmas.
>>>>
>>>> Note that similar arguments may apply to some other warnings that
>>>> somehow made their way into -Wall when they shouldn't have, but for
>>>> now I am only proposing to move -Wmaybe-uninitialized. Some people
>>>> tend to consider that if a warning is not part of -Wall, it might as
>>>> well not exist. Obviously I disagree with that.
>>>
>>> The description of -Wall is vague enough that arguments about which
>>> warnings should or shouldn't be enabled by it are unavoidably subject
>>> to individual biases and unlikely to lead to a consensus, either among
>>> ourselves or, even less likely, among users.
>>
>> True. To me, one of the key documented properties of -Wall is being
>> "easy to avoid". -Wall -Werror should be a usable combination. Anything
>> not easy to avoid belongs in a different class, say -Wextra. We can
>> rename that to W1/W2/... if Wall is too charged with historical meaning.
>
> I would have expected dealing with -Wmaybe-uninitialized to always
> be easy.  Your comment about std::optional suggests there are cases
> when it isn't.  Can we look at some of those cases and see if there
> is something we can do to make it easier?
>
>>> A shared definition of a false positive should be one of the very
>>> first steps to coming closer to a consensus.  Real world (as opposed
>>> to anecdotal) data on the rates of actual rates of false positives and
>>> negatives vs true positives would be also most helpful, as would some
>>> consensus of the severity of the bugs the true positives expose, as
>>> well as some objective measure of the ease of suppression.  There
>>> probably are others but these would be a start.
>>
>> This data is going to be super hard to get. Most projects have been
>> compiling for years and tweaking their code to avoid some warnings. We
>> do not get to see the code that people originally write, we can only see
>> what they commit.
>
> It's not perfect but we should be able to get a rough idea based
> on bug reports in Bugzilla.  Many distros rebuild the world before
> a GCC release and new instances of build-breaking warnings tend to
> get filed there.  Meta-bugs can be helpful as the first step.
> A finer-grained classification by search terms (e.g., "bogus"
> "spurious", etc.) helps narrow it down even more.
>
> Martin
>

Re: Move -Wmaybe-uninitialized to -Wextra

[Tom Tromey]

>>>>> "Marc" == Marc Glisse <marc.glisse@inria.fr> writes:

>> Lastly, in the case of uninitialized variables, the usual solution
>> of initializing them is trivial and always safe (some coding styles
>> even require it).

Marc> Here it shows that we don't work with the same type of code at all. If
Marc> I am using a boost::optional, i.e. a class with a buffer and a boolean
Marc> that says if the buffer is initialized, how do I initialize the
Marc> (private) buffer? Or should boost itself zero out the buffer whenever
Marc> the boolean is set to false?

This is https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 (I know you
know, but maybe others on the thread don't).

I think in this specific case (std::optional and similar classes), GCC
should provide a way for the class to indicate that
-Wmaybe-uninitialized should not apply to the payload.

>> A shared definition of a false positive should be one of the very
>> first steps to coming closer to a consensus.  Real world (as opposed
>> to anecdotal) data on the rates of actual rates of false positives
>> and negatives vs true positives would be also most helpful, as would
>> some consensus of the severity of the bugs the true positives
>> expose, as well as some objective measure of the ease of
>> suppression.  There probably are others but these would be a start.

Marc> This data is going to be super hard to get. Most projects have been
Marc> compiling for years and tweaking their code to avoid some warnings. We
Marc> do not get to see the code that people originally write, we can only
Marc> see what they commit.

gdb has gone through this over the years -- it turns on many warnings
and sometimes false positives show up.  Most of the time there's a
comment, for -Wmaybe-uninitialized grep for "init.*gcc" in the source.
Unfortunately the comment isn't standardized; but I only get ~20 hits
for this in gdb, so it isn't really so bad in practice.

Tom

Re: Move -Wmaybe-uninitialized to -Wextra

[Jeff Law]

On 2/14/19 7:23 AM, Tom Tromey wrote:
>>>>>> "Marc" == Marc Glisse <marc.glisse@inria.fr> writes:
> 
>>> Lastly, in the case of uninitialized variables, the usual solution
>>> of initializing them is trivial and always safe (some coding styles
>>> even require it).
> 
> Marc> Here it shows that we don't work with the same type of code at all. If
> Marc> I am using a boost::optional, i.e. a class with a buffer and a boolean
> Marc> that says if the buffer is initialized, how do I initialize the
> Marc> (private) buffer? Or should boost itself zero out the buffer whenever
> Marc> the boolean is set to false?
> 
> This is https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635 (I know you
> know, but maybe others on the thread don't).
> 
> I think in this specific case (std::optional and similar classes), GCC
> should provide a way for the class to indicate that
> -Wmaybe-uninitialized should not apply to the payload.
> 
>>> A shared definition of a false positive should be one of the very
>>> first steps to coming closer to a consensus.  Real world (as opposed
>>> to anecdotal) data on the rates of actual rates of false positives
>>> and negatives vs true positives would be also most helpful, as would
>>> some consensus of the severity of the bugs the true positives
>>> expose, as well as some objective measure of the ease of
>>> suppression.  There probably are others but these would be a start.
> 
> Marc> This data is going to be super hard to get. Most projects have been
> Marc> compiling for years and tweaking their code to avoid some warnings. We
> Marc> do not get to see the code that people originally write, we can only
> Marc> see what they commit.
> 
> gdb has gone through this over the years -- it turns on many warnings
> and sometimes false positives show up.  Most of the time there's a
> comment, for -Wmaybe-uninitialized grep for "init.*gcc" in the source.
> Unfortunately the comment isn't standardized; but I only get ~20 hits
> for this in gdb, so it isn't really so bad in practice.
Yea, in retrospect we should have had a consistent marker for GCC as
well.  I suspect a goodly number of those initializations that went in
early in the process are no longer needed.

Jeff

Re: Move -Wmaybe-uninitialized to -Wextra

[Jeff Law]

On 2/3/19 3:02 AM, Marc Glisse wrote:
> On Sat, 2 Feb 2019, Martin Sebor wrote:
> 
>> I don't feel too strongly about whether -Wmaybe-uninitialized should
>> be in -Wall or in -Wextra, and I might even be somewhat more inclined
>> to expect to find it in the latter.  But since you sound like you are
>> gearing up for proposing other changes in the same vein down the line
>> where I might have stronger opinions, I should comment.
>>
>> [It's a bit of a long-winded response because I've been thinking about
>> this topic a lot lately.]
> 
> Thank you for taking the time to write this down.
> 
>> In general, I think a discussion of warning groupings is worth having
>> (even needed) at the right time, but stage 4 doesn't strike me as
>> the most opportune occasion.
>>
>> Specifically for -Wmaybe-uninitialized, the option has been in -Wall
>> for many releases, and no major changes to it have been made recently
>> that I know.  So what I'm missing in this proposal is: why now?  What
>> has changed to make this a pressing issue now?  Has its rate of false
>> positives gone up significantly?  If so, by how much and why?
> 
> I've been wanting to post this for years, but I only got motivated
> enough recently after seeing negative effects of -Wmaybe-uninitialized
> in two projects within a few days. I don't think the rate of false
> positives has gone up significantly in gcc-9, they just randomly appear
> or disappear from one release to the next. In some sense, for projects
> that support multiple compiler versions, each new gcc release makes the
> number of false positives (on at least one version) go up, but that
> doesn't count as the warning getting worse.
> 
> The discussion and/or the change don't need to happen now, but if I
> didn't post this patch while motivated, it might be years before I
> actually do it, so I ignored stage 4.
I'll also note that I proposed changes to how we handled Wuninitialized
many years ago which were designed to address the problem of instability
in these warnings from release to release.  We would have the ability to
issue warnings from early in the pipeline -- before the optimizers
really muck things up.  This gives you much more stable warnings from
release to release (at the expense of many more false positives).  We'd
also be able to use the results of that very early analysis to indicate
to the user when the optimizers were able to ultimately prove the early
uninitialized use was optimized away (or the path which lead to the flow
of an uninitialized value was optimized away).

I ultimately gave up, not because of technical issues, but because of
the inability to get any kind of consensus on changing things in this
space.  It was an amazingly frustrating experience, but did certainly
highlight that there is no one answer in this space.  Everyone wants
different defaults.


> 
> I am mostly looking at what I see in practice. I regularly see false
> positives of -Wstrict-overflow (less now that I neutered parts of it)
> and -Wmaybe-uninitialized. And when I have strange crashes that later
> turn out to be caused by uninitialized memory uses, it is very seldom
> that -Wmaybe-uninitialized helps detect them. IIRC, last time, I had
> slightly more success with -Wreturn-local-addr -O3
> -fkeep-inline-functions -fkeep-static-functions.
Which is consistent with the belief that we simply don't do a good job
at uninitialized warnings for addressables and aggregates.

> 
>> "May be unitialized" doesn't preclude the possibility that the
>> variable is not used uninitialized.
> 
> But if the variable can never be used uninitialized and gcc is simply
> unable to prove it, the warning is bad. It may not be a "false positive"
> depending on your definition, but it is undesirable.
It's certainly undesirable and we go to nearly heroic lengths to avoid
them.  I'm really looking forward to the new range infrastructure as it
can be exploited in the backwards threader to address a significant
class of these false positives.

I'd also really like to see the predicate analysis we currently use to
prune uninit warnings be split out as a separate analysis.  THe ability
to prove a path is infeasible, even if we have to leave it in the CFG
has uses elsewhere.

I'd also like a way to mark infeasible paths found by the threader, but
which the threader decides are not worth optimizing (usually because of
the amount of code it would need to duplicate).  Marking them (and
somehow keeping the data accurate) would also eliminate classes of false
positives from the uninitialized warnings.

Finally, we also still have cases where iteration of threading + dom is
needed to remove infeasible paths and avoid false positives.  I'm not
going to suggest we return to the that world, but we do need to continue
to explore ways to thread deeper in a single iteration.

>\
> True. To me, one of the key documented properties of -Wall is being
> "easy to avoid". -Wall -Werror should be a usable combination. Anything
> not easy to avoid belongs in a different class, say -Wextra. We can
> rename that to W1/W2/... if Wall is too charged with historical meaning.
> 
> One issue is that, especially in a large, long-lived code base, people
> want to know when a change (or a new compiler) introduces a new warning.
> However, they do not want to have to go through hundreds or
> pre-existing, already analysed warnings. The easiest way to do that is
> to make sure there are no warnings at all (-Werror). For speculative
> warnings, that's really not convenient. I don't think there is a good
> solution to this (marking with a pragma the known false positives may be
> hard when instantiating some class somewhere causes a warning in some
> almost unrelated place, and you want the marking to be narrow enough
> that it won't also disable other warnings), and it is independent on
> whether the warning is in -Wall or -Wextra. I just have a feeling that
> splitting the easy and the not-easy warnings may help developers handle
> them differently.
I understand this sentiment.  Believe me I do.  I've worked with folks
that have to deal with legacy codebases and warning stability from
release to release is very important to them.  We don't have a real good
solution for them (this class of user was the motivation behind the
-Wuninitialized changes I proposed many years ago).  The best right now
is to identify a set of warnings that are important to those
organizations and use those and only those with -Werror.

Jeff

ps.  Yes, I know I did a lot of snipping of the discussion.  I didn't
feel I had much to add to the other parts.  I hope that in doing so I
haven't taken anything out of context or conflated any of the issues.

Re: Move -Wmaybe-uninitialized to -Wextra

[Michael Witten]

The problem with false positives is correlated with the degree of
optimization; a lot of people  have reported problems at only the
`-Og'  optimization level  (even  when the  code  in question  is
embarrassingly correct).

Therefore,  the general  solution  is probably  that the  implem-
entation of  `-Wmaybe-uninitialized' be customized for  the given
level of optimization.

However, I get the impression that this is easier said than done.

From  what  I've  read,  `-Wmaybe-uninitialized'  is  essentially
customized for `-O2',  which means that it will  work pretty darn
well at that optimization level. So,  in the interim, I propose a
simple approximation of the general solution:

  At an optimization level below `-O2' (or other than `-O2'):

    * `-Wmaybe-uninitialized' is moved to `-Wextra'.

      If  `-Wall'   has  been   specified,  then  gcc   emits  an
      informational note stating that `-Wmaybe-uninitialized' (or
      any  other  warning that  has  similar  problems) has  been
      disabled.

    * Provide a command-line option to disable such a note.

    * The user  may always enable  `-Wmaybe-uninitialized' either
      explicitly or as part of `-Wextra'.

    * If `-Wmaybe-uninitialized' is enabled  by the user, then it
      implies `-O2' computations.

      That is to say, when the user specifies:

        -Og -Wmaybe-uninitialized

      or:

        -Og -Wextra

      then the user is explicitly  telling the compiler to do all
      the optimizations  of `-O2',  but ONLY  for the  purpose of
      implementing `-Wmaybe-uninitialized'  (or whichever warning
      requires those  optimizations to function well);  all those
      optimizations are  to be  thrown out  after they  have been
      used to  good effect  by `-Wmaybe-uninitialized'.  The Code
      generation, etc.,  shall be  performed at  the optimization
      level the user specified (namely, `-Og' in this case).

In other  words, save the  user from  gcc's foibles, but  let the
user pay for the extra computation if so desired!

What do you think?

Sincerely,
Michael Witten


PS

All  of  this trouble  indicates  that  C (and  other  languages)
are  just not  expressive enough  with regard  to initialization.

Initialization semantics  are basically a matter  of API contract
specification; the programmer needs the tools to write it down.

Surely, gcc could  provide `__builtin_assume_initialized(x);' and
parameter attributes  to help  inform the  reader (i.e.,  to help
inform  the  compiler)  about  the  code;  a  function  parameter
attribute  could  specify  whether  the  given  argument  can  be
considered initialized  after a  call, and maybe  specify further
constraints, such as whether the  guarantee is made only when the
function return value is nonzero (or a certain value), etc.

We need the language to write our thoughts down!

Re: Move -Wmaybe-uninitialized to -Wextra

[Jeff Law]

On Tue, 2019-11-26 at 19:33 +0000, Michael Witten wrote:
> The problem with false positives is correlated with the degree of
> optimization; a lot of people  have reported problems at only the
> `-Og'  optimization level  (even  when the  code  in question  is
> embarrassingly correct).
> 
> Therefore,  the general  solution  is probably  that the  implem-
> entation of  `-Wmaybe-uninitialized' be customized for  the given
> level of optimization.
> 
> However, I get the impression that this is easier said than done.
> 
> From  what  I've  read,  `-Wmaybe-uninitialized'  is  essentially
> customized for `-O2',  which means that it will  work pretty darn
> well at that optimization level. So,  in the interim, I propose a
> simple approximation of the general solution:
> 
>   At an optimization level below `-O2' (or other than `-O2'):
> 
>     * `-Wmaybe-uninitialized' is moved to `-Wextra'.
> 
>       If  `-Wall'   has  been   specified,  then  gcc   emits  an
>       informational note stating that `-Wmaybe-uninitialized' (or
>       any  other  warning that  has  similar  problems) has  been
>       disabled.
> 
>     * Provide a command-line option to disable such a note.
> 
>     * The user  may always enable  `-Wmaybe-uninitialized' either
>       explicitly or as part of `-Wextra'.
> 
>     * If `-Wmaybe-uninitialized' is enabled  by the user, then it
>       implies `-O2' computations.
> 
>       That is to say, when the user specifies:
> 
>         -Og -Wmaybe-uninitialized
> 
>       or:
> 
>         -Og -Wextra
> 
>       then the user is explicitly  telling the compiler to do all
>       the optimizations  of `-O2',  but ONLY  for the  purpose of
>       implementing `-Wmaybe-uninitialized'  (or whichever warning
>       requires those  optimizations to function well);  all those
>       optimizations are  to be  thrown out  after they  have been
>       used to  good effect  by `-Wmaybe-uninitialized'.  The Code
>       generation, etc.,  shall be  performed at  the optimization
>       level the user specified (namely, `-Og' in this case).
> 
> In other  words, save the  user from  gcc's foibles, but  let the
> user pay for the extra computation if so desired!
> 
> What do you think?
I think this would be a terrible idea.   Yes, it's absolutely true that
changing options can get you different warning results.  Yes it's
absolutely true that there are false positives.  But, no the warning is
not customized to -O2.  It's just that O2 enables more optimizations
that are particularly good at weeding out false positives.

The whole point behind the uninitialized warning is to capture cases
where objects may not be properly initialized.  For modern code the
simple cases typically "just work".  What is by far the most
interesting cases are those with complex flow control, often
interacting with inline functions, address-of stripping, etc.  These
are precisely the cases that humans aren't particularly good at
catching and having the compiler analyze those paths and issue warnings
that humans fix ultimately results in better quality code.

Experience has shown that if you put something in -Wall, people will
pay attention to it, and that is good for the long term quality of code
bases.  If the diagnostic is outside of -Wall, it's largely ignored.  I
think the pain of dealing with the Wuninitialized warts is smaller than
the pain of allowing these errors to persist.
 
jeff

Re: Move -Wmaybe-uninitialized to -Wextra

[Martin Jambor]

Hi Jeff,

On Sat, Dec 07 2019, Jeff Law wrote:
> [...]
> The whole point behind the uninitialized warning is to capture cases
> where objects may not be properly initialized.  For modern code the
> simple cases typically "just work".  What is by far the most
> interesting cases are those with complex flow control, often
> interacting with inline functions, address-of stripping, etc.  These
> are precisely the cases that humans aren't particularly good at
> catching and having the compiler analyze those paths and issue warnings
> that humans fix ultimately results in better quality code.

Or "fixes" like:

https://gcc.gnu.org/viewcvs/gcc/trunk/gcc/expmed.c?r1=277864&r2=277863&pathrev=277864

if even we cannot deal with the false positive in our own compiler, how
can we expect our users to do so?

>
> Experience has shown that if you put something in -Wall, people will
> pay attention to it, and that is good for the long term quality of code
> bases.  If the diagnostic is outside of -Wall, it's largely ignored.  I
> think the pain of dealing with the Wuninitialized warts is smaller than
> the pain of allowing these errors to persist.

I'm afraid I that -Wmaybe-uninitialized is getting out of hand.  I bet
that some users regularly get these warnings coming from c++ header
"libraries" (like they sometimes come out our vec.h which recently
"broke" bootstrap) which they sometimes even cannot change and they then
conclude that our -Wall is "unusable" and stop paying attention to all
warnings.

Martin

Re: Move -Wmaybe-uninitialized to -Wextra

[Pedro Alves]

On 12/16/19 2:45 PM, Martin Jambor wrote:
> On Sat, Dec 07 2019, Jeff Law wrote:
>> [...]

> I'm afraid I that -Wmaybe-uninitialized is getting out of hand.  I bet
> that some users regularly get these warnings coming from c++ header
> "libraries" (like they sometimes come out our vec.h which recently
> "broke" bootstrap) which they sometimes even cannot change and they then
> conclude that our -Wall is "unusable" and stop paying attention to all
> warnings.

-Wmaybe-uninitialized that trigger in std::optional (and clones) (PR80635 [1])
are particularly annoying, and there's no sane workaround the user can apply.
You'll find quite a number of those just by googling for it:

  https://www.google.com/search?q=std+optional+"-Wmaybe-uninitialized"

We have a few of those in GDB, and because GDB uses -Wall + -Werror, GDB
nowadays builds with -Wno-error=maybe-uninitialized so that we see the
warnings but the build continues without error.  People still occasionally
get confused and waste time with those warnings, though.  Here, just
this week, point 5:

  https://sourceware.org/ml/gdb-patches/2019-12/msg00706.html

FWIW, I've considered completely disabling -Wmaybe-uninitialized in
GDB instead of downgrading it from -Werror to a warning with -Wno-error.

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

-- 
Pedro Alves

Re: Move -Wmaybe-uninitialized to -Wextra

[Jeff Law]

On Mon, 2019-12-16 at 15:45 +0100, Martin Jambor wrote:
> Hi Jeff,
> 
> On Sat, Dec 07 2019, Jeff Law wrote:
> > [...]
> > The whole point behind the uninitialized warning is to capture cases
> > where objects may not be properly initialized.  For modern code the
> > simple cases typically "just work".  What is by far the most
> > interesting cases are those with complex flow control, often
> > interacting with inline functions, address-of stripping, etc.  These
> > are precisely the cases that humans aren't particularly good at
> > catching and having the compiler analyze those paths and issue warnings
> > that humans fix ultimately results in better quality code.
> 
> Or "fixes" like:
> 
> https://gcc.gnu.org/viewcvs/gcc/trunk/gcc/expmed.c?r1=277864&r2=277863&pathrev=277864
:(


> 
> if even we cannot deal with the false positive in our own compiler, how
> can we expect our users to do so?
Fair point.  And I've certainly seen painful-to-fix fallout in my
Fedora builds -- by far the most painful cases have been addressables
where we're able to strip the address-of due to inlining.


> 
> > Experience has shown that if you put something in -Wall, people will
> > pay attention to it, and that is good for the long term quality of code
> > bases.  If the diagnostic is outside of -Wall, it's largely ignored.  I
> > think the pain of dealing with the Wuninitialized warts is smaller than
> > the pain of allowing these errors to persist.
> 
> I'm afraid I that -Wmaybe-uninitialized is getting out of hand.  I bet
> that some users regularly get these warnings coming from c++ header
> "libraries" (like they sometimes come out our vec.h which recently
> "broke" bootstrap) which they sometimes even cannot change and they then
> conclude that our -Wall is "unusable" and stop paying attention to all
> warnings.
If it's coming from our headers, then we make a huge effort to fix the
issues :-)

jeff

Re: Move -Wmaybe-uninitialized to -Wextra

[Martin Jambor]

Hi,

On Tue, Nov 26 2019, Michael Witten wrote:
> [...]
> From  what  I've  read,  `-Wmaybe-uninitialized'  is  essentially
> customized for `-O2',

I don't think that is true.  It can be perfectly useful -O1 and there
are many nasty false positives at -O2 too.

> [...]
>     * If `-Wmaybe-uninitialized' is enabled  by the user, then it
>       implies `-O2' computations.
>
>       That is to say, when the user specifies:
>
>         -Og -Wmaybe-uninitialized
>
>       or:
>
>         -Og -Wextra
>
>       then the user is explicitly  telling the compiler to do all
>       the optimizations  of `-O2',  but ONLY  for the  purpose of
>       implementing `-Wmaybe-uninitialized'

If you try to implement this proposal, you'll quickly find that there is
no such thing as like "do all optimizations only for purposes of a
warning."  You either perform a transformation or not and subsequent
passes, such as pass_late_warn_uninitialized, start work where the
previous ones left off.

Martin