python / cpython

The Python programming language
https://www.python.org
Other
63.36k stars 30.34k forks source link

variadic function call broken on armhf when passing a float argument #83813

Open ce15e04c-0b87-43fe-a8b0-0528fcb65623 opened 4 years ago

ce15e04c-0b87-43fe-a8b0-0528fcb65623 commented 4 years ago
BPO 39632
Nosy @FFY00, @ndessart
PRs
  • python/cpython#18560
  • Files
  • ctypes_variadic_function_tests.diff: Reproduce this issue with ctypes unit tests on an armhf target.
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields: ```python assignee = None closed_at = None created_at = labels = ['3.8', '3.9', '3.10', '3.11', 'ctypes', '3.7'] title = 'variadic function call broken on armhf when passing a float argument' updated_at = user = 'https://github.com/ndessart' ``` bugs.python.org fields: ```python activity = actor = 'Nicolas Dessart' assignee = 'none' closed = False closed_date = None closer = None components = ['ctypes'] creation = creator = 'Nicolas Dessart' dependencies = [] files = ['48896'] hgrepos = [] issue_num = 39632 keywords = ['patch'] message_count = 4.0 messages = ['361992', '362307', '398026', '398040'] nosy_count = 2.0 nosy_names = ['FFY00', 'Nicolas Dessart'] pr_nums = ['18560'] priority = 'normal' resolution = None stage = 'patch review' status = 'open' superseder = None type = None url = 'https://bugs.python.org/issue39632' versions = ['Python 3.7', 'Python 3.8', 'Python 3.9', 'Python 3.10', 'Python 3.11'] ```

    ce15e04c-0b87-43fe-a8b0-0528fcb65623 commented 4 years ago

    On armhf and for variadic functions (and contrary to non-variadic functions), the VFP co-processor registers are not used for float argument parameter passing. This specificity is apparently completely disregarded by ctypes which always uses ffi_prep_cif to prepare the parameter passing of a function while it should most probably use ffi_prep_cif_var for variadic functions.

    As such variadic function call with float arguments through ctypes is currently broken on armhf targets.

    I think that ctypes API should be updated to let the user specify if a function is variadic.

    I've attached a patch to the ctypes unit tests that I'm using to reproduce this bug.

    pi@raspberrypi:~/code/cpython $ ./python -m test test_ctypes    
    0:00:00 load avg: 0.00 Run tests sequentially
    0:00:00 load avg: 0.00 [1/1] test_ctypes
    _testfunc_d_bhilfd_var got 2 3 4 -1242230680 -0.000000 -0.000000
    test test_ctypes failed -- Traceback (most recent call last):
      File "/home/pi/code/cpython/Lib/ctypes/test/test_functions.py", line 146, in test_doubleresult_var
        self.assertEqual(result, 21)
    AssertionError: -7.086855952261741e-44 != 21

    test_ctypes failed

    == Tests result: FAILURE ==

    1 test failed: test_ctypes

    Total duration: 3.8 sec Tests result: FAILURE

    ce15e04c-0b87-43fe-a8b0-0528fcb65623 commented 4 years ago

    As I said in the associated PR, the build is failing on macOS because ctypes uses an obsolete libffi version bundled into Modules/_ctypes/libffi_osx for this platform. This old version of libffi is missing ffi_prep_cif_var.

    There is an open issue that propose to remove the bundled libffi for OSX. https://bugs.python.org/issue28491

    FFY00 commented 3 years ago

    bpo-28491 is now resolved, so the PR should be unblocked. Would you mind rebasing it on main to retrigger the CI?

    ce15e04c-0b87-43fe-a8b0-0528fcb65623 commented 3 years ago

    I've just rebased this PR but the CI builds failed (for unrelated reasons?).

    That being said I think that this particular issue should mostly be resolved by bpo-41100 and PR-22855 in particular. My PR 18560 still brings the support for Ellipsis/... inside ctypes function arguments type list. This is nice because it allows ctypes to automatically perform the necessary type promotions even if a variadic function is called without extra arguments.

    Since I've rebased my PR on top of bpo-41100 PRs, I had to resolve a few conflicts but didn't tried to limit the diff to the minimum (PR 18560 may be simplified).

    https://bugs.python.org/issue41100 https://github.com/python/cpython/pull/22855