Bug 21951 - [4.0 only] std::vector.reserve() unusable with -Werror -Wall -O -fno-exceptions
Summary: [4.0 only] std::vector.reserve() unusable with -Werror -Wall -O -fno-exceptions
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 4.0.0
: P2 normal
Target Milestone: 4.0.2
Assignee: Geoff Keating
URL:
Keywords: diagnostic
: 24498 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-06-07 17:58 UTC by dank
Modified: 2005-10-24 01:31 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2005-07-01 19:10:02


Attachments
proposed workaround (964 bytes, patch)
2005-06-11 19:29 UTC, dank
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description dank 2005-06-07 17:58:19 UTC
Compiling the following code:

#include <vector>
std::vector<int> *factory()
{
        std::vector<int> *p = new std::vector<int>;
        p->reserve(10);
        return p;
}

with -Wall -O -fno-exceptions yields the following error in gcc-4.0.0
and gcc-4.0-20050602:

.../include/c++/4.0.0/bits/vector.tcc: In member function 'void std::vector<_Tp,
_Alloc>::reserve(size_t) [with _Tp = int, _Alloc = std::allocator<int>]':
.../include/c++/4.0.0/bits/vector.tcc:78: warning: control may reach end of
non-void function 'typename _Alloc::pointer std::vector<_Tp,
_Alloc>::_M_allocate_and_copy(size_t, _ForwardIterator, _ForwardIterator) [with
_ForwardIterator = int*, _Tp = int, _Alloc = std::allocator<int>]' being inlined

The warning comes from a catch/rethrow block deep in the bowels of stl
which (because of -fno-exceptions) evaluates to if(false).

This is a false positive warning, which would be fine except
for shops whose policy is to always compile with -Werror -Wall;
there, the warning is fatal.  And because this problem is not
confined to just one user source file, it's hard to work around.

This problem does not occur in gcc-3.4.3 nor in recent gcc-4.1 snapshots.

cf. discussion here: http://gcc.gnu.org/ml/libstdc++/2005-06/msg00073.html
Comment 1 Andrew Pinski 2005-06-07 18:00:50 UTC
And I already filed this way back, see PR 19699 which I am closing this bug as a dup.
See comment #2 in that PR from RTH:
There is zero chance that this will be fixed for 4.0.  That's exactly why Ian
implemented the minimalistic check that he did in solving PR19583 and related.

You may retartget this pr to fixing the silliness in libstdc++, if you want.


*** This bug has been marked as a duplicate of 19699 ***
Comment 2 Paolo Carlini 2005-06-07 18:12:34 UTC
> You may retartget this pr to fixing the silliness in libstdc++, if you want.

And the "silliness" would be? Personally, I'm finding quite a bit of silliness
in this remark, to tell you the truth and indeed, mainline is ok, probably the
current compiler judges that "silliness" not so silly, after all.

When -fno-exceptions, the catch becomes simply an 'if (false)' and I don't see
why the implementors of v3 have necessarily to care about the branch not
returning.
Comment 3 Andrew Pinski 2005-06-07 18:16:59 UTC
(In reply to comment #2)
> > You may retartget this pr to fixing the silliness in libstdc++, if you want.

 Paolo I copied the quote fully when I really should not have.  RTH did not know we could not fix the 
if(0) part in libstdc++ at the time he wrote that comment, if you read the next comment in that bug I 
explain why it cannot be fixed in libstdc++ (comment #3 in PR 19699):
(In reply to comment #2)
> You may retartget this pr to fixing the silliness in libstdc++, if you want.
Actually it is only silliness as try/catch is changed to be "if (true)"/"if (false)" if we don't have exceptions.  
There is nothing can be done to the libstdc++ headers to fix this.
Comment 4 Michael Veksler 2005-06-07 19:46:40 UTC
(In reply to comment #3)
>  Paolo I copied the quote fully when I really should not have.  RTH did not
know we could not fix the 
> if(0) part in libstdc++ at the time he wrote that comment, if you read the
next comment in that bug I 
> explain why it cannot be fixed in libstdc++ (comment #3 in PR 19699):
> (In reply to comment #2)
> > You may retartget this pr to fixing the silliness in libstdc++, if you want.
> Actually it is only silliness as try/catch is changed to be "if (true)"/"if
(false)" if we don't have exceptions.  
> There is nothing can be done to the libstdc++ headers to fix this.

I disagree. libstdc++ can do exactly what everybody else does in such cases:
 add a dummy (unreachable) "return __result" at the end, after both try
 and catch blocks.

This will be a hack, no doubt, a pragmatic one. I don't think that emitting
false positive diagnostic that the user does not control is a good thing.
I have been working in a verification (research) department for 10 years,
and I can assure you that false positives, which can't be turned off, 
are worse than no diagnostic at all. Such diagnostics *will* turn
customers/users away from your tool, or at best "just" ignore diagnostics.

I have used a (bought) tool that gave a false positive every 500 lines of
code. For 500 KLOC it would give 1K false positive. Now, try to find out a
single true bug out of the noise of 1000 false positives. I can tell you that
the tool was dropped very fast.

We write tools used for verification and testing. Our customers are more likely
to be willing to live with uncovered events, than to get false positives. Think
of millions of tests/events that cause even 0.1% of false positive. These may
overshadow hundreds of real bugs.

Same with gcc, if something as central as vector.reserve() give false positive
diagnostics then the sheer volume of warnings will render either the warning
or reserve() unusable.

I suggest to reopen PR19699 against libstdc++ (or maybe open a new one)
Comment 5 Wolfgang Bangerth 2005-06-07 23:28:02 UTC
I concur with the last post -- a dummy return at the end would solve 
the problem and seems like an acceptable solution for a release 
branch. 
 
W. 
Comment 6 Andrew Pinski 2005-06-08 13:20:47 UTC
Note Mark has said in the past (and in a private mail which was supposed to be a public one too) that 
warnings cannot cause this to be a rejects valid.
Comment 7 dank 2005-06-08 13:57:41 UTC
It sure as hell is for those shops that require -Werror.

But ok, I'll be happy if it's fixed for 4.0.2.
Comment 8 Michael Veksler 2005-06-08 14:11:39 UTC
(In reply to comment #6)
> Note Mark has said in the past (and in a private mail which was supposed to be
a public one too) that 
> warnings cannot cause this to be a rejects valid.


For the same reason, PR 21183 should be reopened or merged with this PR:

> When compiling a C++ program with -Wall, I get the following compiler warning
>
>/usr/local/bin/../lib/gcc/i686-pc-linux-gnu/4.0.0/../../../../include/c++/4.0.0/bits/stl_uninitialized.h:113:
>warning: control may reach end of non-void function '_ForwardIterator
> std::__uninitialized_copy_aux(_InputIterator, _InputIterator, 
...
> The problem seems to be in __uninitialized_copy_aux() in stl_uninitialized.h
>
> The catch statement block beginning on line 89 merely exits the routine without 
> actually returning a value.

Comment 9 Michael Veksler 2005-06-08 14:43:38 UTC
(In reply to comment #7)
> It sure as hell is for those shops that require -Werror.
>
> But ok, I'll be happy if it's fixed for 4.0.2.

I think that your argument (as phrased) does not hold.
Maybe you meant "It sure as hell is for those shops that require -Werror, 
in this particular instance".
(Pardon my language, I just quoted the original ;-)

Consider:
1. 1: int main()
   2: {
   3:   int a;
   4:   never_return(); // the halting problem
   5:   return a;       // Uninitialized variable?
   6: }
   This is perfectly valid code and well defined, yet -Wall -Werror
   will reject it. No compiler will ever be able to determine if
   line 5 is ever reached.

2. 1: int main()
   2: {
   3:    int a;
   4:    if(foo())
   5:      a= bar();
   6:    if(foo())
   7:      return a;  // Uninitialized variable?
   8:    return 0;
   9: }

   Again, no compiler will be able to prove that a nontrivial foo() does
   not change over time (unless declared const/pure/whatever).
   And as a result, -Wall -Werror will reject valid code.

In both examples, the user has a simple work-around, initialize 'a'.
Adding initialization will make the code more stable, as foo() is
no longer constrained to be const/pure (forgive me for not remembering if
it is called pure or const).

In contrast (as mentioned in comment #4), this PR and PR 21183 do not
give the user the tools to shut this specific diagnostic instance up.
Comment 10 Benjamin Kosnik 2005-06-08 19:16:16 UTC
This is a compiler bug. If they don't fix it in 4.0.x, then we'll have to work
around it in libstdc++ 4.0 sources only. 

People have suggested a patch to add a dummy return. If that's posted, it seems
to be perfectly acceptable to me. I suggest that be posted...

-benjamin
Comment 11 dank 2005-06-11 11:25:04 UTC
OK, I'm testing a patch now.
Comment 12 dank 2005-06-11 19:29:30 UTC
Created attachment 9071 [details]
proposed workaround
Comment 13 Geoff Keating 2005-07-01 19:10:02 UTC
I have a patch for the compiler here:

http://gcc.gnu.org/ml/gcc-patches/2005-07/msg00049.html

which will fix this problem.
Comment 14 dank 2005-07-01 19:42:34 UTC
That's certainly prettier than my header changes.  
I'll give it a shot.  Thanks!
Comment 15 Andrew Pinski 2005-07-16 21:00:52 UTC
Fixed by:
2005-07-08 Geoffrey Keating  <geoffk@apple.com>

        * tree-inline.c (expand_call_inline): Prevent 'may reach end'
        warning in system headers.
        
Comment 16 Andrew Pinski 2005-10-24 01:31:45 UTC
*** Bug 24498 has been marked as a duplicate of this bug. ***