Fixing undefined behavior introduced by the incorrect use of comma operator with the conditional operator. The middle expression in a conditional operator between the ? and : is implicitly parenthesized but the end expression is not. Since the comma operator as he lowest precendence( http://en.cppreference.com/w/cpp/language/operator_precedence) this means the conditional operator will be evaluated first and then the expression on the right hand side of the comma operator will be evaluated. This leads to undefined behavior because the last member of the union being updated will not be the member that will be used next which is strictly undefined in C++ although gcc and clang aloow this type punning as an extension but is clearly not portable behavior nor was this the intended behavior. instead of parenthesising the end expression I choose to use an if/else which is not subject to such easy to miss precdence issues. This Coliru live code demonstrates the bug with simple example: http://coliru.stacked-crooked.com/a/1041aaa8380feeaa the code also demonstrates the using the right warning flags gcc will generate a warning for this code.

This commit is contained in:
Shafik Yaghmour 2014-09-18 14:29:11 -04:00
parent 847a7852e5
commit d8f366daf2
2 changed files with 119 additions and 11 deletions

View File

@ -115,16 +115,61 @@ inline packer<Stream>& operator<< (packer<Stream>& o, const type::fix_uint64& v)
inline void operator<< (object& o, type::fix_int8 v)
{ v.get() < 0 ? o.type = type::NEGATIVE_INTEGER, o.via.i64 = v.get() : o.type = type::POSITIVE_INTEGER, o.via.u64 = v.get(); }
{
if ( v.get() < 0 )
{
o.type = type::NEGATIVE_INTEGER ;
o.via.i64 = v.get() ;
}
else
{
o.type = type::POSITIVE_INTEGER ;
o.via.u64 = v.get() ;
}
}
inline void operator<< (object& o, type::fix_int16 v)
{ v.get() < 0 ? o.type = type::NEGATIVE_INTEGER, o.via.i64 = v.get() : o.type = type::POSITIVE_INTEGER, o.via.u64 = v.get(); }
{
if ( v.get() < 0 )
{
o.type = type::NEGATIVE_INTEGER ;
o.via.i64 = v.get() ;
}
else
{
o.type = type::POSITIVE_INTEGER ;
o.via.u64 = v.get() ;
}
}
inline void operator<< (object& o, type::fix_int32 v)
{ v.get() < 0 ? o.type = type::NEGATIVE_INTEGER, o.via.i64 = v.get() : o.type = type::POSITIVE_INTEGER, o.via.u64 = v.get(); }
{
if ( v.get() < 0 )
{
o.type = type::NEGATIVE_INTEGER ;
o.via.i64 = v.get() ;
}
else
{
o.type = type::POSITIVE_INTEGER ;
o.via.u64 = v.get() ;
}
}
inline void operator<< (object& o, type::fix_int64 v)
{ v.get() < 0 ? o.type = type::NEGATIVE_INTEGER, o.via.i64 = v.get() : o.type = type::POSITIVE_INTEGER, o.via.u64 = v.get(); }
{
if( v.get() < 0 )
{
o.type = type::NEGATIVE_INTEGER ;
o.via.i64 = v.get() ;
}
else
{
o.type = type::POSITIVE_INTEGER ;
o.via.u64 = v.get() ;
}
}
inline void operator<< (object& o, type::fix_uint8 v)

View File

@ -98,8 +98,16 @@ namespace detail {
template <>
struct object_char_sign<true> {
static inline void make(object& o, char v) {
v < 0 ? o.type = type::NEGATIVE_INTEGER, o.via.i64 = v
: o.type = type::POSITIVE_INTEGER, o.via.u64 = v;
if( v < 0 )
{
o.type = type::NEGATIVE_INTEGER ;
o.via.i64 = v ;
}
else
{
o.type = type::POSITIVE_INTEGER ;
o.via.u64 = v ;
}
}
};
@ -203,19 +211,74 @@ inline void operator<< (object& o, char v)
inline void operator<< (object& o, signed char v)
{ v < 0 ? o.type = type::NEGATIVE_INTEGER, o.via.i64 = v : o.type = type::POSITIVE_INTEGER, o.via.u64 = v; }
{
if( v < 0 )
{
o.type = type::NEGATIVE_INTEGER ;
o.via.i64 = v;
}
else
{
o.type = type::POSITIVE_INTEGER ;
o.via.u64 = v ;
}
}
inline void operator<< (object& o, signed short v)
{ v < 0 ? o.type = type::NEGATIVE_INTEGER, o.via.i64 = v : o.type = type::POSITIVE_INTEGER, o.via.u64 = v; }
{
if(v < 0 )
{
o.type = type::NEGATIVE_INTEGER ;
o.via.i64 = v ;
}
else
{
o.type = type::POSITIVE_INTEGER ;
o.via.u64 = v;
}
}
inline void operator<< (object& o, signed int v)
{ v < 0 ? o.type = type::NEGATIVE_INTEGER, o.via.i64 = v : o.type = type::POSITIVE_INTEGER, o.via.u64 = v; }
{
if( v < 0 )
{
o.type = type::NEGATIVE_INTEGER ;
o.via.i64 = v ;
}
else
{
o.type = type::POSITIVE_INTEGER ;
o.via.u64 = v ;
}
}
inline void operator<< (object& o, signed long v)
{ v < 0 ? o.type = type::NEGATIVE_INTEGER, o.via.i64 = v : o.type = type::POSITIVE_INTEGER, o.via.u64 = v; }
{
if( v < 0 )
{
o.type = type::NEGATIVE_INTEGER ;
o.via.i64 = v ;
}
else
{
o.type = type::POSITIVE_INTEGER ;
o.via.u64 = v ;
}
}
inline void operator<< (object& o, signed long long v)
{ v < 0 ? o.type = type::NEGATIVE_INTEGER, o.via.i64 = v : o.type = type::POSITIVE_INTEGER, o.via.u64 = v; }
{
if( v < 0 )
{
o.type = type::NEGATIVE_INTEGER ;
o.via.i64 = v ;
}
else
{
o.type = type::POSITIVE_INTEGER ;
o.via.u64 = v ;
}
}
inline void operator<< (object& o, unsigned char v)