Forgot to include, this change are ABI compatible, as clearing _M_sbuf is de-facto optimization of the EoF checks, and if we get (N - after the patch, O - before the patch): * N op*, O op++ -> we never set _M_sbuf to nullptr, so equals will call sgetc() for also when the iterator reaches end-of-stream (same as old behavior) * O op*, N op++ -> impacts only programs that deference past the end iterators (we allow that as extension), otherwise no-impact, except duplicated _S_is_eof(_M_c) check.
On Wed, Jan 7, 2026 at 3:41 PM Tomasz Kamiński <[email protected]> wrote: > The warning was produced by following seqeunce, given an > istream_iterator<char> > it, such that *it will result in hitting EoF in it->_M_get(), and thus > clearing > _M_sbuf, the subsequent call to ++it, will result in _M_sbuf->sbumpc() call > on null pointer dereference. This is however an false-positive, as in such > sitution > it == istream_iteator() returns true, and the iterator should not be > dereferenced > in first place. > > This patch addresses the above by clearing the _M_sbuf in operator++, > instead of > _M_get(). This removes the need for making _M_sbuf mutable, and thus make > the > implementation conforming with regards to C++11 [res.on.data.races] p3. > > This change should not or positive have performance impact on the usual > iteration > patterns, in form: > while (it != end) { process(*it); ++it; } > In case when it is end-of-stream iterator, the it != end returns in one > call of > _M_sbuf->sgetc() both before and after the change. However we do not > modify _M_sbuf > in this case. For non-empty range, we replace call to _M_sbuf->sbumpc() > with > _M_sbuf->snextc() in pre-increment, and extract the check again from *it to > ++it. However, as _M_sbuf is now cleared during increment, so last it != > end > check avoids _M_sbuf->sgetc() call to check against EoF. > > However, this change impact the behavior of the post-increment (*it++), as > we now load both current character (for return value) and next character > (to > check against EoF). In consequence we call both sgetc() and snextc(), in > contrast > to previous single sbumpc() call. > > PR libstdc++/105580 > > libstdc++-v3/ChangeLog: > > * include/bits/streambuf_iterator.h (istreambuf_iterator::_M_sbuf): > Remove mutable and adjust whitespace. > (istreambuf_iterator::_M_c): Adjust whitespace. > (istreambuf_iterator::operator++()): Clear _M_sbuf if next > character > is EoF. > (istreambuf_iterator::operator++(int)): Use _M_sbuf->sgetc() to > load current character, and define in terms of __*this. > (istreambuf_iterator::_M_get()): Do not clear _M_sbuf in case of > EoF. > * testsuite/24_iterators/istreambuf_iterator/2.cc: Test for using > multiple iterators to same rdbuf. > --- > This warning turned out to be false-positive (as explained in the commit > decription), so we can just add add pragrams to address that. But this > change seem to be beneficial anyway, as we are no longer causing > data-races. > > Tested on x86_64-linux. Locally tested with -Wnull-dereference, > and the istream_iterator warning in 24_iterators/istreambuf_iterator/2.cc > disappears (there are some other warning present I haven't looked into > that yet). > > .../include/bits/streambuf_iterator.h | 20 +++---- > .../24_iterators/istreambuf_iterator/2.cc | 58 +++++++++++++++++++ > 2 files changed, 66 insertions(+), 12 deletions(-) > > diff --git a/libstdc++-v3/include/bits/streambuf_iterator.h > b/libstdc++-v3/include/bits/streambuf_iterator.h > index 93d3dd24f93..095928ca4d8 100644 > --- a/libstdc++-v3/include/bits/streambuf_iterator.h > +++ b/libstdc++-v3/include/bits/streambuf_iterator.h > @@ -112,8 +112,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > // the "end of stream" iterator value. > // NB: This implementation assumes the "end of stream" value > // is EOF, or -1. > - mutable streambuf_type* _M_sbuf; > - int_type _M_c; > + streambuf_type* _M_sbuf; > + int_type _M_c; > > public: > /// Construct end of input stream iterator. > @@ -172,7 +172,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > _M_message(__gnu_debug::__msg_inc_istreambuf) > ._M_iterator(*this)); > > - _M_sbuf->sbumpc(); > + if (_S_is_eof(_M_sbuf->snextc())) > + _M_sbuf = 0; > _M_c = traits_type::eof(); > return *this; > } > @@ -181,14 +182,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > istreambuf_iterator > operator++(int) > { > - __glibcxx_requires_cond(_M_sbuf && > - (!_S_is_eof(_M_c) || > !_S_is_eof(_M_sbuf->sgetc())), > - > _M_message(__gnu_debug::__msg_inc_istreambuf) > - ._M_iterator(*this)); > - > istreambuf_iterator __old = *this; > - __old._M_c = _M_sbuf->sbumpc(); > - _M_c = traits_type::eof(); > + __old._M_c = _M_sbuf->sgetc(); > + ++*this; > return __old; > } > > @@ -206,8 +202,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > _M_get() const > { > int_type __ret = _M_c; > - if (_M_sbuf && _S_is_eof(__ret) && _S_is_eof(__ret = > _M_sbuf->sgetc())) > - _M_sbuf = 0; > + if (_M_sbuf && _S_is_eof(__ret)) > + __ret = _M_sbuf->sgetc(); > return __ret; > } > > diff --git a/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/2.cc > b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/2.cc > index 78701d71cee..0318a677f35 100644 > --- a/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/2.cc > +++ b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/2.cc > @@ -109,8 +109,66 @@ void test02(void) > } > } > > +void > +test_empty() > +{ > + std::istreambuf_iterator<char> null(0), end; > + VERIFY( null == end ); > + > + std::istringstream ess; > + // Thie specification hear seem to indicate, that such iterators > + // are not end-of-stream iterators, as rdbuf pointer is nullptr, > + // however we treat them as such, as otherwise we would produce > + // a range containing single eof character. > + std::istreambuf_iterator<char> it1(ess), it2(ess); > + VERIFY( it1 == end ); > + VERIFY( it2 == end ); > +} > + > +void > +test_multi() > +{ > + // C++98 (and later) define operator* in [istreambuf.iterator.ops] as: > + // Returns: The character obtained via the streambuf member > sbuf_->sgetc(). > + // This nails down behavior of mulptiple iterators to same sequence. > + std::istringstream iss("abcd"); > + std::istreambuf_iterator<char> it1(iss), it2(iss), end; > + > + VERIFY( it1 != end ); > + VERIFY( it2 != end ); > + VERIFY( *it1 == 'a' ); > + VERIFY( *it2 == 'a' ); > + ++it1; > + > + VERIFY( it1 != end ); > + VERIFY( it2 != end ); > + VERIFY( *it1 == 'b' ); > + VERIFY( *it2 == 'b' ); > + ++it2; > + > + VERIFY( it1 != end ); > + VERIFY( it2 != end ); > + VERIFY( *it1 == 'c' ); > + VERIFY( *it2 == 'c' ); > + ++it2; > + > + VERIFY( it1 != end ); > + VERIFY( it2 != end ); > + VERIFY( *it1 == 'd' ); > + VERIFY( *it2 == 'd' ); > + // second dereference > + VERIFY( *it1 == 'd' ); > + VERIFY( *it2 == 'd' ); > + ++it1; > + > + VERIFY( it1 == end ); > + VERIFY( it2 == end ); > +} > + > int main() > { > test02(); > + test_empty(); > + test_multi(); > return 0; > } > -- > 2.52.0 > >
