Repository navigation
_PyTime_FromDouble() fails to detect an integer overflow when converting a C double to a C int64_t #83458
Description
| BPO | 39277 |
|---|---|
| Nosy | @tim-one, @vstinner, @benjaminp, @skrah, @graingert |
| PRs |
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 2020-01-09.14:43:45.587>
labels = ['3.8', 'type-bug', '3.7']
title = '_PyTime_FromDouble() fails to detect an integer overflow when converting a C double to a C int64_t'
updated_at = <Date 2022-02-07.15:21:14.076>
user = 'https://github.com/graingert'bugs.python.org fields:
activity = <Date 2022-02-07.15:21:14.076>
actor = 'vstinner'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = []
creation = <Date 2020-01-09.14:43:45.587>
creator = 'graingert'
dependencies = []
files = []
hgrepos = []
issue_num = 39277
keywords = ['patch']
message_count = 8.0
messages = ['359682', '359683', '359710', '359728', '364021', '372294', '372335', '412754']
nosy_count = 5.0
nosy_names = ['tim.peters', 'vstinner', 'benjamin.peterson', 'skrah', 'graingert']
pr_nums = ['17933', '31195']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue39277'
versions = ['Python 3.7', 'Python 3.8']
Activity
graingert commentedon Jan 9, 2020
_PyTime_FromDouble() fails to detect an integer overflow when converting a C double to a C int64_t
vstinner commentedon Jan 9, 2020
It's a bug in _PyTime_FromDouble() which fails to detect the integer overflow when casting a C double to a C _PyTime_T (int64_t): bug in _Py_InIntegralTypeRange(_PyTime_t, d) where d is a C double.
On my Fedora 31, double is a 64-bit IEEE 754 float, _PyTime_t is int64_t (64-bit signed integer).
_PyTime_t maximum is 9223372036854775807. But casted as C double, it becomes 9223372036854775808:
vstinner commentedon Jan 10, 2020
_PyTime_FromDouble() checks if!(_Py_DoubleInIntegralTypeRange(_PyTime_t, d)) with the macro:
and _Py_IntegralTypeMax(type)=2**63-1.
"v <= _Py_IntegralTypeMax(type)" compares a C double to a C int64_t: the compiler casts the C int64_t to a C double.
The problem is that 2**63-1 casted to a C double rounds using ROUND_HALF_EVEN rounding mode which gives a number *greater* than 2**63-1: we get 2**63.
To implement "v <= max", we have to round max towards zero (ROUND_DOWN), not round it using ROUND_HALF_EVEN.
I didn't find a way to control the rounding mode of casting C int64_t to C double, but we can round it *afterwards* using nextafter(max, 0.0) (ROUND_DOWN).
vstinner commentedon Jan 10, 2020
Similar issue in HHVM:
vstinner commentedon Mar 12, 2020
Oh, clang on FreeBSD spotted a similar bug in float.__trunc__()!
skrah commentedon Jun 24, 2020
Is it a bug in float.__trunc__()? It seems to me that the detailed comment accounts for case of rounding up.
That's why < is used instead of <=.
skrah commentedon Jun 25, 2020
C99, 6.3.1.4 Real floating and integer:
"""
When a value of integer type is converted to a real floating type, if the value being
converted can be represented exactly in the new type, it is unchanged. If the value being
converted is in the range of values that can be represented but cannot be represented
exactly, the result is either the nearest higher or nearest lower representable value, chosen
in an implementation-defined manner. If the value being converted is outside the range of
values that can be represented, the behavior is undefined.
"""
So int64_t => double is implementation defined, but not undefined.
float___trunc___impl() looks correct to me (of course we can squash
that warning).
vstinner commentedon Feb 7, 2022
New changeset d3e53bc by Victor Stinner in branch 'main':
bpo-39277: Fix PY_TIMEOUT_MAX cast in _threadmodule.c (GH-31195)
d3e53bc
mdickinson commentedon Mar 4, 2023
This was fixed for 3.10 and later in #101826 and its backports.