Bug 20681 - [4.6 Regression] wrong "control reaches" warning with switches
Summary: [4.6 Regression] wrong "control reaches" warning with switches
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.0.0
: P4 minor
Target Milestone: 4.7.0
Assignee: Jason Merrill
URL:
Keywords: diagnostic
: 25390 31783 38552 45497 47864 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-03-29 09:55 UTC by Oliver Stoeneberg
Modified: 2021-08-10 01:03 UTC (History)
12 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.7.0
Known to fail: 4.0.4
Last reconfirmed: 2005-06-26 17:11:12


Attachments
preprocessed source (23.35 KB, application/octet-stream)
2005-03-29 09:56 UTC, Oliver Stoeneberg
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Stoeneberg 2005-03-29 09:55:04 UTC
The following code causes a wrong warning (I couldn't reduce it to less):

#include <set>

using namespace std;

struct CExtendedEmailInfo {
	   bool _s;
};

struct CMetaBitField {
	   int _type;
	   int GetType() const{
	   	   return _type;
	   }
	   
	   bool operator < (const CMetaBitField&) const {
	   		return true;
	   }
};

int GetMetaCombination (CExtendedEmailInfo& info)
{
     try
     {
          if (info._s)
          {
               set<CMetaBitField> _bitfields;

               set<CMetaBitField>::iterator bi;
               if (bi != _bitfields.end())
               {
                    const CMetaBitField& bf = *bi;
                    switch (bf.GetType())
                    {
                    case 0:
                         {
                              return 17;
                         }
                         break;

                    case 1:
                         {
                              return 18;
                         }
                         break;

                    default:
                         // really shouldn't happen, but just in case...
                         return 0;
                    }
                    
               }
               else
               {
                    return 0;
               }
          }
          else
          {
               return 0;
          }
     }
     catch (...)
     {
          return -1;
     }
}

 C:\Dev-Cpp\Projects\test-stlport\main_6.cpp In function 'int
GetMetaCombination(CExtendedEmailInfo&)': 
66 C:\Dev-Cpp\Projects\test-stlport\main_6.cpp [Warning] control reaches end of
non-void function 

I am using:

Using built-in specs.
Target: i686-pc-mingw32
Configured with: /datal/gcc/gcc/configure --prefix=/datal/gcc/build/wingcc
--build=i686-pc-linux-gnu --host=i686-pc-mingw32 --target=i686-pc-mingw32
--enable-languages=c,c++,java --with-gcc --with-gnu-as --with-gnu-ld
--enable-threads=win32 --disable-nls --disable-win32-registry --disable-shared
--disable-debug --without-newlib --enable-libgcj --disable-java-awt --without-x
--enable-java-gc=boehm --disable-libgcj-debug --enable-interpreter
--enable-hash-synchronization --enable-sjlj-exceptions --enable-libgcj-multifile
--enable-libgcj-mingw-osapi=ansi
Thread model: win32
gcc version 4.0.0 20050324 (prerelease)
Comment 1 Oliver Stoeneberg 2005-03-29 09:56:25 UTC
Created attachment 8482 [details]
preprocessed source
Comment 2 Andrew Pinski 2005-03-29 20:13:15 UTC
Reduced testcase:
struct a{~a();a();};
int GetMetaCombination (int a2)
{
  a bi;
  switch (a2)
  {
    case 1:
      return 18;
      break;//removing this works around the warning
    default:
      return 0;
  }
}
Comment 3 Andrew Pinski 2005-04-05 01:33:39 UTC
This is also a rejects valid with -Werror.
Comment 4 Mark Mitchell 2005-04-08 21:46:15 UTC
Removing rejects-valid; treating all incorrect warnings as rejects-valid due to
-Werror is not useful.
Comment 5 fsm 2005-04-09 16:58:34 UTC
*** Bug 20918 has been marked as a duplicate of this bug. ***
Comment 6 Andrew Pinski 2005-08-03 00:37:51 UTC
Hmm, seems like anything after a "return" should be removed even before CFG was created:

        return D.1766;
        goto <D1765>;

I think I might get around to implementing that but it might take me some time.
Comment 7 Andrew Pinski 2005-08-17 15:55:51 UTC
This is a dup of bug 20624, see comment #13.  I wonder why useless  did not remove the goto.

*** This bug has been marked as a duplicate of 20624 ***
Comment 8 Andrew Pinski 2005-08-17 22:33:21 UTC
Reopening as I have a fix for PR 20624 but this one is harder, there might be others too.
Comment 9 Mark Mitchell 2005-10-17 00:41:04 UTC
Why is this marked as a C++ front-end bug?  Isn't this a middle-end problem?
Comment 10 Andrew Pinski 2005-10-17 00:45:35 UTC
Subject: Re:  [4.0/4.1 Regression] wrong "control reaches" warning with switches

> 
> 
> 
> ------- Comment #9 from mmitchel at gcc dot gnu dot org  2005-10-17 00:41 -------
> Why is this marked as a C++ front-end bug?  Isn't this a middle-end problem?

Because there is a "work around" in the C front-end, that could be copied
into the C++ front-end.

-- Pinski
Comment 11 Ian Lance Taylor 2005-10-17 04:39:25 UTC
For the record, this is the work-around in the C frontend:
    http://gcc.gnu.org/ml/gcc-patches/2005-01/msg01452.html
A corresponding patch in the C++ frontend would be more complicated, in order to continue to emit the error "break statement not within loop or switch" when required.

This class of bugs is a pain to work with at present.  The warning just follows the CFG, which makes sense.  But code like
  case 1:
    return;
    break;
builds a CFG which has a return followed by a goto.  remove_useless_stmts is currently not clever enough to remove code after a return, so the goto gets incorporated into the CFG.  Note that we don't get the warning when optimizing; we only get it with -O0.  And indeed when not optimizing the assembly code has the code path which returns an uninitialized value, although of course it never gets executed in practice.

Fixing this in the middle-end will require a version of remove_useless_stmts which is clever enough to efficiently discard statements which follow a return statement.  Andrew was working on that at one point; I don't recall what the status is.
Comment 12 Andrew Pinski 2005-10-17 04:46:43 UTC
Subject: Re:  [4.0/4.1 Regression] wrong "control reaches" warning with switches


On Oct 17, 2005, at 12:39 AM, ian at airs dot com wrote:

> Fixing this in the middle-end will require a version of 
> remove_useless_stmts
> which is clever enough to efficiently discard statements which follow 
> a return
> statement.  Andrew was working on that at one point; I don't recall 
> what the
> status is.

The status is that it will only be approved for 4.2 (this was decided
by RTH).  The problem is also comes into play with extra unreachable
labels which was not going to be fixed by my patch and is what
really causes this bug, rather than the stuff after a goto/return.
We lower the eh before removing the unreachable label which causes
us to think that there is a way to get fallthrough the switch.  This 
means
that we need a switch table for cleanup of the variable (the call to 
the dtor).

-- Pinski

Comment 13 Mark Mitchell 2005-10-31 02:55:21 UTC
Given the kind of solutions we're looking at, I can't imagine this being fixed for 4.0, and probably not even for 4.1, so I've set this to P4.

However, it seems sad to me that we can't find some efficient way to skip statements after a return at -O0.  If we really can't do that, then we ought to think hard about whether or not we should be emitting this warning at -O0, given that we want to do this warning via the optimization framework.
Comment 14 Andrew Pinski 2005-12-13 15:02:40 UTC
*** Bug 25390 has been marked as a duplicate of this bug. ***
Comment 15 Pawel Sikora 2006-07-03 15:27:26 UTC
one more valid code rejected by 4.1/4.2:

typedef enum { foo, bar } e;
int zoo( e __e )
{
        switch ( __e )
        {
                case foo: return -1;
                case bar: return +1;
        }
}

$ x86_64-gnu-linux-g++ bug.cpp -c -Wall -O2
bug.cpp: In function 'int zoo(e)':
bug.cpp:9: warning: control reaches end of non-void function
Comment 16 Pawel Sikora 2006-07-03 15:30:10 UTC
(In reply to comment #15)
> one more valid code rejected by 4.1/4.2:
> 
> typedef enum { foo, bar } e;
> int zoo( e __e )
> {
>         switch ( __e )
>         {
>                 case foo: return -1;
>                 case bar: return +1;
>         }
> }
> 
> $ x86_64-gnu-linux-g++ bug.cpp -c -Wall -O2
> bug.cpp: In function 'int zoo(e)':
> bug.cpp:9: warning: control reaches end of non-void function
> 

ohh, 3.3.6 also fails.
Comment 17 Andrew Pinski 2006-07-03 15:34:09 UTC
(In reply to comment #16)
> ohh, 3.3.6 also fails.

That is a different issue and really should be filed in a different bug.  The issue there is C++'s enums are only defined for those two values.
Comment 18 Pawel Sikora 2006-07-03 16:55:16 UTC
(In reply to comment #17)
> (In reply to comment #16)
> > ohh, 3.3.6 also fails.
> 
> That is a different issue and really should be filed in a different bug.  The
> issue there is C++'s enums are only defined for those two values.

reported in PR28236
Comment 19 Gabriel Dos Reis 2007-01-18 03:41:40 UTC
won't fix for GCC-4.0.x
Comment 20 Andrew Pinski 2007-05-02 07:20:06 UTC
*** Bug 31783 has been marked as a duplicate of this bug. ***
Comment 21 Joseph S. Myers 2008-07-04 16:51:19 UTC
Closing 4.1 branch.
Comment 22 Karol Szkudlarek 2008-11-19 15:13:46 UTC
(In reply to comment #2)
> Reduced testcase:
> struct a{~a();a();};
> int GetMetaCombination (int a2)
> {
>   a bi;
>   switch (a2)
>   {
>     case 1:
>       return 18;
>       break;//removing this works around the warning
>     default:
>       return 0;
>   }
> }

This also fails in gcc-4.2 (GCC) 4.2.4
Comment 23 Andrew Pinski 2008-12-22 02:36:24 UTC
*** Bug 38552 has been marked as a duplicate of this bug. ***
Comment 24 Joseph S. Myers 2009-03-31 18:43:43 UTC
Closing 4.2 branch.
Comment 25 Richard Biener 2009-08-04 12:26:19 UTC
GCC 4.3.4 is being released, adjusting target milestone.
Comment 26 Richard Biener 2010-05-22 18:10:19 UTC
GCC 4.3.5 is being released, adjusting target milestone.
Comment 27 Andrew Pinski 2010-09-03 04:53:38 UTC
*** Bug 45497 has been marked as a duplicate of this bug. ***
Comment 28 Andrew Pinski 2011-02-23 22:08:42 UTC
*** Bug 47864 has been marked as a duplicate of this bug. ***
Comment 29 Ami Fischman 2011-02-23 23:35:45 UTC
Further reduced test case from dup bug 47864:
$ nl t.cc ; g++ -finstrument-functions -Wreturn-type -Werror -c t.cc
     1  int foo(int type) {
     2    switch(type) {
     3      case 0: return 1; break;
     4      default: return 0;
     5    }
     6  }
cc1plus: warnings being treated as errors
t.cc: In function ‘int foo(int)’:
t.cc:6: error: control reaches end of non-void function

(in 4.4.3)
Comment 30 Richard Biener 2011-06-27 12:13:05 UTC
4.3 branch is being closed, moving to 4.4.7 target.
Comment 31 Dmitry G. Dyachenko 2011-11-15 14:10:33 UTC
gcc -Wreturn-type -c r.c
r.c: In function 'bar':
r.c:8:1: warning: control reaches end of non-void function [-Wreturn-type]

enum foo { e_1 };
int bar(enum foo x)
{
    switch(x) {
    case e_1:
	return 0;
    }
}

gcc version 4.7.0 20111115 (experimental) [trunk revision 181378] (GCC)
Comment 32 Jonathan Wakely 2011-11-15 14:53:13 UTC
(In reply to comment #31)
> gcc -Wreturn-type -c r.c
> r.c: In function 'bar':
> r.c:8:1: warning: control reaches end of non-void function [-Wreturn-type]
> 
> enum foo { e_1 };
> int bar(enum foo x)
> {
>     switch(x) {
>     case e_1:
>     return 0;
>     }
> }


Not a bug, enum foo can have other values except e_1.

What happens if you call bar( (foo)1 ) ?
Comment 33 Dmitry G. Dyachenko 2011-11-16 09:04:40 UTC
> Not a bug, enum foo can have other values except e_1.
> 
> What happens if you call bar( (foo)1 ) ?

sorry for noise :(
You are right - no error in c#31
Comment 34 Jason Merrill 2012-01-13 20:06:25 UTC
Author: jason
Date: Fri Jan 13 20:06:16 2012
New Revision: 183161

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=183161
Log:
	PR c++/20681
	* semantics.c (finish_break_stmt): Avoid adding an unreachable
	BREAK_STMT.

Added:
    trunk/gcc/testsuite/g++.dg/warn/Wreturn-type-7.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/semantics.c
    trunk/gcc/testsuite/ChangeLog
Comment 35 Jason Merrill 2012-01-13 20:17:50 UTC
Fixed for 4.7.  Could backport if there's interest.
Comment 36 Jakub Jelinek 2012-03-13 12:45:56 UTC
4.4 branch is being closed, moving to 4.5.4 target.
Comment 37 Paolo Carlini 2012-10-15 23:53:18 UTC
Closing as fixed.