From 67c7d8765e80931d19f926af1dc766a47a8fcd5c Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 14 Apr 2021 13:10:42 +0100 Subject: [PATCH 1/6] Modify compiler to reduce stack consumption for large expressions. --- Python/compile.c | 167 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 121 insertions(+), 46 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 85bc87dbf68206..27b31a49d225a5 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -43,6 +43,16 @@ #define COMP_SETCOMP 2 #define COMP_DICTCOMP 3 +/* A soft limit for stack use, to avoid excessive + * memory use for large constants, etc. + * + * The value 30 is plucked out of thin air. + * Code that this impacts is rare (dynamically), + * so exact value is unimportant. + * An experimentally determined + */ +#define STACK_USE_GUIDELINE 30 + #define IS_TOP_LEVEL_AWAIT(c) ( \ (c->c_flags->cf_flags & PyCF_ALLOW_TOP_LEVEL_AWAIT) \ && (c->u->u_ste->ste_type == ModuleBlock)) @@ -1403,7 +1413,7 @@ compiler_addop_name(struct compiler *c, int opcode, PyObject *dict, */ static int -compiler_addop_i(struct compiler *c, int opcode, Py_ssize_t oparg) +compiler_addop_i_line(struct compiler *c, int opcode, Py_ssize_t oparg, int lineno) { struct instr *i; int off; @@ -1424,10 +1434,22 @@ compiler_addop_i(struct compiler *c, int opcode, Py_ssize_t oparg) i = &c->u->u_curblock->b_instr[off]; i->i_opcode = opcode; i->i_oparg = Py_SAFE_DOWNCAST(oparg, Py_ssize_t, int); - i->i_lineno = c->u->u_lineno; + i->i_lineno = lineno; return 1; } +static int +compiler_addop_i(struct compiler *c, int opcode, Py_ssize_t oparg) +{ + return compiler_addop_i_line(c, opcode, oparg, c->u->u_lineno); +} + +static int +compiler_addop_i_noline(struct compiler *c, int opcode, Py_ssize_t oparg) +{ + return compiler_addop_i_line(c, opcode, oparg, -1); +} + static int add_jump_to_block(basicblock *b, int opcode, int lineno, basicblock *target) { assert(HAS_ARG(opcode)); @@ -1527,6 +1549,11 @@ compiler_addop_j_noline(struct compiler *c, int opcode, basicblock *b) return 0; \ } +#define ADDOP_I_NOLINE(C, OP, O) { \ + if (!compiler_addop_i_noline((C), (OP), (O))) \ + return 0; \ +} + #define ADDOP_JUMP(C, OP, O) { \ if (!compiler_addop_j((C), (OP), (O))) \ return 0; \ @@ -3741,31 +3768,7 @@ starunpack_helper(struct compiler *c, asdl_expr_seq *elts, int pushed, seen_star = 1; } } - if (seen_star) { - seen_star = 0; - for (i = 0; i < n; i++) { - expr_ty elt = asdl_seq_GET(elts, i); - if (elt->kind == Starred_kind) { - if (seen_star == 0) { - ADDOP_I(c, build, i+pushed); - seen_star = 1; - } - VISIT(c, expr, elt->v.Starred.value); - ADDOP_I(c, extend, 1); - } - else { - VISIT(c, expr, elt); - if (seen_star) { - ADDOP_I(c, add, 1); - } - } - } - assert(seen_star); - if (tuple) { - ADDOP(c, LIST_TO_TUPLE); - } - } - else { + if (!seen_star && n+pushed < STACK_USE_GUIDELINE) { for (i = 0; i < n; i++) { expr_ty elt = asdl_seq_GET(elts, i); VISIT(c, expr, elt); @@ -3775,6 +3778,35 @@ starunpack_helper(struct compiler *c, asdl_expr_seq *elts, int pushed, } else { ADDOP_I(c, build, n+pushed); } + return 1; + } + if (n+pushed >= STACK_USE_GUIDELINE) { + ADDOP_I(c, build, pushed); + seen_star = 1; + } + else { + seen_star = 0; + } + for (i = 0; i < n; i++) { + expr_ty elt = asdl_seq_GET(elts, i); + if (elt->kind == Starred_kind) { + if (seen_star == 0) { + ADDOP_I(c, build, i+pushed); + seen_star = 1; + } + VISIT(c, expr, elt->v.Starred.value); + ADDOP_I(c, extend, 1); + } + else { + VISIT(c, expr, elt); + if (seen_star) { + ADDOP_I(c, add, 1); + } + } + } + assert(seen_star); + if (tuple) { + ADDOP(c, LIST_TO_TUPLE); } return 1; } @@ -3874,7 +3906,8 @@ compiler_subdict(struct compiler *c, expr_ty e, Py_ssize_t begin, Py_ssize_t end { Py_ssize_t i, n = end - begin; PyObject *keys, *key; - if (n > 1 && are_all_items_const(e->v.Dict.keys, begin, end)) { + int big = n > STACK_USE_GUIDELINE/2; + if (n > 1 && !big && are_all_items_const(e->v.Dict.keys, begin, end)) { for (i = begin; i < end; i++) { VISIT(c, expr, (expr_ty)asdl_seq_GET(e->v.Dict.values, i)); } @@ -3889,12 +3922,19 @@ compiler_subdict(struct compiler *c, expr_ty e, Py_ssize_t begin, Py_ssize_t end } ADDOP_LOAD_CONST_NEW(c, keys); ADDOP_I(c, BUILD_CONST_KEY_MAP, n); + return 1; } - else { - for (i = begin; i < end; i++) { - VISIT(c, expr, (expr_ty)asdl_seq_GET(e->v.Dict.keys, i)); - VISIT(c, expr, (expr_ty)asdl_seq_GET(e->v.Dict.values, i)); + if (big) { + ADDOP_I(c, BUILD_MAP, 0); + } + for (i = begin; i < end; i++) { + VISIT(c, expr, (expr_ty)asdl_seq_GET(e->v.Dict.keys, i)); + VISIT(c, expr, (expr_ty)asdl_seq_GET(e->v.Dict.values, i)); + if (big) { + ADDOP_I(c, MAP_ADD, 1); } + } + if (!big) { ADDOP_I(c, BUILD_MAP, n); } return 1; @@ -3930,7 +3970,7 @@ compiler_dict(struct compiler *c, expr_ty e) ADDOP_I(c, DICT_UPDATE, 1); } else { - if (elements == 0xFFFF) { + if (elements > STACK_USE_GUIDELINE/2) { if (!compiler_subdict(c, e, i - elements, i + 1)) { return 0; } @@ -4126,11 +4166,15 @@ maybe_optimize_method_call(struct compiler *c, expr_ty e) /* Check that the call node is an attribute access, and that the call doesn't have keyword parameters. */ if (meth->kind != Attribute_kind || meth->v.Attribute.ctx != Load || - asdl_seq_LEN(e->v.Call.keywords)) + asdl_seq_LEN(e->v.Call.keywords)) { return -1; - - /* Check that there are no *varargs types of arguments. */ + } + /* Check that there aren't too many arguments */ argsl = asdl_seq_LEN(args); + if (argsl >= STACK_USE_GUIDELINE) { + return -1; + } + /* Check that there are no *varargs types of arguments. */ for (i = 0; i < argsl; i++) { expr_ty elt = asdl_seq_GET(args, i); if (elt->kind == Starred_kind) { @@ -4192,9 +4236,29 @@ compiler_call(struct compiler *c, expr_ty e) static int compiler_joined_str(struct compiler *c, expr_ty e) { - VISIT_SEQ(c, expr, e->v.JoinedStr.values); - if (asdl_seq_LEN(e->v.JoinedStr.values) != 1) - ADDOP_I(c, BUILD_STRING, asdl_seq_LEN(e->v.JoinedStr.values)); + + Py_ssize_t value_count = asdl_seq_LEN(e->v.JoinedStr.values); + if (value_count > STACK_USE_GUIDELINE) { + ADDOP_LOAD_CONST_NEW(c, _PyUnicode_FromASCII("", 0)); + PyObject *join = _PyUnicode_FromASCII("join", 4); + if (join == NULL) { + return 0; + } + ADDOP_NAME(c, LOAD_METHOD, join, names); + Py_DECREF(join); + ADDOP_I(c, BUILD_LIST, 0); + for (Py_ssize_t i = 0; i < asdl_seq_LEN(e->v.JoinedStr.values); i++) { + VISIT(c, expr, asdl_seq_GET(e->v.JoinedStr.values, i)); + ADDOP_I(c, LIST_APPEND, 1); + } + ADDOP_I(c, CALL_METHOD, 1); + } + else { + VISIT_SEQ(c, expr, e->v.JoinedStr.values); + if (asdl_seq_LEN(e->v.JoinedStr.values) != 1) { + ADDOP_I(c, BUILD_STRING, asdl_seq_LEN(e->v.JoinedStr.values)); + } + } return 1; } @@ -4251,7 +4315,8 @@ compiler_subkwargs(struct compiler *c, asdl_keyword_seq *keywords, Py_ssize_t be keyword_ty kw; PyObject *keys, *key; assert(n > 0); - if (n > 1) { + int big = n > STACK_USE_GUIDELINE/2; + if (n > 1 && !big) { for (i = begin; i < end; i++) { kw = asdl_seq_GET(keywords, i); VISIT(c, expr, kw->value); @@ -4267,14 +4332,20 @@ compiler_subkwargs(struct compiler *c, asdl_keyword_seq *keywords, Py_ssize_t be } ADDOP_LOAD_CONST_NEW(c, keys); ADDOP_I(c, BUILD_CONST_KEY_MAP, n); + return 1; } - else { - /* a for loop only executes once */ - for (i = begin; i < end; i++) { - kw = asdl_seq_GET(keywords, i); - ADDOP_LOAD_CONST(c, kw->arg); - VISIT(c, expr, kw->value); + if (big) { + ADDOP_I_NOLINE(c, BUILD_MAP, 0); + } + for (i = begin; i < end; i++) { + kw = asdl_seq_GET(keywords, i); + ADDOP_LOAD_CONST(c, kw->arg); + VISIT(c, expr, kw->value); + if (big) { + ADDOP_I_NOLINE(c, MAP_ADD, 1); } + } + if (!big) { ADDOP_I(c, BUILD_MAP, n); } return 1; @@ -4296,6 +4367,9 @@ compiler_call_helper(struct compiler *c, nelts = asdl_seq_LEN(args); nkwelts = asdl_seq_LEN(keywords); + if (nelts + nkwelts >= STACK_USE_GUIDELINE) { + goto ex_call; + } for (i = 0; i < nelts; i++) { expr_ty elt = asdl_seq_GET(args, i); if (elt->kind == Starred_kind) { @@ -6203,6 +6277,7 @@ stackdepth(struct compiler *c) maxdepth = new_depth; } assert(depth >= 0); /* invalid code or bug in stackdepth() */ + //assert(depth < 50); /* Needs fixing in compiler */ if (is_jump(instr)) { effect = stack_effect(instr->i_opcode, instr->i_oparg, 1); assert(effect != PY_INVALID_STACK_EFFECT); From a5afaad0785ad3e42dc3b3c21b69f7299f9d3988 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 14 Apr 2021 13:51:49 +0100 Subject: [PATCH 2/6] Add more tests for stack usage. --- Lib/test/test_compile.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/Lib/test/test_compile.py b/Lib/test/test_compile.py index c0de3bef5b6c9e..fd1ef612fd6131 100644 --- a/Lib/test/test_compile.py +++ b/Lib/test/test_compile.py @@ -961,6 +961,32 @@ def test_if_else(self): def test_binop(self): self.check_stack_size("x + " * self.N + "x") + def test_list(self): + self.check_stack_size("[" + "x, " * self.N + "x]") + + def test_tuple(self): + self.check_stack_size("(" + "x, " * self.N + "x)") + + def test_set(self): + self.check_stack_size("{" + "x, " * self.N + "x}") + + def test_dict(self): + self.check_stack_size("{" + "x:x, " * self.N + "x:x}") + + def test_func_args(self): + self.check_stack_size("f(" + "x, " * self.N + ")") + + def test_func_kwargs(self): + kwargs = (f'a{i}=x' for i in range(self.N)) + self.check_stack_size("f(" + ", ".join(kwargs) + ")") + + def test_func_args(self): + self.check_stack_size("o.m(" + "x, " * self.N + ")") + + def test_meth_kwargs(self): + kwargs = (f'a{i}=x' for i in range(self.N)) + self.check_stack_size("o.m(" + ", ".join(kwargs) + ")") + def test_func_and(self): code = "def f(x):\n" code += " x and x\n" * self.N From 8c3cc5cf64158f866c61c75291eacc767ad0adec Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 14 Apr 2021 13:53:19 +0100 Subject: [PATCH 3/6] Add NEWS item. --- .../Core and Builtins/2021-04-14-13-53-08.bpo-43846.2jO97c.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2021-04-14-13-53-08.bpo-43846.2jO97c.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2021-04-14-13-53-08.bpo-43846.2jO97c.rst b/Misc/NEWS.d/next/Core and Builtins/2021-04-14-13-53-08.bpo-43846.2jO97c.rst new file mode 100644 index 00000000000000..220690cd813743 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2021-04-14-13-53-08.bpo-43846.2jO97c.rst @@ -0,0 +1 @@ +Data stack usage is much reduced for large literal and call expressions. From 87b7b6d635d03cc762ef6033807171c8d7c515ea Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 14 Apr 2021 14:14:05 +0100 Subject: [PATCH 4/6] Remove assertion and raise SystemError for truly excessive stack use. --- Python/compile.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/Python/compile.c b/Python/compile.c index 27b31a49d225a5..d4dd3f4ad7eb1b 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -53,6 +53,17 @@ */ #define STACK_USE_GUIDELINE 30 +/* If we exceed this limit, it should + * be considered a compiler bug. + * Currently it should be impossible + * to exceed STACK_USE_GUIDELINE * 100, + * as 100 is the maximum parse depth. + * For performance reasons we will + * want to reduce this to a + * few hundred in the future. + */ +#define MAX_ALLOWED_STACK_USE (STACK_USE_GUIDELINE * 100) + #define IS_TOP_LEVEL_AWAIT(c) ( \ (c->c_flags->cf_flags & PyCF_ALLOW_TOP_LEVEL_AWAIT) \ && (c->u->u_ste->ste_type == ModuleBlock)) @@ -6277,7 +6288,6 @@ stackdepth(struct compiler *c) maxdepth = new_depth; } assert(depth >= 0); /* invalid code or bug in stackdepth() */ - //assert(depth < 50); /* Needs fixing in compiler */ if (is_jump(instr)) { effect = stack_effect(instr->i_opcode, instr->i_oparg, 1); assert(effect != PY_INVALID_STACK_EFFECT); @@ -6678,6 +6688,13 @@ makecode(struct compiler *c, struct assembler *a, PyObject *consts) Py_DECREF(consts); goto error; } + if (maxdepth > MAX_ALLOWED_STACK_USE) { + PyErr_Format(PyExc_SystemError, + "excessive stack use: stack is %d deep", + maxdepth); + Py_DECREF(consts); + goto error; + } co = PyCode_NewWithPosOnlyArgs(posonlyargcount+posorkeywordargcount, posonlyargcount, kwonlyargcount, nlocals_int, maxdepth, flags, a->a_bytecode, consts, names, From a523a6edd2f2a21cea1e4737bbdce2341c520aaa Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 14 Apr 2021 14:19:43 +0100 Subject: [PATCH 5/6] Clarify comments for STACK_USE_GUIDELINE and MAX_ALLOWED_STACK_USE. --- Python/compile.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index d4dd3f4ad7eb1b..d807032ee3dbe6 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -47,9 +47,8 @@ * memory use for large constants, etc. * * The value 30 is plucked out of thin air. - * Code that this impacts is rare (dynamically), - * so exact value is unimportant. - * An experimentally determined + * Code that could use more stack than this is + * rare, so the exact value is unimportant. */ #define STACK_USE_GUIDELINE 30 @@ -61,6 +60,10 @@ * For performance reasons we will * want to reduce this to a * few hundred in the future. + * + * NOTE: Whatever MAX_ALLOWED_STACK_USE is + * set to, it should never restrict what Python + * we can write, just how we compile it. */ #define MAX_ALLOWED_STACK_USE (STACK_USE_GUIDELINE * 100) From 95feeb81a2cc8736a9f192a61c7fc95d03cd5ef3 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Thu, 15 Apr 2021 13:45:21 +0100 Subject: [PATCH 6/6] Clean up starunpack_helper. Make sure all tests against STACK_USE_GUIDELINE use the same comparison. --- Python/compile.c | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index d807032ee3dbe6..11d910b5b7d58a 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -3748,14 +3748,13 @@ starunpack_helper(struct compiler *c, asdl_expr_seq *elts, int pushed, int build, int add, int extend, int tuple) { Py_ssize_t n = asdl_seq_LEN(elts); - Py_ssize_t i, seen_star = 0; if (n > 2 && are_all_items_const(elts, 0, n)) { PyObject *folded = PyTuple_New(n); if (folded == NULL) { return 0; } PyObject *val; - for (i = 0; i < n; i++) { + for (Py_ssize_t i = 0; i < n; i++) { val = ((expr_ty)asdl_seq_GET(elts, i))->v.Constant.value; Py_INCREF(val); PyTuple_SET_ITEM(folded, i, val); @@ -3776,14 +3775,16 @@ starunpack_helper(struct compiler *c, asdl_expr_seq *elts, int pushed, return 1; } - for (i = 0; i < n; i++) { + int big = n+pushed > STACK_USE_GUIDELINE; + int seen_star = 0; + for (Py_ssize_t i = 0; i < n; i++) { expr_ty elt = asdl_seq_GET(elts, i); if (elt->kind == Starred_kind) { seen_star = 1; } } - if (!seen_star && n+pushed < STACK_USE_GUIDELINE) { - for (i = 0; i < n; i++) { + if (!seen_star && !big) { + for (Py_ssize_t i = 0; i < n; i++) { expr_ty elt = asdl_seq_GET(elts, i); VISIT(c, expr, elt); } @@ -3794,31 +3795,29 @@ starunpack_helper(struct compiler *c, asdl_expr_seq *elts, int pushed, } return 1; } - if (n+pushed >= STACK_USE_GUIDELINE) { + int sequence_built = 0; + if (big) { ADDOP_I(c, build, pushed); - seen_star = 1; - } - else { - seen_star = 0; + sequence_built = 1; } - for (i = 0; i < n; i++) { + for (Py_ssize_t i = 0; i < n; i++) { expr_ty elt = asdl_seq_GET(elts, i); if (elt->kind == Starred_kind) { - if (seen_star == 0) { + if (sequence_built == 0) { ADDOP_I(c, build, i+pushed); - seen_star = 1; + sequence_built = 1; } VISIT(c, expr, elt->v.Starred.value); ADDOP_I(c, extend, 1); } else { VISIT(c, expr, elt); - if (seen_star) { + if (sequence_built) { ADDOP_I(c, add, 1); } } } - assert(seen_star); + assert(sequence_built); if (tuple) { ADDOP(c, LIST_TO_TUPLE); } @@ -3920,7 +3919,7 @@ compiler_subdict(struct compiler *c, expr_ty e, Py_ssize_t begin, Py_ssize_t end { Py_ssize_t i, n = end - begin; PyObject *keys, *key; - int big = n > STACK_USE_GUIDELINE/2; + int big = n*2 > STACK_USE_GUIDELINE; if (n > 1 && !big && are_all_items_const(e->v.Dict.keys, begin, end)) { for (i = begin; i < end; i++) { VISIT(c, expr, (expr_ty)asdl_seq_GET(e->v.Dict.values, i)); @@ -3984,7 +3983,7 @@ compiler_dict(struct compiler *c, expr_ty e) ADDOP_I(c, DICT_UPDATE, 1); } else { - if (elements > STACK_USE_GUIDELINE/2) { + if (elements*2 > STACK_USE_GUIDELINE) { if (!compiler_subdict(c, e, i - elements, i + 1)) { return 0; } @@ -4329,7 +4328,7 @@ compiler_subkwargs(struct compiler *c, asdl_keyword_seq *keywords, Py_ssize_t be keyword_ty kw; PyObject *keys, *key; assert(n > 0); - int big = n > STACK_USE_GUIDELINE/2; + int big = n*2 > STACK_USE_GUIDELINE; if (n > 1 && !big) { for (i = begin; i < end; i++) { kw = asdl_seq_GET(keywords, i); @@ -4381,7 +4380,7 @@ compiler_call_helper(struct compiler *c, nelts = asdl_seq_LEN(args); nkwelts = asdl_seq_LEN(keywords); - if (nelts + nkwelts >= STACK_USE_GUIDELINE) { + if (nelts + nkwelts*2 > STACK_USE_GUIDELINE) { goto ex_call; } for (i = 0; i < nelts; i++) {