Accessors should be kept alive during their invocation (#5167)

This patch fixes #4900.

The implementation is based on PR #4943, only resolved the conflicts.

Co-authored-by: Robert Fancsik robert.fancsik@h-lab.eu
JerryScript-DCO-1.0-Signed-off-by: Gergo Csizi gergocs@inf.u-szeged.hu

jerry-core/ecma/operations/ecma-function-object.c

@@ -1359,6 +1359,55 @@ ecma_op_function_call_bound (ecma_object_t *func_obj_p, /**< Function object */
   return ret_value;
 } /* ecma_op_function_call_bound */
 
+/**
+ * Invoke accessor getter function
+ *
+ * @return ecma value
+ *         Returned value must be freed with ecma_free_value
+ */
+extern inline ecma_value_t JERRY_ATTR_ALWAYS_INLINE
+ecma_op_invoke_getter (ecma_getter_setter_pointers_t *get_set_pair_p, /**< accessor pair */
+                       ecma_value_t this_value) /**< 'this' argument's value */
+{
+  if (get_set_pair_p->getter_cp == JMEM_CP_NULL)
+  {
+    return ECMA_VALUE_UNDEFINED;
+  }
+
+  ecma_object_t *getter_p = ECMA_GET_NON_NULL_POINTER (ecma_object_t, get_set_pair_p->getter_cp);
+  ecma_ref_object (getter_p);
+
+  ecma_value_t result = ecma_op_function_call (getter_p, this_value, NULL, 0);
+  ecma_deref_object (getter_p);
+
+  return result;
+} /* ecma_op_invoke_getter */
+
+/**
+ * Invoke accessor setter function
+ *
+ * @return ecma value
+ *         Returned value must be freed with ecma_free_value
+ */
+extern inline ecma_value_t JERRY_ATTR_ALWAYS_INLINE
+ecma_op_invoke_setter (ecma_getter_setter_pointers_t *get_set_pair_p, /**< accessor pair */
+                       ecma_value_t this_value, /**< 'this' argument's value */
+                       ecma_value_t value) /**< value to set */
+{
+  if (get_set_pair_p->setter_cp == JMEM_CP_NULL)
+  {
+    return ecma_raise_type_error (ECMA_ERR_PRIVATE_FIELD_WAS_DEFINED_WITHOUT_A_SETTER);
+  }
+
+  ecma_object_t *setter_p = ECMA_GET_NON_NULL_POINTER (ecma_object_t, get_set_pair_p->setter_cp);
+  ecma_ref_object (setter_p);
+
+  ecma_value_t result = ecma_op_function_call (setter_p, this_value, &value, 1);
+  ecma_deref_object (setter_p);
+
+  return result;
+} /* ecma_op_invoke_setter */
+
 /**
  * General [[Call]] implementation
  *

jerry-core/ecma/operations/ecma-function-object.h

@@ -79,6 +79,11 @@ ecma_object_t *ecma_op_get_prototype_from_constructor (ecma_object_t *ctor_obj_p
 
 ecma_value_t ecma_op_function_has_instance (ecma_object_t *func_obj_p, ecma_value_t value);
 
+ecma_value_t ecma_op_invoke_getter (ecma_getter_setter_pointers_t *get_set_pair_p, ecma_value_t this_value);
+
+ecma_value_t
+ecma_op_invoke_setter (ecma_getter_setter_pointers_t *get_set_pair_p, ecma_value_t this_value, ecma_value_t value);
+
 ecma_value_t ecma_op_function_validated_call (ecma_value_t callee,
                                               ecma_value_t this_arg_value,
                                               const ecma_value_t *arguments_list_p,

jerry-core/ecma/operations/ecma-objects.c

@@ -697,16 +697,7 @@ ecma_op_object_find_own (ecma_value_t base_value, /**< base value */
     return ecma_fast_copy_value (prop_value_p->value);
   }
 
-  ecma_getter_setter_pointers_t *get_set_pair_p = ecma_get_named_accessor_property (prop_value_p);
+  return ecma_op_invoke_getter (ecma_get_named_accessor_property (prop_value_p), base_value);
-
-  if (get_set_pair_p->getter_cp == JMEM_CP_NULL)
-  {
-    return ECMA_VALUE_UNDEFINED;
-  }
-
-  ecma_object_t *getter_p = ECMA_GET_NON_NULL_POINTER (ecma_object_t, get_set_pair_p->getter_cp);
-
-  return ecma_op_function_call (getter_p, base_value, NULL, 0);
 } /* ecma_op_object_find_own */
 
 /**
@@ -1474,6 +1465,7 @@ ecma_op_object_put_with_receiver (ecma_object_t *object_p, /**< the object */
     }
   }
 
+  ecma_getter_setter_pointers_t *get_set_pair_p = NULL;
   jmem_cpointer_t setter_cp = JMEM_CP_NULL;
 
   if (property_p != NULL)
@@ -1497,7 +1489,6 @@ ecma_op_object_put_with_receiver (ecma_object_t *object_p, /**< the object */
     }
     else
     {
-      ecma_getter_setter_pointers_t *get_set_pair_p;
       get_set_pair_p = ecma_get_named_accessor_property (ECMA_PROPERTY_VALUE_PTR (property_p));
       setter_cp = get_set_pair_p->setter_cp;
     }
@@ -1538,7 +1529,8 @@ ecma_op_object_put_with_receiver (ecma_object_t *object_p, /**< the object */
 
         if (!(inherited_property & ECMA_PROPERTY_FLAG_DATA))
         {
-          setter_cp = ecma_get_named_accessor_property (property_ref.value_p)->setter_cp;
+          get_set_pair_p = ecma_get_named_accessor_property (property_ref.value_p);
+          setter_cp = get_set_pair_p->setter_cp;
           create_new_property = false;
           break;
         }
@@ -1611,8 +1603,7 @@ ecma_op_object_put_with_receiver (ecma_object_t *object_p, /**< the object */
     return ecma_raise_readonly_assignment (property_name_p, is_throw);
   }
 
-  ecma_value_t ret_value =
+  ecma_value_t ret_value = ecma_op_invoke_setter (get_set_pair_p, receiver, value);
-    ecma_op_function_call (ECMA_GET_NON_NULL_POINTER (ecma_object_t, setter_cp), receiver, &value, 1);
 
   if (!ECMA_IS_VALUE_ERROR (ret_value))
   {

jerry-core/ecma/operations/ecma-reference.c

@@ -354,17 +354,8 @@ ecma_op_resolve_reference_value (ecma_object_t *lex_env_p, /**< starting lexical
               return ecma_fast_copy_value (prop_value_p->value);
             }
 
-            ecma_getter_setter_pointers_t *get_set_pair_p = ecma_get_named_accessor_property (prop_value_p);
+            return ecma_op_invoke_getter (ecma_get_named_accessor_property (prop_value_p),
-
+                                          ecma_make_object_value (binding_obj_p));
-            if (get_set_pair_p->getter_cp == JMEM_CP_NULL)
-            {
-              return ECMA_VALUE_UNDEFINED;
-            }
-
-            ecma_object_t *getter_p = ECMA_GET_NON_NULL_POINTER (ecma_object_t, get_set_pair_p->getter_cp);
-
-            ecma_value_t base_value = ecma_make_object_value (binding_obj_p);
-            return ecma_op_function_call (getter_p, base_value, NULL, 0);
           }
 #endif /* JERRY_LCACHE */
         }

jerry-core/vm/opcodes.c

@@ -1430,17 +1430,7 @@ opfunc_private_set (ecma_value_t base, /**< this object */
   else
   {
     ecma_getter_setter_pointers_t *get_set_pair_p = ecma_get_named_accessor_property (ECMA_PROPERTY_VALUE_PTR (prop_p));
-
+    result = ecma_op_invoke_setter (get_set_pair_p, base, value);
-    if (get_set_pair_p->setter_cp == JMEM_CP_NULL)
-    {
-      result = ecma_raise_type_error (ECMA_ERR_PRIVATE_FIELD_WAS_DEFINED_WITHOUT_A_SETTER);
-    }
-    else
-    {
-      ecma_object_t *setter_p = ECMA_GET_NON_NULL_POINTER (ecma_object_t, get_set_pair_p->setter_cp);
-
-      result = ecma_op_function_call (setter_p, base, &value, 1);
-    }
   }
 
   ecma_deref_object (obj_p);
@@ -1493,8 +1483,7 @@ opfunc_private_get (ecma_value_t base, /**< this object */
     }
     else
     {
-      ecma_object_t *getter_p = ECMA_GET_NON_NULL_POINTER (ecma_object_t, get_set_pair_p->getter_cp);
+      result = ecma_op_invoke_getter (get_set_pair_p, base);
-      result = ecma_op_function_call (getter_p, base, NULL, 0);
     }
   }
 

tests/jerry/es.next/regression-test-issue-4900.js

@@ -0,0 +1,41 @@
+// 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 assertArrayEqual(actual, expected) {
+  assert(actual.length === expected.length);
+
+  for (var i = 0; i < actual.length; i++) {
+    assert(actual[i] === expected[i]);
+  }
+}
+
+var i = 0;
+var a = [];
+var JSEtest = [];
+
+JSEtest.__defineGetter__(0, function NaN() {
+  if (i++ > 2) {
+    return;
+  }
+
+  JSEtest.shift();
+  gc();
+  a.push(0);
+  a.concat(JSEtest);
+});
+
+JSEtest[0];
+
+assertArrayEqual(a, [0, 0, 0]);
+assertArrayEqual(JSEtest, []);