From bd556b69960cafc97353e736f825eca5c00b0c29 Mon Sep 17 00:00:00 2001 From: Damien George Date: Fri, 1 Apr 2022 00:00:58 +1100 Subject: [PATCH] py: Fix compiling and decoding of *args at large arg positions. There were two issues with the existing code: 1. "1 << i" is computed as a 32-bit number so would overflow when executed on 64-bit machines (when mp_uint_t is 64-bit). This meant that *args beyond 32 positions would not be handled correctly. 2. star_args must fit as a positive small int so that it is encoded correctly in the emitted code. MP_SMALL_INT_BITS is too big because it overflows a small int by 1 bit. MP_SMALL_INT_BITS - 1 does not work because it produces a signed small int which is then sign extended when extracted (even by mp_obj_get_int_truncated), and this sign extension means that any position arg after *args is also treated as a star-arg. So the maximum bit position is MP_SMALL_INT_BITS - 2. This means that MP_OBJ_SMALL_INT_VALUE() can be used instead of mp_obj_get_int_truncated() to get the value of star_args. These issues are fixed by this commit, and a test added. Signed-off-by: Damien George --- py/compile.c | 6 ++--- py/runtime.c | 6 ++--- tests/stress/fun_call_limit.py | 36 ++++++++++++++++++++++++++++++ tests/stress/fun_call_limit.py.exp | 2 ++ 4 files changed, 44 insertions(+), 6 deletions(-) create mode 100644 tests/stress/fun_call_limit.py create mode 100644 tests/stress/fun_call_limit.py.exp diff --git a/py/compile.c b/py/compile.c index fc11062705..e4ead7f1ac 100644 --- a/py/compile.c +++ b/py/compile.c @@ -2408,9 +2408,9 @@ STATIC void compile_trailer_paren_helper(compiler_t *comp, mp_parse_node_t pn_ar return; } #if MICROPY_DYNAMIC_COMPILER - if (i > mp_dynamic_compiler.small_int_bits) + if (i >= (size_t)mp_dynamic_compiler.small_int_bits - 1) #else - if (i > MP_SMALL_INT_BITS) + if (i >= MP_SMALL_INT_BITS - 1) #endif { // If there are not enough bits in a small int to fit the flag, then we consider @@ -2419,7 +2419,7 @@ STATIC void compile_trailer_paren_helper(compiler_t *comp, mp_parse_node_t pn_ar return; } star_flags |= MP_EMIT_STAR_FLAG_SINGLE; - star_args |= 1 << i; + star_args |= (mp_uint_t)1 << i; compile_node(comp, pns_arg->nodes[0]); n_positional++; } else if (MP_PARSE_NODE_STRUCT_KIND(pns_arg) == PN_arglist_dbl_star) { diff --git a/py/runtime.c b/py/runtime.c index 594e63082b..ad066acb16 100644 --- a/py/runtime.c +++ b/py/runtime.c @@ -702,7 +702,7 @@ void mp_call_prepare_args_n_kw_var(bool have_self, size_t n_args_n_kw, const mp_ } size_t n_args = n_args_n_kw & 0xff; size_t n_kw = (n_args_n_kw >> 8) & 0xff; - mp_uint_t star_args = mp_obj_get_int_truncated(args[n_args + 2 * n_kw]); + mp_uint_t star_args = MP_OBJ_SMALL_INT_VALUE(args[n_args + 2 * n_kw]); DEBUG_OP_printf("call method var (fun=%p, self=%p, n_args=%u, n_kw=%u, args=%p, map=%u)\n", fun, self, n_args, n_kw, args, star_args); @@ -720,7 +720,7 @@ void mp_call_prepare_args_n_kw_var(bool have_self, size_t n_args_n_kw, const mp_ if (star_args != 0) { for (size_t i = 0; i < n_args; i++) { - if (star_args & (1 << i)) { + if ((star_args >> i) & 1) { mp_obj_t len = mp_obj_len_maybe(args[i]); if (len != MP_OBJ_NULL) { // -1 accounts for 1 of n_args occupied by this arg @@ -773,7 +773,7 @@ void mp_call_prepare_args_n_kw_var(bool have_self, size_t n_args_n_kw, const mp_ for (size_t i = 0; i < n_args; i++) { mp_obj_t arg = args[i]; - if (star_args & (1 << i)) { + if ((star_args >> i) & 1) { // star arg if (mp_obj_is_type(arg, &mp_type_tuple) || mp_obj_is_type(arg, &mp_type_list)) { // optimise the case of a tuple and list diff --git a/tests/stress/fun_call_limit.py b/tests/stress/fun_call_limit.py new file mode 100644 index 0000000000..b802aadd55 --- /dev/null +++ b/tests/stress/fun_call_limit.py @@ -0,0 +1,36 @@ +# Test the limit of the number of arguments to a function call. +# This currently tests the case of *args after many positional args. + + +def f(*args): + return len(args) + + +def test(n): + pos_args = ",".join(str(i) for i in range(n)) + s = "f({}, *(100, 101), 102, 103)".format(pos_args) + try: + return eval(s) + except SyntaxError: + return "SyntaxError" + + +# If the port has at least 32-bits then this test should pass. +print(test(29)) + +# This test should fail on all ports (overflows a small int). +print(test(70)) + +# Check that there is a correct transition to the limit of too many args before *args. +reached_limit = False +for i in range(30, 70): + result = test(i) + if reached_limit: + if result != "SyntaxError": + print("FAIL") + else: + if result == "SyntaxError": + reached_limit = True + else: + if result != i + 4: + print("FAIL") diff --git a/tests/stress/fun_call_limit.py.exp b/tests/stress/fun_call_limit.py.exp new file mode 100644 index 0000000000..53d2b28043 --- /dev/null +++ b/tests/stress/fun_call_limit.py.exp @@ -0,0 +1,2 @@ +33 +SyntaxError