Changeset 45039 in webkit for trunk/JavaScriptCore

Timestamp:

Jun 23, 2009, 7:47:48 PM (16 years ago)

Author:

[email protected]

Message:

<rdar://problem/6992806> REGRESSION: Enumeration can skip new properties in cases of prototypes that have more than 64 (26593)
<​https://bugs.webkit.org/show_bug.cgi?id=26593>

Reviewed by Gavin Barraclough.

Add tests to ensure we correctly invalidate caching that depends on structure chains
that include dictionaries.

Location:

trunk/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• interpreter/Interpreter.cpp (modified) (3 diffs)
• jit/JITStubs.cpp (modified) (3 diffs)
• runtime/Structure.cpp (modified) (2 diffs)
• runtime/StructureChain.cpp (modified) (1 diff)
• runtime/StructureChain.h (modified) (1 diff)

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2009-06-23  Oliver Hunt  <[email protected]>
+
+        Reviewed by Gavin Barraclough.
+
+        <rdar://problem/6992806> REGRESSION: Enumeration can skip new properties in cases of prototypes that have more than 64 (26593)
+        <https://bugs.webkit.org/show_bug.cgi?id=26593>
+
+        Do not attempt to cache structure chains if they contain a dictionary at any level.
+
+        * interpreter/Interpreter.cpp:
+        (JSC::Interpreter::tryCachePutByID):
+        (JSC::Interpreter::tryCacheGetByID):
+        * jit/JITStubs.cpp:
+        (JSC::JITThunks::tryCachePutByID):
+        * runtime/Structure.cpp:
+        (JSC::Structure::getEnumerablePropertyNames):
+        (JSC::Structure::addPropertyTransition):
+        * runtime/StructureChain.cpp:
+        (JSC::StructureChain::isCacheable):
+        * runtime/StructureChain.h:
+
 2009-06-23  Yong Li  <[email protected]>
 

trunk/JavaScriptCore/interpreter/Interpreter.cpp

@@ -971 +971 @@
     }
 
+    StructureChain* protoChain = structure->prototypeChain(callFrame);
+    if (!protoChain->isCacheable()) {
+        vPC[0] = getOpcode(op_put_by_id_generic);
+        return;
+    }
+
     // Structure transition, cache transition info
     if (slot.type() == PutPropertySlot::NewProperty) {
@@ -976 +982 @@
         vPC[4] = structure->previousID();
         vPC[5] = structure;
-        vPC[6] = structure->prototypeChain(callFrame);
+        vPC[6] = protoChain;
         vPC[7] = slot.cachedOffset();
         codeBlock->refStructures(vPC);
@@ -1078 +1084 @@
     }
 
+    StructureChain* protoChain = structure->prototypeChain(callFrame);
+    if (!protoChain->isCacheable()) {
+        vPC[0] = getOpcode(op_put_by_id_generic);
+        return;
+    }
+
     vPC[0] = getOpcode(op_get_by_id_chain);
     vPC[4] = structure;
-    vPC[5] = structure->prototypeChain(callFrame);
+    vPC[5] = protoChain;
     vPC[6] = count;
     vPC[7] = slot.cachedOffset();

trunk/JavaScriptCore/jit/JITStubs.cpp

@@ -384 +384 @@
     if (slot.type() == PutPropertySlot::NewProperty) {
         StructureChain* prototypeChain = structure->prototypeChain(callFrame);
+        if (!prototypeChain->isCacheable()) {
+            ctiPatchCallByReturnAddress(returnAddress, FunctionPtr(JITStubs::cti_op_put_by_id_generic));
+            return;
+        }
         stubInfo->initPutByIdTransition(structure->previousID(), structure, prototypeChain);
         JIT::compilePutByIdTransition(callFrame->scopeChain()->globalData, codeBlock, stubInfo, structure->previousID(), structure, slot.cachedOffset(), prototypeChain, returnAddress);
@@ -471 +475 @@
 
     StructureChain* prototypeChain = structure->prototypeChain(callFrame);
+    if (!prototypeChain->isCacheable()) {
+        ctiPatchCallByReturnAddress(returnAddress, FunctionPtr(JITStubs::cti_op_get_by_id_generic));
+        return;
+    }
     stubInfo->initGetByIdChain(structure, prototypeChain);
     JIT::compileGetByIdChain(callFrame->scopeChain()->globalData, callFrame, codeBlock, stubInfo, structure, prototypeChain, count, slot.cachedOffset(), returnAddress);
@@ -1044 +1052 @@
             ctiPatchCallByReturnAddress(STUB_RETURN_ADDRESS, FunctionPtr(cti_op_get_by_id_proto_list_full));
     } else if (size_t count = countPrototypeChainEntriesAndCheckForProxies(callFrame, baseValue, slot)) {
+        StructureChain* protoChain = structure->prototypeChain(callFrame);
+        if (!protoChain->isCacheable()) {
+            ctiPatchCallByReturnAddress(STUB_RETURN_ADDRESS, FunctionPtr(cti_op_get_by_id_proto_fail));
+            return JSValue::encode(result);
+        }
+        
         int listIndex;
         PolymorphicAccessStructureList* prototypeStructureList = getPolymorphicAccessStructureListSlot(stubInfo, listIndex);
-        JIT::compileGetByIdChainList(callFrame->scopeChain()->globalData, callFrame, codeBlock, stubInfo, prototypeStructureList, listIndex, structure, structure->prototypeChain(callFrame), count, slot.cachedOffset());
+        JIT::compileGetByIdChainList(callFrame->scopeChain()->globalData, callFrame, codeBlock, stubInfo, prototypeStructureList, listIndex, structure, protoChain, count, slot.cachedOffset());
 
         if (listIndex == (POLYMORPHIC_LIST_CACHE_SIZE - 1))

trunk/JavaScriptCore/runtime/Structure.cpp

@@ -307 +307 @@
 
     if (shouldCache) {
+        StructureChain* protoChain = prototypeChain(exec);
         m_cachedPropertyNameArrayData = propertyNames.data();
-        m_cachedPropertyNameArrayData->setCachedPrototypeChain(prototypeChain(exec));
+        if (!protoChain->isCacheable())
+            return;
+        m_cachedPropertyNameArrayData->setCachedPrototypeChain(protoChain);
         m_cachedPropertyNameArrayData->setCachedStructure(this);
     }
@@ -408 +411 @@
     if (structure->transitionCount() > s_maxTransitionLength) {
         RefPtr<Structure> transition = toDictionaryTransition(structure);
+        ASSERT(structure != transition);
         offset = transition->put(propertyName, attributes, specificValue);
         if (transition->propertyStorageSize() > transition->propertyStorageCapacity())

trunk/JavaScriptCore/runtime/StructureChain.cpp

@@ -47 +47 @@
 }
 
+bool StructureChain::isCacheable() const
+{
+    uint32_t i = 0;
+    
+    while (m_vector[i]) {
+        if (m_vector[i++]->isDictionary())
+            return false;
+    }
+    return true;
+}
+
 } // namespace JSC

trunk/JavaScriptCore/runtime/StructureChain.h

@@ -40 +40 @@
         static PassRefPtr<StructureChain> create(Structure* head) { return adoptRef(new StructureChain(head)); }
         RefPtr<Structure>* head() { return m_vector.get(); }
+        bool isCacheable() const;
 
     private: