This project is read-only.
1
Vote

SafeInt<int64_t> v = pow(2.0, 63.0) doesn't throw

description

The following code produces an overflow but doesn't throw:
SafeInt<int64_t> v = pow(2.0, 63.0);
For an explanation, see http://stackoverflow.com/a/30424410/1956010

comments

dcleblanc wrote Dec 2, 2016 at 12:24 AM

My apologies for not noticing this earlier. Here's the problem - as noted in the stackoverflow comment INT_MAX and INT_MIN (for 64-bit) don't have exact representations. The double returned by pow is this:

9.2233720368547758e+18

In this code - SafeCastHelper < T, U, CastFromFloat > we have:
    if( u <= (U)IntTraits< T >::maxInt &&
        u >= (U)IntTraits< T >::minInt )
which succeeds. Now if we look at IntTraits< T >::maxInt, which is 0x7fffffffffffffff, that's

9223372036854775807

Notice that the double has truncated the '07' off the end because of precision loss. This means that you're feeding a value in that's actually less than the range of an int64. To be clear, you're exercising an untested corner case - the support for floating point is really just a best effort.

When we perform the test, we cast back to double, which could put us in trouble - we have this -
u <= (U)IntTraits< T >::maxInt

But it tests out as equal, and so we do what seems to be the right thing, and then we try to cast it to type T, and it comes out as negative! This is certainly not by design.

This then goes and causes the following assembly:

00007FF7F2B617DE cvttsd2si rax,mmword ptr [u]
00007FF7F2B617E7 mov rcx,qword ptr [t]
00007FF7F2B617EE mov qword ptr [rcx],rax

So it looks to me like the cvttsd2si instruction is not behaving as one would expect - it is documented as truncating, but it is not documented as changing sign. If you go bigger than this exact value, then it behaves as expected, and will throw.

Your suggestion of doing this:

if (!(my_double >= -9223372036854775808.0 // -2^63
&& my_double < 9223372036854775808.0) // 2^63

is a good one, and I'll update the class to use that type of comparison to avoid the problem.