Minor fixes to ecma_builtin_object_prototype_define_getter_setter (#4104)

Fixed tests from the exclude list:
annexB/built-ins/Object/prototype/__defineGetter__/define-non-configurable.js
annexB/built-ins/Object/prototype/__defineGetter__/define-non-extensible.js
annexB/built-ins/Object/prototype/__defineGetter__/getter-non-callable.js
annexB/built-ins/Object/prototype/__defineGetter__/this-non-obj.js
annexB/built-ins/Object/prototype/__defineSetter__/define-non-configurable.js
annexB/built-ins/Object/prototype/__defineSetter__/define-non-extensible.js
annexB/built-ins/Object/prototype/__defineSetter__/setter-non-callable.js
annexB/built-ins/Object/prototype/__defineSetter__/this-non-obj.js

JerryScript-DCO-1.0-Signed-off-by: Adam Szilagyi aszilagy@inf.u-szeged.hu

jerry-core/ecma/builtin-objects/ecma-builtin-object-prototype.c

@@ -221,15 +221,26 @@ ecma_builtin_object_prototype_object_property_is_enumerable (ecma_object_t *obj_
  *         ECMA_VALUE_UNDEFINED - otherwise
  */
 static ecma_value_t
-ecma_builtin_object_prototype_define_getter_setter (ecma_object_t *obj_p, /**< this argument */
+ecma_builtin_object_prototype_define_getter_setter (ecma_value_t this_arg, /**< this argument */
-                                                    ecma_string_t *prop_name_p, /**< property name */
+                                                    ecma_value_t prop, /**< property */
                                                     ecma_value_t accessor, /**< getter/setter function */
                                                     bool define_getter) /**< true - defineGetter method
                                                                              false - defineSetter method */
 {
+  /* 1. */
+  ecma_value_t to_obj = ecma_op_to_object (this_arg);
+
+  if (ECMA_IS_VALUE_ERROR (to_obj))
+  {
+    return to_obj;
+  }
+
+  ecma_object_t *obj_p = ecma_get_object_from_value (to_obj);
+
   /* 2. */
   if (!ecma_op_is_callable (accessor))
   {
+    ecma_deref_object (obj_p);
     return ecma_raise_type_error (ECMA_ERR_MSG ("Getter is not callable."));
   }
 
@@ -240,7 +251,8 @@ ecma_builtin_object_prototype_define_getter_setter (ecma_object_t *obj_p, /**< t
   desc.flags |= (ECMA_PROP_IS_ENUMERABLE
                  | ECMA_PROP_IS_CONFIGURABLE
                  | ECMA_PROP_IS_ENUMERABLE_DEFINED
-                 | ECMA_PROP_IS_CONFIGURABLE_DEFINED);
+                 | ECMA_PROP_IS_CONFIGURABLE_DEFINED
+                 | ECMA_PROP_IS_THROW);
 
   if (define_getter)
   {
@@ -253,9 +265,21 @@ ecma_builtin_object_prototype_define_getter_setter (ecma_object_t *obj_p, /**< t
     desc.flags |= ECMA_PROP_IS_SET_DEFINED;
   }
 
+  /* 4. */
+  ecma_string_t *prop_name_p = ecma_op_to_property_key (prop);
+
+  if (JERRY_UNLIKELY (prop_name_p == NULL))
+  {
+    ecma_deref_object (obj_p);
+    return ECMA_VALUE_ERROR;
+  }
+
   /* 5. */
   ecma_value_t define_prop = ecma_op_object_define_own_property (obj_p, prop_name_p, &desc);
 
+  ecma_deref_object (obj_p);
+  ecma_deref_ecma_string (prop_name_p);
+
   if (ECMA_IS_VALUE_ERROR (define_prop))
   {
     return define_prop;
@@ -463,6 +487,20 @@ ecma_builtin_object_prototype_dispatch_routine (uint16_t builtin_routine_id, /**
   {
     return ecma_builtin_object_prototype_lookup_getter_setter (this_arg, arguments_list_p[0], false);
   }
+  else if (builtin_routine_id == ECMA_OBJECT_PROTOTYPE_DEFINE_GETTER)
+  {
+    return ecma_builtin_object_prototype_define_getter_setter (this_arg,
+                                                               arguments_list_p[0],
+                                                               arguments_list_p[1],
+                                                               true);
+  }
+  else if (builtin_routine_id == ECMA_OBJECT_PROTOTYPE_DEFINE_SETTER)
+  {
+    return ecma_builtin_object_prototype_define_getter_setter (this_arg,
+                                                               arguments_list_p[0],
+                                                               arguments_list_p[1],
+                                                               false);
+  }
 #endif /* ENABLED (JERRY_BUILTIN_ANNEXB) */
 #endif /* ENABLED (JERRY_ESNEXT)*/
 
@@ -489,16 +527,6 @@ ecma_builtin_object_prototype_dispatch_routine (uint16_t builtin_routine_id, /**
   {
     ret_value = ecma_builtin_object_prototype_object_has_own_property (obj_p, prop_name_p);
   }
-#if ENABLED (JERRY_ESNEXT) && ENABLED (JERRY_BUILTIN_ANNEXB)
-  else if (builtin_routine_id == ECMA_OBJECT_PROTOTYPE_DEFINE_GETTER)
-  {
-    ret_value = ecma_builtin_object_prototype_define_getter_setter (obj_p, prop_name_p, arguments_list_p[1], true);
-  }
-  else if (builtin_routine_id == ECMA_OBJECT_PROTOTYPE_DEFINE_SETTER)
-  {
-    ret_value = ecma_builtin_object_prototype_define_getter_setter (obj_p, prop_name_p, arguments_list_p[1], false);
-  }
-#endif /* ENABLED (JERRY_ESNEXT)  && ENABLED (JERRY_BUILTIN_ANNEXB) */
   else
   {
     ret_value = ecma_builtin_object_prototype_object_property_is_enumerable (obj_p, prop_name_p);

tests/jerry/es.next/object-prototype-define-getter.js

@@ -49,3 +49,96 @@ var def = [];
 var p = new Proxy({}, { defineProperty: function(o, v, desc) { def.push(v); Object.defineProperty(o, v, desc); return true; }});
 Object.prototype.__defineGetter__.call(p, "foo", Object);
 assert(def + '' === "foo");
+
+var func = function() {};
+var subject = Object.defineProperty(
+  {}, 'foo', { value: 1, configurable: false }
+);
+
+try {
+  subject.__defineGetter__('foo', func);
+  assert(false);
+} catch (e) {
+  assert(e instanceof TypeError);
+}
+
+var subject = Object.preventExtensions({ existing: null });
+
+subject.__defineGetter__('existing', func);
+
+try {
+  subject.__defineGetter__('brand new', func);
+  assert(false);
+} catch (e) {
+  assert(e instanceof TypeError);
+}
+
+var __defineGetter__ = Object.prototype.__defineGetter__;
+var counter = 0;
+var key = {
+  toString: function() {
+    counter += 1;
+  }
+};
+
+try {
+  __defineGetter__.call(undefined, key, func);
+  assert(false);
+} catch (e) {
+  assert(e instanceof TypeError);
+}
+
+try {
+  __defineGetter__.call(null, key, func);
+  assert(false);
+} catch (e) {
+  assert(e instanceof TypeError);
+}
+
+assert(counter === 0);
+
+var subject = {};
+var symbol = Symbol('');
+var counter = 0;
+var key = {
+  toString: function() {
+    counter += 1;
+  }
+};
+
+try {
+  subject.__defineGetter__(key, '');
+  assert(false);
+} catch (e) {
+  assert(e instanceof TypeError);
+}
+
+try {
+  subject.__defineGetter__(key, 23);
+  assert(false);
+} catch (e) {
+  assert(e instanceof TypeError);
+}
+
+try {
+  subject.__defineGetter__(key, true);
+  assert(false);
+} catch (e) {
+  assert(e instanceof TypeError);
+}
+
+try {
+  subject.__defineGetter__(key, symbol);
+  assert(false);
+} catch (e) {
+  assert(e instanceof TypeError);
+}
+
+try {
+  subject.__defineGetter__(key, {});
+  assert(false);
+} catch (e) {
+  assert(e instanceof TypeError);
+}
+
+assert(counter === 0);

tests/jerry/es.next/object-prototype-define-setter.js

@@ -51,3 +51,96 @@ var def = [];
 var p = new Proxy({}, { defineProperty: function(o, v, desc) { def.push(v); Object.defineProperty(o, v, desc); return true; }});
 Object.prototype.__defineSetter__.call(p, "foo", Object);
 assert(def + '' === "foo");
+
+var func = function() {};
+var subject = Object.defineProperty(
+  {}, 'attr', { value: 1, configurable: false }
+);
+
+try {
+  subject.__defineSetter__('attr', func);
+  assert(false);
+} catch (e) {
+  assert(e instanceof TypeError);
+}
+
+var subject = Object.preventExtensions({ existing: null });
+
+subject.__defineSetter__('existing', func);
+
+try {
+  subject.__defineSetter__('brand new', func);
+  assert(false);
+} catch (e) {
+  assert(e instanceof TypeError);
+}
+
+var __defineSetter__ = Object.prototype.__defineSetter__;
+var counter = 0;
+var key = {
+  toString: function() {
+    counter += 1;
+  }
+};
+
+try {
+  __defineSetter__.call(undefined, key, func);
+  assert(false);
+} catch (e) {
+  assert(e instanceof TypeError);
+}
+
+try {
+  __defineSetter__.call(null, key, func);
+  assert(false);
+} catch (e) {
+  assert(e instanceof TypeError);
+}
+
+assert(counter === 0);
+
+var subject = {};
+var symbol = Symbol('');
+var counter = 0;
+var key = {
+  toString: function() {
+    counter += 1;
+  }
+};
+
+try {
+  subject.__defineSetter__(key, '');
+  assert(false);
+} catch (e) {
+  assert(e instanceof TypeError);
+}
+
+try {
+  subject.__defineSetter__(key, 23);
+  assert(false);
+} catch (e) {
+  assert(e instanceof TypeError);
+}
+
+try {
+  subject.__defineSetter__(key, true);
+  assert(false);
+} catch (e) {
+  assert(e instanceof TypeError);
+}
+
+try {
+  subject.__defineSetter__(key, symbol);
+  assert(false);
+} catch (e) {
+  assert(e instanceof TypeError);
+}
+
+try {
+  subject.__defineSetter__(key, {});
+  assert(false);
+} catch (e) {
+  assert(e instanceof TypeError);
+}
+
+assert(counter === 0);

tests/test262-esnext-excludelist.xml

@@ -10,14 +10,6 @@
   <test id="annexB/built-ins/Function/createdynfn-html-open-comment-body.js"><reason></reason></test>
   <test id="annexB/built-ins/Function/createdynfn-html-open-comment-params.js"><reason></reason></test>
   <test id="annexB/built-ins/Function/createdynfn-no-line-terminator-html-close-comment-body.js"><reason></reason></test>
-  <test id="annexB/built-ins/Object/prototype/__defineGetter__/define-non-configurable.js"><reason></reason></test>
-  <test id="annexB/built-ins/Object/prototype/__defineGetter__/define-non-extensible.js"><reason></reason></test>
-  <test id="annexB/built-ins/Object/prototype/__defineGetter__/getter-non-callable.js"><reason></reason></test>
-  <test id="annexB/built-ins/Object/prototype/__defineGetter__/this-non-obj.js"><reason></reason></test>
-  <test id="annexB/built-ins/Object/prototype/__defineSetter__/define-non-configurable.js"><reason></reason></test>
-  <test id="annexB/built-ins/Object/prototype/__defineSetter__/define-non-extensible.js"><reason></reason></test>
-  <test id="annexB/built-ins/Object/prototype/__defineSetter__/setter-non-callable.js"><reason></reason></test>
-  <test id="annexB/built-ins/Object/prototype/__defineSetter__/this-non-obj.js"><reason></reason></test>
   <test id="annexB/built-ins/RegExp/match-indices/indices-groups-object.js"><reason></reason></test>
   <test id="annexB/built-ins/RegExp/named-groups/groups-object.js"><reason></reason></test>
   <test id="annexB/built-ins/RegExp/named-groups/non-unicode-malformed-lookbehind.js"><reason></reason></test>