Repository navigation
Use of ffi_type_complex_double is unguarded #128156
Copy link
Copy link
Closed
Labels
OS-macbuildThe build process and cross-buildThe build process and cross-buildextension-modulesC modules in the Modules dirC modules in the Modules dirtype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
Description
Bug report
Bug description:
e.g.
-std=c11 -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wstrict-prototypes -Werror=implicit-function-declaration -fvisibility=hidden -Werror=unguarded-availability-new -I./Include/internal -I./Include/internal/mimalloc -I. -I./Include -c ./Modules/_ctypes/cfield.c -o Modules/_ctypes/cfield.o
./Modules/_ctypes/cfield.c:1610:36: error: 'ffi_type_complex_double' is only available on macOS 10.15 or newer [-Werror,-Wunguarded-availability-new]
case 'C': fd->pffi_type = &ffi_type_complex_double; break;
^~~~~~~~~~~~~~~~~~~~~~~
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/ffi/ffi.h:236:46: note: 'ffi_type_complex_double' has been marked as being introduced in macOS 10.15 here, but the deployment target is macOS 10.9.0
FFI_AVAILABLE_APPLE_2019 FFI_EXTERN ffi_type ffi_type_complex_double;
^
./Modules/_ctypes/cfield.c:1610:36: note: enclose 'ffi_type_complex_double' in a __builtin_available check to silence this warning
case 'C': fd->pffi_type = &ffi_type_complex_double; break;
^~~~~~~~~~~~~~~~~~~~~~~
./Modules/_ctypes/cfield.c:1611:36: error: 'ffi_type_complex_float' is only available on macOS 10.15 or newer [-Werror,-Wunguarded-availability-new]
case 'E': fd->pffi_type = &ffi_type_complex_float; break;
^~~~~~~~~~~~~~~~~~~~~~
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/ffi/ffi.h:235:46: note: 'ffi_type_complex_float' has been marked as being introduced in macOS 10.15 here, but the deployment target is macOS 10.9.0
FFI_AVAILABLE_APPLE_2019 FFI_EXTERN ffi_type ffi_type_complex_float;
^
./Modules/_ctypes/cfield.c:1611:36: note: enclose 'ffi_type_complex_float' in a __builtin_available check to silence this warning
case 'E': fd->pffi_type = &ffi_type_complex_float; break;
^~~~~~~~~~~~~~~~~~~~~~
./Modules/_ctypes/cfield.c:1612:36: error: 'ffi_type_complex_double' is only available on macOS 10.15 or newer [-Werror,-Wung
uarded-availability-new]
case 'F': fd->pffi_type = &ffi_type_complex_longdouble; break;
^~~~~~~~~~~~~~~~~~~~~~~~~~~
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/ffi/ffi.h:240:37: note: expanded from macro 'ffi_type_complex_longdouble'
#define ffi_type_complex_longdouble ffi_type_complex_double
^~~~~~~~~~~~~~~~~~~~~~~
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/ffi/ffi.h:236:46: note: 'ffi_type_complex_double' has been marked as being introduced in macOS 10.15 here, but the deployment target is macOS 10.9.0
FFI_AVAILABLE_APPLE_2019 FFI_EXTERN ffi_type ffi_type_complex_double;
^
./Modules/_ctypes/cfield.c:1612:36: note: enclose 'ffi_type_complex_double' in a __builtin_available check to silence this warning
case 'F': fd->pffi_type = &ffi_type_complex_longdouble; break;
^~~~~~~~~~~~~~~~~~~~~~~~~~~
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/ffi/ffi.h:240:37: note: expanded from macro 'ffi_type_complex_longdouble'
#define ffi_type_complex_longdouble ffi_type_complex_double
^~~~~~~~~~~~~~~~~~~~~~~
See #100384 for context and similar issues.
CPython versions tested on:
CPython main branch
Operating systems tested on:
macOS
Linked PRs
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
OS-macbuildThe build process and cross-buildThe build process and cross-buildextension-modulesC modules in the Modules dirC modules in the Modules dirtype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
Activity
zanieb commentedon Dec 21, 2024
I started taking a look at a patch for this, but it doesn't look trivial.
The existing checks use
defined(Py_HAVE_C_COMPLEX) && defined(Py_FFI_SUPPORT_C_COMPLEX)and I think it needs to checkdefined(USING_APPLE_OS_LIBFFI) && defined(HAVE_FFI_TYPE_COMPLEX_DOUBLE_RUNTIME)where the latter is defined withThe ergonomic way to do this is beyond my knowledge, but I'm very happy to submit a patch if someone wants to provide some guidance.
picnixz commentedon Dec 21, 2024
cc @skirpichev
zanieb commentedon Dec 21, 2024
@picnixz you can also apply the
OS-maclabel, there are no runtime symbol checks for Linux yet.Thank you for your quick triage :)
skirpichev commentedon Dec 22, 2024
Looks as a false positive to me. I don't understand why we need MacOS-specific workaround when nothing is broken.
zanieb commentedon Dec 22, 2024
@skirpichev won't CPython segfault when this function is called on macOS older than 10.15?
The typical pattern on macOS is to guard usage, e.g., in these similar situations:
ptsname_ris not checked at runtime on macOS #123797This is also blocking adding a guard against future regressions, see #128155 (comment).
cc @ned-deily
ned-deily commentedon Dec 22, 2024
Context: this is for cases where CPython is built to run on a range of macOS releases, like we do for the python.org macOS installer builds. This involves building on a newer (current) version of macOS (like
15) but with MACOSX_DEPLOYMENT_TARGET set to an earlier version, say,10.13.skirpichev commentedon Dec 22, 2024
How? If libffi has no proper support for complex types, then configure script will not define
Py_FFI_SUPPORT_C_COMPLEX.Edit:
I see. So, for such builds you want to revert compile-time decision on
Py_FFI_SUPPORT_C_COMPLEXat runtime, right?That will require substantial rewrite of complex numbers support in ctypes.
skirpichev commentedon Dec 22, 2024
I would suggest rather unset Py_FFI_SUPPORT_C_COMPLEX for such builds at compile time.
zanieb commentedon Dec 22, 2024
Basically, yes.
That's a possibility, but weak symbols allow for portable builds which retain the functionality when available. This is common on macOS because there's robust support for it, but there's also interest in using weak symbols for Linux targets.
This is unfortunate. I'm not sure how to weigh the trade-offs here — but I'm curious if the idea that it's not macOS-specific changes your opinion?
Anyway, I don't think it's a great outcome to need to rewrite this module. I think an "easy" path forward here is to bump the minimum supported macOS version to 10.15 — it's not a big bump and @ned-deily had previously suggested this. There's some data on usage here.
We may need to revisit this module when adding weak symbols for Linux, but I have no clue how modern of library versions the functionality here depends on yet.
1 remaining item
zanieb commentedon Dec 22, 2024
We've set the minimum in CI to 10.15 but formalizing that in a policy will be a separate task (see #128155 (comment)).
I can't tell if it makes sense to leave this issue open since
ffi_type_complex_doubleusage is unguarded (and all of the other macOS 10.15 built-in usage is guarded). It sounds quite challenging to do anything about it though and we won't be enforcing guards for <=10.15 in the foreseeable future.encukou commentedon Jan 2, 2025
On the
ctypesside, I think I can add run-time checks to all uses ofPy_FFI_SUPPORT_C_COMPLEX, and everything that depends on that. (There's a few changes in this area that I wanted to make anyway.)But, I don't know much about macOS and I can only test on one macOS version.
I'd appreciate a patch that defines, on any platform with
Py_FFI_SUPPORT_C_COMPLEX, a function/macro that returnstrueiffffi_type_complex_*can be used. With that I can write a PR; after that I'd ask you to test it.To be clear:
float complexorcsqrtf()are fine; justffi_type_complex_*, right?Note that I changed the
cfield.ccode recently. If you have a draft in progress, you'll probably get conflicts with currentmain. If you need them resolved, push to a public branch and I can do the merge.skirpichev commentedon Jan 3, 2025
Not sure what you meant. We have two defines:
Py_HAVE_C_COMPLEX- if Annex G complex types are present and correctly works, andPy_FFI_SUPPORT_C_COMPLEX- if libffi really support complex types (it might just defineFFI_TARGET_HAS_COMPLEX_TYPE, I'm not sure if this issue is valid also for MacOS libffi).For the struct module,
Py_HAVE_C_COMPLEX- is enough. But in the ctypes - adding complex types is useless withoutPy_FFI_SUPPORT_C_COMPLEX. In principle, we could add c_*_complex in pure-Python (if platform support Annex G complex types):And than call e.g. libm's functions like
csqrt(). That was working for me before. But I'm not sure if that will work for all platforms.encukou commentedon Jan 3, 2025
Thanks for the clarification!
So,
ctypes.c_*_complexwill be missing if not available in libffi at runtime.(Later, we could add them, so that you can pass pointers to them as arguments or put them in Structures, but they wouldn't be usable as function arguments. But that's out of scope here.)
skirpichev commentedon Jan 3, 2025
Yes, if not properly supported by libffi at runtime. We can add such types, if Annex G C types are available, but using them as function arguments will produce garbage (see #125206).
encukou commentedon Jan 3, 2025
Not that different from something like
c_int.__ctype_be__, then...Anyway, back to this issue: I'd appreciate if a macOS expert could write the guard code -- it feels like a few lines that would take a lot of research for me.
zanieb commentedon Jan 3, 2025
@encukou for what it's worth, the macOS-specific parts are pretty transferable from other cases where we check for runtime availability, e.g., #128156 (comment) and #128156 (comment).
gh-128156: Guard use of `ffi_type_complex_double` on macOS system lib…
encukou commentedon Jan 21, 2025
Fixed in 3.14.
How important is it to backport this? Due to recent
ctypeschanges a backport wouldn't be straightforward, but I can do it if needed.skirpichev commentedon Jan 21, 2025
AFAIR, we don't have complex types in 3.13. (BTW, maybe it's worth a whatsnew entry.) So, I think this might be closed.
encukou commentedon Jan 21, 2025
Oh! Well, that's one assumption I didn't check :)
encukou commentedon Jan 21, 2025
PR for the whatsnew entry: #129129
zanieb commentedon Jan 21, 2025
Thanks @encukou !