From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 67859 invoked by alias); 28 Nov 2017 08:49:42 -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 67837 invoked by uid 89); 28 Nov 2017 08:49:41 -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= 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; Tue, 28 Nov 2017 08:49:39 +0000 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 11FC65F159; Tue, 28 Nov 2017 08:49:37 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-116-77.ams2.redhat.com [10.36.116.77]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 4FEEE5C1A3; Tue, 28 Nov 2017 08:49:34 +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 vAS8nVKc032350; Tue, 28 Nov 2017 09:49:31 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id vAS8nSg1032349; Tue, 28 Nov 2017 09:49:28 +0100 Date: Tue, 28 Nov 2017 09:12:00 -0000 From: Jakub Jelinek To: "Joseph S. Myers" , Marek Polacek , Richard Biener Cc: gcc-patches@gcc.gnu.org Subject: [C PATCH] Handle C SWITCH_EXPR in block_may_fallthru (PR sanitizer/81275) Message-ID: <20171128084928.GO2353@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/msg02374.txt.bz2 Hi! This is the C version of the switch block_may_fallthru handling. Unlike C++ SWITCH_STMT, break; is represented in SWITCH_EXPR by a goto to a label emitted after the SWITCH_EXPR, so either block_may_fallthru finds such label (but then doesn't find the SWITCH_EXPR), or it finds SWITCH_EXPR, in which case if the body doesn't fall through (e.g. ends with a return stmt), then it may fall through only if it doesn't cover all the cases. This patch adds a bit that signals that, and computes whether all cases are covered (either if default: is present, or by walking the splay tree). Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2017-11-28 Jakub Jelinek PR sanitizer/81275 * tree.c (block_may_fallthru): Return false if SWITCH_ALL_CASES_P is set on SWITCH_EXPR and !block_may_fallthru (SWITCH_BODY ()). c/ * c-typeck.c (c_finish_case): Set SWITCH_ALL_CASES_P if c_switch_covers_all_cases_p returns true. c-family/ * c-common.c (c_switch_covers_all_cases_p_1, c_switch_covers_all_cases_p): New functions. * c-common.h (c_switch_covers_all_cases_p): Declare. testsuite/ * c-c++-common/tsan/pr81275.c: New test. --- gcc/tree.c.jj 2017-11-27 14:36:09.000000000 +0100 +++ gcc/tree.c 2017-11-27 15:11:15.715131528 +0100 @@ -12339,6 +12339,12 @@ block_may_fallthru (const_tree block) return false; case SWITCH_EXPR: + /* If there is a default: label or case labels cover all possible + SWITCH_COND values, then the SWITCH_EXPR will transfer control + to some case label in all cases and all we care is whether the + SWITCH_BODY falls through. */ + if (SWITCH_ALL_CASES_P (stmt)) + return block_may_fallthru (SWITCH_BODY (stmt)); return true; case COND_EXPR: --- gcc/tree.h.jj 2017-11-27 14:34:38.000000000 +0100 +++ gcc/tree.h 2017-11-27 15:08:23.510250289 +0100 @@ -1175,6 +1175,10 @@ extern void protected_set_expr_location /* SWITCH_EXPR accessors. These give access to the condition and body. */ #define SWITCH_COND(NODE) TREE_OPERAND (SWITCH_EXPR_CHECK (NODE), 0) #define SWITCH_BODY(NODE) TREE_OPERAND (SWITCH_EXPR_CHECK (NODE), 1) +/* True if there all case labels for all possible values of SWITCH_COND, either + because there is a default: case label or because the case label ranges cover + all values. */ +#define SWITCH_ALL_CASES_P(NODE) (SWITCH_EXPR_CHECK (NODE)->base.private_flag) /* CASE_LABEL_EXPR accessors. These give access to the high and low values of a case label, respectively. */ --- gcc/c/c-typeck.c.jj 2017-11-27 14:27:53.000000000 +0100 +++ gcc/c/c-typeck.c 2017-11-27 16:00:03.180982468 +0100 @@ -10407,6 +10407,8 @@ c_finish_case (tree body, tree type) type ? type : TREE_TYPE (cs->switch_expr), SWITCH_COND (cs->switch_expr), cs->bool_cond_p, cs->outside_range_p); + if (c_switch_covers_all_cases_p (cs->cases, TREE_TYPE (cs->switch_expr))) + SWITCH_ALL_CASES_P (cs->switch_expr) = 1; /* Pop the stack. */ c_switch_stack = cs->next; --- gcc/c-family/c-common.h.jj 2017-11-20 19:55:39.000000000 +0100 +++ gcc/c-family/c-common.h 2017-11-27 16:08:30.740827358 +0100 @@ -975,6 +975,7 @@ extern int case_compare (splay_tree_key, extern tree c_add_case_label (location_t, splay_tree, tree, tree, tree, tree, bool *); +extern bool c_switch_covers_all_cases_p (splay_tree, tree); extern tree build_function_call (location_t, tree, tree); --- gcc/c-family/c-common.c.jj 2017-11-21 14:56:50.000000000 +0100 +++ gcc/c-family/c-common.c 2017-11-27 16:09:52.555839861 +0100 @@ -4904,6 +4904,64 @@ c_add_case_label (location_t loc, splay_ return error_mark_node; } +/* Subroutine of c_switch_covers_all_cases_p, called via + splay_tree_foreach. Return 1 if it doesn't cover all the cases. + ARGS[0] is initially NULL and after the first iteration is the + so far highest case label. ARGS[1] is the minimum of SWITCH_COND's + type. */ + +static int +c_switch_covers_all_cases_p_1 (splay_tree_node node, void *data) +{ + tree label = (tree) node->value; + tree *args = (tree *) data; + + /* If there is a default case, we shouldn't have called this. */ + gcc_assert (CASE_LOW (label)); + + if (args[0] == NULL_TREE) + { + if (wi::to_widest (args[1]) < wi::to_widest (CASE_LOW (label))) + return 1; + } + else if (wi::add (wi::to_widest (args[0]), 1) + != wi::to_widest (CASE_LOW (label))) + return 1; + if (CASE_HIGH (label)) + args[0] = CASE_HIGH (label); + else + args[0] = CASE_LOW (label); + return 0; +} + +/* Return true if switch with CASES and switch condition with type + covers all possible values in the case labels. */ + +bool +c_switch_covers_all_cases_p (splay_tree cases, tree type) +{ + /* If there is default:, this is always the case. */ + splay_tree_node default_node + = splay_tree_lookup (cases, (splay_tree_key) NULL); + if (default_node) + return true; + + if (!INTEGRAL_TYPE_P (type)) + return false; + + tree args[2] = { NULL_TREE, TYPE_MIN_VALUE (type) }; + if (splay_tree_foreach (cases, c_switch_covers_all_cases_p_1, args)) + return false; + + /* If there are no cases at all, or if the highest case label + is smaller than TYPE_MAX_VALUE, return false. */ + if (args[0] == NULL_TREE + || wi::to_widest (args[0]) < wi::to_widest (TYPE_MAX_VALUE (type))) + return false; + + return true; +} + /* Finish an expression taking the address of LABEL (an IDENTIFIER_NODE). Returns an expression for the address. --- gcc/testsuite/c-c++-common/tsan/pr81275.c.jj 2017-11-27 16:22:25.674749806 +0100 +++ gcc/testsuite/c-c++-common/tsan/pr81275.c 2017-11-27 16:20:42.000000000 +0100 @@ -0,0 +1,111 @@ +/* PR sanitizer/81275 */ +/* { dg-do compile } */ +/* { dg-options "-Wreturn-type -fsanitize=thread" } */ + +int +f1 (int a, int b) +{ + switch (a) + { + case 0: + switch (b) + { + case 5: + return 6; + case 7: + return 8; + default: + return 0; + } + break; + default: + return 0; + } +} /* { dg-bogus "control reaches end of non-void function" } */ + +int +f2 (int a, int b) +{ + switch (a) + { + case 0: + switch (b) + { + case 5: + return 6; + case 7: + return 8; + default: + return 0; + } + default: + return 0; + } +} /* { dg-bogus "control reaches end of non-void function" } */ + +int +f3 (int a, int b) +{ + switch (a) + { + case 0: + switch (b) + { + case 5: + return 6; + case 7: + return 8; + case 8: + break; + default: + return 0; + } + break; + default: + return 0; + } +} /* { dg-warning "control reaches end of non-void function" } */ + +int +f4 (int a, int b) +{ + switch (a) + { + case 0: + switch (b) + { + case 5: + return 6; + case 7: + return 8; + } + break; + default: + return 0; + } +} /* { dg-warning "control reaches end of non-void function" } */ + +int +f5 (int a, unsigned char b) +{ + switch (a) + { + case 0: + switch (b) + { + case 0: + return 1; + case 3 ... 10: + return 2; + case 1 ... 2: + return 3; + case 126 ... (unsigned char) ~0: + return 4; + case 11 ... 125: + return 5; + } + break; + default: + return 0; + } +} /* { dg-bogus "control reaches end of non-void function" } */ Jakub