GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — _PyTime_FromDouble() fails to detect an integer overflow when converting a C double to a C int64_t · Issue #83458 · python/cpython

_PyTime_FromDouble() fails to detect an integer overflow when converting a C double to a C int64_t #83458

New issue
Copy link
New issue
Copy link
Closed
Closed
_PyTime_FromDouble() fails to detect an integer overflow when converting a C double to a C int64_t#83458
Copy link
Labels
3.10 (EOL)end of lifeend of life3.11only security fixesonly security fixes3.12only security fixesonly security fixestype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

Description

graingert
mannequin
opened on Jan 9, 2020
Issue body actions
BPO 39277
Nosy @tim-one, @vstinner, @benjaminp, @skrah, @graingert
PRs
  • bpo-39277, pytime: Fix overflow check on double to int cast #17933
  • bpo-39277: Fix PY_TIMEOUT_MAX cast in _threadmodule.c #31195
  • 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

    1. graingert commented on Jan 9, 2020

      @graingert
      graingertmannequin
      on Jan 9, 2020
      MannequinAuthor
      More actions

      _PyTime_FromDouble() fails to detect an integer overflow when converting a C double to a C int64_t

      Python 3.7.5 (default, Nov 20 2019, 09:21:52) 
      [GCC 9.2.1 20191008] on linux
      Type "help", "copyright", "credits" or "license" for more information.
      >>> import time
      >>> time.sleep(9223372036.854777)
      Traceback (most recent call last):
        File "<stdin>", line 1, in <module>
      ValueError: sleep length must be non-negative
    2. graingert
      added
      3.7 (EOL)end of lifeend of life
      3.8 (EOL)end of lifeend of life
      type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
      on Jan 9, 2020
    3. vstinner commented on Jan 9, 2020

      @vstinner
      vstinner
      on Jan 9, 2020
      Member
      More actions

      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:

      >>> int(float(9223372036854775807))
      9223372036854775808
    4. vstinner commented on Jan 10, 2020

      @vstinner
      vstinner
      on Jan 10, 2020
      Member
      More actions

      _PyTime_FromDouble() checks if!(_Py_DoubleInIntegralTypeRange(_PyTime_t, d)) with the macro:

      #define _Py_InIntegralTypeRange(type, v) (_Py_IntegralTypeMin(type) <= v && v <= _Py_IntegralTypeMax(type))

      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).

    5. vstinner commented on Jan 10, 2020

      @vstinner
      vstinner
      on Jan 10, 2020
      Member
      More actions

      Similar issue in HHVM:

      • Bug on casting double to uint64_t meta/hhvm#5932
      • PPC64/hhvm@7cdb76b
    6. vstinner commented on Mar 12, 2020

      @vstinner
      vstinner
      on Mar 12, 2020
      Member
      More actions

      Oh, clang on FreeBSD spotted a similar bug in float.__trunc__()!

      static PyObject *
      float___trunc___impl(PyObject *self)
      /*[clinic end generated code: output=dd3e289dd4c6b538 input=591b9ba0d650fdff]*/
      {
          double x = PyFloat_AsDouble(self);
          double wholepart;           /* integral portion of x, rounded toward 0 */
      
          (void)modf(x, &wholepart);
          /* Try to get out cheap if this fits in a Python int.  The attempt
           * to cast to long must be protected, as C doesn't define what
           * happens if the double is too big to fit in a long.  Some rare
           * systems raise an exception then (RISCOS was mentioned as one,
           * and someone using a non-default option on Sun also bumped into
           * that).  Note that checking for >= and <= LONG_{MIN,MAX} would
           * still be vulnerable:  if a long has more bits of precision than
           * a double, casting MIN/MAX to double may yield an approximation,
           * and if that's rounded up, then, e.g., wholepart=LONG_MAX+1 would
           * yield true from the C expression wholepart<=LONG_MAX, despite
           * that wholepart is actually greater than LONG_MAX.
           */
          if (LONG_MIN < wholepart && wholepart < LONG_MAX) {  /* <=== HERE */
              const long aslong = (long)wholepart;
              return PyLong_FromLong(aslong);
          }
          return PyLong_FromDouble(wholepart);
      }
      
      
      Objects/floatobject.c:881:45: warning: implicit conversion from 'long' to 'double' changes value from 9223372036854775807 to 9223372036854775808
            [-Wimplicit-int-float-conversion]
          if (LONG_MIN < wholepart && wholepart < LONG_MAX) {
                                                ~ ^~~~~~~~
      /usr/include/sys/limits.h:64:18: note: expanded from macro 'LONG_MAX'
      #define LONG_MAX        __LONG_MAX      /* max for a long */
                              ^~~~~~~~~~
      /usr/include/x86/_limits.h:64:20: note: expanded from macro '__LONG_MAX'
      #define __LONG_MAX      0x7fffffffffffffff      /* max for a long */
                              ^~~~~~~~~~~~~~~~~~
      1 warning generated.
    7. skrah commented on Jun 24, 2020

      @skrah
      skrahmannequin
      on Jun 24, 2020
      Mannequin
      More actions

      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 <=.

    8. skrah commented on Jun 25, 2020

      @skrah
      skrahmannequin
      on Jun 25, 2020
      Mannequin
      More actions

      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).

    9. vstinner commented on Feb 7, 2022

      @vstinner
      vstinner
      on Feb 7, 2022
      Member
      More actions

      New changeset d3e53bc by Victor Stinner in branch 'main':
      bpo-39277: Fix PY_TIMEOUT_MAX cast in _threadmodule.c (GH-31195)
      d3e53bc

    10. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    11. mdickinson
      added
      3.11only security fixesonly security fixes
      3.10 (EOL)end of lifeend of life
      3.12only security fixesonly security fixes
      and removed
      3.8 (EOL)end of lifeend of life
      3.7 (EOL)end of lifeend of life
      on Mar 4, 2023
    12. mdickinson commented on Mar 4, 2023

      @mdickinson
      mdickinson
      on Mar 4, 2023
      Member
      More actions

      This was fixed for 3.10 and later in #101826 and its backports.

      Python 3.12.0a5+ (heads/main:8de59c1bb9, Mar  4 2023, 08:28:13) [Clang 14.0.0 (clang-1400.0.29.202)] on darwin
      Type "help", "copyright", "credits" or "license" for more information.
      >>> import time
      >>> time.sleep(9223372036.854777)
      Traceback (most recent call last):
        File "<stdin>", line 1, in <module>
      OverflowError: timestamp out of range for platform time_t
    13. mdickinson
      closed this as completedon Mar 4, 2023
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      3.10 (EOL)end of lifeend of life3.11only security fixesonly security fixes3.12only security fixesonly security fixestype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions

        You are visitor 01402503 — GeezerVison does not store the pages you look up.
        Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.