gh-125206: Bug in ctypes with old libffi is fixed by efimov-mikhail · Pull Request #125322 · python/cpython

Mostly LGTM.

One minor suggestion, lets rename Py_FFI_TARGET_HAS_COMPLEX_TYPE to something like Py_FFI_SUPPORT_C_COMPLEX.

Only non-trivial change here is in the configure.ac. I assume, you have tested this check on your system with old libffi.

Probably, this should be tested with build bots. Old workaround with FFI_TARGET_HAS_COMPLEX_TYPE was working e.g. on Sparc #120894 (comment)

One minor suggestion, lets rename Py_FFI_TARGET_HAS_COMPLEX_TYPE to something like Py_FFI_SUPPORT_C_COMPLEX.

Ok.

Only non-trivial change here is in the configure.ac.

Definitely.

I assume, you have tested this check on your system with old libffi.

Yes. It works fine with libffi.so.6:

root@efimov:/usr/lib/x86_64-linux-gnu# rm libffi.so && ln -s libffi.so.6 libffi.so
-> % (./configure --with-pydebug && make -j && ./python -m unittest -v test.test_ctypes.test_libc.LibTest.test_csqrt) 2>&1 | grep -E 'libffi|lffi'
checking for libffi... yes
checking libffi has complex type support... no
gcc -pthread -shared      Modules/_ctypes/_ctypes.o Modules/_ctypes/callbacks.o Modules/_ctypes/callproc.o Modules/_ctypes/stgdict.o Modules/_ctypes/cfield.o -lffi -ldl  -o Modules/_ctypes.cpython-314d-x86_64-linux-gnu.so
gcc -pthread -shared      Modules/_ctypes/_ctypes_test.o -lffi -ldl -lm  -o Modules/_ctypes_test.cpython-314d-x86_64-linux-gnu.so
test_csqrt (test.test_ctypes.test_libc.LibTest.test_csqrt) ... skipped 'requires C11 complex type and libffi >= 3.3.0'

And with libffi.so.7:

root@efimov:/usr/lib/x86_64-linux-gnu# rm libffi.so && ln -s libffi.so.7 libffi.so
-> % (./configure --with-pydebug && make -j && ./python -m unittest -v test.test_ctypes.test_libc.LibTest.test_csqrt) 2>&1 | grep -E 'libffi|lffi'
checking for libffi... yes
checking libffi has complex type support... yes
gcc -pthread -shared      Modules/_ctypes/_ctypes.o Modules/_ctypes/callbacks.o Modules/_ctypes/callproc.o Modules/_ctypes/stgdict.o Modules/_ctypes/cfield.o -lffi -ldl  -o Modules/_ctypes.cpython-314d-x86_64-linux-gnu.so
gcc -pthread -shared      Modules/_ctypes/_ctypes_test.o -lffi -ldl -lm  -o Modules/_ctypes_test.cpython-314d-x86_64-linux-gnu.so

Probably, this should be tested with build bots. Old workaround with FFI_TARGET_HAS_COMPLEX_TYPE was working e.g. on Sparc #120894 (comment)

I know nothing about build bot configuration.
It would be great if somebody helps me with it.

LGTM

But someone else should trigger testing with buildbots. This and/or this.

@skirpichev You can request build bots now that you're a member :)

SPARCv9 fails, but I see no failures, related to affected test. On another hand, configure now prints: checking libffi has complex type support... yes. Maybe this bot now uses newer libffi?

Lets wait PPC64LE, I guess it's queried.

SPARCv9 fails, but I see no failures, related to affected test. On another hand, configure now prints: checking libffi has complex type support... yes. Maybe this bot now uses newer libffi?

Maybe it is.

I can see such line in log from this issue:

gcc -fno-strict-overflow -I/usr/lib/sparcv9/libffi-3.2.1/include -fno-strict-overflow -fstack-protector-strong -Wtrampolines -Wsign-compare -g -Og -Wall -D_REENTRANT   -std=c11 -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wstrict-prototypes -Werror=implicit-function-declaration -fvisibility=hidden  -I./Include/internal -I./Include/internal/mimalloc  -I. -I./Include    -fPIC -c ./Modules/_ctypes/_ctypes.c -o Modules/_ctypes/_ctypes.o

But in actual log it differs:

gcc -fno-strict-overflow -DFFI_NO_RAW_API -fno-strict-overflow -Wsign-compare -g -Og -Wall -D_REENTRANT   -std=c11 -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wstrict-prototypes -Werror=implicit-function-declaration -fvisibility=hidden  -I./Include/internal -I./Include/internal/mimalloc  -I. -I./Include    -fPIC -c ./Modules/_ctypes/_ctypes.c -o Modules/_ctypes/_ctypes.o

Lets wait PPC64LE, I guess it's queried.

Yes, it is.

Ok, PPC64LE build was successful (and it doesn't detect a working libffi, as expected). I can't suggest some other built bot to test. LGTM.

PS: It's silly that PKG_CHECK_MODULES doesn't report a version found. (Of course, we could echoing somewhere pkg-config --modversion libffi, but...)

Not to derail this at all, but may want to consider looking at #103438 (#103439) in a followup too?

Yes, I know. I'm mentioning it because there's other libffi cleanups which have gone awry because an approach couldn't be agreed upon for them. The same approach used here could work, but nevermind.

The same approach used here could work

It was already suggested in the referenced issue thread.

Maybe, there is anything I could improve?