Skip to content

Commit 3cb1ab0

Browse files
efimov-mikhailmpage
efimov-mikhail
and
mpage
authored
pythongh-131527: Stackref debug borrow checker (python#140599)
Add borrow checking to the stackref debug mode --------- Co-authored-by: mpage <[email protected]>
1 parent baa9f33 commit 3cb1ab0

File tree

3 files changed

+110
-14
lines changed

3 files changed

+110
-14
lines changed

‎Include/internal/pycore_stackref.h‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ PyAPI_FUNC(PyObject *) _Py_stackref_get_object(_PyStackRef ref);
6363
PyAPI_FUNC(PyObject *) _Py_stackref_close(_PyStackRef ref, const char *filename, int linenumber);
6464
PyAPI_FUNC(_PyStackRef) _Py_stackref_create(PyObject *obj, uint16_t flags, const char *filename, int linenumber);
6565
PyAPI_FUNC(void) _Py_stackref_record_borrow(_PyStackRef ref, const char *filename, int linenumber);
66+
PyAPI_FUNC(_PyStackRef) _Py_stackref_get_borrowed_from(_PyStackRef ref, const char *filename, int linenumber);
67+
PyAPI_FUNC(void) _Py_stackref_set_borrowed_from(_PyStackRef ref, _PyStackRef borrowed_from, const char *filename, int linenumber);
6668
extern void _Py_stackref_associate(PyInterpreterState *interp, PyObject *obj, _PyStackRef ref);
6769

6870
static const _PyStackRef PyStackRef_NULL = { .index = 0 };
@@ -248,7 +250,12 @@ _PyStackRef_DUP(_PyStackRef ref, const char *filename, int linenumber)
248250
} else {
249251
flags = Py_TAG_REFCNT;
250252
}
251-
return _Py_stackref_create(obj, flags, filename, linenumber);
253+
_PyStackRef new_ref = _Py_stackref_create(obj, flags, filename, linenumber);
254+
if (flags == Py_TAG_REFCNT && !_Py_IsImmortal(obj)) {
255+
_PyStackRef borrowed_from = _Py_stackref_get_borrowed_from(ref, filename, linenumber);
256+
_Py_stackref_set_borrowed_from(new_ref, borrowed_from, filename, linenumber);
257+
}
258+
return new_ref;
252259
}
253260
#define PyStackRef_DUP(REF) _PyStackRef_DUP(REF, __FILE__, __LINE__)
254261

@@ -259,6 +266,7 @@ _PyStackRef_CLOSE_SPECIALIZED(_PyStackRef ref, destructor destruct, const char *
259266
assert(!PyStackRef_IsNull(ref));
260267
assert(!PyStackRef_IsTaggedInt(ref));
261268
PyObject *obj = _Py_stackref_close(ref, filename, linenumber);
269+
assert(Py_REFCNT(obj) > 0);
262270
if (PyStackRef_RefcountOnObject(ref)) {
263271
_Py_DECREF_SPECIALIZED(obj, destruct);
264272
}
@@ -274,7 +282,11 @@ _PyStackRef_Borrow(_PyStackRef ref, const char *filename, int linenumber)
274282
return ref;
275283
}
276284
PyObject *obj = _Py_stackref_get_object(ref);
277-
return _Py_stackref_create(obj, Py_TAG_REFCNT, filename, linenumber);
285+
_PyStackRef new_ref = _Py_stackref_create(obj, Py_TAG_REFCNT, filename, linenumber);
286+
if (!_Py_IsImmortal(obj)) {
287+
_Py_stackref_set_borrowed_from(new_ref, ref, filename, linenumber);
288+
}
289+
return new_ref;
278290
}
279291
#define PyStackRef_Borrow(REF) _PyStackRef_Borrow((REF), __FILE__, __LINE__)
280292

@@ -310,13 +322,22 @@ PyStackRef_IsHeapSafe(_PyStackRef ref)
310322
static inline _PyStackRef
311323
_PyStackRef_MakeHeapSafe(_PyStackRef ref, const char *filename, int linenumber)
312324
{
313-
if (PyStackRef_IsHeapSafe(ref)) {
325+
// Special references that can't be closed.
326+
if (ref.index < INITIAL_STACKREF_INDEX) {
314327
return ref;
315328
}
316329

330+
bool heap_safe = PyStackRef_IsHeapSafe(ref);
317331
PyObject *obj = _Py_stackref_close(ref, filename, linenumber);
318-
Py_INCREF(obj);
319-
return _Py_stackref_create(obj, 0, filename, linenumber);
332+
uint16_t flags = 0;
333+
if (heap_safe) {
334+
// Close old ref and create a new one with the same flags.
335+
// This is necessary for correct borrow checking.
336+
flags = ref.index & Py_TAG_BITS;
337+
} else {
338+
Py_INCREF(obj);
339+
}
340+
return _Py_stackref_create(obj, flags, filename, linenumber);
320341
}
321342
#define PyStackRef_MakeHeapSafe(REF) _PyStackRef_MakeHeapSafe(REF, __FILE__, __LINE__)
322343

‎Misc/NEWS.d/next/Core_and_Builtins/2025-10-25-21-31-43.gh-issue-131527.V-JVNP.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Dynamic borrow checking for stackrefs is added to ``Py_STACKREF_DEBUG``
2+
mode. Patch by Mikhail Efimov.

‎Python/stackrefs.c‎

Lines changed: 82 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ typedef struct _table_entry {
1919
int linenumber;
2020
const char *filename_borrow;
2121
int linenumber_borrow;
22+
int borrows;
23+
_PyStackRef borrowed_from;
2224
} TableEntry;
2325

2426
TableEntry *
@@ -34,6 +36,8 @@ make_table_entry(PyObject *obj, const char *filename, int linenumber)
3436
result->linenumber = linenumber;
3537
result->filename_borrow = NULL;
3638
result->linenumber_borrow = 0;
39+
result->borrows = 0;
40+
result->borrowed_from = PyStackRef_NULL;
3741
return result;
3842
}
3943

@@ -47,11 +51,13 @@ _Py_stackref_get_object(_PyStackRef ref)
4751
PyInterpreterState *interp = PyInterpreterState_Get();
4852
assert(interp != NULL);
4953
if (ref.index >= interp->next_stackref) {
50-
_Py_FatalErrorFormat(__func__, "Garbled stack ref with ID %" PRIu64 "\n", ref.index);
54+
_Py_FatalErrorFormat(__func__,
55+
"Garbled stack ref with ID %" PRIu64 "\n", ref.index);
5156
}
5257
TableEntry *entry = _Py_hashtable_get(interp->open_stackrefs_table, (void *)ref.index);
5358
if (entry == NULL) {
54-
_Py_FatalErrorFormat(__func__, "Accessing closed stack ref with ID %" PRIu64 "\n", ref.index);
59+
_Py_FatalErrorFormat(__func__,
60+
"Accessing closed stack ref with ID %" PRIu64 "\n", ref.index);
5561
}
5662
return entry->obj;
5763
}
@@ -68,13 +74,16 @@ _Py_stackref_close(_PyStackRef ref, const char *filename, int linenumber)
6874
assert(!PyStackRef_IsError(ref));
6975
PyInterpreterState *interp = PyInterpreterState_Get();
7076
if (ref.index >= interp->next_stackref) {
71-
_Py_FatalErrorFormat(__func__, "Invalid StackRef with ID %" PRIu64 " at %s:%d\n", (void *)ref.index, filename, linenumber);
72-
77+
_Py_FatalErrorFormat(__func__,
78+
"Invalid StackRef with ID %" PRIu64 " at %s:%d\n",
79+
ref.index, filename, linenumber);
7380
}
7481
PyObject *obj;
7582
if (ref.index < INITIAL_STACKREF_INDEX) {
7683
if (ref.index == 0) {
77-
_Py_FatalErrorFormat(__func__, "Passing NULL to PyStackRef_CLOSE at %s:%d\n", filename, linenumber);
84+
_Py_FatalErrorFormat(__func__,
85+
"Passing NULL to _Py_stackref_close at %s:%d\n",
86+
filename, linenumber);
7887
}
7988
// Pre-allocated reference to None, False or True -- Do not clear
8089
TableEntry *entry = _Py_hashtable_get(interp->open_stackrefs_table, (void *)ref.index);
@@ -88,10 +97,27 @@ _Py_stackref_close(_PyStackRef ref, const char *filename, int linenumber)
8897
if (entry != NULL) {
8998
_Py_FatalErrorFormat(__func__,
9099
"Double close of ref ID %" PRIu64 " at %s:%d. Referred to instance of %s at %p. Closed at %s:%d\n",
91-
(void *)ref.index, filename, linenumber, entry->classname, entry->obj, entry->filename, entry->linenumber);
100+
ref.index, filename, linenumber, entry->classname, entry->obj, entry->filename, entry->linenumber);
92101
}
93102
#endif
94-
_Py_FatalErrorFormat(__func__, "Invalid StackRef with ID %" PRIu64 "\n", (void *)ref.index);
103+
_Py_FatalErrorFormat(__func__,
104+
"Invalid StackRef with ID %" PRIu64 " at %s:%d\n",
105+
ref.index, filename, linenumber);
106+
}
107+
if (!PyStackRef_IsNull(entry->borrowed_from)) {
108+
_PyStackRef borrowed_from = entry->borrowed_from;
109+
TableEntry *entry_borrowed = _Py_hashtable_get(interp->open_stackrefs_table, (void *)borrowed_from.index);
110+
if (entry_borrowed == NULL) {
111+
_Py_FatalErrorFormat(__func__,
112+
"Invalid borrowed StackRef with ID %" PRIu64 " at %s:%d\n",
113+
borrowed_from.index, filename, linenumber);
114+
}
115+
entry_borrowed->borrows--;
116+
}
117+
if (entry->borrows > 0) {
118+
_Py_FatalErrorFormat(__func__,
119+
"StackRef with ID %" PRIu64 " closed with %d borrowed refs at %s:%d. Opened at %s:%d\n",
120+
ref.index, entry->borrows, filename, linenumber, entry->filename, entry->linenumber);
95121
}
96122
obj = entry->obj;
97123
free(entry);
@@ -143,15 +169,62 @@ _Py_stackref_record_borrow(_PyStackRef ref, const char *filename, int linenumber
143169
if (entry != NULL) {
144170
_Py_FatalErrorFormat(__func__,
145171
"Borrow of closed ref ID %" PRIu64 " at %s:%d. Referred to instance of %s at %p. Closed at %s:%d\n",
146-
(void *)ref.index, filename, linenumber, entry->classname, entry->obj, entry->filename, entry->linenumber);
172+
ref.index, filename, linenumber, entry->classname, entry->obj, entry->filename, entry->linenumber);
147173
}
148174
#endif
149-
_Py_FatalErrorFormat(__func__, "Invalid StackRef with ID %" PRIu64 " at %s:%d\n", (void *)ref.index, filename, linenumber);
175+
_Py_FatalErrorFormat(__func__,
176+
"Invalid StackRef with ID %" PRIu64 " at %s:%d\n",
177+
ref.index, filename, linenumber);
150178
}
151179
entry->filename_borrow = filename;
152180
entry->linenumber_borrow = linenumber;
153181
}
154182

183+
_PyStackRef
184+
_Py_stackref_get_borrowed_from(_PyStackRef ref, const char *filename, int linenumber)
185+
{
186+
assert(!PyStackRef_IsError(ref));
187+
PyInterpreterState *interp = PyInterpreterState_Get();
188+
189+
TableEntry *entry = _Py_hashtable_get(interp->open_stackrefs_table, (void *)ref.index);
190+
if (entry == NULL) {
191+
_Py_FatalErrorFormat(__func__,
192+
"Invalid StackRef with ID %" PRIu64 " at %s:%d\n",
193+
ref.index, filename, linenumber);
194+
}
195+
196+
return entry->borrowed_from;
197+
}
198+
199+
// This function should be used no more than once per ref.
200+
void
201+
_Py_stackref_set_borrowed_from(_PyStackRef ref, _PyStackRef borrowed_from, const char *filename, int linenumber)
202+
{
203+
assert(!PyStackRef_IsError(ref));
204+
PyInterpreterState *interp = PyInterpreterState_Get();
205+
206+
TableEntry *entry = _Py_hashtable_get(interp->open_stackrefs_table, (void *)ref.index);
207+
if (entry == NULL) {
208+
_Py_FatalErrorFormat(__func__,
209+
"Invalid StackRef (ref) with ID %" PRIu64 " at %s:%d\n",
210+
ref.index, filename, linenumber);
211+
}
212+
213+
assert(PyStackRef_IsNull(entry->borrowed_from));
214+
if (PyStackRef_IsNull(borrowed_from)) {
215+
return;
216+
}
217+
218+
TableEntry *entry_borrowed = _Py_hashtable_get(interp->open_stackrefs_table, (void *)borrowed_from.index);
219+
if (entry_borrowed == NULL) {
220+
_Py_FatalErrorFormat(__func__,
221+
"Invalid StackRef (borrowed_from) with ID %" PRIu64 " at %s:%d\n",
222+
borrowed_from.index, filename, linenumber);
223+
}
224+
225+
entry->borrowed_from = borrowed_from;
226+
entry_borrowed->borrows++;
227+
}
155228

156229
void
157230
_Py_stackref_associate(PyInterpreterState *interp, PyObject *obj, _PyStackRef ref)

0 commit comments

Comments
 (0)