Changeset 175249 in webkit

Timestamp:

Oct 28, 2014 8:29:51 AM (9 years ago)

Author:

mark.lam@apple.com

Message:

Holes are not copied properly when Arrays change shape to ArrayStorage type.
<​https://webkit.org/b/138118>

Reviewed by Mark Hahnenberg.

When we convert non-ArrayStorage typed arrays into ArrayStorage typed arrays,
we skipped the holes. As a result, the slots in the ArrayStorage vector that
corresponds to those holes are uninitialize. This is now fixed.

• runtime/JSObject.cpp:

(JSC::JSObject::convertUndecidedToArrayStorage):
(JSC::JSObject::convertInt32ToArrayStorage):
(JSC::JSObject::convertDoubleToArrayStorage):
(JSC::JSObject::convertContiguousToArrayStorage):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/JSObject.cpp (modified) (4 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2014-10-27  Mark Lam  <mark.lam@apple.com>
+
+        Holes are not copied properly when Arrays change shape to ArrayStorage type.
+        <https://webkit.org/b/138118>
+
+        Reviewed by Mark Hahnenberg.
+
+        When we convert non-ArrayStorage typed arrays into ArrayStorage typed arrays,
+        we skipped the holes.  As a result, the slots in the ArrayStorage vector that
+        corresponds to those holes are uninitialize.  This is now fixed.
+
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::convertUndecidedToArrayStorage):
+        (JSC::JSObject::convertInt32ToArrayStorage):
+        (JSC::JSObject::convertDoubleToArrayStorage):
+        (JSC::JSObject::convertContiguousToArrayStorage):
+
 2014-10-27  Mark Lam  <mark.lam@apple.com>
 

trunk/Source/JavaScriptCore/runtime/JSObject.cpp

@@ -733 +733 @@
     ArrayStorage* storage = constructConvertedArrayStorageWithoutCopyingElements(vm, vectorLength);
     // No need to copy elements.
+    ASSERT(!m_butterfly->publicLength());
     
     Structure* newStructure = Structure::nonPropertyTransition(vm, structure(vm), transition);
@@ -779 +780 @@
     unsigned vectorLength = m_butterfly->vectorLength();
     ArrayStorage* newStorage = constructConvertedArrayStorageWithoutCopyingElements(vm, vectorLength);
-    for (unsigned i = m_butterfly->publicLength(); i--;) {
+    for (unsigned i = 0; i < m_butterfly->publicLength(); i++) {
         JSValue v = m_butterfly->contiguous()[i].get();
-        if (!v)
-            continue;
-        newStorage->m_vector[i].setWithoutWriteBarrier(v);
-        newStorage->m_numValuesInVector++;
+        if (v) {
+            newStorage->m_vector[i].setWithoutWriteBarrier(v);
+            newStorage->m_numValuesInVector++;
+        } else
+            newStorage->m_vector[i].clear();
     }
     
@@ -848 +850 @@
     unsigned vectorLength = m_butterfly->vectorLength();
     ArrayStorage* newStorage = constructConvertedArrayStorageWithoutCopyingElements(vm, vectorLength);
-    for (unsigned i = m_butterfly->publicLength(); i--;) {
+    for (unsigned i = 0; i < m_butterfly->publicLength(); i++) {
         double value = m_butterfly->contiguousDouble()[i];
-        if (value != value)
-            continue;
-        newStorage->m_vector[i].setWithoutWriteBarrier(JSValue(JSValue::EncodeAsDouble, value));
-        newStorage->m_numValuesInVector++;
+        if (value == value) {
+            newStorage->m_vector[i].setWithoutWriteBarrier(JSValue(JSValue::EncodeAsDouble, value));
+            newStorage->m_numValuesInVector++;
+        } else
+            newStorage->m_vector[i].clear();
     }
     
@@ -873 +876 @@
     unsigned vectorLength = m_butterfly->vectorLength();
     ArrayStorage* newStorage = constructConvertedArrayStorageWithoutCopyingElements(vm, vectorLength);
-    for (unsigned i = m_butterfly->publicLength(); i--;) {
+    for (unsigned i = 0; i < m_butterfly->publicLength(); i++) {
         JSValue v = m_butterfly->contiguous()[i].get();
-        if (!v)
-            continue;
-        newStorage->m_vector[i].setWithoutWriteBarrier(v);
-        newStorage->m_numValuesInVector++;
+        if (v) {
+            newStorage->m_vector[i].setWithoutWriteBarrier(v);
+            newStorage->m_numValuesInVector++;
+        } else
+            newStorage->m_vector[i].clear();
     }