[SCM] WebKit Debian packaging branch, webkit-1.1, updated. upstream/1.1.19-706-ge5415e9
oliver at apple.com
oliver at apple.com
Thu Feb 4 21:37:27 UTC 2010
The following commit has been merged in the webkit-1.1 branch:
commit 99c30d95781608e8b101a796e8deed9f5fff7f2c
Author: oliver at apple.com <oliver at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Wed Feb 3 01:13:47 2010 +0000
2010-02-02 Oliver Hunt <oliver at 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: http://svn.webkit.org/repository/webkit/trunk@54265 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index 3aecad3..5d2450d 100644
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,40 @@
+2010-02-02 Oliver Hunt <oliver at 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 at apple.com>
Reviewed by Darin Adler.
diff --git a/JavaScriptCore/runtime/PropertyMapHashTable.h b/JavaScriptCore/runtime/PropertyMapHashTable.h
index 5b63f79..44dc2b8 100644
--- a/JavaScriptCore/runtime/PropertyMapHashTable.h
+++ b/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];
diff --git a/JavaScriptCore/runtime/Structure.cpp b/JavaScriptCore/runtime/Structure.cpp
index 86eaebd..546e2bf 100644
--- a/JavaScriptCore/runtime/Structure.cpp
+++ b/JavaScriptCore/runtime/Structure.cpp
@@ -272,12 +272,11 @@ void Structure::materializePropertyMap()
if (sizeForKeyCount(m_offset + 1) > m_propertyTable->size)
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;
}
@@ -620,8 +627,7 @@ 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)
@@ -858,8 +865,7 @@ 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;
diff --git a/JavaScriptCore/runtime/Structure.h b/JavaScriptCore/runtime/Structure.h
index b395448..1353a5a 100644
--- a/JavaScriptCore/runtime/Structure.h
+++ b/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;
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list