diff --git a/Include/object.h b/Include/object.h index 5bcb3937b8c896..9058558e3cd4d9 100644 --- a/Include/object.h +++ b/Include/object.h @@ -660,17 +660,15 @@ static inline void Py_DECREF(PyObject *op) { #elif defined(Py_REF_DEBUG) static inline void Py_DECREF(const char *filename, int lineno, PyObject *op) { + if (op->ob_refcnt <= 0) { + _Py_NegativeRefcount(filename, lineno, op); + } if (_Py_IsImmortal(op)) { return; } _Py_DECREF_STAT_INC(); _Py_DECREF_DecRefTotal(); - if (--op->ob_refcnt != 0) { - if (op->ob_refcnt < 0) { - _Py_NegativeRefcount(filename, lineno, op); - } - } - else { + if (--op->ob_refcnt == 0) { _Py_Dealloc(op); } } diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index c9cc76a4c80ae8..57edacbae35634 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -301,24 +301,40 @@ def test_getitem_with_error(self): def test_buildvalue_N(self): _testcapi.test_buildvalue_N() - @unittest.skipUnless(hasattr(_testcapi, 'negative_refcount'), - 'need _testcapi.negative_refcount') - def test_negative_refcount(self): + def check_negative_refcount(self, code): # bpo-35059: Check that Py_DECREF() reports the correct filename # when calling _Py_NegativeRefcount() to abort Python. - code = textwrap.dedent(""" - import _testcapi - from test import support - - with support.SuppressCrashReport(): - _testcapi.negative_refcount() - """) + code = textwrap.dedent(code) rc, out, err = assert_python_failure('-c', code) self.assertRegex(err, br'_testcapimodule\.c:[0-9]+: ' br'_Py_NegativeRefcount: Assertion failed: ' br'object has negative ref count') + @unittest.skipUnless(hasattr(_testcapi, 'negative_refcount'), + 'need _testcapi.negative_refcount()') + def test_negative_refcount(self): + code = """ + import _testcapi + from test import support + + with support.SuppressCrashReport(): + _testcapi.negative_refcount() + """ + self.check_negative_refcount(code) + + @unittest.skipUnless(hasattr(_testcapi, 'decref_freed_object'), + 'need _testcapi.decref_freed_object()') + def test_decref_freed_object(self): + code = """ + import _testcapi + from test import support + + with support.SuppressCrashReport(): + _testcapi.decref_freed_object() + """ + self.check_negative_refcount(code) + def test_trashcan_subclass(self): # bpo-35983: Check that the trashcan mechanism for "list" is NOT # activated when its tp_dealloc is being called by a subclass diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-09-18-15-35-08.gh-issue-109496.Kleoz3.rst b/Misc/NEWS.d/next/Core and Builtins/2023-09-18-15-35-08.gh-issue-109496.Kleoz3.rst new file mode 100644 index 00000000000000..51b2144fed7841 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-09-18-15-35-08.gh-issue-109496.Kleoz3.rst @@ -0,0 +1,5 @@ +On a Python built in debug mode, :c:func:`Py_DECREF()` now calls +``_Py_NegativeRefcount()`` if the object is a dangling pointer to +deallocated memory: memory filled with ``0xDD`` "dead byte" by the debug +hook on memory allocators. The fix is to check the reference count *before* +checking for ``_Py_IsImmortal()``. Patch by Victor Stinner. diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 85d8401435e1b5..f356fc5a6a016e 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2034,6 +2034,26 @@ negative_refcount(PyObject *self, PyObject *Py_UNUSED(args)) Py_RETURN_NONE; } + +static PyObject * +decref_freed_object(PyObject *self, PyObject *Py_UNUSED(args)) +{ + PyObject *obj = PyUnicode_FromString("decref_freed_object"); + if (obj == NULL) { + return NULL; + } + assert(Py_REFCNT(obj) == 1); + + // Deallocate the memory + Py_DECREF(obj); + // obj is a now a dangling pointer + + // gh-109496: If Python is built in debug mode, Py_DECREF() must call + // _Py_NegativeRefcount() and abort Python. + Py_DECREF(obj); + + Py_RETURN_NONE; +} #endif @@ -3299,6 +3319,7 @@ static PyMethodDef TestMethods[] = { {"bad_get", _PyCFunction_CAST(bad_get), METH_FASTCALL}, #ifdef Py_REF_DEBUG {"negative_refcount", negative_refcount, METH_NOARGS}, + {"decref_freed_object", decref_freed_object, METH_NOARGS}, #endif {"meth_varargs", meth_varargs, METH_VARARGS}, {"meth_varargs_keywords", _PyCFunction_CAST(meth_varargs_keywords), METH_VARARGS|METH_KEYWORDS},