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.

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

Some C-API symbols (e.g. Py_FrozenMain) are not always exported on Unix #88299

Closed
vstinner opened this issue May 14, 2021 · 21 comments
Closed

Some C-API symbols (e.g. Py_FrozenMain) are not always exported on Unix #88299

vstinner opened this issue May 14, 2021 · 21 comments
Labels
3.11 bug and security fixes topic-C-API

Comments

@vstinner
Copy link
Member

BPO 44133
Nosy @vstinner, @encukou, @miss-islington, @kulikjak, @shihai1991
PRs
  • bpo-44133: Export Py_FrozenMain() symbol #26130
  • [3.10] bpo-43795: Add a test for Stable ABI symbol availability using ctypes (GH-26354) #29148
  • bpo-44133: Export Py_FrozenMain() symbol. #29876
  • bpo-44133: Link Python executable with object files #30556
  • bpo-44133: Skip PyThread_get_thread_native_id() if not available #30636
  • Files
  • reproduce.tar.gz
  • 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-01-17.13:50:21.465>
    created_at = <Date 2021-05-14.16:58:44.058>
    labels = ['expert-C-API', '3.11']
    title = 'Some C-API symbols (e.g. Py_FrozenMain) are not always exported on Unix'
    updated_at = <Date 2022-04-01.08:55:02.796>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2022-04-01.08:55:02.796>
    actor = 'kulikjak'
    assignee = 'none'
    closed = True
    closed_date = <Date 2022-01-17.13:50:21.465>
    closer = 'vstinner'
    components = ['C API']
    creation = <Date 2021-05-14.16:58:44.058>
    creator = 'vstinner'
    dependencies = []
    files = ['50043']
    hgrepos = []
    issue_num = 44133
    keywords = ['patch']
    message_count = 17.0
    messages = ['393675', '393676', '393693', '404596', '404723', '404748', '406980', '410401', '410503', '410504', '410756', '410757', '410762', '410776', '410787', '416378', '416475']
    nosy_count = 5.0
    nosy_names = ['vstinner', 'petr.viktorin', 'miss-islington', 'kulikjak', 'shihai1991']
    pr_nums = ['26130', '29148', '29876', '30556', '30636']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue44133'
    versions = ['Python 3.11']

    @vstinner
    Copy link
    Member Author

    When Python is built without --enable-shared, the "Py_FrozenMain" is not exported.

    When Python is built with --enable-shared, the "Py_FrozenMain" is exported as expected.

    I can reproduce the issue with attached reproduce.tar.gz example:

    • build.sh exports "func1" symbol
    • build_ar.sh doesn't export the "func1" symbol

    The difference is that build.sh links directly two object files (.o) into a binary, whereas build_ar.sh creates a static library (.a) and then link the static library into a binary.

    Python is always built with a static library (libpythonVERSION.a) which causes the issue.

    A solution is to pass -Wl,--whole-archive option to the linker to export *all* symbols exported by all object files, and not only export symbols of the object files which are used.

    I'm not sure why, but "Py_FrozenMain" is the *only* impacted symbol of the whole C API.

    This issue was discovered by Petr Viktorin who works on the stable ABI:
    https://mail.python.org/archives/list/capi-sig@python.org/thread/5QLI3NUP3OSGLCCIBAQOTX4GEJQBWJ6F/

    @vstinner vstinner added 3.11 bug and security fixes topic-C-API labels May 14, 2021
    @vstinner
    Copy link
    Member Author

    The "python" binary exports 1610 symbols. With the attached fix, it exports 1611 symbols (+1): "Py_FrozenMain" is also exported :-)

    @vstinner
    Copy link
    Member Author

    The symbol is not exported on Windows neither. My PR 26126 ("Test Py_FrozenMain()") fails on Windows with:

    "_testembed.obj : error LNK2001: unresolved external symbol __imp_Py_FrozenMain"

    ("__imp_" prefix is added by Windows DLL loader.)

    @encukou
    Copy link
    Member

    encukou commented Oct 21, 2021

    I'm not sure why, but "Py_FrozenMain" is the *only* impacted symbol of the whole C API.

    Apparently, on some platforms PyModule_Create2 and PyModule_FromDefAndSpec2 don't appear either.

    Should I rename the issue to cover these as well?

    @encukou
    Copy link
    Member

    encukou commented Oct 22, 2021

    New changeset 276468d by Petr Viktorin in branch 'main':
    bpo-43795: Add a test for Stable ABI symbol availability using ctypes (GH-26354)
    276468d

    @encukou
    Copy link
    Member

    encukou commented Oct 22, 2021

    Repurposing this bug to track all symbols that don't appear in some configurations.

    • Py_FrozenMain should be added to stable ABI when this is fixed. It was left out of the 3.10 stable ABI for only this reason. See discussion here: https://mail.python.org/archives/list/capi-sig@python.org/thread/5QLI3NUP3OSGLCCIBAQOTX4GEJQBWJ6F/

    • PyModule_Create2 and PyModule_FromDefAndSpec2 are "temporarily" excluded from tests in [Tools/scripts/stable_abi.py](https://github.com/python/cpython/blob/main/Tools/scripts/stable_abi.py)'s gen_ctypes_test, so that all the other symbols can get regression testing. When this issue is fixed, that special case should be removed.

    @encukou encukou changed the title "Py_FrozenMain" symbol is not exported Some C-API symbols (e.g. Py_FrozenMain) are not always exported Oct 22, 2021
    @encukou encukou changed the title "Py_FrozenMain" symbol is not exported Some C-API symbols (e.g. Py_FrozenMain) are not always exported Oct 22, 2021
    @kulikjak
    Copy link
    Mannequin

    kulikjak mannequin commented Nov 25, 2021

    On Solaris (and most likely several other platforms), PyThread_get_thread_native_id is also not available.

    @vstinner
    Copy link
    Member Author

    On Solaris (and most likely several other platforms), PyThread_get_thread_native_id is also not available.

    Oh, I added an explicit test for that in my PR 30556.

    Py_FrozenMain should be added to stable ABI

    I suggest to first fix the issue (export the symbol), and then write a second PR to add it to the stable ABI.

    @vstinner
    Copy link
    Member Author

    New changeset 6be8489 by Victor Stinner in branch 'main':
    bpo-44133: Link Python executable with object files (GH-30556)
    6be8489

    @vstinner
    Copy link
    Member Author

    New changeset 6be8489 by Victor Stinner in branch 'main':
    bpo-44133: Link Python executable with object files (GH-30556)

    Sadly, Py_FrozenMain() is still missing on Windows. See:
    #30556 (comment)

    Until Windows also exports the symbol, I don't think that we can add the symbol to the stable ABI.

    I prefer to not backport the change since it's always risky to break the build system on a stable branch.

    If someone wants to fix the Windows build to also export Py_FrozenMain(), please open a new issue.

    I consider that the initial issue is fixed, so I close the issue.

    @vstinner vstinner changed the title Some C-API symbols (e.g. Py_FrozenMain) are not always exported Some C-API symbols (e.g. Py_FrozenMain) are not always exported on Unix Jan 13, 2022
    @vstinner vstinner changed the title Some C-API symbols (e.g. Py_FrozenMain) are not always exported Some C-API symbols (e.g. Py_FrozenMain) are not always exported on Unix Jan 13, 2022
    @kulikjak
    Copy link
    Mannequin

    kulikjak mannequin commented Jan 17, 2022

    On Solaris (and most likely several other platforms), PyThread_get_thread_native_id is also not available.

    Oh, I added an explicit test for that in my PR 30556.

    Now it started failing on a different place:

    ======================================================================
    FAIL: test_export_symbols (test.test_capi.CAPITest) (name='PyThread_get_thread_native_id')
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/..../cpython-main/Lib/test/test_capi.py", line 662, in test_export_symbols
        self.assertTrue(hasattr(ctypes.pythonapi, name))
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    AssertionError: False is not true

    Looking at the test, is the expectation that all OSes must implement it since 3.11?

    @vstinner
    Copy link
    Member Author

    Oh no, I expected that the new way to build Python would also export PyThread_get_thread_native_id() on Solaris. I reopen the issue.

    Can you please specify your configure command? Can you check without Python if the symbol is exported or not? If you use --enable-shared, check in libpython, otherwise check in the "python" binary.

    @vstinner vstinner reopened this Jan 17, 2022
    @vstinner vstinner reopened this Jan 17, 2022
    @kulikjak
    Copy link
    Mannequin

    kulikjak mannequin commented Jan 17, 2022

    Ah, sorry, I could have described the issue better. It's not a problem with exporting, PyThread_get_thread_native_id() isn't available on Solaris (and possibly other platforms) at all.

    https://github.com/python/cpython/blob/main/Include/pythread.h#L28
    https://github.com/python/cpython/blob/main/Python/thread_pthread.h#L329

    The reason I didn't implement it yet is that Solaris doesn't expose anything like native thread id. We do have functions like _lwp_self() or pthread_self() or thr_self() but neither of them returns id where "value may be used to uniquely identify this particular thread system-wide". (I presume that means that no other thread of no other process running on a given system would return the same number - all these functions return single digit numbers so there is no way they are unique system wide).

    If necessary, I guess that such a number can be created by masking pid and thread id together, but then there's a question of how it is supposed to be used (because OS would not understand it).

    @vstinner
    Copy link
    Member Author

    Ah, sorry, I could have described the issue better. It's not a problem with exporting, PyThread_get_thread_native_id() isn't available on Solaris (and possibly other platforms) at all.

    Oh ok, it's a simple bug in my test. I wrote #74821 to fix it.

    @vstinner
    Copy link
    Member Author

    New changeset 16901c0 by Victor Stinner in branch 'main':
    bpo-44133: Skip PyThread_get_thread_native_id() if not available (GH-30636)
    16901c0

    @encukou
    Copy link
    Member

    encukou commented Mar 30, 2022

    Ah, sorry, I could have described the issue better. It's not a problem with exporting, PyThread_get_thread_native_id() isn't available on Solaris (and possibly other platforms) at all.

    Jakub, does this mean test_stable_abi_ctypes fails on Solaris?
    Are there any other missing functions?
    I opened bpo-47169 to improve the situation.

    @kulikjak
    Copy link
    Mannequin

    kulikjak mannequin commented Apr 1, 2022

    Yes, it still does, and PyThread_get_thread_native_id is the only symbol missing.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @encukou
    Copy link
    Member

    encukou commented Apr 29, 2022

    PyModule_Create2 and PyModule_FromDefAndSpec2 are still missing on Arch Linux TraceRefs BuildBot :(
    https://buildbot.python.org/all/#/builders/82/builds/548

    @encukou encukou reopened this Apr 29, 2022
    @vstinner
    Copy link
    Member Author

    PyModule_Create2 and PyModule_FromDefAndSpec2 are still missing on Arch Linux TraceRefs BuildBot :(
    https://buildbot.python.org/all/#/builders/82/builds/548

    The Py_TRACE_REF macro changes the ABI on purpose, I suggest you to open a separated issue if you plan to implement the stable ABI for this special debug build:

    This build is not ABI compatible with release build (default build) or debug build (Py_DEBUG and Py_REF_DEBUG macros).

    Extract of modsupport.h:

    #ifdef Py_TRACE_REFS
     /* When we are tracing reference counts, rename module creation functions so
        modules compiled with incompatible settings will generate a
        link-time error. */
     #define PyModule_Create2 PyModule_Create2TraceRefs
     #define PyModule_FromDefAndSpec2 PyModule_FromDefAndSpec2TraceRefs
    #endif

    Py_TRACE_REF is incompatible with the stable ABI, extract of object.h:

    #if defined(Py_LIMITED_API) && defined(Py_TRACE_REFS)
    #  error Py_LIMITED_API is incompatible with Py_TRACE_REFS
    #endif
    

    The PyObject layout is different, it adds two pointers to the structure: _ob_next and _ob_prev. In Python 3.8, I modified the Python debug build to make it ABI compatible with the release build by not defining Py_TRACE_REF if Py_DEBUG is defined.

    Someone proposed to reimplement Py_TRACE_REF with a separated list or hash table of all objects (the _ob_prev and _ob_next linked list), rather than modifying PyObject. But nobody implemented it.

    Note: Py_TRACE_REF requires additional _Py_AddToAllObjects() and _Py_ForgetReference() function calls in _Py_NewReference() and _Py_Dealloc(). But hopefully, _Py_NewReference() and _Py_Dealloc() are function calls at the ABI level, so the different implementation is not an issue. It seems like the only blocker issue is the PyObject layout change.

    @encukou
    Copy link
    Member

    encukou commented Apr 29, 2022

    Ah! I see. Sorry, I just added a quick comment and didn't investigate further immediately.
    I reopened #91325 instead :)

    @encukou encukou closed this as completed Apr 29, 2022
    @vstinner
    Copy link
    Member Author

    No problem, thanks for working on clarifying the stable ABI!

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.11 bug and security fixes topic-C-API
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants