bpo-41100: ctypes: use the correct ABI for variadic functions #21242
Conversation
Modules/_ctypes/callproc.c
Outdated
| return -1; | ||
|
|
||
| #if defined(__APPLE__) && __arm64__ | ||
| if (argtypecount != 0 && argcount > argtypecount) { |
There was a problem hiding this comment.
Is this test correct?
http://www.chiark.greenend.org.uk/doc/libffi-dev/html/The-Basics.html indicates that ffi_prep_cif_var is not the same as ffi_prep_cif even if argtypecount == argcount.
There was a problem hiding this comment.
"Is this test correct?"
It's not correct, but it is less incorrect than it was before.
"The iOS ABI for functions that take a variable number of arguments is entirely different from the generic version."
That's the same ABI as arm64 Mac OS.
But the problem is, I don't see a way to specify a function to be variadic in the ctypes API, and lots of existing code out there isn't going to know to specify it even if we add it. They're just going to set the arg types and call the function with extra args and expect it to work.
This change at least cover the typical case where there is at least one variable arg passed to the function.
Should we add a new documented feature to ctypes allowing the user to specify a function is variadic?
(edit) OK I have a patch to add a variadic flag, just waiting for approval to post it. It should be up shortly.
Modules/_ctypes/callproc.c
Outdated
| "ffi_prep_cif failed"); | ||
| return -1; | ||
|
|
||
| #if defined(__APPLE__) && __arm64__ |
There was a problem hiding this comment.
I'm not sure about this. This is the least impacting change to get a working ctypes on macOS/arm64, but this complicates the code base.
For the trunk I'd prefer to use ffi_pre_cif_var unconditionally when available instead of only on macOS/arm64.
There was a problem hiding this comment.
how can we determine if it is available? We do not decide which libffi to use until setup.py gets run, so we can't use autoconf to probe for it. Should we add a autoconf style probe in setup.py?
There was a problem hiding this comment.
OK, if you like the idea of setup.py probing for functions in libffi, take a look here: #21249
I thought to keep things simple it would be best to just grep the headers, rather than to try to do what autoconf would have done.
On arm64 the calling convention for variardic functions is different than the convention for fixed-arg functions of the same arg types. ctypes needs to use ffi_prep_cif_var to tell libffi which calling convention to use. This patch adds a new attribute "f.variadic" for ctypes function pointers, so users can specify which functions are variadic. It will also auto-detect varargs when a function is called with more arguments than f.argtypes specifies. Since this is a new option and it only matters on arm64 on apple platforms, lots of existing code will not set f.variadic. This will do the right thing in most situations.
lawrence-danna-apple
force-pushed
the
postback-0005
branch
from
bf3fbbf
to
fc54fcb
Compare
|
superseded by #21249 |
On arm64 the calling convention for variardic functions is different
than the convention for fixed-arg functions of the same arg types.
ctypes needs to use ffi_prep_cif_var to tell libffi which calling
convention to use.
https://bugs.python.org/issue41100