libc++ operator>>(istream&, std::string &s) not throwing exception when failbit is set and failbit exceptions enabled

[Quuxplusone]

Bugzilla Link	PR21586
Status	RESOLVED FIXED
Importance	P normal
Reported by	Seth (seth.cantrell@gmail.com)
Reported on	2014-11-16 17:56:23 -0800
Last modified on	2019-04-02 14:46:21 -0700
Version	unspecified
Hardware	All All
CC	francisco@oblita.com, hfinkel@anl.gov, ldionne@apple.com, llvm-bugs@lists.llvm.org, mclow.lists@gmail.com
Fixed by commit(s)	rL357531
Attachments
Blocks
Blocked by
See also

I have a simple program which I believe should be throwing an exception:

    // http://coliru.stacked-crooked.com/a/464c105f4ec53104

    #include <sstream>
    #include <string>

    int main() {
        std::stringstream ss("");
        ss.exceptions(std::ios::failbit);
        std::string s;
        ss >> s; // not throwing an exception under libc++
    }

With libstdc++ and VC++'s this program does indeed throw an exception. However
with libc++ it does not. Interestingly a direct check of the streams failbit
shows that libc++'s implementation is setting the failbit, as are the other
implementations, but for some reason this does not cause an exception to be
thrown when the streaming operator fails.

    // http://coliru.stacked-crooked.com/a/e4a649ee145b0475

    #include <sstream>
    #include <string>
    #include <iostream>

    int main() {
        std::stringstream ss("");
        std::string s;
        ss >> s;
        std::cout << ss.fail() << '\n'; // prints '1' for all tested implementations
    }

If failbit exceptions are enabled after the bit is set, the .exceptions()
member function throws as expected:

    // http://coliru.stacked-crooked.com/a/601cb5d7c5c2b156

    #include <sstream>
    #include <string>

    int main() {
        std::stringstream ss("");
        std::string s;
        ss >> s;
        ss.exceptions(std::ios::failbit); // throws on all tested implementations
    }

I don't know what version of libc++ is being used on coliru.stacked-
crooked.com, but I'm also reproducing this issue with r222085 /
9a1468f79ea23f14a258af094af06993fe72e3a9

[Quuxplusone]

This is what's happening.

when you try to read from the empty string, it fails. (Note that this is not just for strings, it works the same for int).

It calls use_facet<_Fp>(this->getloc()).get, which throws.

this exception is caught internally, and the catch clause sets the badbit on the stream, and checks the exception state, and if the state & badbit != 0, then rethrows the exception.

But, you say, I set exceptions! And you did. But you set "failbit", not "badbit", and so the exception does not get rethrown.

Now I'm off to read this (very old) part of the C++ standard to figure out what the correct behavior is in this case.

[Quuxplusone]

More. In section 27.7.2.2.1 [istream.formatted.reqmts], there's a general
description of how formatted input is handled:

    Each formatted input function begins execution by constructing an object of class sentry with the noskipws (second) argument false. If the sentry object returns true, when converted to a value of type bool, the function endeavors to obtain the requested input. If an exception is thrown during input then ios::badbit is turned on in *this’s error state. If (exceptions()&badbit) != 0 then the exception is rethrown. In any case, the formatted input function destroys the sentry object. If no exception has been thrown, it returns *this.

That seems to match what libc++ is doing.

More as I discover it.

[Quuxplusone]

But, you say, I set exceptions! And you did. But you set "failbit", not "badbit", and so the exception does not get rethrown.

But what of the fact that the failbit is set when I check it explicitly? If exceptions are enabled for the failbit only, an exception should still be produced when that bit is eventually set as a result of the earlier 'badbit' error, right?

[Quuxplusone]

Well, sure. Failbit gets set because the operation failed -- that's in the stream state. (rdstate())

However, if you look at the section of the standard that I quoted, it says "If (exceptions()&badbit) != 0 then the exception is rethrown." - that's in exceptions().

This makes me think that there's more behavior specified, and I haven't found it yet.

[Quuxplusone]

Okay, coming at it from another direction:

> 21.4.8.9 Inserters and extractors
> template<class charT, class traits, class Allocator>
>     basic_istream<charT,traits>&
>          operator>>(basic_istream<charT,traits>& is,
>                basic_string<charT,traits,Allocator>& str);

> If the function extracts no characters, it calls is.setstate(ios::failbit),
which may throw ios_base::failure (27.5.5.4).

> 27.5.5.4 basic_ios flags functions
> void setstate(iostate state);
> Effects: Calls clear(rdstate() | state) (which may throw basic_ios::failure
(27.5.3.1.1)).

> void clear(iostate state = goodbit);
> Effects: If ((state | (rdbuf() ? goodbit : badbit)) & exceptions()) == 0,
returns. Otherwise, the function throws an object fail of class
basic_ios::failure (27.5.3.1.1), constructed with implementation-defined
argument values.

So since ios::failbit has been passed through to here we get a non-zero result
from the bitwise expression and so the requirement to throw a
basic_ios::failure object is triggered. Going back up the chain of member
requirements I don't believe anything is allowed to catch this exception and so
the spec does require my test case to throw a basic_ios::failure.

---

Here's an interesting fact: If instead of performing a failing io operation, we
instead just call .setstate(std::ios::failbit) on the stream then the expected
exception occurs: http://coliru.stacked-crooked.com/a/e93056957bb8adaf

It looks to me like the bug is that the implementation of operator>> is calling
setstate(ios::failbit), but it then catches that exception and then calls
__set_badbit_and_consider_rethrow. So the exception due to failbit is lost and
no other exception is thrown, because __set_badbit_and_consider_rethrow will
only check for badbit.

Also the issue is not specific to strings, or to empty streams. It seems as
though the extractors ignore failbits in general:

http://coliru.stacked-crooked.com/a/0d7aeff185624503
http://coliru.stacked-crooked.com/a/65dc0b6aab78f5fe

[Quuxplusone]

Additionally, libc++'s implementation does not seem to allow for any circumstances in which IO fails but does not cause the stream to go bad. It is certainly possible for extractors to fail without having triggered anything on the underlying stream causing it to go bad:

http://coliru.stacked-crooked.com/a/a875841c675b709f

[Quuxplusone]

I think we have a related bug with the unformated input operations. The
following test case fails:

    #include <sstream>
    #include <istream>
    #include <cassert>

    int main() {
        std::stringbuf sb("rrrrrrrrr");
        std::istream is(&sb);
        is.exceptions(std::ios::eofbit);

        bool threw = false;
        try {
            while (true) {
                is.get();
                if (is.eof())
                    break;
            }
        } catch (std::ios::failure const&) {
            threw = true;
        }

        assert(!is.bad());
        assert(is.fail());
        assert(is.eof());
        assert(threw);
    }

(live example: https://wandbox.org/permlink/PKV7Hu8iKfBphkEb)

I believe the bug in libc++ comes from the following in the specification:

    [30.7.4.3/1]: Exceptions thrown from basic_ios<>::clear() are not caught or rethrown.

combined with

    [30.7.4.3/4]: Effects: Behaves as an unformatted input function (as described above). After constructing a sentry object, extracts a character c, if one is available. Otherwise, the function calls setstate(failbit), which may throw ios_base::failure.

The problem is that libc++ currently does the following:

    template<class _CharT, class _Traits>
    typename basic_istream<_CharT, _Traits>::int_type
    basic_istream<_CharT, _Traits>::get()
    {
        __gc_ = 0;
        int_type __r = traits_type::eof();
        try
        {
            sentry __s(*this, true);
            if (__s)
            {
                __r = this->rdbuf()->sbumpc();
                if (traits_type::eq_int_type(__r, traits_type::eof()))
                   this->setstate(ios_base::failbit | ios_base::eofbit);
                else
                    __gc_ = 1;
            }
        }
        catch (...)
        {
            __rdstate_ |= badbit;
            if (__exceptions_ & badbit)
                throw;
        }
        return __r;
    }

What happens is that when we hit EOF, we properly call setstate(). setstate()
then throws an exception (because setstate() is basically calling clear()),
which we catch and only rethrow if badbit had been set. However, per the part
of [30.7.4.3/1] I quoted above, I think we're not allowed to catch that
exception. In other words, I believe the correct behavior would be:

    template<class _CharT, class _Traits>
    typename basic_istream<_CharT, _Traits>::int_type
    basic_istream<_CharT, _Traits>::get()
    {
        __gc_ = 0;
        int_type __r = traits_type::eof();
        try
        {
            sentry __s(*this, true);
            if (__s)
            {
                __r = this->rdbuf()->sbumpc();
                if (traits_type::eq_int_type(__r, traits_type::eof()))
                   this->setstate(ios_base::failbit | ios_base::eofbit);
                else
                    __gc_ = 1;
            }
        }
        catch (...)
        {
            __rdstate_ |= badbit;
            if (__exceptions_ & eofbit && this->eof())
                throw;
            if (__exceptions_ & failbit && this->fail())
                throw;
            if (__exceptions_ & badbit)
                throw;
        }
        return __r;
    }

We would effectively be catching and rethrowing exceptions thrown from
setstate(), which is apparently against the specification, but I don't see
another way of implementing it.

I believe both this bug and the original one files for formatted input
operations come from the fact that the wording is very unclear and hard to
follow. Should I file a LWG issue so we can talk about it with other
implementers and clarify what the intended behavior is? After the wording has
been fixed, we could proceed to fix this bug (if it's a bug).

[Quuxplusone]

Actually, it appears that my bug with unformatted input has already been the target of a LWG issue (LWG 61): https://cplusplus.github.io/LWG/issue61 . It appears that libc++ simply did not implement the issue resolution. The problem with formatted input operations is a different one, although it is related and should probably be the target of a LWG issue too.

[Quuxplusone]

Okay -- sorry for the spam, but I think I finally found what I wanted. Marshall, you can confirm this, but this bug (both my test case using unformatted operations and the OP's test case using formatted operations) is basically what is meant to be addressed by LWG 2349 (https://cplusplus.github.io/LWG/issue2349). If that is correct, I can prepare a patch that implements the proposed resolution.

[Quuxplusone]

_Bug 15949 has been marked as a duplicate of this bug._

[Quuxplusone]

Fixed in r357531.