Changeset 54265 in webkit


Ignore:
Timestamp:
Feb 2, 2010 5:13:47 PM (14 years ago)
Author:
oliver@apple.com
Message:

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):
Location:
trunk/JavaScriptCore
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/JavaScriptCore/ChangeLog

    r54263 r54265  
     12010-02-02  Oliver Hunt  <oliver@apple.com>
     2
     3        Reviewed by Geoffrey Garen.
     4
     5        Crash in CollectorBitmap::get at nbcolympics.com
     6        https://bugs.webkit.org/show_bug.cgi?id=34504
     7
     8        This was caused by the use of m_offset to determine the offset of
     9        a new property into the property storage.  This patch corrects
     10        the effected cases by incorporating the anonymous slot count. It
     11        also removes the duplicate copy of anonymous slot count from the
     12        property table as keeping this up to date merely increased the
     13        chance of a mismatch.  Finally I've added a large number of
     14        assertions in an attempt to prevent such a bug from happening
     15        again.
     16
     17        With the new assertions in place the existing anonymous slot tests
     18        all fail without the m_offset fixes.
     19
     20        * runtime/PropertyMapHashTable.h:
     21        * runtime/Structure.cpp:
     22        (JSC::Structure::materializePropertyMap):
     23        (JSC::Structure::addPropertyTransitionToExistingStructure):
     24        (JSC::Structure::addPropertyTransition):
     25        (JSC::Structure::removePropertyTransition):
     26        (JSC::Structure::flattenDictionaryStructure):
     27        (JSC::Structure::addPropertyWithoutTransition):
     28        (JSC::Structure::removePropertyWithoutTransition):
     29        (JSC::Structure::copyPropertyTable):
     30        (JSC::Structure::get):
     31        (JSC::Structure::put):
     32        (JSC::Structure::remove):
     33        (JSC::Structure::insertIntoPropertyMapHashTable):
     34        (JSC::Structure::createPropertyMapHashTable):
     35        (JSC::Structure::rehashPropertyMapHashTable):
     36        (JSC::Structure::checkConsistency):
     37
    1382010-02-02  Steve Falkenburg  <sfalken@apple.com>
    239

  • trunk/JavaScriptCore/runtime/PropertyMapHashTable.h

    r48403 r54265  
    6262        unsigned keyCount;
    6363        unsigned deletedSentinelCount;
    64         unsigned anonymousSlotCount;
    6564        unsigned lastIndexUsed;
    6665        Vector<unsigned>* deletedOffsets;

  • trunk/JavaScriptCore/runtime/Structure.cpp

    r54129 r54265  
    273273            rehashPropertyMapHashTable(sizeForKeyCount(m_offset + 1)); // This could be made more efficient by combining with the copy above.
    274274    }
    275    
    276     m_propertyTable->anonymousSlotCount = m_anonymousSlotCount;
     275
    277276    for (ptrdiff_t i = structures.size() - 2; i >= 0; --i) {
    278277        structure = structures[i];
    279278        structure->m_nameInPrevious->ref();
    280         PropertyMapEntry entry(structure->m_nameInPrevious.get(), structure->m_offset, structure->m_attributesInPrevious, structure->m_specificValueInPrevious, ++m_propertyTable->lastIndexUsed);
     279        PropertyMapEntry entry(structure->m_nameInPrevious.get(), m_anonymousSlotCount + structure->m_offset, structure->m_attributesInPrevious, structure->m_specificValueInPrevious, ++m_propertyTable->lastIndexUsed);
    281280        insertIntoPropertyMapHashTable(entry);
    282281    }
    … …  
    344343    if (Structure* existingTransition = structure->table.get(make_pair(propertyName.ustring().rep(), attributes), specificValue)) {
    345344        ASSERT(existingTransition->m_offset != noOffset);
    346         offset = existingTransition->m_offset;
     345        offset = existingTransition->m_offset + existingTransition->m_anonymousSlotCount;
     346        ASSERT(offset >= structure->m_anonymousSlotCount);
     347        ASSERT(structure->m_anonymousSlotCount == existingTransition->m_anonymousSlotCount);
    347348        return existingTransition;
    348349    }
    … …  
    364365        ASSERT(structure != transition);
    365366        offset = transition->put(propertyName, attributes, specificValue);
     367        ASSERT(offset >= structure->m_anonymousSlotCount);
     368        ASSERT(structure->m_anonymousSlotCount == transition->m_anonymousSlotCount);
    366369        if (transition->propertyStorageSize() > transition->propertyStorageCapacity())
    367370            transition->growPropertyStorageCapacity();
    … …  
    382385
    383386    if (structure->m_propertyTable) {
    384         ASSERT(structure->m_propertyTable->anonymousSlotCount == structure->m_anonymousSlotCount);
    385387        if (structure->m_isPinnedPropertyTable)
    386388            transition->m_propertyTable = structure->copyPropertyTable();
    … …  
    397399
    398400    offset = transition->put(propertyName, attributes, specificValue);
     401    ASSERT(offset >= structure->m_anonymousSlotCount);
     402    ASSERT(structure->m_anonymousSlotCount == transition->m_anonymousSlotCount);
    399403    if (transition->propertyStorageSize() > transition->propertyStorageCapacity())
    400404        transition->growPropertyStorageCapacity();
    401405
    402     transition->m_offset = offset;
     406    transition->m_offset = offset - structure->m_anonymousSlotCount;
    403407    ASSERT(structure->anonymousSlotCount() == transition->anonymousSlotCount());
    404408    structure->table.add(make_pair(propertyName.ustring().rep(), attributes), transition.get(), specificValue);
    … …  
    413417
    414418    offset = transition->remove(propertyName);
     419    ASSERT(offset >= structure->m_anonymousSlotCount);
     420    ASSERT(structure->m_anonymousSlotCount == transition->m_anonymousSlotCount);
    415421
    416422    return transition.release();
    … …  
    530536        // reorder the storage, so we have to copy the current values out
    531537        Vector<JSValue> values(propertyCount);
    532         ASSERT(m_propertyTable->anonymousSlotCount == m_anonymousSlotCount);
    533         unsigned anonymousSlotCount = m_propertyTable->anonymousSlotCount;
     538        unsigned anonymousSlotCount = m_anonymousSlotCount;
    534539        for (unsigned i = 0; i < propertyCount; i++) {
    535540            PropertyMapEntry* entry = sortedPropertyEntries[i];
    … …  
    566571
    567572    size_t offset = put(propertyName, attributes, specificValue);
     573    ASSERT(offset >= m_anonymousSlotCount);
    568574    if (propertyStorageSize() > propertyStorageCapacity())
    569575        growPropertyStorageCapacity();
    … …  
    580586    m_isPinnedPropertyTable = true;
    581587    size_t offset = remove(propertyName);
     588    ASSERT(offset >= m_anonymousSlotCount);
    582589    return offset;
    583590}
    … …  
    621628    if (!m_propertyTable)
    622629        return 0;
    623    
    624     ASSERT(m_propertyTable->anonymousSlotCount == m_anonymousSlotCount);
     630
    625631    size_t tableSize = PropertyMapHashTable::allocationSize(m_propertyTable->size);
    626632    PropertyMapHashTable* newTable = static_cast<PropertyMapHashTable*>(fastMalloc(tableSize));
    … …  
    637643        newTable->deletedOffsets = new Vector<unsigned>(*m_propertyTable->deletedOffsets);
    638644
    639     newTable->anonymousSlotCount = m_propertyTable->anonymousSlotCount;
    640645    return newTable;
    641646}
    … …  
    660665        attributes = m_propertyTable->entries()[entryIndex - 1].attributes;
    661666        specificValue = m_propertyTable->entries()[entryIndex - 1].specificValue;
     667        ASSERT(m_propertyTable->entries()[entryIndex - 1].offset >= m_anonymousSlotCount);
    662668        return m_propertyTable->entries()[entryIndex - 1].offset;
    663669    }
    … …  
    683689            attributes = m_propertyTable->entries()[entryIndex - 1].attributes;
    684690            specificValue = m_propertyTable->entries()[entryIndex - 1].specificValue;
     691            ASSERT(m_propertyTable->entries()[entryIndex - 1].offset >= m_anonymousSlotCount);
    685692            return m_propertyTable->entries()[entryIndex - 1].offset;
    686693        }
    … …  
    764771    if (!m_propertyTable)
    765772        createPropertyMapHashTable();
    766     ASSERT(m_propertyTable->anonymousSlotCount == m_anonymousSlotCount);
    767773
    768774    // FIXME: Consider a fast case for tables with no deleted sentinels.
    … …  
    832838        m_propertyTable->deletedOffsets->removeLast();
    833839    } else
    834         newOffset = m_propertyTable->keyCount + m_propertyTable->anonymousSlotCount;
     840        newOffset = m_propertyTable->keyCount + m_anonymousSlotCount;
    835841    m_propertyTable->entries()[entryIndex - 1].offset = newOffset;
    836 
     842   
     843    ASSERT(newOffset >= m_anonymousSlotCount);
    837844    ++m_propertyTable->keyCount;
    838845
    … …  
    859866    if (!m_propertyTable)
    860867        return notFound;
    861    
    862     ASSERT(m_propertyTable->anonymousSlotCount == m_anonymousSlotCount);
     868
    863869#if DUMP_PROPERTYMAP_STATS
    864870    ++numProbes;
    … …  
    899905
    900906    size_t offset = m_propertyTable->entries()[entryIndex - 1].offset;
     907    ASSERT(offset >= m_anonymousSlotCount);
    901908
    902909    key->deref();
    … …  
    924931{
    925932    ASSERT(m_propertyTable);
    926    
    927     ASSERT(m_propertyTable->anonymousSlotCount == m_anonymousSlotCount);
     933    ASSERT(entry.offset >= m_anonymousSlotCount);
    928934    unsigned i = entry.key->existingHash();
    929935    unsigned k = 0;
    … …  
    975981    m_propertyTable->size = newTableSize;
    976982    m_propertyTable->sizeMask = newTableSize - 1;
    977     m_propertyTable->anonymousSlotCount = m_anonymousSlotCount;
    978983
    979984    checkConsistency();
    … …  
    10051010    m_propertyTable->size = newTableSize;
    10061011    m_propertyTable->sizeMask = newTableSize - 1;
    1007     m_propertyTable->anonymousSlotCount = oldTable->anonymousSlotCount;
    10081012
    10091013    unsigned lastIndexUsed = 0;
    … …  
    11351139        ASSERT(m_hasNonEnumerableProperties || !(m_propertyTable->entries()[c].attributes & DontEnum));
    11361140        UString::Rep* rep = m_propertyTable->entries()[c].key;
     1141        ASSERT(m_propertyTable->entries()[c].offset >= m_anonymousSlotCount);
    11371142        if (!rep)
    11381143            continue;

  • trunk/JavaScriptCore/runtime/Structure.h

    r54141 r54265  
    205205
    206206        uint32_t m_propertyStorageCapacity;
     207
     208        // m_offset does not account for anonymous slots
    207209        signed char m_offset;
    208210
Note: See TracChangeset for help on using the changeset viewer.