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 <[email protected]>
---
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,
+ // (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.
+ 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.
+ 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
