classification
Title: ctypes memory error on Apple Silicon with external libffi
Type: behavior Stage: patch review
Components: ctypes, macOS Versions: Python 3.9
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: erykoff, maxbelanger, miss-islington, ned.deily, ronaldoussoren
Priority: normal Keywords: patch

Created on 2020-12-20 05:52 by erykoff, last changed 2021-04-08 07:25 by maxbelanger.

Pull Requests
URL Status Linked Edit
PR 23868 merged erykoff, 2020-12-20 06:09
PR 23888 merged miss-islington, 2020-12-22 11:13
PR 25274 open maxbelanger, 2021-04-08 07:25
Messages (5)
msg383419 - (view) Author: Eli Rykoff (erykoff) * Date: 2020-12-20 05:52
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
msg383426 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2020-12-20 09:56
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.
msg383446 - (view) Author: Eli Rykoff (erykoff) * Date: 2020-12-20 20:43
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.
msg383583 - (view) Author: miss-islington (miss-islington) Date: 2020-12-22 11:12
New changeset b3c77ecbbe0ad3e3cc6dbd885792203e9e6ec858 by erykoff in branch 'master':
bpo-42688: Fix ffi alloc/free when using external libffi on macos (GH-23868)
https://github.com/python/cpython/commit/b3c77ecbbe0ad3e3cc6dbd885792203e9e6ec858
msg386056 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2021-02-01 05:15
New changeset 7e729978fa08a360cbf936dc215ba7dd25a06a08 by Miss Islington (bot) in branch '3.9':
bpo-42688: Fix ffi alloc/free when using external libffi on macos (GH-23868) (GH-23888)
https://github.com/python/cpython/commit/7e729978fa08a360cbf936dc215ba7dd25a06a08
History
Date User Action Args
2021-04-08 07:25:47maxbelangersetnosy: + maxbelanger
pull_requests: + pull_request24011
2021-02-01 05:15:54ned.deilysetmessages: + msg386056
2020-12-22 11:13:00miss-islingtonsetpull_requests: + pull_request22747
2020-12-22 11:12:40miss-islingtonsetnosy: + miss-islington
messages: + msg383583
2020-12-20 20:43:50erykoffsetmessages: + msg383446
2020-12-20 09:56:29ronaldoussorensetmessages: + msg383426
2020-12-20 09:52:11ronaldoussorensetnosy: + ronaldoussoren, ned.deily
components: + macOS
2020-12-20 06:09:39erykoffsetkeywords: + patch
stage: patch review
pull_requests: + pull_request22730
2020-12-20 05:52:54erykoffcreate