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

PYLONG_BITS_IN_DIGIT differs between MinGW and MSVC #79218

Open
scoder opened this issue Oct 21, 2018 · 11 comments
Open

PYLONG_BITS_IN_DIGIT differs between MinGW and MSVC #79218

scoder opened this issue Oct 21, 2018 · 11 comments
Labels
3.9 only security fixes 3.10 only security fixes 3.11 only security fixes build The build process and cross-build extension-modules C modules in the Modules dir OS-windows

Comments

@scoder
Copy link
Contributor

scoder commented Oct 21, 2018

BPO 35037
Nosy @pfmoore, @mdickinson, @scoder, @tjguk, @zware, @serhiy-storchaka, @zooba, @BenjaminLiuPenrose
PRs
  • bpo-35037 Fix issue in cython project: Cython files don't compile on Mingw-w64 64-bit  #19326
  • 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-10-21.08:22:52.766>
    labels = ['3.9', '3.10', '3.11', 'extension-modules', 'build', 'OS-windows']
    title = 'PYLONG_BITS_IN_DIGIT differs between MinGW and MSVC'
    updated_at = <Date 2021-12-22.13:16:07.818>
    user = 'https://github.com/scoder'

    bugs.python.org fields:

    activity = <Date 2021-12-22.13:16:07.818>
    actor = 'mark.dickinson'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Extension Modules', 'Windows']
    creation = <Date 2018-10-21.08:22:52.766>
    creator = 'scoder'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 35037
    keywords = ['patch']
    message_count = 9.0
    messages = ['328197', '328198', '328202', '328203', '328204', '328206', '328341', '404733', '409034']
    nosy_count = 8.0
    nosy_names = ['paul.moore', 'mark.dickinson', 'scoder', 'tim.golden', 'zach.ware', 'serhiy.storchaka', 'steve.dower', 'Beier Liu']
    pr_nums = ['19326']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'compile error'
    url = 'https://bugs.python.org/issue35037'
    versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

    Linked PRs

    @scoder
    Copy link
    Contributor Author

    scoder commented Oct 21, 2018

    I see reports from Cython users on Windows-64 that extension modules that use "longintrepr.h" get miscompiled by MinGW. A failing setup looks as follows:

    Stock 64 bit CPython on Windows, e.g.
    Python 3.6.3 (v3.6.3:2c5fed8, Oct 3 2017, 18:11:49) [MSC v.1900 64 bit (AMD64)]

    MinGW uses this compile time setup, i.e. 15 bit digits:
    PyLong_BASE 0x8000
    PyLong_MASK 7FFF
    PyLong_SHIFT 15
    sizeof(digit) 2
    sizeof(sdigit) 2

    Whereas CPython reports the following, indicating that it was in fact built with 30 bit digits:
    sys.getsize(1, 2**14, 2**15, 2**29, 2**30, 2**63, 2**64) (28, 28, 28, 28, 32, 36, 36)

    I'm not sure if this also applies to Py2.7, but I don't think the PyLong implementations differ in this regard.

    The compile time PyLong digit size is not remembered by the CPython installation and instead determined on the fly by the extension compiler. It seems that the MSVC build of the stock CPython packages differs from what MinGW decides here. This renders MinGW unusable for code that uses "longintrepr.h", which Cython does by default in order to accelerate its unpacking of PyLong values.

    Is there a reason why CPython cannot store its compile time value for the PYLONG_BITS_IN_DIGIT setting somewhere?

    @scoder scoder added 3.7 (EOL) end of life 3.8 only security fixes extension-modules C modules in the Modules dir OS-windows build The build process and cross-build labels Oct 21, 2018
    @scoder
    Copy link
    Contributor Author

    scoder commented Oct 21, 2018

    Some more information. CPython uses this code snippet to decide on the PyLong digit size:

    #ifndef PYLONG_BITS_IN_DIGIT
    #if SIZEOF_VOID_P >= 8
    #define PYLONG_BITS_IN_DIGIT 30
    #else
    #define PYLONG_BITS_IN_DIGIT 15
    #endif
    #endif

    In MinGW, "SIZEOF_VOID_P" is 4, whereas "sizeof(void*)" resolves to 8.

    @mdickinson
    Copy link
    Member

    Is there a reason why CPython cannot store its compile time value for the PYLONG_BITS_IN_DIGIT setting somewhere?

    There's PyLong_GetInfo [1], and at Python level, the corresponding sys.long_info (Python 2) / sys.int_info (Python 3). Does that help?

    [1]

    cpython/Objects/longobject.c

    Lines 5578 to 5595 in b2e2025

    PyObject *
    PyLong_GetInfo(void)
    {
    PyObject* int_info;
    int field = 0;
    int_info = PyStructSequence_New(&Int_InfoType);
    if (int_info == NULL)
    return NULL;
    PyStructSequence_SET_ITEM(int_info, field++,
    PyLong_FromLong(PyLong_SHIFT));
    PyStructSequence_SET_ITEM(int_info, field++,
    PyLong_FromLong(sizeof(digit)));
    if (PyErr_Occurred()) {
    Py_CLEAR(int_info);
    return NULL;
    }
    return int_info;
    }

    @serhiy-storchaka
    Copy link
    Member

    At runtime this information is available as sys.int_info.bits_per_digit.

    In MinGW, "SIZEOF_VOID_P" is 4, whereas "sizeof(void*)" resolves to 8.

    Looks like a misconfiguration. I suppose this will cause more issues than just wrong PYLONG_BITS_IN_DIGIT.

    @scoder
    Copy link
    Contributor Author

    scoder commented Oct 21, 2018

    There's PyLong_GetInfo ...

    Thanks, I understand that this information can be found at runtime. However, in order to correctly compile C code against a given CPython runtime, there needs to be a guarantee that extension module builds use the same configuration as the CPython build.

    Misconfiguration

    I searched some more, and it looks like this was already reported almost ten years ago in bpo-4709.

    The work-around there is to pass -DMS_WIN64, which apparently is not defined by MinGW but required by the CPython headers.

    @zooba
    Copy link
    Member

    zooba commented Oct 21, 2018

    That variable should be inferred somewhere, either in PC/pyconfig.h or pyport.h. If there's a way to also infer it on 64-bit MinGW then we would consider a patch for that.

    @scoder
    Copy link
    Contributor Author

    scoder commented Oct 23, 2018

    The relevant macro seems to be "__MINGW64__". I have neither a Windows environment nor MinGW-64 for testing, but the logic should be the same as for the "_WIN64" macro, i.e.

    #if defined(_WIN64) || defined(__MINGW64__)
    #define MS_WIN64
    #endif

    And then there's probably also something to do to record the compiler version in the long CPython version string, etc. I also can't say if MinGW-64 sets the "MS_WIN32" macro. Having "MS_WIN64" without "MS_WIN32" might be unexpected for some code.

    @mdickinson
    Copy link
    Member

    This should probably be a separate issue, but I wonder whether the 15-bit digit option has value any more. Should we just drop that option and always use 30-bit digits?

    30-bit digits were introduced at a time when we couldn't rely on a 64-bit integer type always being available, so we still needed to keep the 15-bit fallback option. But I think that's no longer the case.

    @iritkatriel iritkatriel added 3.9 only security fixes 3.10 only security fixes 3.11 only security fixes and removed 3.7 (EOL) end of life 3.8 only security fixes labels Dec 11, 2021
    @mdickinson
    Copy link
    Member

    This should probably be a separate issue,

    Specifically, bpo-45569.

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

    gpshead commented Aug 5, 2022

    is this still relevant now that we always default to 30-bit?

    @mdickinson
    Copy link
    Member

    @gpshead I guess the PYLONG_BITS_IN_DIGIT issue still exists in theory, but it would be harder to hit it by accident at this point. The wider issue of SIZEOF_VOID_P being wrong seems like it would still be relevant.

    carljm added a commit to carljm/cpython that referenced this issue Dec 14, 2022
    * main: (103 commits)
      pythongh-100248: Add missing `ssl_shutdown_timeout` parameter in `asyncio` docs (python#100249)
      Assorted minor fixes for specialization stats. (pythonGH-100219)
      pythongh-100176: venv: Remove redundant compat code for Python <= 3.2 (python#100177)
      pythonGH-100222: Redefine _Py_CODEUNIT as a union to clarify structure of code unit. (pythonGH-100223)
      pythongh-99955: undef ERROR and SUCCESS before redefining (fixes sanitizer warning) (python#100215)
      pythonGH-100206: use versionadded for the addition of sysconfig.get_default_scheme (python#100207)
      pythongh-81057: Move _Py_RefTotal to the "Ignored Globals" List (pythongh-100203)
      pythongh-81057: Move Signal-Related Globals to _PyRuntimeState (pythongh-100085)
      pythongh-81057: Move faulthandler Globals to _PyRuntimeState (pythongh-100152)
      pythongh-81057: Move tracemalloc Globals to _PyRuntimeState (pythongh-100151)
      pythonGH-100143: Improve collecting pystats for parts of runs (pythonGH-100144)
      pythongh-99955: standardize return values of functions in compiler's code-gen (python#100010)
      pythongh-79218: Define `MS_WIN64` macro for Mingw-w64 64bit on Windows (pythonGH-100137)
      Fix: typo (Indention) (pythonGH-99904)
      pythongh-96715 Remove redundant NULL check in `profile_trampoline` function (python#96716)
      pythongh-100176: remove incorrect version compatibility check from argument clinic (python#100190)
      clarify the 4300-digit limit on int-str conversion (python#100175)
      pythongh-70393: Clarify mention of "middle" scope (python#98839)
      pythongh-99688: Fix outdated tests in test_unary (python#99712)
      pythongh-100174: [Enum] Correct PowersOfThree example. (pythonGH-100178)
      ...
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.9 only security fixes 3.10 only security fixes 3.11 only security fixes build The build process and cross-build extension-modules C modules in the Modules dir OS-windows
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants