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 cfield.c defines duplicate ffi_type_* symbols #90056

Closed
tiran opened this issue Nov 25, 2021 · 5 comments
Closed

ctypes cfield.c defines duplicate ffi_type_* symbols #90056

tiran opened this issue Nov 25, 2021 · 5 comments
Labels
3.11 only security fixes topic-ctypes type-bug An unexpected behavior, bug, or error

Comments

@tiran
Copy link
Member

tiran commented Nov 25, 2021

BPO 45898
Nosy @pfmoore, @tiran, @tjguk, @zware, @zooba
PRs
  • bpo-45898: Remove duplicate symbols from _ctypes/cfield.c (GH-29791) #29791
    		•

    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 2022-02-24.20:00:27.039>
created_at = <Date 2021-11-25.11:57:40.881>
labels = ['ctypes', 'type-bug', '3.11']
title = 'ctypes cfield.c defines duplicate ffi_type_* symbols'
updated_at = <Date 2022-02-24.20:00:27.038>
user = 'https://github.com/tiran'

    bugs.python.org fields:

    activity = <Date 2022-02-24.20:00:27.038>
actor = 'christian.heimes'
assignee = 'none'
closed = True
closed_date = <Date 2022-02-24.20:00:27.039>
closer = 'christian.heimes'
components = ['ctypes']
creation = <Date 2021-11-25.11:57:40.881>
creator = 'christian.heimes'
dependencies = []
files = []
hgrepos = []
issue_num = 45898
keywords = ['patch']
message_count = 5.0
messages = ['406991', '407038', '407088', '413936', '413937']
nosy_count = 6.0
nosy_names = ['paul.moore', 'christian.heimes', 'tim.golden', 'zach.ware', 'steve.dower', 'Roman Yurchak']
pr_nums = ['29791']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue45898'
versions = ['Python 3.11']
    @tiran
    Copy link
    Member Author

    tiran commented Nov 25, 2021

    ctypes's cfield.c redefines a couple of symbols like ffi_type_void and others. The symbols are exported by ffi.h and libffi for more than 12 years: https://github.com/libffi/libffi/blame/e1539266e6c6dde3c99832323586f33f977d1dc0/include/ffi.h.in#L184

    I think we can safely remove the symbols from the file. The idea is inspired by pyodide patch https://github.com/pyodide/pyodide/blob/main/cpython/patches/remove-duplicate-symbols-from-cfield.c.patch

    @tiran tiran added 3.9 only security fixes 3.10 only security fixes 3.11 only security fixes topic-ctypes type-bug An unexpected behavior, bug, or error labels Nov 25, 2021
    @tiran
    Copy link
    Member Author

    tiran commented Nov 26, 2021

    Windows tests are failing without ffi_type_ symbols:

    cfield.obj : error LNK2001: unresolved external symbol _ffi_type_double [D:\a\cpython\cpython\PCbuild\_ctypes.vcxproj]
    cfield.obj : error LNK2001: unresolved external symbol _ffi_type_float [D:\a\cpython\cpython\PCbuild\_ctypes.vcxproj]
    cfield.obj : error LNK2001: unresolved external symbol _ffi_type_uint32 [D:\a\cpython\cpython\PCbuild\_ctypes.vcxproj]
    cfield.obj : error LNK2001: unresolved external symbol _ffi_type_sint8 [D:\a\cpython\cpython\PCbuild\_ctypes.vcxproj]
    cfield.obj : error LNK2001: unresolved external symbol _ffi_type_uint8 [D:\a\cpython\cpython\PCbuild\_ctypes.vcxproj]
    cfield.obj : error LNK2001: unresolved external symbol _ffi_type_uint64 [D:\a\cpython\cpython\PCbuild\_ctypes.vcxproj]
    cfield.obj : error LNK2001: unresolved external symbol _ffi_type_uint16 [D:\a\cpython\cpython\PCbuild\_ctypes.vcxproj]
    cfield.obj : error LNK2001: unresolved external symbol _ffi_type_sint16 [D:\a\cpython\cpython\PCbuild\_ctypes.vcxproj]

    @zooba
    Copy link
    Member

    zooba commented Nov 26, 2021

    This is because we use libffi as a DLL and you can't statically reference dynamically loaded addresses on Windows.

    You could change that format table to be initialised on first use, or resolve the FFI type lazily. Or change ctypes to statically link libffi (including updating our FFI build, which IIRC is the one that requires Cygwin, to generate static libraries instead of dynamic) - we can't backport this change, either, because it affects the layout on disk.

    @tiran
    Copy link
    Member Author

    tiran commented Feb 24, 2022

    New changeset 38f331d by Christian Heimes in branch 'main':
    bpo-45898: Remove duplicate symbols from _ctypes/cfield.c (GH-29791)
    38f331d

    @tiran
    Copy link
    Member Author

    tiran commented Feb 24, 2022

    I have applied the patch to main branch (3.11). The workaround for Windows turned out to be more elaborate than initially anticipated. I'm not going to backport the fix to 3.10 and 3.9.

    @tiran tiran removed 3.9 only security fixes 3.10 only security fixes labels Feb 24, 2022
    @tiran tiran closed this as completed Feb 24, 2022
    @tiran tiran removed the 3.10 only security fixes label Feb 24, 2022
    @tiran tiran closed this as completed Feb 24, 2022
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @ronaldoussoren ronaldoussoren mentioned this issue Oct 10, 2022
    Closed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.11 only security fixes topic-ctypes type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @tiran @zooba