Bug 89888 - [8 Regression] When switch controlling expression is promoted from type narrower than int, GCC does not diagnose identical cases
Summary: [8 Regression] When switch controlling expression is promoted from type narro...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 8.3.0
: P2 normal
Target Milestone: 9.0
Assignee: Jakub Jelinek
URL:
Keywords: accepts-invalid, diagnostic
Depends on:
Blocks:
 
Reported: 2019-03-29 16:24 UTC by Pascal Cuoq
Modified: 2021-05-14 11:56 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2019-04-01 00:00:00


Attachments
gcc9-pr89888.patch (4.54 KB, patch)
2019-04-08 18:37 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pascal Cuoq 2019-03-29 16:24:47 UTC
Consider the C function:

long long X;

void f(unsigned char x)
{
  switch(x) {
    case -1: X=-1; break;
    case 0xffffffff: X=0xffffffff; break;
  }
}

The controlling expression of the switch, x, has type unsigned char and is promoted to int before its type being used as reference for the constants -1 and 0xffffffff. This is according to C11 6.8.4.2:5 (https://port70.net/~nsz/c/c11/n1570.html#6.8.4.2p5 )

GCC 8.3 emits very good warnings about each of the constants being, after conversion, outside the range of an unsigned int and thus unreachable:

<source>: In function 'f':
<source>:6:5: warning: case label value is less than minimum value for type
     case -1: X=-1; break;
     ^~~~
<source>:7:5: warning: case label value is less than minimum value for type
     case 0xffffffff: X=0xffffffff; break;
     ^~~~

(Compiler Explorer link: https://gcc.godbolt.org/z/gvnvoa )

However, GCC does not warn about the labels being identical after conversion. I feel silly reporting this, because it only happens for discarded labels that were unreachable, and there isn't any ambiguity about the meaning of the program. Still, the C11 clause 6.8.4.2:3 about identical switch case labels (after conversion) (https://port70.net/~nsz/c/c11/n1570.html#6.8.4.2p3 ) is under a “Constraints” heading, so depending how much GCC cares about adhering to the letter of the standard, it may want to diagnose this situation.

Clang diagnoses this situation and emits an “error”:

<source>:7:10: error: duplicate case value '-1'

Clang also emits two misleading warnings about the constants -1 and 0xffffffff. The wording of these warnings is so misleading that it can be considered a Clang bug, which has been reported in the appropriate place.
Comment 1 Pascal Cuoq 2019-03-29 16:31:19 UTC
errata: “outside the range of an unsigned char”
Comment 2 Richard Biener 2019-04-01 11:29:02 UTC
Confirmed.  When making the promotion explicit (switch ((int)x))) we reject the testcase:

> gcc t.c -Wall -S
t.c: In function ‘f’:
t.c:7:5: error: duplicate case value
     case 0xffffffff: X=0xffffffff; break;
     ^
t.c:6:5: error: previously used here
     case -1: X=-1; break;
     ^

and GCC 8 warns:

> gcc-8 t.c -Wall -S
t.c: In function ‘f’:
t.c:6:5: warning: case label value is less than minimum value for type
     case -1: X=-1; break;
     ^~~~
t.c:7:5: warning: case label value is less than minimum value for type
     case 0xffffffff: X=0xffffffff; break;
     ^~~~
t.c:6:15: warning: statement will never be executed [-Wswitch-unreachable]
     case -1: X=-1; break;
              ~^~~
Comment 3 Joseph S. Myers 2019-04-01 21:31:47 UTC
This is a regression introduced in 4.0 relative to 3.4.
Comment 4 Jakub Jelinek 2019-04-08 14:51:27 UTC
Guess this started with r84947 aka https://gcc.gnu.org/ml/gcc-patches/2004-07/msg01859.html
So, if we want to error here, we should move what check_case_bounds does (except for the fold + convert) from c_add_case_labels to start of c_do_switch_warnings.
Comment 5 Jakub Jelinek 2019-04-08 18:37:25 UTC
Created attachment 46100 [details]
gcc9-pr89888.patch

Untested fix.
Comment 6 Jakub Jelinek 2019-04-19 11:56:39 UTC
Author: jakub
Date: Fri Apr 19 11:56:07 2019
New Revision: 270455

URL: https://gcc.gnu.org/viewcvs?rev=270455&root=gcc&view=rev
Log:
	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.

Added:
    trunk/gcc/testsuite/c-c++-common/pr89888.c
Modified:
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c-common.c
    trunk/gcc/c-family/c-common.h
    trunk/gcc/c-family/c-warn.c
    trunk/gcc/c/ChangeLog
    trunk/gcc/c/c-typeck.c
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/decl.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/g++.dg/torture/pr40335.C
Comment 7 Jakub Jelinek 2019-04-19 12:07:43 UTC
Fixed for 9.1 (so far).
Comment 8 Richard Biener 2019-11-14 07:53:50 UTC
The GCC 7 branch is being closed, re-targeting to GCC 8.4.
Comment 9 Jakub Jelinek 2020-03-04 09:35:09 UTC
GCC 8.4.0 has been released, adjusting target milestone.
Comment 10 Jakub Jelinek 2021-05-14 11:56:16 UTC
The GCC 8 branch is being closed, fixed in GCC 9.1.