8000 [3.8] bpo-36389: Backport debug enhancements from master (GH-16796) · python/cpython@f82ce5b · GitHub
[go: up one dir, main page]
Skip to content

Commit f82ce5b

Browse files
vstinnervstinner
authored
[3.8] bpo-36389: Backport debug enhancements from master (GH-16796)
* bpo-36389: _PyObject_CheckConsistency() available in release mode (GH-16612) bpo-36389, bpo-38376: The _PyObject_CheckConsistency() function is now also available in release mode. For example, it can be used to debug a crash in the visit_decref() function of the GC. Modify the following functions to also work in release mode: * _PyDict_CheckConsistency() * _PyObject_CheckConsistency() * _PyType_CheckConsistency() * _PyUnicode_CheckConsistency() Other changes: * _PyMem_IsPtrFreed(ptr) now also returns 1 if ptr is NULL (equals to 0). * _PyBytesWriter_CheckConsistency() now returns 1 and is only used with assert(). * Reorder _PyObject_Dump() to write safe fields first, and only attempt to render repr() at the end. (cherry picked from commit 6876257) * bpo-36389: Fix _PyBytesWriter in release mode (GH-16624) Fix _PyBytesWriter API when Python is built in release mode with assertions. (cherry picked from commit 60ec6ef) * bpo-38070: Enhance visit_decref() debug trace (GH-16631) subtract_refs() now pass the parent object to visit_decref() which pass it to _PyObject_ASSERT(). So if the "is freed" assertion fails, the parent is used in debug trace, rather than the freed object. The parent object is more likely to contain useful information. Freed objects cannot be inspected are are displayed as "<object at xxx is freed>" with no other detail. (cherry picked from commit 4d5f94b) * Fix also a typo in PYMEM_DEADBYTE macro comment * bpo-36389: Add newline to _PyObject_AssertFailed() (GH-16629) Add a newline between the verbose object dump and the Py_FatalError() logs for readability. (cherry picked from commit 7775349)
1 parent 42308e8 commit f82ce5b

File tree

12 files changed

+200
-173
lines changed

12 files changed

+200
-173
lines changed

‎Include/internal/pycore_pymem.h

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -155,9 +155,25 @@ PyAPI_FUNC(int) _PyMem_SetDefaultAllocator(
155155
PyMemAllocatorDomain domain,
156156
PyMemAllocatorEx *old_alloc);
157157

158+
/* Special bytes broadcast into debug memory blocks at appropriate times.
159+
Strings of these are unlikely to be valid addresses, floats, ints or
160+
7-bit ASCII.
161+
162+
- PYMEM_CLEANBYTE: clean (newly allocated) memory
163+
- PYMEM_DEADBYTE dead (newly freed) memory
164+
- PYMEM_FORBIDDENBYTE: untouchable bytes at each end of a block
165+
166+
Byte patterns 0xCB, 0xDB and 0xFB have been replaced with 0xCD, 0xDD and
167+
0xFD to use the same values than Windows CRT debug malloc() and free().
168+
If modified, _PyMem_IsPtrFreed() should be updated as well. */
169+
#define PYMEM_CLEANBYTE 0xCD
170+
#define PYMEM_DEADBYTE 0xDD
171+
#define PYMEM_FORBIDDENBYTE 0xFD
172+
158173
/* Heuristic checking if a pointer value is newly allocated
159-
(uninitialized) or newly freed. The pointer is not dereferenced, only the
160-
pointer value is checked.
174+
(uninitialized), newly freed or NULL (is equal to zero).
175+
176+
The pointer is not dereferenced, only the pointer value is checked.
161177
162178
The heuristic relies on the debug hooks on Python memory allocators which
163179
fills newly allocated memory with CLEANBYTE (0xCD) and newly freed memory
@@ -167,11 +183,13 @@ static inline int _PyMem_IsPtrFreed(void *ptr)
167183
{
168184
uintptr_t value = (uintptr_t)ptr;
169185
#if SIZEOF_VOID_P == 8
170-
return (value == (uintptr_t)0xCDCDCDCDCDCDCDCD
186+
return (value == 0
187+
|| value == (uintptr_t)0xCDCDCDCDCDCDCDCD
171188
|| value == (uintptr_t)0xDDDDDDDDDDDDDDDD
172189
|| value == (uintptr_t)0xFDFDFDFDFDFDFDFD);
173190
#elif SIZEOF_VOID_P == 4
174-
return (value == (uintptr_t)0xCDCDCDCD
191+
return (value == 0
192+
|| value == (uintptr_t)0xCDCDCDCD
175193
|| value == (uintptr_t)0xDDDDDDDD
176194
|| value == (uintptr_t)0xFDFDFDFD);
177195
#else

‎Lib/test/test_capi.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -692,6 +692,9 @@ def check_pyobject_is_freed(self, func_name):
692692
''')
693693
assert_python_ok('-c', code, PYTHONMALLOC=self.PYTHONMALLOC)
694694

695+
def test_pyobject_null_is_freed(self):
696+
self.check_pyobject_is_freed('check_pyobject_null_is_freed')
697+
695698
def test_pyobject_uninitialized_is_freed(self):
696699
self.check_pyobject_is_freed('check_pyobject_uninitialized_is_freed')
697700

‎Lib/test/test_gc.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1055,16 +1055,19 @@ def test_refcount_errors(self):
10551055
br'gcmodule\.c:[0-9]+: gc_decref: Assertion "gc_get_refs\(g\) > 0" failed.')
10561056
self.assertRegex(stderr,
10571057
br'refcount is too small')
1058+
# "address : 0x7fb5062efc18"
1059+
# "address : 7FB5062EFC18"
1060+
address_regex = br'[0-9a-fA-Fx]+'
10581061
self.assertRegex(stderr,
1059-
br'object : \[1, 2, 3\]')
1062+
br'object address : ' + address_regex)
10601063
self.assertRegex(stderr,
1061-
br'type : list')
1064+
br'object refcount : 1')
10621065
self.assertRegex(stderr,
1063-
br'refcount: 1')
1064-
# "address : 0x7fb5062efc18"
1065-
# "address : 7FB5062EFC18"
1066+
br'object type : ' + address_regex)
1067+
self.assertRegex(stderr,
1068+
br'object type name: list')
10661069
self.assertRegex(stderr,
1067-
br'address : [0-9a-fA-Fx]+')
1070+
br'object repr : \[1, 2, 3\]')
10681071

10691072

10701073
class GCTogglingTests(unittest.TestCase):

‎Misc/NEWS.d/next/C API/2019-10-07-17-15-09.bpo-36389.hFX_jD.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
The ``_PyObject_CheckConsistency()`` function is now also available in release
2+
mode. For example, it can be used to debug a crash in the ``visit_decref()``
3+
function of the GC.

‎Modules/_testcapimodule.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4500,6 +4500,14 @@ test_pyobject_is_freed(const char *test_name, PyObject *op)
45004500
}
45014501

45024502

4503+
static PyObject*
4504+
check_pyobject_null_is_freed(PyObject *self, PyObject *Py_UNUSED(args))
4505+
{
4506+
PyObject *op = NULL;
4507+
return test_pyobject_is_freed("check_pyobject_null_is_freed", op);
4508+
}
4509+
4510+
45034511
static PyObject*
45044512
check_pyobject_uninitialized_is_freed(PyObject *self, PyObject *Py_UNUSED(args))
45054513
{
@@ -5268,6 +5276,7 @@ static PyMethodDef TestMethods[] = {
52685276
{"pymem_api_misuse", pymem_api_misuse, METH_NOARGS},
52695277
{"pymem_malloc_without_gil", pymem_malloc_without_gil, METH_NOARGS},
52705278
{"pymem_getallocatorsname", test_pymem_getallocatorsname, METH_NOARGS},
5279+
{"check_pyobject_null_is_freed", check_pyobject_null_is_freed, METH_NOARGS},
52715280
{"check_pyobject_uninitialized_is_freed", check_pyobject_uninitialized_is_freed, METH_NOARGS},
52725281
{"check_pyobject_forbidden_bytes_is_freed", check_pyobject_forbidden_bytes_is_freed, METH_NOARGS},
52735282
{"check_pyobject_freed_is_freed", check_pyobject_freed_is_freed, METH_NOARGS},

‎Modules/gcmodule.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -373,10 +373,9 @@ update_refs(PyGC_Head *containers)
373373

374374
/* A traversal callback for subtract_refs. */
375375
static int
376-
visit_decref(PyObject *op, void *data)
376+
visit_decref(PyObject *op, void *parent)
377377
{
378-
assert(op != NULL);
379-
_PyObject_ASSERT(op, !_PyObject_IsFreed(op));
378+
_PyObject_ASSERT(_PyObject_CAST(parent), !_PyObject_IsFreed(op));
380379

381380
if (PyObject_IS_GC(op)) {
382381
PyGC_Head *gc = AS_GC(op);
@@ -402,10 +401,11 @@ subtract_refs(PyGC_Head *containers)
402401
traverseproc traverse;
403402
PyGC_Head *gc = GC_NEXT(containers);
404403
for (; gc != containers; gc = GC_NEXT(gc)) {
405-
traverse = Py_TYPE(FROM_GC(gc))->tp_traverse;
404+
PyObject *op = FROM_GC(gc);
405+
traverse = Py_TYPE(op)->tp_traverse;
406406
(void) traverse(FROM_GC(gc),
407407
(visitproc)visit_decref,
408-
NULL);
408+
op);
409409
}
410410
}
411411

‎Objects/bytesobject.c

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -667,9 +667,6 @@ _PyBytes_FormatEx(const char *format, Py_ssize_t format_len,
667667
Py_ssize_t len = 0;
668668
char onechar; /* For byte_converter() */
669669
Py_ssize_t alloc;
670-
#ifdef Py_DEBUG
671-
char *before;
672-
#endif
673670

674671
fmt++;
675672
if (*fmt == '%') {
@@ -981,8 +978,8 @@ _PyBytes_FormatEx(const char *format, Py_ssize_t format_len,
981978
if (res == NULL)
982979
goto error;
983980
}
984-
#ifdef Py_DEBUG
985-
before = res;
981+
#ifndef NDEBUG
982+
char *before = res;
986983
#endif
987984

988985
/* Write the sign if needed */
@@ -1047,7 +1044,7 @@ _PyBytes_FormatEx(const char *format, Py_ssize_t format_len,
10471044
}
10481045
Py_XDECREF(temp);
10491046

1050-
#ifdef Py_DEBUG
1047+
#ifndef NDEBUG
10511048
/* check that we computed the exact size for this write */
10521049
assert((res - before) == alloc);
10531050
#endif
@@ -3225,8 +3222,9 @@ _PyBytesWriter_Init(_PyBytesWriter *writer)
32253222
{
32263223
/* Set all attributes before small_buffer to 0 */
32273224
memset(writer, 0, offsetof(_PyBytesWriter, small_buffer));
3228-
#ifdef Py_DEBUG
3229-
memset(writer->small_buffer, 0xCB, sizeof(writer->small_buffer));
3225+
#ifndef NDEBUG
3226+
memset(writer->small_buffer, PYMEM_CLEANBYTE,
3227+
sizeof(writer->small_buffer));
32303228
#endif
32313229
}
32323230

@@ -3263,10 +3261,10 @@ _PyBytesWriter_GetSize(_PyBytesWriter *writer, char *str)
32633261
return str - start;
32643262
}
32653263

3266-
Py_LOCAL_INLINE(void)
3264+
#ifndef NDEBUG
3265+
Py_LOCAL_INLINE(int)
32673266
_PyBytesWriter_CheckConsistency(_PyBytesWriter *writer, char *str)
32683267
{
3269-
#ifdef Py_DEBUG
32703268
char *start, *end;
32713269

32723270
if (writer->use_small_buffer) {
@@ -3296,15 +3294,16 @@ _PyBytesWriter_CheckConsistency(_PyBytesWriter *writer, char *str)
32963294
end = start + writer->allocated;
32973295
assert(str != NULL);
32983296
assert(start <= str && str <= end);
3299-
#endif
3297+
return 1;
33003298
}
3299+
#endif
33013300

33023301
void*
33033302
_PyBytesWriter_Resize(_PyBytesWriter *writer, void *str, Py_ssize_t size)
33043303
{
33053304
Py_ssize_t allocated, pos;
33063305

3307-
_PyBytesWriter_CheckConsistency(writer, str);
3306+
assert(_PyBytesWriter_CheckConsistency(writer, str));
33083307
assert(writer->allocated < size);
33093308

33103309
allocated = size;
@@ -3353,14 +3352,15 @@ _PyBytesWriter_Resize(_PyBytesWriter *writer, void *str, Py_ssize_t size)
33533352
}
33543353

33553354
writer->use_small_buffer = 0;
3356-
#ifdef Py_DEBUG
3357-
memset(writer->small_buffer, 0xDB, sizeof(writer->small_buffer));
3355+
#ifndef NDEBUG
3356+
memset(writer->small_buffer, PYMEM_CLEANBYTE,
3357+
sizeof(writer->small_buffer));
33583358
#endif
33593359
}
33603360
writer->allocated = allocated;
33613361

33623362
str = _PyBytesWriter_AsString(writer) + pos;
3363-
_PyBytesWriter_CheckConsistency(writer, str);
3363+
assert(_PyBytesWriter_CheckConsistency(writer, str));
33643364
return str;
33653365

33663366
error:
@@ -3373,7 +3373,7 @@ _PyBytesWriter_Prepare(_PyBytesWriter *writer, void *str, Py_ssize_t size)
33733373
{
33743374
Py_ssize_t new_min_size;
33753375

3376-
_PyBytesWriter_CheckConsistency(writer, str);
3376+
assert(_PyBytesWriter_CheckConsistency(writer, str));
33773377
assert(size >= 0);
33783378

33793379
if (size == 0) {
@@ -3406,7 +3406,7 @@ _PyBytesWriter_Alloc(_PyBytesWriter *writer, Py_ssize_t size)
34063406
assert(size >= 0);
34073407

34083408
writer->use_small_buffer = 1;
3409-
#ifdef Py_DEBUG
3409+
#ifndef NDEBUG
34103410
writer->allocated = sizeof(writer->small_buffer) - 1;
34113411
/* In debug mode, don't use the full small buffer because it is less
34123412
efficient than bytes and bytearray objects to detect buffer underflow
@@ -3434,7 +3434,7 @@ _PyBytesWriter_Finish(_PyBytesWriter *writer, void *str)
34343434
Py_ssize_t size;
34353435
PyObject *result;
34363436

3437-
_PyBytesWriter_CheckConsistency(writer, str);
3437+
assert(_PyBytesWriter_CheckConsistency(writer, str));
34383438

34393439
size = _PyBytesWriter_GetSize(writer, str);
34403440
if (size == 0 && !writer->use_bytearray) {

‎Objects/dictobject.c

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -459,23 +459,26 @@ static PyObject *empty_values[1] = { NULL };
459459
int
460460
_PyDict_CheckConsistency(PyObject *op, int check_content)
461461
{
462-
#ifndef NDEBUG
463-
_PyObject_ASSERT(op, PyDict_Check(op));
462+
#define CHECK(expr) \
463+
do { if (!(expr)) { _PyObject_ASSERT_FAILED_MSG(op, Py_STRINGIFY(expr)); } } while (0)
464+
465+
assert(op != NULL);
466+
CHECK(PyDict_Check(op));
464467
PyDictObject *mp = (PyDictObject *)op;
465468

466469
PyDictKeysObject *keys = mp->ma_keys;
467470
int splitted = _PyDict_HasSplitTable(mp);
468471
Py_ssize_t usable = USABLE_FRACTION(keys->dk_size);
469472

470-
_PyObject_ASSERT(op, 0 <= mp->ma_used && mp->ma_used <= usable);
471-
_PyObject_ASSERT(op, IS_POWER_OF_2(keys->dk_size));
472-
_PyObject_ASSERT(op, 0 <= keys->dk_usable && keys->dk_usable <= usable);
473-
_PyObject_ASSERT(op, 0 <= keys->dk_nentries && keys->dk_nentries <= usable);
474-
_PyObject_ASSERT(op, keys->dk_usable + keys->dk_nentries <= usable);
473+
CHECK(0 <= mp->ma_used && mp->ma_used <= usable);
474+
CHECK(IS_POWER_OF_2(keys->dk_size));
475+
CHECK(0 <= keys->dk_usable && keys->dk_usable <= usable);
476+
CHECK(0 <= keys->dk_nentries && keys->dk_nentries <= usable);
477+
CHECK(keys->dk_usable + keys->dk_nentries <= usable);
475478

476479
if (!splitted) {
477480
/* combined table */
478-
_PyObject_ASSERT(op, keys->dk_refcnt == 1);
481+
CHECK(keys->dk_refcnt == 1);
479482
}
480483

481484
if (check_content) {
@@ -484,7 +487,7 @@ _PyDict_CheckConsistency(PyObject *op, int check_content)
484487

485488
for (i=0; i < keys->dk_size; i++) {
486489
Py_ssize_t ix = dictkeys_get_index(keys, i);
487-
_PyObject_ASSERT(op, DKIX_DUMMY <= ix && ix <= usable);
490+
CHECK(DKIX_DUMMY <= ix && ix <= usable);
488491
}
489492

490493
for (i=0; i < usable; i++) {
@@ -494,32 +497,33 @@ _PyDict_CheckConsistency(PyObject *op, int check_content)
494497
if (key != NULL) {
495498
if (PyUnicode_CheckExact(key)) {
496499
Py_hash_t hash = ((PyASCIIObject *)key)->hash;
497-
_PyObject_ASSERT(op, hash != -1);
498-
_PyObject_ASSERT(op, entry->me_hash == hash);
500+
CHECK(hash != -1);
501+
CHECK(entry->me_hash == hash);
499502
}
500503
else {
501504
/* test_dict fails if PyObject_Hash() is called again */
502-
_PyObject_ASSERT(op, entry->me_hash != -1);
505+
CHECK(entry->me_hash != -1);
503506
}
504507
if (!splitted) {
505-
_PyObject_ASSERT(op, entry->me_value != NULL);
508+
CHECK(entry->me_value != NULL);
506509
}
507510
}
508511

509512
if (splitted) {
510-
_PyObject_ASSERT(op, entry->me_value == NULL);
513+
CHECK(entry->me_value == NULL);
511514
}
512515
}
513516

514517
if (splitted) {
515518
/* splitted table */
516519
for (i=0; i < mp->ma_used; i++) {
517-
_PyObject_ASSERT(op, mp->ma_values[i] != NULL);
520+
CHECK(mp->ma_values[i] != NULL);
518521
}
519522
}
520523
}
521-
#endif
522524
return 1;
525+
526+
#undef CHECK
523527
}
524528

525529

0 commit comments

Comments
 (0)
0