From patchwork Wed Nov 24 15:21:31 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 48066 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 7E53D385780E for ; Wed, 24 Nov 2021 15:22:03 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7E53D385780E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1637767323; bh=sJbSfI7uHldhGeuJySdkxlf5iyi+0RWZBD1C1s2Gtt4=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=CHdiF8m27iB4Zgxfcg9AHPXj0IHdoq/myS/4GJBqbRAlfN0eyUo7VKYqPd+wSSS/K jshk8M2VMcbCHkDC2KI0FAaGQCngRHsoertsFbiLIRv99u5mCLMRjSS26y2JrDiiEv JNUdinYdD4nxajDnpptJl2Xta33gjnxxZKXXZzHY= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id A249A3858C39 for ; Wed, 24 Nov 2021 15:21:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A249A3858C39 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 6369F1FD2F; Wed, 24 Nov 2021 15:21:32 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 4582B13F2C; Wed, 24 Nov 2021 15:21:32 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id M95eD3xYnmFvAgAAMHmgww (envelope-from ); Wed, 24 Nov 2021 15:21:32 +0000 Date: Wed, 24 Nov 2021 16:21:31 +0100 (CET) To: gcc-patches@gcc.gnu.org Subject: [PATCH][RFC] middle-end/46476 - resurrect -Wunreachable-code Message-ID: MIME-Version: 1.0 X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Richard Biener via Gcc-patches From: Richard Biener Reply-To: Richard Biener Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" 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 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 (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 bbs_to_cleanup; + auto_vec 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]; }