From 622ec0afd1a6f1a607a542a861fdd21237a16fb7 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Sat, 11 Feb 2023 13:12:14 +0000 Subject: [PATCH 1/3] Fix compiler warnings in pytime.c --- Python/pytime.c | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/Python/pytime.c b/Python/pytime.c index 01c07da074757e..e566051bdbb0d6 100644 --- a/Python/pytime.c +++ b/Python/pytime.c @@ -1,5 +1,4 @@ #include "Python.h" -#include "pycore_pymath.h" // _Py_InIntegralTypeRange() #ifdef MS_WINDOWS # include // struct timeval #endif @@ -41,6 +40,14 @@ # error "unsupported time_t size" #endif +#if PY_TIME_T_MAX + PY_TIME_T_MIN != -1 +# error "time_t is not a two's complement integer type" +#endif + +#if _PyTime_MIN + _PyTime_MAX != -1 +# error "_PyTime_t is not a two's complement integer type" +#endif + static void pytime_time_t_overflow(void) @@ -294,7 +301,19 @@ pytime_double_to_denominator(double d, time_t *sec, long *numerator, } assert(0.0 <= floatpart && floatpart < denominator); - if (!_Py_InIntegralTypeRange(time_t, intpart)) { + /* + Conversion of an out-of-range value to time_t gives undefined behaviour + (C99 ยง6.3.1.4p1), so we must guard against it. However, checking that + `intpart` is in range is delicate: the obvious expression `intpart <= + PY_TIME_T_MAX` will first convert the value `PY_TIME_T_MAX` to a double, + potentially changing its value and leading to us failing to catch some + UB-inducing values. The code below works correctly under the mild + assumption that time_t is a two's complement integer type with no trap + representation, and that `PY_TIME_T_MIN` is within the representable + range of a C double. + */ + assert(fmod(intpart, 1.0) == 0.0); + if (!((double)PY_TIME_T_MIN <= intpart && intpart < -(double)PY_TIME_T_MIN)) { pytime_time_t_overflow(); return -1; } @@ -349,7 +368,9 @@ _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)) { + /* See comments in pytime_double_to_denominator */ + assert(fmod(intpart, 1.0) == 0.0); + if (!((double)PY_TIME_T_MIN <= intpart && intpart < -(double)PY_TIME_T_MIN)) { pytime_time_t_overflow(); return -1; } @@ -515,8 +536,10 @@ pytime_from_double(_PyTime_t *tp, double value, _PyTime_round_t round, d *= (double)unit_to_ns; d = pytime_round(d, round); - if (!_Py_InIntegralTypeRange(_PyTime_t, d)) { - pytime_overflow(); + /* See comments in pytime_double_to_denominator */ + assert(fmod(d, 1.0) == 0.0); + if (!((double)_PyTime_MIN <= d && d < -(double)_PyTime_MIN)) { + pytime_time_t_overflow(); return -1; } _PyTime_t ns = (_PyTime_t)d; @@ -910,7 +933,7 @@ py_get_system_clock(_PyTime_t *tp, _Py_clock_info_t *info, int raise_exc) info->monotonic = 0; info->adjustable = 1; if (clock_getres(CLOCK_REALTIME, &res) == 0) { - info->resolution = res.tv_sec + res.tv_nsec * 1e-9; + info->resolution = (double)res.tv_sec + (double)res.tv_nsec * 1e-9; } else { info->resolution = 1e-9; From 32aee25e3605d0bee891ab7255d12244436cb30b Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Sat, 11 Feb 2023 13:23:36 +0000 Subject: [PATCH 2/3] Add news --- .../next/Library/2023-02-11-13-23-29.gh-issue-97786.QjvQ1B.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2023-02-11-13-23-29.gh-issue-97786.QjvQ1B.rst diff --git a/Misc/NEWS.d/next/Library/2023-02-11-13-23-29.gh-issue-97786.QjvQ1B.rst b/Misc/NEWS.d/next/Library/2023-02-11-13-23-29.gh-issue-97786.QjvQ1B.rst new file mode 100644 index 00000000000000..df194b67590d67 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-02-11-13-23-29.gh-issue-97786.QjvQ1B.rst @@ -0,0 +1,2 @@ +Fix potential undefined behaviour in corner cases of floating-point-to-time +conversions. From c381e4c1170fe02efcde3a7d61719e4aecbf741c Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Sun, 12 Feb 2023 10:47:21 +0000 Subject: [PATCH 3/3] Add note on nans; remove unnecessary asserts --- Python/pytime.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Python/pytime.c b/Python/pytime.c index e566051bdbb0d6..acd1842056af43 100644 --- a/Python/pytime.c +++ b/Python/pytime.c @@ -311,8 +311,10 @@ pytime_double_to_denominator(double d, time_t *sec, long *numerator, assumption that time_t is a two's complement integer type with no trap representation, and that `PY_TIME_T_MIN` is within the representable range of a C double. + + Note: we want the `if` condition below to be true for NaNs; therefore, + resist any temptation to simplify by applying De Morgan's laws. */ - assert(fmod(intpart, 1.0) == 0.0); if (!((double)PY_TIME_T_MIN <= intpart && intpart < -(double)PY_TIME_T_MIN)) { pytime_time_t_overflow(); return -1; @@ -369,7 +371,6 @@ _PyTime_ObjectToTime_t(PyObject *obj, time_t *sec, _PyTime_round_t round) (void)modf(d, &intpart); /* See comments in pytime_double_to_denominator */ - assert(fmod(intpart, 1.0) == 0.0); if (!((double)PY_TIME_T_MIN <= intpart && intpart < -(double)PY_TIME_T_MIN)) { pytime_time_t_overflow(); return -1; @@ -537,7 +538,6 @@ pytime_from_double(_PyTime_t *tp, double value, _PyTime_round_t round, d = pytime_round(d, round); /* See comments in pytime_double_to_denominator */ - assert(fmod(d, 1.0) == 0.0); if (!((double)_PyTime_MIN <= d && d < -(double)_PyTime_MIN)) { pytime_time_t_overflow(); return -1;