https://gcc.gnu.org/g:a068d1b7c9da60a9b21efa4baf65f7325211be12
commit r17-376-ga068d1b7c9da60a9b21efa4baf65f7325211be12 Author: Tomasz Kamiński <[email protected]> Date: Wed Jan 7 14:01:41 2026 +0100 libstdc++: Rework istreambuf_iterator::_M_sbuf handling to slice null-dereference warning [PR105580] The warning was produced by following sequence, 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. This is however an false-positive, as in such situation it == istream_iteator() returns true, and the iterator should not be incremented 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. Also removes no longer needed "-Wnull-dereference" disabling pragmas from streambuf (see r16-7844-gbfc2b87f8244a1). 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. * include/std/streambuf (streambuf::gptr, streambuf::egptr) (streambuf::gbump): Remove surrounding pragma disabling -Wnull-dereference. * testsuite/24_iterators/istreambuf_iterator/2.cc: Test for using multiple iterators to same rdbuf. Reviewed-by: Jonathan Wakely <[email protected]> Signed-off-by: Tomasz Kamiński <[email protected]> Diff: --- libstdc++-v3/include/bits/streambuf_iterator.h | 20 +++---- libstdc++-v3/include/std/streambuf | 5 -- .../24_iterators/istreambuf_iterator/2.cc | 63 +++++++++++++++++++++- 3 files changed, 70 insertions(+), 18 deletions(-) diff --git a/libstdc++-v3/include/bits/streambuf_iterator.h b/libstdc++-v3/include/bits/streambuf_iterator.h index 93d3dd24f931..095928ca4d8f 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/include/std/streambuf b/libstdc++-v3/include/std/streambuf index f013aa06c28b..d6036bd5ddc4 100644 --- a/libstdc++-v3/include/std/streambuf +++ b/libstdc++-v3/include/std/streambuf @@ -490,10 +490,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION char_type* eback() const { return _M_in_beg; } -// Required to silence false-positive warnings originating from istream_iterator::operator++, -// see PR105580. -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wnull-dereference" char_type* gptr() const { return _M_in_cur; } @@ -509,7 +505,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION */ void gbump(int __n) { _M_in_cur += __n; } -#pragma GCC diagnostic pop /** * @brief Setting the three read area pointers. diff --git a/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/2.cc b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/2.cc index 78701d71cee9..f84bd5d845f0 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,69 @@ void test02(void) } } +void +test_empty() +{ + std::istreambuf_iterator<char> null(0), end; + VERIFY( null == end ); + + std::istringstream ess; + // 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 means the behavior of multiple iterators to the same sequence + // is specified by the standard (which is not generally true for + // input iterators) + 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; }
