Fix deleting native pointers (#4570)

JerryScript-DCO-1.0-Signed-off-by: Zoltan Herczeg zherczeg.u-szeged@partner.samsung.com

jerry-core/ecma/base/ecma-gc.c

@@ -1084,12 +1084,19 @@ ecma_gc_free_native_pointer (ecma_property_t *property_p) /**< property */
   JERRY_ASSERT (property_p != NULL);
 
   ecma_property_value_t *value_p = ECMA_PROPERTY_VALUE_PTR (property_p);
+
+  if (value_p->value == JMEM_CP_NULL)
+  {
+    return;
+  }
+
   ecma_native_pointer_t *native_pointer_p;
 
   native_pointer_p = ECMA_GET_INTERNAL_VALUE_POINTER (ecma_native_pointer_t,
                                                       value_p->value);
+  JERRY_ASSERT (native_pointer_p != NULL);
 
-  while (native_pointer_p != NULL)
+  do
   {
     if (native_pointer_p->info_p != NULL)
     {
@@ -1107,6 +1114,7 @@ ecma_gc_free_native_pointer (ecma_property_t *property_p) /**< property */
 
     native_pointer_p = next_p;
   }
+  while (native_pointer_p != NULL);
 } /* ecma_gc_free_native_pointer */
 
 /**

jerry-core/ecma/base/ecma-helpers-external-pointers.c

@@ -63,32 +63,39 @@ ecma_create_native_pointer_property (ecma_object_t *obj_p, /**< object to create
   {
     ecma_property_value_t *value_p = ECMA_PROPERTY_VALUE_PTR (property_p);
 
-    ecma_native_pointer_t *iter_p = ECMA_GET_INTERNAL_VALUE_POINTER (ecma_native_pointer_t, value_p->value);
+    if (value_p->value == JMEM_CP_NULL)
-
-    /* There should be at least 1 native pointer in the chain */
-    JERRY_ASSERT (iter_p != NULL);
-
-    while (true)
     {
-      if (iter_p->info_p == info_p)
+      native_pointer_p = jmem_heap_alloc_block (sizeof (ecma_native_pointer_t));
-      {
+      ECMA_SET_INTERNAL_VALUE_POINTER (value_p->value, native_pointer_p);
-        /* The native info already exists -> update the corresponding data */
-        iter_p->data_p = native_p;
-        return false;
-      }
-
-      if (iter_p->next_p == NULL)
-      {
-        /* The native info does not exist -> append a new element to the chain */
-        break;
-      }
-
-      iter_p = iter_p->next_p;
     }
+    else
+    {
+      ecma_native_pointer_t *iter_p = ECMA_GET_INTERNAL_VALUE_POINTER (ecma_native_pointer_t, value_p->value);
 
-    native_pointer_p = jmem_heap_alloc_block (sizeof (ecma_native_pointer_t));
+      /* There should be at least 1 native pointer in the chain */
+      JERRY_ASSERT (iter_p != NULL);
 
-    iter_p->next_p = native_pointer_p;
+      while (true)
+      {
+        if (iter_p->info_p == info_p)
+        {
+          /* The native info already exists -> update the corresponding data */
+          iter_p->data_p = native_p;
+          return false;
+        }
+
+        if (iter_p->next_p == NULL)
+        {
+          /* The native info does not exist -> append a new element to the chain */
+          break;
+        }
+
+        iter_p = iter_p->next_p;
+      }
+
+      native_pointer_p = jmem_heap_alloc_block (sizeof (ecma_native_pointer_t));
+      iter_p->next_p = native_pointer_p;
+    }
   }
 
   native_pointer_p->data_p = native_p;
@@ -128,12 +135,16 @@ ecma_get_native_pointer_value (ecma_object_t *obj_p, /**< object to get property
 
   ecma_property_value_t *value_p = ECMA_PROPERTY_VALUE_PTR (property_p);
 
+  if (value_p->value == JMEM_CP_NULL)
+  {
+    return NULL;
+  }
+
   ecma_native_pointer_t *native_pointer_p = ECMA_GET_INTERNAL_VALUE_POINTER (ecma_native_pointer_t,
                                                                              value_p->value);
-
   JERRY_ASSERT (native_pointer_p != NULL);
 
-  while (native_pointer_p != NULL)
+  do
   {
     if (native_pointer_p->info_p == info_p)
     {
@@ -142,6 +153,7 @@ ecma_get_native_pointer_value (ecma_object_t *obj_p, /**< object to get property
 
     native_pointer_p = native_pointer_p->next_p;
   }
+  while (native_pointer_p != NULL);
 
   return NULL;
 } /* ecma_get_native_pointer_value */
@@ -176,49 +188,40 @@ ecma_delete_native_pointer_property (ecma_object_t *obj_p, /**< object to delete
 
   ecma_property_value_t *value_p = ECMA_PROPERTY_VALUE_PTR (property_p);
 
+  if (value_p->value == JMEM_CP_NULL)
+  {
+    return false;
+  }
+
   ecma_native_pointer_t *native_pointer_p = ECMA_GET_INTERNAL_VALUE_POINTER (ecma_native_pointer_t,
                                                                              value_p->value);
-  ecma_native_pointer_t *prev_p = NULL;
-
   JERRY_ASSERT (native_pointer_p != NULL);
 
-  while (native_pointer_p != NULL)
+  ecma_native_pointer_t *prev_p = NULL;
+
+  do
   {
     if (native_pointer_p->info_p == info_p)
     {
       if (prev_p == NULL)
       {
-        if (native_pointer_p->next_p == NULL)
+        /* The first element is deleted from the chain: change the property value. */
-        {
+        ECMA_SET_INTERNAL_VALUE_ANY_POINTER (value_p->value, native_pointer_p->next_p);
-          /* Only one native pointer property exists, so the property can be deleted as well. */
-          ecma_op_general_object_delete (obj_p, name_p, false);
-
-          jmem_heap_free_block (native_pointer_p, sizeof (ecma_native_pointer_t));
-          return true;
-        }
-        else
-        {
-          /* There are at least two native pointers and the first one should be deleted.
-             In this case the second element's data is copied to the head of the chain, and freed as well. */
-          ecma_native_pointer_t *next_p = native_pointer_p->next_p;
-          memcpy (native_pointer_p, next_p, sizeof (ecma_native_pointer_t));
-          jmem_heap_free_block (next_p, sizeof (ecma_native_pointer_t));
-          return true;
-        }
       }
       else
       {
-        /* There are at least two native pointers and not the first element should be deleted.
+        /* A non-first element is deleted from the chain: update the previous pointer. */
-           In this case the current element's next element reference is copied to the previous element. */
         prev_p->next_p = native_pointer_p->next_p;
-        jmem_heap_free_block (native_pointer_p, sizeof (ecma_native_pointer_t));
-        return true;
       }
+
+      jmem_heap_free_block (native_pointer_p, sizeof (ecma_native_pointer_t));
+      return true;
     }
 
     prev_p = native_pointer_p;
     native_pointer_p = native_pointer_p->next_p;
   }
+  while (native_pointer_p != NULL);
 
   return false;
 } /* ecma_delete_native_pointer_property */

tests/unit-core/CMakeLists.txt

@@ -59,6 +59,7 @@ set(SOURCE_UNIT_TEST_MAIN_MODULES
   test-mem-stats.c
   test-native-callback-nested.c
   test-native-instanceof.c
+  test-native-pointer.c
   test-newtarget.c
   test-number-converter.c
   test-number-to-int32.c

tests/unit-core/test-native-pointer.c

@@ -0,0 +1,118 @@
+ /* 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.
+ */
+
+#include "jerryscript.h"
+
+#include "test-common.h"
+
+static int global_int = 4;
+static void *global_p = (void *) &global_int;
+static int global_counter = 0;
+
+static void
+native_free_callback (void *native_p) /**< native pointer */
+{
+  (void) native_p;
+  global_counter++;
+} /* native_free_callback */
+
+static const jerry_object_native_info_t native_info_1 =
+{
+  .free_cb = native_free_callback,
+};
+
+static const jerry_object_native_info_t native_info_2 =
+{
+  .free_cb = NULL,
+};
+
+static void
+check_native_info (jerry_value_t object_value, /**< object value */
+                   const jerry_object_native_info_t *native_info_p, /**< native info */
+                   void *expected_pointer_p) /**< expected pointer */
+{
+  void *native_pointer_p;
+  TEST_ASSERT (jerry_get_object_native_pointer (object_value, &native_pointer_p, native_info_p));
+  TEST_ASSERT (native_pointer_p == expected_pointer_p);
+} /* check_native_info */
+
+int
+main (void)
+{
+  TEST_INIT ();
+  jerry_init (JERRY_INIT_EMPTY);
+
+  jerry_value_t object_value = jerry_create_object ();
+
+  jerry_set_object_native_pointer (object_value, global_p, &native_info_1);
+  jerry_set_object_native_pointer (object_value, NULL, &native_info_2);
+
+  check_native_info (object_value, &native_info_1, global_p);
+  check_native_info (object_value, &native_info_2, NULL);
+
+  jerry_release_value (object_value);
+
+  jerry_gc (JERRY_GC_PRESSURE_HIGH);
+  TEST_ASSERT (global_counter == 1);
+  global_counter = 0;
+
+  object_value = jerry_create_object ();
+
+  jerry_set_object_native_pointer (object_value, global_p, &native_info_1);
+  jerry_set_object_native_pointer (object_value, NULL, &native_info_2);
+
+  TEST_ASSERT (jerry_delete_object_native_pointer (object_value, &native_info_1));
+
+  TEST_ASSERT (!jerry_get_object_native_pointer (object_value, NULL, &native_info_1));
+  check_native_info (object_value, &native_info_2, NULL);
+
+  TEST_ASSERT (!jerry_delete_object_native_pointer (object_value, &native_info_1));
+
+  TEST_ASSERT (!jerry_get_object_native_pointer (object_value, NULL, &native_info_1));
+  check_native_info (object_value, &native_info_2, NULL);
+
+  TEST_ASSERT (jerry_delete_object_native_pointer (object_value, &native_info_2));
+
+  TEST_ASSERT (!jerry_get_object_native_pointer (object_value, NULL, &native_info_1));
+  TEST_ASSERT (!jerry_get_object_native_pointer (object_value, NULL, &native_info_2));
+
+  jerry_set_object_native_pointer (object_value, NULL, &native_info_1);
+
+  check_native_info (object_value, &native_info_1, NULL);
+  TEST_ASSERT (!jerry_get_object_native_pointer (object_value, NULL, &native_info_2));
+
+  jerry_set_object_native_pointer (object_value, global_p, &native_info_2);
+
+  check_native_info (object_value, &native_info_1, NULL);
+  check_native_info (object_value, &native_info_2, global_p);
+
+  jerry_set_object_native_pointer (object_value, global_p, &native_info_1);
+
+  check_native_info (object_value, &native_info_1, global_p);
+  check_native_info (object_value, &native_info_2, global_p);
+
+  TEST_ASSERT (jerry_delete_object_native_pointer (object_value, &native_info_1));
+  TEST_ASSERT (jerry_delete_object_native_pointer (object_value, &native_info_2));
+
+  TEST_ASSERT (!jerry_get_object_native_pointer (object_value, NULL, &native_info_1));
+  TEST_ASSERT (!jerry_get_object_native_pointer (object_value, NULL, &native_info_2));
+
+  jerry_release_value (object_value);
+
+  jerry_cleanup ();
+
+  TEST_ASSERT (global_counter == 0);
+  return 0;
+} /* main */