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

PEP 353: Drop support for PyArg_ParseTuple() "#" formats when PY_SSIZE_T_CLEAN is not defined #85115

Closed
vstinner opened this issue Jun 10, 2020 · 11 comments
Labels
3.10 only security fixes topic-C-API

Comments

@vstinner
Copy link
Member

BPO 40943
Nosy @vstinner, @methane, @serhiy-storchaka, @miss-islington
PRs
  • bpo-40943: Replace PY_FORMAT_SIZE_T with "z" #20781
  • bpo-40943: PY_SSIZE_T_CLEAN required for '#' formats #20784
  • bpo-40943: Fix skipitem() didn't raise SystemError #25937
  • [3.10] bpo-40943: Fix skipitem() didn't raise SystemError (GH-25937) #25961
  • 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 2021-05-07.03:29:07.518>
    created_at = <Date 2020-06-10.16:09:12.080>
    labels = ['expert-C-API', '3.10']
    title = 'PEP 353: Drop support for PyArg_ParseTuple() "#" formats when PY_SSIZE_T_CLEAN is not defined'
    updated_at = <Date 2021-05-10.21:29:50.046>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2021-05-10.21:29:50.046>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-05-07.03:29:07.518>
    closer = 'methane'
    components = ['C API']
    creation = <Date 2020-06-10.16:09:12.080>
    creator = 'vstinner'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 40943
    keywords = ['patch']
    message_count = 11.0
    messages = ['371216', '371218', '371219', '371862', '380656', '393064', '393067', '393068', '393069', '393165', '393434']
    nosy_count = 4.0
    nosy_names = ['vstinner', 'methane', 'serhiy.storchaka', 'miss-islington']
    pr_nums = ['20781', '20784', '25937', '25961']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue40943'
    versions = ['Python 3.10']

    @vstinner
    Copy link
    Member Author

    Follow-up of bpo-36381: In Python 3.8, PyArg_ParseTuple() and Py_BuildValue() formats using "int" when PY_SSIZE_T_CLEAN is not defined, but Py_ssize_t when PY_SSIZE_T_CLEAN is defined, were deprecated by:

    commit d3c72a2
    Author: Inada Naoki <songofacandy@gmail.com>
    Date: Sat Mar 23 21:04:40 2019 +0900

    bpo-36381: warn when no PY_SSIZE_T_CLEAN defined (GH-12473)
    
    We will remove int support from 3.10 or 4.0.
    

    I propose to drop support for these formats in Python 3.10. It is a backward incompatible change on purpose, to ensure that all C extensions are compatible with objects larger than 2 GB, and that all C extensions behave the same.

    I'm not sure of the effects of this issue on bpo-27499 "PY_SSIZE_T_CLEAN conflicts with Py_LIMITED_API".

    @vstinner vstinner added 3.10 only security fixes topic-C-API labels Jun 10, 2020
    @vstinner vstinner changed the title Drop support for PyArg_ParseTuple() format when PY_SSIZE_T_CLEAN is not defined PEP 353: Drop support for PyArg_ParseTuple() format when PY_SSIZE_T_CLEAN is not defined Jun 10, 2020
    @vstinner vstinner changed the title Drop support for PyArg_ParseTuple() format when PY_SSIZE_T_CLEAN is not defined PEP 353: Drop support for PyArg_ParseTuple() format when PY_SSIZE_T_CLEAN is not defined Jun 10, 2020
    @vstinner
    Copy link
    Member Author

    New changeset d36cf5f by Victor Stinner in branch 'master':
    bpo-40943: Replace PY_FORMAT_SIZE_T with "z" (GH-20781)
    d36cf5f

    @vstinner
    Copy link
    Member Author

    I started to write a long rationale to explain why "#" variants of formats must be deprecated, before I noticed that they are already deprecated! To not lost it, here is my rationale :-)

    The C code base of Python switched from int to Py_ssize_t with PEP-353. For not break the backward compatibility, C extension modules have to opt-in for Py_ssize_t by defining PY_SSIZE_T_CLEAN macro in their code base. It affects a few format PyArg_ParseTuple() and Py_BuildValue():
    https://docs.python.org/dev/c-api/arg.html

    Currently, the documentation says "This behavior will change in a future Python version to only support Py_ssize_t and drop int support. It is best to always define PY_SSIZE_T_CLEAN."

    Continue to use int by default prevents to accept content larger than 2 GB and causes issues with limited C API: bpo-27499.

    I propose to raise a DeprecationWarning on C extension modules built without the PY_SSIZE_T_CLEAN macro defined.

    #warning could be used to emit a warning when building a C extension without PY_SSIZE_T_CLEAN. The problem is that the compiler output is hidden by "pip install", only few developers pay attention to such warnings, and also we should not bother C extensions which don't use PyArg_ParseTuple() and Py_BuildValue().

    I prefer to raise a DeprecationWarning exception at runtime since this warning can be made an error when using -Werror: it eases detection of deprecated modules without preventing to build or use a C extension using the deprecated functions.

    Two releases after the DeprecationWarning is introduced, we can consider to always raise an exception. Again, I dislike the idea of always require PY_SSIZE_T_CLEAN macro when including "Python.h" header file. I suggest to only raise an exception at runtime when the few affected functions are called.

    For example, PyArg_ParseTuple(args, "y*", &buffer) always Py_buffer which uses Py_ssize_t: the behavior does not depend on PY_SSIZE_T_CLEAN.

    An alternative is to even deprecate the few "#" formats which are affected by PY_SSIZE_T_CLEAN.

    @vstinner vstinner changed the title PEP 353: Drop support for PyArg_ParseTuple() format when PY_SSIZE_T_CLEAN is not defined PEP 353: Drop support for PyArg_ParseTuple() "#" formats when PY_SSIZE_T_CLEAN is not defined Jun 10, 2020
    @vstinner vstinner changed the title PEP 353: Drop support for PyArg_ParseTuple() format when PY_SSIZE_T_CLEAN is not defined PEP 353: Drop support for PyArg_ParseTuple() "#" formats when PY_SSIZE_T_CLEAN is not defined Jun 10, 2020
    @vstinner
    Copy link
    Member Author

    New changeset 37bb289 by Victor Stinner in branch 'master':
    bpo-40943: PY_SSIZE_T_CLEAN required for '#' formats (GH-20784)
    37bb289

    @vstinner
    Copy link
    Member Author

    @methane
    Copy link
    Member

    methane commented May 6, 2021

    There is a still warning, not error.

    cpython/Python/getargs.c

    Lines 2535 to 2542 in 985ac01

    if (flags & FLAG_SIZE_T)
    (void) va_arg(*p_va, Py_ssize_t *);
    else {
    if (PyErr_WarnEx(PyExc_DeprecationWarning,
    "PY_SSIZE_T_CLEAN will be required for '#' formats", 1)) {
    return NULL;
    }
    (void) va_arg(*p_va, int *);

    @methane methane reopened this May 6, 2021
    @methane methane reopened this May 6, 2021
    @serhiy-storchaka
    Copy link
    Member

    Why PR 20784 has been merged? Was not the plan to emit a deprecation warning first?

    @methane
    Copy link
    Member

    methane commented May 6, 2021

    Why PR 20784 has been merged? Was not the plan to emit a deprecation warning first?

    We had been emitted DeprecationWarning since Python 3.8. See #56682.
    #64983 changed the DeprecationWarning to SystemError.

    @serhiy-storchaka
    Copy link
    Member

    Sorry, I was confused by Victor's long msg371219.

    @miss-islington
    Copy link
    Contributor

    New changeset 569ca81 by Miss Islington (bot) in branch '3.10':
    bpo-40943: Fix skipitem() didn't raise SystemError (GH-25937)
    569ca81

    @methane methane closed this as completed May 7, 2021
    @methane methane closed this as completed May 7, 2021
    @vstinner
    Copy link
    Member Author

    commit 4ebf4a6
    Author: Inada Naoki <songofacandy@gmail.com>
    Date: Fri May 7 11:56:48 2021 +0900

    bpo-40943: Fix skipitem() didn't raise SystemError (GH-25937)
    
    `convertitem()` raises `SystemError` when '#' is used without `PY_SSIZE_T_CLEAN`.
    This commit makes `skipitem()` raise it too.
    

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

    No branches or pull requests

    4 participants