From cdf6472af5fd6ecdbd9d1b676dc538dd87c10672 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 26 Oct 2018 00:24:52 +0200 Subject: [PATCH 1/3] bpo-9263: CheckConsistency() use _PyObject_ASSERT() Use _PyObject_ASSERT() in: * _PyDict_CheckConsistency() * _PyType_CheckConsistency() * _PyUnicode_CheckConsistency() _PyObject_ASSERT() dumps the object if the assertion fails to help debug. --- Objects/dictobject.c | 30 ++++++++++------- Objects/typeobject.c | 26 ++++++++------ Objects/unicodeobject.c | 75 +++++++++++++++++++++-------------------- 3 files changed, 72 insertions(+), 59 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 370895d6bcc8fd..09ea027a9b9b30 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -439,6 +439,8 @@ static PyObject *empty_values[1] = { NULL }; static int _PyDict_CheckConsistency(PyDictObject *mp) { +#define ASSERT(expr) _PyObject_ASSERT((PyObject *)mp, expr) + PyDictKeysObject *keys = mp->ma_keys; int splitted = _PyDict_HasSplitTable(mp); Py_ssize_t usable = USABLE_FRACTION(keys->dk_size); @@ -447,23 +449,23 @@ _PyDict_CheckConsistency(PyDictObject *mp) Py_ssize_t i; #endif - assert(0 <= mp->ma_used && mp->ma_used <= usable); - assert(IS_POWER_OF_2(keys->dk_size)); - assert(0 <= keys->dk_usable + ASSERT(0 <= mp->ma_used && mp->ma_used <= usable); + ASSERT(IS_POWER_OF_2(keys->dk_size)); + ASSERT(0 <= keys->dk_usable && keys->dk_usable <= usable); - assert(0 <= keys->dk_nentries + ASSERT(0 <= keys->dk_nentries && keys->dk_nentries <= usable); - assert(keys->dk_usable + keys->dk_nentries <= usable); + ASSERT(keys->dk_usable + keys->dk_nentries <= usable); if (!splitted) { /* combined table */ - assert(keys->dk_refcnt == 1); + ASSERT(keys->dk_refcnt == 1); } #ifdef DEBUG_PYDICT for (i=0; i < keys->dk_size; i++) { Py_ssize_t ix = dk_get_index(keys, i); - assert(DKIX_DUMMY <= ix && ix <= usable); + ASSERT(DKIX_DUMMY <= ix && ix <= usable); } for (i=0; i < usable; i++) { @@ -473,32 +475,34 @@ _PyDict_CheckConsistency(PyDictObject *mp) if (key != NULL) { if (PyUnicode_CheckExact(key)) { Py_hash_t hash = ((PyASCIIObject *)key)->hash; - assert(hash != -1); - assert(entry->me_hash == hash); + ASSERT(hash != -1); + ASSERT(entry->me_hash == hash); } else { /* test_dict fails if PyObject_Hash() is called again */ - assert(entry->me_hash != -1); + ASSERT(entry->me_hash != -1); } if (!splitted) { - assert(entry->me_value != NULL); + ASSERT(entry->me_value != NULL); } } if (splitted) { - assert(entry->me_value == NULL); + ASSERT(entry->me_value == NULL); } } if (splitted) { /* splitted table */ for (i=0; i < mp->ma_used; i++) { - assert(mp->ma_values[i] != NULL); + ASSERT(mp->ma_values[i] != NULL); } } #endif return 1; + +#undef ASSERT } #endif diff --git a/Objects/typeobject.c b/Objects/typeobject.c index b9e69bf1bd1d64..cf02c7b71a5f29 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -134,15 +134,19 @@ skip_signature(const char *doc) static int _PyType_CheckConsistency(PyTypeObject *type) { +#define ASSERT(expr) _PyObject_ASSERT((PyObject *)type, expr) + if (!(type->tp_flags & Py_TPFLAGS_READY)) { /* don't check types before PyType_Ready() */ return 1; } - assert(!(type->tp_flags & Py_TPFLAGS_READYING)); - assert(type->tp_mro != NULL && PyTuple_Check(type->tp_mro)); - assert(type->tp_dict != NULL); + ASSERT(!(type->tp_flags & Py_TPFLAGS_READYING)); + ASSERT(type->tp_mro != NULL && PyTuple_Check(type->tp_mro)); + ASSERT(type->tp_dict != NULL); return 1; + +#undef ASSERT } #endif @@ -1116,7 +1120,7 @@ subtype_dealloc(PyObject *self) /* Extract the type; we expect it to be a heap type */ type = Py_TYPE(self); - assert(type->tp_flags & Py_TPFLAGS_HEAPTYPE); + _PyObject_ASSERT(self, type->tp_flags & Py_TPFLAGS_HEAPTYPE); /* Test whether the type has GC exactly once */ @@ -2214,9 +2218,10 @@ subtype_getweakref(PyObject *obj, void *context) "This object has no __weakref__"); return NULL; } - assert(Py_TYPE(obj)->tp_weaklistoffset > 0); - assert(Py_TYPE(obj)->tp_weaklistoffset + sizeof(PyObject *) <= - (size_t)(Py_TYPE(obj)->tp_basicsize)); + _PyObject_ASSERT(obj, Py_TYPE(obj)->tp_weaklistoffset > 0); + _PyObject_ASSERT(obj, + (Py_TYPE(obj)->tp_weaklistoffset + sizeof(PyObject *)) \ + <= (size_t)(Py_TYPE(obj)->tp_basicsize)); weaklistptr = (PyObject **) ((char *)obj + Py_TYPE(obj)->tp_weaklistoffset); if (*weaklistptr == NULL) @@ -3279,7 +3284,7 @@ type_dealloc(PyTypeObject *type) PyObject *tp, *val, *tb; /* Assert this is a heap-allocated type object */ - assert(type->tp_flags & Py_TPFLAGS_HEAPTYPE); + _PyObject_ASSERT((PyObject *)type, type->tp_flags & Py_TPFLAGS_HEAPTYPE); _PyObject_GC_UNTRACK(type); PyErr_Fetch(&tp, &val, &tb); remove_all_subclasses(type, type->tp_bases); @@ -3503,7 +3508,7 @@ type_clear(PyTypeObject *type) PyDictKeysObject *cached_keys; /* Because of type_is_gc(), the collector only calls this for heaptypes. */ - assert(type->tp_flags & Py_TPFLAGS_HEAPTYPE); + _PyObject_ASSERT((PyObject *)type, type->tp_flags & Py_TPFLAGS_HEAPTYPE); /* We need to invalidate the method cache carefully before clearing the dict, so that other objects caught in a reference cycle @@ -5117,7 +5122,8 @@ PyType_Ready(PyTypeObject *type) assert(_PyType_CheckConsistency(type)); return 0; } - assert((type->tp_flags & Py_TPFLAGS_READYING) == 0); + _PyObject_ASSERT((PyObject *)type, + (type->tp_flags & Py_TPFLAGS_READYING) == 0); type->tp_flags |= Py_TPFLAGS_READYING; diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c index 31703d37ddbd70..e0c9695fe7c55a 100644 --- a/Objects/unicodeobject.c +++ b/Objects/unicodeobject.c @@ -363,17 +363,18 @@ PyUnicode_GetMax(void) int _PyUnicode_CheckConsistency(PyObject *op, int check_content) { +#define ASSERT(expr) _PyObject_ASSERT(op, expr) PyASCIIObject *ascii; unsigned int kind; - assert(PyUnicode_Check(op)); + ASSERT(PyUnicode_Check(op)); ascii = (PyASCIIObject *)op; kind = ascii->state.kind; if (ascii->state.ascii == 1 && ascii->state.compact == 1) { - assert(kind == PyUnicode_1BYTE_KIND); - assert(ascii->state.ready == 1); + ASSERT(kind == PyUnicode_1BYTE_KIND); + ASSERT(ascii->state.ready == 1); } else { PyCompactUnicodeObject *compact = (PyCompactUnicodeObject *)op; @@ -381,41 +382,41 @@ _PyUnicode_CheckConsistency(PyObject *op, int check_content) if (ascii->state.compact == 1) { data = compact + 1; - assert(kind == PyUnicode_1BYTE_KIND + ASSERT(kind == PyUnicode_1BYTE_KIND || kind == PyUnicode_2BYTE_KIND || kind == PyUnicode_4BYTE_KIND); - assert(ascii->state.ascii == 0); - assert(ascii->state.ready == 1); - assert (compact->utf8 != data); + ASSERT(ascii->state.ascii == 0); + ASSERT(ascii->state.ready == 1); + ASSERT (compact->utf8 != data); } else { PyUnicodeObject *unicode = (PyUnicodeObject *)op; data = unicode->data.any; if (kind == PyUnicode_WCHAR_KIND) { - assert(ascii->length == 0); - assert(ascii->hash == -1); - assert(ascii->state.compact == 0); - assert(ascii->state.ascii == 0); - assert(ascii->state.ready == 0); - assert(ascii->state.interned == SSTATE_NOT_INTERNED); - assert(ascii->wstr != NULL); - assert(data == NULL); - assert(compact->utf8 == NULL); + ASSERT(ascii->length == 0); + ASSERT(ascii->hash == -1); + ASSERT(ascii->state.compact == 0); + ASSERT(ascii->state.ascii == 0); + ASSERT(ascii->state.ready == 0); + ASSERT(ascii->state.interned == SSTATE_NOT_INTERNED); + ASSERT(ascii->wstr != NULL); + ASSERT(data == NULL); + ASSERT(compact->utf8 == NULL); } else { - assert(kind == PyUnicode_1BYTE_KIND + ASSERT(kind == PyUnicode_1BYTE_KIND || kind == PyUnicode_2BYTE_KIND || kind == PyUnicode_4BYTE_KIND); - assert(ascii->state.compact == 0); - assert(ascii->state.ready == 1); - assert(data != NULL); + ASSERT(ascii->state.compact == 0); + ASSERT(ascii->state.ready == 1); + ASSERT(data != NULL); if (ascii->state.ascii) { - assert (compact->utf8 == data); - assert (compact->utf8_length == ascii->length); + ASSERT (compact->utf8 == data); + ASSERT (compact->utf8_length == ascii->length); } else - assert (compact->utf8 != data); + ASSERT (compact->utf8 != data); } } if (kind != PyUnicode_WCHAR_KIND) { @@ -427,16 +428,16 @@ _PyUnicode_CheckConsistency(PyObject *op, int check_content) #endif ) { - assert(ascii->wstr == data); - assert(compact->wstr_length == ascii->length); + ASSERT(ascii->wstr == data); + ASSERT(compact->wstr_length == ascii->length); } else - assert(ascii->wstr != data); + ASSERT(ascii->wstr != data); } if (compact->utf8 == NULL) - assert(compact->utf8_length == 0); + ASSERT(compact->utf8_length == 0); if (ascii->wstr == NULL) - assert(compact->wstr_length == 0); + ASSERT(compact->wstr_length == 0); } /* check that the best kind is used */ if (check_content && kind != PyUnicode_WCHAR_KIND) @@ -455,23 +456,25 @@ _PyUnicode_CheckConsistency(PyObject *op, int check_content) } if (kind == PyUnicode_1BYTE_KIND) { if (ascii->state.ascii == 0) { - assert(maxchar >= 128); - assert(maxchar <= 255); + ASSERT(maxchar >= 128); + ASSERT(maxchar <= 255); } else - assert(maxchar < 128); + ASSERT(maxchar < 128); } else if (kind == PyUnicode_2BYTE_KIND) { - assert(maxchar >= 0x100); - assert(maxchar <= 0xFFFF); + ASSERT(maxchar >= 0x100); + ASSERT(maxchar <= 0xFFFF); } else { - assert(maxchar >= 0x10000); - assert(maxchar <= MAX_UNICODE); + ASSERT(maxchar >= 0x10000); + ASSERT(maxchar <= MAX_UNICODE); } - assert(PyUnicode_READ(kind, data, ascii->length) == 0); + ASSERT(PyUnicode_READ(kind, data, ascii->length) == 0); } return 1; + +#undef ASSERT } #endif From 6e9d91c0347a54e838c00f74216226f15a0d9a90 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 26 Oct 2018 17:20:32 +0200 Subject: [PATCH 2/3] ASSERT: add parenthesis around expr --- Objects/dictobject.c | 2 +- Objects/typeobject.c | 2 +- Objects/unicodeobject.c | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 09ea027a9b9b30..ee656953e0723c 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -439,7 +439,7 @@ static PyObject *empty_values[1] = { NULL }; static int _PyDict_CheckConsistency(PyDictObject *mp) { -#define ASSERT(expr) _PyObject_ASSERT((PyObject *)mp, expr) +#define ASSERT(expr) _PyObject_ASSERT((PyObject *)mp, (expr)) PyDictKeysObject *keys = mp->ma_keys; int splitted = _PyDict_HasSplitTable(mp); diff --git a/Objects/typeobject.c b/Objects/typeobject.c index cf02c7b71a5f29..f18b9ace5b5c44 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -134,7 +134,7 @@ skip_signature(const char *doc) static int _PyType_CheckConsistency(PyTypeObject *type) { -#define ASSERT(expr) _PyObject_ASSERT((PyObject *)type, expr) +#define ASSERT(expr) _PyObject_ASSERT((PyObject *)type, (expr)) if (!(type->tp_flags & Py_TPFLAGS_READY)) { /* don't check types before PyType_Ready() */ diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c index e0c9695fe7c55a..f3f940ac9ef573 100644 --- a/Objects/unicodeobject.c +++ b/Objects/unicodeobject.c @@ -363,7 +363,8 @@ PyUnicode_GetMax(void) int _PyUnicode_CheckConsistency(PyObject *op, int check_content) { -#define ASSERT(expr) _PyObject_ASSERT(op, expr) +#define ASSERT(expr) _PyObject_ASSERT(op, (expr)) + PyASCIIObject *ascii; unsigned int kind; From 092d6f95c13eaac8cc1d853c0b8bd757967413d2 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 26 Oct 2018 18:01:42 +0200 Subject: [PATCH 3/3] Revert unwanted changes --- Objects/typeobject.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index f18b9ace5b5c44..28976ee2a858d5 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -1120,7 +1120,7 @@ subtype_dealloc(PyObject *self) /* Extract the type; we expect it to be a heap type */ type = Py_TYPE(self); - _PyObject_ASSERT(self, type->tp_flags & Py_TPFLAGS_HEAPTYPE); + assert(type->tp_flags & Py_TPFLAGS_HEAPTYPE); /* Test whether the type has GC exactly once */ @@ -2218,10 +2218,9 @@ subtype_getweakref(PyObject *obj, void *context) "This object has no __weakref__"); return NULL; } - _PyObject_ASSERT(obj, Py_TYPE(obj)->tp_weaklistoffset > 0); - _PyObject_ASSERT(obj, - (Py_TYPE(obj)->tp_weaklistoffset + sizeof(PyObject *)) \ - <= (size_t)(Py_TYPE(obj)->tp_basicsize)); + assert(Py_TYPE(obj)->tp_weaklistoffset > 0); + assert(Py_TYPE(obj)->tp_weaklistoffset + sizeof(PyObject *) <= + (size_t)(Py_TYPE(obj)->tp_basicsize)); weaklistptr = (PyObject **) ((char *)obj + Py_TYPE(obj)->tp_weaklistoffset); if (*weaklistptr == NULL) @@ -3284,7 +3283,7 @@ type_dealloc(PyTypeObject *type) PyObject *tp, *val, *tb; /* Assert this is a heap-allocated type object */ - _PyObject_ASSERT((PyObject *)type, type->tp_flags & Py_TPFLAGS_HEAPTYPE); + assert(type->tp_flags & Py_TPFLAGS_HEAPTYPE); _PyObject_GC_UNTRACK(type); PyErr_Fetch(&tp, &val, &tb); remove_all_subclasses(type, type->tp_bases); @@ -3508,7 +3507,7 @@ type_clear(PyTypeObject *type) PyDictKeysObject *cached_keys; /* Because of type_is_gc(), the collector only calls this for heaptypes. */ - _PyObject_ASSERT((PyObject *)type, type->tp_flags & Py_TPFLAGS_HEAPTYPE); + assert(type->tp_flags & Py_TPFLAGS_HEAPTYPE); /* We need to invalidate the method cache carefully before clearing the dict, so that other objects caught in a reference cycle @@ -5122,8 +5121,7 @@ PyType_Ready(PyTypeObject *type) assert(_PyType_CheckConsistency(type)); return 0; } - _PyObject_ASSERT((PyObject *)type, - (type->tp_flags & Py_TPFLAGS_READYING) == 0); + assert((type->tp_flags & Py_TPFLAGS_READYING) == 0); type->tp_flags |= Py_TPFLAGS_READYING;