Fix an issue with ecma collections in the Promise implementation

Fixes jerryscript-project#2486 Fixes jerryscript-project#2506 Co-authored-by: Robert Fancsik [email protected] JerryScript-DCO-1.0-Signed-off-by: Daniel Balla [email protected]
DanielBallaSZTE · Sep 11, 2018 · e9c7812 · e9c7812
1 parent 5472aff
commit e9c7812
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 20 deletions.
diff --git a/jerry-core/ecma/base/ecma-gc.c b/jerry-core/ecma/base/ecma-gc.c
@@ -231,16 +231,30 @@ ecma_gc_mark_promise_object (ecma_extended_object_t *ext_object_p) /**< extended
 
   while (ecma_value_p != NULL)
   {
-    ecma_gc_set_object_visited (ecma_get_object_from_value (*ecma_value_p));
-    ecma_value_p = ecma_collection_iterator_next (ecma_value_p);
+    if (ecma_is_value_object (*ecma_value_p))
+    {
+      ecma_gc_set_object_visited (ecma_get_object_from_value (*ecma_value_p));
+      ecma_value_p = ecma_collection_iterator_next (ecma_value_p);
+    }
+    else
+    {
+      break;
+    }
   }
 
   ecma_value_p = ecma_collection_iterator_init (((ecma_promise_object_t *) ext_object_p)->reject_reactions);
 
   while (ecma_value_p != NULL)
   {
-    ecma_gc_set_object_visited (ecma_get_object_from_value (*ecma_value_p));
-    ecma_value_p = ecma_collection_iterator_next (ecma_value_p);
+    if (ecma_is_value_object (*ecma_value_p))
+    {
+      ecma_gc_set_object_visited (ecma_get_object_from_value (*ecma_value_p));
+      ecma_value_p = ecma_collection_iterator_next (ecma_value_p);
+    }
+    else
+    {
+      break;
+    }
   }
 } /* ecma_gc_mark_promise_object */
 

diff --git a/jerry-core/ecma/operations/ecma-promise-object.c b/jerry-core/ecma/operations/ecma-promise-object.c
@@ -176,16 +176,12 @@ ecma_reject_promise (ecma_value_t promise, /**< promise */
   ecma_promise_set_result (obj_p, ecma_copy_value_if_not_object (reason));
   ecma_promise_object_t *promise_p = (ecma_promise_object_t *) obj_p;
 
-  /* GC can be triggered by ecma_new_values_collection so freeing the collection
-     first and creating a new one might cause a heap after use event. */
-  ecma_collection_header_t *reject_reactions = promise_p->reject_reactions;
-  ecma_collection_header_t *fulfill_reactions = promise_p->fulfill_reactions;
+  /* Fulfill reactions will never be triggered. */
+  ecma_free_values_collection (promise_p->fulfill_reactions, ECMA_COLLECTION_NO_REF_OBJECTS);
+  ecma_promise_trigger_reactions (promise_p->reject_reactions, reason);
+
   promise_p->reject_reactions = ecma_new_values_collection ();
   promise_p->fulfill_reactions = ecma_new_values_collection ();
-
-  /* Fulfill reactions will never be triggered. */
-  ecma_free_values_collection (fulfill_reactions, ECMA_COLLECTION_NO_REF_OBJECTS);
-  ecma_promise_trigger_reactions (reject_reactions, reason);
 } /* ecma_reject_promise */
 
 /**
@@ -205,16 +201,12 @@ ecma_fulfill_promise (ecma_value_t promise, /**< promise */
   ecma_promise_set_result (obj_p, ecma_copy_value_if_not_object (value));
   ecma_promise_object_t *promise_p = (ecma_promise_object_t *) obj_p;
 
-  /* GC can be triggered by ecma_new_values_collection so freeing the collection
-     first and creating a new one might cause a heap after use event. */
-  ecma_collection_header_t *reject_reactions = promise_p->reject_reactions;
-  ecma_collection_header_t *fulfill_reactions = promise_p->fulfill_reactions;
+  /* Reject reactions will never be triggered. */
+  ecma_free_values_collection (promise_p->reject_reactions, ECMA_COLLECTION_NO_REF_OBJECTS);
+  ecma_promise_trigger_reactions (promise_p->fulfill_reactions, value);
+
   promise_p->reject_reactions = ecma_new_values_collection ();
   promise_p->fulfill_reactions = ecma_new_values_collection ();
-
-  /* Reject reactions will never be triggered. */
-  ecma_free_values_collection (reject_reactions, ECMA_COLLECTION_NO_REF_OBJECTS);
-  ecma_promise_trigger_reactions (fulfill_reactions, value);
 } /* ecma_fulfill_promise */
 
 /**

diff --git a/tests/jerry/es2015/regression-test-issue-2486.js b/tests/jerry/es2015/regression-test-issue-2486.js
@@ -0,0 +1,18 @@
+// 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.
+
+Object.setPrototypeOf(Math, Int32Array);
+for (var i = 0; i < 200; i++) {
+  Promise.race([, [,] % {}]).then();
+}
diff --git a/tests/jerry/es2015/regression-test-issue-2506.js b/tests/jerry/es2015/regression-test-issue-2506.js
@@ -0,0 +1,20 @@
+// 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.
+
+print(Promise.race([,,,,,,,,,,,,,,,this,,this,,,,,,{ },function(){}])
+	.then(function(){$('The promise should be rejected')},function(){$.id_1(escape,URIError)})
+	.then(-Math.PI/0)<+RangeError);Date.UTC("",0);
+new Date( ).getHours(-{ });
+$.$("arg1,arg2,arg3","arg1,arg2",$,$);
+$.setYear