Changeset 154366 in webkit


Ignore:
Timestamp:
Aug 20, 2013 3:17:29 PM (11 years ago)
Author:
mhahnenberg@apple.com
Message:

<​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):

Location:
trunk/Source/JavaScriptCore
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r154362 r154366  
     12013-08-20  Mark Hahnenberg  <mhahnenberg@apple.com>
     2
     3        <https://webkit.org/b/120079> Flattening a dictionary can cause CopiedSpace corruption
     4
     5        Reviewed by Oliver Hunt.
     6
     7        When we flatten an object in dictionary mode, we compact its properties. If the object
     8        had out-of-line storage in the form of a Butterfly prior to this compaction, and after
     9        compaction its properties fit inline, the object's Structure "forgets" that the object
     10        has a non-zero Butterfly pointer. During GC, we check the Butterfly and reportLiveBytes
     11        with bytes = 0, which causes all sorts of badness in CopiedSpace.
     12
     13        Instead, after we flatten a dictionary, if properties fit inline we should clear the
     14        Butterfly pointer so that the GC doesn't get confused later.
     15
     16        This patch does this clearing, and it also adds JSObject::checkStructure, which overrides
     17        JSCell::checkStructure to add an ASSERT that makes sure that the Structure being assigned
     18        agrees with the whether or not the object has a Butterfly. Also added an ASSERT to check
     19        that the number of bytes reported to SlotVisitor::copyLater is non-zero.
     20
     21        * heap/SlotVisitorInlines.h:
     22        (JSC::SlotVisitor::copyLater):
     23        * runtime/JSObject.cpp:
     24        (JSC::JSObject::notifyPresenceOfIndexedAccessors):
     25        (JSC::JSObject::convertUndecidedToInt32):
     26        (JSC::JSObject::convertUndecidedToDouble):
     27        (JSC::JSObject::convertUndecidedToContiguous):
     28        (JSC::JSObject::convertInt32ToDouble):
     29        (JSC::JSObject::convertInt32ToContiguous):
     30        (JSC::JSObject::genericConvertDoubleToContiguous):
     31        (JSC::JSObject::switchToSlowPutArrayStorage):
     32        (JSC::JSObject::setPrototype):
     33        (JSC::JSObject::putDirectAccessor):
     34        (JSC::JSObject::seal):
     35        (JSC::JSObject::freeze):
     36        (JSC::JSObject::preventExtensions):
     37        (JSC::JSObject::reifyStaticFunctionsForDelete):
     38        (JSC::JSObject::removeDirect):
     39        * runtime/JSObject.h:
     40        (JSC::JSObject::setButterfly):
     41        (JSC::JSObject::putDirectInternal):
     42        (JSC::JSObject::setStructure):
     43        (JSC::JSObject::setStructureAndReallocateStorageIfNecessary):
     44        * runtime/Structure.cpp:
     45        (JSC::Structure::flattenDictionaryStructure):
     46
    1472013-08-20  Alex Christensen  <achristensen@apple.com>
    248

  • trunk/Source/JavaScriptCore/heap/SlotVisitorInlines.h

    r153720 r154366  
    211211inline void SlotVisitor::copyLater(JSCell* owner, CopyToken token, void* ptr, size_t bytes)
    212212{
     213    ASSERT(bytes);
    213214    CopiedBlock* block = CopiedSpace::blockFor(ptr);
    214215    if (block->isOversize()) {

  • trunk/Source/JavaScriptCore/runtime/JSObject.cpp

    r154337 r154366  
    584584        return;
    585585   
    586     setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AddIndexedAccessors));
     586    setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AddIndexedAccessors), m_butterfly);
    587587   
    588588    if (!vm.prototypeMap.isPrototype(this))
    … …  
    670670{
    671671    ASSERT(hasUndecided(structure()->indexingType()));
    672     setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AllocateInt32));
     672    setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AllocateInt32), m_butterfly);
    673673    return m_butterfly->contiguousInt32();
    674674}
    … …  
    681681        m_butterfly->contiguousDouble()[i] = QNaN;
    682682   
    683     setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AllocateDouble));
     683    setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AllocateDouble), m_butterfly);
    684684    return m_butterfly->contiguousDouble();
    685685}
    … …  
    688688{
    689689    ASSERT(hasUndecided(structure()->indexingType()));
    690     setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AllocateContiguous));
     690    setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AllocateContiguous), m_butterfly);
    691691    return m_butterfly->contiguous();
    692692}
    … …  
    754754    }
    755755   
    756     setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AllocateDouble));
     756    setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AllocateDouble), m_butterfly);
    757757    return m_butterfly->contiguousDouble();
    758758}
    … …  
    762762    ASSERT(hasInt32(structure()->indexingType()));
    763763   
    764     setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AllocateContiguous));
     764    setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AllocateContiguous), m_butterfly);
    765765    return m_butterfly->contiguous();
    766766}
    … …  
    820820    }
    821821   
    822     setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AllocateContiguous));
     822    setStructure(vm, Structure::nonPropertyTransition(vm, structure(), AllocateContiguous), m_butterfly);
    823823    return m_butterfly->contiguous();
    824824}
    … …  
    11181118    case ArrayWithArrayStorage: {
    11191119        Structure* newStructure = Structure::nonPropertyTransition(vm, structure(), SwitchToSlowPutArrayStorage);
    1120         setStructure(vm, newStructure);
     1120        setStructure(vm, newStructure, m_butterfly);
    11211121        break;
    11221122    }
    … …  
    11421142   
    11431143    Structure* newStructure = Structure::changePrototypeTransition(vm, structure(), prototype);
    1144     setStructure(vm, newStructure);
     1144    setStructure(vm, newStructure, m_butterfly);
    11451145   
    11461146    if (!newStructure->anyObjectInChainMayInterceptIndexedAccesses())
    … …  
    11991199    // if we override an existing non-getter or non-setter.
    12001200    if (slot.type() != PutPropertySlot::NewProperty)
    1201         setStructure(vm, Structure::attributeChangeTransition(vm, structure(), propertyName, attributes));
     1201        setStructure(vm, Structure::attributeChangeTransition(vm, structure(), propertyName, attributes), m_butterfly);
    12021202
    12031203    if (attributes & ReadOnly)
    … …  
    15561556        return;
    15571557    preventExtensions(vm);
    1558     setStructure(vm, Structure::sealTransition(vm, structure()));
     1558    setStructure(vm, Structure::sealTransition(vm, structure()), m_butterfly);
    15591559}
    15601560
    … …  
    15641564        return;
    15651565    preventExtensions(vm);
    1566     setStructure(vm, Structure::freezeTransition(vm, structure()));
     1566    setStructure(vm, Structure::freezeTransition(vm, structure()), m_butterfly);
    15671567}
    15681568
    … …  
    15711571    enterDictionaryIndexingMode(vm);
    15721572    if (isExtensible())
    1573         setStructure(vm, Structure::preventExtensionsTransition(vm, structure()));
     1573        setStructure(vm, Structure::preventExtensionsTransition(vm, structure()), m_butterfly);
    15741574}
    15751575
    … …  
    15891589
    15901590    if (!structure()->isUncacheableDictionary())
    1591         setStructure(vm, Structure::toUncacheableDictionaryTransition(vm, structure()));
     1591        setStructure(vm, Structure::toUncacheableDictionaryTransition(vm, structure()), m_butterfly);
    15921592
    15931593    for (const ClassInfo* info = classInfo(); info; info = info->parentClass) {
    … …  
    16191619    }
    16201620
    1621     setStructure(vm, Structure::removePropertyTransition(vm, structure(), propertyName, offset));
     1621    setStructure(vm, Structure::removePropertyTransition(vm, structure(), propertyName, offset), m_butterfly);
    16221622    if (offset == invalidOffset)
    16231623        return false;

  • trunk/Source/JavaScriptCore/runtime/JSObject.h

    r154337 r154366  
    611611    void setButterflyWithoutChangingStructure(Butterfly*); // You probably don't want to call this.
    612612       
     613    void setStructure(VM&, Structure*, Butterfly* = 0);
    613614    void setStructureAndReallocateStorageIfNecessary(VM&, unsigned oldCapacity, Structure*);
    614615    void setStructureAndReallocateStorageIfNecessary(VM&, Structure*);
    … …  
    11251126    ASSERT(structure);
    11261127    ASSERT(!butterfly == (!structure->outOfLineCapacity() && !structure->hasIndexingHeader(this)));
    1127     setStructure(vm, structure);
     1128    setStructure(vm, structure, butterfly);
    11281129    m_butterfly = butterfly;
    11291130}
    … …  
    13421343            }
    13431344            // case (2) Despecify, fall through to (3).
    1344             setStructure(vm, Structure::despecifyFunctionTransition(vm, structure(), propertyName));
     1345            setStructure(vm, Structure::despecifyFunctionTransition(vm, structure(), propertyName), m_butterfly);
    13451346        }
    13461347
    … …  
    13701371}
    13711372
     1373inline void JSObject::setStructure(VM& vm, Structure* structure, Butterfly* butterfly)
     1374{
     1375    JSCell::setStructure(vm, structure);
     1376    ASSERT_UNUSED(butterfly, !butterfly == !(structure->outOfLineCapacity() || structure->hasIndexingHeader(this)));
     1377}
     1378
    13721379inline void JSObject::setStructureAndReallocateStorageIfNecessary(VM& vm, unsigned oldCapacity, Structure* newStructure)
    13731380{
    … …  
    13751382   
    13761383    if (oldCapacity == newStructure->outOfLineCapacity()) {
    1377         setStructure(vm, newStructure);
     1384        setStructure(vm, newStructure, m_butterfly);
    13781385        return;
    13791386    }

  • trunk/Source/JavaScriptCore/runtime/Structure.cpp

    r154199 r154366  
    703703
    704704    m_dictionaryKind = NoneDictionaryKind;
     705
     706    // If the object had a Butterfly but after flattening/compacting we no longer have need of it,
     707    // we need to zero it out because the collector depends on the Structure to know the size for copying.
     708    if (object->butterfly() && !this->outOfLineCapacity() && !this->hasIndexingHeader(object))
     709        object->setButterfly(vm, 0, this);
     710
    705711    return this;
    706712}
Note: See TracChangeset for help on using the changeset viewer.