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

Workaround for old libffi versions is added.
Module ctypes now supports C11 double complex only with libffi >= 3.3.0.

@skirpichev, could you please review this?

Issue: CTypes test failed, complex double problem #125206

Workaround for old libffi versions is added. Module ctypes now supports C11 double complex only with libffi >= 3.3.0.

…WRRK6.rst

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.

…OMPLEX

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...)

PEP 7 Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>

CC @vstinner

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

@thesamesam, that's a separate issue.

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?

CC @erlend-aasland

LGTM

Merged, thanks for the fix @efimov-mikhail.

Correction: #126104

And yet another: #132865. Sorry :(

pythongh-125206 : Bug in ctypes with old libffi is fixed

911788f

Workaround for old libffi versions is added. Module ctypes now supports C11 double complex only with libffi >= 3.3.0.

efimov-mikhail requested review from corona10 and erlend-aasland as code owners October 11, 2024 15:06

bedevere-app Bot mentioned this pull request Oct 11, 2024

CTypes test failed, complex double problem #125206

Closed

bedevere-app Bot added the awaiting review label Oct 11, 2024

skirpichev self-requested a review October 11, 2024 15:09

skirpichev reviewed Oct 11, 2024

View reviewed changes

Comment thread Misc/NEWS.d/next/Library/2024-10-11-18-03-05.gh-issue-125206.pWRRK6.rst Outdated

Update Misc/NEWS.d/next/Library/2024-10-11-18-03-05.gh-issue-125206.p…

b583c21

…WRRK6.rst

skirpichev reviewed Oct 11, 2024

View reviewed changes

pythongh-125206 : Py_FFI_TARGET_HAS_COMPLEX_TYPE -> Py_FFI_SUPPORT_C_C…

f0f54e2

…OMPLEX

efimov-mikhail requested a review from skirpichev October 11, 2024 15:42

skirpichev approved these changes Oct 11, 2024

View reviewed changes

bedevere-app Bot added awaiting core review and removed awaiting review labels Oct 11, 2024

This comment was marked as outdated.

Sign in to view

This comment was marked as resolved.

Sign in to view

This comment was marked as outdated.

Sign in to view

skirpichev reviewed Oct 12, 2024

View reviewed changes

Comment thread configure.ac Outdated

Update configure.ac

bcdf7b0

PEP 7 Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>

skirpichev reviewed Oct 12, 2024

View reviewed changes

Comment thread configure Outdated

Update configure

05f4b17

skirpichev approved these changes Oct 15, 2024

View reviewed changes

vstinner approved these changes Oct 15, 2024

View reviewed changes

bedevere-app Bot added awaiting merge and removed awaiting core review labels Oct 15, 2024

vstinner enabled auto-merge (squash) October 15, 2024 16:16

vstinner merged commit aac89b5 into python:main Oct 15, 2024

bedevere-app Bot removed the awaiting merge label Oct 15, 2024

efimov-mikhail deleted the ctypes_complex_double_libffi_issue branch October 15, 2024 16:23

skirpichev mentioned this pull request Oct 29, 2024

gh-125206: Correct detection of complex numbers support in libffi #126104

Merged

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

Uh oh!

Uh oh!

gh-125206: Bug in ctypes with old libffi is fixed - #125322

gh-125206: Bug in ctypes with old libffi is fixed#125322vstinner merged 5 commits intopython:mainpython/cpython:mainfrom efimov-mikhail:ctypes_complex_double_libffi_issueefimov-mikhail/cpython:ctypes_complex_double_libffi_issueCopy head branch name to clipboard

Conversation

efimov-mikhail commented Oct 11, 2024 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

skirpichev left a comment

Choose a reason for hiding this comment

Uh oh!

efimov-mikhail commented Oct 11, 2024

Uh oh!

skirpichev left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as outdated.

This comment was marked as outdated.

picnixz commented Oct 11, 2024

Uh oh!

skirpichev commented Oct 11, 2024

Uh oh!

efimov-mikhail commented Oct 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

skirpichev commented Oct 12, 2024

Uh oh!

Uh oh!

skirpichev commented Oct 12, 2024

Uh oh!

thesamesam commented Oct 14, 2024

Uh oh!

skirpichev commented Oct 14, 2024

Uh oh!

thesamesam commented Oct 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skirpichev commented Oct 14, 2024

Uh oh!

efimov-mikhail commented Oct 15, 2024

Uh oh!

skirpichev commented Oct 15, 2024

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

vstinner commented Oct 15, 2024

Uh oh!

skirpichev commented Oct 29, 2024

Uh oh!

skirpichev commented Apr 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

gh-125206: Bug in ctypes with old libffi is fixed#125322
vstinner merged 5 commits into
python:mainfrom
efimov-mikhail:ctypes_complex_double_libffi_issue

efimov-mikhail commented Oct 11, 2024 •
edited by bedevere-app Bot
Loading

efimov-mikhail commented Oct 11, 2024 •
edited
Loading

thesamesam commented Oct 14, 2024 •
edited
Loading