2010-02-02 Oliver Hunt <oliver@apple.com>

        Reviewed by Geoffrey Garen.

        Crash in CollectorBitmap::get at nbcolympics.com
        https://bugs.webkit.org/show_bug.cgi?id=34504

        This was caused by the use of m_offset to determine the offset of
        a new property into the property storage.  This patch corrects
        the effected cases by incorporating the anonymous slot count. It
        also removes the duplicate copy of anonymous slot count from the
        property table as keeping this up to date merely increased the
        chance of a mismatch.  Finally I've added a large number of
        assertions in an attempt to prevent such a bug from happening
        again.

        With the new assertions in place the existing anonymous slot tests
        all fail without the m_offset fixes.

        * runtime/PropertyMapHashTable.h:
        * runtime/Structure.cpp:
        (JSC::Structure::materializePropertyMap):
        (JSC::Structure::addPropertyTransitionToExistingStructure):
        (JSC::Structure::addPropertyTransition):
        (JSC::Structure::removePropertyTransition):
        (JSC::Structure::flattenDictionaryStructure):
        (JSC::Structure::addPropertyWithoutTransition):
        (JSC::Structure::removePropertyWithoutTransition):
        (JSC::Structure::copyPropertyTable):
        (JSC::Structure::get):
        (JSC::Structure::put):
        (JSC::Structure::remove):
        (JSC::Structure::insertIntoPropertyMapHashTable):
        (JSC::Structure::createPropertyMapHashTable):
        (JSC::Structure::rehashPropertyMapHashTable):
        (JSC::Structure::checkConsistency):

git-svn-id: svn://svn.chromium.org/blink/trunk@54265 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/JavaScriptCore/ChangeLog

+2010-02-02  Oliver Hunt  <oliver@apple.com>
+
+        Reviewed by Geoffrey Garen.
+
+        Crash in CollectorBitmap::get at nbcolympics.com
+        https://bugs.webkit.org/show_bug.cgi?id=34504
+
+        This was caused by the use of m_offset to determine the offset of
+        a new property into the property storage.  This patch corrects
+        the effected cases by incorporating the anonymous slot count. It
+        also removes the duplicate copy of anonymous slot count from the
+        property table as keeping this up to date merely increased the
+        chance of a mismatch.  Finally I've added a large number of
+        assertions in an attempt to prevent such a bug from happening
+        again.
+
+        With the new assertions in place the existing anonymous slot tests
+        all fail without the m_offset fixes.
+
+        * runtime/PropertyMapHashTable.h:
+        * runtime/Structure.cpp:
+        (JSC::Structure::materializePropertyMap):
+        (JSC::Structure::addPropertyTransitionToExistingStructure):
+        (JSC::Structure::addPropertyTransition):
+        (JSC::Structure::removePropertyTransition):
+        (JSC::Structure::flattenDictionaryStructure):
+        (JSC::Structure::addPropertyWithoutTransition):
+        (JSC::Structure::removePropertyWithoutTransition):
+        (JSC::Structure::copyPropertyTable):
+        (JSC::Structure::get):
+        (JSC::Structure::put):
+        (JSC::Structure::remove):
+        (JSC::Structure::insertIntoPropertyMapHashTable):
+        (JSC::Structure::createPropertyMapHashTable):
+        (JSC::Structure::rehashPropertyMapHashTable):
+        (JSC::Structure::checkConsistency):
+
 2010-02-02  Steve Falkenburg  <sfalken@apple.com>
 
         Reviewed by Darin Adler.

third_party/WebKit/JavaScriptCore/runtime/PropertyMapHashTable.h

@@ -61,7 +61,6 @@ namespace JSC {
         unsigned size;
         unsigned keyCount;
         unsigned deletedSentinelCount;
-        unsigned anonymousSlotCount;
         unsigned lastIndexUsed;
         Vector<unsigned>* deletedOffsets;
         unsigned entryIndices[1];

third_party/WebKit/JavaScriptCore/runtime/Structure.cpp

@@ -273,11 +273,10 @@ void Structure::materializePropertyMap()
             rehashPropertyMapHashTable(sizeForKeyCount(m_offset + 1)); // This could be made more efficient by combining with the copy above. 
     }
 
-    m_propertyTable->anonymousSlotCount = m_anonymousSlotCount;
     for (ptrdiff_t i = structures.size() - 2; i >= 0; --i) {
         structure = structures[i];
         structure->m_nameInPrevious->ref();
-        PropertyMapEntry entry(structure->m_nameInPrevious.get(), structure->m_offset, structure->m_attributesInPrevious, structure->m_specificValueInPrevious, ++m_propertyTable->lastIndexUsed);
+        PropertyMapEntry entry(structure->m_nameInPrevious.get(), m_anonymousSlotCount + structure->m_offset, structure->m_attributesInPrevious, structure->m_specificValueInPrevious, ++m_propertyTable->lastIndexUsed);
         insertIntoPropertyMapHashTable(entry);
     }
 }
@@ -343,7 +342,9 @@ PassRefPtr<Structure> Structure::addPropertyTransitionToExistingStructure(Struct
 
     if (Structure* existingTransition = structure->table.get(make_pair(propertyName.ustring().rep(), attributes), specificValue)) {
         ASSERT(existingTransition->m_offset != noOffset);
-        offset = existingTransition->m_offset;
+        offset = existingTransition->m_offset + existingTransition->m_anonymousSlotCount;
+        ASSERT(offset >= structure->m_anonymousSlotCount);
+        ASSERT(structure->m_anonymousSlotCount == existingTransition->m_anonymousSlotCount);
         return existingTransition;
     }
 
@@ -363,6 +364,8 @@ PassRefPtr<Structure> Structure::addPropertyTransition(Structure* structure, con
         RefPtr<Structure> transition = toCacheableDictionaryTransition(structure);
         ASSERT(structure != transition);
         offset = transition->put(propertyName, attributes, specificValue);
+        ASSERT(offset >= structure->m_anonymousSlotCount);
+        ASSERT(structure->m_anonymousSlotCount == transition->m_anonymousSlotCount);
         if (transition->propertyStorageSize() > transition->propertyStorageCapacity())
             transition->growPropertyStorageCapacity();
         return transition.release();
@@ -381,7 +384,6 @@ PassRefPtr<Structure> Structure::addPropertyTransition(Structure* structure, con
     transition->m_specificFunctionThrashCount = structure->m_specificFunctionThrashCount;
 
     if (structure->m_propertyTable) {
-        ASSERT(structure->m_propertyTable->anonymousSlotCount == structure->m_anonymousSlotCount);
         if (structure->m_isPinnedPropertyTable)
             transition->m_propertyTable = structure->copyPropertyTable();
         else {
@@ -396,10 +398,12 @@ PassRefPtr<Structure> Structure::addPropertyTransition(Structure* structure, con
     }
 
     offset = transition->put(propertyName, attributes, specificValue);
+    ASSERT(offset >= structure->m_anonymousSlotCount);
+    ASSERT(structure->m_anonymousSlotCount == transition->m_anonymousSlotCount);
     if (transition->propertyStorageSize() > transition->propertyStorageCapacity())
         transition->growPropertyStorageCapacity();
 
-    transition->m_offset = offset;
+    transition->m_offset = offset - structure->m_anonymousSlotCount;
     ASSERT(structure->anonymousSlotCount() == transition->anonymousSlotCount());
     structure->table.add(make_pair(propertyName.ustring().rep(), attributes), transition.get(), specificValue);
     return transition.release();
@@ -412,6 +416,8 @@ PassRefPtr<Structure> Structure::removePropertyTransition(Structure* structure,
     RefPtr<Structure> transition = toUncacheableDictionaryTransition(structure);
 
     offset = transition->remove(propertyName);
+    ASSERT(offset >= structure->m_anonymousSlotCount);
+    ASSERT(structure->m_anonymousSlotCount == transition->m_anonymousSlotCount);
 
     return transition.release();
 }
@@ -529,8 +535,7 @@ PassRefPtr<Structure> Structure::flattenDictionaryStructure(JSObject* object)
         // in the order that they are expected to be in, but we need to
         // reorder the storage, so we have to copy the current values out
         Vector<JSValue> values(propertyCount);
-        ASSERT(m_propertyTable->anonymousSlotCount == m_anonymousSlotCount);
-        unsigned anonymousSlotCount = m_propertyTable->anonymousSlotCount;
+        unsigned anonymousSlotCount = m_anonymousSlotCount;
         for (unsigned i = 0; i < propertyCount; i++) {
             PropertyMapEntry* entry = sortedPropertyEntries[i];
             values[i] = object->getDirectOffset(entry->offset);
@@ -565,6 +570,7 @@ size_t Structure::addPropertyWithoutTransition(const Identifier& propertyName, u
     m_isPinnedPropertyTable = true;
 
     size_t offset = put(propertyName, attributes, specificValue);
+    ASSERT(offset >= m_anonymousSlotCount);
     if (propertyStorageSize() > propertyStorageCapacity())
         growPropertyStorageCapacity();
     return offset;
@@ -579,6 +585,7 @@ size_t Structure::removePropertyWithoutTransition(const Identifier& propertyName
 
     m_isPinnedPropertyTable = true;
     size_t offset = remove(propertyName);
+    ASSERT(offset >= m_anonymousSlotCount);
     return offset;
 }
 
@@ -621,7 +628,6 @@ PropertyMapHashTable* Structure::copyPropertyTable()
     if (!m_propertyTable)
         return 0;
 
-    ASSERT(m_propertyTable->anonymousSlotCount == m_anonymousSlotCount);
     size_t tableSize = PropertyMapHashTable::allocationSize(m_propertyTable->size);
     PropertyMapHashTable* newTable = static_cast<PropertyMapHashTable*>(fastMalloc(tableSize));
     memcpy(newTable, m_propertyTable, tableSize);
@@ -636,7 +642,6 @@ PropertyMapHashTable* Structure::copyPropertyTable()
     if (m_propertyTable->deletedOffsets)
         newTable->deletedOffsets = new Vector<unsigned>(*m_propertyTable->deletedOffsets);
 
-    newTable->anonymousSlotCount = m_propertyTable->anonymousSlotCount;
     return newTable;
 }
 
@@ -659,6 +664,7 @@ size_t Structure::get(const UString::Rep* rep, unsigned& attributes, JSCell*& sp
     if (rep == m_propertyTable->entries()[entryIndex - 1].key) {
         attributes = m_propertyTable->entries()[entryIndex - 1].attributes;
         specificValue = m_propertyTable->entries()[entryIndex - 1].specificValue;
+        ASSERT(m_propertyTable->entries()[entryIndex - 1].offset >= m_anonymousSlotCount);
         return m_propertyTable->entries()[entryIndex - 1].offset;
     }
 
@@ -682,6 +688,7 @@ size_t Structure::get(const UString::Rep* rep, unsigned& attributes, JSCell*& sp
         if (rep == m_propertyTable->entries()[entryIndex - 1].key) {
             attributes = m_propertyTable->entries()[entryIndex - 1].attributes;
             specificValue = m_propertyTable->entries()[entryIndex - 1].specificValue;
+            ASSERT(m_propertyTable->entries()[entryIndex - 1].offset >= m_anonymousSlotCount);
             return m_propertyTable->entries()[entryIndex - 1].offset;
         }
     }
@@ -763,7 +770,6 @@ size_t Structure::put(const Identifier& propertyName, unsigned attributes, JSCel
 
     if (!m_propertyTable)
         createPropertyMapHashTable();
-    ASSERT(m_propertyTable->anonymousSlotCount == m_anonymousSlotCount);
 
     // FIXME: Consider a fast case for tables with no deleted sentinels.
 
@@ -831,9 +837,10 @@ size_t Structure::put(const Identifier& propertyName, unsigned attributes, JSCel
         newOffset = m_propertyTable->deletedOffsets->last();
         m_propertyTable->deletedOffsets->removeLast();
     } else
-        newOffset = m_propertyTable->keyCount + m_propertyTable->anonymousSlotCount;
+        newOffset = m_propertyTable->keyCount + m_anonymousSlotCount;
     m_propertyTable->entries()[entryIndex - 1].offset = newOffset;
     
+    ASSERT(newOffset >= m_anonymousSlotCount);
     ++m_propertyTable->keyCount;
 
     if ((m_propertyTable->keyCount + m_propertyTable->deletedSentinelCount) * 2 >= m_propertyTable->size)
@@ -859,7 +866,6 @@ size_t Structure::remove(const Identifier& propertyName)
     if (!m_propertyTable)
         return notFound;
 
-    ASSERT(m_propertyTable->anonymousSlotCount == m_anonymousSlotCount);
 #if DUMP_PROPERTYMAP_STATS
     ++numProbes;
     ++numRemoves;
@@ -898,6 +904,7 @@ size_t Structure::remove(const Identifier& propertyName)
     m_propertyTable->entryIndices[i & m_propertyTable->sizeMask] = deletedSentinelIndex;
 
     size_t offset = m_propertyTable->entries()[entryIndex - 1].offset;
+    ASSERT(offset >= m_anonymousSlotCount);
 
     key->deref();
     m_propertyTable->entries()[entryIndex - 1].key = 0;
@@ -923,8 +930,7 @@ size_t Structure::remove(const Identifier& propertyName)
 void Structure::insertIntoPropertyMapHashTable(const PropertyMapEntry& entry)
 {
     ASSERT(m_propertyTable);
-    
-    ASSERT(m_propertyTable->anonymousSlotCount == m_anonymousSlotCount);
+    ASSERT(entry.offset >= m_anonymousSlotCount);
     unsigned i = entry.key->existingHash();
     unsigned k = 0;
 
@@ -974,7 +980,6 @@ void Structure::createPropertyMapHashTable(unsigned newTableSize)
     m_propertyTable = static_cast<PropertyMapHashTable*>(fastZeroedMalloc(PropertyMapHashTable::allocationSize(newTableSize)));
     m_propertyTable->size = newTableSize;
     m_propertyTable->sizeMask = newTableSize - 1;
-    m_propertyTable->anonymousSlotCount = m_anonymousSlotCount;
 
     checkConsistency();
 }
@@ -1004,7 +1009,6 @@ void Structure::rehashPropertyMapHashTable(unsigned newTableSize)
     m_propertyTable = static_cast<PropertyMapHashTable*>(fastZeroedMalloc(PropertyMapHashTable::allocationSize(newTableSize)));
     m_propertyTable->size = newTableSize;
     m_propertyTable->sizeMask = newTableSize - 1;
-    m_propertyTable->anonymousSlotCount = oldTable->anonymousSlotCount;
 
     unsigned lastIndexUsed = 0;
     unsigned entryCount = oldTable->keyCount + oldTable->deletedSentinelCount;
@@ -1134,6 +1138,7 @@ void Structure::checkConsistency()
     for (unsigned c = 1; c <= m_propertyTable->keyCount + m_propertyTable->deletedSentinelCount; ++c) {
         ASSERT(m_hasNonEnumerableProperties || !(m_propertyTable->entries()[c].attributes & DontEnum));
         UString::Rep* rep = m_propertyTable->entries()[c].key;
+        ASSERT(m_propertyTable->entries()[c].offset >= m_anonymousSlotCount);
         if (!rep)
             continue;
         ++nonEmptyEntryCount;

third_party/WebKit/JavaScriptCore/runtime/Structure.h

@@ -204,6 +204,8 @@ namespace JSC {
         PropertyMapHashTable* m_propertyTable;
 
         uint32_t m_propertyStorageCapacity;
+
+        // m_offset does not account for anonymous slots
         signed char m_offset;
 
         unsigned m_dictionaryKind : 2;