Changeset 27487 in webkit

Timestamp:

Nov 6, 2007, 1:37:40 PM (17 years ago)

Author:

Darin Adler

Message:

Reviewed by Maciej.

• ​http://bugs.webkit.org/show_bug.cgi?id=15846 REGRESSION (r27387): Memory corruption when running fast/js/kde/delete.html

There was a mistake in the algorithm used to find an empty slot in the property
map entries vector; when we were putting in a new property value and not overwriting
an existing deleted sentinel, we would enlarge the entries vector, but would not
overwrite the stale data that's in the new part. It was easy to pin this down by
turning on property map consistency checks -- I never would have landed with this
bug if I had run the regression tests once with consistency checks on!

• kjs/property_map.cpp: (KJS::PropertyMap::put): Changed logic for the case where foundDeletedElement is false to always use the item at the end of the entries vector. Also allowed me to merge with the logic for the "no deleted sentinels at all" case.

Location:

trunk/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• kjs/property_map.cpp (modified) (1 diff)

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2007-11-06  Darin Adler  <darin@apple.com>
+
+        Reviewed by Maciej.
+
+        - http://bugs.webkit.org/show_bug.cgi?id=15846
+          REGRESSION (r27387): Memory corruption when running fast/js/kde/delete.html
+
+        There was a mistake in the algorithm used to find an empty slot in the property
+        map entries vector; when we were putting in a new property value and not overwriting
+        an existing deleted sentinel, we would enlarge the entries vector, but would not
+        overwrite the stale data that's in the new part. It was easy to pin this down by
+        turning on property map consistency checks -- I never would have landed with this
+        bug if I had run the regression tests once with consistency checks on!
+
+        * kjs/property_map.cpp: (KJS::PropertyMap::put): Changed logic for the case where
+        foundDeletedElement is false to always use the item at the end of the entries vector.
+        Also allowed me to merge with the logic for the "no deleted sentinels at all" case.
+
 2007-11-06  Oliver Hunt  <oliver@apple.com>
 

trunk/JavaScriptCore/kjs/property_map.cpp

@@ -414 +414 @@
 
     // Figure out which entry to use.
-    unsigned entryIndex;
-    if (!m_u.table->deletedSentinelCount)
-        entryIndex = m_u.table->keyCount + 2;
-    else {
-        if (foundDeletedElement) {
-            i = deletedElementIndex;
-            --m_u.table->deletedSentinelCount;
-        }
-        for (entryIndex = m_u.table->keyCount + m_u.table->deletedSentinelCount + 2;
-                m_u.table->entries()[entryIndex - 1].key; --entryIndex)
+    unsigned entryIndex = m_u.table->keyCount + m_u.table->deletedSentinelCount + 2;
+    if (foundDeletedElement) {
+        i = deletedElementIndex;
+        --m_u.table->deletedSentinelCount;
+
+        // Since we're not making the table bigger, we can't use the entry one past
+        // the end that we were planning on using, so search backwards for the empty
+        // slot that we can use. We know it will be there because we did at least one
+        // deletion in the past that left an entry empty.
+        while (m_u.table->entries()[--entryIndex].key)
             ;
     }