Skip to content

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

Closed
Closed
MSVC ffi_prep_args doesn't handle 64-bit arguments properly#66922
Labels
topic-ctypes

Description

@zooba
zooba
opened on 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']

    Activity

    1. zooba commented on Oct 26, 2014

      @zooba
      zooba
      on Oct 26, 2014
      MemberAuthor

      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.

    2. zooba
      added
      topic-ctypes
      on Oct 26, 2014

    3. zooba commented on Nov 22, 2014

      @zooba
      zooba
      on Nov 22, 2014
      MemberAuthor

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

    4. zooba commented on Dec 16, 2014

      @zooba
      zooba
      on Dec 16, 2014
      MemberAuthor

      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 :)

    5. zware commented on Dec 16, 2014

      @zware
      zware
      on Dec 16, 2014
      Member

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

    6. tjguk commented on Dec 16, 2014

      @tjguk
      tjguk
      on Dec 16, 2014
      Member

      Likewise.

    7. python-dev commented on Dec 17, 2014

      @python-dev
      python-devmannequin
      on Dec 17, 2014
      Mannequin

      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

    8. python-dev commented on Dec 17, 2014

      @python-dev
      python-devmannequin
      on Dec 17, 2014
      Mannequin

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

    9. zooba
      closed this as completedon Dec 17, 2014

    10. RafaChodnicki commented on Sep 15, 2016

      @RafaChodnicki
      RafaChodnickimannequin
      on Sep 15, 2016
      Mannequin

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

    11. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      topic-ctypes

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions