From 282cf4f592043e33d7b0f3fcfa142a49aefee455 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 3 Jul 2018 23:08:02 +0300 Subject: [PATCH 1/3] bpo-33720: Refactor marshalling/unmarshalling floats. This may reduce the stack consumption for unmarshalling deeply nested structures. --- Include/pyport.h | 6 +- Objects/call.c | 2 +- Python/marshal.c | 181 +++++++++++++++++++---------------------------- 3 files changed, 77 insertions(+), 112 deletions(-) diff --git a/Include/pyport.h b/Include/pyport.h index c1f4c7fbb52c99..f4b547a50b8580 100644 --- a/Include/pyport.h +++ b/Include/pyport.h @@ -525,8 +525,10 @@ extern "C" { * Usage: * int _Py_NO_INLINE x(void) { return 3; } */ -#if defined(__GNUC__) || defined(__clang__) -# define _Py_NO_INLINE __attribute__((noinline)) +#if defined(_MSC_VER) +# define _Py_NO_INLINE __declspec(noinline) +#elif defined(__GNUC__) || defined(__clang__) +# define _Py_NO_INLINE __attribute__ ((noinline)) #else # define _Py_NO_INLINE #endif diff --git a/Objects/call.c b/Objects/call.c index b53a98c76b0e33..1937a8b2278e85 100644 --- a/Objects/call.c +++ b/Objects/call.c @@ -1271,7 +1271,7 @@ PyObject_CallFunctionObjArgs(PyObject *callable, ...) /* Issue #29234: Inlining _PyStack_AsTuple() into callers increases their stack consumption, Disable inlining to optimize the stack consumption. */ -PyObject* _Py_NO_INLINE +_Py_NO_INLINE PyObject * _PyStack_AsTuple(PyObject *const *stack, Py_ssize_t nargs) { PyObject *args; diff --git a/Python/marshal.c b/Python/marshal.c index 6d06266c6a8e2e..21cdd60c7e1377 100644 --- a/Python/marshal.c +++ b/Python/marshal.c @@ -266,6 +266,32 @@ w_PyLong(const PyLongObject *ob, char flag, WFILE *p) } while (d != 0); } +static void +w_float_bin(double v, WFILE *p) +{ + unsigned char buf[8]; + if (_PyFloat_Pack8(v, buf, 1) < 0) { + p->error = WFERR_UNMARSHALLABLE; + return; + } + w_string((const char *)buf, 8, p); +} + +static void +w_float_str(double v, WFILE *p) +{ + int n; + char *buf = PyOS_double_to_string(v, 'g', 17, 0, NULL); + if (!buf) { + p->error = WFERR_NOMEMORY; + return; + } + n = (int)strlen(buf); + w_byte(n, p); + w_string(buf, n, p); + PyMem_Free(buf); +} + static int w_ref(PyObject *v, char *flag, WFILE *p) { @@ -375,69 +401,24 @@ w_complex_object(PyObject *v, char flag, WFILE *p) } else if (PyFloat_CheckExact(v)) { if (p->version > 1) { - unsigned char buf[8]; - if (_PyFloat_Pack8(PyFloat_AsDouble(v), - buf, 1) < 0) { - p->error = WFERR_UNMARSHALLABLE; - return; - } W_TYPE(TYPE_BINARY_FLOAT, p); - w_string((char*)buf, 8, p); + w_float_bin(PyFloat_AS_DOUBLE(v), p); } else { - char *buf = PyOS_double_to_string(PyFloat_AS_DOUBLE(v), - 'g', 17, 0, NULL); - if (!buf) { - p->error = WFERR_NOMEMORY; - return; - } - n = strlen(buf); W_TYPE(TYPE_FLOAT, p); - w_byte((int)n, p); - w_string(buf, n, p); - PyMem_Free(buf); + w_float_str(PyFloat_AS_DOUBLE(v), p); } } else if (PyComplex_CheckExact(v)) { if (p->version > 1) { - unsigned char buf[8]; - if (_PyFloat_Pack8(PyComplex_RealAsDouble(v), - buf, 1) < 0) { - p->error = WFERR_UNMARSHALLABLE; - return; - } W_TYPE(TYPE_BINARY_COMPLEX, p); - w_string((char*)buf, 8, p); - if (_PyFloat_Pack8(PyComplex_ImagAsDouble(v), - buf, 1) < 0) { - p->error = WFERR_UNMARSHALLABLE; - return; - } - w_string((char*)buf, 8, p); + w_float_bin(PyComplex_RealAsDouble(v), p); + w_float_bin(PyComplex_ImagAsDouble(v), p); } else { - char *buf; W_TYPE(TYPE_COMPLEX, p); - buf = PyOS_double_to_string(PyComplex_RealAsDouble(v), - 'g', 17, 0, NULL); - if (!buf) { - p->error = WFERR_NOMEMORY; - return; - } - n = strlen(buf); - w_byte((int)n, p); - w_string(buf, n, p); - PyMem_Free(buf); - buf = PyOS_double_to_string(PyComplex_ImagAsDouble(v), - 'g', 17, 0, NULL); - if (!buf) { - p->error = WFERR_NOMEMORY; - return; - } - n = strlen(buf); - w_byte((int)n, p); - w_string(buf, n, p); - PyMem_Free(buf); + w_float_str(PyComplex_RealAsDouble(v), p); + w_float_str(PyComplex_ImagAsDouble(v), p); } } else if (PyBytes_CheckExact(v)) { @@ -880,6 +861,38 @@ r_PyLong(RFILE *p) return NULL; } +static double +r_float_bin(RFILE *p) +{ + const unsigned char *buf = (const unsigned char *) r_string(8, p); + if (buf == NULL) + return -1; + return _PyFloat_Unpack8(buf, 1); +} + +/* Issue #33720: Disable inlining for reducing the C stack consumption + on PGO builds. */ +_Py_NO_INLINE static double +r_float_str(RFILE *p) +{ + int n; + char buf[256]; + const char *ptr; + n = r_byte(p); + if (n == EOF) { + PyErr_SetString(PyExc_EOFError, + "EOF read where object expected"); + return -1; + } + ptr = r_string(n, p); + if (ptr == NULL) { + return -1; + } + memcpy(buf, ptr, n); + buf[n] = '\0'; + return PyOS_string_to_double(buf, NULL, NULL); +} + /* allocate the reflist index for a new object. Return -1 on failure */ static Py_ssize_t r_ref_reserve(int flag, RFILE *p) @@ -1016,36 +1029,17 @@ r_object(RFILE *p) case TYPE_FLOAT: { - char buf[256]; - const char *ptr; - double dx; - n = r_byte(p); - if (n == EOF) { - PyErr_SetString(PyExc_EOFError, - "EOF read where object expected"); - break; - } - ptr = r_string(n, p); - if (ptr == NULL) - break; - memcpy(buf, ptr, n); - buf[n] = '\0'; - dx = PyOS_string_to_double(buf, NULL, NULL); - if (dx == -1.0 && PyErr_Occurred()) + double x = r_float_str(p); + if (x == -1.0 && PyErr_Occurred()) break; - retval = PyFloat_FromDouble(dx); + retval = PyFloat_FromDouble(x); R_REF(retval); break; } case TYPE_BINARY_FLOAT: { - const unsigned char *buf; - double x; - buf = (const unsigned char *) r_string(8, p); - if (buf == NULL) - break; - x = _PyFloat_Unpack8(buf, 1); + double x = r_float_bin(p); if (x == -1.0 && PyErr_Occurred()) break; retval = PyFloat_FromDouble(x); @@ -1055,35 +1049,11 @@ r_object(RFILE *p) case TYPE_COMPLEX: { - char buf[256]; - const char *ptr; Py_complex c; - n = r_byte(p); - if (n == EOF) { - PyErr_SetString(PyExc_EOFError, - "EOF read where object expected"); - break; - } - ptr = r_string(n, p); - if (ptr == NULL) - break; - memcpy(buf, ptr, n); - buf[n] = '\0'; - c.real = PyOS_string_to_double(buf, NULL, NULL); + c.real = r_float_str(p); if (c.real == -1.0 && PyErr_Occurred()) break; - n = r_byte(p); - if (n == EOF) { - PyErr_SetString(PyExc_EOFError, - "EOF read where object expected"); - break; - } - ptr = r_string(n, p); - if (ptr == NULL) - break; - memcpy(buf, ptr, n); - buf[n] = '\0'; - c.imag = PyOS_string_to_double(buf, NULL, NULL); + c.imag = r_float_str(p); if (c.imag == -1.0 && PyErr_Occurred()) break; retval = PyComplex_FromCComplex(c); @@ -1093,18 +1063,11 @@ r_object(RFILE *p) case TYPE_BINARY_COMPLEX: { - const unsigned char *buf; Py_complex c; - buf = (const unsigned char *) r_string(8, p); - if (buf == NULL) - break; - c.real = _PyFloat_Unpack8(buf, 1); + c.real = r_float_bin(p); if (c.real == -1.0 && PyErr_Occurred()) break; - buf = (const unsigned char *) r_string(8, p); - if (buf == NULL) - break; - c.imag = _PyFloat_Unpack8(buf, 1); + c.imag = r_float_bin(p); if (c.imag == -1.0 && PyErr_Occurred()) break; retval = PyComplex_FromCComplex(c); From f97d50c78455b0654db6abd9b6b557592a9ec763 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 4 Jul 2018 09:42:57 +0300 Subject: [PATCH 2/3] Increase the maximum marshal recursion depth on Windows debug builds. --- Lib/test/test_marshal.py | 8 +------- .../Windows/2018-06-04-09-20-53.bpo-33720.VKDXHK.rst | 3 ++- Python/marshal.c | 11 ----------- 3 files changed, 3 insertions(+), 19 deletions(-) diff --git a/Lib/test/test_marshal.py b/Lib/test/test_marshal.py index a8a43d22bc3651..e19bd26a0df151 100644 --- a/Lib/test/test_marshal.py +++ b/Lib/test/test_marshal.py @@ -222,13 +222,7 @@ def test_recursion_limit(self): # Create a deeply nested structure. head = last = [] # The max stack depth should match the value in Python/marshal.c. - # BUG: https://bugs.python.org/issue33720 - # Windows always limits the maximum depth on release and debug builds - #if os.name == 'nt' and hasattr(sys, 'gettotalrefcount'): - if os.name == 'nt': - MAX_MARSHAL_STACK_DEPTH = 1000 - else: - MAX_MARSHAL_STACK_DEPTH = 2000 + MAX_MARSHAL_STACK_DEPTH = 2000 for i in range(MAX_MARSHAL_STACK_DEPTH - 2): last.append([0]) last = last[-1] diff --git a/Misc/NEWS.d/next/Windows/2018-06-04-09-20-53.bpo-33720.VKDXHK.rst b/Misc/NEWS.d/next/Windows/2018-06-04-09-20-53.bpo-33720.VKDXHK.rst index f7e2f9d1eae4f5..cd155af0418413 100644 --- a/Misc/NEWS.d/next/Windows/2018-06-04-09-20-53.bpo-33720.VKDXHK.rst +++ b/Misc/NEWS.d/next/Windows/2018-06-04-09-20-53.bpo-33720.VKDXHK.rst @@ -1 +1,2 @@ -Reduces maximum marshal recursion depth on release builds. +Increased the maximum marshal recursion depth on Windows debug builds. +It is now the same on all platforms. diff --git a/Python/marshal.c b/Python/marshal.c index 21cdd60c7e1377..713ff2398ca821 100644 --- a/Python/marshal.c +++ b/Python/marshal.c @@ -24,19 +24,8 @@ module marshal /* High water mark to determine when the marshalled object is dangerously deep * and risks coring the interpreter. When the object stack gets this deep, * raise an exception instead of continuing. - * On Windows debug builds, reduce this value. - * - * BUG: https://bugs.python.org/issue33720 - * On Windows PGO builds, the r_object function overallocates its stack and - * can cause a stack overflow. We reduce the maximum depth for all Windows - * releases to protect against this. - * #if defined(MS_WINDOWS) && defined(_DEBUG) */ -#if defined(MS_WINDOWS) -#define MAX_MARSHAL_STACK_DEPTH 1000 -#else #define MAX_MARSHAL_STACK_DEPTH 2000 -#endif #define TYPE_NULL '0' #define TYPE_NONE 'N' From 2cb6e1916e7a72beb7500b01a140025a60570519 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Thu, 5 Jul 2018 11:01:39 +0300 Subject: [PATCH 3/3] Revert "Increase the maximum marshal recursion depth on Windows debug builds." This reverts commit f97d50c78455b0654db6abd9b6b557592a9ec763. --- Lib/test/test_marshal.py | 8 +++++++- .../Windows/2018-06-04-09-20-53.bpo-33720.VKDXHK.rst | 3 +-- Python/marshal.c | 11 +++++++++++ 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_marshal.py b/Lib/test/test_marshal.py index e19bd26a0df151..a8a43d22bc3651 100644 --- a/Lib/test/test_marshal.py +++ b/Lib/test/test_marshal.py @@ -222,7 +222,13 @@ def test_recursion_limit(self): # Create a deeply nested structure. head = last = [] # The max stack depth should match the value in Python/marshal.c. - MAX_MARSHAL_STACK_DEPTH = 2000 + # BUG: https://bugs.python.org/issue33720 + # Windows always limits the maximum depth on release and debug builds + #if os.name == 'nt' and hasattr(sys, 'gettotalrefcount'): + if os.name == 'nt': + MAX_MARSHAL_STACK_DEPTH = 1000 + else: + MAX_MARSHAL_STACK_DEPTH = 2000 for i in range(MAX_MARSHAL_STACK_DEPTH - 2): last.append([0]) last = last[-1] diff --git a/Misc/NEWS.d/next/Windows/2018-06-04-09-20-53.bpo-33720.VKDXHK.rst b/Misc/NEWS.d/next/Windows/2018-06-04-09-20-53.bpo-33720.VKDXHK.rst index cd155af0418413..f7e2f9d1eae4f5 100644 --- a/Misc/NEWS.d/next/Windows/2018-06-04-09-20-53.bpo-33720.VKDXHK.rst +++ b/Misc/NEWS.d/next/Windows/2018-06-04-09-20-53.bpo-33720.VKDXHK.rst @@ -1,2 +1 @@ -Increased the maximum marshal recursion depth on Windows debug builds. -It is now the same on all platforms. +Reduces maximum marshal recursion depth on release builds. diff --git a/Python/marshal.c b/Python/marshal.c index 713ff2398ca821..21cdd60c7e1377 100644 --- a/Python/marshal.c +++ b/Python/marshal.c @@ -24,8 +24,19 @@ module marshal /* High water mark to determine when the marshalled object is dangerously deep * and risks coring the interpreter. When the object stack gets this deep, * raise an exception instead of continuing. + * On Windows debug builds, reduce this value. + * + * BUG: https://bugs.python.org/issue33720 + * On Windows PGO builds, the r_object function overallocates its stack and + * can cause a stack overflow. We reduce the maximum depth for all Windows + * releases to protect against this. + * #if defined(MS_WINDOWS) && defined(_DEBUG) */ +#if defined(MS_WINDOWS) +#define MAX_MARSHAL_STACK_DEPTH 1000 +#else #define MAX_MARSHAL_STACK_DEPTH 2000 +#endif #define TYPE_NULL '0' #define TYPE_NONE 'N'