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. Fix: --- include/bits/ios_base.h +++ include/bits/ios_base.h @@ -87,7 +87,7 @@ inline _GLIBCXX_CONSTEXPR _Ios_Fmtflags operator~(_Ios_Fmtflags __a) - { return _Ios_Fmtflags(~static_cast<int>(__a)); } + { return _Ios_Fmtflags(static_cast<int>(__a) ^ static_cast<int>(_S_ios_fmtflags_end - 1)); } inline const _Ios_Fmtflags& operator|=(_Ios_Fmtflags& __a, _Ios_Fmtflags __b) @@ -127,7 +127,7 @@ inline _GLIBCXX_CONSTEXPR _Ios_Openmode operator~(_Ios_Openmode __a) - { return _Ios_Openmode(~static_cast<int>(__a)); } + { return _Ios_Openmode(static_cast<int>(__a) ^ static_cast<int>(_S_ios_openmode_end - 1)); } inline const _Ios_Openmode& operator|=(_Ios_Openmode& __a, _Ios_Openmode __b) @@ -165,7 +165,7 @@ inline _GLIBCXX_CONSTEXPR _Ios_Iostate operator~(_Ios_Iostate __a) - { return _Ios_Iostate(~static_cast<int>(__a)); } + { return _Ios_Iostate(static_cast<int>(__a) ^ static_cast<int>(_S_ios_iostate_end - 1)); } inline const _Ios_Iostate& operator|=(_Ios_Iostate& __a, _Ios_Iostate __b)
Thanks, Richard
Crazy nobody noticed so far. So... Is this something we can commit right now? Assuming there aren't ABI implications, I would be in favor. I'm also adding Benjamin in CC, I think I wasn't contributing yet, when this code went in ;)
I'm going to regression test the fix.
I'm wondering: before doing anything in v3, is this a C++11 issue? Because in 17.5.2.1.3 I see a fixed underlying type but otherwise I see exactly ~static_cast<int_type>(X) like in v3?!?
[dcl.enum]/7 "For an enumeration whose underlying type is fixed, the values of the enumeration are the values of the underlying type." Because the underlying type in 17.5.2.1.3 is fixed those operations cannot create a value outside the range of the enumeration type. See "The underlying type should be fixed" in http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2010/n3110.html
Oh, I was missing that, thanks. Now, I don't know if we should really try to fix this now after so many years. I'm tempted to just leave it alone until we break the ABI, unless we are really sure that the value returned by the amended operator can intetoperate with old code and viceversa. Do you jnow in practice which values the current operator is returning for GCC?
We should double check but I'm pretty sure that *in practice* *for GCC* things are Ok, because the sizeof of these enums is 4 (and in practice the systems we support have sizeof int <= 4). If we don't have a concrete testcase, we definitely want to change this later.
I think since 4.6 the default behaviour (i.e. without -fstrict-enums) is safe. With -fstrict-enums (or in releases before 4.6) the optimisers can assume that no invalid values are ever produced, so Enum(~static_cast<int>(e)) has undefined behaviour as Richard says.
Sure, sure. If we really want to support -fstrict-enums, I'm afraid we are going to open a big can of worms... Still, are you sure it causes problems *here*? I'm asking because we have the final 1L << 16.
*** Bug 66624 has been marked as a duplicate of this bug. ***
Now that sanitisers are complaining about this we should really fix it.
Richard's patch changes the values returned by operator~ which is not desirable. To fix the underlying type to int in C++03 (so that all values of int will be valid values of the enumeration type) we can do: --- a/libstdc++-v3/include/bits/ios_base.h +++ b/libstdc++-v3/include/bits/ios_base.h @@ -74,7 +74,9 _S_adjustfield = _S_left | _S_right | _S_internal, _S_basefield = _S_dec | _S_oct | _S_hex, _S_floatfield = _S_scientific | _S_fixed, - _S_ios_fmtflags_end = 1L << 16 + _S_ios_fmtflags_end = 1L << 16, + _S_ios_fmtflags_max = __INT_MAX__, + _S_ios_fmtflags_min = ~(int)__INT_MAX__ }; inline _GLIBCXX_CONSTEXPR _Ios_Fmtflags @@ -114,7 +116,9 _S_in = 1L << 3, _S_out = 1L << 4, _S_trunc = 1L << 5, - _S_ios_openmode_end = 1L << 16 + _S_ios_openmode_end = 1L << 16, + _S_ios_openmode_max = __INT_MAX__, + _S_ios_openmode_min = ~(int)__INT_MAX__ }; inline _GLIBCXX_CONSTEXPR _Ios_Openmode @@ -152,7 +156,9 _S_badbit = 1L << 0, _S_eofbit = 1L << 1, _S_failbit = 1L << 2, - _S_ios_iostate_end = 1L << 16 + _S_ios_iostate_end = 1L << 16, + _S_ios_iostate_max = __INT_MAX__, + _S_ios_iostate_min = ~(int)__INT_MAX__ }; inline _GLIBCXX_CONSTEXPR _Ios_Iostate
N.B. we could also get rid of the _S_ios_xxx_end enumerators, but that would break any code which (foolishly) refers to them, e.g. to suppress Clang's -Wswitch warnings. My suggestion assumes that __INT_MAX__ > (1 << 16), i.e. the compiler really will choose int as the underlying type, but I think that's OK.
Author: redi Date: Thu Nov 12 17:08:42 2015 New Revision: 230267 URL: https://gcc.gnu.org/viewcvs?rev=230267&root=gcc&view=rev Log: Extend valid values of iostream bitmask types PR libstdc++/56158 * include/bits/ios_base.h (_Ios_Fmtflags, _Ios_Openmode, _Ios_Iostate): Define enumerators to ensure all values of type int are valid values of the enumeration type. * testsuite/27_io/ios_base/types/fmtflags/case_label.cc: Add new cases. * testsuite/27_io/ios_base/types/iostate/case_label.cc: Likewise. * testsuite/27_io/ios_base/types/openmode/case_label.cc: Likewise. Modified: trunk/libstdc++-v3/ChangeLog trunk/libstdc++-v3/include/bits/ios_base.h trunk/libstdc++-v3/testsuite/27_io/ios_base/types/fmtflags/case_label.cc trunk/libstdc++-v3/testsuite/27_io/ios_base/types/iostate/case_label.cc trunk/libstdc++-v3/testsuite/27_io/ios_base/types/openmode/case_label.cc
Fixed on trunk, probably worth backporting.
Author: redi Date: Wed Nov 25 16:11:40 2015 New Revision: 230884 URL: https://gcc.gnu.org/viewcvs?rev=230884&root=gcc&view=rev Log: Extend valid values of iostream bitmask types Backport from mainline 2015-11-12 Jonathan Wakely <jwakely@redhat.com> PR libstdc++/56158 * include/bits/ios_base.h (_Ios_Fmtflags, _Ios_Openmode, _Ios_Iostate): Define enumerators to ensure all values of type int are valid values of the enumeration type. * testsuite/27_io/ios_base/types/fmtflags/case_label.cc: Add new cases. * testsuite/27_io/ios_base/types/iostate/case_label.cc: Likewise. * testsuite/27_io/ios_base/types/openmode/case_label.cc: Likewise. Modified: branches/gcc-5-branch/libstdc++-v3/ChangeLog branches/gcc-5-branch/libstdc++-v3/include/bits/ios_base.h branches/gcc-5-branch/libstdc++-v3/testsuite/27_io/ios_base/types/fmtflags/case_label.cc branches/gcc-5-branch/libstdc++-v3/testsuite/27_io/ios_base/types/iostate/case_label.cc branches/gcc-5-branch/libstdc++-v3/testsuite/27_io/ios_base/types/openmode/case_label.cc
Fixed for 5.3 and later.
*** Bug 80282 has been marked as a duplicate of this bug. ***