bpo-39277, pytime: Fix overflow check on double to int cast by vstinner · Pull Request #17933 · python/cpython
Fix time.sleep() to properly detect integer overflow when converting a floating-point number of seconds to an integer.
| /* Check if the floating-point number v (double) would overflow when casted to | ||
| * the integral type 'type'. | ||
| * | ||
| * Test (double)type_min(type) <= v <= (double)type_max(type) where v is a |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The two conditions "v would overflow when cast to 'type'" and "type_MIN <= v <= type_MAX" are not equivalent. Which one do you want to test here?
For example, assuming a 32-bit int type: type_MAX would be 2147483647. If v = 2147483647.5 (which is exactly representable in IEEE 754 binary64 floating-point), then it satisfies the first condition - it doesn't overflow when cast to int - but not the second.
| * | ||
| * In short, nextafter((double)x, 0.0) rounds the integer x towards zero. */ | ||
| #define _Py_DoubleInIntegralTypeRange(type, v) \ | ||
| (nextafter((double)_Py_IntegralTypeMin(type), 0.0) <= v \ |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, but this isn't right.
For a 32-bit signed integer type, this condition becomes:
-2147483647.9999998 <= v <= 2147483646.9999998
but what we actually want is:
-2147483649.0 < v < 2147483648.0
or if we want to use <= and express the limits in terms of doubles:
-2147483648.9999995 <= v <= 2147483647.9999998
For a 64-bit signed integer type, this condition becomes:
-9223372036854774784.0 <= v <= 9223372036854774784.0
but the actual limits we need are
-9223372036854775808.0 <= v <= 9223372036854774784.0
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested alternative approach: given a double v that we want to safely convert to an int (say), first use modf to extract the fractional part fv and the integer part iv of v (both fv and iv are still doubles at this point). We can just ignore the fractional part fv: it doesn't affect whether we can safely cast to int or not. Now for iv, check whether it satisfies (double)INT_MIN <= iv and iv < -(double)INT_MIN. Note that INT_MIN is (always in practice, even though not guaranteed by the standard) the negation of a power of two, so the conversion to double doesn't change the value and we're effectively checking whether iv lies in the half-open interval [INT_MIN, -INT_MIN). Since iv is an integer, that's equivalent to checking whether iv lies in the closed interval [INT_MIN, -INT_MIN-1]. But making all our usual assumptions about integer representation (two's complement, no trap representation, no padding bits, etc.), -INT_MIN-1 is the same as INT_MAX, so we're effectively checking that the integer part of v is representable as an int, which is exactly what we want to be checking.
Similarly for long, long long, etc.
A safe way to do what you want is:
(1) Take the integral part of the double v that you want to check, for example using modf, still represented as a double. (Note that the integral part of an IEEE 754 floating-point number is always exactly representable in the same format.)
(2) Check that (double)TYPE_MIN <= int_part_of_v < (double)(TYPE_MAX + 1) (using a mixture of C casts and Python chained comparisons here, but you know what I mean)
Computing TYPE_MAX + 1 while avoiding overflow may take some type-specific effort.
The reason that step (2) is safe is that, making the fairly safe assumption of 2s complement (no trap representation, etc., etc.) for signed integers, TYPE_MIN and TYPE_MAX + 1 will always be exactly representable as a double (well, assuming that we're not dealing with 1024-bit integers, that is). But TYPE_MAX can't be assumed to be exactly representable as a double - whether it is or not will depend on the width of the type being tested.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't the right way to check for convertibility; see the other comments I made on the PR.
When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.
And if you don't make the requested changes, you will be put in the comfy chair!
I failed finding time to finish the PR. I prefer to abandon it.