On Wed, Nov 24, 2021 at 10:22 AM Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > 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.
There are several bugs about reviving -Wunreachable-code open, all for different aspects of it. Do we want to consider making it an umbrella flag that's split into multiple sub-options? Bug 46476, which you mentioned, was suggested to be -Wunreachable-code-return specifically: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476 Meanwhile, there's also bug 92479, for a warning to be named -Wunreachable-code-break: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92479 Then there's also bug 82100, which doesn't have a name suggested for it yet: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82100 I think having separate flags for the 3 of these that are all enabled by -Wunreachable-code as an umbrella would be good. Eric > > 2021-11-24 Richard Biener <rguent...@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
