Attachment #232672 for bug #133606

View Details Raw Unified Return to Bug 133606

| Differences between



2014-06-07  Mark Lam  <mark.lam@apple.com>

        Structure should initialize its previousID in its constructor.
        <https://webkit.org/b/133606>

        Reviewed by NOBODY (OOPS!).

        Currently, the Structure constructor that takes a previous structure will
        initialize its previousID to point to the previous structure's previousID.
        This is incorrect.  However, the caller of the Structure::create() factory
        method (which instantiated the Structure) will later call setPreviousID()
        to set the previousID to the correct previous structure.  This makes the
        code confusing to read and more error prone in that the structure relies
        on client code to fix its invalid previousID.

        This patch fixes this by making the Structure constructor initialize
        previousID correctly.

        * runtime/Structure.cpp:
        (JSC::Structure::Structure):
        (JSC::Structure::addPropertyTransition):
        (JSC::Structure::nonPropertyTransition):
        * runtime/Structure.h:
        * runtime/StructureInlines.h:
        (JSC::Structure::create):

2014-06-06  Andreas Kling  <akling@apple.com>

        Indexed getters should return values directly on the PropertySlot.



    ASSERT(hasGetterSetterProperties() || !m_classInfo->hasStaticSetterOrReadonlyProperties(vm));
}

Structure::Structure(VM& vm, Structure* previous)
    : JSCell(vm, vm.structureStructure.get())
    , m_prototype(vm, this, previous->storedPrototype())
    , m_classInfo(previous->m_classInfo)

    ASSERT(!previous->typeInfo().structureIsImmortal());
    if (previous->typeInfo().structureHasRareData() && previous->rareData()->needsCloning())
        cloneRareDataFrom(vm, previous);
    setPreviousID(vm, this, previous);
        m_previousOrRareData.set(vm, this, previous->previousID());

    previous->notifyTransitionFromThisStructure();
    if (previous->m_globalObject)

    Structure* transition = create(vm, structure);

    transition->m_cachedPrototypeChain.setMayBeNull(vm, transition, structure->m_cachedPrototypeChain.get());
    transition->setPreviousID(vm, transition, structure);
    transition->m_nameInPrevious = propertyName.uid();
    transition->m_attributesInPrevious = attributes;
    transition->m_specificValueInPrevious.setMayBeNull(vm, transition, specificValue);

    }
    
    Structure* transition = create(vm, structure);
    transition->setPreviousID(vm, transition, structure);
    transition->m_attributesInPrevious = attributes;
    transition->m_blob.setIndexingType(indexingType);
    transition->propertyTable().set(vm, transition, structure->takePropertyTableOrCloneIfPinned(vm, transition));




    JS_EXPORT_PRIVATE Structure(VM&, JSGlobalObject*, JSValue prototype, const TypeInfo&, const ClassInfo*, IndexingType, unsigned inlineCapacity);
    Structure(VM&);
    Structure(VM&, Structure*);

    static Structure* create(VM&, Structure*);
    
    static Structure* addPropertyTransitionToExistingStructureImpl(Structure*, StringImpl* uid, unsigned attributes, JSCell* specificValue, PropertyOffset&);


Lines 49-55 inline Structure* Structure::createStruc Source/JavaScriptCore/runtime/StructureInlines.h_sec1
- Source/JavaScriptCore/runtime/StructureInlines.h -1 / +1 lines
49	return structure;	49	return structure;
50	}	50	}
51		51
52	inline Structure* Structure::create(VM& vm, const Structure* structure)	52	inline Structure* Structure::create(VM& vm, Structure* structure)
53	{	53	{
54	ASSERT(vm.structureStructure);	54	ASSERT(vm.structureStructure);
55	Structure* newStructure = new (NotNull, allocateCell<Structure>(vm.heap)) Structure(vm, structure);	55	Structure* newStructure = new (NotNull, allocateCell<Structure>(vm.heap)) Structure(vm, structure);

Return to Bug 133606

Lines 389-397 private: Source/JavaScriptCore/runtime/Structure.h_sec1
- Source/JavaScriptCore/runtime/Structure.h -2 / +2 lines
389		389
390	JS_EXPORT_PRIVATE Structure(VM&, JSGlobalObject, JSValue prototype, const TypeInfo&, const ClassInfo, IndexingType, unsigned inlineCapacity);	390	JS_EXPORT_PRIVATE Structure(VM&, JSGlobalObject, JSValue prototype, const TypeInfo&, const ClassInfo, IndexingType, unsigned inlineCapacity);
391	Structure(VM&);	391	Structure(VM&);
392	Structure(VM&, const Structure*);	392	Structure(VM&, Structure*);
393		393
394	static Structure* create(VM&, const Structure*);	394	static Structure* create(VM&, Structure*);
395		395
396	static Structure* addPropertyTransitionToExistingStructureImpl(Structure, StringImpl uid, unsigned attributes, JSCell* specificValue, PropertyOffset&);	396	static Structure* addPropertyTransitionToExistingStructureImpl(Structure, StringImpl uid, unsigned attributes, JSCell* specificValue, PropertyOffset&);
397		397

Lines 1-3 Source/JavaScriptCore/ChangeLog_sec1
- Source/JavaScriptCore/ChangeLog +26 lines
		1	2014-06-07 Mark Lam <mark.lam@apple.com>
		2
		3	Structure should initialize its previousID in its constructor.
		4	<https://webkit.org/b/133606>
		5
		6	Reviewed by NOBODY (OOPS!).
		7
		8	Currently, the Structure constructor that takes a previous structure will
		9	initialize its previousID to point to the previous structure's previousID.
		10	This is incorrect. However, the caller of the Structure::create() factory
		11	method (which instantiated the Structure) will later call setPreviousID()
		12	to set the previousID to the correct previous structure. This makes the
		13	code confusing to read and more error prone in that the structure relies
		14	on client code to fix its invalid previousID.
		15
		16	This patch fixes this by making the Structure constructor initialize
		17	previousID correctly.
		18
		19	* runtime/Structure.cpp:
		20	(JSC::Structure::Structure):
		21	(JSC::Structure::addPropertyTransition):
		22	(JSC::Structure::nonPropertyTransition):
		23	* runtime/Structure.h:
		24	* runtime/StructureInlines.h:
		25	(JSC::Structure::create):
		26
1	2014-06-06 Andreas Kling <akling@apple.com>	27	2014-06-06 Andreas Kling <akling@apple.com>
2		28
3	Indexed getters should return values directly on the PropertySlot.	29	Indexed getters should return values directly on the PropertySlot.

Lines 211-217 Structure::Structure(VM& vm) Source/JavaScriptCore/runtime/Structure.cpp_sec1
- Source/JavaScriptCore/runtime/Structure.cpp -5 / +2 lines
211	ASSERT(hasGetterSetterProperties() \|\| !m_classInfo->hasStaticSetterOrReadonlyProperties(vm));	211	ASSERT(hasGetterSetterProperties() \|\| !m_classInfo->hasStaticSetterOrReadonlyProperties(vm));
212	}	212	}
213		213
214	Structure::Structure(VM& vm, const Structure* previous)	214	Structure::Structure(VM& vm, Structure* previous)
215	: JSCell(vm, vm.structureStructure.get())	215	: JSCell(vm, vm.structureStructure.get())
216	, m_prototype(vm, this, previous->storedPrototype())	216	, m_prototype(vm, this, previous->storedPrototype())
217	, m_classInfo(previous->m_classInfo)	217	, m_classInfo(previous->m_classInfo)
Lines 236-243 Structure::Structure(VM& vm, const Struc Source/JavaScriptCore/runtime/Structure.cpp_sec2
236	ASSERT(!previous->typeInfo().structureIsImmortal());	236	ASSERT(!previous->typeInfo().structureIsImmortal());
237	if (previous->typeInfo().structureHasRareData() && previous->rareData()->needsCloning())	237	if (previous->typeInfo().structureHasRareData() && previous->rareData()->needsCloning())
238	cloneRareDataFrom(vm, previous);	238	cloneRareDataFrom(vm, previous);
239	else if (previous->previousID())	239	setPreviousID(vm, this, previous);
240	m_previousOrRareData.set(vm, this, previous->previousID());
241		240
242	previous->notifyTransitionFromThisStructure();	241	previous->notifyTransitionFromThisStructure();
243	if (previous->m_globalObject)	242	if (previous->m_globalObject)
Lines 459-465 Structure* Structure::addPropertyTransit Source/JavaScriptCore/runtime/Structure.cpp_sec3
459	Structure* transition = create(vm, structure);	458	Structure* transition = create(vm, structure);
460		459
461	transition->m_cachedPrototypeChain.setMayBeNull(vm, transition, structure->m_cachedPrototypeChain.get());	460	transition->m_cachedPrototypeChain.setMayBeNull(vm, transition, structure->m_cachedPrototypeChain.get());
462	transition->setPreviousID(vm, transition, structure);
463	transition->m_nameInPrevious = propertyName.uid();	461	transition->m_nameInPrevious = propertyName.uid();
464	transition->m_attributesInPrevious = attributes;	462	transition->m_attributesInPrevious = attributes;
465	transition->m_specificValueInPrevious.setMayBeNull(vm, transition, specificValue);	463	transition->m_specificValueInPrevious.setMayBeNull(vm, transition, specificValue);
Lines 672-678 Structure* Structure::nonPropertyTransit Source/JavaScriptCore/runtime/Structure.cpp_sec4
672	}	670	}
673		671
674	Structure* transition = create(vm, structure);	672	Structure* transition = create(vm, structure);
675	transition->setPreviousID(vm, transition, structure);
676	transition->m_attributesInPrevious = attributes;	673	transition->m_attributesInPrevious = attributes;
677	transition->m_blob.setIndexingType(indexingType);	674	transition->m_blob.setIndexingType(indexingType);
678	transition->propertyTable().set(vm, transition, structure->takePropertyTableOrCloneIfPinned(vm, transition));	675	transition->propertyTable().set(vm, transition, structure->takePropertyTableOrCloneIfPinned(vm, transition));