[RFC] Only warn for maybe-uninitialized SRAed bits in -Wextra (PR 80635)

Martin Jambor mjambor@suse.cz
Fri Nov 8 12:41:00 GMT 2019


Hi,

this patch is an attempt to implement my idea from a previous thread
about moving -Wmaybe-uninitialized to -Wextra:

https://gcc.gnu.org/ml/gcc-patches/2019-02/msg00220.html

Specifically, it attempts to split -Wmaybe-uninitialized into those that
are about SRA DECLs and those which are not, and move to -Wextra only
the former ones.  The main idea is that false -Wmaybe-uninitialized
warings about values that are scalar in user's code are easy to silence
by initializing them to zero or something, as opposed to bits of
aggregates such as a value in std::optional which are not.  Therefore,
the warnings about user-scalars can remain in -Wall but warnings about
SRAed pieces should be moved to -Wextra.

The patch is a bit bigger because of documentation (which I'll be happy
to improve based on your suggestions) and testsuite churn, but the main
bit is the following added test in warn_uninit function:

  if (wc == OPT_Wmaybe_uninitialized
      && SSA_NAME_VAR (t)
      && DECL_ARTIFICIAL (SSA_NAME_VAR (t))
      && DECL_HAS_DEBUG_EXPR_P (SSA_NAME_VAR (t)))
    {
      if (warn_maybe_uninitialized_aggregates)
	wc = OPT_Wmaybe_uninitialized_aggregates;
      else
	return;
    }

The reason why I also test DECL_HAS_DEBUG_EXPR_P is
gfortran.dg/pr39666-2.f90 - without it the test silences a warning about
a decl representing the return value which is an artificial scalar
variable (probably all the way from the front-end).  We can of course
not care about not warning for it but then I don't know how to call and
document the new option :-)  Generally, if someone can think of a better
test to identify SRA DECLs, I'll be happy to change that.  We might put
a bit to identify SRA decls in the decl tree, but I tend to think that
is not a good use of the few remaining bits there.

What do you think, is something along these lines a good idea?

Thanks,

Martin



2019-11-08  Martin Jambor  <mjambor@suse.cz>

	* common.opt (Wmaybe-uninitialized-aggregates): New.
	* tree-ssa-uninit.c (gate_warn_uninitialized): Also run if
	warn_maybe_uninitialized_aggregates is set.
	(warn_uninit): Warn for artificial DECLs only if
	warn_maybe_uninitialized_aggregates is set.
	* doc/invoke.texi (Warning Options): Add
	-Wmaybe-uninitialized-aggregates to the list.
	(-Wextra): Likewise.
	(-Wmaybe-uninitialized): Document that it only works on scalars.
	(-Wmaybe-uninitialized-aggregates): Document.

	testsuite/
	* gcc.dg/pr45083.c: Add Wmaybe-uninitialized-aggregates to options.
	* gcc.dg/ubsan/pr81981.c: Likewise.
	* gfortran.dg/pr25923.f90: Likewise.
	* g++.dg/warn/pr80635.C: New.
---
 gcc/common.opt                        |  4 +++
 gcc/doc/invoke.texi                   | 18 +++++++++--
 gcc/testsuite/g++.dg/warn/pr80635.C   | 45 +++++++++++++++++++++++++++
 gcc/testsuite/gcc.dg/pr45083.c        |  2 +-
 gcc/testsuite/gcc.dg/ubsan/pr81981.c  |  2 +-
 gcc/testsuite/gfortran.dg/pr25923.f90 |  2 +-
 gcc/tree-ssa-uninit.c                 | 14 ++++++++-
 7 files changed, 81 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/pr80635.C

diff --git a/gcc/common.opt b/gcc/common.opt
index cc279f411d7..03769299df8 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -783,6 +783,10 @@ Wmaybe-uninitialized
 Common Var(warn_maybe_uninitialized) Warning EnabledBy(Wuninitialized)
 Warn about maybe uninitialized automatic variables.
 
+Wmaybe-uninitialized-aggregates
+Common Var(warn_maybe_uninitialized_aggregates) Warning EnabledBy(Wextra)
+Warn about maybe uninitialized automatic parts of aggregates.
+
 Wunreachable-code
 Common Ignore Warning
 Does nothing. Preserved for backward compatibility.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index faa7fa95a0e..dbc3219b770 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -328,7 +328,8 @@ Objective-C and Objective-C++ Dialects}.
 -Wzero-length-bounds @gol
 -Winvalid-pch  -Wlarger-than=@var{byte-size} @gol
 -Wlogical-op  -Wlogical-not-parentheses  -Wlong-long @gol
--Wmain  -Wmaybe-uninitialized  -Wmemset-elt-size  -Wmemset-transposed-args @gol
+-Wmain  -Wmaybe-uninitialized -Wmaybe-uninitialized-aggregates @gol
+-Wmemset-elt-size  -Wmemset-transposed-args @gol
 -Wmisleading-indentation  -Wmissing-attributes  -Wmissing-braces @gol
 -Wmissing-field-initializers  -Wmissing-format-attribute @gol
 -Wmissing-include-dirs  -Wmissing-noreturn  -Wmissing-profile @gol
@@ -4498,6 +4499,7 @@ name is still supported, but the newer name is more descriptive.)
 -Wempty-body  @gol
 -Wignored-qualifiers @gol
 -Wimplicit-fallthrough=3 @gol
+-Wmaybe-uninitialized-aggregates @gol
 -Wmissing-field-initializers  @gol
 -Wmissing-parameter-type @r{(C only)}  @gol
 -Wold-style-declaration @r{(C only)}  @gol
@@ -5690,10 +5692,22 @@ in fact be called at the place that would cause a problem.
 
 Some spurious warnings can be avoided if you declare all the functions
 you use that never return as @code{noreturn}.  @xref{Function
-Attributes}.
+Attributes}.  This option only warns about scalar variables, use
+@option{-Wmaybe-uninitialized-aggregates} to enable the warnings on parts of
+aggregates which the compiler can track.
 
 This warning is enabled by @option{-Wall} or @option{-Wextra}.
 
+@item -Wmaybe-uninitialized-aggregates
+@opindex Wmaybe-uninitialized-aggregates
+@opindex Wmaybe-uninitialized-aggregates
+
+This option enables the same warning like @option{-Wmaybe-uninitialized} but
+for parts of aggregates (i.g.@: structures, arrays or unions) that GCC
+optimizers can track.  These warnings are only possible in optimizing
+compilation, because otherwise GCC does not keep track of the state of
+variables.  This warning is enabled by @option{-Wextra}.
+
 @item -Wunknown-pragmas
 @opindex Wunknown-pragmas
 @opindex Wno-unknown-pragmas
diff --git a/gcc/testsuite/g++.dg/warn/pr80635.C b/gcc/testsuite/g++.dg/warn/pr80635.C
new file mode 100644
index 00000000000..eaf6b34a29d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/pr80635.C
@@ -0,0 +1,45 @@
+// PR C++/80635
+// { dg-options "-std=c++11 -Wuninitialized -O" }
+
+void* operator new (__SIZE_TYPE__, void *p) { return p; }
+
+int f ();
+int x;
+
+struct A {
+  int i;
+  A (): i (f ()) { }  // { dg-bogus "uninitialized" }
+  ~A () { x = i; }
+};
+
+struct B {
+  B ();
+  ~B ();
+};
+
+
+template <class T>
+struct C {
+  C (): t (), b () { }
+  ~C () { if (b) t.~T (); }
+
+  void f () {
+    new (&t) T ();
+    b = true;
+  }
+
+  union {
+    T t;
+    char x[sizeof (T)];
+  };
+  bool b;
+};
+
+void g ()
+{
+  C<A> c1;
+  C<B> c2;
+
+  c1.f ();
+  c2.f ();
+}
diff --git a/gcc/testsuite/gcc.dg/pr45083.c b/gcc/testsuite/gcc.dg/pr45083.c
index c9a4dbfe191..bce7d0bd7a6 100644
--- a/gcc/testsuite/gcc.dg/pr45083.c
+++ b/gcc/testsuite/gcc.dg/pr45083.c
@@ -1,6 +1,6 @@
 /* PR tree-optimization/45083 */
 /* { dg-do compile } */
-/* { dg-options "-O2 -Wuninitialized" } */
+/* { dg-options "-O2 -Wuninitialized -Wmaybe-uninitialized-aggregates" } */
 
 struct S { char *a; unsigned b; unsigned c; };
 extern int foo (const char *);
diff --git a/gcc/testsuite/gcc.dg/ubsan/pr81981.c b/gcc/testsuite/gcc.dg/ubsan/pr81981.c
index b2636d4c934..6a381f6b180 100644
--- a/gcc/testsuite/gcc.dg/ubsan/pr81981.c
+++ b/gcc/testsuite/gcc.dg/ubsan/pr81981.c
@@ -1,6 +1,6 @@
 /* PR sanitizer/81981 */
 /* { dg-do compile } */
-/* { dg-options "-O2 -Wmaybe-uninitialized -fsanitize=undefined -ffat-lto-objects" } */
+/* { dg-options "-O2 -Wmaybe-uninitialized -Wmaybe-uninitialized-aggregates -fsanitize=undefined -ffat-lto-objects" } */
 
 int v;
 
diff --git a/gcc/testsuite/gfortran.dg/pr25923.f90 b/gcc/testsuite/gfortran.dg/pr25923.f90
index 3283ba21f32..2ddc3b92485 100644
--- a/gcc/testsuite/gfortran.dg/pr25923.f90
+++ b/gcc/testsuite/gfortran.dg/pr25923.f90
@@ -1,5 +1,5 @@
 ! { dg-do compile }
-! { dg-options "-O -Wuninitialized" }
+! { dg-options "-O -Wuninitialized -Wmaybe-uninitialized-aggregates" }
 
 module foo
 implicit none
diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c
index fe8f8f0bc28..362af73e5d2 100644
--- a/gcc/tree-ssa-uninit.c
+++ b/gcc/tree-ssa-uninit.c
@@ -134,6 +134,16 @@ warn_uninit (enum opt_code wc, tree t, tree expr, tree var,
     return;
   if (!has_undefined_value_p (t))
     return;
+  if (wc == OPT_Wmaybe_uninitialized
+      && SSA_NAME_VAR (t)
+      && DECL_ARTIFICIAL (SSA_NAME_VAR (t))
+      && DECL_HAS_DEBUG_EXPR_P (SSA_NAME_VAR (t)))
+    {
+      if (warn_maybe_uninitialized_aggregates)
+	wc = OPT_Wmaybe_uninitialized_aggregates;
+      else
+	return;
+    }
 
   /* Anonymous SSA_NAMEs shouldn't be uninitialized, but ssa_undefined_value_p
      can return true if the def stmt of anonymous SSA_NAME is COMPLEX_EXPR
@@ -2622,7 +2632,9 @@ warn_uninitialized_phi (gphi *phi, vec<gphi *> *worklist,
 static bool
 gate_warn_uninitialized (void)
 {
-  return warn_uninitialized || warn_maybe_uninitialized;
+  return (warn_uninitialized
+	  || warn_maybe_uninitialized
+	  || warn_maybe_uninitialized_aggregates);
 }
 
 namespace {
-- 
2.23.0



More information about the Gcc-patches mailing list