diff options
author | Neil Schemenauer <nas-github@arctrix.com> | 2024-12-03 18:33:06 (GMT) |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-12-03 18:33:06 (GMT) |
commit | fc5a0dc22483a35068888e828c65796d7a792c14 (patch) | |
tree | f3c92006e7f12df9515e20346d31abe01c21c800 /Objects | |
parent | 276cd66ccbbf85996a57bd1db3dd29b93a6eab64 (diff) | |
download | cpython-fc5a0dc22483a35068888e828c65796d7a792c14.zip cpython-fc5a0dc22483a35068888e828c65796d7a792c14.tar.gz cpython-fc5a0dc22483a35068888e828c65796d7a792c14.tar.bz2 |
gh-127271: Replace use of PyCell_GET/SET (gh-127272)
* Replace uses of `PyCell_GET` and `PyCell_SET`. These macros are not
safe to use in the free-threaded build. Use `PyCell_GetRef()` and
`PyCell_SetTakeRef()` instead.
* Since `PyCell_GetRef()` returns a strong rather than borrowed ref, some
code restructuring was required, e.g. `frame_get_var()` returns a strong
ref now.
* Add critical sections to `PyCell_GET` and `PyCell_SET`.
* Move critical_section.h earlier in the Python.h file.
* Add `PyCell_GET` to the free-threading howto table of APIs that return
borrowed refs.
* Add additional unit tests for free-threading.
Diffstat (limited to 'Objects')
-rw-r--r-- | Objects/cellobject.c | 41 | ||||
-rw-r--r-- | Objects/frameobject.c | 28 | ||||
-rw-r--r-- | Objects/typeobject.c | 57 |
3 files changed, 81 insertions, 45 deletions
diff --git a/Objects/cellobject.c b/Objects/cellobject.c index 4ab9083..ec2eeb1a 100644 --- a/Objects/cellobject.c +++ b/Objects/cellobject.c @@ -83,6 +83,17 @@ cell_dealloc(PyObject *self) } static PyObject * +cell_compare_impl(PyObject *a, PyObject *b, int op) +{ + if (a != NULL && b != NULL) { + return PyObject_RichCompare(a, b, op); + } + else { + Py_RETURN_RICHCOMPARE(b == NULL, a == NULL, op); + } +} + +static PyObject * cell_richcompare(PyObject *a, PyObject *b, int op) { /* neither argument should be NULL, unless something's gone wrong */ @@ -92,27 +103,28 @@ cell_richcompare(PyObject *a, PyObject *b, int op) if (!PyCell_Check(a) || !PyCell_Check(b)) { Py_RETURN_NOTIMPLEMENTED; } + PyObject *a_ref = PyCell_GetRef((PyCellObject *)a); + PyObject *b_ref = PyCell_GetRef((PyCellObject *)b); /* compare cells by contents; empty cells come before anything else */ - a = ((PyCellObject *)a)->ob_ref; - b = ((PyCellObject *)b)->ob_ref; - if (a != NULL && b != NULL) - return PyObject_RichCompare(a, b, op); + PyObject *res = cell_compare_impl(a_ref, b_ref, op); - Py_RETURN_RICHCOMPARE(b == NULL, a == NULL, op); + Py_XDECREF(a_ref); + Py_XDECREF(b_ref); + return res; } static PyObject * cell_repr(PyObject *self) { - PyCellObject *op = _PyCell_CAST(self); - if (op->ob_ref == NULL) { - return PyUnicode_FromFormat("<cell at %p: empty>", op); + PyObject *ref = PyCell_GetRef((PyCellObject *)self); + if (ref == NULL) { + return PyUnicode_FromFormat("<cell at %p: empty>", self); } - - return PyUnicode_FromFormat("<cell at %p: %.80s object at %p>", - op, Py_TYPE(op->ob_ref)->tp_name, - op->ob_ref); + PyObject *res = PyUnicode_FromFormat("<cell at %p: %.80s object at %p>", + self, Py_TYPE(ref)->tp_name, ref); + Py_DECREF(ref); + return res; } static int @@ -135,11 +147,12 @@ static PyObject * cell_get_contents(PyObject *self, void *closure) { PyCellObject *op = _PyCell_CAST(self); - if (op->ob_ref == NULL) { + PyObject *res = PyCell_GetRef(op); + if (res == NULL) { PyErr_SetString(PyExc_ValueError, "Cell is empty"); return NULL; } - return Py_NewRef(op->ob_ref); + return res; } static int diff --git a/Objects/frameobject.c b/Objects/frameobject.c index c743c25..03ed2b9 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -8,6 +8,7 @@ #include "pycore_moduleobject.h" // _PyModule_GetDict() #include "pycore_modsupport.h" // _PyArg_CheckPositional() #include "pycore_object.h" // _PyObject_GC_UNTRACK() +#include "pycore_cell.h" // PyCell_GetRef() PyCell_SetTakeRef() #include "pycore_opcode_metadata.h" // _PyOpcode_Deopt, _PyOpcode_Caches @@ -187,11 +188,8 @@ framelocalsproxy_setitem(PyObject *self, PyObject *key, PyObject *value) } } if (cell != NULL) { - PyObject *oldvalue_o = PyCell_GET(cell); - if (value != oldvalue_o) { - PyCell_SET(cell, Py_XNewRef(value)); - Py_XDECREF(oldvalue_o); - } + Py_XINCREF(value); + PyCell_SetTakeRef((PyCellObject *)cell, value); } else if (value != PyStackRef_AsPyObjectBorrow(oldvalue)) { PyStackRef_XCLOSE(fast[i]); fast[i] = PyStackRef_FromPyObjectNew(value); @@ -1987,19 +1985,25 @@ frame_get_var(_PyInterpreterFrame *frame, PyCodeObject *co, int i, if (kind & CO_FAST_FREE) { // The cell was set by COPY_FREE_VARS. assert(value != NULL && PyCell_Check(value)); - value = PyCell_GET(value); + value = PyCell_GetRef((PyCellObject *)value); } else if (kind & CO_FAST_CELL) { if (value != NULL) { if (PyCell_Check(value)) { assert(!_PyFrame_IsIncomplete(frame)); - value = PyCell_GET(value); + value = PyCell_GetRef((PyCellObject *)value); + } + else { + // (likely) Otherwise it is an arg (kind & CO_FAST_LOCAL), + // with the initial value set when the frame was created... + // (unlikely) ...or it was set via the f_locals proxy. + Py_INCREF(value); } - // (likely) Otherwise it is an arg (kind & CO_FAST_LOCAL), - // with the initial value set when the frame was created... - // (unlikely) ...or it was set via the f_locals proxy. } } + else { + Py_XINCREF(value); + } } *pvalue = value; return 1; @@ -2076,14 +2080,14 @@ PyFrame_GetVar(PyFrameObject *frame_obj, PyObject *name) continue; } - PyObject *value; // borrowed reference + PyObject *value; if (!frame_get_var(frame, co, i, &value)) { break; } if (value == NULL) { break; } - return Py_NewRef(value); + return value; } PyErr_Format(PyExc_NameError, "variable %R does not exist", name); diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 2611404..bf9049b 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -19,6 +19,7 @@ #include "pycore_typeobject.h" // struct type_cache #include "pycore_unionobject.h" // _Py_union_type_or #include "pycore_weakref.h" // _PyWeakref_GET_REF() +#include "pycore_cell.h" // PyCell_GetRef() #include "opcode.h" // MAKE_CELL #include <stddef.h> // ptrdiff_t @@ -11676,23 +11677,28 @@ super_init_without_args(_PyInterpreterFrame *cframe, PyTypeObject **type_p, assert(_PyFrame_GetCode(cframe)->co_nlocalsplus > 0); PyObject *firstarg = PyStackRef_AsPyObjectBorrow(_PyFrame_GetLocalsArray(cframe)[0]); + if (firstarg == NULL) { + PyErr_SetString(PyExc_RuntimeError, "super(): arg[0] deleted"); + return -1; + } // The first argument might be a cell. - if (firstarg != NULL && (_PyLocals_GetKind(co->co_localspluskinds, 0) & CO_FAST_CELL)) { - // "firstarg" is a cell here unless (very unlikely) super() - // was called from the C-API before the first MAKE_CELL op. - if (_PyInterpreterFrame_LASTI(cframe) >= 0) { - // MAKE_CELL and COPY_FREE_VARS have no quickened forms, so no need - // to use _PyOpcode_Deopt here: - assert(_PyCode_CODE(co)[0].op.code == MAKE_CELL || - _PyCode_CODE(co)[0].op.code == COPY_FREE_VARS); - assert(PyCell_Check(firstarg)); - firstarg = PyCell_GET(firstarg); + // "firstarg" is a cell here unless (very unlikely) super() + // was called from the C-API before the first MAKE_CELL op. + if ((_PyLocals_GetKind(co->co_localspluskinds, 0) & CO_FAST_CELL) && + (_PyInterpreterFrame_LASTI(cframe) >= 0)) { + // MAKE_CELL and COPY_FREE_VARS have no quickened forms, so no need + // to use _PyOpcode_Deopt here: + assert(_PyCode_CODE(co)[0].op.code == MAKE_CELL || + _PyCode_CODE(co)[0].op.code == COPY_FREE_VARS); + assert(PyCell_Check(firstarg)); + firstarg = PyCell_GetRef((PyCellObject *)firstarg); + if (firstarg == NULL) { + PyErr_SetString(PyExc_RuntimeError, "super(): arg[0] deleted"); + return -1; } } - if (firstarg == NULL) { - PyErr_SetString(PyExc_RuntimeError, - "super(): arg[0] deleted"); - return -1; + else { + Py_INCREF(firstarg); } // Look for __class__ in the free vars. @@ -11707,18 +11713,22 @@ super_init_without_args(_PyInterpreterFrame *cframe, PyTypeObject **type_p, if (cell == NULL || !PyCell_Check(cell)) { PyErr_SetString(PyExc_RuntimeError, "super(): bad __class__ cell"); + Py_DECREF(firstarg); return -1; } - type = (PyTypeObject *) PyCell_GET(cell); + type = (PyTypeObject *) PyCell_GetRef((PyCellObject *)cell); if (type == NULL) { PyErr_SetString(PyExc_RuntimeError, "super(): empty __class__ cell"); + Py_DECREF(firstarg); return -1; } if (!PyType_Check(type)) { PyErr_Format(PyExc_RuntimeError, "super(): __class__ is not a type (%s)", Py_TYPE(type)->tp_name); + Py_DECREF(type); + Py_DECREF(firstarg); return -1; } break; @@ -11727,6 +11737,7 @@ super_init_without_args(_PyInterpreterFrame *cframe, PyTypeObject **type_p, if (type == NULL) { PyErr_SetString(PyExc_RuntimeError, "super(): __class__ cell not found"); + Py_DECREF(firstarg); return -1; } @@ -11773,16 +11784,24 @@ super_init_impl(PyObject *self, PyTypeObject *type, PyObject *obj) { return -1; } } + else { + Py_INCREF(type); + Py_XINCREF(obj); + } - if (obj == Py_None) + if (obj == Py_None) { + Py_DECREF(obj); obj = NULL; + } if (obj != NULL) { obj_type = supercheck(type, obj); - if (obj_type == NULL) + if (obj_type == NULL) { + Py_DECREF(type); + Py_DECREF(obj); return -1; - Py_INCREF(obj); + } } - Py_XSETREF(su->type, (PyTypeObject*)Py_NewRef(type)); + Py_XSETREF(su->type, (PyTypeObject*)type); Py_XSETREF(su->obj, obj); Py_XSETREF(su->obj_type, obj_type); return 0; |