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

Jump to bottom

PyStructSequence_NewType broken in 3.8 #86249

Closed
wdi2 mannequin opened this issue Oct 19, 2020 · 6 comments
Closed

PyStructSequence_NewType broken in 3.8 #86249

wdi2 mannequin opened this issue Oct 19, 2020 · 6 comments
Labels
3.9 only security fixes 3.10 only security fixes topic-C-API type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@wdi2
Copy link
Mannequin

wdi2 mannequin commented Oct 19, 2020

BPO 42083
Nosy @encukou, @ambv, @miss-islington, @stestagg, @Fidget-Spinner
PRs
  • [3.9] bpo-41832, bpo-42083: PyType_FromModuleAndSpec() can accept the NULL tp_doc slot (GH-25687) #25852
  • bpo-42083: Add C-API tests for PyStructSequence_NewType doc=NULL #25886
  • [3.10] bpo-42083: Add C-API tests for PyStructSequence_NewType doc=NULL #25887
  • [3.9] bpo-42083: Allow NULL doc in PyStructSequence_NewType #25896
    		•

    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-04.14:23:08.466>
created_at = <Date 2020-10-19.18:27:05.410>
labels = ['expert-C-API', '3.10', '3.9', 'type-crash']
title = 'PyStructSequence_NewType broken in 3.8'
updated_at = <Date 2021-05-04.14:23:08.466>
user = 'https://bugs.python.org/wdi2'

    bugs.python.org fields:

    activity = <Date 2021-05-04.14:23:08.466>
actor = 'kj'
assignee = 'none'
closed = True
closed_date = <Date 2021-05-04.14:23:08.466>
closer = 'kj'
components = ['C API']
creation = <Date 2020-10-19.18:27:05.410>
creator = 'wdi2'
dependencies = []
files = []
hgrepos = []
issue_num = 42083
keywords = ['patch']
message_count = 6.0
messages = ['378978', '382853', '392686', '392893', '392899', '392901']
nosy_count = 6.0
nosy_names = ['petr.viktorin', 'lukasz.langa', 'miss-islington', 'stestagg', 'kj', 'wdi2']
pr_nums = ['25852', '25886', '25887', '25896']
priority = 'high'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue42083'
versions = ['Python 3.9', 'Python 3.10']
    @wdi2
    Copy link
    Mannequin Author

    wdi2 mannequin commented Oct 19, 2020

    Calling PyStructSequence_NewType() with a NULL field in the desc.doc parameter (explicitly allowed as per docs) leads to a crash in

    Objects/typeobject.c:2956
    2956 size_t len = strlen(old_doc)+1;

    where old_doc is NULL.
    If the doc string is set, the call succeeds, but with a warning

    (stdin):1: DeprecationWarning: builtin type G_SGROUP has no __module__ attribute

    (where G_SGROUP is my new type), which did not happen in 3.6, and which I do not think can be suppressed by function call arguments.

    @wdi2 wdi2 mannequin added 3.8 only security fixes topic-C-API type-crash A hard crash of the interpreter, possibly with a core dump labels Oct 19, 2020
    @stestagg
    Copy link
    Mannequin

    stestagg mannequin commented Dec 10, 2020

    It looks like the segfault was fixed in 88c2cfd

    as part of https://bugs.python.org/issue41832.

    The code in this area of typeobject.c looks a bit different, now, but the backport seems simple?

    Simple testcase:

    #include <stdio.h>
#include <Python.h>
int main() {
    Py_Initialize();
    PyStructSequence_Field fields[2] = {
        {NULL, NULL}
    };
    PyStructSequence_Desc d = {"test", NULL, &fields[0], 0};
    PyStructSequence_NewType(&d);
    Py_Finalize();
}

    Segfault reproducible on 3.8 and 3.9

    @ambv
    Copy link
    Contributor

    ambv commented May 2, 2021

    This missed the train for inclusion in 3.8. There's still time for a backport for 3.9.

    @ambv ambv added 3.9 only security fixes 3.10 only security fixes and removed 3.8 only security fixes labels May 2, 2021
    @encukou
    Copy link
    Member

    encukou commented May 4, 2021

    Changing PyType_FromSpec* to accept NULL has an issue: extensions built and tested with 3.9.5 would not work with the earlier 3.9s.

    I'll send a PR to fix just PyStructSequence_NewType.

    @ambv
    Copy link
    Contributor

    ambv commented May 4, 2021

    New changeset ec18362 by Petr Viktorin in branch '3.9':
    [3.9] bpo-42083: Allow NULL doc in PyStructSequence_NewType (bpo-25896)
    ec18362

    @Fidget-Spinner
    Copy link
    Member

    Steve, thank you for your invaluable investigation. Thanks Petr for a better fix - your issue didn't come to my mind at the time.

    Since all PRs have landed and the fix should arrive in Python 3.9.6, I am closing this issue. Please don't hesitate to reopen this if anyone feels it needs revisiting. Thanks!

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.9 only security fixes 3.10 only security fixes topic-C-API type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @ambv @encukou @Fidget-Spinner