diff --git a/Include/refcount.h b/Include/refcount.h index afbecd48ab31a1b..2bc2e83ef1c340f 100644 --- a/Include/refcount.h +++ b/Include/refcount.h @@ -489,7 +489,7 @@ static inline Py_ALWAYS_INLINE void Py_DECREF(PyObject *op) _Py_TYPEOF(op) _tmp_old_op = (*_tmp_op_ptr); \ if (_tmp_old_op != _Py_NULL) { \ *_tmp_op_ptr = _Py_NULL; \ - Py_DECREF(_tmp_old_op); \ + Py_DECREF(_PyObject_CAST(_tmp_old_op)); \ } \ } while (0) #elif defined(__cplusplus) && (__cplusplus >= 201103L || _MSVC_LANG >= 201103L) @@ -499,14 +499,15 @@ static inline Py_ALWAYS_INLINE void Py_DECREF(PyObject *op) auto _tmp_old_op = (*_tmp_op_ptr); \ if (_tmp_old_op != _Py_NULL) { \ *_tmp_op_ptr = _Py_NULL; \ - Py_DECREF(_tmp_old_op); \ + Py_DECREF(_PyObject_CAST(_tmp_old_op)); \ } \ } while (0) #else #define Py_CLEAR(op) \ do { \ PyObject **_tmp_op_ptr = _Py_CAST(PyObject**, &(op)); \ - PyObject *_tmp_old_op = (*_tmp_op_ptr); \ + PyObject *_tmp_old_op; \ + memcpy(&_tmp_old_op, _tmp_op_ptr, sizeof(PyObject*)); \ if (_tmp_old_op != _Py_NULL) { \ PyObject *_null_ptr = _Py_NULL; \ memcpy(_tmp_op_ptr, &_null_ptr, sizeof(PyObject*)); \ @@ -545,7 +546,7 @@ static inline Py_ALWAYS_INLINE void Py_DECREF(PyObject *op) _Py_TYPEOF(&(dst)) _tmp_dst_ptr = &(dst); \ _Py_TYPEOF(dst) _tmp_old_dst = (*_tmp_dst_ptr); \ *_tmp_dst_ptr = (src); \ - Py_DECREF(_tmp_old_dst); \ + Py_DECREF(_PyObject_CAST(_tmp_old_dst)); \ } while (0) #elif defined(__cplusplus) && (__cplusplus >= 201103L || _MSVC_LANG >= 201103L) #define Py_SETREF(dst, src) \ @@ -553,13 +554,14 @@ static inline Py_ALWAYS_INLINE void Py_DECREF(PyObject *op) auto _tmp_dst_ptr = &(dst); \ auto _tmp_old_dst = (*_tmp_dst_ptr); \ *_tmp_dst_ptr = (src); \ - Py_DECREF(_tmp_old_dst); \ + Py_DECREF(_PyObject_CAST(_tmp_old_dst)); \ } while (0) #else #define Py_SETREF(dst, src) \ do { \ PyObject **_tmp_dst_ptr = _Py_CAST(PyObject**, &(dst)); \ - PyObject *_tmp_old_dst = (*_tmp_dst_ptr); \ + PyObject *_tmp_old_dst; \ + memcpy(&_tmp_old_dst, _tmp_dst_ptr, sizeof(PyObject*)); \ PyObject *_tmp_src = _PyObject_CAST(src); \ memcpy(_tmp_dst_ptr, &_tmp_src, sizeof(PyObject*)); \ Py_DECREF(_tmp_old_dst); \ @@ -575,7 +577,7 @@ static inline Py_ALWAYS_INLINE void Py_DECREF(PyObject *op) _Py_TYPEOF(&(dst)) _tmp_dst_ptr = &(dst); \ _Py_TYPEOF(dst) _tmp_old_dst = (*_tmp_dst_ptr); \ *_tmp_dst_ptr = (src); \ - Py_XDECREF(_tmp_old_dst); \ + Py_XDECREF(_PyObject_CAST(_tmp_old_dst)); \ } while (0) #elif defined(__cplusplus) && (__cplusplus >= 201103L || _MSVC_LANG >= 201103L) #define Py_XSETREF(dst, src) \ @@ -583,13 +585,14 @@ static inline Py_ALWAYS_INLINE void Py_DECREF(PyObject *op) auto _tmp_dst_ptr = &(dst); \ auto _tmp_old_dst = (*_tmp_dst_ptr); \ *_tmp_dst_ptr = (src); \ - Py_XDECREF(_tmp_old_dst); \ + Py_XDECREF(_PyObject_CAST(_tmp_old_dst)); \ } while (0) #else #define Py_XSETREF(dst, src) \ do { \ PyObject **_tmp_dst_ptr = _Py_CAST(PyObject**, &(dst)); \ - PyObject *_tmp_old_dst = (*_tmp_dst_ptr); \ + PyObject *_tmp_old_dst; \ + memcpy(&_tmp_old_dst, _tmp_dst_ptr, sizeof(PyObject*)); \ PyObject *_tmp_src = _PyObject_CAST(src); \ memcpy(_tmp_dst_ptr, &_tmp_src, sizeof(PyObject*)); \ Py_XDECREF(_tmp_old_dst); \ diff --git a/Lib/test/test_cext/extension.c b/Lib/test/test_cext/extension.c index 779ea9facca8c78..15e58cf8bba723f 100644 --- a/Lib/test/test_cext/extension.c +++ b/Lib/test/test_cext/extension.c @@ -140,6 +140,91 @@ test_macros(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args)) } +// sizeof(PyObject) in PySlot below is incompatible with opaque PyObject +#ifndef Py_TARGET_ABI3T +Py_NO_INLINE static void +pyclear_strict_aliasing(PyTypeObject **a, PyTypeObject **b) +{ + Py_CLEAR(*a); + *b = NULL; +} + + +Py_NO_INLINE static void +setref_strict_aliasing(PyTypeObject **a, PyTypeObject **b, + PyTypeObject *first, PyTypeObject *second) +{ + *a = first; + Py_SETREF(*b, second); +} + + +Py_NO_INLINE static void +xsetref_strict_aliasing(PyTypeObject **a, PyTypeObject **b, + PyTypeObject *first, PyTypeObject *second) +{ + *a = first; + Py_XSETREF(*b, second); +} + + +// gh-161185: Test that Py_SETREF(), Py_XSETREF() and Py_CLEAR() macros +// have no strict aliasing issue +static PyObject * +test_setref_strict_aliasing(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args)) +{ + PySlot slots[] = { + PySlot_PTR_STATIC(Py_tp_name, STR(MODULE_NAME) ".setref_type"), + PySlot_PTR_STATIC(Py_tp_basicsize, sizeof(PyObject)), + PySlot_PTR_STATIC(Py_tp_flags, Py_TPFLAGS_DEFAULT), + PySlot_END + }; + PyObject *first, *second; + Py_ssize_t first_refcnt, second_refcnt; + PyTypeObject *slot = NULL; + + first = PyType_FromSlots(slots); + if (first == NULL) { + return NULL; + } + second = PyType_FromSlots(slots); + if (second == NULL) { + Py_DECREF(first); + return NULL; + } + first_refcnt = Py_REFCNT(first); + second_refcnt = Py_REFCNT(first); + + // Both pointer arguments alias. The assignment makes first the old value + // that Py_SETREF() must release before storing second. + setref_strict_aliasing(&slot, &slot, + (PyTypeObject*)Py_NewRef(first), + (PyTypeObject*)Py_NewRef(second)); + assert((PyObject*)slot == second); + assert(Py_REFCNT(first) == first_refcnt); + assert(Py_REFCNT(second) == (second_refcnt + 1)); + + pyclear_strict_aliasing(&slot, &slot); + assert(slot == NULL); + assert(Py_REFCNT(first) == first_refcnt); + assert(Py_REFCNT(second) == second_refcnt); + + xsetref_strict_aliasing(&slot, &slot, + (PyTypeObject*)Py_NewRef(first), + (PyTypeObject*)Py_NewRef(second)); + assert((PyObject*)slot == second); + assert(Py_REFCNT(first) == first_refcnt); + assert(Py_REFCNT(second) == (second_refcnt + 1)); + + Py_DECREF(first); + Py_DECREF(second); + Py_DECREF((PyObject*)slot); + + Py_RETURN_NONE; +} +#endif // !Py_TARGET_ABI3T + + static PyObject * test_datetime(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args)) { @@ -366,6 +451,9 @@ test_virtual_object(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args)) static PyMethodDef module_methods[] = { {"add", test_add, METH_VARARGS, test_add_doc}, {"test_macros", test_macros, METH_NOARGS, _Py_NULL}, +#ifndef Py_TARGET_ABI3T + {"test_setref_strict_aliasing", test_setref_strict_aliasing, METH_NOARGS, _Py_NULL}, +#endif {"test_datetime", test_datetime, METH_NOARGS, _Py_NULL}, {"test_unicode", test_unicode, METH_NOARGS, _Py_NULL}, #ifdef __cplusplus diff --git a/Misc/NEWS.d/next/C_API/2026-11-02-15-17-50.gh-issue-161185.4bpyDC.rst b/Misc/NEWS.d/next/C_API/2026-11-02-15-17-50.gh-issue-161185.4bpyDC.rst new file mode 100644 index 000000000000000..92887e84ade4cd2 --- /dev/null +++ b/Misc/NEWS.d/next/C_API/2026-11-02-15-17-50.gh-issue-161185.4bpyDC.rst @@ -0,0 +1,3 @@ +Fix a strict aliasing issue in the :c:func:`!memcpy` implementation of +:c:macro:`Py_SETREF`, :c:macro:`Py_XSETREF` and :c:macro:`Py_CLEAR` macros. +Patch by Nathan Goldbaum and Victor Stinner.