From c280b7ee5c5dfcc1b4ae2c0389987b7b67ec8cf8 Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Fri, 19 Apr 2019 13:56:07 +0200 Subject: [PATCH] re PR c/89888 (When switch controlling expression is promoted from type narrower than int, GCC does not diagnose identical cases) PR c/89888 * c-common.h (c_add_case_label): Remove orig_type and outside_range_p arguments. (c_do_switch_warnings): Remove outside_range_p argument. * c-common.c (check_case_bounds): Removed. (c_add_case_label): Remove orig_type and outside_range_p arguments. Don't call check_case_bounds. Fold low_value as well as high_value. * c-warn.c (c_do_switch_warnings): Remove outside_range_p argument. Check for case labels outside of range of original type here and adjust them. c/ * c-typeck.c (struct c_switch): Remove outside_range_p member. (c_start_case): Don't clear it. (do_case): Adjust c_add_case_label caller. (c_finish_case): Adjust c_do_switch_warnings caller. cp/ * decl.c (struct cp_switch): Remove outside_range_p member. (push_switch): Don't clear it. (pop_switch): Adjust c_do_switch_warnings caller. (finish_case_label): Adjust c_add_case_label caller. testsuite/ * c-c++-common/pr89888.c: New test. * g++.dg/torture/pr40335.C: Change dg-bogus into dg-warning. Don't expect -Wswitch-unreachable warning. From-SVN: r270455 --- gcc/c-family/ChangeLog | 13 ++++ gcc/c-family/c-common.c | 102 ++----------------------- gcc/c-family/c-common.h | 6 +- gcc/c-family/c-warn.c | 79 ++++++++++++++++++- gcc/c/ChangeLog | 6 ++ gcc/c/c-typeck.c | 12 +-- gcc/cp/ChangeLog | 6 ++ gcc/cp/decl.c | 11 +-- gcc/testsuite/ChangeLog | 5 ++ gcc/testsuite/c-c++-common/pr89888.c | 67 ++++++++++++++++ gcc/testsuite/g++.dg/torture/pr40335.C | 4 +- 11 files changed, 188 insertions(+), 123 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/pr89888.c diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog index aeceba7beeb..12d4587bb0d 100644 --- a/gcc/c-family/ChangeLog +++ b/gcc/c-family/ChangeLog @@ -1,3 +1,16 @@ +2019-04-19 Jakub Jelinek + + PR c/89888 + * c-common.h (c_add_case_label): Remove orig_type and outside_range_p + arguments. + (c_do_switch_warnings): Remove outside_range_p argument. + * c-common.c (check_case_bounds): Removed. + (c_add_case_label): Remove orig_type and outside_range_p arguments. + Don't call check_case_bounds. Fold low_value as well as high_value. + * c-warn.c (c_do_switch_warnings): Remove outside_range_p argument. + Check for case labels outside of range of original type here and + adjust them. + 2019-04-12 Jakub Jelinek PR translation/90041 diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index 67f68f0b76f..99ca1ad3727 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -314,8 +314,6 @@ const struct fname_var_t fname_vars[] = struct visibility_flags visibility_options; static tree check_case_value (location_t, tree); -static bool check_case_bounds (location_t, tree, tree, tree *, tree *, - bool *); static void check_nonnull_arg (void *, tree, unsigned HOST_WIDE_INT); @@ -2103,86 +2101,6 @@ check_case_value (location_t loc, tree value) return value; } -/* See if the case values LOW and HIGH are in the range of the original - type (i.e. before the default conversion to int) of the switch testing - expression. - TYPE is the promoted type of the testing expression, and ORIG_TYPE is - the type before promoting it. CASE_LOW_P is a pointer to the lower - bound of the case label, and CASE_HIGH_P is the upper bound or NULL - if the case is not a case range. - The caller has to make sure that we are not called with NULL for - CASE_LOW_P (i.e. the default case). OUTSIDE_RANGE_P says whether there - was a case value that doesn't fit into the range of the ORIG_TYPE. - Returns true if the case label is in range of ORIG_TYPE (saturated or - untouched) or false if the label is out of range. */ - -static bool -check_case_bounds (location_t loc, tree type, tree orig_type, - tree *case_low_p, tree *case_high_p, - bool *outside_range_p) -{ - tree min_value, max_value; - tree case_low = *case_low_p; - tree case_high = case_high_p ? *case_high_p : case_low; - - /* If there was a problem with the original type, do nothing. */ - if (orig_type == error_mark_node) - return true; - - min_value = TYPE_MIN_VALUE (orig_type); - max_value = TYPE_MAX_VALUE (orig_type); - - /* We'll really need integer constants here. */ - case_low = fold (case_low); - case_high = fold (case_high); - - /* Case label is less than minimum for type. */ - if (tree_int_cst_compare (case_low, min_value) < 0 - && tree_int_cst_compare (case_high, min_value) < 0) - { - warning_at (loc, 0, "case label value is less than minimum value " - "for type"); - *outside_range_p = true; - return false; - } - - /* Case value is greater than maximum for type. */ - if (tree_int_cst_compare (case_low, max_value) > 0 - && tree_int_cst_compare (case_high, max_value) > 0) - { - warning_at (loc, 0, "case label value exceeds maximum value for type"); - *outside_range_p = true; - return false; - } - - /* Saturate lower case label value to minimum. */ - if (tree_int_cst_compare (case_high, min_value) >= 0 - && tree_int_cst_compare (case_low, min_value) < 0) - { - warning_at (loc, 0, "lower value in case label range" - " less than minimum value for type"); - *outside_range_p = true; - case_low = min_value; - } - - /* Saturate upper case label value to maximum. */ - if (tree_int_cst_compare (case_low, max_value) <= 0 - && tree_int_cst_compare (case_high, max_value) > 0) - { - warning_at (loc, 0, "upper value in case label range" - " exceeds maximum value for type"); - *outside_range_p = true; - case_high = max_value; - } - - if (*case_low_p != case_low) - *case_low_p = convert (type, case_low); - if (case_high_p && *case_high_p != case_high) - *case_high_p = convert (type, case_high); - - return true; -} - /* Return an integer type with BITS bits of precision, that is unsigned if UNSIGNEDP is nonzero, otherwise signed. */ @@ -4873,13 +4791,12 @@ case_compare (splay_tree_key k1, splay_tree_key k2) usual C/C++ syntax, rather than the GNU case range extension. CASES is a tree containing all the case ranges processed so far; COND is the condition for the switch-statement itself. - OUTSIDE_RANGE_P says whether there was a case value that doesn't - fit into the range of the ORIG_TYPE. Returns the CASE_LABEL_EXPR - created, or ERROR_MARK_NODE if no CASE_LABEL_EXPR is created. */ + Returns the CASE_LABEL_EXPR created, or ERROR_MARK_NODE if no + CASE_LABEL_EXPR is created. */ tree -c_add_case_label (location_t loc, splay_tree cases, tree cond, tree orig_type, - tree low_value, tree high_value, bool *outside_range_p) +c_add_case_label (location_t loc, splay_tree cases, tree cond, + tree low_value, tree high_value) { tree type; tree label; @@ -4913,6 +4830,7 @@ c_add_case_label (location_t loc, splay_tree cases, tree cond, tree orig_type, { low_value = check_case_value (loc, low_value); low_value = convert_and_check (loc, type, low_value); + low_value = fold (low_value); if (low_value == error_mark_node) goto error_out; } @@ -4920,6 +4838,7 @@ c_add_case_label (location_t loc, splay_tree cases, tree cond, tree orig_type, { high_value = check_case_value (loc, high_value); high_value = convert_and_check (loc, type, high_value); + high_value = fold (high_value); if (high_value == error_mark_node) goto error_out; } @@ -4935,15 +4854,6 @@ c_add_case_label (location_t loc, splay_tree cases, tree cond, tree orig_type, warning_at (loc, 0, "empty range specified"); } - /* See if the case is in range of the type of the original testing - expression. If both low_value and high_value are out of range, - don't insert the case label and return NULL_TREE. */ - if (low_value - && !check_case_bounds (loc, type, orig_type, - &low_value, high_value ? &high_value : NULL, - outside_range_p)) - return NULL_TREE; - /* Look up the LOW_VALUE in the table of case labels we already have. */ node = splay_tree_lookup (cases, (splay_tree_key) low_value); diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h index 104c74226de..1cf2cae6395 100644 --- a/gcc/c-family/c-common.h +++ b/gcc/c-family/c-common.h @@ -988,8 +988,7 @@ extern tree boolean_increment (enum tree_code, tree); 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 tree c_add_case_label (location_t, splay_tree, tree, tree, tree); extern bool c_switch_covers_all_cases_p (splay_tree, tree); extern tree build_function_call (location_t, tree, tree); @@ -1291,8 +1290,7 @@ extern void sizeof_pointer_memaccess_warning (location_t *, tree, bool (*) (tree, tree)); extern void check_main_parameter_types (tree decl); extern void warnings_for_convert_and_check (location_t, tree, tree, tree); -extern void c_do_switch_warnings (splay_tree, location_t, tree, tree, bool, - bool); +extern void c_do_switch_warnings (splay_tree, location_t, tree, tree, bool); extern void warn_for_omitted_condop (location_t, tree); extern bool warn_for_restrict (unsigned, tree *, unsigned); extern void warn_for_address_or_pointer_of_packed_member (tree, tree); diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c index f6acc67af36..322cf98eb02 100644 --- a/gcc/c-family/c-warn.c +++ b/gcc/c-family/c-warn.c @@ -1428,12 +1428,87 @@ match_case_to_enum (splay_tree_node node, void *data) void c_do_switch_warnings (splay_tree cases, location_t switch_location, - tree type, tree cond, bool bool_cond_p, - bool outside_range_p) + tree type, tree cond, bool bool_cond_p) { splay_tree_node default_node; splay_tree_node node; tree chain; + bool outside_range_p = false; + + if (type != error_mark_node + && type != TREE_TYPE (cond) + && INTEGRAL_TYPE_P (type) + && INTEGRAL_TYPE_P (TREE_TYPE (cond)) + && (!tree_int_cst_equal (TYPE_MIN_VALUE (type), + TYPE_MIN_VALUE (TREE_TYPE (cond))) + || !tree_int_cst_equal (TYPE_MAX_VALUE (type), + TYPE_MAX_VALUE (TREE_TYPE (cond))))) + { + tree min_value = TYPE_MIN_VALUE (type); + tree max_value = TYPE_MAX_VALUE (type); + + node = splay_tree_predecessor (cases, (splay_tree_key) min_value); + if (node && node->key) + { + outside_range_p = true; + /* There is at least one case smaller than TYPE's minimum value. + NODE itself could be still a range overlapping the valid values, + but any predecessors thereof except the default case will be + completely outside of range. */ + if (CASE_HIGH ((tree) node->value) + && tree_int_cst_compare (CASE_HIGH ((tree) node->value), + min_value) >= 0) + { + location_t loc = EXPR_LOCATION ((tree) node->value); + warning_at (loc, 0, "lower value in case label range" + " less than minimum value for type"); + CASE_LOW ((tree) node->value) = convert (TREE_TYPE (cond), + min_value); + node->key = (splay_tree_key) CASE_LOW ((tree) node->value); + } + /* All the following ones are completely outside of range. */ + do + { + node = splay_tree_predecessor (cases, + (splay_tree_key) min_value); + if (node == NULL || !node->key) + break; + location_t loc = EXPR_LOCATION ((tree) node->value); + warning_at (loc, 0, "case label value is less than minimum " + "value for type"); + splay_tree_remove (cases, node->key); + } + while (1); + } + node = splay_tree_lookup (cases, (splay_tree_key) max_value); + if (node == NULL) + node = splay_tree_predecessor (cases, (splay_tree_key) max_value); + /* Handle a single node that might partially overlap the range. */ + if (node + && node->key + && CASE_HIGH ((tree) node->value) + && tree_int_cst_compare (CASE_HIGH ((tree) node->value), + max_value) > 0) + { + location_t loc = EXPR_LOCATION ((tree) node->value); + warning_at (loc, 0, "upper value in case label range" + " exceeds maximum value for type"); + CASE_HIGH ((tree) node->value) + = convert (TREE_TYPE (cond), max_value); + outside_range_p = true; + } + /* And any nodes that are completely outside of the range. */ + while ((node = splay_tree_successor (cases, + (splay_tree_key) max_value)) + != NULL) + { + location_t loc = EXPR_LOCATION ((tree) node->value); + warning_at (loc, 0, + "case label value exceeds maximum value for type"); + splay_tree_remove (cases, node->key); + outside_range_p = true; + } + } if (!warn_switch && !warn_switch_enum && !warn_switch_default && !warn_switch_bool) diff --git a/gcc/c/ChangeLog b/gcc/c/ChangeLog index 27660b98e7d..e5bc2b75303 100644 --- a/gcc/c/ChangeLog +++ b/gcc/c/ChangeLog @@ -1,5 +1,11 @@ 2019-04-19 Jakub Jelinek + PR c/89888 + * c-typeck.c (struct c_switch): Remove outside_range_p member. + (c_start_case): Don't clear it. + (do_case): Adjust c_add_case_label caller. + (c_finish_case): Adjust c_do_switch_warnings caller. + PR c++/90108 * c-decl.c (merge_decls): If remove is main variant and DECL_ORIGINAL_TYPE is some other type, remove a DECL_ORIGINAL_TYPE diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c index 2de4d0f85fd..8286b7d3329 100644 --- a/gcc/c/c-typeck.c +++ b/gcc/c/c-typeck.c @@ -10686,10 +10686,6 @@ struct c_switch { /* Remember whether the controlling expression had boolean type before integer promotions for the sake of -Wswitch-bool. */ bool bool_cond_p; - - /* Remember whether there was a case value that is outside the - range of the ORIG_TYPE. */ - bool outside_range_p; }; /* A stack of the currently active switch statements. The innermost @@ -10766,7 +10762,6 @@ c_start_case (location_t switch_loc, cs->cases = splay_tree_new (case_compare, NULL, NULL); cs->bindings = c_get_switch_bindings (); cs->bool_cond_p = bool_cond_p; - cs->outside_range_p = false; cs->next = c_switch_stack; c_switch_stack = cs; @@ -10812,9 +10807,7 @@ do_case (location_t loc, tree low_value, tree high_value) label = c_add_case_label (loc, c_switch_stack->cases, SWITCH_COND (c_switch_stack->switch_expr), - c_switch_stack->orig_type, - low_value, high_value, - &c_switch_stack->outside_range_p); + low_value, high_value); if (label == error_mark_node) label = NULL_TREE; return label; @@ -10835,8 +10828,7 @@ c_finish_case (tree body, tree type) switch_location = EXPR_LOCATION (cs->switch_expr); c_do_switch_warnings (cs->cases, switch_location, type ? type : TREE_TYPE (cs->switch_expr), - SWITCH_COND (cs->switch_expr), - cs->bool_cond_p, cs->outside_range_p); + SWITCH_COND (cs->switch_expr), cs->bool_cond_p); if (c_switch_covers_all_cases_p (cs->cases, TREE_TYPE (cs->switch_expr))) SWITCH_ALL_CASES_P (cs->switch_expr) = 1; diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index 9c1c49bda28..b35d8758730 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,5 +1,11 @@ 2019-04-19 Jakub Jelinek + PR c/89888 + * decl.c (struct cp_switch): Remove outside_range_p member. + (push_switch): Don't clear it. + (pop_switch): Adjust c_do_switch_warnings caller. + (finish_case_label): Adjust c_add_case_label caller. + PR c++/90108 * decl.c (duplicate_decls): If remove is main variant and DECL_ORIGINAL_TYPE is some other type, remove a DECL_ORIGINAL_TYPE diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 21b4cc6b83c..01c89cd572d 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -3495,9 +3495,6 @@ struct cp_switch label. We need a tree, rather than simply a hash table, because of the GNU case range extension. */ splay_tree cases; - /* Remember whether there was a case value that is outside the - range of the original type of the controlling expression. */ - bool outside_range_p; /* Remember whether a default: case label has been seen. */ bool has_default_p; /* Remember whether a BREAK_STMT has been seen in this SWITCH_STMT. */ @@ -3526,7 +3523,6 @@ push_switch (tree switch_stmt) p->next = switch_stack; p->switch_stmt = switch_stmt; p->cases = splay_tree_new (case_compare, NULL, NULL); - p->outside_range_p = false; p->has_default_p = false; p->break_stmt_seen_p = false; p->in_loop_body_p = false; @@ -3547,8 +3543,7 @@ pop_switch (void) if (!processing_template_decl) c_do_switch_warnings (cs->cases, switch_location, SWITCH_STMT_TYPE (cs->switch_stmt), - SWITCH_STMT_COND (cs->switch_stmt), - bool_cond_p, cs->outside_range_p); + SWITCH_STMT_COND (cs->switch_stmt), bool_cond_p); /* For the benefit of block_may_fallthru remember if the switch body case labels cover all possible values and if there are break; stmts. */ @@ -3663,9 +3658,7 @@ finish_case_label (location_t loc, tree low_value, tree high_value) low_value = case_conversion (type, low_value); high_value = case_conversion (type, high_value); - r = c_add_case_label (loc, switch_stack->cases, cond, type, - low_value, high_value, - &switch_stack->outside_range_p); + r = c_add_case_label (loc, switch_stack->cases, cond, low_value, high_value); /* After labels, make any new cleanups in the function go into their own new (temporary) binding contour. */ diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 15e1d14038c..a5e5af59f54 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,5 +1,10 @@ 2019-04-19 Jakub Jelinek + PR c/89888 + * c-c++-common/pr89888.c: New test. + * g++.dg/torture/pr40335.C: Change dg-bogus into dg-warning. + Don't expect -Wswitch-unreachable warning. + PR c++/90108 * c-c++-common/pr90108.c: New test. diff --git a/gcc/testsuite/c-c++-common/pr89888.c b/gcc/testsuite/c-c++-common/pr89888.c new file mode 100644 index 00000000000..d9e11d6f26a --- /dev/null +++ b/gcc/testsuite/c-c++-common/pr89888.c @@ -0,0 +1,67 @@ +/* PR c/89888 */ +/* { dg-do compile { target { int32 } } } */ +/* { dg-options "" } */ + +long long y; + +void +foo (unsigned char x) +{ + switch (x) + { + case -1: y = -1; break; /* { dg-message "previously used here" } */ + /* { dg-warning "case label value is less than minimum value for type" "" { target *-*-* } .-1 } */ + case 0xffffffff: y = 0xffffffff; break; /* { dg-error "duplicate case value" } */ + case ~0U: y = ~0U; break; /* { dg-error "duplicate case value" } */ + } +} + +void +bar (unsigned char x) +{ + switch (x) + { + case -1: y = -1; break; /* { dg-message "previously used here" } */ + /* { dg-warning "case label value is less than minimum value for type" "" { target *-*-* } .-1 } */ + case -1: y = -1; break; /* { dg-error "duplicate case value" } */ + case -1: y = -1; break; /* { dg-error "duplicate case value" } */ + } +} + +void +baz (unsigned char x) +{ + switch (x) + { + case -7: y = -7; break; /* { dg-warning "case label value is less than minimum value for type" } */ + case -5 ... 2: y = -5; break; /* { dg-warning "lower value in case label range less than minimum value for type" } */ + case 18: y = 18; break; + case (unsigned char) -2 ... 4 + (unsigned char) -2: y = 2; break; /* { dg-warning "upper value in case label range exceeds maximum value for type" } */ + case 24 + (unsigned char) -2: y = 3; break; /* { dg-warning "case label value exceeds maximum value for type" } */ + } +} + +void +qux (unsigned char x) +{ + switch (x) + { + case (unsigned char) -1 ... 1 + (unsigned char) -1: y = 2; break; /* { dg-warning "upper value in case label range exceeds maximum value for type" } */ + case -12: y = -7; break; /* { dg-warning "case label value is less than minimum value for type" } */ + case 18: y = 18; break; + case 27 + (unsigned char) -1: y = 3; break; /* { dg-warning "case label value exceeds maximum value for type" } */ + case -1 ... 0: y = -5; break; /* { dg-warning "lower value in case label range less than minimum value for type" } */ + } +} + +void +quux (unsigned char x) +{ + switch (x) + { + case (unsigned char) -2 ... (unsigned char) -1: y = 2; break; + case 18: y = 18; break; + case 1 + (unsigned char) -1: y = 3; break; /* { dg-warning "case label value exceeds maximum value for type" } */ + case -2 ... -1: y = -5; break; /* { dg-warning "case label value is less than minimum value for type" } */ + } +} diff --git a/gcc/testsuite/g++.dg/torture/pr40335.C b/gcc/testsuite/g++.dg/torture/pr40335.C index 295a356d3fa..bf335957e60 100644 --- a/gcc/testsuite/g++.dg/torture/pr40335.C +++ b/gcc/testsuite/g++.dg/torture/pr40335.C @@ -7,8 +7,8 @@ main (void) int i = -1; switch ((signed char) i) { - case 255: /* { dg-bogus "exceeds maximum value" "" { xfail *-*-* } } */ - abort (); /* { dg-warning "statement will never be executed" } */ + case 255: /* { dg-warning "exceeds maximum value" } */ + abort (); default: break; } -- 2.30.2