[Bug libstdc++/42857] std::istream::ignore(std::streamsize n) calls unnecessary underflow

[Bug libstdc++/42857] std::istream::ignore(std::streamsize n) calls unnecessary underflow

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42857

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |claytongdavis at gmail dot
                   |                            |com

--- Comment #3 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-12-22 00:55:04 UTC ---
*** Bug 51651 has been marked as a duplicate of this bug. ***

[Bug libstdc++/42857] std::istream::ignore(std::streamsize n) calls unnecessary underflow

[bugs at mm dot beanwood.com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42857

Andrew Ayer <bugs at mm dot beanwood.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bugs at mm dot beanwood.com

--- Comment #4 from Andrew Ayer <bugs at mm dot beanwood.com> 2013-02-24 22:05:26 UTC ---
(In reply to comment #2)
> I want to understand how critical this additional underflow is,
> performance-wise

There are also correctness implications: what if you're trying to ignore
all the bytes you know are available to be read without blocking?  Then
the extra underflow blocks, possibly forever if the program is prevented
from taking action that would result in the underflow completing.
I got bitten by this because I was trying to ignore data from a socket.

Note that the standard has similar language for read(char_type*,
streamsize) ("either of the following conditions") but the library in
effect checks for n characters being reached before checking for eof:
if you read() exactly the number of characters left in the stream,
eofbit is not set.  So the library isn't currently consistent across
all the various istream functions.

I would argue that the current behavior of ignore() is actually contrary
to the standard.  If the standard says "until any" then ignore()
needs to terminate when n characters have been extracted.  But if it
first checks for eof and blocks forever, then it doesn't terminate.
Perhaps this is why getline explicitly mentions the order: in that case
the standard really does want eof to be checked first.

[Bug libstdc++/42857] std::istream::ignore(std::streamsize n) calls unnecessary underflow

[cubbi at cubbi dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42857

Sergey Zubkov <cubbi at cubbi dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |cubbi at cubbi dot org

--- Comment #5 from Sergey Zubkov <cubbi at cubbi dot org> ---
Looking at existing implementations, libstdc++ is the odd one out: LLVM libc++,
IBM XL C++, Microsoft Visual Studio, and Oracle libCstd and stlport4 all let me
ignore exactly the number of characters requested, without an extra read from
the socket.

[Bug libstdc++/42857] std::istream::ignore(std::streamsize n) calls unnecessary underflow

[bugs at mm dot beanwood.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42857

--- Comment #6 from Andrew Ayer <bugs at mm dot beanwood.com> ---
Any word on if this will be fixed in GCC?  To summarize, GCC's current behavior
is wrong because:

* Underflowing after ignoring the requested number of bytes could block
forever, breaking applications.

* The standard says "characters are extracted until ANY of the following
occurs: ... n characters are extracted" (emphasis mine).  If ignore() blocks
forever after extracting n characters, it's a violation of the standard because
it doesn't terminate as is required.

* GCC's std::basic_istream::read() implementation does NOT underflow after
reading the requested number of bytes (even though the standard uses similar
language for read and ignore).  This inconsistent behavior is confusing as one
would expect read and ignore to behave equivalently.

* Other major C++ implementations don't underflow after ignoring the requested
number of bytes. (Thanks, Sergey, for checking this.)

I wrote about this in greater depth at [1] and also wrote my own non-member
ignore() which I'm using in place of basic_istream::ignore() [2] which others
may find useful until this bug is fixed.

[1]
https://www.agwa.name/blog/post/gccs_implementation_of_basicistreamignore_is_broken

[2]
https://www.agwa.name/blog/post/gccs_implementation_of_basicistreamignore_is_broken/media/ignore.h

[Bug libstdc++/42857] std::istream::ignore(std::streamsize n) calls unnecessary underflow

[ncm at cantrip dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42857

--- Comment #7 from ncm at cantrip dot org ---
This bug appears not to manifest in g++-10.

[Bug libstdc++/42857] std::istream::ignore(std::streamsize n) calls unnecessary underflow

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42857

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Probably changed by one of the patches for PR 94749 or PR 96161, although I
still see two reads for the first example.

[Bug libstdc++/42857] std::istream::ignore(std::streamsize n) calls unnecessary underflow

[ncm at cantrip dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42857

--- Comment #9 from ncm at cantrip dot org ---
(In reply to Jonathan Wakely from comment #8)
> Probably changed by one of the patches for PR 94749 or PR 96161, although I
> still see two reads for the first example.

Thank you, I was mistaken. This bug is still present in g++-10.

[Bug libstdc++/42857] std::istream::ignore(std::streamsize n) calls unnecessary underflow

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42857

--- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Untested patch:

--- a/libstdc++-v3/src/c++98/compatibility.cc
+++ b/libstdc++-v3/src/c++98/compatibility.cc
@@ -88,7 +88,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
                        {
                          __sb->__safe_gbump(__size);
                          _M_gcount += __size;
-                         __c = __sb->sgetc();
+                         if (_M_gcount < __n
+                             || __n ==
__gnu_cxx::__numeric_traits<streamsize>::__max)
+                           __c = __sb->sgetc();
                        }
                      else
                        {


A similar change would be needed in the other versions of this function (the
wistream specialization, and the generic version).

[Bug libstdc++/42857] std::istream::ignore(std::streamsize n) calls unnecessary underflow

[paolo dot carlini at oracle dot com]

------- Comment #2 from paolo dot carlini at oracle dot com  2010-09-20 12:15 -------
I was having a second look to this issue, and noticed something more which I
missed the first time: the Standard, *only* in the case of getline(char_type*,
streamsize, char_type) explicitly says "These conditions are tested in the
order shown.". In my opinion that means that the get and ignore overloads using
the famous "any of the following occurs" can in principle check the conditions
in *any* implementation defined order and being conforming. Now, we have a case
here where we have an additional underflow because in our implementation we
uniformly insist on always checking whether end-of-file occurs in the sequence,
thus setting eofbit (besides the special case of ignore(), as already noticed)
in that case, like the above mentioned getline does, for example. I want to
understand how critical this additional underflow is, performance-wise, which,
as far as I can see normally can be triggered only by passing in_avail to
ignore, because otherwise, frankly, I find our consistent implementation
defined behavior across the various get, getline, ignore overloads pretty nice.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42857

[Bug libstdc++/42857] std::istream::ignore(std::streamsize n) calls unnecessary underflow

[paolo dot carlini at oracle dot com]

------- Comment #1 from paolo dot carlini at oracle dot com  2010-01-27 21:09 -------
Putting aside the strange inconsistency of the second example, which could be
easily fixed, and probably should anyway (we have an overload corresponding to
n == 1 which calls sbumpc and should probably call snextc instead for a
consistent behavior), I think the issue boils down to a very old interpretation
issue in these sections of the standard, where it uses wording of the form "any
of the following occurs" (the same happens for get, getline, the issue isn't
limited to ignore) and it's not clear at all whether, when the target n is
reached, thus n characters are extracted, the second termination condition,
which involves checking for end-of-file, is still relevant or not, or, in
principle could even have been computed first, before checking the value of n. 

In short, I understand your request as a request of evaluating the conditions
exactly in the order written in the Standard, and as soon as one holds, the
following ones simply ignored. It's not what we have been doing, uniformly
(modulo the buglet above) across this area, but indeed makes sense. Note
however that a rather noticeable consequence of your interpretation is that
ignoring exactly to end-of-line would not set the eofbit anymore in the stream,
because we would not do getc on it. Now instead we uniformly set eofbit when
it's actually the case (and setting it could even throw, a very noticeable
behavior)

Thus, I believe that, besides the buglet above, we should give this issue much
more thought, likely it's even too late for 4.5.0, and I'd like to involve
Nathan in the discussion, I'm pretty sure I learned from him, some time ago,
about the tricky point of the "any of the following occurs"-type
specifications.


-- 

paolo dot carlini at oracle dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ncm at cantrip dot org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42857