Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-39277, pytime: Fix overflow check on double to int cast #17933

Closed
wants to merge 1 commit into from
Closed

bpo-39277, pytime: Fix overflow check on double to int cast #17933

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 22 additions & 5 deletions Include/pymath.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,10 +221,27 @@ PyAPI_FUNC(void) _Py_set_387controlword(unsigned short);
#define _Py_IntegralTypeMax(type) ((_Py_IntegralTypeSigned(type)) ? (((((type)1 << (sizeof(type)*CHAR_BIT - 2)) - 1) << 1) + 1) : ~(type)0)
/* Return the minimum value of integral type *type*. */
#define _Py_IntegralTypeMin(type) ((_Py_IntegralTypeSigned(type)) ? -_Py_IntegralTypeMax(type) - 1 : 0)
/* Check whether *v* is in the range of integral type *type*. This is most
* useful if *v* is floating-point, since demoting a floating-point *v* to an
* integral type that cannot represent *v*'s integral part is undefined
* behavior. */
#define _Py_InIntegralTypeRange(type, v) (_Py_IntegralTypeMin(type) <= v && v <= _Py_IntegralTypeMax(type))

/* 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
Copy link
Member

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.

* double, and type_min() and type_max() integers are rounded towards zero when
* casted to a double.
*
* (double)int cast rounds to nearest with ties going to nearest even integer
* (ROUND_HALF_EVEN). Use nextafter() to round towards zeros (ROUND_DOWN).
*
* For example, _Py_IntegralTypeMax(int64_t)=2**63-1 casted to double gives
* 2**63 which is greater than 2**63-1. The problem is that "v <= 2**63" fails
* to detect that v will overflow when casted to int64_t.
* nextafter((double)(2**63-1), 0.0) gives the floating-point number 2**63-1024
* which is less than or equal to the integer 2**63-1 and so can be used to
* test that v would overflow.
*
* 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 \
Copy link
Member

@mdickinson mdickinson May 29, 2020 •
edited
Loading

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

Copy link
Member

@mdickinson mdickinson Jul 22, 2021 •
edited
Loading

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.

&& v <= nextafter((double)_Py_IntegralTypeMax(type), 0.0))

#endif /* Py_PYMATH_H */
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Library/2020-01-10-10-45-08.bpo-39277.A2qvZ2.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix :func:`time.sleep` to properly detect integer overflow when converting a
floating-point number of seconds to an integer.
6 changes: 3 additions & 3 deletions Python/pytime.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ _PyTime_DoubleToDenominator(double d, time_t *sec, long *numerator,
}
assert(0.0 <= floatpart && floatpart < denominator);

if (!_Py_InIntegralTypeRange(time_t, intpart)) {
if (!_Py_DoubleInIntegralTypeRange(time_t, intpart)) {
error_time_t_overflow();
return -1;
}
Expand Down Expand Up @@ -204,7 +204,7 @@ _PyTime_ObjectToTime_t(PyObject *obj, time_t *sec, _PyTime_round_t round)
d = _PyTime_Round(d, round);
(void)modf(d, &intpart);

if (!_Py_InIntegralTypeRange(time_t, intpart)) {
if (!_Py_DoubleInIntegralTypeRange(time_t, intpart)) {
error_time_t_overflow();
return -1;
}
Expand Down Expand Up @@ -389,7 +389,7 @@ _PyTime_FromDouble(_PyTime_t *t, double value, _PyTime_round_t round,
d *= (double)unit_to_ns;
d = _PyTime_Round(d, round);

if (!_Py_InIntegralTypeRange(_PyTime_t, d)) {
if (!_Py_DoubleInIntegralTypeRange(_PyTime_t, d)) {
_PyTime_overflow();
return -1;
}
Expand Down