DirectArguments::create needs to initialize to undefined instead of the empty value
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Jun 2018 01:11:45 +0000 (01:11 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Jun 2018 01:11:45 +0000 (01:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=186818
<rdar://problem/38415177>

Reviewed by Filip Pizlo.

JSTests:

* stress/create-direct-arguments-in-osr-should-initialize-to-undefined.js: Added.
(foo):
(bar):

Source/JavaScriptCore:

The bug here is that we will emit code that just loads from DirectArguments as
long as the index is within the known capacity of the arguments object (op_get_from_arguments).
The arguments object has at least enough capacity to hold the declared parameters.
When we materialized this object in OSR exit, we initialized up to to the capacity
with JSValue(). In OSR exit, though, we only filled up to the length of the
object with actual values. So we'd end up with a DirectArguments object with
capacity minus length slots of JSValue(). To fix this, we need initialize up to
capacity with jsUndefined during construction. The invariant of this object is
that the capacity minus length slots at the end are filled in with jsUndefined.

* runtime/DirectArguments.cpp:
(JSC::DirectArguments::create):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@233000 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/create-direct-arguments-in-osr-should-initialize-to-undefined.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/DirectArguments.cpp

index 1b3c787..b83e27e 100644 (file)
@@ -1,3 +1,15 @@
+2018-06-19  Saam Barati  <sbarati@apple.com>
+
+        DirectArguments::create needs to initialize to undefined instead of the empty value
+        https://bugs.webkit.org/show_bug.cgi?id=186818
+        <rdar://problem/38415177>
+
+        Reviewed by Filip Pizlo.
+
+        * stress/create-direct-arguments-in-osr-should-initialize-to-undefined.js: Added.
+        (foo):
+        (bar):
+
 2018-06-19  Tadeu Zagallo  <tzagallo@apple.com>
 
         ShadowChicken crashes with stack overflow in the LLInt
diff --git a/JSTests/stress/create-direct-arguments-in-osr-should-initialize-to-undefined.js b/JSTests/stress/create-direct-arguments-in-osr-should-initialize-to-undefined.js
new file mode 100644 (file)
index 0000000..6c79048
--- /dev/null
@@ -0,0 +1,17 @@
+// This should not crash.
+
+function foo(a, b) {
+    let x = arguments;
+    OSRExit();
+    return a + b;
+}
+
+function bar(b) {
+    if (b)
+        foo();
+}
+noInline(bar);
+
+for (let i = 0; i < 1000; ++i) {
+    bar(true);
+}
index 982ba62..d1a45ad 100644 (file)
@@ -1,3 +1,24 @@
+2018-06-19  Saam Barati  <sbarati@apple.com>
+
+        DirectArguments::create needs to initialize to undefined instead of the empty value
+        https://bugs.webkit.org/show_bug.cgi?id=186818
+        <rdar://problem/38415177>
+
+        Reviewed by Filip Pizlo.
+
+        The bug here is that we will emit code that just loads from DirectArguments as
+        long as the index is within the known capacity of the arguments object (op_get_from_arguments).
+        The arguments object has at least enough capacity to hold the declared parameters.
+        When we materialized this object in OSR exit, we initialized up to to the capacity
+        with JSValue(). In OSR exit, though, we only filled up to the length of the
+        object with actual values. So we'd end up with a DirectArguments object with
+        capacity minus length slots of JSValue(). To fix this, we need initialize up to
+        capacity with jsUndefined during construction. The invariant of this object is
+        that the capacity minus length slots at the end are filled in with jsUndefined.
+
+        * runtime/DirectArguments.cpp:
+        (JSC::DirectArguments::create):
+
 2018-06-19  Michael Saboff  <msaboff@apple.com>
 
         Crash in sanitizeStackForVMImpl sometimes when switching threads with same VM
index b629556..f910f09 100644 (file)
@@ -61,7 +61,7 @@ DirectArguments* DirectArguments::create(VM& vm, Structure* structure, unsigned
     DirectArguments* result = createUninitialized(vm, structure, length, capacity);
     
     for (unsigned i = capacity; i--;)
-        result->storage()[i].clear();
+        result->storage()[i].setUndefined();
     
     return result;
 }