Catch illegal hash table modifications when they happen.

third_party/WebKit/Source/wtf/HashTable.h

Index: third_party/WebKit/Source/wtf/HashTable.h
diff --git a/third_party/WebKit/Source/wtf/HashTable.h b/third_party/WebKit/Source/wtf/HashTable.h
index a1b96bc013b4352635951b34ab8a47f4d793831e..d43dc6cb63e1d316023172dbc294dc3f822c30fd 100644
--- a/third_party/WebKit/Source/wtf/HashTable.h
+++ b/third_party/WebKit/Source/wtf/HashTable.h
@@ -103,6 +103,29 @@ class LinkedHashSet;
 template <WeakHandlingFlag x, typename T, typename U, typename V, typename W, typename X, typename Y, typename Z>
 struct WeakProcessingHashTableHelper;
 
+#if ENABLE(ASSERT)
+// HashTable and collections that build on it do not support modifications
+// while there is an iterator or HashTableAddResult on the stack.
+// The exception is ListHashSet, which has its own iterators that tolerate modification
+// of the underlying set.
+//
+// Constraint enforced by this scope object.
+template <typename HashTableType>
+class HashTableModificationForbiddenScope {
+public:
+    HashTableModificationForbiddenScope();
+    explicit HashTableModificationForbiddenScope(const HashTableType*);
+    HashTableModificationForbiddenScope(const HashTableModificationForbiddenScope&);
+
+    ~HashTableModificationForbiddenScope();
+
+    void leaveScope();
+
+private:
+    HashTableType* m_container;
+};
+#endif
+
 typedef enum { HashItemKnownGood } HashItemKnownGoodTag;
 
 template <typename Key, typename Value, typename Extractor, typename HashFunctions, typename Traits, typename KeyTraits, typename Allocator>
@@ -129,8 +152,7 @@ private:
         : m_position(position)
         , m_endPosition(endPosition)
 #if ENABLE(ASSERT)
-        , m_container(container)
-        , m_containerModifications(container->modifications())
+        , m_modificationForbiddenScope(position != endPosition ? container : nullptr)
 #endif
     {
         skipEmptyBuckets();
@@ -140,21 +162,9 @@ private:
         : m_position(position)
         , m_endPosition(endPosition)
 #if ENABLE(ASSERT)
-        , m_container(container)
-        , m_containerModifications(container->modifications())
+        , m_modificationForbiddenScope(position != endPosition ? container : nullptr)
 #endif
     {
-        ASSERT(m_containerModifications == m_container->modifications());
-    }
-
-    void checkModifications() const
-    {
-        // HashTable and collections that build on it do not support
-        // modifications while there is an iterator in use. The exception is
-        // ListHashSet, which has its own iterators that tolerate modification
-        // of the underlying set.
-        ASSERT(m_containerModifications == m_container->modifications());
-        ASSERT(!m_container->accessForbidden());
     }
 
 public:
@@ -162,7 +172,6 @@ public:
 
     GetType get() const
     {
-        checkModifications();
         return m_position;
     }
     typename Traits::IteratorConstReferenceType operator*() const { return Traits::getToReferenceConstConversion(get()); }
@@ -171,9 +180,15 @@ public:
     const_iterator& operator++()
     {
         ASSERT(m_position != m_endPosition);
-        checkModifications();
         ++m_position;
         skipEmptyBuckets();
+#if ENABLE(ASSERT)
+        // If now at the end, leave the forbidden scope. Waiting until
+        // the iterator goes out of (stack) scope disallows perfectly valid
+        // code idioms.
+        if (m_position == m_endPosition)
+            m_modificationForbiddenScope.leaveScope();
+#endif
         return *this;
     }
 
@@ -201,8 +216,7 @@ private:
     PointerType m_position;
     PointerType m_endPosition;
 #if ENABLE(ASSERT)
-    const HashTableType* m_container;
-    int64_t m_containerModifications;
+    HashTableModificationForbiddenScope<HashTableType> m_modificationForbiddenScope;
 #endif
 };
 
@@ -277,14 +291,14 @@ public:
     template <typename T, typename U, typename V> static void translate(T& location, U&&, V&& value) { location = std::forward<V>(value); }
 };
 
-template <typename HashTableType, typename ValueType> struct HashTableAddResult final {
+template <typename HashTableType, typename ValueType>
+struct HashTableAddResult final {
     STACK_ALLOCATED();
     HashTableAddResult(const HashTableType* container, ValueType* storedValue, bool isNewEntry)
         : storedValue(storedValue)
         , isNewEntry(isNewEntry)
 #if ENABLE(SECURITY_ASSERT)
-        , m_container(container)
-        , m_containerModifications(container->modifications())
+        , m_modificationForbiddenScope(container)
 #endif
     {
         ALLOW_UNUSED_LOCAL(container);
@@ -298,18 +312,12 @@ template <typename HashTableType, typename ValueType> struct HashTableAddResult
     ~HashTableAddResult()
     {
         // If rehash happened before accessing storedValue, it's
-        // use-after-free. Any modification may cause a rehash, so we check for
-        // modifications here.
-
-        // Rehash after accessing storedValue is harmless but will assert if the
-        // AddResult destructor takes place after a modification. You may need
-        // to limit the scope of the AddResult.
-        ASSERT_WITH_SECURITY_IMPLICATION(m_containerModifications == m_container->modifications());
+        // use-after-free. Any modification may cause a rehash, so we enter
+        // a modification forbidden scope for the lifetime of this value.
     }
 
 private:
-    const HashTableType* m_container;
-    const int64_t m_containerModifications;
+    HashTableModificationForbiddenScope<HashTableType> m_modificationForbiddenScope;
 #endif
 };
 
@@ -413,14 +421,7 @@ public:
         ASSERT(!Allocator::isGarbageCollected);
         if (LIKELY(!m_table))
             return;
-        ASSERT(!m_accessForbidden);
-#if ENABLE(ASSERT)
-        m_accessForbidden = true;
-#endif
         deleteAllBucketsAndDeallocate(m_table, m_tableSize);
-#if ENABLE(ASSERT)
-        m_accessForbidden = false;
-#endif
         m_table = nullptr;
     }
 
@@ -441,17 +442,14 @@ public:
 
     unsigned size() const
     {
-        ASSERT(!m_accessForbidden);
         return m_keyCount;
     }
     unsigned capacity() const
     {
-        ASSERT(!m_accessForbidden);
         return m_tableSize;
     }
     bool isEmpty() const
     {
-        ASSERT(!m_accessForbidden);
         return !m_keyCount;
     }
 
@@ -493,17 +491,19 @@ public:
     template <typename VisitorDispatcher> void trace(VisitorDispatcher);
 
 #if ENABLE(ASSERT)
-    bool accessForbidden() const { return m_accessForbidden; }
-    int64_t modifications() const { return m_modifications; }
-    void registerModification() { m_modifications++; }
-    // HashTable and collections that build on it do not support modifications
-    // while there is an iterator in use. The exception is ListHashSet, which
-    // has its own iterators that tolerate modification of the underlying set.
-    void checkModifications(int64_t mods) const { ASSERT(mods == m_modifications); }
+    void enterModificationForbiddenScope() { m_modificationForbidden++; }
+    void leaveModificationForbiddenScope()
+    {
+        ASSERT(m_modificationForbidden > 0);
+        m_modificationForbidden--;
+    }
+
+    void registerModification()
+    {
+        ASSERT(!m_modificationForbidden);
+    }
 #else
-    int64_t modifications() const { return 0; }
-    void registerModification() {}
-    void checkModifications(int64_t mods) const {}
+    void registerModification() { }
 #endif
 
 private:
@@ -569,14 +569,10 @@ private:
     ValueType* m_table;
     unsigned m_tableSize;
     unsigned m_keyCount;
-#if ENABLE(ASSERT)
-    unsigned m_deletedCount:30;
-    unsigned m_queueFlag:1;
-    unsigned m_accessForbidden:1;
-    unsigned m_modifications;
-#else
     unsigned m_deletedCount:31;
     unsigned m_queueFlag:1;
+#if ENABLE(ASSERT)
+    unsigned m_modificationForbidden;
 #endif
 
 #if DUMP_HASHTABLE_STATS_PER_TABLE
@@ -596,8 +592,7 @@ inline HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Alloca
     , m_deletedCount(0)
     , m_queueFlag(false)
 #if ENABLE(ASSERT)
-    , m_accessForbidden(false)
-    , m_modifications(0)
+    , m_modificationForbidden(0)
 #endif
 #if DUMP_HASHTABLE_STATS_PER_TABLE
     , m_stats(adoptPtr(new Stats))
@@ -647,7 +642,6 @@ template <typename Key, typename Value, typename Extractor, typename HashFunctio
 template <typename HashTranslator, typename T>
 inline const Value* HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Allocator>::lookup(const T& key) const
 {
-    ASSERT(!m_accessForbidden);
     ASSERT((HashTableKeyChecker<HashTranslator, KeyTraits, HashFunctions::safeToCompareToEmptyOrDeleted>::checkKey(key)));
     const ValueType* table = m_table;
     if (!table)
@@ -687,7 +681,6 @@ template <typename Key, typename Value, typename Extractor, typename HashFunctio
 template <typename HashTranslator, typename T>
 inline typename HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Allocator>::LookupType HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Allocator>::lookupForWriting(const T& key)
 {
-    ASSERT(!m_accessForbidden);
     ASSERT(m_table);
     registerModification();
 
@@ -730,7 +723,6 @@ template <typename Key, typename Value, typename Extractor, typename HashFunctio
 template <typename HashTranslator, typename T>
 inline typename HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Allocator>::FullLookupType HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Allocator>::fullLookupForWriting(const T& key)
 {
-    ASSERT(!m_accessForbidden);
     ASSERT(m_table);
     registerModification();
 
@@ -801,7 +793,6 @@ template <typename Key, typename Value, typename Extractor, typename HashFunctio
 template <typename HashTranslator, typename T, typename Extra>
 typename HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Allocator>::AddResult HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Allocator>::add(T&& key, Extra&& extra)
 {
-    ASSERT(!m_accessForbidden);
     ASSERT(Allocator::isAllocationAllowed());
     if (!m_table)
         expand();
@@ -867,7 +858,6 @@ template <typename Key, typename Value, typename Extractor, typename HashFunctio
 template <typename HashTranslator, typename T, typename Extra>
 typename HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Allocator>::AddResult HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Allocator>::addPassingHashCode(T&& key, Extra&& extra)
 {
-    ASSERT(!m_accessForbidden);
     ASSERT(Allocator::isAllocationAllowed());
     if (!m_table)
         expand();
@@ -957,14 +947,7 @@ void HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Allocato
     ++m_stats->numRemoves;
 #endif
 
-    ASSERT(!m_accessForbidden);
-#if ENABLE(ASSERT)
-    m_accessForbidden = true;
-#endif
     deleteBucket(*pos);
-#if ENABLE(ASSERT)
-    m_accessForbidden = false;
-#endif
     ++m_deletedCount;
     --m_keyCount;
 
@@ -1100,15 +1083,7 @@ Value* HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Alloca
     }
     newEntry = rehashTo(originalTable, newTableSize, newEntry);
 
-    ASSERT(!m_accessForbidden);
-#if ENABLE(ASSERT)
-    m_accessForbidden = true;
-#endif
     deleteAllBucketsAndDeallocate(temporaryTable, oldTableSize);
-#if ENABLE(ASSERT)
-    m_accessForbidden = false;
-#endif
-
     return newEntry;
 }
 
@@ -1178,15 +1153,7 @@ Value* HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Alloca
     ValueType* newTable = allocateTable(newTableSize);
     Value* newEntry = rehashTo(newTable, newTableSize, entry);
 
-    ASSERT(!m_accessForbidden);
-#if ENABLE(ASSERT)
-    m_accessForbidden = true;
-#endif
     deleteAllBucketsAndDeallocate(oldTable, oldTableSize);
-#if ENABLE(ASSERT)
-    m_accessForbidden = false;
-#endif
-
     return newEntry;
 }
 
@@ -1197,14 +1164,7 @@ void HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Allocato
     if (!m_table)
         return;
 
-    ASSERT(!m_accessForbidden);
-#if ENABLE(ASSERT)
-    m_accessForbidden = true;
-#endif
     deleteAllBucketsAndDeallocate(m_table, m_tableSize);
-#if ENABLE(ASSERT)
-    m_accessForbidden = false;
-#endif
     m_table = nullptr;
     m_tableSize = 0;
     m_keyCount = 0;
@@ -1218,8 +1178,7 @@ HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Allocator>::H
     , m_deletedCount(0)
     , m_queueFlag(false)
 #if ENABLE(ASSERT)
-    , m_accessForbidden(false)
-    , m_modifications(0)
+    , m_modificationForbidden(0)
 #endif
 #if DUMP_HASHTABLE_STATS_PER_TABLE
     , m_stats(adoptPtr(new Stats(*other.m_stats)))
@@ -1241,8 +1200,7 @@ HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Allocator>::H
     , m_deletedCount(0)
     , m_queueFlag(false)
 #if ENABLE(ASSERT)
-    , m_accessForbidden(false)
-    , m_modifications(0)
+    , m_modificationForbidden(0)
 #endif
 #if DUMP_HASHTABLE_STATS_PER_TABLE
     , m_stats(adoptPtr(new Stats(*other.m_stats)))
@@ -1255,7 +1213,7 @@ HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Allocator>::H
 template <typename Key, typename Value, typename Extractor, typename HashFunctions, typename Traits, typename KeyTraits, typename Allocator>
 void HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Allocator>::swap(HashTable& other)
 {
-    ASSERT(!m_accessForbidden);
+    registerModification();
     std::swap(m_table, other.m_table);
     std::swap(m_tableSize, other.m_tableSize);
     std::swap(m_keyCount, other.m_keyCount);
@@ -1267,7 +1225,7 @@ void HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Allocato
     ASSERT(!other.m_queueFlag);
 
 #if ENABLE(ASSERT)
-    std::swap(m_modifications, other.m_modifications);
+    std::swap(m_modificationForbidden, other.m_modificationForbidden);
 #endif
 
 #if DUMP_HASHTABLE_STATS_PER_TABLE
@@ -1290,6 +1248,45 @@ HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Allocator>& H
     return *this;
 }
 
+#if ENABLE(ASSERT)
+template <typename HashTableType>
+HashTableModificationForbiddenScope<HashTableType>::HashTableModificationForbiddenScope()
+    : m_container(nullptr)
+{
+}
+
+template <typename HashTableType>
+HashTableModificationForbiddenScope<HashTableType>::HashTableModificationForbiddenScope(const HashTableType* container)
+    : m_container(const_cast<HashTableType*>(container))
+{
+    if (m_container)
+        m_container->enterModificationForbiddenScope();
+}
+
+template <typename HashTableType>
+HashTableModificationForbiddenScope<HashTableType>::HashTableModificationForbiddenScope(const HashTableModificationForbiddenScope& other)
+    : m_container(other.m_container)
+{
+    if (m_container)
+        m_container->enterModificationForbiddenScope();
+}
+
+template <typename HashTableType>
+HashTableModificationForbiddenScope<HashTableType>::~HashTableModificationForbiddenScope()
+{
+    leaveScope();
+}
+
+template <typename HashTableType>
+void HashTableModificationForbiddenScope<HashTableType>::leaveScope()
+{
+    if (!m_container)
+        return;
+    m_container->leaveModificationForbiddenScope();
+    m_container = nullptr;
+}
+#endif
+
 template <WeakHandlingFlag weakHandlingFlag, typename Key, typename Value, typename Extractor, typename HashFunctions, typename Traits, typename KeyTraits, typename Allocator>
 struct WeakProcessingHashTableHelper;