JavaScriptCore:
authorggaren <ggaren@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 3 Mar 2006 23:33:05 +0000 (23:33 +0000)
committerggaren <ggaren@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 3 Mar 2006 23:33:05 +0000 (23:33 +0000)
        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.

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

JavaScriptCore/ChangeLog
JavaScriptCore/kjs/property_map.cpp
WebCore/ChangeLog
WebCore/manual-tests/property-map-save-crash.html [new file with mode: 0644]

index d5112576c4bcd450b8fe4434ddd0df6ceaa0b1f0..5234d287b103d5056b5717f1bd66b4d041081bd8 100644 (file)
@@ -1,3 +1,26 @@
+2006-03-03  Geoffrey Garen  <ggaren@apple.com>
+
+        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()
+
 2006-03-02  Maciej Stachowiak  <mjs@apple.com>
 
         - now fix mac build again
index a9244d51b1b76de6111db0d69f581bcde24a9003..88ceccfad8ca2a86b8fad6126ea0f9bb27b98330 100644 (file)
@@ -100,7 +100,13 @@ SavedProperties::~SavedProperties() { }
 // Algorithm concepts from Algorithms in C++, Sedgewick.
 
 // 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()
 {
@@ -117,9 +123,10 @@ PropertyMap::~PropertyMap()
     Entry *entries = _table->entries;
     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;
     }
     fastFree(_table);
@@ -142,7 +149,7 @@ void PropertyMap::clear()
     Entry *entries = _table->entries;
     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;
             entries[i].value = 0;
@@ -446,13 +453,10 @@ void PropertyMap::rehash(int newTableSize)
     for (int i = 0; i != oldTableSize; ++i) {
         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);
         }
     }
     _table->lastIndexUsed = lastIndexUsed;
@@ -631,7 +635,7 @@ void PropertyMap::addSparseArrayPropertiesToReferenceList(ReferenceList &list, J
     Entry *entries = _table->entries;
     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;
             k.toUInt32(&fitsInUInt32);
@@ -654,7 +658,7 @@ void PropertyMap::save(SavedProperties &p) const
         int size = _table->size;
         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;
     }
 
@@ -690,7 +694,7 @@ void PropertyMap::save(SavedProperties &p) const
         Entry* entries = _table->entries;
         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;
         }
         assert(p - sortedEntries.data() == count);
index 420f17aa967c4a76ee47e16fd22cdf25eade19d9..4788f49bf7ae3c81952597db4772c89b83a51dab 100644 (file)
@@ -1,3 +1,10 @@
+2006-03-03  Geoffrey Garen  <ggaren@apple.com>
+
+        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.
+
 2006-03-03  Eric Seidel  <eseidel@apple.com>
 
         Reviewed by adele.
diff --git a/WebCore/manual-tests/property-map-save-crash.html b/WebCore/manual-tests/property-map-save-crash.html
new file mode 100644 (file)
index 0000000..c4dce1a
--- /dev/null
@@ -0,0 +1,49 @@
+<html>
+    <head>
+        <script>
+            function test()
+            {
+                if (window.layoutTestController) {
+                    layoutTestController.dumpAsText();
+                    layoutTestController.waitUntilDone();
+                }
+                
+                window.crash = "crash";
+                delete window.crash;
+                    
+                if (window.navigationController)
+                    navigationController.evalAfterBackForwardNavigation("continueTestAfterNavigation()");
+            }
+            
+            function continueTestAfterNavigation()
+            {
+                print("PASS: You didn't crash");
+                
+                if (window.layoutTestController)
+                    layoutTestController.notifyDone();
+            }
+
+            function print(message) {
+                var paragraph = document.createElement("p");
+                paragraph.appendChild(document.createTextNode(message));
+                document.getElementById("console").appendChild(paragraph);
+            }
+        </script>
+    </head>
+
+    <body onload="test()">
+        <p>Bug: rdar://problem/4465598 REGRESSION (TOT): Crash occurs at http://maps.google.com/?output=html ( KJS::Identifier::add(KJS::UString::Rep*)</p> 
+        <p>This cause for this bug was that the code to save the window object's property map tried to use
+        the deleted property sentinel key as a normal pointer.</p>
+        <p>To run this test in Safari:</p>
+        <ol>
+            <li><a href="resources/go-back.html">Click here to do a back/forward navigation.</a></li>
+            <li>You should not crash.</li>
+        </ol>
+        <p>When the automated version of this test passes, you'll see a PASS message below.
+        (The automated version is currently disabled because DumpRenderTree doesn't work
+        with the back/forward cache enabled.)</p>
+        <hr>
+        <div id="console"></div>
+    </body>
+</html>