ctypes: pass by value for structs broken on Cygwin/MinGW 64-bit

[embray]

BPO	30353
Nosy	@vsajip, @ned-deily, @embray, @eryksun, @zhangyangyu, @stratakis, @irushchyshyn
PRs	bpo-30353 Fix pass by value for structs on 64-bit Cygwin/MinGW #1559 [3.6] bpo-30353: Fix pass by value for structs on 64-bit Cygwin/MinGW (GH-1559) #5954
Files	arm64_build_log.txt

^{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:

assignee = None
closed_at = <Date 2018-03-08.15:34:17.694>
created_at = <Date 2017-05-12.16:19:48.894>
labels = ['ctypes', 'type-bug', '3.7']
title = 'ctypes: pass by value for structs broken on Cygwin/MinGW 64-bit'
updated_at = <Date 2018-03-08.15:34:17.692>
user = 'https://github.com/embray'

bugs.python.org fields:

activity = <Date 2018-03-08.15:34:17.692>
actor = 'ned.deily'
assignee = 'none'
closed = True
closed_date = <Date 2018-03-08.15:34:17.694>
closer = 'ned.deily'
components = ['ctypes']
creation = <Date 2017-05-12.16:19:48.894>
creator = 'erik.bray'
dependencies = []
files = ['46892']
hgrepos = []
issue_num = 30353
keywords = ['patch']
message_count = 14.0
messages = ['293556', '293600', '293685', '293742', '293771', '293921', '294249', '294336', '294337', '294692', '295362', '295530', '313439', '313444']
nosy_count = 7.0
nosy_names = ['vinay.sajip', 'ned.deily', 'erik.bray', 'eryksun', 'xiang.zhang', 'cstratak', 'ishcherb']
pr_nums = ['1559', '5954']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue30353'
versions = ['Python 3.6', 'Python 3.7']

[embray]

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: libffi/libffi#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.

[zhangyangyu]

Looks same as bpo-29804.

[embray]

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

[stratakis]

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.

[irushchyshyn]

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 bpo-29804 as well?

[embray]

Sure, thanks for pointing that out.

[embray]

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.

[irushchyshyn]

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.

[embray]

Thanks for checking!

[vsajip]

Patch LGTM now, perhaps others will concur?

[vsajip]

New changeset 9ba3aa4 by Vinay Sajip (Erik Bray) in branch 'master':
bpo-30353: Fix pass by value for structs on 64-bit Cygwin/MinGW (GH-1559)
9ba3aa4

[stratakis]

This bug affects also the 3.6 branch. Can the fix be backported?

[embray]

This has a backport PR now for 3.6. Once that PR is merged I think we can close this as fixed.