Bug 31246 - -Wunreachable-code warnings for compiler-generated code
Summary: -Wunreachable-code warnings for compiler-generated code
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.3.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL: http://gcc.gnu.org/ml/gcc-patches/200...
Keywords: diagnostic, patch
Depends on:
Blocks: 36833
  Show dependency treegraph
 
Reported: 2007-03-17 16:38 UTC by Sylvain Pion
Modified: 2009-07-07 22:20 UTC (History)
6 users (show)

See Also:
Host: i686-pc-linux-gnu
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2007-03-19 23:13:32


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sylvain Pion 2007-03-17 16:38:35 UTC
Compiling the following with "-Wunreachable-code -D_GLIBCXX_DEBUG"
--------------------------
#include <vector>

int main()
{
  std::vector<int>::iterator a;
}
--------------------------

produces the warning :

--------------------------
/usr/lib/gcc/i386-redhat-linux/4.1.1/../../../../include/c++/4.1.1/debug/safe_iterator.h: In constructor ‘__gnu_debug::_Safe_iterator<_Iterator, _Sequence>::_Safe_iterator() [with _Iterator = __gnu_cxx::__normal_iterator<int*, __gnu_norm::vector<int, std::allocator<int> > >, _Sequence = __gnu_debug_def::vector<int, std::allocator<int> >]’:
/usr/lib/gcc/i386-redhat-linux/4.1.1/../../../../include/c++/4.1.1/debug/safe_iterator.h:103: warning: will never be executed
--------------------------

I tried g++ 4.1, 4.2 and 4.3.  They all produce this annoying warning.

Looking at the code in safe_iterator.h shows nothing obviously wrong.
This might even suggest a bug in -Wunreachable-code, although I categorized
it for now in libstdc++, as I have not been able to isolate a small piece
of code that reproduces the problem.


Some other compilers tend to have the equivalent of -Wunreachable-code
by default.  It would be nice if it worked well, and if it was enabled
by -Wall or -Wextra.
Comment 1 Andrew Pinski 2007-03-17 16:58:01 UTC
Lets look at the IR:
;; Function __gnu_debug::_Safe_iterator<_Iterator, _Sequence>::_Safe_iterator() [with _Iterator = __gnu_cxx::__normal_iterator<int*, __gnu_norm::vector<int, std::allocator<int> > >, _Sequence = __gnu_debug_def::vector<int, std::allocator<int> >] (_ZN11__gnu_debug14_Safe_iteratorIN9__gnu_cxx17__normal_iteratorIPiN10__gnu_norm6vectorIiSaIiEEEEEN15__gnu_debug_def6vectorIiS6_EEEC4Ev *INTERNAL* )
;; enabled by -tree-original

{
  <<cleanup_point <<< Unknown tree: expr_stmt
  __base_ctor  (&((struct _Safe_iterator<__gnu_cxx::__normal_iterator<int*, __gnu_norm::vector<int, std::allocator<int> > >,__gnu_debug_def::vector<int, std::allocator<int> > > *) this)->D.12082) >>>
>>;
  try
    {
      <<cleanup_point <<< Unknown tree: expr_stmt
  __comp_ctor  (&((struct _Safe_iterator<__gnu_cxx::__normal_iterator<int*, __gnu_norm::vector<int, std::allocator<int> > >,__gnu_debug_def::vector<int, std::allocator<int> > > *) this)->_M_current) >>>
>>;
    }
  catch
    {
      __base_dtor  ((struct _Safe_iterator_base *) this);
    }
}


and the constructor for ->M_current:
__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, __gnu_norm::vector<int, std::allocator<int> > >, __gnu_debug_def::vector<int, std::allocator<int> > >::~_Safe_iterator() (this, __in_chrg)
{
  struct _Safe_iterator_base * this.102;

  [/usr/include/c++/4.0.0/debug/safe_iterator.h : 65] {
    [/usr/include/c++/4.0.0/debug/safe_iterator.h : 65] try
      {

      }
    finally
      {
        [/usr/include/c++/4.0.0/debug/safe_iterator.h : 65] this.102 = (struct _Safe_iterator_base *) this;
        [/usr/include/c++/4.0.0/debug/safe_iterator.h : 65] __base_dtor  (this.102);
      }
  }
  [/usr/include/c++/4.0.0/debug/safe_iterator.h : 65] <D12095>:;
}


So the constructor for that cannot throw so the catch part is removed in both cases.

The warning is not strange and is correct in that the __base_dtor cannot be called ever.
Comment 2 Sylvain Pion 2007-03-17 17:26:48 UTC
Trying further, I noticed that simply default constructing an std::vector<int>,
even without the debug mode _GLIBCXX_DEBUG, generates also a bunch (14 !) of
these warnings.

Andrew, I did not follow all the details of your explanation.  I don't
understand if there is finally a problem with the libstdc++ code or not.
And if there is, what should be done.

Is this a warning meant to be useful at all, or meant for GCC developers?

How do we get rid of this warning for the simplest C++ code I showed?


Other compilers are able to warn for unreachable code in better conditions.
for example, they warn for "int f() { return 1 ; return -1; }", and I
was trying to see if GCC was able to generate the same kind of warnings.
I only found -Wunreachable-code, but so far it proves unusable for C++.
Comment 3 Chris Jefferson 2007-03-17 17:46:43 UTC
This is a general problem I've come across with templates. You commonly want to try/catch around some operation, but when you have templated on a simple type, like int, then it's easy to find out that no throwing can ever occur, and for this type the try/catch is a waste of time.

I'm not sure what the best way of fixing this is.
Comment 4 Chris Jefferson 2007-03-17 18:07:18 UTC
Here is a minimal code example:

#include <new>

int* get_ptr(void* ptr)
{
  return new(ptr) int();
}

Which gives (on my computer)

/usr/include/c++/4.0.0/new: In function 'int* get_ptr(void*)':
/usr/include/c++/4.0.0/new:94: warning: will never be executed

But a similar problem exists in much of the templated code, and in general.

The real problem I think here is that the system_header pragma is still broken for templates. If that was fixed, this warning would go away.
Comment 5 Paolo Carlini 2007-03-17 20:40:54 UTC
Note, however, that, as far as I can see, such try/catch are *not* in the library code proper, but *all* synthesized by the C++ front-end. Is it so weird to imagine for the C++ front-end to automatically suppress warning in such case? I think this is a very general issue, which goes even beyond the already general issue about pragma system headers: if the user writes such code and sees such kind of warnings becomes *completely* confused. Gaby, do you have an opinion?
Comment 6 Paolo Carlini 2007-03-17 20:48:23 UTC
And I'm recategorizing as C++: either (as I strongly believe) a very general diagnostic issue, or maybe a duplicate of C++/30500: certainly there is nothing we can do on the library side to "fix" the implementation of placement new itself (per Chris' comment #4. By the way, in that specific case I think we are missing the pragma from the header, because there are no templates involved and should work. But that's not the point).
Comment 7 gdr@cs.tamu.edu 2007-03-17 23:35:28 UTC
Subject: Re:  Strange -Wunreachable-code warning with _GLIBCXX_DEBUG

"pcarlini at suse dot de" <gcc-bugzilla@gcc.gnu.org> writes:

| Note, however, that, as far as I can see, such try/catch are *not* in the
| library code proper, but *all* synthesized by the C++ front-end. Is it so weird
| to imagine for the C++ front-end to automatically suppress warning in such
| case? I think this is a very general issue, which goes even beyond the already
| general issue about pragma system headers: if the user writes such code and
| sees such kind of warnings becomes *completely* confused. Gaby, do you have an
| opinion?

I agree with you Paolo.  The front-end should make sure that its
artefacts don't adversily affect diagnostics we emit.

-- Gaby
Comment 8 Andrew Pinski 2007-03-18 05:38:25 UTC
>I agree with you Paolo.  The front-end should make sure that its
> artefacts don't adversily affect diagnostics we emit.

I agree to some extend.  The reason why the try/catch is there is because of what the C++ standard says should happen and not really an artafact of what the GCC is doing really.  When I first say this bug I was going to say this should not warned about, but when I looked into it a little more, I was thinking the warning is correct except for the fact, there is no way of working around the issue.

I think we need to decide what -Wunreachable-code actually means, does it mean if there is a way to "fix" the code, then warn about unreachable code or does it mean to warn about code which is even hard to work around like in templates and constructors?
Comment 9 Manuel López-Ibáñez 2007-03-19 13:56:23 UTC
(In reply to comment #8)
> >I agree with you Paolo.  The front-end should make sure that its
> > artefacts don't adversily affect diagnostics we emit.
> 
> I agree to some extend.  The reason why the try/catch is there is because of
> what the C++ standard says should happen and not really an artafact of what the
> GCC is doing really.  When I first say this bug I was going to say this should
> not warned about, but when I looked into it a little more, I was thinking the
> warning is correct except for the fact, there is no way of working around the
> issue.
> 
> I think we need to decide what -Wunreachable-code actually means, does it mean
> if there is a way to "fix" the code, then warn about unreachable code or does
> it mean to warn about code which is even hard to work around like in templates
> and constructors?
> 

And I think that we should not warn about generated code. No matter if it is generated by optimisers or front-ends. If it is not user code, then there should be no warning. In that sense, this is similar to PR31227.

Comment 10 gdr@cs.tamu.edu 2007-03-19 15:19:26 UTC
Subject: Re:  Strange -Wunreachable-code warning with _GLIBCXX_DEBUG

"manu at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:

| And I think that we should not warn about generated code. No matter if it is
| generated by optimisers or front-ends. If it is not user code, then there
| should be no warning.

I fully agree.

-- Gaby
Comment 11 Andrew Pinski 2007-03-19 22:31:01 UTC
(In reply to comment #10)
> 
> I fully agree.

I am not agreeing fully,  This warning is only because we can prove something is pure/const/cannot throw and that only comes because of simple optimization.  What about this case:

int f(int a) { return a;}

int g(int b)
{
  try
  {
   return f(b);
  }catch (...) { return 0; }
}

Should we warn about the catch being unreachable?
This is the same issue as -Wuninitialized warning in that we warn about a lot of cases where we should not.  I think this is why this option is not turned on via either -W or -Wall, it is hard sometimes to work around.

Take even more extrem example where templates come into play:


int f(int a) { return a;}
int f(float a);

template <typename a>
int g(a b)
{
  try
  {
   return f(a);
  }catch (...) { return 0; }
}

int d = g<int>(10);

Should we warn that the catch case is unreachable, I think so as it is obvious but how do we avoid it, well you can specialize the template but that could get messy.
Comment 12 gdr@cs.tamu.edu 2007-03-19 22:45:24 UTC
Subject: Re:  Strange -Wunreachable-code warning with _GLIBCXX_DEBUG

"pinskia at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:

| (In reply to comment #10)
| > 
| > I fully agree.
| 
| I am not agreeing fully,

Well, you've got a problem.

[...]

| What about this case:

There is a distinction betwen user code and compiler-generated codes.
Warning about compiler-generated codes is pointless.

-- Gaby
Comment 13 Manuel López-Ibáñez 2007-03-19 23:13:32 UTC
Andrew, as you say, -Wunreachable-code is not enabled by -Wall. The user has to give it explicitly. And in your testcases the code is not reachable. So in that case, it could be argued whether the warning is warranted or not. So, yes, you got a point. But... that is not what this PR is about.

This PR is that the user cannot see the code we are warning about. Even if it were code with an easy workaround unlike your testcases, that doesn't matter at all, we should not emit the warning.

Comment 14 Andrew Pinski 2007-03-19 23:18:54 UTC
But the user can see the code, it is what is produced by what the C++ standard says is produced, now you could say the user has no control over fixing it, it is also true with the template case.  Both cases are hard to fix without much thought.  There is also something like:

static inline int f(int a)
{
  if (a)
   return g();
  return 0;
}


int h(void)
{
  return f(0);
}

With -Wunreachable-code -O1, we warn that we cannot reach the line containing return g();  Now should we, it is the same case, how can an user fix that code if the static inline function comes in from a header, they cannot.  -Wunreachable-code warning is useless except if you want to see if you do coverage.
Comment 15 Manuel López-Ibáñez 2007-03-19 23:35:13 UTC
Andrew, you have Paolo and Gabriel expressing that the warning should not be emitted because the code is generated. Then you close as wontfix. Sometimes I don't understand you at all.
Comment 16 gdr@cs.tamu.edu 2007-03-19 23:40:00 UTC
Subject: Re:  Strange -Wunreachable-code warning with _GLIBCXX_DEBUG

"pinskia at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:

| But the user can see the code,

Andrew --

   When you don't understand an issue, please refrain from offering
your help to close it.

-- Gaby
Comment 17 Paolo Carlini 2007-03-19 23:43:10 UTC
WONTFIX is simply ridicolous.
Comment 18 Manuel López-Ibáñez 2008-08-23 15:59:00 UTC
(In reply to comment #0)
> Compiling the following with "-Wunreachable-code -D_GLIBCXX_DEBUG"
> --------------------------
> #include <vector>
> 
> int main()
> {
>   std::vector<int>::iterator a;
> }
> --------------------------
> 
> produces the warning :

I cannot reproduce this in GCC 4.4.
Comment 19 Manuel López-Ibáñez 2008-08-23 16:01:16 UTC
(In reply to comment #4)
> Here is a minimal code example:
> 
> #include <new>
> 
> int* get_ptr(void* ptr)
> {
>   return new(ptr) int();
> }

I can reproduce this, though.
Comment 20 Manuel López-Ibáñez 2008-08-23 16:13:37 UTC
For testcase in comment #4 I get 2 warnings now:

home/manuel/test2/src/gcc/testsuite/g++.dg/warn/pr31246-2.C: In function ‘int* get_ptr(void*)’:
/home/manuel/test2/src/gcc/testsuite/g++.dg/warn/pr31246-2.C:8: warning: will never be executed

/home/manuel/test2/src/libstdc++-v3/libsupc++/new: In function ‘void* operator new(size_t, void*)’:
/home/manuel/test2/src/libstdc++-v3/libsupc++/new:105: warning: will never be executed

libsupc++/new does not contain #pragma GCC system_headers, so even using warning_at doesn't suppress the warning.
Comment 21 Manuel López-Ibáñez 2008-08-23 16:21:50 UTC
If gimple stmts do not have the equivalent of DECL_ARTIFICIAL, then the C++ front-end should use gimple_set_no_warning(stmt) when generating such constructs. So, anyone knows where this comes from?
Comment 22 Manuel López-Ibáñez 2008-08-23 17:24:00 UTC
In addition, __cxa_call_unexpected should probably have both TREE_NO_WARNING and DECL_ARTIFICIAL set but this is orthogonal because at the point of the warning we should not be testing that.
Comment 23 Paolo Carlini 2008-08-23 18:43:08 UTC
To be clear, there are no #pragma GCC system_header at all in the entire libsupc++ directory. I hope we don't have to begin...
Comment 24 Paolo Carlini 2008-08-23 18:45:37 UTC
I'm going to tweak a bit the Summary, seems misleading now.
Comment 25 Manuel López-Ibáñez 2008-08-23 18:50:03 UTC
This looks clearer to me. Maybe Jason or Mark have some idea where this code is generated. It should be clear that it is compiler-generated and we can set DECL_ARTIFICIAL or TREE_NO_WARNING. Then we just have to make sure to propagate that. And finally, check it before warning.
Comment 26 Gabriel Dos Reis 2008-08-26 05:45:31 UTC
Subject: Re:  Strange -Wunreachable-code warning with _GLIBCXX_DEBUG

On Sat, Aug 23, 2008 at 1:43 PM, paolo dot carlini at oracle dot com
<gcc-bugzilla@gcc.gnu.org> wrote:
> ------- Comment #23 from paolo dot carlini at oracle dot com  2008-08-23 18:43 -------
> To be clear, there are no #pragma GCC system_header at all in the entire
> libsupc++ directory. I hope we don't have to begin...

I think I agree.
Comment 27 Manuel López-Ibáñez 2008-10-22 08:35:17 UTC
(In reply to comment #5)
> Note, however, that, as far as I can see, such try/catch are *not* in the
> library code proper, but *all* synthesized by the C++ front-end.

Does anyone has any idea where in the C++ front-end this is synthesized?
Comment 28 Manuel López-Ibáñez 2009-02-07 21:07:11 UTC
There is a patch here:

http://gcc.gnu.org/ml/gcc-patches/2008-10/msg00972.html
Comment 29 Manuel López-Ibáñez 2009-07-07 22:18:50 UTC
Subject: Bug 31246

Author: manu
Date: Tue Jul  7 22:18:35 2009
New Revision: 149354

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=149354
Log:
2009-07-08  Manuel López-Ibáñez  <manu@gcc.gnu.org>

	PR c++/31246
	* gimplify.c (gimplify_expr): Propagate no_warning flag when
	gimplifying.
	* gimple (gimple_build_call_from_tree): Likewise.
	* tree-cfg.c (remove_useless_stmts_warn_notreached): Check
	no_warning flag before warning.
cp/
	* init.c (build_new_1): Set TREE_NO_WARNING for compiler-generated
	code.
	* cp-gimplify.c (genericize_eh_spec_block): Likewise.
	
testsuite/	
	* g++.dg/warn/pr31246.C: New.
	* g++.dg/warn/pr31246-2.C: New.

Added:
    trunk/gcc/testsuite/g++.dg/warn/pr31246-2.C
    trunk/gcc/testsuite/g++.dg/warn/pr31246.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/cp-gimplify.c
    trunk/gcc/cp/init.c
    trunk/gcc/gimple.c
    trunk/gcc/gimplify.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-cfg.c

Comment 30 Manuel López-Ibáñez 2009-07-07 22:20:34 UTC
FIXED in GCC 4.5.