Overflow when casting from double to time_t, and_PyTime_t.

[enedil]

BPO	34423
Nosy	@malemburg, @mdickinson, @abalkin, @vstinner, @pganssle, @enedil
PRs	bpo-34423: Fix check for overflow when casting from a double to integral types. #8802

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2018-08-17.21:56:49.149>
labels = ['interpreter-core', '3.8', 'type-bug', '3.7']
title = 'Overflow when casting from double to time_t, and_PyTime_t.'
updated_at = <Date 2019-02-20.11:13:50.802>
user = 'https://github.com/enedil'

bugs.python.org fields:

activity = <Date 2019-02-20.11:13:50.802>
actor = 'vstinner'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2018-08-17.21:56:49.149>
creator = 'enedil'
dependencies = []
files = []
hgrepos = []
issue_num = 34423
keywords = ['patch']
message_count = 5.0
messages = ['323676', '323929', '323930', '323959', '336072']
nosy_count = 6.0
nosy_names = ['lemburg', 'mark.dickinson', 'belopolsky', 'vstinner', 'p-ganssle', 'enedil']
pr_nums = ['8802']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue34423'
versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']

[enedil]

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.

[vstinner]

It seems like a regression introduced by:

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

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

[vstinner]

I'm not sure that the pytime.c change in commit a853a8b 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))

[enedil]

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.

[vstinner]

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

[iritkatriel]

Closed along with the PR.