Calculating postCapacity in unshiftCountSlowCase is wrong

https://bugs.webkit.org/show_bug.cgi?id=173992
<rdar://problem/32283199>

Reviewed by Keith Miller.

JSTests:

* stress/unshiftCountSlowCase-correct-postCapacity.js: Added.
(temp):

Source/JavaScriptCore:

This patch fixes a bug inside unshiftCountSlowCase where we would use
more memory than we allocated. The bug was when deciding how much extra
space we have after the vector we've allocated. This area is called the
postCapacity. The largest legal postCapacity value we could use is the
space we allocated minus the space we need:
largestPossiblePostCapacity = newStorageCapacity - requiredVectorLength;
However, the code was calculating the postCapacity as:
postCapacity = max(newStorageCapacity - requiredVectorLength, count);

where count is how many elements we're appending. Depending on the inputs,
count could be larger than (newStorageCapacity - requiredVectorLength). This
would cause us to use more memory than we actually allocated.

* runtime/JSArray.cpp:
(JSC::JSArray::unshiftCountSlowCase):

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

diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
index 2a952d3..71e5e02 100644 (file)
--- a/JSTests/ChangeLog
+++ b/JSTests/ChangeLog
@@ -1,3 +1,14 @@
+2017-06-29  Saam Barati  <sbarati@apple.com>
+
+        Calculating postCapacity in unshiftCountSlowCase is wrong
+        https://bugs.webkit.org/show_bug.cgi?id=173992
+        <rdar://problem/32283199>
+
+        Reviewed by Keith Miller.
+
+        * stress/unshiftCountSlowCase-correct-postCapacity.js: Added.
+        (temp):
+
 2017-06-29  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r218512.

diff --git a/JSTests/stress/unshiftCountSlowCase-correct-postCapacity.js b/JSTests/stress/unshiftCountSlowCase-correct-postCapacity.js
new file mode 100644 (file)
index 0000000..f0bf8c3
--- /dev/null
+++ b/JSTests/stress/unshiftCountSlowCase-correct-postCapacity.js
@@ -0,0 +1,26 @@
+//@ runDefault
+
+function temp(i) {
+    let a1 = [{}];
+    a1.foo = 20;
+    a1.foo1 = 20;
+    a1.foo2 = 20;
+    a1.foo3 = 20;
+    a1.foo4 = 20;
+    a1.foo5 = 20;
+    a1.foo6 = 20;
+    a1.foo8 = 20;
+    a1.foo10 = 20;
+    a1.foo11 = 20;
+    delete a1[0];
+    try {
+        let args = [-15, 1, 'foo', 20, 'bar'];
+        for (let j = 0; j < i; ++j)
+            args.push(j);
+        for (let i = 0; i < 2**31 - 1; ++i) {
+            Array.prototype.splice.apply(a1, args);
+        }
+    } catch(e) { }
+}
+let i = 62;
+temp(i);

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index d66515f..53fd9ff 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,27 @@
+2017-06-29  Saam Barati  <sbarati@apple.com>
+
+        Calculating postCapacity in unshiftCountSlowCase is wrong
+        https://bugs.webkit.org/show_bug.cgi?id=173992
+        <rdar://problem/32283199>
+
+        Reviewed by Keith Miller.
+
+        This patch fixes a bug inside unshiftCountSlowCase where we would use
+        more memory than we allocated. The bug was when deciding how much extra
+        space we have after the vector we've allocated. This area is called the
+        postCapacity. The largest legal postCapacity value we could use is the
+        space we allocated minus the space we need:
+        largestPossiblePostCapacity = newStorageCapacity - requiredVectorLength;
+        However, the code was calculating the postCapacity as:
+        postCapacity = max(newStorageCapacity - requiredVectorLength, count);
+        
+        where count is how many elements we're appending. Depending on the inputs,
+        count could be larger than (newStorageCapacity - requiredVectorLength). This
+        would cause us to use more memory than we actually allocated.
+
+        * runtime/JSArray.cpp:
+        (JSC::JSArray::unshiftCountSlowCase):
+
 2017-06-29  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r218512.

diff --git a/Source/JavaScriptCore/runtime/JSArray.cpp b/Source/JavaScriptCore/runtime/JSArray.cpp
index 3cb4dfe..f10ed9d 100644 (file)
--- a/Source/JavaScriptCore/runtime/JSArray.cpp
+++ b/Source/JavaScriptCore/runtime/JSArray.cpp
@@ -377,7 +377,7 @@ bool JSArray::unshiftCountSlowCase(const AbstractLocker&, VM& vm, DeferGC&, bool
     // vector with half the post-capacity it had previously.
     unsigned postCapacity = 0;
     if (!addToFront)
-        postCapacity = max(newStorageCapacity - requiredVectorLength, count);
+        postCapacity = newStorageCapacity - requiredVectorLength;
     else if (length < storage->vectorLength()) {
         // Atomic decay, + the post-capacity cannot be greater than what is available.
         postCapacity = min((storage->vectorLength() - length) >> 1, newStorageCapacity - requiredVectorLength);
@@ -386,6 +386,7 @@ bool JSArray::unshiftCountSlowCase(const AbstractLocker&, VM& vm, DeferGC&, bool
     }
 
     unsigned newVectorLength = requiredVectorLength + postCapacity;
+    RELEASE_ASSERT(newVectorLength <= MAX_STORAGE_VECTOR_LENGTH);
     unsigned newIndexBias = newStorageCapacity - newVectorLength;
 
     Butterfly* newButterfly = Butterfly::fromBase(newAllocBase, newIndexBias, propertyCapacity);