Changeset 13120 in webkit for trunk/JavaScriptCore/kjs

Timestamp:

Mar 3, 2006, 3:33:05 PM (19 years ago)

Author:

ggaren

Message:

JavaScriptCore:

Reviewed by Darin.

• Fixed <rdar://problem/4465598> REGRESSION (TOT): Crash occurs at ​http://maps.google.com/?output=html ( KJS::Identifier::add(KJS::UString::Rep*)

This regression was caused by my fix for 4448098. I failed to account for the
deleted entry sentinel in the mehtod that saves the contents of a property map to
the back/forward cache.

Manual test in WebCore/manual-tests/property-map-save-crash.html

• kjs/property_map.cpp: (KJS::deletedSentinel): Use 1 instead of -1 to facilitate an easy bit mask (KJS::isValid): New function: checks if a key is null or the deleted sentinel (KJS::PropertyMap::~PropertyMap): Fixed up the branch logic here for readability and a slight performance win (KJS::PropertyMap::clear): (KJS::PropertyMap::rehash): (KJS::PropertyMap::addSparseArrayPropertiesToReferenceList): (KJS::PropertyMap::save): Check keys with isValid()

WebCore:

Test case for <rdar://problem/4465598> REGRESSION (TOT): Crash occurs at
​http://maps.google.com/?output=html ( KJS::Identifier::add(KJS::UString::Rep*)

• manual-tests/property-map-save-crash.html: Added.

File:

• trunk/JavaScriptCore/kjs/property_map.cpp (modified) (7 diffs)

trunk/JavaScriptCore/kjs/property_map.cpp

@@ -101 +101 @@
 
 // This is a method rather than a variable to work around <rdar://problem/4462053>
-static inline UString::Rep* deletedSentinel() { return reinterpret_cast<UString::Rep*>(-1); }
+static inline UString::Rep* deletedSentinel() { return reinterpret_cast<UString::Rep*>(0x1); }
+
+// Returns true if the key is not null or the deleted sentinel, false otherwise
+static inline bool isValid(UString::Rep* key)
+{
+    return reinterpret_cast<uintptr_t>(key) & ~0x1;
+}
 
 PropertyMap::~PropertyMap()
@@ -118 +124 @@
     for (int i = 0; i < minimumKeysToProcess; i++) {
         UString::Rep *key = entries[i].key;
-        if (key && key != deletedSentinel())
-            key->deref();
-        else if (key != deletedSentinel())
+        if (key) {
+            if (key != deletedSentinel())
+                key->deref();
+        } else
             ++minimumKeysToProcess;
     }
@@ -143 +150 @@
     for (int i = 0; i < size; i++) {
         UString::Rep *key = entries[i].key;
-        if (key && key != deletedSentinel()) {
+        if (isValid(key)) {
             key->deref();
             entries[i].key = 0;
@@ -447 +454 @@
         Entry &entry = oldTable->entries[i];
         UString::Rep *key = entry.key;
-        if (key) {
-            // Don't copy deleted-element sentinels.
-            if (key != deletedSentinel()) {
-                int index = entry.index;
-                lastIndexUsed = max(index, lastIndexUsed);
-                insert(key, entry.value, entry.attributes, index);
-            }
+        if (isValid(key)) {
+            int index = entry.index;
+            lastIndexUsed = max(index, lastIndexUsed);
+            insert(key, entry.value, entry.attributes, index);
         }
     }
@@ -632 +636 @@
     for (int i = 0; i != size; ++i) {
         UString::Rep *key = entries[i].key;
-        if (key && key != deletedSentinel()) {
+        if (isValid(key)) {
             UString k(key);
             bool fitsInUInt32;
@@ -655 +659 @@
         Entry *entries = _table->entries;
         for (int i = 0; i != size; ++i)
-            if (entries[i].key && !(entries[i].attributes & (ReadOnly | Function)))
+            if (isValid(entries[i].key) && !(entries[i].attributes & (ReadOnly | Function)))
                 ++count;
     }
@@ -691 +695 @@
         for (int i = 0; i != size; ++i) {
             Entry *e = &entries[i];
-            if (e->key && !(e->attributes & (ReadOnly | Function)))
+            if (isValid(e->key) && !(e->attributes & (ReadOnly | Function)))
                 *p++ = e;
         }