libstdc++: Silence false positive null-dereference warning in istreambuf_iterator::operator++ [PR105580]

  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(-)
  

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 <tkaminsk@redhat.com> 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
>
>
  

I have performed some microbenchmarking, on following example:
* bench_empty: input is empty istreamstream, creates iterator and compares
against end
* bench_stream_create: only resetting stringstream with new content
* bench_preinc: for loop over using pre increment
* bench_postinc: for loop over using post increment
* bench_deref_posting: while with *it++

This are the result I have observed, for the changes with the patch:
--------------------------------------------------------------
Benchmark                    Time             CPU   Iterations
--------------------------------------------------------------
bench_empty               2.10 ns         2.09 ns    337921434
bench_stream_create       10.6 ns         10.6 ns     65736840
bench_preinc               419 ns          419 ns      1662732
bench_postinc              436 ns          435 ns      1609828
bench_deref_postinc        306 ns          305 ns      22866

And the result before the patch:
--------------------------------------------------------------
Benchmark                    Time             CPU   Iterations
--------------------------------------------------------------
bench_empty               2.27 ns         2.26 ns    314944156
bench_stream_create       11.7 ns         11.6 ns     60679957
bench_preinc               873 ns          871 ns       807352
bench_postinc              920 ns          917 ns       751565
bench_deref_postinc        706 ns          704 ns      1038942

For sanity, I have also check the result on g++ 15.2 that ships with my
system:
--------------------------------------------------------------
Benchmark                    Time             CPU   Iterations
--------------------------------------------------------------
bench_empty               2.30 ns         2.30 ns    303070664
bench_stream_create       10.6 ns         10.6 ns     65778662
bench_preinc               767 ns          765 ns       915085
bench_postinc              767 ns          764 ns       913985
bench_deref_postinc        675 ns          673 ns      1107772

So it looks like eliminating save to member from both equal and operator*
makes implementation
two times faster. I will look into that.

Attaching also my benchmark file.

Regards,
Tomasz

On Wed, Jan 7, 2026 at 3:41 PM Tomasz Kamiński <tkaminsk@redhat.com> 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
>
>
  

On Thu, Jan 8, 2026 at 10:38 AM Tomasz Kaminski <tkaminsk@redhat.com> wrote:

> I have performed some microbenchmarking, on following example:
> * bench_empty: input is empty istreamstream, creates iterator and compares
> against end
> * bench_stream_create: only resetting stringstream with new content
> * bench_preinc: for loop over using pre increment
> * bench_postinc: for loop over using post increment
> * bench_deref_posting: while with *it++
>
> This are the result I have observed, for the changes with the patch:
> --------------------------------------------------------------
> Benchmark                    Time             CPU   Iterations
> --------------------------------------------------------------
> bench_empty               2.10 ns         2.09 ns    337921434
> bench_stream_create       10.6 ns         10.6 ns     65736840
> bench_preinc               419 ns          419 ns      1662732
> bench_postinc              436 ns          435 ns      1609828
> bench_deref_postinc        306 ns          305 ns      22866
>
> And the result before the patch:
> --------------------------------------------------------------
> Benchmark                    Time             CPU   Iterations
> --------------------------------------------------------------
> bench_empty               2.27 ns         2.26 ns    314944156
> bench_stream_create       11.7 ns         11.6 ns     60679957
> bench_preinc               873 ns          871 ns       807352
> bench_postinc              920 ns          917 ns       751565
> bench_deref_postinc        706 ns          704 ns      1038942
>
> For sanity, I have also check the result on g++ 15.2 that ships with my
> system:
> --------------------------------------------------------------
> Benchmark                    Time             CPU   Iterations
> --------------------------------------------------------------
> bench_empty               2.30 ns         2.30 ns    303070664
> bench_stream_create       10.6 ns         10.6 ns     65778662
> bench_preinc               767 ns          765 ns       915085
> bench_postinc              767 ns          764 ns       913985
> bench_deref_postinc        675 ns          673 ns      1107772
>
> So it looks like eliminating save to member from both equal and operator*
> makes implementation
> two times faster. I will look into that.
>
> Attaching also my benchmark file.
>
> Regards,
> Tomasz
>
> On Wed, Jan 7, 2026 at 3:41 PM Tomasz Kamiński <tkaminsk@redhat.com>
> 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();
>>
> The benefit seem to be result of eliminating the if condition here, as we
are now having
one less eof check per loop, previously we got:
O: 4: 2x2 in _M_get per operator* and equal
N: 3: 2x1 in _M_get per operator* and equal, and one in operator++

>         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
>>
>>
  

On Thu, Jan 8, 2026 at 10:55 AM Tomasz Kaminski <tkaminsk@redhat.com> wrote:

>
>
> On Thu, Jan 8, 2026 at 10:38 AM Tomasz Kaminski <tkaminsk@redhat.com>
> wrote:
>
>> I have performed some microbenchmarking, on following example:
>> * bench_empty: input is empty istreamstream, creates iterator and
>> compares against end
>> * bench_stream_create: only resetting stringstream with new content
>> * bench_preinc: for loop over using pre increment
>> * bench_postinc: for loop over using post increment
>> * bench_deref_posting: while with *it++
>>
>> This are the result I have observed, for the changes with the patch:
>> --------------------------------------------------------------
>> Benchmark                    Time             CPU   Iterations
>> --------------------------------------------------------------
>> bench_empty               2.10 ns         2.09 ns    337921434
>> bench_stream_create       10.6 ns         10.6 ns     65736840
>> bench_preinc               419 ns          419 ns      1662732
>> bench_postinc              436 ns          435 ns      1609828
>> bench_deref_postinc        306 ns          305 ns      22866
>>
>> And the result before the patch:
>> --------------------------------------------------------------
>> Benchmark                    Time             CPU   Iterations
>> --------------------------------------------------------------
>> bench_empty               2.27 ns         2.26 ns    314944156
>> bench_stream_create       11.7 ns         11.6 ns     60679957
>> bench_preinc               873 ns          871 ns       807352
>> bench_postinc              920 ns          917 ns       751565
>> bench_deref_postinc        706 ns          704 ns      1038942
>>
>> For sanity, I have also check the result on g++ 15.2 that ships with my
>> system:
>> --------------------------------------------------------------
>> Benchmark                    Time             CPU   Iterations
>> --------------------------------------------------------------
>> bench_empty               2.30 ns         2.30 ns    303070664
>> bench_stream_create       10.6 ns         10.6 ns     65778662
>> bench_preinc               767 ns          765 ns       915085
>> bench_postinc              767 ns          764 ns       913985
>> bench_deref_postinc        675 ns          673 ns      1107772
>>
>> So it looks like eliminating save to member from both equal and operator*
>> makes implementation
>> two times faster. I will look into that.
>>
>> Attaching also my benchmark file.
>>
>> Regards,
>> Tomasz
>>
>> On Wed, Jan 7, 2026 at 3:41 PM Tomasz Kamiński <tkaminsk@redhat.com>
>> 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();
>>>
>> The benefit seem to be result of eliminating the if condition here, as we
> are now having
> one less eof check per loop, previously we got:
> O: 4: 2x2 in _M_get per operator* and equal
> N: 3: 2x1 in _M_get per operator* and equal, and one in operator++
>
We could also eliminate the  _S_is_eof(__ret) check, by returning __proxy
instead of istream_iterator
for pre-increment, and eliminate uses of _M_c completely. This will however
break already non-portable
code that uses:
  istream_iteartor i;
  istream_iteartor i2 = i++;
We cannot provide the conversion operator, as this would require
reintroducing S_is_eof check.

I have prototyped that, and there is a measurable benefit from doing it,
but I think something that we should
do in stage 1 for GCC 17.

        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
>>>
>>>
  

On Thu, Jan 8, 2026 at 11:52 AM Tomasz Kaminski <tkaminsk@redhat.com> wrote:

>
>
> On Thu, Jan 8, 2026 at 10:55 AM Tomasz Kaminski <tkaminsk@redhat.com>
> wrote:
>
>>
>>
>> On Thu, Jan 8, 2026 at 10:38 AM Tomasz Kaminski <tkaminsk@redhat.com>
>> wrote:
>>
>>> I have performed some microbenchmarking, on following example:
>>> * bench_empty: input is empty istreamstream, creates iterator and
>>> compares against end
>>> * bench_stream_create: only resetting stringstream with new content
>>> * bench_preinc: for loop over using pre increment
>>> * bench_postinc: for loop over using post increment
>>> * bench_deref_posting: while with *it++
>>>
>>> This are the result I have observed, for the changes with the patch:
>>> --------------------------------------------------------------
>>> Benchmark                    Time             CPU   Iterations
>>> --------------------------------------------------------------
>>> bench_empty               2.10 ns         2.09 ns    337921434
>>> bench_stream_create       10.6 ns         10.6 ns     65736840
>>> bench_preinc               419 ns          419 ns      1662732
>>> bench_postinc              436 ns          435 ns      1609828
>>> bench_deref_postinc        306 ns          305 ns      22866
>>>
>>> And the result before the patch:
>>> --------------------------------------------------------------
>>> Benchmark                    Time             CPU   Iterations
>>> --------------------------------------------------------------
>>> bench_empty               2.27 ns         2.26 ns    314944156
>>> bench_stream_create       11.7 ns         11.6 ns     60679957
>>> bench_preinc               873 ns          871 ns       807352
>>> bench_postinc              920 ns          917 ns       751565
>>> bench_deref_postinc        706 ns          704 ns      1038942
>>>
>>
>>> For sanity, I have also check the result on g++ 15.2 that ships with my
>>> system:
>>> --------------------------------------------------------------
>>> Benchmark                    Time             CPU   Iterations
>>> --------------------------------------------------------------
>>> bench_empty               2.30 ns         2.30 ns    303070664
>>> bench_stream_create       10.6 ns         10.6 ns     65778662
>>> bench_preinc               767 ns          765 ns       915085
>>> bench_postinc              767 ns          764 ns       913985
>>> bench_deref_postinc        675 ns          673 ns      1107772
>>>
>> The times reported when using __proxy as return type for post-increment,
without providing conversion
to istream_iterator:
--------------------------------------------------------------
Benchmark                    Time             CPU   Iterations
--------------------------------------------------------------
bench_empty               1.80 ns         1.79 ns    398182529
bench_stream_create       10.6 ns         10.6 ns     65450500
bench_preinc               314 ns          314 ns      2231714
bench_postinc              360 ns          359 ns      2029998
bench_deref_postinc        315 ns          313 ns      2235176


>
>>> So it looks like eliminating save to member from both equal and
>>> operator* makes implementation
>>> two times faster. I will look into that.
>>>
>>> Attaching also my benchmark file.
>>>
>>> Regards,
>>> Tomasz
>>>
>>> On Wed, Jan 7, 2026 at 3:41 PM Tomasz Kamiński <tkaminsk@redhat.com>
>>> 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();
>>>>
>>> The benefit seem to be result of eliminating the if condition here, as
>> we are now having
>> one less eof check per loop, previously we got:
>> O: 4: 2x2 in _M_get per operator* and equal
>> N: 3: 2x1 in _M_get per operator* and equal, and one in operator++
>>
> We could also eliminate the  _S_is_eof(__ret) check, by returning __proxy
> instead of istream_iterator
> for pre-increment, and eliminate uses of _M_c completely. This will
> however break already non-portable
> code that uses:
>   istream_iteartor i;
>   istream_iteartor i2 = i++;
> We cannot provide the conversion operator, as this would require
> reintroducing S_is_eof check.
>
> I have prototyped that, and there is a measurable benefit from doing it,
> but I think something that we should
> do in stage 1 for GCC 17.
>
>         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
>>>>
>>>>