From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 34658 invoked by alias); 24 Nov 2017 22:00:01 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 34624 invoked by uid 89); 24 Nov 2017 22:00:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.7 required=5.0 tests=BAYES_00,GIT_PATCH_2,GIT_PATCH_3,KB_WAM_FROM_NAME_SINGLEWORD,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=Inside X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 24 Nov 2017 21:59:58 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7EB9881DE5; Fri, 24 Nov 2017 21:59:57 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-116-247.ams2.redhat.com [10.36.116.247]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 0E46B60BEF; Fri, 24 Nov 2017 21:59:56 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id vAOLxsjB010605; Fri, 24 Nov 2017 22:59:54 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id vAOLxrPc010604; Fri, 24 Nov 2017 22:59:53 +0100 Date: Sat, 25 Nov 2017 00:37:00 -0000 From: Jakub Jelinek To: Jason Merrill , Nathan Sidwell Cc: gcc-patches@gcc.gnu.org Subject: [C++ PATCH] Avoid -Wreturn-type warnings if a switch has default label, no breaks inside of it, but is followed by a break (PR sanitizer/81275) Message-ID: <20171124215953.GE14653@tucnak> Reply-To: Jakub Jelinek MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.7.1 (2016-10-04) X-IsSubscribed: yes X-SW-Source: 2017-11/txt/msg02228.txt.bz2 Hi! The testcase below has a useless break; that causes a bogus -Wreturn-type warning. The C++ FE already has code to avoid adding a BREAK_STMT after a return or similar sequence that is known not to return. The following patch extends block_may_fallthrough to also return false for SWITCH_STMTs that can't fall through. Those are ones with non-empty body where the whole body can't fallthrough, additionally they need to have a default: case label (or cover the whole range of values, but that is not what this patch can compute, that would be too big duplication of the gimplification processing) and no BREAK_STMT. For the default: case label we need to look in all SWITCH_BODY children except for nested SWITCH_STMTs, for BREAK_STMTs also not in {FOR,DO,WHILE}_BODY. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2017-11-24 Jakub Jelinek PR sanitizer/81275 * cp-objcp-common.c (struct find_default_and_break_s): New type. (find_default_and_break): New function. (cxx_block_may_fallthru): Return false for SWITCH_STMT which contains no BREAK_STMTs and contains a default: CASE_LABEL_EXPR. * g++.dg/warn/pr81275.C: New test. --- gcc/cp/cp-objcp-common.c.jj 2017-11-15 09:38:25.000000000 +0100 +++ gcc/cp/cp-objcp-common.c 2017-11-24 20:08:35.047132170 +0100 @@ -335,6 +335,96 @@ init_shadowed_var_for_decl (void) = hash_table::create_ggc (512); } +/* Data structure for find_default_and_break. */ + +struct find_default_and_break_s +{ + hash_set *pset; + bool found_default; + bool look_for_break; +}; + +/* Helper function for cxx_block_may_fallthru SWITCH_STMT processing. + Determine if SWITCH_STMT body contains a BREAK_STMT and also record + if it contains default: CASE_LABEL_EXPR. */ + +static tree +find_default_and_break (tree *stmt_p, int *walk_subtrees, void *d) +{ + find_default_and_break_s *data = (find_default_and_break_s *) d; + tree stmt = *stmt_p; + switch (TREE_CODE (stmt)) + { + case FOR_STMT: + if (cp_walk_tree (&FOR_INIT_STMT (stmt), find_default_and_break, + d, data->pset) + || cp_walk_tree (&FOR_COND (stmt), find_default_and_break, + d, data->pset) + || cp_walk_tree (&FOR_EXPR (stmt), find_default_and_break, + d, data->pset)) + return stmt; + stmt = FOR_BODY (stmt); + goto loop_body; + case WHILE_STMT: + if (cp_walk_tree (&WHILE_COND (stmt), find_default_and_break, + d, data->pset)) + return stmt; + stmt = WHILE_BODY (stmt); + goto loop_body; + case DO_STMT: + if (cp_walk_tree (&DO_COND (stmt), find_default_and_break, + d, data->pset)) + return stmt; + stmt = DO_BODY (stmt); + loop_body: + *walk_subtrees = 0; + /* Inside of {FOR,WHILE,DO}_BODY we can ignore BREAK_STMTs that + are related to the corresponding loop, not the outer SWITCH_STMT. + If we haven't still found a default:, we need to look for it + though. */ + if (!data->found_default) + { + if (data->look_for_break) + { + if (cp_walk_tree (&stmt, find_default_and_break, d, data->pset)) + return *stmt_p; + } + else + { + data->look_for_break = true; + if (cp_walk_tree (&stmt, find_default_and_break, d, data->pset)) + { + data->look_for_break = false; + return *stmt_p; + } + data->look_for_break = false; + } + } + return NULL_TREE; + case CASE_LABEL_EXPR: + if (CASE_LOW (stmt) == NULL_TREE && CASE_HIGH (stmt) == NULL_TREE) + { + /* Found default:. */ + data->found_default = true; + if (!data->look_for_break) + return stmt; + } + return NULL_TREE; + case BREAK_STMT: + /* Found a break that can fall through out of the SWITCH_STMT. */ + return data->look_for_break ? stmt : NULL_TREE; + case SWITCH_STMT: + /* Nested switch can't contain a break; nor default: for the outer + switch. */ + *walk_subtrees = 0; + return NULL_TREE; + default: + if (TYPE_P (stmt)) + *walk_subtrees = 0; + return NULL_TREE; + } +} + /* Return true if stmt can fall through. Used by block_may_fallthru default case. */ @@ -349,6 +439,25 @@ cxx_block_may_fallthru (const_tree stmt) case THROW_EXPR: return false; + case SWITCH_STMT: + if (SWITCH_STMT_BODY (stmt) == NULL_TREE + || block_may_fallthru (SWITCH_STMT_BODY (stmt))) + return true; + else + { + /* If the switch stmt body can't fall through, it can still + fall through if there is no default: label (checking whether + without default: case labels cover the whole range might be + too expensive) or if there are any break; stmts. */ + hash_set pset; + find_default_and_break_s data = { &pset, false, true }; + if (cp_walk_tree (&SWITCH_STMT_BODY (stmt), find_default_and_break, + &data, &pset) + || !data.found_default) + return true; + return false; + } + default: return true; } --- gcc/testsuite/g++.dg/warn/pr81275.C.jj 2017-11-24 20:24:08.232869743 +0100 +++ gcc/testsuite/g++.dg/warn/pr81275.C 2017-11-24 20:23:38.000000000 +0100 @@ -0,0 +1,29 @@ +// PR sanitizer/81875 +// { dg-do compile } +// { dg-options "-Wreturn-type" } + +struct C { C (); ~C (); }; + +int +foo (int a, int b) +{ + C c; + switch (a) + { + case 0: + switch (b) + { + case 13: + return 7; + case 24: + return 19; + default: + return 0; + } + break; + default: + return 0; + case 9: + return 17; + } +} // { dg-bogus "control reaches end of non-void function" } Jakub