Activity

1. alex

   added

   type-crashA hard crash of the interpreter, possibly with a core dumpA hard crash of the interpreter, possibly with a core dump

   on Jan 8, 2025
2. tom-pytel commented on Jan 8, 2025

   

   tom-pytel

   on Jan 8, 2025

   Last edited by tom-pytel

   Contributor

   More actions

   Hi there, I'm not familiar with the code so can't say WHY this is happening but the immediate cause of this seems to be the offset calculation in Python/assemble.c compute_localsplus_info() line 535, the last loop for freevars does not account for a cellvar put immediately above and overwrites that pointer instead of putting at the next location in the tuple:

   https://github.com/python/cpython/blob/main/Python/assemble.c#L535

   When corrected the following error appears instead (no segfault):

   Traceback (most recent call last):
     File "<python-input-1>", line 1, in <module>
       compile(src, '<fuzz>', start, optimize=opt)
       ~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   SystemError: compiler_lookup_arg(name='__classdict__') with reftype=5 failed in in2; freevars of code <generic parameters of Gí>: ('__classdict__',)

   EDIT: Segfault reproducible with just this script:

   class A:
     class B[__classdict__]: pass
3. ZeroIntensity

   added

   3.12only security fixesonly security fixes

   3.13only security fixesonly security fixes

   3.14bugs and security fixesbugs and security fixes

   on Jan 8, 2025
4. ZeroIntensity commented on Jan 8, 2025

   

   ZeroIntensity

   on Jan 8, 2025

   Member

   More actions

   EDIT: Segfault reproducible with just this script:

   Thanks for that! Confirmed on current main back to 3.12.8. This is possibly a security problem, because the interpreter can be crashed with just compile(). I'm speculating, though.

   cc @Eclips4 as an ast expert (you might like this issue!)
5. tom-pytel commented on Jan 8, 2025

   

   tom-pytel

   on Jan 8, 2025

   Last edited by tom-pytel

   Contributor

   More actions

   Tracked the problem down to the fact that in this particular case a cell var and a free var get the same offset in locals which causes the free var pointer for the last __classdict__ to overwrite the cell var pointer for .type_params and leaves a NULL in the tuple where it is meant to be written into. At the point of error in the function compute_localsplus_info() the var dicts are as follows:

   umd->u_varnames = {'__classdict__': 0, '.generic_base': 1}
   umd->u_cellvars = {'.type_params': 0}
   umd->u_freevars = {'__classdict__': 0}  # assuming the value should be a 1?

   Included a snippet of code here below which will detect the condition and raise a SystemError exception (which would be raised if this were handled correctly anyway as this is a very degenerate condition). Included the "fix" here and not a PR because it is a bandaid just to avoid the crash. The proper way to fix this would be to figure out why the __classdict__ freevar gets an index of 0 and fix that (the __classdict__ being added in codegen_load_classdict_freevar() doesn't seem to take into account variables already present in u_metadata.u_cellvars to offset its index?).

   "Fix": Replace Python/assemble.c lines 506-538 with the following:

       // This counter mirrors the fix done in fix_cell_offsets().
       int numdropped = 0, maxcelloffset = -1;
       pos = 0;
       while (PyDict_Next(umd->u_cellvars, &pos, &k, &v)) {
           int has_name = PyDict_Contains(umd->u_varnames, k);
           RETURN_IF_ERROR(has_name);
           if (has_name) {
               // Skip cells that are already covered by locals.
               numdropped += 1;
               continue;
           }
   
           int offset = PyLong_AsInt(v);
           if (offset == -1 && PyErr_Occurred()) {
               return ERROR;
           }
           assert(offset >= 0);
           offset += nlocals - numdropped;
           maxcelloffset = Py_MAX(maxcelloffset, offset);
           assert(offset < nlocalsplus);
           _Py_set_localsplus_info(offset, k, CO_FAST_CELL, names, kinds);
       }
   
       pos = 0;
       while (PyDict_Next(umd->u_freevars, &pos, &k, &v)) {
           int offset = PyLong_AsInt(v);
           if (offset == -1 && PyErr_Occurred()) {
               return ERROR;
           }
           assert(offset >= 0);
           offset += nlocals - numdropped;
           assert(offset < nlocalsplus);
   
           // TODO: remove once gh-128632 is fixed properly or leave to prevent future unforseen segfaults?
           if (offset <= maxcelloffset) {
               PyErr_SetString(PyExc_SystemError,
                               "overlapping cell and free variable offsets detected (see gh-128632)");
               return ERROR;
           }
   
           _Py_set_localsplus_info(offset, k, CO_FAST_FREE, names, kinds);
       }

   P.S. If this niche case is not worth fixing properly let me know and I will send up a PR with this "fix" to at least avoid a crash and a test case.
6. alex commented on Jan 8, 2025

   

   alex

   on Jan 8, 2025

   MemberAuthor

   More actions

   Thanks for minimizing this!
7. iritkatriel commented on Jan 9, 2025

   

   iritkatriel

   on Jan 9, 2025

   Member

   More actions

   Thank you @tom-pytel for the analysis.

   CC @JelleZijlstra .
8. JelleZijlstra

   self-assigned this

   on Jan 9, 2025
9. picnixz

   added

   interpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)

   on Jan 10, 2025
10. tom-pytel commented on Jan 10, 2025

    

    tom-pytel

    on Jan 10, 2025

    Last edited by tom-pytel

    Contributor

    More actions

    The crash happens because __classdict__ is a special variable, how about just disallowing that completely (can't be used anyway):

    >>> class A:
    ...   class B[__classdict__]: pass
    ... 
      File "<python-input-0>", line 2
        class B[__classdict__]: pass
                ^^^^^^^^^^^^^
    SyntaxError: reserved name '__classdict__' can not be used for type parameter

    Very simple in codegen.c, or maybe better parser? Could restrict to only class annotations or other details but idea is this. Expand to list of forbidden names just in case?

    $ git diff main
    diff --git a/Python/codegen.c b/Python/codegen.c
    index 61707ba677..a84e4fc84e 100644
    --- a/Python/codegen.c
    +++ b/Python/codegen.c
    @@ -1160,6 +1160,12 @@ codegen_type_params(compiler *c, asdl_type_param_seq *type_params)
             location loc = LOC(typeparam);
             switch(typeparam->kind) {
             case TypeVar_kind:
    +            if (_PyUnicode_EqualToASCIIString(typeparam->v.TypeVar.name, "__class__") ||
    +                _PyUnicode_EqualToASCIIString(typeparam->v.TypeVar.name, "__classdict__")) {
    +                return _PyCompile_Error(c, loc, "reserved name '%U' "
    +                                        "can not be used for type parameter",
    +                                        typeparam->v.TypeVar.name);
    +            }
                 ADDOP_LOAD_CONST(c, loc, typeparam->v.TypeVar.name);
                 if (typeparam->v.TypeVar.bound) {
                     expr_ty bound = typeparam->v.TypeVar.bound;
    @@ -1192,6 +1198,12 @@ codegen_type_params(compiler *c, asdl_type_param_seq *type_params)
                 RETURN_IF_ERROR(codegen_nameop(c, loc, typeparam->v.TypeVar.name, Store));
                 break;
             case TypeVarTuple_kind:
    +            if (_PyUnicode_EqualToASCIIString(typeparam->v.TypeVarTuple.name, "__class__") ||
    +                _PyUnicode_EqualToASCIIString(typeparam->v.TypeVarTuple.name, "__classdict__")) {
    +                return _PyCompile_Error(c, loc, "reserved name '%U' "
    +                                        "can not be used for type parameter",
    +                                        typeparam->v.TypeVarTuple.name);
    +            }
                 ADDOP_LOAD_CONST(c, loc, typeparam->v.TypeVarTuple.name);
                 ADDOP_I(c, loc, CALL_INTRINSIC_1, INTRINSIC_TYPEVARTUPLE);
                 if (typeparam->v.TypeVarTuple.default_value) {
    @@ -1211,6 +1223,12 @@ codegen_type_params(compiler *c, asdl_type_param_seq *type_params)
                 RETURN_IF_ERROR(codegen_nameop(c, loc, typeparam->v.TypeVarTuple.name, Store));
                 break;
             case ParamSpec_kind:
    +            if (_PyUnicode_EqualToASCIIString(typeparam->v.ParamSpec.name, "__class__") ||
    +                _PyUnicode_EqualToASCIIString(typeparam->v.ParamSpec.name, "__classdict__")) {
    +                return _PyCompile_Error(c, loc, "reserved name '%U' "
    +                                        "can not be used for type parameter",
    +                                        typeparam->v.ParamSpec.name);
    +            }
                 ADDOP_LOAD_CONST(c, loc, typeparam->v.ParamSpec.name);
                 ADDOP_I(c, loc, CALL_INTRINSIC_1, INTRINSIC_PARAMSPEC);
                 if (typeparam->v.ParamSpec.default_value) {
11. iritkatriel commented on Jan 11, 2025

    

    iritkatriel

    on Jan 11, 2025

    Last edited by iritkatriel

    Member

    More actions

    The crash happens because classdict is a special variable, how about just disallowing that completely

    That may well be the solution. It was added in the implementation of PEP 695, but I don't see it mentioned in the PEP itself.

    There's more here from @JelleZijlstra : https://jellezijlstra.github.io/pep695.html

    CC @carljm.
12. JelleZijlstra commented on Jan 11, 2025

    

    JelleZijlstra

    on Jan 11, 2025

    Member

    More actions

    I'm OK with making this a SyntaxError, thanks for the investigation! We should probably do this in symtable.c though (unless Irit has different opinions on where this sort of check should happen?).
13. iritkatriel commented on Jan 11, 2025

    

    iritkatriel

    on Jan 11, 2025

    Member

    More actions

    I'm OK with making this a SyntaxError, thanks for the investigation! We should probably do this in symtable.c though (unless Irit has different opinions on where this sort of check should happen?).

    It's fine in the symtable for now if it's easy to do.

    Ideally syntax errors would be identified during ast construction so they show up in linting/static analysis. But we also want the errors detected when a user-constructed ast is compiled. I think we should add a syntax check pass on the ast that can be applied after ast is constructed before it is returned, and also before a user defined ast is processed. But that's out of scope for this issue, so for now we can put the check where it's easiest to plug it in.
14. 4 remaining items

    Load more

15. tom-pytel commented on Jan 11, 2025

    

    tom-pytel

    on Jan 11, 2025

    Contributor

    More actions

    Yes, an invalid AST shouldn't result in a segfault.

    I didn't mean the segfault would still be allowed to happen, but moot as the check in symtable covers it.

    The validation in ast.c is only performed on user-generated ASTs.

    Your sure? I was getting the call to ast.c:validate_typeparam() when running a script, maybe you thinking py ast module?

    Would you like to create a PR?

    Its up, have a look.
16. iritkatriel commented on Jan 11, 2025

    

    iritkatriel

    on Jan 11, 2025

    Member

    More actions

    The validation in ast.c is only performed on user-generated ASTs.
    Your sure? I was getting the call to ast.c:validate_typeparam() when running a script, maybe you thinking py ast module?

    Ah, maybe it was that it only runs in debug mode?
    (I once tried to add a syntax errors there and it didn't always work)
17. tom-pytel commented on Jan 11, 2025

    

    tom-pytel

    on Jan 11, 2025

    Contributor

    More actions

    Ah, maybe it was that it only runs in debug mode?

    Nope, its called optimized as well, gonna say its a version thing.
18. JelleZijlstra

    added a commit that references this issue on Apr 4, 2025

    gh-128632: fix segfault on nested __classdict__ type param (#128744)

    891c61c
19. tom-pytel

    added 2 commits that reference this issue on Apr 4, 2025

    pythongh-128632: fix segfault on nested __classdict__ type param (pyt…

    ...

    4b391b1

    [3.13] pythongh-128632: fix segfault on nested __classdict__ type par…

    ...

    617d4e6
20. JelleZijlstra

    added 2 commits that reference this issue on Apr 4, 2025

    [3.13] gh-128632: fix segfault on nested __classdict__ type param (GH…

    ...

    900dc2b

    [3.12] gh-128632: fix segfault on nested __classdict__ type param (GH… (

    ...

    3d3e7b0
21. picnixz

    closed this as completedon Apr 4, 2025
22. petrvaganoff

    added 2 commits that reference this issue on Jun 30, 2026

    pythongh-152682: Fix NULL dereference on OOM in symtable_visit_type_p…

    ...

    c284d9c

    pythongh-152682: Fix NULL dereference on OOM in symtable_visit_type_p…

    ...

    ad57b62
23. sobolevn

    added a commit that references this issue on Jun 30, 2026

    gh-152682: Fix NULL dereference on OOM in `symtable_visit_type_param_…

    ...

    10ed03e
24. JelleZijlstra

    added 3 commits that reference this issue on Jul 6, 2026

    [3.14] gh-152682: Fix NULL dereference on OOM in `symtable_visit_type…

    ...

    0903be8

    [3.13] gh-152682: Fix NULL dereference on OOM in `symtable_visit_type…

    ...

    eca0efe

    [3.15] gh-152682: Fix NULL dereference on OOM in `symtable_visit_type…

    ...

    ffb5184