JSObject::putByIndexBeyondVectorLengthWithoutAttributes needs to go to the sparse...

https://bugs.webkit.org/show_bug.cgi?id=154201
rdar://problem/24291387

Reviewed by Saam Barati.

I decided against adding a test for this, because it runs for a very long time.

* runtime/JSObject.cpp:
(JSC::JSObject::putByIndexBeyondVectorLengthWithoutAttributes): Fix the bug.
* runtime/StringPrototype.cpp:
(JSC::stringProtoFuncSplit): Fix a related bug: if this code creates an array that would have
    hit the above bug, then it would probably manifest as a spin or as swapping.

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

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index cd7957e..ee3fbb8 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,19 @@
+2016-02-12  Filip Pizlo  <fpizlo@apple.com>
+
+        JSObject::putByIndexBeyondVectorLengthWithoutAttributes needs to go to the sparse map based on MAX_STORAGE_VECTOR_INDEX
+        https://bugs.webkit.org/show_bug.cgi?id=154201
+        rdar://problem/24291387
+
+        Reviewed by Saam Barati.
+
+        I decided against adding a test for this, because it runs for a very long time.
+
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::putByIndexBeyondVectorLengthWithoutAttributes): Fix the bug.
+        * runtime/StringPrototype.cpp:
+        (JSC::stringProtoFuncSplit): Fix a related bug: if this code creates an array that would have
+            hit the above bug, then it would probably manifest as a spin or as swapping.
+
 2016-02-12  Jonathan Davis  <jond@apple.com>
 
         Add WebAssembly to the status page

diff --git a/Source/JavaScriptCore/runtime/JSObject.cpp b/Source/JavaScriptCore/runtime/JSObject.cpp
index 033b403..a65f703 100644 (file)
--- a/Source/JavaScriptCore/runtime/JSObject.cpp
+++ b/Source/JavaScriptCore/runtime/JSObject.cpp
@@ -1943,7 +1943,7 @@ void JSObject::putByIndexBeyondVectorLengthWithoutAttributes(ExecState* exec, un
     
     VM& vm = exec->vm();
     
-    if (i >= MAX_ARRAY_INDEX - 1
+    if (i > MAX_STORAGE_VECTOR_INDEX
         || (i >= MIN_SPARSE_ARRAY_INDEX
             && !isDenseEnoughForVector(i, countElements<indexingShape>(butterfly)))
         || indexIsSufficientlyBeyondLengthForSparseMap(i, butterfly->vectorLength())) {

diff --git a/Source/JavaScriptCore/runtime/StringPrototype.cpp b/Source/JavaScriptCore/runtime/StringPrototype.cpp
index 50e37ed..7df93e5 100644 (file)
--- a/Source/JavaScriptCore/runtime/StringPrototype.cpp
+++ b/Source/JavaScriptCore/runtime/StringPrototype.cpp
@@ -1190,8 +1190,23 @@ EncodedJSValue JSC_HOST_CALL stringProtoFuncSplit(ExecState* exec)
 
             // 3. Increment lengthA by 1.
             // 4. If lengthA == lim, return A.
-            if (++resultLength == limit)
+            ++resultLength;
+            if (resultLength == limit)
                 return JSValue::encode(result);
+            if (resultLength >= MAX_STORAGE_VECTOR_INDEX) {
+                // Let's consider what's best for users here. We're about to increase the length of
+                // the split array beyond the maximum length that we can support efficiently. This
+                // will cause us to use a HashMap for the new entries after this point. That's going
+                // to result in a very long running time of this function and very large memory
+                // usage. In my experiments, JSC will sit spinning for minutes after getting here and
+                // it was using >4GB of memory and eventually grew to 8GB. It kept running without
+                // finishing until I killed it. That's probably not what the user wanted. The user,
+                // or the program that the user is running, probably made a mistake by calling this
+                // method in such a way that it resulted in such an obnoxious array. Therefore, to
+                // protect ourselves, we bail at this point.
+                throwOutOfMemoryError(exec);
+                return JSValue::encode(jsUndefined());
+            }
 
             // 5. Let p = e.
             // 8. Let q = p.