Re: [boost] Wave C++ Review Begins Today - February 7, 2005

From: Doug Gregor (dgregor_at_[hidden])
Date: 2005-02-09 11:22:34

On Feb 9, 2005, at 10:50 AM, Michiel Salters wrote:
>> Michiel Salters wrote:
>>> one serious bug in wave\cpplexer\re2clex\cpp_re2c_lexer.hpp:
>>> template <typename IteratorT, typename PositionT> inline
>>> lexer<IteratorT, PositionT>::lexer(IteratorT const &first,
>>>         IteratorT const &last, PositionT const &pos,
>>>         boost::wave::language_support language)
>>> :   filename(pos.get_file()), at_eof(false)
>>> {
>>>     memset(&scanner, '\0', sizeof(Scanner));
>>>     scanner.fd = -1;
>>>     scanner.eol_offsets = aq_create();
>>>     scanner.first = scanner.act = (uchar *)&(*first);
>>>     scanner.last = (uchar *)&(*last);
>>> ...
>>> [ IteratorT = std::string::iterator in the example ]
>>>
>>> This assumes that the range [first,last] is dereferenacable and
>>> contiguous in memory. This of course is not mandated by the
>> standard.
>>
>> Yes, this is a known problem (see the Changelog file), I
>> didn't realise, that it will assert on some systems. Seems,
>> that I'll have to fix that soon ;-). This shouldn't be a
>> problem on other systems, though, if you're using iterators
>> acting on a continuous chunk of memory (such as
>> std::string::iterator).
>
> No, you're mistaken here and in the Changelog as well. Even
> std::string::iterator doesn't have to refer to contiguous
> memory. std::vector<char>::iterator would, though, so I built
> a string around that and found the actual bug (and a VC8 bug
> as well I think).
>
> The bug is that context<IteratorT> stores the two iterators
> first and last as const&. The cpp example passed .begin() and
> .end(), which are temporaries and they go out of scope.
> The result is that the context ends up with references to
> destructed iterators. Again, the VC8 STL iterator debugging
> found the problem on first access.
>
> Line 281, cpp_context.hpp

Even dereferencing *last could be a bug, if it is past-the-end. For 
iterators referencing contiguous memory (vectors and pointers only, as 
you've said), one would need to write

        scanner.last = scanner.first + std::distance(first, last);

> I really have to say Dinkumware did a fine job here, this
> would have been very hard to find without these checks.

FWIW, STLport and newer versions of GCC (Apple GCC 3.3, FSF GCC 3.4) 
have similar debugging modes. They've been invaluable in rooting out 
subtle bugs like those that stem from iterator invalidation.

        Doug