CMultiplyNested closure crash: Python 3.13 regression

Test extracted from Cython tests/run/methodmangling_T5.py:

class CMultiplyNested:
    def f1(self):
        __arg = 1
        class D:
            def g(self, __arg):
                return __arg
        return D().g(_CMultiplyNested__arg=2)

    def f2(self):
        __arg = 1
        class D:
            def g(self, __arg):
                return __arg
        return D().g

if __name__ == "__main__":
    import faulthandler; faulthandler.enable()

    inst = CMultiplyNested()
    name = '_CMultiplyNested__arg'
    try:
        inst.f1()
    except TypeError:
        print("TypeError")

    closure = inst.f2()
    closure(_CMultiplyNested__arg=2)

Output:

$ ./python bug.py
TypeError
Fatal Python error: Segmentation fault

Current thread 0x00007fa713149740 (most recent call first):
  File "/home/vstinner/python/3.13/bug.py", line 27 in <module>
Erreur de segmentation (core dumped)

Linked PRs

Just checking - the code here crashes when run with Python and you don't need to compile with Cython to get the bug?

@encukou

Regression introduced by commit 9769b7a.

cc @encukou

commit 9769b7ae064a0546a98cbcbec2561dbaba20cd23 (HEAD)
Author: Petr Viktorin <encukou@gmail.com>
Date:   Mon Jun 24 20:24:19 2024 +0200

    [3.13] gh-113993: Allow interned strings to be mortal, and fix related issues (GH-120520) (GH-120945)

the code here crashes when run with Python and you don't need to compile with Cython to get the bug?

The reproducer doesn't need Cython at all. Only these few lines of Python code without any import.

The bug can be worked around by reverting codeobject.c changes:

diff --git a/Objects/codeobject.c b/Objects/codeobject.c
index 7b1244a8d5f..55b512b3f1a 100644
--- a/Objects/codeobject.c
+++ b/Objects/codeobject.c
@@ -137,7 +137,6 @@ static PyObject *intern_one_constant(PyObject *op);
 static int
 intern_strings(PyObject *tuple)
 {
-    PyInterpreterState *interp = _PyInterpreterState_GET();
     Py_ssize_t i;
 
     for (i = PyTuple_GET_SIZE(tuple); --i >= 0; ) {
@@ -147,7 +146,7 @@ intern_strings(PyObject *tuple)
                             "non-string found in code slot");
             return -1;
         }
-        _PyUnicode_InternMortal(interp, &_PyTuple_ITEMS(tuple)[i]);
+        PyUnicode_InternInPlace(&_PyTuple_ITEMS(tuple)[i]);
     }
     return 0;
 }
@@ -158,13 +157,12 @@ intern_strings(PyObject *tuple)
 static int
 intern_constants(PyObject *tuple, int *modified)
 {
-    PyInterpreterState *interp = _PyInterpreterState_GET();
     for (Py_ssize_t i = PyTuple_GET_SIZE(tuple); --i >= 0; ) {
         PyObject *v = PyTuple_GET_ITEM(tuple, i);
         if (PyUnicode_CheckExact(v)) {
             if (should_intern_string(v)) {
                 PyObject *w = v;
-                _PyUnicode_InternMortal(interp, &v);
+                PyUnicode_InternInPlace(&v);
                 if (w != v) {
                     PyTuple_SET_ITEM(tuple, i, v);
                     if (modified) {
@@ -460,13 +458,12 @@ init_code(PyCodeObject *co, struct _PyCodeConstructor *con)
         con->stacksize = 1;
     }
 
-    PyInterpreterState *interp = _PyInterpreterState_GET();
     co->co_filename = Py_NewRef(con->filename);
     co->co_name = Py_NewRef(con->name);
     co->co_qualname = Py_NewRef(con->qualname);
-    _PyUnicode_InternMortal(interp, &co->co_filename);
-    _PyUnicode_InternMortal(interp, &co->co_name);
-    _PyUnicode_InternMortal(interp, &co->co_qualname);
+    PyUnicode_InternInPlace(&co->co_filename);
+    PyUnicode_InternInPlace(&co->co_name);
+    PyUnicode_InternInPlace(&co->co_qualname);
     co->co_flags = con->flags;
 
     co->co_firstlineno = con->firstlineno;
@@ -492,6 +489,7 @@ init_code(PyCodeObject *co, struct _PyCodeConstructor *con)
     co->co_framesize = nlocalsplus + con->stacksize + FRAME_SPECIALS_SIZE;
     co->co_ncellvars = ncellvars;
     co->co_nfreevars = nfreevars;
+    PyInterpreterState *interp = _PyInterpreterState_GET();
 #ifdef Py_GIL_DISABLED
     PyMutex_Lock(&interp->func_state.mutex);
 #endif

pythongh-121863: Immortalize names in code objects to avoid crash

Thanks for the bisect!
Immortalizing in intern_strings (a.k.a. the first part of the diff) fixes this for me. That is used for names and localsplusnames; it should be OK to immortalize those (and accept that the memory for these strings will not be reclaimed if the code object is GCd). See discussion in #113993.
PR: #121903

intern_strings(PyObject *tuple) looks suspect to me, not because of the interning, but because it modifies a tuple.
That is risky as other code might (rightly) assume that tuples are immutable.
Have you tried having it return a new tuple?

I'll investigate that. I'd still like to merge the quick partial revert for 3.13.0b4 today.

gh-121863: Immortalize names in code objects to avoid crash (GH-121903)

pythongh-121863: Immortalize names in code objects to avoid crash (py…

[3.13] gh-121863: Immortalize names in code objects to avoid crash (G…

intern_strings can indeed modify existing tuples. It's not a 3.13 regression, so I filed a new issue: #121954

The regression is fixed, to the best of my knowledge.

[3.12] pythongh-121863: Immortalize names in code objects to avoid cr…

Navigation Menu

Repository navigation

CMultiplyNested closure crash: Python 3.13 regression #121863

Linked PRs

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

CMultiplyNested closure crash: Python 3.13 regression #121863

Description

Linked PRs

Activity

da-woods commented on Jul 16, 2024

vstinner commented on Jul 16, 2024

vstinner commented on Jul 16, 2024

vstinner commented on Jul 16, 2024

encukou commented on Jul 17, 2024

markshannon commented on Jul 17, 2024

encukou commented on Jul 17, 2024

encukou commented on Jul 18, 2024

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions