https://github.com/python/cpython/commit/78d50e91ff31bc7fd0ac877cf59ee083e94d0915
commit: 78d50e91ff31bc7fd0ac877cf59ee083e94d0915
branch: main
author: Mark Shannon <m...@hotpy.org>
committer: markshannon <m...@hotpy.org>
date: 2025-03-05T14:00:42Z
summary:
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.
files:
M Include/internal/pycore_interp.h
M Include/internal/pycore_stackref.h
M Objects/object.c
M Python/pystate.c
M Python/stackrefs.c
diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h
index 7fdfc7903477de..c247c7270d1caa 100644
--- a/Include/internal/pycore_interp.h
+++ b/Include/internal/pycore_interp.h
@@ -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
};
diff --git a/Include/internal/pycore_stackref.h
b/Include/internal/pycore_stackref.h
index 92b10d21100a25..699f31371a2c7f 100644
--- a/Include/internal/pycore_stackref.h
+++ b/Include/internal/pycore_stackref.h
@@ -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)
diff --git a/Objects/object.c b/Objects/object.c
index b3309bac7afdee..051b668dfcbb64 100644
--- a/Objects/object.c
+++ b/Objects/object.c
@@ -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);
diff --git a/Python/pystate.c b/Python/pystate.c
index 72e2627e95b559..4b01942f40e123 100644
--- a/Python/pystate.c
+++ b/Python/pystate.c
@@ -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) {
diff --git a/Python/stackrefs.c b/Python/stackrefs.c
index 9bb46897685570..2e889b19bf38f2 100644
--- a/Python/stackrefs.c
+++ b/Python/stackrefs.c
@@ -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.");
}
_______________________________________________
Python-checkins mailing list -- python-checkins@python.org
To unsubscribe send an email to python-checkins-le...@python.org
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: arch...@mail-archive.com
