[Bug c++/79950] G++ cannot detect simple off by one error in STL classes

2017-03-08 Thread msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79950

Martin Sebor  changed:

   What|Removed |Added

 CC||msebor at gcc dot gnu.org

--- Comment #7 from Martin Sebor  ---
One part of the problem is that v[v.size()] isn't necessarily out of bounds (in
the -Warray-bounds sense) because v.size() <= v.capacity().  At a minimum,
though, v[v.size()] is an uninitialized read (in the -Wuninitialized sense),
but GCC can't tell that from just f1's definition.  For a checker to diagnose
this problem it would need be taught about std::vector.  Not only that, because
vector is represented using pointers (begin, end, end-of-storage), GCC would
probably also need be taught about pointer relationships (i.e., that begin <=
end <= end-of-storage always holds).  Basically support some form of pointer
ranges.  That would be a great feature to have (not just for vectors) but I
don't have the impression anyone is working on it.  (The alternative to pointer
ranges is to implement some sort of a pattern checker for containers as
suggested in comment #4.  Such checkers are usually the province of static
analyzers.  I'm not aware of a precedent for something like that in GCC.)

[Bug c++/79950] G++ cannot detect simple off by one error in STL classes

2017-03-08 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79950

--- Comment #6 from Jonathan Wakely  ---
(In reply to Jonathan Wakely from comment #5)
> (In reply to David Binderman from comment #4)
> > Anyway, I think some progress could be made by finding the pattern
> > 
> > for (something = 0; something <= somethingElse.size(); ++ something)
> > 
> > at compile time, which is what the  static analyser 'cppcheck' seems to be
> > doing.
> 
> That's what I said, it would have to be a special case built in to the
> front-end.

And it has to be a lot smarter than that, as there's nothing wrong with:

  for (unsigned i = 0; i <= v.size(); ++i)
std::cout << i;

It's only a problem if you do v[i] when i == v.size()

[Bug c++/79950] G++ cannot detect simple off by one error in STL classes

2017-03-08 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79950

--- Comment #5 from Jonathan Wakely  ---
(In reply to David Binderman from comment #4)
> (In reply to Jonathan Wakely from comment #3)
> > And for this example it's possible that g(int) modifies the vector that the
> > reference v is bound to
> 
> I doubt g can modify v.


Of course it can:


#include 

std::vector v;

void g(int i) {
  std::cout << i << '\n';
  if (i < 5)
v.push_back(i + 5);
}

int main()
{
  v = { 1, 10 };
  f1(v);
}

This is entirely valid. If linked to your code in comment 0 it has undefined
behaviour but that's because of your i <= size() not because of this code.



> Anyway, I think some progress could be made by finding the pattern
> 
> for (something = 0; something <= somethingElse.size(); ++ something)
> 
> at compile time, which is what the  static analyser 'cppcheck' seems to be
> doing.

That's what I said, it would have to be a special case built in to the
front-end.

[Bug c++/79950] G++ cannot detect simple off by one error in STL classes

2017-03-08 Thread dcb314 at hotmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79950

--- Comment #4 from David Binderman  ---
(In reply to Jonathan Wakely from comment #3)
> And for this example it's possible that g(int) modifies the vector that the
> reference v is bound to

I doubt g can modify v.

Anyway, I think some progress could be made by finding the pattern

for (something = 0; something <= somethingElse.size(); ++ something)

at compile time, which is what the  static analyser 'cppcheck' seems to be
doing.

$ ~/cppcheck/trunk/cppcheck mar7a.cc
[mar7a.cc:22]: (error) Array 'a[10]' accessed at index 10, which is out of
bounds.
[mar7a.cc:12]: (error) When i==v.size(), v[i] is out of bounds.
$ 

No work required at run time, AFAIK.

[Bug c++/79950] G++ cannot detect simple off by one error in STL classes

2017-03-08 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79950

--- Comment #3 from Jonathan Wakely  ---
And for this example it's possible that g(int) modifies the vector that the
reference v is bound to, so the size could even change on every loop iteration.
So if we added some runtime checks to std::vector (maybe only enabled for
_FORTIFY_SOURCE) then the size() would need to be reloaded on every loop
iteration, it couldn't be hoisted out of the loop. So that also suggests it
would have to be a special case handled by the front end, just for
std::vector::size()

[Bug c++/79950] G++ cannot detect simple off by one error in STL classes

2017-03-08 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79950

--- Comment #2 from Jonathan Wakely  ---
The C++ front-end could be taught about std::vector::size() as a special case,
but that would only help that special case, it wouldn't help for
std::deque::size(), or boost::vector::size() or other types.

[Bug c++/79950] G++ cannot detect simple off by one error in STL classes

2017-03-08 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79950

--- Comment #1 from Jonathan Wakely  ---
The two cases are not equivalent, because the bounds of a[10] are known at
compile-time, but v.size() is not. The only way to know v.size() is with a
run-time check, which has a non-zero cost.

[Bug c++/79950] G++ cannot detect simple off by one error in STL classes

2017-03-08 Thread rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79950

Richard Biener  changed:

   What|Removed |Added

   Keywords||diagnostic
   Severity|normal  |enhancement