From patchwork Fri Nov 26 12:18:42 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 48181 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 4E9583858017 for ; Fri, 26 Nov 2021 12:19:13 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4E9583858017 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1637929153; bh=4IdYnu2iCYxnv8+WkVe8zJSN8Xe1KIed936t0iutor0=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=IldQ2rsneeXMlEuTpTScDZJvoQ1df/j3To2iZmXD2FCb7E3Fc7aZ1gz9zXrQojKjt fY/Jv8qJG/Kljm2eoaVoJUYdJAyrHbxdWTkg9DHuIszl1dxYcVtm0BfPZOc8eu2gGu xYZLQyY5ytOXAjLiVy790qfCUGJceGOw0SzWJQ3s= 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 A678B3858D39 for ; Fri, 26 Nov 2021 12:18:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A678B3858D39 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 9659A1FDFC for ; Fri, 26 Nov 2021 12:18:42 +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 824E013C36 for ; Fri, 26 Nov 2021 12:18:42 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id U2mPHqLQoGGJEwAAMHmgww (envelope-from ) for ; Fri, 26 Nov 2021 12:18:42 +0000 Date: Fri, 26 Nov 2021 13:18:42 +0100 (CET) To: gcc-patches@gcc.gnu.org Subject: [RFC][PATCH] c++/46476 - implement -Wunreachable-code-return Message-ID: MIME-Version: 1.0 X-Spam-Status: No, score=-11.8 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 implements a subset of -Wunreachable-code, unreachable code after a return stmt. Contrary to the previous attemt at CFG construction time this implements the bits during GIMPLE lowering where there are still all GIMPLE return stmts in the IL. The lowering phase keeps track of whether stmts can fallthru which is used to determine if the following stmt is reachable. The implementation only considers labels here. The fallthru flag is transparently extended to allow tracking a reason for non-fallthruness which is used to mark returns. This patch runs in to the same stray return/gcc_unreachable as the previous one and thus requires cleanup across the GCC code base which seems controversical. So I'm putting this on hold unless I receive some OK for cleanup in any way, meaning this isn't going to make stage3. Sorry. Richard. 2021-11-26 Richard Biener PR c++/46476 gcc/cp/ * decl.c (finish_function): Set input_location to BUILTINS_LOCATION around the code building the return 0 for main(). * cp-gimplify.c (genericize_if_stmt): Avoid optimizing if (true) and if (false) when -Wunreachable-code-return is in effect. gcc/ * common.opt (Wunreachable-code): Re-enable. (Wunreachable-code-return): New diagnostic, enabled by -Wextra and -Wunreachable-code. * doc/invoke.texi (Wunreachable-code): Document. (Wunreachable-code-return): Likewise. * gimple-low.c: Include diagnostic.h. (struct cft_reason): New. (lower_data::cannot_fallthru): Make a cft_reason. (lower_stmt): Diagnose unreachable stmts after a return. * Makefile.in (insn-emit.o-warn): Disable -Wunreachable-code-return. gcc/testsuite/ * c-c++-common/Wunreachable-code-return-1.c: New testcase. --- gcc/Makefile.in | 1 + gcc/common.opt | 8 +++-- gcc/cp/cp-gimplify.c | 6 ++-- gcc/cp/decl.c | 9 +++++- gcc/doc/invoke.texi | 13 ++++++++ gcc/gimple-low.c | 30 ++++++++++++++++--- .../c-c++-common/Wunreachable-code-return-1.c | 9 ++++++ 7 files changed, 67 insertions(+), 9 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/Wunreachable-code-return-1.c diff --git a/gcc/Makefile.in b/gcc/Makefile.in index a4344d67f44..71d326ff98c 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -222,6 +222,7 @@ libgcov-merge-tool.o-warn = -Wno-error gimple-match.o-warn = -Wno-unused generic-match.o-warn = -Wno-unused dfp.o-warn = -Wno-strict-aliasing +insn-emit.o-warn = -Wno-unreachable-code-return # All warnings have to be shut off in stage1 if the compiler used then # isn't gcc; configure determines that. WARN_CFLAGS will be either diff --git a/gcc/common.opt b/gcc/common.opt index 755e1a233b7..486ea36c8e5 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -806,8 +806,12 @@ 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 +Catch all for -Wunreachable-code-*. + +Wunreachable-code-return +Common Var(warn_unreachable_code_return) Warning EnabledBy(Wunreachable-code || Wextra) +Warn about unreachable statements after a return. Wunused Common Var(warn_unused) Init(0) Warning diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c index c1691c3e073..55e03abba43 100644 --- a/gcc/cp/cp-gimplify.c +++ b/gcc/cp/cp-gimplify.c @@ -167,9 +167,11 @@ genericize_if_stmt (tree *stmt_p) the then_ block regardless of whether else_ has side-effects or not. */ if (IF_STMT_CONSTEVAL_P (stmt)) stmt = else_; - else if (integer_nonzerop (cond) && !TREE_SIDE_EFFECTS (else_)) + else if (!warn_unreachable_code_return + && integer_nonzerop (cond) && !TREE_SIDE_EFFECTS (else_)) stmt = then_; - else if (integer_zerop (cond) && !TREE_SIDE_EFFECTS (then_)) + else if (!warn_unreachable_code_return + && integer_zerop (cond) && !TREE_SIDE_EFFECTS (then_)) stmt = else_; else stmt = build3 (COND_EXPR, void_type_node, cond, then_, else_); diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 56f80775ca0..cb81a794e5b 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -17573,7 +17573,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 3bddfbaae6a..7d7f2e7fddc 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -400,6 +400,7 @@ Objective-C and Objective-C++ Dialects}. -Wsystem-headers -Wtautological-compare -Wtrampolines -Wtrigraphs @gol -Wtsan -Wtype-limits -Wundef @gol -Wuninitialized -Wunknown-pragmas @gol +-Wunreachable-code -Wunreachable-code-return @gol -Wunsuffixed-float-constants -Wunused @gol -Wunused-but-set-parameter -Wunused-but-set-variable @gol -Wunused-const-variable -Wunused-const-variable=@var{n} @gol @@ -5721,6 +5722,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-return @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{)}} @@ -6865,6 +6867,17 @@ This warning is enabled by default for C and C++ programs. Warn when @code{__sync_fetch_and_nand} and @code{__sync_nand_and_fetch} built-in functions are used. These functions changed semantics in GCC 4.4. +@item -Wunreachable-code +@opindex Wunreachable-code +@opindex Wno-unreachable-code +Warn about unreachable code. Enables @option{-Wunreachable-code-return}. + +@item -Wunreachable-code-return +@opindex Wunreachable-code-return +@opindex Wno-unreachable-code-return +Warn about unreachable code after a return stmt. Enabled by +@option{-Wunreachable-code} and @option{-Wextra}. + @item -Wunused-but-set-parameter @opindex Wunused-but-set-parameter @opindex Wno-unused-but-set-parameter diff --git a/gcc/gimple-low.c b/gcc/gimple-low.c index 7e39c22df44..f0eb82f72f6 100644 --- a/gcc/gimple-low.c +++ b/gcc/gimple-low.c @@ -33,6 +33,7 @@ along with GCC; see the file COPYING3. If not see #include "predict.h" #include "gimple-predict.h" #include "gimple-fold.h" +#include "diagnostic.h" /* The differences between High GIMPLE and Low GIMPLE are the following: @@ -56,6 +57,22 @@ struct return_statements_t }; typedef struct return_statements_t return_statements_t; +/* Helper tracking the reason a previous stmt cannot fallthru. */ +struct cft_reason +{ + enum reason { CAN_FALLTHRU = false, UNKNOWN = true, RETURN }; + cft_reason () : m_reason (CAN_FALLTHRU) {} + cft_reason (bool b) : m_reason (b ? UNKNOWN : CAN_FALLTHRU) {} + cft_reason (reason r) : m_reason (r) {} + operator bool () const { return m_reason != CAN_FALLTHRU; } + cft_reason& operator|= (const cft_reason& other) + { + if (other.m_reason != m_reason && (bool)other) + m_reason = UNKNOWN; + return *this; + } + reason m_reason; +}; struct lower_data { @@ -67,7 +84,7 @@ struct lower_data vec return_statements; /* True if the current statement cannot fall through. */ - bool cannot_fallthru; + cft_reason cannot_fallthru; }; static void lower_stmt (gimple_stmt_iterator *, struct lower_data *); @@ -84,7 +101,7 @@ static void lower_builtin_posix_memalign (gimple_stmt_iterator *); static unsigned int lower_function_body (void) { - struct lower_data data; + struct lower_data data = {}; gimple_seq body = gimple_body (current_function_decl); gimple_seq lowered_body; gimple_stmt_iterator i; @@ -96,7 +113,6 @@ lower_function_body (void) gcc_assert (gimple_seq_first (body) == gimple_seq_last (body) && gimple_code (gimple_seq_first_stmt (body)) == GIMPLE_BIND); - memset (&data, 0, sizeof (data)); data.block = DECL_INITIAL (current_function_decl); BLOCK_SUBBLOCKS (data.block) = NULL_TREE; BLOCK_CHAIN (data.block) = NULL_TREE; @@ -249,6 +265,12 @@ lower_stmt (gimple_stmt_iterator *gsi, struct lower_data *data) gimple_set_block (stmt, data->block); + if (data->cannot_fallthru.m_reason == cft_reason::RETURN + && gimple_code (stmt) != GIMPLE_LABEL + && LOCATION_LOCUS (gimple_location (stmt)) > BUILTINS_LOCATION) + warning_at (gimple_location (stmt), OPT_Wunreachable_code_return, + "statement after return is not reachable"); + switch (gimple_code (stmt)) { case GIMPLE_BIND: @@ -272,7 +294,7 @@ lower_stmt (gimple_stmt_iterator *gsi, struct lower_data *data) else { lower_gimple_return (gsi, data); - data->cannot_fallthru = true; + data->cannot_fallthru = cft_reason::RETURN; } return; diff --git a/gcc/testsuite/c-c++-common/Wunreachable-code-return-1.c b/gcc/testsuite/c-c++-common/Wunreachable-code-return-1.c new file mode 100644 index 00000000000..17a35939182 --- /dev/null +++ b/gcc/testsuite/c-c++-common/Wunreachable-code-return-1.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-Wunreachable-code-return" } */ + +void baz(); +void foo () +{ + return; + baz (); /* { dg-warning "statement after return is not reachable" } */ +}