[PATCH v2] libstdc++: Rework istreambuf_iterator::_M_sbuf handling to slince null-dereference warning [PR105580]
Jonathan Wakely
jwakely@redhat.com
Wed May 6 20:41:43 GMT 2026
On Mon, 02 Mar 2026 at 08:25 +0100, Tomasz Kamiński 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
>situation 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 have zero or positive 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 against EoF 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.
> * testsuite/24_iterators/istreambuf_iterator/105580.cc: Remove check for
> warning being emitted.
>
>Signed-off-by: Tomasz Kamiński <tkaminsk@redhat.com>
>---
>v2:
>- fixes typos in commit description
>- mentions LWG2366 in test_empty()
>- modifies already merged test for 105580
>I haven't reverted removal of mutable of _M_sbuf, as I thin we agreed
>it should not have impact.
>
>I will post a separate patch adding pragmas to silence warning, however
>they need to placed surrounbding gptr, egptr, gbump function in streambuf.
>However, after recheching the patch, I think that it seem to be safe for
>GCC16.
>
>Testing on x86_64-linux. istreambuf_iterator test passsed.
>OK for GCC16 (if not 17) if test passes?
>
> .../include/bits/streambuf_iterator.h | 20 +++---
> .../istreambuf_iterator/105580.cc | 2 -
> .../24_iterators/istreambuf_iterator/2.cc | 61 ++++++++++++++++++-
> 3 files changed, 68 insertions(+), 15 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/105580.cc b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/105580.cc
>index 85f888b86e7..87edf999ffc 100644
>--- a/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/105580.cc
>+++ b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/105580.cc
>@@ -1,5 +1,4 @@
> // { dg-compile }
>-// { dg-require-normal-mode "" }
> // { dg-additional-options "-Wnull-dereference" }
>
> #include <string>
>@@ -12,5 +11,4 @@ int main()
> std::string ss(it, end);
> return 0;
> }
>-// { dg-warning ".*null pointer dereference" "" { target *-*-* } 0 }
>
>diff --git a/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/2.cc b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/2.cc
>index 78701d71cee..cdf4a0df0f4 100644
>--- a/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/2.cc
>+++ b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/2.cc
>@@ -48,7 +48,7 @@ void test02(void)
> cistreambuf_iter istrb_it05(istrs01);
> cistreambuf_iter istrb_it06(istrs01.rdbuf());
> VERIFY( istrb_it05 == istrb_it06 );
>-
>+
> // bool equal(istreambuf_iter& b)
> cistreambuf_iter istrb_it07(0);
> cistreambuf_iter istrb_it08;
>@@ -109,8 +109,67 @@ void test02(void)
> }
> }
>
>+void
>+test_empty()
>+{
>+ std::istreambuf_iterator<char> null(0), end;
>+ VERIFY( null == end );
>+
>+ std::istringstream ess;
>+ // This specification hear seem to indicate that such iterators
>+ // are not end-of-stream iterators, as rdbuf pointer is nullptr,
... as rdbuf pointer is *not* nullptr?
>+ // (see LWG 2366: istreambuf_iterator end-of-stream equality).
>+ // However we treat them as such, as otherwise we would produce
>+ // a range containing single EoF character.
I think the comment would be clearer as:
// The specification seems to indicate that these iterators are not
// end-of-stream iterators, as they have non-null rdbuf pointers.
// (see LWG 2366: istreambuf_iterator end-of-stream equality).
// However we treat them as end-of-stream, as otherwise they would
// produce a range containing a 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 multiple iterators to same sequence.
For the last sentence, I would prefer:
// This means the behavior of multiple iterators to the same sequence
// is specified by the standard (which is not generally true for
// input iterators).
OK for trunk with those changes to the comments in the test.
>+ 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.53.0
>
>
More information about the Libstdc++
mailing list