summaryrefslogtreecommitdiffstats
path: root/Objects
diff options
context:
space:
mode:
authorNeil Schemenauer <nas-github@arctrix.com>2024-12-03 18:33:06 (GMT)
committerGitHub <noreply@github.com>2024-12-03 18:33:06 (GMT)
commitfc5a0dc22483a35068888e828c65796d7a792c14 (patch)
treef3c92006e7f12df9515e20346d31abe01c21c800 /Objects
parent276cd66ccbbf85996a57bd1db3dd29b93a6eab64 (diff)
downloadcpython-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.c41
-rw-r--r--Objects/frameobject.c28
-rw-r--r--Objects/typeobject.c57
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;