classification
Title: ctypes: pass by value for structs broken on Cygwin/MinGW 64-bit
Type: behavior Stage:
Components: ctypes Versions:
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: cstratak, erik.bray, ishcherb, vinay.sajip, xiang.zhang
Priority: normal Keywords:

Created on 2017-05-12 16:19 by erik.bray, last changed 2017-05-24 09:02 by erik.bray.

Files
File name Uploaded Description Edit
arm64_build_log.txt ishcherb, 2017-05-24 08:53
Pull Requests
URL Status Linked Edit
PR 1559 open erik.bray, 2017-05-12 16:27
Messages (9)
msg293556 - (view) Author: Erik Bray (erik.bray) * Date: 2017-05-12 16:19
The test ctypes.test.test_structures.StructureTestCase.test_pass_by_value fails on 64-bit Cygwin and MinGW using the system libffi with:

======================================================================
FAIL: test_pass_by_value (ctypes.test.test_structures.StructureTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/embray/src/python/cpython/Lib/ctypes/test/test_structures.py", line 416, in test_pass_by_value
    self.assertEqual(s.first, 0xdeadbeef)
AssertionError: 195948557 != 3735928559


It seems that libffi does not handle passing structs by value properly on those platforms as I explained here: https://github.com/libffi/libffi/issues/305

The upstream bug hasn't been confirmed yet by the libffi developers so I could be wrong, but I think this is fairly clearly broken there.

I have a PR forthcoming to work around the issue.
msg293600 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-05-13 07:49
Looks same as #29804.
msg293685 - (view) Author: Erik Bray (erik.bray) * Date: 2017-05-15 08:20
Thanks for pointing that out.  Indeed it's the same symptom, but slightly different cause.  libffi has different implementations of its calling routines for different architectures/platforms depending on the machine architecture and calling conventions used.  So this case is probably buggy for the arm64 implementation as well, as in #29804.

Off the top of my head I don't know a reliable way to check for this case, but this fix would work around the arm64 issue as well if I could check for that architecture easily at compile time.
msg293742 - (view) Author: Charalampos Stratakis (cstratak) * Date: 2017-05-15 22:34
A 'defined(__aarch64__)' can be used for the arm64 arch. I will add it to your patch and test it on an arm64 machine to see if the test passes.
msg293771 - (view) Author: Iryna Shcherbina (ishcherb) * Date: 2017-05-16 16:20
I have added `defined(__aarch64__)` check to the if statement and tested the patch on arm64. The test passed.

So could you please also add the check for `defined(__aarch64__)` to the pull request so that it fixes #29804 as well?
msg293921 - (view) Author: Erik Bray (erik.bray) * Date: 2017-05-18 12:38
Sure, thanks for pointing that out.
msg294249 - (view) Author: Erik Bray (erik.bray) * Date: 2017-05-23 14:33
Iryna, I updated the pull request with a slightly updated fix.  Could you confirm again, when you get a chance, that it works for arm64?  Thanks.
msg294336 - (view) Author: Iryna Shcherbina (ishcherb) * Date: 2017-05-24 08:53
Thank you, Erik. I have applied your new patch, and ran another build on arm64. The tests passed. Attaching the build log for more information.
msg294337 - (view) Author: Erik Bray (erik.bray) * Date: 2017-05-24 09:02
Thanks for checking!
History
Date User Action Args
2017-05-24 09:02:00erik.braysetmessages: + msg294337
2017-05-24 08:55:32ishcherbsetfiles: + arm64_build_log.txt

messages: + msg294336
2017-05-23 14:33:01erik.braysetmessages: + msg294249
2017-05-18 12:38:46erik.braysetmessages: + msg293921
2017-05-16 16:20:34ishcherbsetmessages: + msg293771
2017-05-15 22:47:16cstrataksetnosy: + ishcherb
2017-05-15 22:34:34cstrataksetnosy: + cstratak
messages: + msg293742
2017-05-15 08:20:07erik.braysetmessages: + msg293685
2017-05-13 07:49:43xiang.zhangsetnosy: + vinay.sajip, xiang.zhang
messages: + msg293600
2017-05-12 16:27:46erik.braysetpull_requests: + pull_request1656
2017-05-12 16:19:48erik.braycreate