Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overflow when casting from double to time_t, and_PyTime_t. #78604

Closed
enedil mannequin opened this issue Aug 17, 2018 · 7 comments
Closed

Overflow when casting from double to time_t, and_PyTime_t. #78604

enedil mannequin opened this issue Aug 17, 2018 · 7 comments
Labels
3.10 only security fixes 3.11 only security fixes 3.12 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@enedil
Copy link
Mannequin

enedil mannequin commented Aug 17, 2018

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
    Copy link
    Mannequin Author

    enedil mannequin commented Aug 17, 2018

    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.

    @enedil enedil mannequin added 3.7 (EOL) end of life 3.8 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Aug 17, 2018
    @vstinner
    Copy link
    Member

    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
    Copy link
    Member

    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
    Copy link
    Mannequin Author

    enedil mannequin commented Aug 23, 2018

    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
    Copy link
    Member

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

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @iritkatriel
    Copy link
    Member

    Closed along with the PR.

    @iritkatriel iritkatriel closed this as not planned Won't fix, can't repro, duplicate, stale Dec 8, 2022
    @mdickinson mdickinson added 3.11 only security fixes 3.10 only security fixes 3.12 bugs and security fixes and removed 3.8 only security fixes 3.7 (EOL) end of life labels Mar 4, 2023
    @mdickinson
    Copy link
    Member

    Fixed for Python 3.10 and later by #101826 and its backports.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 only security fixes 3.11 only security fixes 3.12 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants