From 132a2008c9004d3f6d74234852a6f753ece05fb8 Mon Sep 17 00:00:00 2001 From: Zoltan Herczeg Date: Mon, 4 May 2020 14:55:46 +0200 Subject: [PATCH] Fix function argument handling issues caused by the parser. (#3705) 1) Nested function declarations should not overwrite arguments. 2) Functions should be created in the correct scope. JerryScript-DCO-1.0-Signed-off-by: Zoltan Herczeg zherczeg.u-szeged@partner.samsung.com --- jerry-core/parser/js/js-parser-internal.h | 5 +- jerry-core/parser/js/js-parser-statm.c | 4 +- jerry-core/parser/js/js-scanner-util.c | 66 +++++++++++++++++----- jerry-core/parser/js/js-scanner.c | 12 ++-- jerry-core/parser/js/js-scanner.h | 4 +- tests/jerry/es2015/function-scope.js | 67 +++++++++++++++++++++++ 6 files changed, 135 insertions(+), 23 deletions(-) create mode 100644 tests/jerry/es2015/function-scope.js diff --git a/jerry-core/parser/js/js-parser-internal.h b/jerry-core/parser/js/js-parser-internal.h index d86875e0c..8b6256f1c 100644 --- a/jerry-core/parser/js/js-parser-internal.h +++ b/jerry-core/parser/js/js-parser-internal.h @@ -392,9 +392,10 @@ typedef struct #define PARSER_SCOPE_STACK_REGISTER_MASK 0x3fff /** - * The scope stack item represents a lexical declaration (let/const) + * Function statements with the name specified + * in map_from should not be copied to global scope. */ -#define PARSER_SCOPE_STACK_IS_LEXICAL 0x4000 +#define PARSER_SCOPE_STACK_NO_FUNCTION_COPY 0x4000 /** * The scope stack item represents a const declaration diff --git a/jerry-core/parser/js/js-parser-statm.c b/jerry-core/parser/js/js-parser-statm.c index b41f0fd43..6f0a39731 100644 --- a/jerry-core/parser/js/js-parser-statm.c +++ b/jerry-core/parser/js/js-parser-statm.c @@ -742,7 +742,7 @@ parser_parse_function_statement (parser_context_t *context_p) /**< context */ while (stack_p < scope_stack_p) { if (literal_index == stack_p->map_from - && (stack_p->map_to & PARSER_SCOPE_STACK_IS_LEXICAL)) + && (stack_p->map_to & PARSER_SCOPE_STACK_NO_FUNCTION_COPY)) { copy_value = false; break; @@ -758,7 +758,7 @@ parser_parse_function_statement (parser_context_t *context_p) /**< context */ { if (literal_index == stack_p->map_from) { - JERRY_ASSERT (!(stack_p->map_to & PARSER_SCOPE_STACK_IS_LEXICAL)); + JERRY_ASSERT (!(stack_p->map_to & PARSER_SCOPE_STACK_NO_FUNCTION_COPY)); uint16_t map_to = scanner_decode_map_to (stack_p); uint16_t opcode = ((map_to >= PARSER_REGISTER_START) ? CBC_ASSIGN_LITERAL_SET_IDENT diff --git a/jerry-core/parser/js/js-scanner-util.c b/jerry-core/parser/js/js-scanner-util.c index 9b2e96c14..314b6876d 100644 --- a/jerry-core/parser/js/js-scanner-util.c +++ b/jerry-core/parser/js/js-scanner-util.c @@ -142,17 +142,19 @@ scanner_get_stream_size (scanner_info_t *info_p, /**< scanner info block */ case SCANNER_STREAM_TYPE_LET: case SCANNER_STREAM_TYPE_CONST: case SCANNER_STREAM_TYPE_LOCAL: - case SCANNER_STREAM_TYPE_DESTRUCTURED_ARG: #endif /* ENABLED (JERRY_ES2015) */ +#if ENABLED (JERRY_ES2015_MODULE_SYSTEM) + case SCANNER_STREAM_TYPE_IMPORT: +#endif /* ENABLED (JERRY_ES2015_MODULE_SYSTEM) */ case SCANNER_STREAM_TYPE_ARG: +#if ENABLED (JERRY_ES2015) + case SCANNER_STREAM_TYPE_DESTRUCTURED_ARG: +#endif /* ENABLED (JERRY_ES2015_MODULE_SYSTEM) */ case SCANNER_STREAM_TYPE_ARG_FUNC: #if ENABLED (JERRY_ES2015) case SCANNER_STREAM_TYPE_DESTRUCTURED_ARG_FUNC: #endif /* ENABLED (JERRY_ES2015) */ case SCANNER_STREAM_TYPE_FUNC: -#if ENABLED (JERRY_ES2015_MODULE_SYSTEM) - case SCANNER_STREAM_TYPE_IMPORT: -#endif /* ENABLED (JERRY_ES2015_MODULE_SYSTEM) */ { break; } @@ -1651,8 +1653,8 @@ scanner_is_context_needed (parser_context_t *context_p, /**< context */ || type == SCANNER_STREAM_TYPE_LET || type == SCANNER_STREAM_TYPE_CONST || type == SCANNER_STREAM_TYPE_LOCAL - || type == SCANNER_STREAM_TYPE_DESTRUCTURED_ARG || type == SCANNER_STREAM_TYPE_ARG + || type == SCANNER_STREAM_TYPE_DESTRUCTURED_ARG || type == SCANNER_STREAM_TYPE_ARG_FUNC || type == SCANNER_STREAM_TYPE_DESTRUCTURED_ARG_FUNC || type == SCANNER_STREAM_TYPE_FUNC); @@ -1688,10 +1690,26 @@ scanner_is_context_needed (parser_context_t *context_p, /**< context */ continue; } - if (JERRY_UNLIKELY (check_type == PARSER_CHECK_FUNCTION_CONTEXT) - && (SCANNER_STREAM_TYPE_IS_ARG (type) || SCANNER_STREAM_TYPE_IS_ARG_FUNC (type))) + if (JERRY_UNLIKELY (check_type == PARSER_CHECK_FUNCTION_CONTEXT)) { - continue; + if (SCANNER_STREAM_TYPE_IS_ARG (type)) + { + continue; + } + + if (SCANNER_STREAM_TYPE_IS_ARG_FUNC (type)) + { + /* The return value is true, if the variable is stored in the lexical environment + * or all registers have already been used for function arguments. This can be + * inprecise in the latter case, but that is a very rare corner case. A more + * sophisticated check would require to decode the literal. */ + if ((data & SCANNER_STREAM_NO_REG) + || scope_stack_reg_top >= PARSER_MAXIMUM_NUMBER_OF_REGISTERS) + { + return true; + } + continue; + } } #endif /* ENABLED (JERRY_ES2015) */ @@ -1997,6 +2015,10 @@ scanner_create_variables (parser_context_t *context_p, /**< context */ if (SCANNER_STREAM_TYPE_IS_ARG_FUNC (type) && (option_flags & SCANNER_CREATE_VARS_IS_FUNCTION_BODY)) { JERRY_ASSERT (scope_stack_p >= context_p->scope_stack_p + 2); + JERRY_ASSERT (context_p->status_flags & PARSER_IS_FUNCTION); +#if ENABLED (JERRY_ES2015) + JERRY_ASSERT (!(context_p->status_flags & PARSER_FUNCTION_IS_PARSING_ARGS)); +#endif /* ENABLED (JERRY_ES2015) */ parser_scope_stack_t *function_map_p = scope_stack_p - 2; uint16_t literal_index = context_p->lit_object.index; @@ -2010,8 +2032,18 @@ scanner_create_variables (parser_context_t *context_p, /**< context */ JERRY_ASSERT (function_map_p[1].map_from == PARSER_SCOPE_STACK_FUNC); + cbc_opcode_t opcode = CBC_SET_VAR_FUNC; + +#if ENABLED (JERRY_ES2015) + if (JERRY_UNLIKELY (context_p->status_flags & PARSER_LEXICAL_BLOCK_NEEDED) + && (function_map_p[0].map_to & PARSER_SCOPE_STACK_REGISTER_MASK) == 0) + { + opcode = CBC_INIT_LOCAL; + } +#endif /* ENABLED (JERRY_ES2015) */ + parser_emit_cbc_literal_value (context_p, - CBC_SET_VAR_FUNC, + opcode, function_map_p[1].map_to, scanner_decode_map_to (function_map_p)); continue; @@ -2063,8 +2095,12 @@ scanner_create_variables (parser_context_t *context_p, /**< context */ /* FALLTHRU */ } case SCANNER_STREAM_TYPE_LET: + case SCANNER_STREAM_TYPE_ARG: + case SCANNER_STREAM_TYPE_DESTRUCTURED_ARG: + case SCANNER_STREAM_TYPE_ARG_FUNC: + case SCANNER_STREAM_TYPE_DESTRUCTURED_ARG_FUNC: { - scope_stack_p->map_to |= PARSER_SCOPE_STACK_IS_LEXICAL; + scope_stack_p->map_to |= PARSER_SCOPE_STACK_NO_FUNCTION_COPY; break; } } @@ -2097,13 +2133,13 @@ scanner_create_variables (parser_context_t *context_p, /**< context */ /* FALLTHRU */ } case SCANNER_STREAM_TYPE_LET: + case SCANNER_STREAM_TYPE_DESTRUCTURED_ARG: + case SCANNER_STREAM_TYPE_DESTRUCTURED_ARG_FUNC: { - scope_stack_p->map_to |= PARSER_SCOPE_STACK_IS_LEXICAL; + scope_stack_p->map_to |= PARSER_SCOPE_STACK_NO_FUNCTION_COPY; /* FALLTHRU */ } case SCANNER_STREAM_TYPE_LOCAL: - case SCANNER_STREAM_TYPE_DESTRUCTURED_ARG: - case SCANNER_STREAM_TYPE_DESTRUCTURED_ARG_FUNC: #endif /* ENABLED (JERRY_ES2015) */ case SCANNER_STREAM_TYPE_VAR: { @@ -2146,6 +2182,10 @@ scanner_create_variables (parser_context_t *context_p, /**< context */ context_p->scope_stack_top = (uint16_t) (scope_stack_p - context_p->scope_stack_p); #endif /* ENABLED (JERRY_PARSER_DUMP_BYTE_CODE) */ +#if ENABLED (JERRY_ES2015) + scope_stack_p->map_to |= PARSER_SCOPE_STACK_NO_FUNCTION_COPY; +#endif /* ENABLED (JERRY_ES2015) */ + parser_emit_cbc_literal_value (context_p, CBC_INIT_LOCAL, (uint16_t) (PARSER_REGISTER_START + scope_stack_reg_top), diff --git a/jerry-core/parser/js/js-scanner.c b/jerry-core/parser/js/js-scanner.c index a1e4ca0cd..9b5761387 100644 --- a/jerry-core/parser/js/js-scanner.c +++ b/jerry-core/parser/js/js-scanner.c @@ -3224,11 +3224,6 @@ scan_completed: JERRY_DEBUG_MSG (" LOCAL "); break; } - case SCANNER_STREAM_TYPE_DESTRUCTURED_ARG: - { - JERRY_DEBUG_MSG (" DESTRUCTURED_ARG "); - break; - } #endif /* ENABLED (JERRY_ES2015) */ #if ENABLED (JERRY_ES2015_MODULE_SYSTEM) case SCANNER_STREAM_TYPE_IMPORT: @@ -3242,6 +3237,13 @@ scan_completed: JERRY_DEBUG_MSG (" ARG "); break; } +#if ENABLED (JERRY_ES2015) + case SCANNER_STREAM_TYPE_DESTRUCTURED_ARG: + { + JERRY_DEBUG_MSG (" DESTRUCTURED_ARG "); + break; + } +#endif /* ENABLED (JERRY_ES2015) */ case SCANNER_STREAM_TYPE_ARG_FUNC: { JERRY_DEBUG_MSG (" ARG_FUNC "); diff --git a/jerry-core/parser/js/js-scanner.h b/jerry-core/parser/js/js-scanner.h index 613e208eb..c8fb58dca 100644 --- a/jerry-core/parser/js/js-scanner.h +++ b/jerry-core/parser/js/js-scanner.h @@ -148,12 +148,14 @@ typedef enum SCANNER_STREAM_TYPE_LET, /**< let declaration */ SCANNER_STREAM_TYPE_CONST, /**< const declaration */ SCANNER_STREAM_TYPE_LOCAL, /**< local declaration (e.g. catch block) */ - SCANNER_STREAM_TYPE_DESTRUCTURED_ARG, /**< destructuring argument declaration */ #endif /* ENABLED (JERRY_ES2015) */ #if ENABLED (JERRY_ES2015_MODULE_SYSTEM) SCANNER_STREAM_TYPE_IMPORT, /**< module import */ #endif /* ENABLED (JERRY_ES2015_MODULE_SYSTEM) */ SCANNER_STREAM_TYPE_ARG, /**< argument declaration */ +#if ENABLED (JERRY_ES2015) + SCANNER_STREAM_TYPE_DESTRUCTURED_ARG, /**< destructuring argument declaration */ +#endif /* ENABLED (JERRY_ES2015) */ /* Function types should be at the end. See the SCANNER_STREAM_TYPE_IS_FUNCTION macro. */ SCANNER_STREAM_TYPE_ARG_FUNC, /**< argument declaration which * is later initialized with a function */ diff --git a/tests/jerry/es2015/function-scope.js b/tests/jerry/es2015/function-scope.js new file mode 100644 index 000000000..ad37ebd36 --- /dev/null +++ b/tests/jerry/es2015/function-scope.js @@ -0,0 +1,67 @@ +// Copyright JS Foundation and other contributors, http://js.foundation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +function f1(a) +{ + assert(a === 2) + { + assert(a() === 1) + function a() { return 1 } + } + assert(a === 2) +} +f1(2) + +function f2([a]) +{ + assert(a === 4) + { + assert(a() === 3) + function a() { return 3 } + } + assert(a === 4) +} +f2([4]) + +function f3(a) +{ + assert(a() === 5) + { + assert(a() === 6) + function a() { return 6 } + } + assert(a() === 5) + + function a() { return 5 } +} +f3(7) + +function f4(a) +{ + assert(a === 8) + { + eval("function a() { return 9 }") + assert(a() === 9) + } + assert(a() === 9) +} +f4(8) + +function f5(a, b = function() { return a }) { + function a() { return 9 } + + assert(a() === 9) + assert(b() === 10) +} +f5(10)