Is this code really undefined, as Clang seems to indicate?

Question

I switched on -fsanitize=undefined on my project which uses Catch, the unit testing library. One line from Catch was signalled as causing undefined behaviour by this flag. I managed to make an isolated example:

#include <iomanip>
#include <sstream>

int main()
{
    std::ostringstream os; 
    os << "0x" << std::setfill('0') << std::hex;
}

Compiled with:

clang++ -fsanitize=undefined main.cpp

If I run this, the following print is given:

/usr/bin/../lib64/gcc/x86_64-unknown-linux-gnu/4.9.2/../../../../include/c++/4.9.2/bits/ios_base.h:96:24: runtime error: load of value 4294967221, which is not a valid value for type 'std::_Ios_Fmtflags'
/usr/bin/../lib64/gcc/x86_64-unknown-linux-gnu/4.9.2/../../../../include/c++/4.9.2/bits/ios_base.h:76:67: runtime error: load of value 4294967221, which is not a valid value for type 'std::_Ios_Fmtflags'

This happens for me on clang 3.6.0 and for a friend with clang 3.4-1ubuntu3. It does not happen for me on gcc version 4.9.2

So what is up here? Is this code actually bad, or is there something fishy going on on clang's end?

Theurich

Just os << std::hex; seems also to reproduce the issue. – Bein

#20618288 Maybe this? – Holsworth

I would love a more descriptive title but I am having a hard time coming up with one. – Pantaloon

Answer 1

This is a bug in libstdc++, from the cfe-dev mailing list thread with title -fsanitize=undefined and shared libraries says:

This is a bug in libstdc++. You will be able to work around it with a sanitizer blacklist file, once Will's patch for that lands, but for now, filtering them out manually is likely to be your best option.

Here's a patch to fix it; I'll be looking into pushing this to libstdc++ upstream in the next few days. [...]

As I noted to dyp in the comments it is not uncommon to see systems where clang uses libstdc++ as opposed to libc++ and if we test this on Coliru explicitly using libstdc++ via -stdlib=libstdc++ we indeed can reproduce the issue.

The following libstdc++ bug report: bad enum values computed by operator~ in ios_base.h covers this issue and says:

The overloaded operator~s defined for the enumerations in ios_base.h have the following form:

Enum operator~(Enum e) { return Enum(~static_cast<int>(e)); }

The ~ creates values outside the range of values of the enumeration type, so the cast back to the Enum type has an unspecified value (see [expr.static.cast]p10), and in practice it produces an Enum value outside the range of representable values for the Enum type, so behavior is undefined.

For reference [expr.static.cast]p10 says:

A value of integral or enumeration type can be explicitly converted to an enumeration type. The value is unchanged if the original value is within the range of the enumeration values (7.2). Otherwise, the resulting value is unspecified (and might not be in that range). A value of floating-point type can also be converted to an enumeration type. The resulting value is the same as converting the original value to the underlying type of the enumeration (4.9), and subsequently to the enumeration type.

and as hvd says this is formally unspecified behavior but Richard points out that in practice is ends up being undefined behavior.

T.C. points out this was changed from unspecified to undefined behavior by DR 1766: Values outside the range of the values of an enumeration:

Although issue 1094 clarified that the value of an expression of enumeration type might not be within the range of the values of the enumeration after a conversion to the enumeration type (see 5.2.9 [expr.static.cast] paragraph 10), the result is simply an unspecified value. This should probably be strengthened to produce undefined behavior, in light of the fact that undefined behavior makes an expression non-constant. See also 9.6 [class.bit] paragraph 4.

The new wording appears in the draft standard in N4431.

Pantaloon

Oh interesting indeed. Too bad filtering them out manually removes the possibility of using gdb to break on such errors to get a stacktrace for fixing them. – Theurich

The fact that the cast produces an unspecified value means the behaviour is defined. If the behaviour would be undefined, the standard would say the behaviour is undefined. (However, it's still not useful behaviour, so the code should still be changed.) – Tinytinya

@hvd well what Richard says is in practice it produces an Enum value outside the range of representable values for the Enum type, so behavior is undefined by the time the sanitizer sees it that distinction may not be available. – Pantaloon

@Theurich well presumably using libc++ as opposed to libsdc++ should fix this problem. – Pantaloon

@hvd This has been made completely undefined by CWG 1766. – Lewak