Fix variadic calls on Apple platforms with a non-OS libffi#265
Open
ambv wants to merge 2 commits into
Open
Conversation
Closes python-cffi#264 The CFFI_CHECK_FFI_* feature checks only enabled ffi_prep_cif_var(), ffi_closure_alloc() and ffi_prep_closure_loc() on Apple platforms when FFI_AVAILABLE_APPLE was defined. That macro comes from the libffi header shipped with the macOS SDK, so any build against a different libffi - the static libffi that iOS/tvOS/watchOS wheels are built against, or a Homebrew libffi on macOS - silently fell into the fallback branch that disables all three functions. Without ffi_prep_cif_var(), variadic functions are called through a cif prepared by ffi_prep_cif(), i.e. with the fixed-arguments calling convention. On arm64 Apple platforms variadic arguments are passed on the stack, unlike named arguments, so the callee read garbage: integers came back as random values and pointer arguments typically crashed (e.g. any printf-family callee dereferencing a garbage %s). There is no runtime-availability question in this configuration: the libffi the module was linked against always provides all three functions, so the new branch enables them unconditionally. The two "for an unknown reason" iOS markers in test_c.py were this bug: test_call_function_9 (garbage int return) and test_FILE (fscanf() reading nothing on 3.14, crashing on 3.13). Remove the markers so both act as regression tests. Verified on an arm64 iPhone simulator with CPython 3.13: the full test_c.py passes (208 passed, 21 skipped), and both tests fail again when the fix is reverted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #264
The CFFI_CHECK_FFI_* feature checks only enabled ffi_prep_cif_var(), ffi_closure_alloc() and ffi_prep_closure_loc() on Apple platforms when FFI_AVAILABLE_APPLE was defined. That macro comes from the libffi header shipped with the macOS SDK, so any build against a different libffi - the static libffi that iOS/tvOS/watchOS wheels are built against, or a Homebrew libffi on macOS - silently fell into the fallback branch that disables all three functions.
Without ffi_prep_cif_var(), variadic functions are called through a cif prepared by ffi_prep_cif(), i.e. with the fixed-arguments calling convention. On arm64 Apple platforms variadic arguments are passed on the stack, unlike named arguments, so the callee read garbage: integers came back as random values and pointer arguments typically crashed (e.g. any printf-family callee dereferencing a garbage %s).
There is no runtime-availability question in this configuration: the libffi the module was linked against always provides all three functions, so the new branch enables them unconditionally.
The two "for an unknown reason" iOS markers in test_c.py were this bug: test_call_function_9 (garbage int return) and test_FILE (fscanf() reading nothing on 3.14, crashing on 3.13). Remove the markers so both act as regression tests. Verified on an arm64 iPhone simulator with CPython 3.13: the full test_c.py passes (208 passed, 21 skipped), and both tests fail again when the fix is reverted.