https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106392
Bug ID: 106392
Summary: Support iteration over C++ containers in -fanalyzer
Product: gcc
Version: 13.0
Status: UNCONFIRMED
Severity: enhancement
Priority: P3
Component: analyzer
Assignee: dmalcolm at gcc dot gnu.org
Reporter: redi at gcc dot gnu.org
Blocks: 97110
Target Milestone: ---
I haven't really thought this one through completely, and maybe it's too hard,
but it would be good to check that 'for' loops using iterators don't do silly
things, e.g.
for (auto iter = cont.begin(); iter != cont.end() - 1; ++iter)
// ...
This is unsafe if the container is not empty, because end-1 is invalid.
Similarly for begin+1 when the container is empty. Need to check cont.begin()
!= cont.end() before doing begin+1 or end-1
And end+1 is always wrong.
Range-based for loops should not modify the container being iterated over:
for (auto& x : cont)
if (x == 3)
cont.push_back(4);
Modifying the container while traversing it might invalidate the iterators and
have undefined behaviour. (No iterators are visible in the code, but the
compiler simply expands the range-based 'for' into a traditional 'for' using
two iterators, and just because the user can't see them, those iterators are
still invalidated by modifications to the container.
Modifying the container in any loop is potentially risky, if not done
correctly. For an associative container such as std::set, std::map etc. this is
wrong:
for (auto it = set.begin(); it != set.end(); ++it)
if (*it == 3)
set.erase(it);
The ++it increment is undefined after erasing that element, because the erase
call invalidates the iterator.
This solves the invalidation problem, but is still wrong:
for (auto it = set.begin(); it != set.end(); ++it)
if (*it == 3)
it = set.erase(it); // returns iterator to the element after the erased
one
Now we get a valid iterator back from erase, but then we skip the next element
because the ++it always happens at the end of each loop iteration. And if the
erased element was the last one then erase returns the end() iterator, then we
do ++it on the past-the-end iterator, which is undefined.
This is the right way to do it:
for (auto it = set.begin(); it != set.end(); )
if (*it == 3)
it = set.erase(it);
else
++it;
But ideally, just don't. Use std::erase_if(cont, 3) instead.
It would be great if the analyzer understood enough to flag some of these.
Referenced Bugs:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97110
[Bug 97110] [meta-bug] tracker bug for supporting C++ in -fanalyzer
