[webkit-changes] [158124] branches/safari-537.73-branch/Source/JavaScriptCore

Mon, 28 Oct 2013 13:29:05 -0700

Title: [158124] branches/safari-537.73-branch/Source/_javascript_Core

Diff

Modified: branches/safari-537.73-branch/Source/_javascript_Core/ChangeLog (158123 => 158124)


--- branches/safari-537.73-branch/Source/_javascript_Core/ChangeLog	2013-10-28 20:10:56 UTC (rev 158123)
+++ branches/safari-537.73-branch/Source/_javascript_Core/ChangeLog	2013-10-28 20:30:04 UTC (rev 158124)
@@ -1,3 +1,53 @@
+2013-10-28  Lucas Forschler  <[email protected]>
+
+        Merge r154366
+
+    2013-08-20  Mark Hahnenberg  <[email protected]>
+
+            <https://webkit.org/b/120079> Flattening a dictionary can cause CopiedSpace corruption
+
+            Reviewed by Oliver Hunt.
+
+            When we flatten an object in dictionary mode, we compact its properties. If the object 
+            had out-of-line storage in the form of a Butterfly prior to this compaction, and after 
+            compaction its properties fit inline, the object's Structure "forgets" that the object 
+            has a non-zero Butterfly pointer. During GC, we check the Butterfly and reportLiveBytes 
+            with bytes = 0, which causes all sorts of badness in CopiedSpace.
+
+            Instead, after we flatten a dictionary, if properties fit inline we should clear the 
+            Butterfly pointer so that the GC doesn't get confused later.
+
+            This patch does this clearing, and it also adds JSObject::checkStructure, which overrides
+            JSCell::checkStructure to add an ASSERT that makes sure that the Structure being assigned
+            agrees with the whether or not the object has a Butterfly. Also added an ASSERT to check
+            that the number of bytes reported to SlotVisitor::copyLater is non-zero.
+
+            * heap/SlotVisitorInlines.h:
+            (JSC::SlotVisitor::copyLater):
+            * runtime/JSObject.cpp:
+            (JSC::JSObject::notifyPresenceOfIndexedAccessors):
+            (JSC::JSObject::convertUndecidedToInt32):
+            (JSC::JSObject::convertUndecidedToDouble):
+            (JSC::JSObject::convertUndecidedToContiguous):
+            (JSC::JSObject::convertInt32ToDouble):
+            (JSC::JSObject::convertInt32ToContiguous):
+            (JSC::JSObject::genericConvertDoubleToContiguous):
+            (JSC::JSObject::switchToSlowPutArrayStorage):
+            (JSC::JSObject::setPrototype):
+            (JSC::JSObject::putDirectAccessor):
+            (JSC::JSObject::seal):
+            (JSC::JSObject::freeze):
+            (JSC::JSObject::preventExtensions):
+            (JSC::JSObject::reifyStaticFunctionsForDelete):
+            (JSC::JSObject::removeDirect):
+            * runtime/JSObject.h:
+            (JSC::JSObject::setButterfly):
+            (JSC::JSObject::putDirectInternal):
+            (JSC::JSObject::setStructure):
+            (JSC::JSObject::setStructureAndReallocateStorageIfNecessary):
+            * runtime/Structure.cpp:
+            (JSC::Structure::flattenDictionaryStructure):
+
 2013-10-28  Mark Lam  <[email protected]>
 
         Merge r155471.

Modified: branches/safari-537.73-branch/Source/_javascript_Core/heap/SlotVisitorInlines.h (158123 => 158124)


--- branches/safari-537.73-branch/Source/_javascript_Core/heap/SlotVisitorInlines.h	2013-10-28 20:10:56 UTC (rev 158123)
+++ branches/safari-537.73-branch/Source/_javascript_Core/heap/SlotVisitorInlines.h	2013-10-28 20:30:04 UTC (rev 158124)
@@ -174,6 +174,7 @@
 
 inline void SlotVisitor::copyLater(JSCell* owner, void* ptr, size_t bytes)
 {
+    ASSERT(bytes);
     CopiedBlock* block = CopiedSpace::blockFor(ptr);
     if (block->isOversize()) {
         m_shared.m_copiedSpace->pin(block);

Modified: branches/safari-537.73-branch/Source/_javascript_Core/runtime/JSObject.cpp (158123 => 158124)


--- branches/safari-537.73-branch/Source/_javascript_Core/runtime/JSObject.cpp	2013-10-28 20:10:56 UTC (rev 158123)
+++ branches/safari-537.73-branch/Source/_javascript_Core/runtime/JSObject.cpp	2013-10-28 20:30:04 UTC (rev 158124)
@@ -595,7 +595,7 @@
     if (mayInterceptIndexedAccesses())
         return;
     
-    setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AddIndexedAccessors));
+    setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AddIndexedAccessors), m_butterfly);
     
     if (!vm.prototypeMap.isPrototype(this))
         return;
@@ -681,7 +681,7 @@
 ContiguousJSValues JSObject::convertUndecidedToInt32(VM& vm)
 {
     ASSERT(hasUndecided(structure()->indexingType()));
-    setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AllocateInt32));
+    setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AllocateInt32), m_butterfly);
     return m_butterfly->contiguousInt32();
 }
 
@@ -692,14 +692,14 @@
     for (unsigned i = m_butterfly->vectorLength(); i--;)
         m_butterfly->contiguousDouble()[i] = QNaN;
     
-    setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AllocateDouble));
+    setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AllocateDouble), m_butterfly);
     return m_butterfly->contiguousDouble();
 }
 
 ContiguousJSValues JSObject::convertUndecidedToContiguous(VM& vm)
 {
     ASSERT(hasUndecided(structure()->indexingType()));
-    setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AllocateContiguous));
+    setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AllocateContiguous), m_butterfly);
     return m_butterfly->contiguous();
 }
 
@@ -765,7 +765,7 @@
         *currentAsDouble = v.asInt32();
     }
     
-    setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AllocateDouble));
+    setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AllocateDouble), m_butterfly);
     return m_butterfly->contiguousDouble();
 }
 
@@ -773,7 +773,7 @@
 {
     ASSERT(hasInt32(structure()->indexingType()));
     
-    setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AllocateContiguous));
+    setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AllocateContiguous), m_butterfly);
     return m_butterfly->contiguous();
 }
 
@@ -831,7 +831,7 @@
         currentAsValue->setWithoutWriteBarrier(v);
     }
     
-    setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AllocateContiguous));
+    setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AllocateContiguous), m_butterfly);
     return m_butterfly->contiguous();
 }
 
@@ -1129,7 +1129,7 @@
     case NonArrayWithArrayStorage:
     case ArrayWithArrayStorage: {
         Structure* newStructure = Structure::nonPropertyTransition(vm, structure(), SwitchToSlowPutArrayStorage);
-        setStructure(vm, newStructure);
+        setStructure(vm, newStructure, m_butterfly);
         break;
     }
         
@@ -1153,7 +1153,7 @@
         vm.prototypeMap.addPrototype(asObject(prototype));
     
     Structure* newStructure = Structure::changePrototypeTransition(vm, structure(), prototype);
-    setStructure(vm, newStructure);
+    setStructure(vm, newStructure, m_butterfly);
     
     if (!newStructure->anyObjectInChainMayInterceptIndexedAccesses())
         return;
@@ -1213,7 +1213,7 @@
     // getters and setters, though, we also need to change our Structure
     // if we override an existing non-getter or non-setter.
     if (slot.type() != PutPropertySlot::NewProperty)
-        setStructure(vm, Structure::attributeChangeTransition(vm, structure(), propertyName, attributes));
+        setStructure(vm, Structure::attributeChangeTransition(vm, structure(), propertyName, attributes), m_butterfly);
 
     if (attributes & ReadOnly)
         structure()->setContainsReadOnlyProperties();
@@ -1570,7 +1570,7 @@
     if (isSealed(vm))
         return;
     preventExtensions(vm);
-    setStructure(vm, Structure::sealTransition(vm, structure()));
+    setStructure(vm, Structure::sealTransition(vm, structure()), m_butterfly);
 }
 
 void JSObject::freeze(VM& vm)
@@ -1578,14 +1578,14 @@
     if (isFrozen(vm))
         return;
     preventExtensions(vm);
-    setStructure(vm, Structure::freezeTransition(vm, structure()));
+    setStructure(vm, Structure::freezeTransition(vm, structure()), m_butterfly);
 }
 
 void JSObject::preventExtensions(VM& vm)
 {
     enterDictionaryIndexingMode(vm);
     if (isExtensible())
-        setStructure(vm, Structure::preventExtensionsTransition(vm, structure()));
+        setStructure(vm, Structure::preventExtensionsTransition(vm, structure()), m_butterfly);
 }
 
 // This presently will flatten to an uncachable dictionary; this is suitable
@@ -1603,7 +1603,7 @@
     }
 
     if (!structure()->isUncacheableDictionary())
-        setStructure(vm, Structure::toUncacheableDictionaryTransition(vm, structure()));
+        setStructure(vm, Structure::toUncacheableDictionaryTransition(vm, structure()), m_butterfly);
 
     for (const ClassInfo* info = classInfo(); info; info = info->parentClass) {
         const HashTable* hashTable = info->propHashTable(globalObject()->globalExec());
@@ -1633,7 +1633,7 @@
         return true;
     }
 
-    setStructure(vm, Structure::removePropertyTransition(vm, structure(), propertyName, offset));
+    setStructure(vm, Structure::removePropertyTransition(vm, structure(), propertyName, offset), m_butterfly);
     if (offset == invalidOffset)
         return false;
     putDirectUndefined(offset);

Modified: branches/safari-537.73-branch/Source/_javascript_Core/runtime/JSObject.h (158123 => 158124)


--- branches/safari-537.73-branch/Source/_javascript_Core/runtime/JSObject.h	2013-10-28 20:10:56 UTC (rev 158123)
+++ branches/safari-537.73-branch/Source/_javascript_Core/runtime/JSObject.h	2013-10-28 20:30:04 UTC (rev 158124)
@@ -595,6 +595,7 @@
     void setButterfly(VM&, Butterfly*, Structure*);
     void setButterflyWithoutChangingStructure(Butterfly*); // You probably don't want to call this.
         
+    void setStructure(VM&, Structure*, Butterfly* = 0);
     void setStructureAndReallocateStorageIfNecessary(VM&, unsigned oldCapacity, Structure*);
     void setStructureAndReallocateStorageIfNecessary(VM&, Structure*);
 
@@ -1109,7 +1110,7 @@
 {
     ASSERT(structure);
     ASSERT(!butterfly == (!structure->outOfLineCapacity() && !hasIndexingHeader(structure->indexingType())));
-    setStructure(vm, structure);
+    setStructure(vm, structure, butterfly);
     m_butterfly = butterfly;
 }
 
@@ -1316,7 +1317,7 @@
                 return true;
             }
             // case (2) Despecify, fall through to (3).
-            setStructure(vm, Structure::despecifyFunctionTransition(vm, structure(), propertyName));
+            setStructure(vm, Structure::despecifyFunctionTransition(vm, structure(), propertyName), m_butterfly);
         }
 
         // case (3) set the slot, do the put, return.
@@ -1344,12 +1345,18 @@
     return true;
 }
 
+inline void JSObject::setStructure(VM& vm, Structure* structure, Butterfly* butterfly)
+{
+    JSCell::setStructure(vm, structure);
+    ASSERT_UNUSED(butterfly, !butterfly == !(structure->outOfLineCapacity() || structure->hasIndexingHeader(this)));
+}
+
 inline void JSObject::setStructureAndReallocateStorageIfNecessary(VM& vm, unsigned oldCapacity, Structure* newStructure)
 {
     ASSERT(oldCapacity <= newStructure->outOfLineCapacity());
     
     if (oldCapacity == newStructure->outOfLineCapacity()) {
-        setStructure(vm, newStructure);
+        setStructure(vm, newStructure, m_butterfly);
         return;
     }

Modified: branches/safari-537.73-branch/Source/_javascript_Core/runtime/Structure.cpp (158123 => 158124)


--- branches/safari-537.73-branch/Source/_javascript_Core/runtime/Structure.cpp	2013-10-28 20:10:56 UTC (rev 158123)
+++ branches/safari-537.73-branch/Source/_javascript_Core/runtime/Structure.cpp	2013-10-28 20:30:04 UTC (rev 158124)
@@ -649,6 +649,12 @@
     }
 
     m_dictionaryKind = NoneDictionaryKind;
+
+    // If the object had a Butterfly but after flattening/compacting we no longer have need of it,
+    // we need to zero it out because the collector depends on the Structure to know the size for copying.
+    if (object->butterfly() && !this->outOfLineCapacity() && !hasIndexingHeader(this->indexingType()))
+        object->setButterfly(vm, 0, this);
+
     return this;
 }
 

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to