bpo-34423: Fix check for overflow when casting from a double to integral types. #8802
Conversation
…ral types The added comment explains the issue. This fix disallows overflow: if a double `d` passes the test, it can mean that `d == 2**63`. When casted to `int64_t`, the variable overflows to -2**63.
|
this appears linked to the wrong issue -- is this the right one? https://bugs.python.org/issue34423 |
|
@asottile Yes, you're right. My branch is wrongly named too. Should I create new pull request? |
|
It might be enough to edit the commit message and title? I don't actually know |
enedil
changed the title
serhiy-storchaka
changed the title
|
A test should be added to test_time. |
| @@ -917,7 +918,8 @@ class TestCPyTime(CPyTimeTestCase, unittest.TestCase): | |||
| Test the C _PyTime_t API. | |||
| """ | |||
| # _PyTime_t is a 64-bit signed integer | |||
| OVERFLOW_SECONDS = math.ceil((2**63 + 1) / SEC_TO_NS) | |||
| OVERFLOW_SECONDS = math.ceil(( | |||
| + 1) / SEC_TO_NS) | |||
|
I don't think that the condition The problem is that the maximal value by something like |
|
I'm not sure that we will be able to find a solution using a "simple" macro. Maybe we need new functions which takes a double as input and return an integer as output and report overflow. I'm not sure that it's possible to write a generic function. Right now, _Py_InIntegralTypeRange() is only used to convert a double to _PyTime_t (int64_t) and time_t in pytime.c. Maybe 2 functions would be enough? About generic code, we can maybe use the preprocessor as a template engine to generate these functions. |
It would certainly be easier to support specific types only instead of being generic. We could for example assume that the number of bits in a |
The added comment explains the issue. This fix disallows overflow: if a double
dpasses the test, it can mean thatd == 2**63. When casted toint64_t, the variable overflows to -2**63.https://bugs.python.org/issue34423