GH-127705: better double free message. (GH-130785)

* Add location information when accessing already closed stackref

* Add #def option to track closed stackrefs to provide precise information for use after free and double frees.
This commit is contained in:
Mark Shannon 2025-03-05 14:00:42 +00:00 committed by GitHub
parent f33d21e24f
commit 78d50e91ff
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 69 additions and 20 deletions

View file

@ -296,7 +296,10 @@ struct _is {
#if !defined(Py_GIL_DISABLED) && defined(Py_STACKREF_DEBUG)
uint64_t next_stackref;
_Py_hashtable_t *stackref_debug_table;
_Py_hashtable_t *open_stackrefs_table;
# ifdef Py_STACKREF_CLOSE_DEBUG
_Py_hashtable_t *closed_stackrefs_table;
# endif
#endif
};

View file

@ -7,6 +7,11 @@ extern "C" {
// Define this to get precise tracking of stackrefs.
// #define Py_STACKREF_DEBUG 1
// Define this to get precise tracking of closed stackrefs.
// This will use unbounded memory, as it can only grow.
// Use this to track double closes in short-lived programs
// #define Py_STACKREF_CLOSE_DEBUG 1
#ifndef Py_BUILD_CORE
# error "this header requires Py_BUILD_CORE define"
#endif
@ -64,7 +69,7 @@ typedef union _PyStackRef {
#define Py_TAG_BITS 0
PyAPI_FUNC(PyObject *) _Py_stackref_get_object(_PyStackRef ref);
PyAPI_FUNC(PyObject *) _Py_stackref_close(_PyStackRef ref);
PyAPI_FUNC(PyObject *) _Py_stackref_close(_PyStackRef ref, const char *filename, int linenumber);
PyAPI_FUNC(_PyStackRef) _Py_stackref_create(PyObject *obj, const char *filename, int linenumber);
PyAPI_FUNC(void) _Py_stackref_record_borrow(_PyStackRef ref, const char *filename, int linenumber);
extern void _Py_stackref_associate(PyInterpreterState *interp, PyObject *obj, _PyStackRef ref);
@ -111,10 +116,11 @@ _PyStackRef_AsPyObjectBorrow(_PyStackRef ref, const char *filename, int linenumb
#define PyStackRef_AsPyObjectBorrow(REF) _PyStackRef_AsPyObjectBorrow((REF), __FILE__, __LINE__)
static inline PyObject *
PyStackRef_AsPyObjectSteal(_PyStackRef ref)
_PyStackRef_AsPyObjectSteal(_PyStackRef ref, const char *filename, int linenumber)
{
return _Py_stackref_close(ref);
return _Py_stackref_close(ref, filename, linenumber);
}
#define PyStackRef_AsPyObjectSteal(REF) _PyStackRef_AsPyObjectSteal((REF), __FILE__, __LINE__)
static inline _PyStackRef
_PyStackRef_FromPyObjectNew(PyObject *obj, const char *filename, int linenumber)
@ -140,11 +146,12 @@ _PyStackRef_FromPyObjectImmortal(PyObject *obj, const char *filename, int linenu
#define PyStackRef_FromPyObjectImmortal(obj) _PyStackRef_FromPyObjectImmortal(_PyObject_CAST(obj), __FILE__, __LINE__)
static inline void
PyStackRef_CLOSE(_PyStackRef ref)
_PyStackRef_CLOSE(_PyStackRef ref, const char *filename, int linenumber)
{
PyObject *obj = _Py_stackref_close(ref);
PyObject *obj = _Py_stackref_close(ref, filename, linenumber);
Py_DECREF(obj);
}
#define PyStackRef_CLOSE(REF) _PyStackRef_CLOSE((REF), __FILE__, __LINE__)
static inline _PyStackRef
_PyStackRef_DUP(_PyStackRef ref, const char *filename, int linenumber)

View file

@ -2984,7 +2984,7 @@ _Py_Dealloc(PyObject *op)
destructor dealloc = type->tp_dealloc;
#ifdef Py_DEBUG
PyThreadState *tstate = _PyThreadState_GET();
#ifndef Py_GIL_DISABLED
#if !defined(Py_GIL_DISABLED) && !defined(Py_STACKREF_DEBUG)
/* This assertion doesn't hold for the free-threading build, as
* PyStackRef_CLOSE_SPECIALIZED is not implemented */
assert(tstate->current_frame == NULL || tstate->current_frame->stackpointer != NULL);

View file

@ -676,13 +676,22 @@ init_interpreter(PyInterpreterState *interp,
.malloc = malloc,
.free = free,
};
interp->stackref_debug_table = _Py_hashtable_new_full(
interp->open_stackrefs_table = _Py_hashtable_new_full(
_Py_hashtable_hash_ptr,
_Py_hashtable_compare_direct,
NULL,
NULL,
&alloc
);
# ifdef Py_STACKREF_CLOSE_DEBUG
interp->closed_stackrefs_table = _Py_hashtable_new_full(
_Py_hashtable_hash_ptr,
_Py_hashtable_compare_direct,
NULL,
NULL,
&alloc
);
# endif
_Py_stackref_associate(interp, Py_None, PyStackRef_None);
_Py_stackref_associate(interp, Py_False, PyStackRef_False);
_Py_stackref_associate(interp, Py_True, PyStackRef_True);
@ -901,9 +910,13 @@ interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate)
Py_CLEAR(interp->builtins);
#if !defined(Py_GIL_DISABLED) && defined(Py_STACKREF_DEBUG)
# ifdef Py_STACKREF_CLOSE_DEBUG
_Py_hashtable_destroy(interp->closed_stackrefs_table);
interp->closed_stackrefs_table = NULL;
# endif
_Py_stackref_report_leaks(interp);
_Py_hashtable_destroy(interp->stackref_debug_table);
interp->stackref_debug_table = NULL;
_Py_hashtable_destroy(interp->open_stackrefs_table);
interp->open_stackrefs_table = NULL;
#endif
if (tstate->interp == interp) {

View file

@ -47,7 +47,7 @@ _Py_stackref_get_object(_PyStackRef ref)
if (ref.index >= interp->next_stackref) {
_Py_FatalErrorFormat(__func__, "Garbled stack ref with ID %" PRIu64 "\n", ref.index);
}
TableEntry *entry = _Py_hashtable_get(interp->stackref_debug_table, (void *)ref.index);
TableEntry *entry = _Py_hashtable_get(interp->open_stackrefs_table, (void *)ref.index);
if (entry == NULL) {
_Py_FatalErrorFormat(__func__, "Accessing closed stack ref with ID %" PRIu64 "\n", ref.index);
}
@ -55,25 +55,43 @@ _Py_stackref_get_object(_PyStackRef ref)
}
PyObject *
_Py_stackref_close(_PyStackRef ref)
_Py_stackref_close(_PyStackRef ref, const char *filename, int linenumber)
{
PyInterpreterState *interp = PyInterpreterState_Get();
if (ref.index >= interp->next_stackref) {
_Py_FatalErrorFormat(__func__, "Garbled stack ref with ID %" PRIu64 "\n", ref.index);
_Py_FatalErrorFormat(__func__, "Invalid StackRef with ID %" PRIu64 " at %s:%d\n", (void *)ref.index, filename, linenumber);
}
PyObject *obj;
if (ref.index <= LAST_PREDEFINED_STACKREF_INDEX) {
// Pre-allocated reference to None, False or True -- Do not clear
TableEntry *entry = _Py_hashtable_get(interp->stackref_debug_table, (void *)ref.index);
TableEntry *entry = _Py_hashtable_get(interp->open_stackrefs_table, (void *)ref.index);
obj = entry->obj;
}
else {
TableEntry *entry = _Py_hashtable_steal(interp->stackref_debug_table, (void *)ref.index);
TableEntry *entry = _Py_hashtable_steal(interp->open_stackrefs_table, (void *)ref.index);
if (entry == NULL) {
#ifdef Py_STACKREF_CLOSE_DEBUG
entry = _Py_hashtable_get(interp->closed_stackrefs_table, (void *)ref.index);
if (entry != NULL) {
_Py_FatalErrorFormat(__func__,
"Double close of ref ID %" PRIu64 " at %s:%d. Referred to instance of %s at %p. Closed at %s:%d\n",
(void *)ref.index, filename, linenumber, entry->classname, entry->obj, entry->filename, entry->linenumber);
}
#endif
_Py_FatalErrorFormat(__func__, "Invalid StackRef with ID %" PRIu64 "\n", (void *)ref.index);
}
obj = entry->obj;
free(entry);
#ifdef Py_STACKREF_CLOSE_DEBUG
TableEntry *close_entry = make_table_entry(obj, filename, linenumber);
if (close_entry == NULL) {
Py_FatalError("No memory left for stackref debug table");
}
if (_Py_hashtable_set(interp->closed_stackrefs_table, (void *)ref.index, close_entry) < 0) {
Py_FatalError("No memory left for stackref debug table");
}
#endif
}
return obj;
}
@ -90,7 +108,7 @@ _Py_stackref_create(PyObject *obj, const char *filename, int linenumber)
if (entry == NULL) {
Py_FatalError("No memory left for stackref debug table");
}
if (_Py_hashtable_set(interp->stackref_debug_table, (void *)new_id, entry) < 0) {
if (_Py_hashtable_set(interp->open_stackrefs_table, (void *)new_id, entry) < 0) {
Py_FatalError("No memory left for stackref debug table");
}
return (_PyStackRef){ .index = new_id };
@ -103,9 +121,17 @@ _Py_stackref_record_borrow(_PyStackRef ref, const char *filename, int linenumber
return;
}
PyInterpreterState *interp = PyInterpreterState_Get();
TableEntry *entry = _Py_hashtable_get(interp->stackref_debug_table, (void *)ref.index);
TableEntry *entry = _Py_hashtable_get(interp->open_stackrefs_table, (void *)ref.index);
if (entry == NULL) {
_Py_FatalErrorFormat(__func__, "Invalid StackRef with ID %" PRIu64 "\n", (void *)ref.index);
#ifdef Py_STACKREF_CLOSE_DEBUG
entry = _Py_hashtable_get(interp->closed_stackrefs_table, (void *)ref.index);
if (entry != NULL) {
_Py_FatalErrorFormat(__func__,
"Borrow of closed ref ID %" PRIu64 " at %s:%d. Referred to instance of %s at %p. Closed at %s:%d\n",
(void *)ref.index, filename, linenumber, entry->classname, entry->obj, entry->filename, entry->linenumber);
}
#endif
_Py_FatalErrorFormat(__func__, "Invalid StackRef with ID %" PRIu64 " at %s:%d\n", (void *)ref.index, filename, linenumber);
}
entry->filename_borrow = filename;
entry->linenumber_borrow = linenumber;
@ -121,7 +147,7 @@ _Py_stackref_associate(PyInterpreterState *interp, PyObject *obj, _PyStackRef re
if (entry == NULL) {
Py_FatalError("No memory left for stackref debug table");
}
if (_Py_hashtable_set(interp->stackref_debug_table, (void *)ref.index, (void *)entry) < 0) {
if (_Py_hashtable_set(interp->open_stackrefs_table, (void *)ref.index, (void *)entry) < 0) {
Py_FatalError("No memory left for stackref debug table");
}
}
@ -147,7 +173,7 @@ void
_Py_stackref_report_leaks(PyInterpreterState *interp)
{
int leak = 0;
_Py_hashtable_foreach(interp->stackref_debug_table, report_leak, &leak);
_Py_hashtable_foreach(interp->open_stackrefs_table, report_leak, &leak);
if (leak) {
Py_FatalError("Stackrefs leaked.");
}