Issue 34423: Overflow when casting from double to time_t, and_PyTime_t.

Messages (5) msg323676 - (view) Author: Michał Radwański (enedil) * Date: 2018-08-17 21:56

Code triggering bug:

import time
time.sleep(2**63 / 10**9)

Result:

ValueError: sleep length must be non-negative

The problem is with the macro that checks the range of provided double:

Line 228 of Include/pymath.h:
#define _Py_InIntegralTypeRange(type, v) (_Py_IntegralTypeMin(type) <= v && v <= _Py_IntegralTypeMax(type))

The type here is _PyTime_t, which is a typedef to int64_t. 

Proposed solution is to change <= into <, since float(2**63-1) == float(2**63), and that means that none double can ever be equal to 2**63-1.

msg323929 - (view) Author: STINNER Victor (vstinner) * Date: 2018-08-23 08:43

It seems like a regression introduced by:

commit a853a8ba7850381d49b284295dd6f0dc491dbe44
Author: Benjamin Peterson <benjamin@python.org>
Date:   Thu Sep 7 11:13:59 2017 -0700

    bpo-31373: fix undefined floating-point demotions (#3396)

msg323930 - (view) Author: STINNER Victor (vstinner) * Date: 2018-08-23 08:52

I'm not sure that the pytime.c change in commit a853a8ba7850381d49b284295dd6f0dc491dbe44 is correct:

diff --git a/Python/pytime.c b/Python/pytime.c
index 8979adc219..f3c913226c 100644
--- a/Python/pytime.c
+++ b/Python/pytime.c
@@ -276,7 +278,6 @@ static int
 _PyTime_FromFloatObject(_PyTime_t *t, double value, _PyTime_round_t round,
                         long unit_to_ns)
 {
-    double err;
     /* volatile avoids optimization changing how numbers are rounded */
     volatile double d;
 
@@ -285,12 +286,11 @@ _PyTime_FromFloatObject(_PyTime_t *t, double value, _PyTime_round_t round,
     d *= (double)unit_to_ns;
     d = _PyTime_Round(d, round);
 
-    *t = (_PyTime_t)d;
-    err = d - (double)*t;
-    if (fabs(err) >= 1.0) {
+    if (!_Py_InIntegralTypeRange(_PyTime_t, d)) {
         _PyTime_overflow();
         return -1;
     }
+    *t = (_PyTime_t)d;
     return 0;
 }


I don't think that modifying _Py_InIntegralTypeRange() macro to replace "<=" with "<" is correct, since this bug is very specific to pytime.c, when _Py_InIntegralTypeRange() is used with a double. The PR proposes:

#define _Py_InIntegralTypeRange(type, v) (_Py_IntegralTypeMin(type) <= v && v < _Py_IntegralTypeMax(type))

msg323959 - (view) Author: Michał Radwański (enedil) * Date: 2018-08-23 16:19

Although you're right - this issue is specific to pytime.c, when _Py_InIntegralTypeRange() is used with a double, it is actually true that _Py_InIntegralTypeRange() is used with double, in pytime.c only (as a quick recursive grep discovers).

Perhaps the macro should be renamed not to cause confusion (include note about floating point, or define it as a function).

I don't have good idea on how this issue could be resolved otherwise.

msg336072 - (view) Author: STINNER Victor (vstinner) * Date: 2019-02-20 11:13

See also bpo-36048: "Deprecate implicit truncating when convert Python numbers to C integers: use __index__, not __int__".

History Date User Action Args 2022-04-11 14:59:04adminsetgithub: 78604 2019-02-20 11:13:50vstinnersetmessages: + msg336072 2018-08-23 18:32:42p-gansslesetnosy: + p-ganssle
2018-08-23 16:19:02enedilsetmessages: + msg323959 2018-08-23 08:52:43vstinnersetmessages: + msg323930 2018-08-23 08:43:19vstinnersetmessages: + msg323929 2018-08-20 05:51:58serhiy.storchakasetnosy: + mark.dickinson
2018-08-18 00:35:07enedilsetkeywords: + patch
stage: patch review
pull_requests: + pull_request8278 2018-08-17 21:56:49enedilcreate