Changeset 155365 in webkit

Timestamp:

Sep 9, 2013, 11:21:06 AM (13 years ago)

Author:

Lucas Forschler

Message:

Merge fix for <rdar://problem/14909253>

Location:

branches/safari-534.59-branch/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/ArrayPrototype.cpp (modified) (1 diff)
• runtime/JSArray.cpp (modified) (8 diffs)
• runtime/JSArray.h (modified) (1 diff)

branches/safari-534.59-branch/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2013-09-09  Lucas Forschler  <lforschler@apple.com>
+
+        Merge fix for <rdar:/problem/14909253>
+    
+    2013-09-09  Filip Pizlo  <fpizlo@apple.com>
+    
+            * runtime/ArrayPrototype.cpp:
+            (JSC::arrayProtoFuncSort):
+            * runtime/JSArray.cpp:
+            (JSC::JSArray::sortNumeric):
+            (JSC::JSArray::sort):
+            (JSC::JSArray::compactForSorting):
+            * runtime/JSArray.h:
+            (JSC::JSArray::hasSparseMap):
+
 2013-03-14  Lucas Forschler  <lforschler@apple.com>
 

branches/safari-534.59-branch/Source/JavaScriptCore/runtime/ArrayPrototype.cpp

@@ -539 +539 @@
     CallType callType = getCallData(function, callData);
 
-    if (thisObj->classInfo() == &JSArray::s_info) {
+    if (thisObj->classInfo() == &JSArray::s_info && !asArray(thisObj)->hasSparseMap()) {
         if (isNumericCompareFunction(exec, callType, callData))
             asArray(thisObj)->sortNumeric(exec, function, callType, callData);

branches/safari-534.59-branch/Source/JavaScriptCore/runtime/JSArray.cpp

@@ -965 +965 @@
 
     unsigned lengthNotIncludingUndefined = compactForSorting();
-    if (storage->m_sparseValueMap) {
-        throwOutOfMemoryError(exec);
-        return;
-    }
+    
+    ASSERT(!storage->m_sparseValueMap);
 
     if (!lengthNotIncludingUndefined)
@@ -998 +996 @@
 
     unsigned lengthNotIncludingUndefined = compactForSorting();
-    if (storage->m_sparseValueMap) {
-        throwOutOfMemoryError(exec);
-        return;
-    }
+    ASSERT(!storage->m_sparseValueMap);
 
     if (!lengthNotIncludingUndefined)
@@ -1155 +1150 @@
 
     unsigned usedVectorLength = min(storage->m_length, m_vectorLength);
-    unsigned nodeCount = usedVectorLength + (storage->m_sparseValueMap ? storage->m_sparseValueMap->size() : 0);
+    unsigned nodeCount = usedVectorLength;
 
     if (!nodeCount)
@@ -1183 +1178 @@
     // Iterate over the array, ignoring missing values, counting undefined ones, and inserting all other ones into the tree.
     for (; numDefined < usedVectorLength; ++numDefined) {
+        if (numDefined >= m_vectorLength)
+            break;
         JSValue v = storage->m_vector[numDefined].get();
         if (!v || v.isUndefined())
@@ -1190 +1187 @@
     }
     for (unsigned i = numDefined; i < usedVectorLength; ++i) {
+        if (i >= m_vectorLength)
+            break;
         JSValue v = storage->m_vector[i].get();
         if (v) {
@@ -1204 +1203 @@
     unsigned newUsedVectorLength = numDefined + numUndefined;
 
-    if (SparseArrayValueMap* map = storage->m_sparseValueMap) {
-        newUsedVectorLength += map->size();
-        if (newUsedVectorLength > m_vectorLength) {
-            // Check that it is possible to allocate an array large enough to hold all the entries.
-            if ((newUsedVectorLength > MAX_STORAGE_VECTOR_LENGTH) || !increaseVectorLength(newUsedVectorLength)) {
-                throwOutOfMemoryError(exec);
-                return;
-            }
-        }
-        
-        storage = m_storage;
-
-        SparseArrayValueMap::iterator end = map->end();
-        for (SparseArrayValueMap::iterator it = map->begin(); it != end; ++it) {
-            tree.abstractor().m_nodes[numDefined].value = it->second.get();
-            tree.insert(numDefined);
-            ++numDefined;
-        }
-
-        delete map;
-        storage->m_sparseValueMap = 0;
-    }
-
-    ASSERT(tree.abstractor().m_nodes.size() >= numDefined);
-
-    // FIXME: If the compare function changed the length of the array, the following might be
-    // modifying the vector incorrectly.
-
+    ASSERT(!storage->m_sparseValueMap);
+    
+    // The array size may have changed.  Figure out the new bounds.
+    unsigned newestUsedVectorLength = min(m_storage->m_length, m_vectorLength); 
+    
+    unsigned elementsToExtractThreshold = min(min(newestUsedVectorLength, numDefined), static_cast<unsigned>(tree.abstractor().m_nodes.size())); 
+    unsigned undefinedElementsThreshold = min(newestUsedVectorLength, newUsedVectorLength); 
+    unsigned clearElementsThreshold = min(newestUsedVectorLength, usedVectorLength);     
+    
     // Copy the values back into m_storage.
     AVLTree<AVLTreeAbstractorForArrayCompare, 44>::Iterator iter;
     iter.start_iter_least(tree);
     JSGlobalData& globalData = exec->globalData();
-    for (unsigned i = 0; i < numDefined; ++i) {
+    for (unsigned i = 0; i < elementsToExtractThreshold; ++i) {
         storage->m_vector[i].set(globalData, this, tree.abstractor().m_nodes[*iter].value);
         ++iter;
@@ -1242 +1222 @@
 
     // Put undefined values back in.
-    for (unsigned i = numDefined; i < newUsedVectorLength; ++i)
+    for (unsigned i = elementsToExtractThreshold; i < undefinedElementsThreshold; ++i)
         storage->m_vector[i].setUndefined();
 
     // Ensure that unused values in the vector are zeroed out.
-    for (unsigned i = newUsedVectorLength; i < usedVectorLength; ++i)
+    for (unsigned i = undefinedElementsThreshold; i < clearElementsThreshold; ++i)
         storage->m_vector[i].clear();
 
@@ -1319 +1299 @@
     unsigned newUsedVectorLength = numDefined + numUndefined;
 
-    if (SparseArrayValueMap* map = storage->m_sparseValueMap) {
-        newUsedVectorLength += map->size();
-        if (newUsedVectorLength > m_vectorLength) {
-            // Check that it is possible to allocate an array large enough to hold all the entries - if not,
-            // exception is thrown by caller.
-            if ((newUsedVectorLength > MAX_STORAGE_VECTOR_LENGTH) || !increaseVectorLength(newUsedVectorLength))
-                return 0;
-
-            storage = m_storage;
-        }
-
-        SparseArrayValueMap::iterator end = map->end();
-        for (SparseArrayValueMap::iterator it = map->begin(); it != end; ++it)
-            storage->m_vector[numDefined++].setWithoutWriteBarrier(it->second.get());
-
-        delete map;
-        storage->m_sparseValueMap = 0;
-    }
+    ASSERT(!storage->m_sparseValueMap);
 
     for (unsigned i = numDefined; i < newUsedVectorLength; ++i)

branches/safari-534.59-branch/Source/JavaScriptCore/runtime/JSArray.h

@@ -174 +174 @@
 
         static void visitChildren(JSCell*, SlotVisitor&);
+        
+        bool hasSparseMap()
+        {
+            return !!m_storage->m_sparseValueMap;
+        }
 
     protected: