From 79a31480992c3fa5890fc7a6c5d9af6d337d5844 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 21 Sep 2021 23:04:34 +0200 Subject: [PATCH] bpo-45061: Detect refcount bug on empty tuple singleton (GH-28503) Detect refcount bugs in C extensions when the empty tuple singleton is destroyed by mistake. Add the _Py_FatalRefcountErrorFunc() function. --- Include/internal/pycore_pyerrors.h | 6 ++++++ .../2021-09-21-22-27-25.bpo-45061.5IOUf0.rst | 4 ++++ Objects/boolobject.c | 4 ++-- Objects/object.c | 3 +-- Objects/tupleobject.c | 19 ++++++++++++++++++- Python/pylifecycle.c | 10 ++++++++++ 6 files changed, 41 insertions(+), 5 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2021-09-21-22-27-25.bpo-45061.5IOUf0.rst diff --git a/Include/internal/pycore_pyerrors.h b/Include/internal/pycore_pyerrors.h index a5e97fe23fb..0f4d41c7e0b 100644 --- a/Include/internal/pycore_pyerrors.h +++ b/Include/internal/pycore_pyerrors.h @@ -90,6 +90,12 @@ extern PyObject* _Py_Offer_Suggestions(PyObject* exception); PyAPI_FUNC(Py_ssize_t) _Py_UTF8_Edit_Cost(PyObject *str_a, PyObject *str_b, Py_ssize_t max_cost); +PyAPI_FUNC(void) _Py_NO_RETURN _Py_FatalRefcountErrorFunc( + const char *func, + const char *message); + +#define _Py_FatalRefcountError(message) _Py_FatalRefcountErrorFunc(__func__, message) + #ifdef __cplusplus } #endif diff --git a/Misc/NEWS.d/next/Core and Builtins/2021-09-21-22-27-25.bpo-45061.5IOUf0.rst b/Misc/NEWS.d/next/Core and Builtins/2021-09-21-22-27-25.bpo-45061.5IOUf0.rst new file mode 100644 index 00000000000..08924531dc3 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2021-09-21-22-27-25.bpo-45061.5IOUf0.rst @@ -0,0 +1,4 @@ +Add a deallocator to the bool type to detect refcount bugs in C extensions +which call Py_DECREF(Py_True) or Py_DECREF(Py_False) by mistake. Detect also +refcount bugs when the empty tuple singleton is destroyed by mistake. Patch +by Victor Stinner. diff --git a/Objects/boolobject.c b/Objects/boolobject.c index bc1666f5571..c72243a160b 100644 --- a/Objects/boolobject.c +++ b/Objects/boolobject.c @@ -1,6 +1,7 @@ /* Boolean type, a subtype of int */ #include "Python.h" +#include "pycore_pyerrors.h" // _Py_FatalRefcountError() #include "longintrepr.h" /* We define bool_repr to return "False" or "True" */ @@ -156,8 +157,7 @@ static PyNumberMethods bool_as_number = { static void _Py_NO_RETURN bool_dealloc(PyObject* Py_UNUSED(ignore)) { - Py_FatalError("deallocating True or False likely caused by " - "a refcount bug in a C extension"); + _Py_FatalRefcountError("deallocating True or False"); } /* The type object for bool. Note that this cannot be subclassed! */ diff --git a/Objects/object.c b/Objects/object.c index 026262b5448..aa84815e56e 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1563,8 +1563,7 @@ none_repr(PyObject *op) static void _Py_NO_RETURN none_dealloc(PyObject* Py_UNUSED(ignore)) { - Py_FatalError("deallocating None likely caused by a refcount bug " - "in a C extension"); + _Py_FatalRefcountError("deallocating None"); } static PyObject * diff --git a/Objects/tupleobject.c b/Objects/tupleobject.c index 3e3aea47cc2..e64b93ba63e 100644 --- a/Objects/tupleobject.c +++ b/Objects/tupleobject.c @@ -6,6 +6,7 @@ #include "pycore_gc.h" // _PyObject_GC_IS_TRACKED() #include "pycore_initconfig.h" // _PyStatus_OK() #include "pycore_object.h" // _PyObject_GC_TRACK() +#include "pycore_pyerrors.h" // _Py_FatalRefcountError() /*[clinic input] class tuple "PyTupleObject *" "&PyTuple_Type" @@ -287,6 +288,17 @@ tupledealloc(PyTupleObject *op) } #endif } +#if defined(Py_DEBUG) && PyTuple_MAXSAVESIZE > 0 + else { + assert(len == 0); + struct _Py_tuple_state *state = get_tuple_state(); + // The empty tuple singleton must only be deallocated by + // _PyTuple_Fini(): not before, not after + if (op == state->free_list[0] && state->numfree[0] != 0) { + _Py_FatalRefcountError("deallocating the empty tuple singleton"); + } + } +#endif Py_TYPE(op)->tp_free((PyObject *)op); #if PyTuple_MAXSAVESIZE > 0 @@ -1048,11 +1060,16 @@ _PyTuple_Fini(PyInterpreterState *interp) struct _Py_tuple_state *state = &interp->tuple; // The empty tuple singleton must not be tracked by the GC assert(!_PyObject_GC_IS_TRACKED(state->free_list[0])); + +#ifdef Py_DEBUG + state->numfree[0] = 0; +#endif Py_CLEAR(state->free_list[0]); - _PyTuple_ClearFreeList(interp); #ifdef Py_DEBUG state->numfree[0] = -1; #endif + + _PyTuple_ClearFreeList(interp); #endif } diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index b10a19c0d8d..bfddc1922d2 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -2805,6 +2805,16 @@ _Py_FatalErrorFormat(const char *func, const char *format, ...) } +void _Py_NO_RETURN +_Py_FatalRefcountErrorFunc(const char *func, const char *msg) +{ + _Py_FatalErrorFormat(func, + "%s: bug likely caused by a refcount error " + "in a C extension", + msg); +} + + void _Py_NO_RETURN Py_ExitStatusException(PyStatus status) {