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

@@ -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

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

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)
+    if (JERRY_UNLIKELY (check_type == PARSER_CHECK_FUNCTION_CONTEXT))
-        && (SCANNER_STREAM_TYPE_IS_ARG (type) || SCANNER_STREAM_TYPE_IS_ARG_FUNC (type)))
     {
-      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),

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 ");

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 */

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)