[PATCH][RFC] middle-end/46476 - resurrect -Wunreachable-code

Richard Biener rguenther@suse.de
Wed Nov 24 15:21:31 GMT 2021


This resurrects -Wunreachable-code and implements a warning for
trivially unreachable code as of CFG construction.  Most problematic
with this is the C/C++ frontend added 'return 0;' stmt in main
which the patch handles for C++ like the C frontend already does
by using BUILTINS_LOCATION.

Another problem for future enhancement is that after CFG construction
we no longer can point to the stmt making a stmt unreachable, so
this implementation tries to warn on the first unreachable
statement of a region.  It might be possible to retain a pointer
to the stmt that triggered creation of a basic-block but I'm not
sure how reliable that would be.

So this is really a simple attempt for now, triggered by myself
running into such a coding error.  As always, the perfect is the
enemy of the good.

It does not pass bootstrap (which enables -Wextra), because of the
situation in g++.dg/Wunreachable-code-5.C where the C++ frontend
prematurely elides conditions like if (! GATHER_STATISTICS) that
evaluate to true - oddly enough it does _not_ do this for
conditions evaluating to false ... (one of the
c-c++-common/Wunreachable-code-2.c cases).

Richard.

2021-11-24  Richard Biener  <rguenther@suse.de>

	PR middle-end/46476
	* common.opt (Wunreachable-code): No longer ignored,
	add warn_unreachable_code variable, enable with -Wextra.
	* doc/invoke.texi (Wunreachable-code): Document.
	(Wextra): Amend.
	* tree-cfg.c (build_gimple_cfg): Move case label grouping...
	(execute_build_cfg): ... here after new -Wunreachable-code
	warnings.
	(warn_unreachable_code_post_cfg_build): New function.
	(mark_forward_reachable_blocks): Likewise.
	(reverse_guess_deadend): Likewise.

gcc/cp/
	* decl.c (finish_function): Set input_location to
	BUILTINS_LOCATION around the code building the return 0
	for main().

libgomp/
	* oacc-plugin.c (GOMP_PLUGIN_acc_default_dim): Remove spurious
	return.

gcc/testsuite/
	* c-c++-common/Wunreachable-code-1.c: New testcase.
	* c-c++-common/Wunreachable-code-2.c: Likewise.
	* c-c++-common/Wunreachable-code-3.c: Likewise.
	* gcc.dg/Wunreachable-code-4.c: Likewise.
	* g++.dg/Wunreachable-code-5.C: Likewise.
---
 gcc/common.opt                                |   4 +-
 gcc/cp/decl.c                                 |   9 +-
 gcc/doc/invoke.texi                           |   9 +-
 .../c-c++-common/Wunreachable-code-1.c        |   8 ++
 .../c-c++-common/Wunreachable-code-2.c        |   8 ++
 .../c-c++-common/Wunreachable-code-3.c        |  35 ++++++
 gcc/testsuite/g++.dg/Wunreachable-code-5.C    |  11 ++
 gcc/testsuite/gcc.dg/Wunreachable-code-4.c    |  10 ++
 gcc/tree-cfg.c                                | 101 +++++++++++++++++-
 libgomp/oacc-plugin.c                         |   1 -
 10 files changed, 186 insertions(+), 10 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/Wunreachable-code-1.c
 create mode 100644 gcc/testsuite/c-c++-common/Wunreachable-code-2.c
 create mode 100644 gcc/testsuite/c-c++-common/Wunreachable-code-3.c
 create mode 100644 gcc/testsuite/g++.dg/Wunreachable-code-5.C
 create mode 100644 gcc/testsuite/gcc.dg/Wunreachable-code-4.c

diff --git a/gcc/common.opt b/gcc/common.opt
index 755e1a233b7..0a58cb8a668 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -806,8 +806,8 @@ Common Var(warn_maybe_uninitialized) Warning EnabledBy(Wuninitialized)
 Warn about maybe uninitialized automatic variables.
 
 Wunreachable-code
-Common Ignore Warning
-Does nothing. Preserved for backward compatibility.
+Common Var(warn_unreachable_code) Warning EnabledBy(Wextra)
+Warn about trivially unreachable code.
 
 Wunused
 Common Var(warn_unused) Init(0) Warning
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 588094b1db6..26325e41efa 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -17571,7 +17571,14 @@ finish_function (bool inline_p)
     {
       /* Make it so that `main' always returns 0 by default.  */
       if (DECL_MAIN_P (current_function_decl))
-	finish_return_stmt (integer_zero_node);
+	{
+	  /* Hack.  We don't want the middle-end to warn that this return
+	     is unreachable, so we mark its location as special.  */
+	  auto saved_il = input_location;
+	  input_location = BUILTINS_LOCATION;
+	  finish_return_stmt (integer_zero_node);
+	  input_location = saved_il;
+	}
 
       if (use_eh_spec_block (current_function_decl))
 	finish_eh_spec_block (TYPE_RAISES_EXCEPTIONS
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 36fe96b441b..62643e51915 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -267,7 +267,7 @@ in the following sections.
 -Woverloaded-virtual  -Wno-pmf-conversions -Wsign-promo @gol
 -Wsized-deallocation  -Wsuggest-final-methods @gol
 -Wsuggest-final-types  -Wsuggest-override  @gol
--Wno-terminate  -Wuseless-cast  -Wno-vexing-parse  @gol
+-Wno-terminate  -Wunreachable-code  -Wuseless-cast  -Wno-vexing-parse  @gol
 -Wvirtual-inheritance  @gol
 -Wno-virtual-move-assign  -Wvolatile  -Wzero-as-null-pointer-constant}
 
@@ -4357,6 +4357,12 @@ annotations.
 Warn about overriding virtual functions that are not marked with the
 @code{override} keyword.
 
+@item -Wunreachable-code
+@opindex Wunreachable-code
+@opindex Wno-unreachable-code
+Warn about code that is trivially unreachable before optimizing.
+@option{-Wunreachable-code} is enabled by @option{-Wextra}.
+
 @item -Wuseless-cast @r{(C++ and Objective-C++ only)}
 @opindex Wuseless-cast
 @opindex Wno-useless-cast
@@ -5713,6 +5719,7 @@ name is still supported, but the newer name is more descriptive.)
 -Wredundant-move @r{(only for C++)}  @gol
 -Wtype-limits  @gol
 -Wuninitialized  @gol
+-Wunreachable-code @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{)}}
diff --git a/gcc/testsuite/c-c++-common/Wunreachable-code-1.c b/gcc/testsuite/c-c++-common/Wunreachable-code-1.c
new file mode 100644
index 00000000000..0ca6c00e7fb
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wunreachable-code-1.c
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-Wunreachable-code" } */
+
+int main()
+{
+  /* Avoid warning on the auto-added duplicate return 0. */
+  return 0;
+}
diff --git a/gcc/testsuite/c-c++-common/Wunreachable-code-2.c b/gcc/testsuite/c-c++-common/Wunreachable-code-2.c
new file mode 100644
index 00000000000..836d8c30cb2
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wunreachable-code-2.c
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-Wunreachable-code" } */
+
+int main()
+{
+  return 0;
+  return 0; /* { dg-warning "not reachable" } */
+}
diff --git a/gcc/testsuite/c-c++-common/Wunreachable-code-3.c b/gcc/testsuite/c-c++-common/Wunreachable-code-3.c
new file mode 100644
index 00000000000..a01e4119c6f
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wunreachable-code-3.c
@@ -0,0 +1,35 @@
+/* { dg-do compile } */
+/* { dg-options "-Wunreachable-code" } */
+
+/* Unreachable region entry has a predecessor (backedge).  */
+void foo()
+{
+  return;
+  for (int i = 0; i < 5; ++i) /* { dg-warning "not reachable" } */
+    ;
+}
+
+/* Unreachable region not backwards reachable from exit.  */
+void bar1()
+{
+  return;
+  __builtin_abort (); /* { dg-warning "not reachable" } */
+}
+void bar2()
+{
+  return;
+  /* Here we end up with a BB without any statements and an edge
+     to itself.  A location might be obtainable from the edges
+     goto_locus.  */
+  for (;;); /* { dg-warning "not reachable" "" { xfail *-*-* } } */
+}
+
+/* Unreachable code in if (0) block.  */
+void baz(int *p)
+{
+   if (0)
+     {
+        return;  /* { dg-bogus "not reachable" } */
+        *p = 0;  /* { dg-warning "not reachable" } */
+     }
+}
diff --git a/gcc/testsuite/g++.dg/Wunreachable-code-5.C b/gcc/testsuite/g++.dg/Wunreachable-code-5.C
new file mode 100644
index 00000000000..832cb6d865f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/Wunreachable-code-5.C
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-Wunreachable-code" } */
+
+void baz();
+void bar2()
+{
+   if (! 0)
+     return;
+   /* The C++ frontend elides the if above.  */
+   baz (); /* { dg-bogus "not reachable" "" { xfail *-*-* } } */
+}
diff --git a/gcc/testsuite/gcc.dg/Wunreachable-code-4.c b/gcc/testsuite/gcc.dg/Wunreachable-code-4.c
new file mode 100644
index 00000000000..997ec08fb41
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wunreachable-code-4.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-Wunreachable-code" } */
+
+void baz();
+void bar2()
+{
+   if (! 0)
+     return;
+   baz (); /* { dg-bogus "not reachable" } */
+}
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 8ed8c69b5b1..5a9507d0e44 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -239,11 +239,6 @@ build_gimple_cfg (gimple_seq seq)
   /* To speed up statement iterator walks, we first purge dead labels.  */
   cleanup_dead_labels ();
 
-  /* Group case nodes to reduce the number of edges.
-     We do this after cleaning up dead labels because otherwise we miss
-     a lot of obvious case merging opportunities.  */
-  group_case_labels ();
-
   /* Create the edges of the flowgraph.  */
   discriminator_per_locus = new hash_table<locus_discrim_hasher> (13);
   make_edges ();
@@ -362,6 +357,94 @@ replace_loop_annotate (void)
     }
 }
 
+/* Mark BB and blocks forward reachable from it as BB_REACHABLE.  */
+
+static void
+mark_forward_reachable_blocks (basic_block bb)
+{
+  bb->flags |= BB_REACHABLE;
+  edge_iterator ei;
+  edge e;
+  FOR_EACH_EDGE (e, ei, bb->succs)
+    if (!(e->dest->flags & BB_REACHABLE))
+      mark_forward_reachable_blocks (e->dest);
+}
+
+/* Guess a reverse dead end from BB.  */
+
+static basic_block
+reverse_guess_deadend (basic_block bb)
+{
+  /* Quick and simple minded.  */
+  if (EDGE_COUNT (bb->preds) == 0)
+    return bb;
+
+  /* DFS walk.  */
+  auto_bb_flag visited (cfun);
+  auto_bb_flag on_worklist (cfun);
+  auto_vec<basic_block, 64> bbs_to_cleanup;
+  auto_vec<basic_block, 64> worklist;
+  worklist.quick_push (bb);
+  bb->flags |= on_worklist;
+  bbs_to_cleanup.safe_push (bb);
+  while (!worklist.is_empty ())
+    {
+      bb = worklist.pop ();
+      bb->flags &= ~on_worklist;
+      bb->flags |= visited;
+      bool all_preds_visited = true;
+      edge_iterator ei;
+      edge e;
+      FOR_EACH_EDGE (e, ei, bb->preds)
+	if (!(e->src->flags & visited))
+	  {
+	    if (!(e->src->flags & on_worklist))
+	      {
+		worklist.safe_push (e->src);
+		e->src->flags |= on_worklist;
+		bbs_to_cleanup.safe_push (e->src);
+	      }
+	    all_preds_visited = false;
+	  }
+      /* Found one deadend.  */
+      if (all_preds_visited)
+	break;
+    }
+  for (auto bb2 : bbs_to_cleanup)
+    bb2->flags &= ~(on_worklist|visited);
+  return bb;
+}
+
+/* Warn about trivially unreachable code.  */
+
+static void
+warn_unreachable_code_post_cfg_build (void)
+{
+  find_unreachable_blocks ();
+  basic_block bb;
+  FOR_EACH_BB_FN (bb, cfun)
+    {
+      if ((bb->flags & BB_REACHABLE))
+	continue;
+      /* Try to find the entry to the unreachable region.  */
+      bb = reverse_guess_deadend (bb);
+      for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi);
+	   gsi_next (&gsi))
+	{
+	  /* Suppress warnings on BUILTINS_LOCATION which is used by the
+	     C and C++ frontends to emit the artifical return in main.  */
+	  if (LOCATION_LOCUS (gimple_location (gsi_stmt (gsi)))
+	      > BUILTINS_LOCATION)
+	    warning_at (gimple_location (gsi_stmt (gsi)),
+			OPT_Wunreachable_code, "statement is not reachable");
+	  break;
+	}
+      /* Mark blocks reachable from here.  And even better make
+	 sure to process entries to unreachable regions first.  */
+      mark_forward_reachable_blocks (bb);
+    }
+}
+
 static unsigned int
 execute_build_cfg (void)
 {
@@ -374,6 +457,14 @@ execute_build_cfg (void)
       fprintf (dump_file, "Scope blocks:\n");
       dump_scope_blocks (dump_file, dump_flags);
     }
+
+  if (warn_unreachable_code)
+    warn_unreachable_code_post_cfg_build ();
+
+  /* Group case nodes.  We did this prior to materializing edges in
+     build_gimple_cfg to reduce the number of edges, that interferes
+     with the -Wunreachable-code diagnostics above.  */
+  group_case_labels ();
   cleanup_tree_cfg ();
 
   bb_to_omp_idx.release ();
diff --git a/libgomp/oacc-plugin.c b/libgomp/oacc-plugin.c
index e25b462eff0..98166fe5cd1 100644
--- a/libgomp/oacc-plugin.c
+++ b/libgomp/oacc-plugin.c
@@ -62,7 +62,6 @@ GOMP_PLUGIN_acc_default_dim (unsigned int i)
   if (i >= GOMP_DIM_MAX)
     {
       gomp_fatal ("invalid dimension argument: %d", i);
-      return -1;
     }
   return goacc_default_dims[i];
 }
-- 
2.31.1


More information about the Gcc-patches mailing list