JavaScriptCore:

        Reviewed by Mark.

        - Added assertions to protect against adding empty or deleted keys to a HashTable

        * wtf/HashTable.h:
        (WTF::HashTable::lookup):
        (WTF::HashTable::lookupForWriting):
        (WTF::HashTable::fullLookupForWriting):
        (WTF::HashTable::add):

WebCore:

        Reviewed by Mark.

        - fixed REGRESSION(r27176): Reproducible crash while trying to order dinner makes bdash sad
        http://bugs.webkit.org/show_bug.cgi?id=15731

        * bindings/js/kjs_window.cpp:
        (KJS::Window::installTimeout): Avoid putting in or accessing empty or deleted keys.
        (KJS::Window::clearTimeout): ditto
        * manual-tests/bad-clearTimeout-crash.html: Added. Automated test not possible.

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

diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index ecc5592..d47bba4 100644 (file)
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,15 @@
+2007-10-28  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Mark.
+        
+        - Added assertions to protect against adding empty or deleted keys to a HashTable
+
+        * wtf/HashTable.h:
+        (WTF::HashTable::lookup):
+        (WTF::HashTable::lookupForWriting):
+        (WTF::HashTable::fullLookupForWriting):
+        (WTF::HashTable::add):
+
 2007-10-28  Darin Adler  <darin@apple.com>
 
         - fix GTK build

diff --git a/JavaScriptCore/wtf/HashTable.h b/JavaScriptCore/wtf/HashTable.h
index 8df5af5..e08b4e9 100644 (file)
--- a/JavaScriptCore/wtf/HashTable.h
+++ b/JavaScriptCore/wtf/HashTable.h
@@ -402,6 +402,12 @@ namespace WTF {
     inline Value* HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::lookup(const T& key)
     {
         ASSERT(m_table);
+#ifndef ASSERT_DISABLED
+        if (HashFunctions::safeToCompareToEmptyOrDeleted) {
+            ASSERT(!HashTranslator::equal(KeyTraits::emptyValue(), key));
+            ASSERT(!HashTranslator::equal(KeyTraits::deletedValue(), key));
+        }
+#endif
 
         int k = 0;
         int sizeMask = m_tableSizeMask;
@@ -446,6 +452,12 @@ namespace WTF {
     inline typename HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::LookupType HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::lookupForWriting(const T& key)
     {
         ASSERT(m_table);
+#ifndef ASSERT_DISABLED
+        if (HashFunctions::safeToCompareToEmptyOrDeleted) {
+            ASSERT(!HashTranslator::equal(KeyTraits::emptyValue(), key));
+            ASSERT(!HashTranslator::equal(KeyTraits::deletedValue(), key));
+        }
+#endif
 
         int k = 0;
         ValueType* table = m_table;
@@ -497,6 +509,12 @@ namespace WTF {
     inline typename HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::FullLookupType HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::fullLookupForWriting(const T& key)
     {
         ASSERT(m_table);
+#ifndef ASSERT_DISABLED
+        if (HashFunctions::safeToCompareToEmptyOrDeleted) {
+            ASSERT(!HashTranslator::equal(KeyTraits::emptyValue(), key));
+            ASSERT(!HashTranslator::equal(KeyTraits::deletedValue(), key));
+        }
+#endif
 
         int k = 0;
         ValueType* table = m_table;
@@ -547,6 +565,13 @@ namespace WTF {
     template<typename T, typename Extra, typename HashTranslator>
     inline pair<typename HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::iterator, bool> HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::add(const T& key, const Extra& extra)
     {
+#ifndef ASSERT_DISABLED
+        if (HashFunctions::safeToCompareToEmptyOrDeleted) {
+            ASSERT(!HashTranslator::equal(KeyTraits::emptyValue(), key));
+            ASSERT(!HashTranslator::equal(KeyTraits::deletedValue(), key));
+        }
+#endif
+
         invalidateIterators();
 
         if (!m_table)

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 6ceb381..afc60a3 100644 (file)
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,15 @@
+2007-10-28  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Mark.
+
+        - fixed REGRESSION(r27176): Reproducible crash while trying to order dinner makes bdash sad
+        http://bugs.webkit.org/show_bug.cgi?id=15731
+
+        * bindings/js/kjs_window.cpp:
+        (KJS::Window::installTimeout): Avoid putting in or accessing empty or deleted keys.
+        (KJS::Window::clearTimeout): ditto
+        * manual-tests/bad-clearTimeout-crash.html: Added. Automated test not possible.
+
 2007-10-28  Kevin Ollivier  <kevino@theolliviers.com>
 
         wx port defines for graphics and network layers.

diff --git a/WebCore/bindings/js/kjs_window.cpp b/WebCore/bindings/js/kjs_window.cpp
index a0324df..77e3218 100644 (file)
--- a/WebCore/bindings/js/kjs_window.cpp
+++ b/WebCore/bindings/js/kjs_window.cpp
@@ -1521,6 +1521,11 @@ void Window::clearAllTimeouts()
 int Window::installTimeout(ScheduledAction* a, int t, bool singleShot)
 {
     int timeoutId = ++lastUsedTimeoutId;
+
+    // avoid wraparound going negative on us
+    if (timeoutId <= 0)
+        timeoutId = 1;
+
     int nestLevel = timerNestingLevel + 1;
     DOMWindowTimer* timer = new DOMWindowTimer(timeoutId, nestLevel, this, a);
     ASSERT(!d->m_timeouts.get(timeoutId));
@@ -1592,6 +1597,12 @@ void Window::resumeTimeouts(PausedTimeouts* timeouts)
 
 void Window::clearTimeout(int timeoutId, bool delAction)
 {
+    // timeout IDs have to be positive, and 0 and -1 are unsafe to
+    // even look up since they are the empty and deleted value
+    // respectively
+    if (timeoutId <= 0)
+        return;
+
     WindowPrivate::TimeoutsMap::iterator it = d->m_timeouts.find(timeoutId);
     if (it == d->m_timeouts.end())
         return;

diff --git a/WebCore/manual-tests/bad-clearTimeout-crash.html b/WebCore/manual-tests/bad-clearTimeout-crash.html
new file mode 100644 (file)
index 0000000..b7ee23c
--- /dev/null
+++ b/WebCore/manual-tests/bad-clearTimeout-crash.html
@@ -0,0 +1,13 @@
+If the back/forward cache is enabled, this test will crash instead of going to the next page that says PASS.
+
+It cannot be automated because DumpRenderTree doesn't support the back/forward cache.
+
+<script>
+if (window.layoutTestController)
+    layoutTestController.waitUntilDone();
+
+setTimeout('',1000);
+clearTimeout(0);
+clearTimeout(0);
+window.location = "data:text/html,This test shouldn't crash. PASS.<scr" + "ipt>if (window.layoutTestController) layoutTestController.notifyDone()</scr" + "ipt>";
+</script>