https://github.com/python/cpython/commit/1bfb58cf2ff273534f7b92e9dc32873b77acec95
commit: 1bfb58cf2ff273534f7b92e9dc32873b77acec95
branch: 3.12
author: Sam Gross <colesb...@gmail.com>
committer: ambv <luk...@langa.pl>
date: 2024-09-06T15:46:56+02:00
summary:

[3.12] gh-122527: Fix a crash on deallocation of `PyStructSequence` (GH-122577) 
(#122626)

The `PyStructSequence` destructor would crash if it was deallocated after
its type's dictionary was cleared by the GC, because it couldn't compute
the "real size" of the instance. This could occur with relatively
straightforward code in the free-threaded build or with a reference
cycle involving the type in the default build, due to differing orders
in which `tp_clear()` was called.

Account for the non-sequence fields in `tp_basicsize` and use that,
along with `Py_SIZE()`, to compute the "real" size of a
`PyStructSequence` in the dealloc function. This avoids the accesses to
the type's dictionary during dealloc, which were unsafe.
(cherry picked from commit 4b63cd170e5dd840bffc80922f09f2d69932ff5c)

files:
A 
Misc/NEWS.d/next/Core_and_Builtins/2024-08-01-19-13-58.gh-issue-122527.eztso6.rst
M Lib/test/test_structseq.py
M Objects/structseq.c

diff --git a/Lib/test/test_structseq.py b/Lib/test/test_structseq.py
index c6c0afaf077acc..9db35ab835c3a3 100644
--- a/Lib/test/test_structseq.py
+++ b/Lib/test/test_structseq.py
@@ -1,8 +1,10 @@
 import copy
 import os
 import pickle
+import textwrap
 import time
 import unittest
+from test.support import script_helper
 
 
 class StructSeqTest(unittest.TestCase):
@@ -204,6 +206,17 @@ def test_match_args_with_unnamed_fields(self):
         self.assertEqual(os.stat_result.n_unnamed_fields, 3)
         self.assertEqual(os.stat_result.__match_args__, expected_args)
 
+    def test_reference_cycle(self):
+        # gh-122527: Check that a structseq that's part of a reference cycle
+        # with its own type doesn't crash. Previously, if the type's dictionary
+        # was cleared first, the structseq instance would crash in the
+        # destructor.
+        script_helper.assert_python_ok("-c", textwrap.dedent(r"""
+            import time
+            t = time.gmtime()
+            type(t).refcyle = t
+        """))
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git 
a/Misc/NEWS.d/next/Core_and_Builtins/2024-08-01-19-13-58.gh-issue-122527.eztso6.rst
 
b/Misc/NEWS.d/next/Core_and_Builtins/2024-08-01-19-13-58.gh-issue-122527.eztso6.rst
new file mode 100644
index 00000000000000..f697ed99d0c523
--- /dev/null
+++ 
b/Misc/NEWS.d/next/Core_and_Builtins/2024-08-01-19-13-58.gh-issue-122527.eztso6.rst
@@ -0,0 +1,4 @@
+Fix a crash that occurred when a ``PyStructSequence`` was deallocated after
+its type's dictionary was cleared by the GC.  The type's
+:c:member:`~PyTypeObject.tp_basicsize` now accounts for non-sequence fields
+that aren't included in the :c:macro:`Py_SIZE` of the sequence.
diff --git a/Objects/structseq.c b/Objects/structseq.c
index 8b1895957101a4..246d4c7630cfaa 100644
--- a/Objects/structseq.c
+++ b/Objects/structseq.c
@@ -41,12 +41,20 @@ get_type_attr_as_size(PyTypeObject *tp, PyObject *name)
     get_type_attr_as_size(tp, &_Py_ID(n_sequence_fields))
 #define REAL_SIZE_TP(tp) \
     get_type_attr_as_size(tp, &_Py_ID(n_fields))
-#define REAL_SIZE(op) REAL_SIZE_TP(Py_TYPE(op))
+#define REAL_SIZE(op) get_real_size((PyObject *)op)
 
 #define UNNAMED_FIELDS_TP(tp) \
     get_type_attr_as_size(tp, &_Py_ID(n_unnamed_fields))
 #define UNNAMED_FIELDS(op) UNNAMED_FIELDS_TP(Py_TYPE(op))
 
+static Py_ssize_t
+get_real_size(PyObject *op)
+{
+    // Compute the real size from the visible size (i.e., Py_SIZE()) and the
+    // number of non-sequence fields accounted for in tp_basicsize.
+    Py_ssize_t hidden = Py_TYPE(op)->tp_basicsize - offsetof(PyStructSequence, 
ob_item);
+    return Py_SIZE(op) + hidden / sizeof(PyObject *);
+}
 
 PyObject *
 PyStructSequence_New(PyTypeObject *type)
@@ -107,6 +115,9 @@ structseq_dealloc(PyStructSequence *obj)
     PyObject_GC_UnTrack(obj);
 
     PyTypeObject *tp = Py_TYPE(obj);
+    // gh-122527: We can't use REAL_SIZE_TP() or any macros that access the
+    // type's dictionary here, because the dictionary may have already been
+    // cleared by the garbage collector.
     size = REAL_SIZE(obj);
     for (i = 0; i < size; ++i) {
         Py_XDECREF(obj->ob_item[i]);
@@ -467,10 +478,14 @@ initialize_members(PyStructSequence_Desc *desc,
 
 static void
 initialize_static_fields(PyTypeObject *type, PyStructSequence_Desc *desc,
-                         PyMemberDef *tp_members, unsigned long tp_flags)
+                         PyMemberDef *tp_members, Py_ssize_t n_members,
+                         unsigned long tp_flags)
 {
     type->tp_name = desc->name;
-    type->tp_basicsize = sizeof(PyStructSequence) - sizeof(PyObject *);
+    // Account for hidden members in tp_basicsize because they are not
+    // included in the variable size.
+    Py_ssize_t n_hidden = n_members - desc->n_in_sequence;
+    type->tp_basicsize = sizeof(PyStructSequence) + (n_hidden - 1) * 
sizeof(PyObject *);
     type->tp_itemsize = sizeof(PyObject *);
     type->tp_dealloc = (destructor)structseq_dealloc;
     type->tp_repr = (reprfunc)structseq_repr;
@@ -520,7 +535,7 @@ _PyStructSequence_InitBuiltinWithFlags(PyInterpreterState 
*interp,
         if (members == NULL) {
             goto error;
         }
-        initialize_static_fields(type, desc, members, tp_flags);
+        initialize_static_fields(type, desc, members, n_members, tp_flags);
 
         _Py_SetImmortal(type);
     }
@@ -582,7 +597,7 @@ PyStructSequence_InitType2(PyTypeObject *type, 
PyStructSequence_Desc *desc)
     if (members == NULL) {
         return -1;
     }
-    initialize_static_fields(type, desc, members, 0);
+    initialize_static_fields(type, desc, members, n_members, 0);
     if (initialize_static_type(type, desc, n_members, n_unnamed_members) < 0) {
         PyMem_Free(members);
         return -1;
@@ -658,7 +673,8 @@ _PyStructSequence_NewType(PyStructSequence_Desc *desc, 
unsigned long tp_flags)
     /* The name in this PyType_Spec is statically allocated so it is */
     /* expected that it'll outlive the PyType_Spec */
     spec.name = desc->name;
-    spec.basicsize = sizeof(PyStructSequence) - sizeof(PyObject *);
+    Py_ssize_t hidden = n_members - desc->n_in_sequence;
+    spec.basicsize = (int)(sizeof(PyStructSequence) + (hidden - 1) * 
sizeof(PyObject *));
     spec.itemsize = sizeof(PyObject *);
     spec.flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | tp_flags;
     spec.slots = slots;

_______________________________________________
Python-checkins mailing list -- python-checkins@python.org
To unsubscribe send an email to python-checkins-le...@python.org
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: arch...@mail-archive.com

Reply via email to