MSVC ffi_prep_args doesn't handle 64-bit arguments properly

[zooba]

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]

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]

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

[zooba]

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]

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

[tjguk]

Likewise.

[python-dev]

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]

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

[RafaChodnicki]

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