From 65791f426fc950b0af4e6e2ef675c213e623b16f Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Fri, 1 Dec 2017 09:17:06 +0100 Subject: [PATCH] re PR c/79153 (-Wimplicit-fallthrough missed warning) PR c/79153 * tree.h (SWITCH_BREAK_LABEL_P): Define. * gimplify.c (collect_fallthrough_labels): Handle GIMPLE_BIND starting with a GIMPLE_SWITCH and ending with GIMPLE_LABEL with SWITCH_BREAK_LABEL_P set on the label. (gimplify_switch_expr): Set SWITCH_BREAK_LABEL_P on the label added for default case if it was missing and not all cases covered. Wrap GIMPLE_SWITCH and the switch_body_seq into a GIMPLE_BIND if switch_body_seq ends with a GIMPLE_LABEL with SWITCH_BREAK_LABEL_P set on the label. * tree-chrec.c (evolution_function_is_univariate_p): Add return true; to avoid -Wimplicit-fallthrough warning. * config/i386/i386.c (ix86_expand_special_args_builtin): Add FALLTHRU comment to avoid -Wimplicit-fallthrough warning. c/ * c-parser.c: Include tree-iterator.h. (c_parser_switch_statement): Emit LABEL_EXPR for the break label into SWITCH_BODY instead of after it and set SWITCH_BREAK_LABEL_P on it. cp/ * cp-gimplify.c (genericize_switch_stmt): Emit LABEL_EXPR for the break label into SWITCH_BODY instead of after it and set SWITCH_BREAK_LABEL_P on it. * parser.c (cp_parser_objc_expression): Add FALLTHRU comment to avoid -Wimplicit-fallthrough warning. fortran/ * match.c (gfc_match): Add FALLTHRU comment to avoid -Wimplicit-fallthrough warning. testsuite/ * c-c++-common/Wimplicit-fallthrough-7.c: Adjust expected warning line. * c-c++-common/Wimplicit-fallthrough-36.c: New test. From-SVN: r255298 --- gcc/ChangeLog | 15 ++++ gcc/c/ChangeLog | 6 ++ gcc/c/c-parser.c | 6 +- gcc/config/i386/i386.c | 1 + gcc/cp/ChangeLog | 9 +++ gcc/cp/cp-gimplify.c | 4 +- gcc/cp/parser.c | 1 + gcc/fortran/ChangeLog | 6 ++ gcc/fortran/match.c | 1 + gcc/gimplify.c | 60 +++++++++++++++- gcc/testsuite/ChangeLog | 5 ++ .../c-c++-common/Wimplicit-fallthrough-36.c | 72 +++++++++++++++++++ .../c-c++-common/Wimplicit-fallthrough-7.c | 4 +- gcc/tree-chrec.c | 1 + gcc/tree.h | 5 ++ 15 files changed, 188 insertions(+), 8 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/Wimplicit-fallthrough-36.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 46c4545efbe..bd2626e0213 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,5 +1,20 @@ 2017-12-01 Jakub Jelinek + PR c/79153 + * tree.h (SWITCH_BREAK_LABEL_P): Define. + * gimplify.c (collect_fallthrough_labels): Handle GIMPLE_BIND + starting with a GIMPLE_SWITCH and ending with GIMPLE_LABEL with + SWITCH_BREAK_LABEL_P set on the label. + (gimplify_switch_expr): Set SWITCH_BREAK_LABEL_P on the label + added for default case if it was missing and not all cases covered. + Wrap GIMPLE_SWITCH and the switch_body_seq into a GIMPLE_BIND if + switch_body_seq ends with a GIMPLE_LABEL with SWITCH_BREAK_LABEL_P + set on the label. + * tree-chrec.c (evolution_function_is_univariate_p): Add return true; + to avoid -Wimplicit-fallthrough warning. + * config/i386/i386.c (ix86_expand_special_args_builtin): Add + FALLTHRU comment to avoid -Wimplicit-fallthrough warning. + PR tree-optimization/83221 * tree-ssa-reassoc.c (sort_by_operand_rank): Shift bb_rank down by 16. diff --git a/gcc/c/ChangeLog b/gcc/c/ChangeLog index 90db38a0419..1fb0c3d0b7f 100644 --- a/gcc/c/ChangeLog +++ b/gcc/c/ChangeLog @@ -1,5 +1,11 @@ 2017-12-01 Jakub Jelinek + PR c/79153 + * c-parser.c: Include tree-iterator.h. + (c_parser_switch_statement): Emit LABEL_EXPR for the break label + into SWITCH_BODY instead of after it and set SWITCH_BREAK_LABEL_P + on it. + PR c/83222 * c-tree.h (decl_constant_value_1): Declare. * c-typeck.c (decl_constant_value_1): New function. diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c index 755cd2b4341..e9267fe9cc1 100644 --- a/gcc/c/c-parser.c +++ b/gcc/c/c-parser.c @@ -67,6 +67,7 @@ along with GCC; see the file COPYING3. If not see #include "run-rtl-passes.h" #include "intl.h" #include "c-family/name-hint.h" +#include "tree-iterator.h" /* We need to walk over decls with incomplete struct/union/enum types after parsing the whole translation unit. @@ -5846,14 +5847,15 @@ c_parser_switch_statement (c_parser *parser, bool *if_p) if (!open_brace_p && c_parser_peek_token (parser)->type != CPP_SEMICOLON) warn_for_multistatement_macros (loc_after_labels, next_loc, switch_loc, RID_SWITCH); - c_finish_case (body, ce.original_type); if (c_break_label) { location_t here = c_parser_peek_token (parser)->location; tree t = build1 (LABEL_EXPR, void_type_node, c_break_label); SET_EXPR_LOCATION (t, here); - add_stmt (t); + SWITCH_BREAK_LABEL_P (c_break_label) = 1; + append_to_statement_list_force (t, &body); } + c_finish_case (body, ce.original_type); c_break_label = save_break; add_stmt (c_end_compound_stmt (switch_loc, block, flag_isoc99)); c_parser_maybe_reclassify_token (parser); diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 145b1c6043b..9f9db38c4b8 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -34987,6 +34987,7 @@ ix86_expand_special_args_builtin (const struct builtin_description *d, default: break; } + /* FALLTHRU */ case V64QI_FTYPE_PCCHAR_V64QI_UDI: case V32QI_FTYPE_PCCHAR_V32QI_USI: case V16QI_FTYPE_PCCHAR_V16QI_UHI: diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index bb0e2bbb7bf..31b69bf7255 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,12 @@ +2017-12-01 Jakub Jelinek + + PR c/79153 + * cp-gimplify.c (genericize_switch_stmt): Emit LABEL_EXPR for the + break label into SWITCH_BODY instead of after it and set + SWITCH_BREAK_LABEL_P on it. + * parser.c (cp_parser_objc_expression): Add FALLTHRU comment to avoid + -Wimplicit-fallthrough warning. + 2017-11-30 Jason Merrill PR c++/82219 - bogus -Wignored-qualifiers with template diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c index 3187a64a725..68a253a02db 100644 --- a/gcc/cp/cp-gimplify.c +++ b/gcc/cp/cp-gimplify.c @@ -330,11 +330,13 @@ genericize_switch_stmt (tree *stmt_p, int *walk_subtrees, void *data) cp_walk_tree (&type, cp_genericize_r, data, NULL); *walk_subtrees = 0; + if (TREE_USED (break_block)) + SWITCH_BREAK_LABEL_P (break_block) = 1; + finish_bc_block (&body, bc_break, break_block); *stmt_p = build2_loc (stmt_locus, SWITCH_EXPR, type, cond, body); SWITCH_ALL_CASES_P (*stmt_p) = SWITCH_STMT_ALL_CASES_P (stmt); gcc_checking_assert (!SWITCH_STMT_NO_BREAK_P (stmt) || !TREE_USED (break_block)); - finish_bc_block (stmt_p, bc_break, break_block); } /* Genericize a CONTINUE_STMT node *STMT_P. */ diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 03aeaea7597..707c6f55605 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -29003,6 +29003,7 @@ cp_parser_objc_expression (cp_parser* parser) default: break; } + /* FALLTHRU */ default: error_at (kwd->location, "misplaced %<@%D%> Objective-C++ construct", diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog index e6c048090ec..7154ac3c1ef 100644 --- a/gcc/fortran/ChangeLog +++ b/gcc/fortran/ChangeLog @@ -1,3 +1,9 @@ +2017-12-01 Jakub Jelinek + + PR c/79153 + * match.c (gfc_match): Add FALLTHRU comment to avoid + -Wimplicit-fallthrough warning. + 2017-12-01 Thomas Koenig PR fortran/83224 diff --git a/gcc/fortran/match.c b/gcc/fortran/match.c index dcabe269e61..c437c853f71 100644 --- a/gcc/fortran/match.c +++ b/gcc/fortran/match.c @@ -1240,6 +1240,7 @@ loop: default: gfc_internal_error ("gfc_match(): Bad match code %c", c); } + /* FALLTHRU */ default: diff --git a/gcc/gimplify.c b/gcc/gimplify.c index 2c2abd76c08..16a86ce70f0 100644 --- a/gcc/gimplify.c +++ b/gcc/gimplify.c @@ -1897,6 +1897,27 @@ collect_fallthrough_labels (gimple_stmt_iterator *gsi_p, do { + if (gimple_code (gsi_stmt (*gsi_p)) == GIMPLE_BIND) + { + /* Recognize the special GIMPLE_BIND added by gimplify_switch_expr, + which starts on a GIMPLE_SWITCH and ends with a break label. + Handle that as a single statement that can fall through. */ + gbind *bind = as_a (gsi_stmt (*gsi_p)); + gimple *first = gimple_seq_first_stmt (gimple_bind_body (bind)); + gimple *last = gimple_seq_last_stmt (gimple_bind_body (bind)); + if (last + && gimple_code (first) == GIMPLE_SWITCH + && gimple_code (last) == GIMPLE_LABEL) + { + tree label = gimple_label_label (as_a (last)); + if (SWITCH_BREAK_LABEL_P (label)) + { + prev = bind; + gsi_next (gsi_p); + continue; + } + } + } if (gimple_code (gsi_stmt (*gsi_p)) == GIMPLE_BIND || gimple_code (gsi_stmt (*gsi_p)) == GIMPLE_TRY) { @@ -2315,6 +2336,7 @@ gimplify_switch_expr (tree *expr_p, gimple_seq *pre_p) preprocess_case_label_vec_for_gimple (labels, index_type, &default_case); + bool add_bind = false; if (!default_case) { glabel *new_default; @@ -2322,14 +2344,46 @@ gimplify_switch_expr (tree *expr_p, gimple_seq *pre_p) default_case = build_case_label (NULL_TREE, NULL_TREE, create_artificial_label (UNKNOWN_LOCATION)); + if (old_in_switch_expr) + { + SWITCH_BREAK_LABEL_P (CASE_LABEL (default_case)) = 1; + add_bind = true; + } new_default = gimple_build_label (CASE_LABEL (default_case)); gimplify_seq_add_stmt (&switch_body_seq, new_default); } + else if (old_in_switch_expr) + { + gimple *last = gimple_seq_last_stmt (switch_body_seq); + if (last && gimple_code (last) == GIMPLE_LABEL) + { + tree label = gimple_label_label (as_a (last)); + if (SWITCH_BREAK_LABEL_P (label)) + add_bind = true; + } + } switch_stmt = gimple_build_switch (SWITCH_COND (switch_expr), - default_case, labels); - gimplify_seq_add_stmt (pre_p, switch_stmt); - gimplify_seq_add_seq (pre_p, switch_body_seq); + default_case, labels); + /* For the benefit of -Wimplicit-fallthrough, if switch_body_seq + ends with a GIMPLE_LABEL holding SWITCH_BREAK_LABEL_P LABEL_DECL, + wrap the GIMPLE_SWITCH up to that GIMPLE_LABEL into a GIMPLE_BIND, + so that we can easily find the start and end of the switch + statement. */ + if (add_bind) + { + gimple_seq bind_body = NULL; + gimplify_seq_add_stmt (&bind_body, switch_stmt); + gimple_seq_add_seq (&bind_body, switch_body_seq); + gbind *bind = gimple_build_bind (NULL_TREE, bind_body, NULL_TREE); + gimple_set_location (bind, EXPR_LOCATION (switch_expr)); + gimplify_seq_add_stmt (pre_p, bind); + } + else + { + gimplify_seq_add_stmt (pre_p, switch_stmt); + gimplify_seq_add_seq (pre_p, switch_body_seq); + } labels.release (); } else diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index e41a29ddb8f..ea2f2c7764b 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,5 +1,10 @@ 2017-12-01 Jakub Jelinek + PR c/79153 + * c-c++-common/Wimplicit-fallthrough-7.c: Adjust expected warning + line. + * c-c++-common/Wimplicit-fallthrough-36.c: New test. + PR sanitizer/81275 * c-c++-common/tsan/pr81275.c: Remove dg-skip-if. diff --git a/gcc/testsuite/c-c++-common/Wimplicit-fallthrough-36.c b/gcc/testsuite/c-c++-common/Wimplicit-fallthrough-36.c new file mode 100644 index 00000000000..1821e48b928 --- /dev/null +++ b/gcc/testsuite/c-c++-common/Wimplicit-fallthrough-36.c @@ -0,0 +1,72 @@ +/* PR c/79153 */ +/* { dg-do compile } */ +/* { dg-options "-Wimplicit-fallthrough" } */ + +int +test (int v1, int v2) +{ + switch (v1) + { + case 3: + switch (v2) /* { dg-warning "this statement may fall through" } */ + { + case 1: + return 28; + case 2: + return 38; + case 3: + return 88; + default: + break; + } + case 4: /* { dg-message "here" } */ + return 168; + case 5: + switch (v2) /* { dg-warning "this statement may fall through" } */ + { + case 4: + break; + case 5: + return 38; + case 6: + return 88; + } + case 6: /* { dg-message "here" } */ + return 169; + case 7: + switch (v2) /* { dg-warning "this statement may fall through" } */ + { + case 7: + return 38; + case 8: + return 88; + } + case 8: /* { dg-message "here" } */ + return 170; + case 9: + switch (v2) /* { dg-bogus "this statement may fall through" } */ + { + case 9: + return 38; + case 10: + return 88; + default: + return 89; + } + case 10: + return 171; + case 11: + switch (v2) /* { dg-bogus "this statement may fall through" } */ + { + case -__INT_MAX__ - 1 ... 31: + return 38; + case 32: + return 88; + case 33 ... __INT_MAX__: + return 89; + } + case 12: + return 172; + } + return -1; +} diff --git a/gcc/testsuite/c-c++-common/Wimplicit-fallthrough-7.c b/gcc/testsuite/c-c++-common/Wimplicit-fallthrough-7.c index 24a573b4d19..a602216fa73 100644 --- a/gcc/testsuite/c-c++-common/Wimplicit-fallthrough-7.c +++ b/gcc/testsuite/c-c++-common/Wimplicit-fallthrough-7.c @@ -51,9 +51,9 @@ f (int i) { case 1: { - switch (i + 2) + switch (i + 2) /* { dg-warning "statement may fall through" } */ case 4: - bar (1); /* { dg-warning "statement may fall through" } */ + bar (1); case 5: bar (5); return; diff --git a/gcc/tree-chrec.c b/gcc/tree-chrec.c index beddf108104..9c14374e93a 100644 --- a/gcc/tree-chrec.c +++ b/gcc/tree-chrec.c @@ -1161,6 +1161,7 @@ evolution_function_is_univariate_p (const_tree chrec) return false; break; } + return true; default: return true; diff --git a/gcc/tree.h b/gcc/tree.h index db6785820b0..e5a37afcce8 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -766,6 +766,11 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int, #define FALLTHROUGH_LABEL_P(NODE) \ (LABEL_DECL_CHECK (NODE)->base.private_flag) +/* Set on the artificial label created for break; stmt from a switch. + This is used to implement -Wimplicit-fallthrough. */ +#define SWITCH_BREAK_LABEL_P(NODE) \ + (LABEL_DECL_CHECK (NODE)->base.protected_flag) + /* Nonzero means this expression is volatile in the C sense: its address should be of type `volatile WHATEVER *'. In other words, the declared item is volatile qualified.