[Python-checkins] bpo-45061: Detect refcount bug on empty tuple singleton (GH-28503)

[vstinner]

https://github.com/python/cpython/commit/79a31480992c3fa5890fc7a6c5d9af6d337d5844
commit: 79a31480992c3fa5890fc7a6c5d9af6d337d5844
branch: main
author: Victor Stinner <vstinner at python.org>
committer: vstinner <vstinner at python.org>
date: 2021-09-21T23:04:34+02:00
summary:

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.

files:
A Misc/NEWS.d/next/Core and Builtins/2021-09-21-22-27-25.bpo-45061.5IOUf0.rst
M Include/internal/pycore_pyerrors.h
M Objects/boolobject.c
M Objects/object.c
M Objects/tupleobject.c
M Python/pylifecycle.c

diff --git a/Include/internal/pycore_pyerrors.h b/Include/internal/pycore_pyerrors.h
index a5e97fe23fb05..0f4d41c7e0bab 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 0000000000000..08924531dc3f1
--- /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 bc1666f55717c..c72243a160b88 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 026262b544849..aa84815e56e86 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 3e3aea47cc2ab..e64b93ba63e41 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 b10a19c0d8d5f..bfddc1922d25a 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)
 {