Repository navigation
Description
Issue reported by @ngoldbaum on pythoncapi-compat: python/pythoncapi-compat#187 (comment). Copy/paste of his message.
I asked an AI model to look this over and it spotted an issue (CPython's fallback also has this problem):
The memcpy() protects the write, but the preceding read still violates strict aliasing:
PyObject **_tmp_dst_ptr = _Py_CAST(PyObject**, &(dst));
PyObject *_tmp_old_dst = (*_tmp_dst_ptr);If dst is a PyTypeObject *, dereferencing _tmp_dst_ptr accesses a PyTypeObject * object through a PyObject * lvalue. GCC can therefore assume that this read does not observe a write made through PyTypeObject **.
Here is a standalone reproducer using the exact Py_XSETREF() implementation from this PR:
#include <Python.h>
#include <cstdio>
#include <cstring>
#ifndef _Py_CAST
# define _Py_CAST(type, expr) ((type)(expr))
#endif
#ifndef _PyObject_CAST
# define _PyObject_CAST(obj) _Py_CAST(PyObject *, (obj))
#endif
#define REPRO_XSETREF(dst, src) \
do { \
PyObject **_tmp_dst_ptr = _Py_CAST(PyObject **, &(dst)); \
PyObject *_tmp_old_dst = (*_tmp_dst_ptr); \
PyObject *_tmp_src = _PyObject_CAST(src); \
memcpy(_tmp_dst_ptr, &_tmp_src, sizeof(PyObject *)); \
Py_XDECREF(_tmp_old_dst); \
} while (0)
__attribute__((noinline))
static void
replace(PyTypeObject **a, PyTypeObject **b,
PyTypeObject *first, PyTypeObject *second)
{
*a = first;
REPRO_XSETREF(*b, second);
}
int
main()
{
Py_Initialize();
PyType_Slot slots[] = {{0, NULL}};
PyType_Spec spec = {
"repro.T", sizeof(PyObject), 0, Py_TPFLAGS_DEFAULT, slots
};
PyObject *first = PyType_FromSpec(&spec);
PyObject *second = PyType_FromSpec(&spec);
if (first == NULL || second == NULL) {
PyErr_Print();
return 1;
}
Py_ssize_t before = Py_REFCNT(first);
PyTypeObject *slot = NULL;
// Both pointer arguments alias. The assignment makes first the old value
// that REPRO_XSETREF() must release before storing second.
replace(&slot, &slot,
reinterpret_cast<PyTypeObject *>(Py_NewRef(first)),
reinterpret_cast<PyTypeObject *>(Py_NewRef(second)));
std::printf("first refcount: %zd; expected: %zd\n",
Py_REFCNT(first), before);
return 0;
}On CPython 3.10 with GCC 15.2.0:
$ g++ -std=c++11 $(python3.10-config --cflags) \
-DPy_LIMITED_API=0x030a0000 -O2 -fstrict-aliasing \
repro.cpp $(python3.10-config --embed --ldflags) -o repro
$ ./repro
first refcount: 3; expected: 2The reference acquired by Py_NewRef(first) was not released. Compiling the same source with -fno-strict-aliasing produces the expected refcount of 2.
With Py_SETREF() instead of Py_XSETREF(), the stale value can be NULL, causing a segmentation fault when it is passed to Py_DECREF().
Activity