[Bug c/60439] New: No warning for case overflow in switch statement.

[Bug c/60439] New: No warning for case overflow in switch statement.

[chengniansun at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60439

            Bug ID: 60439
           Summary: No warning for case overflow in switch statement.
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: chengniansun at gmail dot com

For the switch statement in the function f2, the controlling expression is
either 1 or 0, and the first "case 1111" exceeds the max value. 


$: cat s.c
int f1(char c) {
  switch(c) {
  case 111111: return 1; /*warning here*/
  default: return 0;
  }
}
int f2(char c, char d) {
  switch(c && d) {
  case 1111: return 1; /*no warning here*/
  default: return 0;
  }
}
$: gcc-trunk -c s.c 
s.c: In function ‘f1’:
s.c:3:3: warning: case label value exceeds maximum value for type
   case 111111: return 1; /*warning here*/
   ^
>From gcc-bugs-return-445550-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Thu Mar 06 07:14:31 2014
Return-Path: <gcc-bugs-return-445550-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 16366 invoked by alias); 6 Mar 2014 07:14:31 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 16345 invoked by uid 48); 6 Mar 2014 07:14:28 -0000
From: "pinskia at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug c/60439] No warning for case overflow in switch statement.
Date: Thu, 06 Mar 2014 07:14:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: c
X-Bugzilla-Version: 4.9.0
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: pinskia at gcc dot gnu.org
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields:
Message-ID: <bug-60439-4-wooFcgLLaG@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-60439-4@http.gcc.gnu.org/bugzilla/>
References: <bug-60439-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2014-03/txt/msg00419.txt.bz2
Content-length: 190

http://gcc.gnu.org/bugzilla/show_bug.cgi?id`439

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Well in C, the type of a && is an int even though the range is 0...1 .

[Bug c/60439] No warning for case overflow in switch statement.

[mpolacek at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60439

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
                 CC|                            |mpolacek at gcc dot gnu.org
         Resolution|---                         |INVALID

--- Comment #2 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Right.  So I think there's nothing much to do.  Please reopen if you strongly
disagree.

[Bug c/60439] No warning for case overflow in switch statement.

[chengniansun at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60439

--- Comment #3 from Chengnian Sun <chengniansun at gmail dot com> ---
(In reply to Marek Polacek from comment #2)
> Right.  So I think there's nothing much to do.  Please reopen if you
> strongly disagree.

Hi Marek, 

>From the perspective of type system, I think no warning is fine. But from the
viewpoint of practice, IMHO this should be an overlooked case, as at runtime
there is indeed overflow. 

The reason that I filed this report is not only that Clang emits a warning on
this issue (shown as below), but also that Gcc is able to handle the value
range of expressions (if I under the report PR60036 correctly).

------------------------------------------------------------------- 

$: clang-trunk -Wswitch s.c  -c
s.c:3:8: warning: overflow converting case value to switch condition type
(111111 to 7) [-Wswitch]
  case 111111: return 1; /*warning here*/
       ^
s.c:8:3: warning: switch condition has boolean value
  switch(c && d) {
  ^      ~~~~~~

[Bug c/60439] No warning for case overflow in switch statement.

[chengniansun at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60439

--- Comment #4 from Chengnian Sun <chengniansun at gmail dot com> ---
(In reply to Chengnian Sun from comment #3)
> (In reply to Marek Polacek from comment #2)
> > Right.  So I think there's nothing much to do.  Please reopen if you
> > strongly disagree.
> 
> Hi Marek, 
> 
> From the perspective of type system, I think no warning is fine. But from
> the viewpoint of practice, IMHO this should be an overlooked case, as at
> runtime there is indeed overflow. 
> 
> The reason that I filed this report is not only that Clang emits a warning
> on this issue (shown as below), but also that Gcc is able to handle the
> value range of expressions (if I under the report PR60036 correctly).
> 
> ------------------------------------------------------------------- 
> 
> $: clang-trunk -Wswitch s.c  -c
> s.c:3:8: warning: overflow converting case value to switch condition type
> (111111 to 7) [-Wswitch]
>   case 111111: return 1; /*warning here*/
>        ^
> s.c:8:3: warning: switch condition has boolean value
>   switch(c && d) {
>   ^      ~~~~~~

Correction. There should be no overflow, but the case 1111 will never be
touched, right? In this case, I still think it is better to alert developers.

[Bug c/60439] No warning for case overflow in switch statement.

[mpolacek at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60439

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |NEW
   Last reconfirmed|                            |2014-03-13
         Resolution|INVALID                     |---
     Ever confirmed|0                           |1

--- Comment #5 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
OK, reopening, sorry.  The "switch condition has boolean value" warning might
make sense; I'll try to look into it after 4.9.

[Bug c/60439] No warning for case overflow in switch statement.

[chengniansun at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60439

--- Comment #6 from Chengnian Sun <chengniansun at gmail dot com> ---
(In reply to Marek Polacek from comment #5)
> OK, reopening, sorry.  The "switch condition has boolean value" warning
> might make sense; I'll try to look into it after 4.9.

Thanks for considering it, Marek.

[Bug c/60439] No warning for case overflow in switch statement.

[mpolacek at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60439

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |mpolacek at gcc dot gnu.org

--- Comment #7 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
I'm testing a patch for "switch condition has boolean value" warning.

[Bug c/60439] No warning for case overflow in switch statement.

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60439

--- Comment #8 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Author: mpolacek
Date: Tue Jun  3 17:35:34 2014
New Revision: 211194

URL: http://gcc.gnu.org/viewcvs?rev=211194&root=gcc&view=rev
Log:
    PR c/60439
    * doc/invoke.texi: Document -Wswitch-bool.
    * function.c (stack_protect_epilogue): Cast controlling expression of
    the switch to int.
    * gengtype.c (walk_type): Generate switch expression with its
    controlling expression cast to int.
c/
    * c-parser.c (c_parser_switch_statement): Pass explicit_cast_p to
    c_start_case.
    * c-tree.h (c_start_case): Update.
    * c-typeck.c (c_start_case): Add new boolean parameter.  Warn if
    switch condition has boolean value.
cp/
    * semantics.c (finish_switch_cond): Warn if switch condition has
    boolean value.
c-family/
    * c.opt (Wswitch-bool): New option.
testsuite/
    * c-c++-common/pr60439.c: New test.
    * g++.dg/eh/scope1.C (f4): Add dg-warning.

Added:
    trunk/gcc/testsuite/c-c++-common/pr60439.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c.opt
    trunk/gcc/c/ChangeLog
    trunk/gcc/c/c-parser.c
    trunk/gcc/c/c-tree.h
    trunk/gcc/c/c-typeck.c
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/semantics.c
    trunk/gcc/doc/invoke.texi
    trunk/gcc/function.c
    trunk/gcc/gengtype.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/g++.dg/eh/scope1.C

[Bug c/60439] No warning for case overflow in switch statement.

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60439

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #9 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Fixed.

[Bug c/60439] No warning for case overflow in switch statement.

[gary at intrepid dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60439

--- Comment #10 from Gary Funck <gary at intrepid dot com> ---
The following test case when compiled against a recent trunk revision (211365
2014-06-08) generates a warning, as intended by the patch described in comment
8.


int a, x;

int
main ()
{

  switch (!x)
    {
    case 0:
      a = 1;
      break;
    case 1:
      a = 2;
      break;
    }

  return 0;
}

s.c: In function ‘main’:
s.c:8:11: warning: switch condition has boolean value [-Wswitch-bool]
   switch (!x)
           ^

However, -Wno-switch-bool does *not* suppress the warning.

Looking at gcc/c-family/c.opt, it has an Init(1) clause but no Var() clause. 
It seems that if an Init() clause is present that a Var() clause must also be
present for this option to work as expected.

This patch fixes the issue.

Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt  (revision 211365)
+++ gcc/c-family/c.opt  (working copy)
@@ -539,7 +539,7 @@ C ObjC C++ ObjC++ Var(warn_switch_enum)
 Warn about all enumerated switches missing a specific case

 Wswitch-bool
-C ObjC C++ ObjC++ Warning Init(1)
+C ObjC C++ ObjC++ Var(warn_switch_bool) Warning Init(1)
 Warn about switches with boolean controlling expression

 Wmissing-format-attribute

However, I'm not so certain that this option should be enabled by default, for
a few reasons:

1) The test case above shows the use of a boolean value used in the case
expression where both alternatives (0 and 1) are accounted for and no other
(overflow) cases are mentioned.  Adding a cast to (int) will not clarify the
code at all and in fact then leaves apparent cases unaccounted for, which might
arguably lead to a warning to that effect on some compilers (present or
future).

2) If the compiler performed control flow analysis and range analysis to
determine that some cases are not accounted for or that some cases are
out-of-range, then enabling by default would seem appropriate.  In the test
case above (in my opinion) no warning should be issued because both cases are
accounted for.

3) Perhaps this option should only be enabled explicitly or by -Wall.  For
example, -Wswitch is enabled by -Wall.

Wswitch
C ObjC C++ ObjC++ Var(warn_switch) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall
)
Warn about enumerated switches, with no default, missing a case
>From gcc-bugs-return-453707-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Wed Jun 11 17:26:20 2014
Return-Path: <gcc-bugs-return-453707-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 17890 invoked by alias); 11 Jun 2014 17:26:20 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 17856 invoked by uid 48); 11 Jun 2014 17:26:16 -0000
From: "pinskia at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug target/61479] wrong code gen with fstack-protector-all for variadic function
Date: Wed, 11 Jun 2014 17:26:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: target
X-Bugzilla-Version: 4.10.0
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: pinskia at gcc dot gnu.org
X-Bugzilla-Status: RESOLVED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields: bug_status resolution
Message-ID: <bug-61479-4-hs4DxO6pzf@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-61479-4@http.gcc.gnu.org/bugzilla/>
References: <bug-61479-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2014-06/txt/msg00789.txt.bz2
Content-length: 541

https://gcc.gnu.org/bugzilla/show_bug.cgi?ida479

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
This code is not just non-portable but it is undefined in c as you accessing
outside of the bounds of the "array" of a.

[Bug c/60439] No warning for case overflow in switch statement.

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60439

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |ASSIGNED
         Resolution|FIXED                       |---
   Target Milestone|---                         |4.10.0

--- Comment #11 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Ah, sorry about that.  I wonder where the Var(warn_switch_bool) disappeared.
In the particular testcase you posted I agree we shouldn't warn, but I think we
should warn in case there is the default label.  I have no problem with moving
the warning to -Wall.  I'll prepare a patch tomorrow.

[Bug c/60439] No warning for case overflow in switch statement.

[gary at intrepid dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60439

--- Comment #12 from Gary Funck <gary at intrepid dot com> ---
I submitted Bug #61480 documenting the interaction between Var() and Init().

The test case that I posted is abstracted from actual code.  As far as which
switches should be default and/or enabled by -Wall, that question probably
needs further review.

[Bug c/60439] No warning for case overflow in switch statement.

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60439

--- Comment #13 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Author: mpolacek
Date: Mon Jun 16 12:38:07 2014
New Revision: 211707

URL: https://gcc.gnu.org/viewcvs?rev=211707&root=gcc&view=rev
Log:
    PR c/60439
    * c.opt (Wswitch-bool): Add Var.

Modified:
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c.opt