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

New matmul operator crashes modules compiled with CPython3.4 #67209

Closed
amauryfa opened this issue Dec 9, 2014 · 12 comments
Closed

New matmul operator crashes modules compiled with CPython3.4 #67209

amauryfa opened this issue Dec 9, 2014 · 12 comments
Labels
docs Documentation in the Doc dir type-feature A feature request or enhancement

Comments

@amauryfa
Copy link
Member

amauryfa commented Dec 9, 2014

BPO 23020
Nosy @malemburg, @loewis, @amauryfa, @pitrou, @benjaminp
Files
  • mytest.c
  • 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 2014-12-09.12:31:43.258>
    labels = ['type-feature', 'docs']
    title = 'New matmul operator crashes modules compiled with CPython3.4'
    updated_at = <Date 2015-07-05.10:41:46.925>
    user = 'https://github.com/amauryfa'

    bugs.python.org fields:

    activity = <Date 2015-07-05.10:41:46.925>
    actor = 'pitrou'
    assignee = 'docs@python'
    closed = False
    closed_date = None
    closer = None
    components = ['Documentation']
    creation = <Date 2014-12-09.12:31:43.258>
    creator = 'amaury.forgeotdarc'
    dependencies = []
    files = ['37397']
    hgrepos = []
    issue_num = 23020
    keywords = []
    message_count = 10.0
    messages = ['232372', '232373', '232381', '232395', '232396', '232398', '232413', '232418', '232430', '246305']
    nosy_count = 7.0
    nosy_names = ['lemburg', 'loewis', 'amaury.forgeotdarc', 'pitrou', 'benjamin.peterson', 'Arfrever', 'docs@python']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'needs patch'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue23020'
    versions = ['Python 3.5']

    @amauryfa
    Copy link
    Member Author

    amauryfa commented Dec 9, 2014

    When an extension module is compiled with CPython3.4, the nb_matrix_multiply slot is not filled, and no memory is allocated for it.
    If the extension module is imported by CPython3.5, nb_matrix_multiply contains garbage and segfaults the interpreter.

    In Python 2.7 there are flags like Py_TPFLAGS_HAVE_INDEX to gate the access to recently added slots. I think Python3.5 should have a similar one.

    @amauryfa amauryfa added the type-crash A hard crash of the interpreter, possibly with a core dump label Dec 9, 2014
    @amauryfa
    Copy link
    Member Author

    amauryfa commented Dec 9, 2014

    Here is a test module that segfaults on Python3.5.
    It's derived from _testcapi, but it can be anything with a PyNumberMethods.

    I compiled with
    gcc -g -I path/to/cpython3.4/Include/ -I /path/to/cpython3.4 mytest.c -fPIC --shared -o mytest.so

    Then I ran python3.5 and:
    import mytest
    print(mytest.matmulType() @ 1)

    When the module is compiled with 3.5, TypeError is correctly raised.

    @benjaminp
    Copy link
    Contributor

    This technically falls under the fact that we don't have ABI compatibility between 3.N and 3.(N + 1).

    @amauryfa
    Copy link
    Member Author

    amauryfa commented Dec 9, 2014

    Oh, has this ABI compatibility requirement changed from the 2.x series?

    @benjaminp
    Copy link
    Contributor

    We never had compatibility between 2.N and 2.(N + 1) either.

    @amauryfa
    Copy link
    Member Author

    amauryfa commented Dec 9, 2014

    I would not mind the change, but I've always thought the opposite (except on Windows, where the string Python27.dll is stored in the .pyd)

    There are traces of this binary compatibility still today in the 3.4 headers:
    https://hg.python.org/cpython/file/v3.4.0/Include/pymem.h#l23
    """...if you do use the macros you must recompile your extensions with each Python release..."""
    Which seems to imply that extensions usually don't need to be recompiled!

    @Arfrever
    Copy link
    Mannequin

    Arfrever mannequin commented Dec 10, 2014

    Extensions need not to be rebuilt only when using stable ABI:
    https://www.python.org/dev/peps/pep-0384/

    But mytest.c fails to build with -DPy_LIMITED_API so it uses features from outside of stable ABI and must be rebuilt.

    @amauryfa
    Copy link
    Member Author

    PEP-384 is presented as a new way to write modules that can be loaded by multiple Python versions, specially on Windows.
    I could not find any place that says that modules not using the stable ABI need to be recompiled.

    I'm not against changing this rule (after all, I started on Windows platform where all this discussion does not apply), but it should be clearly stated somewhere in the API docs, and also in the "whatsnew" pages (all of them?)

    @benjaminp
    Copy link
    Contributor

    This is why C-extensions' PEP-3147 file tags depend on Python version.

    I agree the policy could stand to be documented.

    @taleinat taleinat added the docs Documentation in the Doc dir label Jun 18, 2015
    @larryhastings larryhastings added type-feature A feature request or enhancement and removed release-blocker type-crash A hard crash of the interpreter, possibly with a core dump labels Jul 5, 2015
    @pitrou
    Copy link
    Member

    pitrou commented Jul 5, 2015

    There was probably an informal "best effort ABI compatibility" in 2.x that we de facto dropped in 3.x. Otherwise, as Amaury points out, several Py_TPFLAGS_HAVE_XXX flags would have no purpose.

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

    @encukou Is any of this still relevant?

    @encukou
    Copy link
    Member

    encukou commented May 22, 2023

    It's not. If compiling with 3.4 and then importing in 3.5 ever worked, it was a coincidence.

    The flags Py_TPFLAGS_HAVE_FINALIZE and Py_TPFLAGS_HAVE_VERSION_TAG have no purpose. We're not adding new ones, but can't repurpose the bits for existing ones, so they're still around.
    Perhaps Python 2 had an informal “best effort ABI compatibility” policy, but it's not something we can realistically test and adhere to. The Limited API subset is challenging enough.

    Docs have been improving over time, but even 3.3 mentions “the API compatibility does not extend to binary compatibility” (except with stable ABI).

    @encukou encukou closed this as completed May 22, 2023
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    docs Documentation in the Doc dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    7 participants