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

ctypes memory error on Apple Silicon with external libffi #86854

Closed
erykoff mannequin opened this issue Dec 20, 2020 · 6 comments
Closed

ctypes memory error on Apple Silicon with external libffi #86854

erykoff mannequin opened this issue Dec 20, 2020 · 6 comments
Labels
3.8 only security fixes 3.9 only security fixes 3.10 only security fixes OS-mac topic-ctypes type-bug An unexpected behavior, bug, or error

Comments

@erykoff
Copy link
Mannequin

erykoff mannequin commented Dec 20, 2020

BPO 42688
Nosy @ronaldoussoren, @ned-deily, @ambv, @maxbelanger, @miss-islington, @erykoff
PRs
  • bpo-42688: Fix ffi alloc/free when using external libffi on macos #23868
  • [3.9] bpo-42688: Fix ffi alloc/free when using external libffi on macos (GH-23868) #23888
  • [3.8] bpo-41100: Support macOS 11 and Apple Silicon #25274
  • [3.8] bpo-41100: Support macOS 11 Big Sur and Apple Silicon Macs #25806
  • 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 = <Date 2021-05-02.09:33:11.031>
    created_at = <Date 2020-12-20.05:52:54.593>
    labels = ['OS-mac', 'type-bug', '3.8', '3.9', '3.10', 'ctypes']
    title = 'ctypes memory error on Apple Silicon with external libffi'
    updated_at = <Date 2021-05-02.09:33:11.030>
    user = 'https://github.com/erykoff'

    bugs.python.org fields:

    activity = <Date 2021-05-02.09:33:11.030>
    actor = 'ned.deily'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-05-02.09:33:11.031>
    closer = 'ned.deily'
    components = ['macOS', 'ctypes']
    creation = <Date 2020-12-20.05:52:54.593>
    creator = 'erykoff'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 42688
    keywords = ['patch']
    message_count = 6.0
    messages = ['383419', '383426', '383446', '383583', '386056', '392678']
    nosy_count = 6.0
    nosy_names = ['ronaldoussoren', 'ned.deily', 'lukasz.langa', 'maxbelanger', 'miss-islington', 'erykoff']
    pr_nums = ['23868', '23888', '25274', '25806']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue42688'
    versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

    @erykoff
    Copy link
    Mannequin Author

    erykoff mannequin commented Dec 20, 2020

    Building python 3.9.1 on Apple Silicon compiled against a external (non-os-provided) libffi makes the following code return a MemoryError:

    Test:

    import ctypes
    
    @ctypes.CFUNCTYPE(None, ctypes.c_int, ctypes.c_char_p)
    def error_handler(fif, message):
        pass

    I have tracked this down to the following code in malloc_closure.c:

    #if USING_APPLE_OS_LIBFFI && HAVE_FFI_CLOSURE_ALLOC
        if (__builtin_available(macos 10.15, ios 13, watchos 6, tvos 13, *)) {
            ffi_closure_free(p);
            return;
        }
    #endif

    and

    #if USING_APPLE_OS_LIBFFI && HAVE_FFI_CLOSURE_ALLOC
        if (__builtin_available(macos 10.15, ios 13, watchos 6, tvos 13, *)) {
            return ffi_closure_alloc(size, codeloc);
        }
    #endif

    In fact, while the __builtin_available() call should be guarded by USING_APPLE_OS_LIBFFI, the call to ffi_closure_alloc() should only be guarded by HAVE_FFI_CLOSURE_ALLOC, as this is set as the result of an independent check in setup.py and should be used with external libffi when supported.

    The following code does work instead:

    #if HAVE_FFI_CLOSURE_ALLOC
    #if USING_APPLE_OS_LIBFFI
        if (__builtin_available(macos 10.15, ios 13, watchos 6, tvos 13, *)) {
    #endif
            ffi_closure_free(p);
            return;
    #if USING_APPLE_OS_LIBFFI
        }
    #endif
    #endif
    
    
    #if HAVE_FFI_CLOSURE_ALLOC
    #if USING_APPLE_OS_LIBFFI
        if (__builtin_available(macos 10.15, ios 13, watchos 6, tvos 13, *)) {
    #endif
            return ffi_closure_alloc(size, codeloc);
            return;
    #if USING_APPLE_OS_LIBFFI
        }
    #endif
    #endif

    @erykoff erykoff mannequin added 3.9 only security fixes topic-ctypes type-bug An unexpected behavior, bug, or error labels Dec 20, 2020
    @ronaldoussoren
    Copy link
    Contributor

    Could you please sign the CLA? (See the PR for details on that)

    The PR looks sane.

    Out of interest: why do you use an external version of libffi? AFAIK the system copy of libffi contains some magic sauce to work nicer with signed binaries.

    @erykoff
    Copy link
    Mannequin Author

    erykoff mannequin commented Dec 20, 2020

    Thanks for your quick feedback! I signed the CLA after submitting the PR, but I think it takes a bit of time to percolate through the system.

    As for the "why", until 3.9.1 conda-forge had been successfully using an external ffi (with 3.9.0 + osx-arm64 patches) and then suddenly it broke. For the time being conda-forge is using the system ffi, which does have other advantages as well, having all the latest patches. However, I was curious as to _why_ it broke, and that led me to discover this bug, and it seemed straightforward to fix. When/if conda-forge will switch back to external ffi is TBD, but if that decision is made this issue (at least) will be taken care of.

    @miss-islington
    Copy link
    Contributor

    New changeset b3c77ec by erykoff in branch 'master':
    bpo-42688: Fix ffi alloc/free when using external libffi on macos (GH-23868)
    b3c77ec

    @ned-deily
    Copy link
    Member

    New changeset 7e72997 by Miss Islington (bot) in branch '3.9':
    bpo-42688: Fix ffi alloc/free when using external libffi on macos (GH-23868) (GH-23888)
    7e72997

    @ambv
    Copy link
    Contributor

    ambv commented May 2, 2021

    New changeset b29d0a5 by Ned Deily in branch '3.8':
    [3.8] bpo-41100: Support macOS 11 Big Sur and Apple Silicon Macs (bpo-25806)
    b29d0a5

    @ned-deily ned-deily added 3.8 only security fixes 3.10 only security fixes labels May 2, 2021
    @ned-deily ned-deily added 3.8 only security fixes 3.10 only security fixes labels May 2, 2021
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes 3.9 only security fixes 3.10 only security fixes OS-mac topic-ctypes type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants