From 9d51e41d718a35b6c382ed5140dfa32a69c6a301 Mon Sep 17 00:00:00 2001 From: Thomas Wouters Date: Wed, 29 Mar 2017 09:08:20 -0700 Subject: [PATCH 1/4] Make a non-Py_DEBUG, asserts-enabled build of CPython possible. This means making sure helper functions are defiend when NDEBUG is not defined, not just when Py_DEBUG is defined. Also fix a division-by-zero in obmalloc.c: in Py_DEBUG mode, elsize is never 0, so this assert is never triggered. --- Include/unicodeobject.h | 4 ++++ Objects/dictobject.c | 2 +- Objects/obmalloc.c | 2 +- Objects/typeobject.c | 2 +- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/Include/unicodeobject.h b/Include/unicodeobject.h index c3d0516067c4af..0d10c71ed21ad8 100644 --- a/Include/unicodeobject.h +++ b/Include/unicodeobject.h @@ -2313,6 +2313,10 @@ PyAPI_FUNC(Py_UNICODE*) PyUnicode_AsUnicodeCopy( PyAPI_FUNC(int) _PyUnicode_CheckConsistency( PyObject *op, int check_content); +#elif !defined(NDEBUG) +/* For asserts that call _PyUnicode_CheckConsistency(), which would + * otherwise be a problem when building with asserts but without Py_DEBUG. */ +#define _PyUnicode_CheckConsistency(op, check_content) PyUnicode_Check(op) #endif #ifndef Py_LIMITED_API diff --git a/Objects/dictobject.c b/Objects/dictobject.c index aac7ac4467a026..254311869b6f4b 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -437,7 +437,7 @@ static PyObject *empty_values[1] = { NULL }; /* #define DEBUG_PYDICT */ -#ifdef Py_DEBUG +#ifndef NDEBUG static int _PyDict_CheckConsistency(PyDictObject *mp) { diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index a1142f3b09ad9e..32e7ecbe1e0436 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1227,7 +1227,7 @@ _PyObject_Alloc(int use_calloc, void *ctx, size_t nelem, size_t elsize) _Py_AllocatedBlocks++; - assert(nelem <= PY_SSIZE_T_MAX / elsize); + assert(elsize == 0 || nelem <= PY_SSIZE_T_MAX / elsize); nbytes = nelem * elsize; #ifdef WITH_VALGRIND diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 64a72d2f146612..4a9949e4022690 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -128,7 +128,7 @@ skip_signature(const char *doc) return NULL; } -#ifdef Py_DEBUG +#ifndef NDEBUG static int _PyType_CheckConsistency(PyTypeObject *type) { From 73332986f7b4a3335f48135932b5235c97b977f6 Mon Sep 17 00:00:00 2001 From: Thomas Wouters Date: Wed, 29 Mar 2017 09:27:10 -0700 Subject: [PATCH 2/4] Only check for PyErr_Occurred() in asserts when in a Py_DEBUG build. Unlike other asserts this is easy to trigger indirectly, and the assertion failure will not point to the actual problem. --- Objects/abstract.c | 4 ++++ Objects/call.c | 13 +++++++++++++ 2 files changed, 17 insertions(+) diff --git a/Objects/abstract.c b/Objects/abstract.c index 4a75b92e1d0895..6382f22a555b80 100644 --- a/Objects/abstract.c +++ b/Objects/abstract.c @@ -146,7 +146,9 @@ PyObject_GetItem(PyObject *o, PyObject *key) m = o->ob_type->tp_as_mapping; if (m && m->mp_subscript) { PyObject *item = m->mp_subscript(o, key); +#ifdef Py_DEBUG assert((item != NULL) ^ (PyErr_Occurred() != NULL)); +#endif return item; } @@ -1623,7 +1625,9 @@ PySequence_GetItem(PyObject *s, Py_ssize_t i) if (m->sq_length) { Py_ssize_t l = (*m->sq_length)(s); if (l < 0) { +#ifdef Py_DEBUG assert(PyErr_Occurred()); +#endif return NULL; } i += l; diff --git a/Objects/call.c b/Objects/call.c index 4c74eab44f5e43..c42c46c3c6634a 100644 --- a/Objects/call.c +++ b/Objects/call.c @@ -83,10 +83,12 @@ PyObject * _PyObject_FastCallDict(PyObject *callable, PyObject **args, Py_ssize_t nargs, PyObject *kwargs) { +#ifdef Py_DEBUG /* _PyObject_FastCallDict() must not be called with an exception set, because it can clear it (directly or indirectly) and so the caller loses its exception */ assert(!PyErr_Occurred()); +#endif assert(callable != NULL); assert(nargs >= 0); @@ -136,10 +138,12 @@ PyObject * _PyObject_FastCallKeywords(PyObject *callable, PyObject **stack, Py_ssize_t nargs, PyObject *kwnames) { +#ifdef Py_DEBUG /* _PyObject_FastCallKeywords() must not be called with an exception set, because it can clear it (directly or indirectly) and so the caller loses its exception */ assert(!PyErr_Occurred()); +#endif assert(nargs >= 0); assert(kwnames == NULL || PyTuple_CheckExact(kwnames)); @@ -214,10 +218,13 @@ PyObject_Call(PyObject *callable, PyObject *args, PyObject *kwargs) ternaryfunc call; PyObject *result; +#ifdef Py_DEBUG /* PyObject_Call() must not be called with an exception set, because it can clear it (directly or indirectly) and so the caller loses its exception */ assert(!PyErr_Occurred()); +#endif + assert(PyTuple_Check(args)); assert(kwargs == NULL || PyDict_Check(kwargs)); @@ -445,10 +452,12 @@ PyObject * _PyMethodDef_RawFastCallDict(PyMethodDef *method, PyObject *self, PyObject **args, Py_ssize_t nargs, PyObject *kwargs) { +#ifdef Py_DEBUG /* _PyMethodDef_RawFastCallDict() must not be called with an exception set, because it can clear it (directly or indirectly) and so the caller loses its exception */ assert(!PyErr_Occurred()); +#endif assert(method != NULL); assert(nargs >= 0); @@ -579,10 +588,12 @@ PyObject * _PyMethodDef_RawFastCallKeywords(PyMethodDef *method, PyObject *self, PyObject **args, Py_ssize_t nargs, PyObject *kwnames) { +#ifdef Py_DEBUG /* _PyMethodDef_RawFastCallKeywords() must not be called with an exception set, because it can clear it (directly or indirectly) and so the caller loses its exception */ assert(!PyErr_Occurred()); +#endif assert(method != NULL); assert(nargs >= 0); @@ -716,7 +727,9 @@ _PyCFunction_FastCallKeywords(PyObject *func, PyObject **args, static PyObject * cfunction_call_varargs(PyObject *func, PyObject *args, PyObject *kwargs) { +#ifdef Py_DEBUG assert(!PyErr_Occurred()); +#endif PyCFunction meth = PyCFunction_GET_FUNCTION(func); PyObject *self = PyCFunction_GET_SELF(func); From b61d7cb486122be8233d3eb4d5e70a4a41bad6c4 Mon Sep 17 00:00:00 2001 From: Thomas Wouters Date: Thu, 30 Mar 2017 11:38:57 -0700 Subject: [PATCH 3/4] Revert "Only check for PyErr_Occurred() in asserts when in a Py_DEBUG build." This reverts commit 73332986f7b4a3335f48135932b5235c97b977f6. --- Objects/abstract.c | 4 ---- Objects/call.c | 13 ------------- 2 files changed, 17 deletions(-) diff --git a/Objects/abstract.c b/Objects/abstract.c index 6382f22a555b80..4a75b92e1d0895 100644 --- a/Objects/abstract.c +++ b/Objects/abstract.c @@ -146,9 +146,7 @@ PyObject_GetItem(PyObject *o, PyObject *key) m = o->ob_type->tp_as_mapping; if (m && m->mp_subscript) { PyObject *item = m->mp_subscript(o, key); -#ifdef Py_DEBUG assert((item != NULL) ^ (PyErr_Occurred() != NULL)); -#endif return item; } @@ -1625,9 +1623,7 @@ PySequence_GetItem(PyObject *s, Py_ssize_t i) if (m->sq_length) { Py_ssize_t l = (*m->sq_length)(s); if (l < 0) { -#ifdef Py_DEBUG assert(PyErr_Occurred()); -#endif return NULL; } i += l; diff --git a/Objects/call.c b/Objects/call.c index c42c46c3c6634a..4c74eab44f5e43 100644 --- a/Objects/call.c +++ b/Objects/call.c @@ -83,12 +83,10 @@ PyObject * _PyObject_FastCallDict(PyObject *callable, PyObject **args, Py_ssize_t nargs, PyObject *kwargs) { -#ifdef Py_DEBUG /* _PyObject_FastCallDict() must not be called with an exception set, because it can clear it (directly or indirectly) and so the caller loses its exception */ assert(!PyErr_Occurred()); -#endif assert(callable != NULL); assert(nargs >= 0); @@ -138,12 +136,10 @@ PyObject * _PyObject_FastCallKeywords(PyObject *callable, PyObject **stack, Py_ssize_t nargs, PyObject *kwnames) { -#ifdef Py_DEBUG /* _PyObject_FastCallKeywords() must not be called with an exception set, because it can clear it (directly or indirectly) and so the caller loses its exception */ assert(!PyErr_Occurred()); -#endif assert(nargs >= 0); assert(kwnames == NULL || PyTuple_CheckExact(kwnames)); @@ -218,13 +214,10 @@ PyObject_Call(PyObject *callable, PyObject *args, PyObject *kwargs) ternaryfunc call; PyObject *result; -#ifdef Py_DEBUG /* PyObject_Call() must not be called with an exception set, because it can clear it (directly or indirectly) and so the caller loses its exception */ assert(!PyErr_Occurred()); -#endif - assert(PyTuple_Check(args)); assert(kwargs == NULL || PyDict_Check(kwargs)); @@ -452,12 +445,10 @@ PyObject * _PyMethodDef_RawFastCallDict(PyMethodDef *method, PyObject *self, PyObject **args, Py_ssize_t nargs, PyObject *kwargs) { -#ifdef Py_DEBUG /* _PyMethodDef_RawFastCallDict() must not be called with an exception set, because it can clear it (directly or indirectly) and so the caller loses its exception */ assert(!PyErr_Occurred()); -#endif assert(method != NULL); assert(nargs >= 0); @@ -588,12 +579,10 @@ PyObject * _PyMethodDef_RawFastCallKeywords(PyMethodDef *method, PyObject *self, PyObject **args, Py_ssize_t nargs, PyObject *kwnames) { -#ifdef Py_DEBUG /* _PyMethodDef_RawFastCallKeywords() must not be called with an exception set, because it can clear it (directly or indirectly) and so the caller loses its exception */ assert(!PyErr_Occurred()); -#endif assert(method != NULL); assert(nargs >= 0); @@ -727,9 +716,7 @@ _PyCFunction_FastCallKeywords(PyObject *func, PyObject **args, static PyObject * cfunction_call_varargs(PyObject *func, PyObject *args, PyObject *kwargs) { -#ifdef Py_DEBUG assert(!PyErr_Occurred()); -#endif PyCFunction meth = PyCFunction_GET_FUNCTION(func); PyObject *self = PyCFunction_GET_SELF(func); From a8be9b8512054457ef2199511ed692c5e5108201 Mon Sep 17 00:00:00 2001 From: Thomas Wouters Date: Thu, 30 Mar 2017 11:41:52 -0700 Subject: [PATCH 4/4] Move around some of the code. --- Objects/obmalloc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index 32e7ecbe1e0436..f284d9fc0a2c79 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1227,7 +1227,10 @@ _PyObject_Alloc(int use_calloc, void *ctx, size_t nelem, size_t elsize) _Py_AllocatedBlocks++; - assert(elsize == 0 || nelem <= PY_SSIZE_T_MAX / elsize); + if (nelem == 0 || elsize == 0) + goto redirect; + + assert(nelem <= PY_SSIZE_T_MAX / elsize); nbytes = nelem * elsize; #ifdef WITH_VALGRIND @@ -1237,9 +1240,6 @@ _PyObject_Alloc(int use_calloc, void *ctx, size_t nelem, size_t elsize) goto redirect; #endif - if (nelem == 0 || elsize == 0) - goto redirect; - if ((nbytes - 1) < SMALL_REQUEST_THRESHOLD) { LOCK(); /*