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

MSVC ffi_prep_args doesn't handle 64-bit arguments properly #66922

Closed
zooba opened this issue Oct 26, 2014 · 8 comments
Closed

MSVC ffi_prep_args doesn't handle 64-bit arguments properly #66922

zooba opened this issue Oct 26, 2014 · 8 comments
Labels
topic-ctypes

Comments

@zooba
Copy link
Member

zooba commented Oct 26, 2014

BPO 22733
Nosy @amauryfa, @abalkin, @tjguk, @meadori, @zware, @zooba
Files
  • ffi_prep_args.patch
    		•

    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 2014-12-17.14:41:44.832>
created_at = <Date 2014-10-26.20:35:54.747>
labels = ['ctypes']
title = "MSVC ffi_prep_args doesn't handle 64-bit arguments properly"
updated_at = <Date 2016-09-15.14:04:20.690>
user = 'https://github.com/zooba'

    bugs.python.org fields:

    activity = <Date 2016-09-15.14:04:20.690>
actor = 'Rafa\xc5\x82 Ch\xc5\x82odnicki'
assignee = 'none'
closed = True
closed_date = <Date 2014-12-17.14:41:44.832>
closer = 'steve.dower'
components = ['ctypes']
creation = <Date 2014-10-26.20:35:54.747>
creator = 'steve.dower'
dependencies = []
files = ['37023']
hgrepos = []
issue_num = 22733
keywords = ['patch']
message_count = 8.0
messages = ['230036', '231526', '232706', '232713', '232723', '232818', '232821', '276555']
nosy_count = 8.0
nosy_names = ['amaury.forgeotdarc', 'belopolsky', 'tim.golden', 'meador.inge', 'python-dev', 'zach.ware', 'steve.dower', 'Rafa\xc5\x82 Ch\xc5\x82odnicki']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue22733'
versions = ['Python 3.5']
    @zooba
    Copy link
    Member Author

    zooba commented Oct 26, 2014

    The ffi_prep_args function in libffi_msvc/ffi.c needs the attached patch to handle 64-bit parameters with the correct padding.

    Without this patch, garbage may appear in the top part of 64-bit arguments as the values are not zeroed out by the memcpy.

    I'm not 100% sure who 'owns' this file - is there an upstream project that should get the patch rather than Python? If not, I see no reason not to check this in now, even though it doesn't seem to repro with the VC10 builds.

    @zooba
    Copy link
    Member Author

    zooba commented Nov 22, 2014

    Nosying the other Windows guys - seems like the ctypes nosy list is inactive, and this only affects MSVC.

    @zooba
    Copy link
    Member Author

    zooba commented Dec 16, 2014

    This patch also resolves this test issue:

    [ 68/390/2] test_ttk_textonly
test test_ttk_textonly crashed -- Traceback (most recent call last):
  File "D:\...\lib\test\regrtest.py", line 1280, in runtest_inner
    test_runner()
  File "D:\...\lib\test\test_ttk_textonly.py", line 14, in test_main
    *runtktests.get_tests(gui=False, packages=['test_ttk']))
  File "D:\...\lib\tkinter\test\runtktests.py", line 65, in get_tests
    for module in get_tests_modules(gui=gui, packages=packages):
  File "D:\...\lib\tkinter\test\runtktests.py", line 50, in get_tests_modules
    "tkinter.test")
  File "D:\...\lib\importlib\__init__.py", line 109, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 2208, in _gcd_import
  File "<frozen importlib._bootstrap>", line 2191, in _find_and_load
  File "<frozen importlib._bootstrap>", line 2180, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 1149, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 1420, in exec_module
  File "<frozen importlib._bootstrap>", line 321, in _call_with_frames_removed
  File "D:\...\lib\tkinter\test\test_ttk\test_extensions.py", line 8, in <module>
    requires('gui')
  File "D:\...\lib\test\support\__init__.py", line 492, in requires
    if resource == 'gui' and not _is_gui_available():
  File "D:\...\lib\test\support\__init__.py", line 436, in _is_gui_available
    raise ctypes.WinError()
OSError: [WinError 0] The operation completed successfully.

    I'm not entirely sure when/if I should just commit the fix without any reviews, but this issue needs to be fixed, so eventually I'll probably just do it :)

    @zware
    Copy link
    Member

    zware commented Dec 16, 2014

    I don't know near enough to comment on this one. If you feel confident in it, I'd say go ahead.

    @tjguk
    Copy link
    Member

    tjguk commented Dec 16, 2014

    Likewise.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Dec 17, 2014

    New changeset 4c81b1846e14 by Steve Dower in branch 'default':
    Issue bpo-22733: MSVC ffi_prep_args doesn't handle 64-bit arguments properly
    https://hg.python.org/cpython/rev/4c81b1846e14

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Dec 17, 2014

    New changeset 24f4569a1308 by Steve Dower in branch 'default':
    Issue bpo-22733: Added NEWS item
    https://hg.python.org/cpython/rev/24f4569a1308

    @zooba zooba closed this as completed Dec 17, 2014
    @RafaChodnicki
    Copy link
    Mannequin

    RafaChodnicki mannequin commented Sep 15, 2016

    Maybe worth backporting to 3.3 and up? Especially if bpo-20160 is to be backported as they seem to be related.

    @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
    topic-ctypes
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @tjguk @zooba @zware