Skip to content

GH-127705: better double free message. #130785

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 5, 2025
Merged

GH-127705: better double free message. #130785

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
41cce07
Add location information when accessing already closed stackref
markshannon Mar 3, 2025
a3166f9
Add #def option to track closed stackrefs to provide precise informat…
markshannon Mar 3, 2025
615cdd2
Turn off stackref debugger by default
markshannon Mar 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion Include/internal/pycore_interp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
};

Expand Down
17 changes: 12 additions & 5 deletions Include/internal/pycore_stackref.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion Objects/object.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
19 changes: 16 additions & 3 deletions Python/pystate.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down
46 changes: 36 additions & 10 deletions Python/stackrefs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,33 +47,51 @@ _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);
}
return entry->obj;
}

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;
}
Expand All @@ -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 };
Expand All @@ -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;
Expand All @@ -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");
}
}
Expand All @@ -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.");
}
Expand Down
Loading