From 058f0b9e5f073da9d1d98a91e482cbdead1561ee Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Tue, 28 Nov 2017 22:22:52 +0100 Subject: [PATCH] re PR sanitizer/81275 (-fsanitize=thread produce incorrect -Wreturn-type warning) 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. From-SVN: r255217 --- gcc/ChangeLog | 6 ++ gcc/c-family/ChangeLog | 7 ++ gcc/c-family/c-common.c | 58 +++++++++++ gcc/c-family/c-common.h | 1 + gcc/c/ChangeLog | 6 ++ gcc/c/c-typeck.c | 2 + gcc/testsuite/ChangeLog | 5 + gcc/testsuite/c-c++-common/tsan/pr81275.c | 111 ++++++++++++++++++++++ gcc/tree.c | 6 ++ gcc/tree.h | 4 + 10 files changed, 206 insertions(+) create mode 100644 gcc/testsuite/c-c++-common/tsan/pr81275.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 52903c27c81..02f9a9e5789 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,9 @@ +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 ()). + 2017-11-28 Prathamesh Kulkarni Martin Jambor diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog index ce966bbaa4c..abbcb90bc9b 100644 --- a/gcc/c-family/ChangeLog +++ b/gcc/c-family/ChangeLog @@ -1,3 +1,10 @@ +2017-11-28 Jakub Jelinek + + PR sanitizer/81275 + * 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. + 2017-11-28 Julia Koval Sebastian Peryt diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index 8256165730d..1d79aee01c8 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -4898,6 +4898,64 @@ c_add_case_label (location_t loc, splay_tree cases, tree cond, tree orig_type, 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. diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h index 5e55e5efba6..7561531f98f 100644 --- a/gcc/c-family/c-common.h +++ b/gcc/c-family/c-common.h @@ -969,6 +969,7 @@ extern int case_compare (splay_tree_key, 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); diff --git a/gcc/c/ChangeLog b/gcc/c/ChangeLog index ff730438258..2c363483bb1 100644 --- a/gcc/c/ChangeLog +++ b/gcc/c/ChangeLog @@ -1,3 +1,9 @@ +2017-11-28 Jakub Jelinek + + PR sanitizer/81275 + * c-typeck.c (c_finish_case): Set SWITCH_ALL_CASES_P if + c_switch_covers_all_cases_p returns true. + 2017-11-28 Julia Koval Sebastian Peryt diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c index f761305bfdc..6846bc51d28 100644 --- a/gcc/c/c-typeck.c +++ b/gcc/c/c-typeck.c @@ -10360,6 +10360,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; diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index e0e027b7653..87332af7a29 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2017-11-28 Jakub Jelinek + + PR sanitizer/81275 + * c-c++-common/tsan/pr81275.c: New test. + 2017-11-28 Janne Blomqvist PR fortran/53796 diff --git a/gcc/testsuite/c-c++-common/tsan/pr81275.c b/gcc/testsuite/c-c++-common/tsan/pr81275.c new file mode 100644 index 00000000000..024b0c72f2c --- /dev/null +++ b/gcc/testsuite/c-c++-common/tsan/pr81275.c @@ -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" } */ diff --git a/gcc/tree.c b/gcc/tree.c index b99304c2ff1..e97654cd0f1 100644 --- a/gcc/tree.c +++ b/gcc/tree.c @@ -12348,6 +12348,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: diff --git a/gcc/tree.h b/gcc/tree.h index bd713f81263..db6785820b0 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -1166,6 +1166,10 @@ extern void protected_set_expr_location (tree, location_t); /* 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 are 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. */ -- 2.30.2