Optimize lexenv binding creation (#4502)

rerobika · web-flow · commit 2ade072e5355 · 2021-01-18T17:38:54.000+01:00
- Declarative environment no longer need to lookup the created binding for setting it's value - Unfold vm_decl_var and vm_set_var into vm_loop to reduce error checks - Reduce code duplication in ecma_module_connect_imports - Fix deleted binding setting in `ecma_op_set_mutable_binding` (fixes #4468) JerryScript-DCO-1.0-Signed-off-by: Robert Fancsik frobert@inf.u-szeged.hu
diff --git a/jerry-core/ecma/base/ecma-globals.h b/jerry-core/ecma/base/ecma-globals.h
@@ -2148,6 +2148,11 @@ do \
  */
 #define ECMA_OBJECT_POINTER_ERROR ((ecma_object_t *) 0x01)
 
+/**
+ * Invalid property pointer which represents abrupt completion
+ */
+#define ECMA_PROPERTY_POINTER_ERROR ((ecma_property_t *) 0x01)
+
 #if ENABLED (JERRY_BUILTIN_PROXY)
 /**
  * Description of Proxy objects.
diff --git a/jerry-core/ecma/base/ecma-module.c b/jerry-core/ecma/base/ecma-module.c
@@ -806,6 +806,8 @@ ecma_module_connect_imports (ecma_module_t *module_p)
       const bool is_namespace_import = ecma_compare_ecma_string_to_magic_id (import_names_p->imex_name_p,
                                                                              LIT_MAGIC_STRING_ASTERIX_CHAR);
 
+      ecma_value_t prop_value;
+
       if (is_namespace_import)
       {
         result = ecma_module_create_namespace_object (import_node_p->module_request_p);
@@ -814,11 +816,8 @@ ecma_module_connect_imports (ecma_module_t *module_p)
           return result;
         }
 
-        ecma_op_create_mutable_binding (local_env_p, import_names_p->local_name_p, true /* is_deletable */);
-        ecma_op_set_mutable_binding (local_env_p,
-                                     import_names_p->local_name_p,
-                                     ecma_make_object_value (import_node_p->module_request_p->namespace_object_p),
-                                     false /* is_strict */);
+        ecma_ref_object (import_node_p->module_request_p->namespace_object_p);
+        prop_value = ecma_make_object_value (import_node_p->module_request_p->namespace_object_p);
       }
       else /* !is_namespace_import */
       {
@@ -838,18 +837,8 @@ ecma_module_connect_imports (ecma_module_t *module_p)
         if (record.module_p->state == ECMA_MODULE_STATE_NATIVE)
         {
           ecma_object_t *object_p = record.module_p->namespace_object_p;
-          ecma_value_t prop_value = ecma_op_object_find_own (ecma_make_object_value (object_p),
-                                                             object_p,
-                                                             record.name_p);
+          prop_value = ecma_op_object_find_own (ecma_make_object_value (object_p), object_p, record.name_p);
           JERRY_ASSERT (ecma_is_value_found (prop_value));
-
-          ecma_op_create_mutable_binding (local_env_p, import_names_p->local_name_p, true /* is_deletable */);
-          ecma_op_set_mutable_binding (local_env_p,
-                                       import_names_p->local_name_p,
-                                       prop_value,
-                                       false /* is_strict */);
-
-          ecma_free_value (prop_value);
         }
         else
         {
@@ -861,20 +850,33 @@ ecma_module_connect_imports (ecma_module_t *module_p)
           }
 
           ecma_object_t *ref_base_lex_env_p;
-          ecma_value_t prop_value = ecma_op_get_value_lex_env_base (record.module_p->scope_p,
-                                                                    &ref_base_lex_env_p,
-                                                                    record.name_p);
+          prop_value = ecma_op_get_value_lex_env_base (record.module_p->scope_p,
+                                                       &ref_base_lex_env_p,
+                                                       record.name_p);
 
-          ecma_op_create_mutable_binding (local_env_p, import_names_p->local_name_p, true /* is_deletable */);
-          ecma_op_set_mutable_binding (local_env_p,
-                                       import_names_p->local_name_p,
-                                       prop_value,
-                                       false /* is_strict */);
-
-          ecma_free_value (prop_value);
         }
       }
 
+      ecma_property_t *prop_p = ecma_op_create_mutable_binding (local_env_p,
+                                                                import_names_p->local_name_p,
+                                                                true /* is_deletable */);
+      JERRY_ASSERT (prop_p != ECMA_PROPERTY_POINTER_ERROR);
+
+      if (prop_p != NULL)
+      {
+        JERRY_ASSERT (ecma_is_value_undefined (ECMA_PROPERTY_VALUE_PTR (prop_p)->value));
+        ECMA_PROPERTY_VALUE_PTR (prop_p)->value = prop_value;
+        ecma_deref_if_object (prop_value);
+      }
+      else
+      {
+        ecma_op_set_mutable_binding (local_env_p,
+                                     import_names_p->local_name_p,
+                                     prop_value,
+                                     false /* is_strict */);
+        ecma_free_value (prop_value);
+      }
+
       import_names_p = import_names_p->next_p;
     }
 
diff --git a/jerry-core/ecma/operations/ecma-lex-env.c b/jerry-core/ecma/operations/ecma-lex-env.c
@@ -144,10 +144,11 @@ ecma_op_has_binding (ecma_object_t *lex_env_p, /**< lexical environment */
  *
  * See also: ECMA-262 v5, 10.2.1
  *
- * @return ecma value
- *         Returned value must be freed with ecma_free_value
+ * @return ECMA_PROPERTY_POINTER_ERROR - if the operation raises error
+ *         pointer to the created property - if the binding was created into a declerative environment
+ *         NULL - otherwise
  */
-ecma_value_t
+ecma_property_t *
 ecma_op_create_mutable_binding (ecma_object_t *lex_env_p, /**< lexical environment */
                                 ecma_string_t *name_p, /**< argument N */
                                 bool is_deletable) /**< argument D */
@@ -165,10 +166,13 @@ ecma_op_create_mutable_binding (ecma_object_t *lex_env_p, /**< lexical environme
       prop_attributes = (uint8_t) (prop_attributes | ECMA_PROPERTY_FLAG_CONFIGURABLE);
     }
 
+    ecma_property_t *prop_p;
+
     ecma_create_named_data_property (lex_env_p,
                                      name_p,
                                      prop_attributes,
-                                     NULL);
+                                     &prop_p);
+    return prop_p;
   }
   else
   {
@@ -183,22 +187,22 @@ ecma_op_create_mutable_binding (ecma_object_t *lex_env_p, /**< lexical environme
 
       if (ECMA_IS_VALUE_ERROR (result))
       {
-        return result;
+        return ECMA_PROPERTY_POINTER_ERROR;
       }
 
       if (result == ECMA_VALUE_FALSE)
       {
-        return ECMA_VALUE_EMPTY;
+        return NULL;
       }
     }
     else if (!ecma_op_ordinary_object_is_extensible (binding_obj_p))
     {
-      return ECMA_VALUE_EMPTY;
+      return NULL;
     }
 #else /* !ENABLED (JERRY_BUILTIN_PROXY) || !ENABLED (JERRY_BUILTIN_REALMS) */
     if (!ecma_op_ordinary_object_is_extensible (binding_obj_p))
     {
-      return ECMA_VALUE_EMPTY;
+      return NULL;
     }
 #endif /* ENABLED (JERRY_BUILTIN_PROXY) && ENABLED (JERRY_BUILTIN_REALMS) */
 
@@ -212,15 +216,15 @@ ecma_op_create_mutable_binding (ecma_object_t *lex_env_p, /**< lexical environme
 
     if (ECMA_IS_VALUE_ERROR (completion))
     {
-      return completion;
+      return ECMA_PROPERTY_POINTER_ERROR;
     }
     else
     {
       JERRY_ASSERT (ecma_is_value_boolean (completion));
     }
   }
 
-  return ECMA_VALUE_EMPTY;
+  return NULL;
 } /* ecma_op_create_mutable_binding */
 
 /**
@@ -245,6 +249,12 @@ ecma_op_set_mutable_binding (ecma_object_t *lex_env_p, /**< lexical environment
   {
     ecma_property_t *property_p = ecma_find_named_property (lex_env_p, name_p);
 
+    if (JERRY_UNLIKELY (property_p == NULL))
+    {
+      property_p = ecma_op_create_mutable_binding (lex_env_p, name_p, is_strict);
+      JERRY_ASSERT (property_p != ECMA_PROPERTY_POINTER_ERROR);
+    }
+
     JERRY_ASSERT (property_p != NULL && ECMA_PROPERTY_IS_RAW_DATA (*property_p));
 
     if (ecma_is_property_writable (*property_p))
diff --git a/jerry-core/ecma/operations/ecma-lex-env.h b/jerry-core/ecma/operations/ecma-lex-env.h
@@ -56,7 +56,7 @@ ecma_value_t ecma_op_put_value_lex_env_base (ecma_object_t *lex_env_p, ecma_stri
 
 /* ECMA-262 v5, Table 17. Abstract methods of Environment Records */
 ecma_value_t ecma_op_has_binding (ecma_object_t *lex_env_p, ecma_string_t *name_p);
-ecma_value_t ecma_op_create_mutable_binding (ecma_object_t *lex_env_p, ecma_string_t *name_p, bool is_deletable);
+ecma_property_t *ecma_op_create_mutable_binding (ecma_object_t *lex_env_p, ecma_string_t *name_p, bool is_deletable);
 ecma_value_t ecma_op_set_mutable_binding (ecma_object_t *lex_env_p, ecma_string_t *name_p, ecma_value_t value,
                                           bool is_strict);
 ecma_value_t ecma_op_get_binding_value (ecma_object_t *lex_env_p, ecma_string_t *name_p, bool is_strict);
diff --git a/jerry-core/vm/opcodes.c b/jerry-core/vm/opcodes.c
@@ -40,78 +40,6 @@
  * @{
  */
 
-/**
- * 'Variable declaration' opcode handler.
- *
- * See also: ECMA-262 v5, 10.5 - Declaration binding instantiation (block 8).
- *
- * @return ECMA_VALUE_ERROR - if no the operation fails
- *         ECMA_VALUE_EMPTY - otherwise
- */
-extern inline ecma_value_t JERRY_ATTR_ALWAYS_INLINE
-vm_var_decl (ecma_object_t *lex_env_p, /**< target lexical environment */
-             ecma_string_t *var_name_str_p, /**< variable name */
-             bool is_configurable_bindings) /**< true if the binding can be deleted */
-{
-  ecma_value_t has_binding = ecma_op_has_binding (lex_env_p, var_name_str_p);
-
-#if ENABLED (JERRY_BUILTIN_PROXY)
-  if (ECMA_IS_VALUE_ERROR (has_binding))
-  {
-    return has_binding;
-  }
-#endif /* ENABLED (JERRY_BUILTIN_PROXY) */
-
-  if (ecma_is_value_false (has_binding))
-  {
-    ecma_value_t completion_value = ecma_op_create_mutable_binding (lex_env_p,
-                                                                    var_name_str_p,
-                                                                    is_configurable_bindings);
-
-#if ENABLED (JERRY_BUILTIN_PROXY)
-    if (ECMA_IS_VALUE_ERROR (completion_value))
-    {
-      return completion_value;
-    }
-#endif /* ENABLED (JERRY_BUILTIN_PROXY) */
-
-    JERRY_ASSERT (ecma_is_value_empty (completion_value));
-
-    /* Skipping SetMutableBinding as we have already checked that there were not
-     * any binding with specified name in current lexical environment
-     * and CreateMutableBinding sets the created binding's value to undefined */
-    JERRY_ASSERT (ecma_is_value_undefined (ecma_op_get_binding_value (lex_env_p,
-                                                                      var_name_str_p,
-                                                                      vm_is_strict_mode ())));
-  }
-
-  return ECMA_VALUE_EMPTY;
-} /* vm_var_decl */
-
-/**
- * Set var binding to a function literal value.
- *
- * @return ECMA_VALUE_ERROR - if no the operation fails
- *         ECMA_VALUE_EMPTY - otherwise
- */
-extern inline ecma_value_t JERRY_ATTR_ALWAYS_INLINE
-vm_set_var (ecma_object_t *lex_env_p, /**< target lexical environment */
-            ecma_string_t *var_name_str_p, /**< variable name */
-            bool is_strict, /**< true, if the engine is in strict mode */
-            ecma_value_t lit_value) /**< function value */
-{
-  ecma_value_t put_value_result;
-  put_value_result = ecma_op_put_value_lex_env_base (lex_env_p, var_name_str_p, is_strict, lit_value);
-
-  JERRY_ASSERT (ecma_is_value_boolean (put_value_result)
-                || ecma_is_value_empty (put_value_result)
-                || ECMA_IS_VALUE_ERROR (put_value_result));
-
-  ecma_free_value (lit_value);
-
-  return put_value_result;
-} /* vm_set_var */
-
 /**
  * 'typeof' opcode handler.
  *
diff --git a/jerry-core/vm/opcodes.h b/jerry-core/vm/opcodes.h
@@ -71,12 +71,6 @@ typedef enum
  */
 #define OPFUNC_HAS_SPREAD_ELEMENT (1 << 8)
 
-ecma_value_t
-vm_var_decl (ecma_object_t *lex_env_p, ecma_string_t *var_name_str_p, bool is_configurable_bindings);
-
-ecma_value_t
-vm_set_var (ecma_object_t *lex_env_p, ecma_string_t *var_name_str_p, bool is_strict, ecma_value_t lit_value);
-
 ecma_value_t
 opfunc_equality (ecma_value_t left_value, ecma_value_t right_value);
 
diff --git a/jerry-core/vm/vm.c b/jerry-core/vm/vm.c
@@ -1395,23 +1395,52 @@ vm_loop (vm_frame_ctx_t *frame_ctx_p) /**< frame context */
           }
 #endif /* ENABLED (JERRY_ESNEXT) && !JERRY_NDEBUG */
 
-          result = vm_var_decl (lex_env_p, name_p, (frame_ctx_p->status_flags & VM_FRAME_CTX_DIRECT_EVAL) != 0);
+          /* 'Variable declaration' */
+          result = ecma_op_has_binding (lex_env_p, name_p);
 
+#if ENABLED (JERRY_BUILTIN_PROXY)
           if (ECMA_IS_VALUE_ERROR (result))
           {
             goto error;
           }
+#endif /* ENABLED (JERRY_BUILTIN_PROXY) */
 
-          if (lit_value != ECMA_VALUE_UNDEFINED)
+          ecma_property_t *prop_p = NULL;
+
+          if (ecma_is_value_false (result))
           {
-            result = vm_set_var (lex_env_p, name_p, is_strict, lit_value);
+            bool is_configurable = (frame_ctx_p->status_flags & VM_FRAME_CTX_DIRECT_EVAL) != 0;
+            prop_p = ecma_op_create_mutable_binding (lex_env_p, name_p, is_configurable);
 
-            if (ECMA_IS_VALUE_ERROR (result))
+            if (JERRY_UNLIKELY (prop_p == ECMA_PROPERTY_POINTER_ERROR))
             {
+              result = ECMA_VALUE_ERROR;
               goto error;
             }
           }
 
+          if (lit_value != ECMA_VALUE_UNDEFINED)
+          {
+            JERRY_ASSERT (ecma_is_value_object (lit_value));
+
+            if (prop_p != NULL)
+            {
+              JERRY_ASSERT (ecma_is_value_undefined (ECMA_PROPERTY_VALUE_PTR (prop_p)->value));
+              JERRY_ASSERT (ecma_is_property_writable (*prop_p));
+              ECMA_PROPERTY_VALUE_PTR (prop_p)->value = lit_value;
+              ecma_free_object (lit_value);
+            }
+            else
+            {
+              result = ecma_op_put_value_lex_env_base (lex_env_p, name_p, is_strict, lit_value);
+              ecma_free_object (lit_value);
+
+              if (ECMA_IS_VALUE_ERROR (result))
+              {
+                goto error;
+              }
+            }
+          }
           continue;
         }
 #if ENABLED (JERRY_ESNEXT)
diff --git a/tests/jerry/es.next/regression-test-issue-4468.js b/tests/jerry/es.next/regression-test-issue-4468.js
@@ -0,0 +1,17 @@
+// 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.
+
+var str = 'for (let i=0; i<(eval("1; function x() { }; 2;")); x - i++) { x += delete x;}'
+
+assert(eval(str) === 'function(){/* ecmascript */}true');

Original file line number	Diff line number	Diff line change
`@@ -144,10 +144,11 @@ ecma_op_has_binding (ecma_object_t lex_env_p, /< lexical environment /`
`144`	`144`	`*`
`145`	`145`	`* See also: ECMA-262 v5, 10.2.1`
`146`	`146`	`*`
`147`		`- * @return ecma value`
`148`		`- * Returned value must be freed with ecma_free_value`
	`147`	`+ * @return ECMA_PROPERTY_POINTER_ERROR - if the operation raises error`
	`148`	`+ * pointer to the created property - if the binding was created into a declerative environment`
	`149`	`+ * NULL - otherwise`
`149`	`150`	`*/`
`150`		`-ecma_value_t`
	`151`	`+ecma_property_t *`
`151`	`152`	`ecma_op_create_mutable_binding (ecma_object_t lex_env_p, /< lexical environment /`
`152`	`153`	`ecma_string_t name_p, /< argument N /`
`153`	`154`	`bool is_deletable) /*< argument D /`
`@@ -165,10 +166,13 @@ ecma_op_create_mutable_binding (ecma_object_t lex_env_p, /*< lexical environme`
`165`	`166`	`prop_attributes = (uint8_t) (prop_attributes \| ECMA_PROPERTY_FLAG_CONFIGURABLE);`
`166`	`167`	`}`
`167`	`168`
	`169`	`+ ecma_property_t *prop_p;`
	`170`	`+`
`168`	`171`	`ecma_create_named_data_property (lex_env_p,`
`169`	`172`	`name_p,`
`170`	`173`	`prop_attributes,`
`171`		`- NULL);`
	`174`	`+ &prop_p);`
	`175`	`+ return prop_p;`
`172`	`176`	`}`
`173`	`177`	`else`
`174`	`178`	`{`
`@@ -183,22 +187,22 @@ ecma_op_create_mutable_binding (ecma_object_t lex_env_p, /*< lexical environme`
`183`	`187`
`184`	`188`	`if (ECMA_IS_VALUE_ERROR (result))`
`185`	`189`	`{`
`186`		`- return result;`
	`190`	`+ return ECMA_PROPERTY_POINTER_ERROR;`
`187`	`191`	`}`
`188`	`192`
`189`	`193`	`if (result == ECMA_VALUE_FALSE)`
`190`	`194`	`{`
`191`		`- return ECMA_VALUE_EMPTY;`
	`195`	`+ return NULL;`
`192`	`196`	`}`
`193`	`197`	`}`
`194`	`198`	`else if (!ecma_op_ordinary_object_is_extensible (binding_obj_p))`
`195`	`199`	`{`
`196`		`- return ECMA_VALUE_EMPTY;`
	`200`	`+ return NULL;`
`197`	`201`	`}`
`198`	`202`	`#else /* !ENABLED (JERRY_BUILTIN_PROXY) \|\| !ENABLED (JERRY_BUILTIN_REALMS) */`
`199`	`203`	`if (!ecma_op_ordinary_object_is_extensible (binding_obj_p))`
`200`	`204`	`{`
`201`		`- return ECMA_VALUE_EMPTY;`
	`205`	`+ return NULL;`
`202`	`206`	`}`
`203`	`207`	`#endif /* ENABLED (JERRY_BUILTIN_PROXY) && ENABLED (JERRY_BUILTIN_REALMS) */`
`204`	`208`
`@@ -212,15 +216,15 @@ ecma_op_create_mutable_binding (ecma_object_t lex_env_p, /*< lexical environme`
`212`	`216`
`213`	`217`	`if (ECMA_IS_VALUE_ERROR (completion))`
`214`	`218`	`{`
`215`		`- return completion;`
	`219`	`+ return ECMA_PROPERTY_POINTER_ERROR;`
`216`	`220`	`}`
`217`	`221`	`else`
`218`	`222`	`{`
`219`	`223`	`JERRY_ASSERT (ecma_is_value_boolean (completion));`
`220`	`224`	`}`
`221`	`225`	`}`
`222`	`226`
`223`		`- return ECMA_VALUE_EMPTY;`
	`227`	`+ return NULL;`
`224`	`228`	`} /* ecma_op_create_mutable_binding */`
`225`	`229`
`226`	`230`	`/**`
`@@ -245,6 +249,12 @@ ecma_op_set_mutable_binding (ecma_object_t lex_env_p, /*< lexical environment`
`245`	`249`	`{`
`246`	`250`	`ecma_property_t *property_p = ecma_find_named_property (lex_env_p, name_p);`
`247`	`251`
	`252`	`+ if (JERRY_UNLIKELY (property_p == NULL))`
	`253`	`+ {`
	`254`	`+ property_p = ecma_op_create_mutable_binding (lex_env_p, name_p, is_strict);`
	`255`	`+ JERRY_ASSERT (property_p != ECMA_PROPERTY_POINTER_ERROR);`
	`256`	`+ }`
	`257`	`+`
`248`	`258`	`JERRY_ASSERT (property_p != NULL && ECMA_PROPERTY_IS_RAW_DATA (*property_p));`
`249`	`259`
`250`	`260`	`if (ecma_is_property_writable (*property_p))`