Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
embray opened this issue May 12, 2017 · 14 comments
Closed

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

embray opened this issue May 12, 2017 · 14 comments
Labels
3.7 (EOL) end of life topic-ctypes type-bug An unexpected behavior, bug, or error

Comments

@embray
Copy link
Contributor

embray commented May 12, 2017

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
    Copy link
    Contributor Author

    embray commented May 12, 2017

    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.

    @embray embray added topic-ctypes type-bug An unexpected behavior, bug, or error labels May 12, 2017
    @zhangyangyu
    Copy link
    Member

    Looks same as bpo-29804.

    @embray
    Copy link
    Contributor Author

    embray commented May 15, 2017

    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
    Copy link
    Mannequin

    stratakis mannequin commented May 15, 2017

    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
    Copy link
    Mannequin

    irushchyshyn mannequin commented May 16, 2017

    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
    Copy link
    Contributor Author

    embray commented May 18, 2017

    Sure, thanks for pointing that out.

    @embray
    Copy link
    Contributor Author

    embray commented May 23, 2017

    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
    Copy link
    Mannequin

    irushchyshyn mannequin commented May 24, 2017

    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
    Copy link
    Contributor Author

    embray commented May 24, 2017

    Thanks for checking!

    @vsajip
    Copy link
    Member

    vsajip commented May 29, 2017

    Patch LGTM now, perhaps others will concur?

    @vsajip
    Copy link
    Member

    vsajip commented Jun 7, 2017

    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
    Copy link
    Mannequin

    stratakis mannequin commented Jun 9, 2017

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

    @embray
    Copy link
    Contributor Author

    embray commented Mar 8, 2018

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

    @ned-deily
    Copy link
    Member

    New changeset 2f3ba27 by Ned Deily (Miss Islington (bot)) in branch '3.6':
    [3.6] bpo-30353: Fix pass by value for structs on 64-bit Cygwin/MinGW (GH-1559) (GH-5954)
    2f3ba27

    @ned-deily ned-deily added the 3.7 (EOL) end of life label Mar 8, 2018
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.7 (EOL) end of life topic-ctypes type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    4 participants
    @vsajip @embray @zhangyangyu @ned-deily