Changeset 36081 in webkit for trunk/JavaScriptCore/VM/Machine.cpp

Timestamp:

Sep 4, 2008, 12:21:43 AM (17 years ago)

Author:

[email protected]

Message:

JavaScriptCore:

2008-09-03 Geoffrey Garen <​[email protected]>

Reviewed by Cameron Zwarich.

Fixed <rdar://problem/6193925> REGRESSION: Crash occurs at
KJS::Machine::privateExecute() when attempting to load my Mobile Gallery
(​http://www.me.com/gallery/#home)

also

​https://bugs.webkit.org/show_bug.cgi?id=20633 Crash in privateExecute
@ cs.byu.edu

The underlying problem was that we would cache prototype properties
even if the prototype was a dictionary.

The fix is to transition a prototype back from dictionary to normal
status when an opcode caches access to it. (This is better than just
refusing to cache, since a heavily accessed prototype is almost
certainly not a true dictionary.)

• VM/Machine.cpp: (KJS::Machine::tryCacheGetByID):
• kjs/JSObject.h:

LayoutTests:

2008-09-04 Geoffrey Garen <​[email protected]>

Reviewed by Cameron Zwarich.

Test for <rdar://problem/6193925> REGRESSION: Crash occurs at
KJS::Machine::privateExecute() when attempting to load my Mobile Gallery
(​http://www.me.com/gallery/#home)

also

​https://bugs.webkit.org/show_bug.cgi?id=20633 Crash in privateExecute
@ cs.byu.edu

• fast/js/pic/dictionary-prototype-expected.txt: Added.
• fast/js/pic/dictionary-prototype.html: Added.

File:

• trunk/JavaScriptCore/VM/Machine.cpp (modified) (4 diffs)

trunk/JavaScriptCore/VM/Machine.cpp

@@ -1196 +1196 @@
     }
 
-    JSCell* baseCell = static_cast<JSCell*>(baseValue);
-    StructureID* structureID = baseCell->structureID();
+    StructureID* structureID = static_cast<JSCell*>(baseValue)->structureID();
 
     // FIXME: Remove this !structureID check once all JSCells have StructureIDs.
@@ -1226 +1225 @@
     // Cache hit: Specialize instruction and ref StructureIDs.
 
-    JSValue* slotBase = slot.slotBase();
-    if (slotBase == baseCell) {
+    if (slot.slotBase() == baseValue) {
         vPC[0] = getOpcode(op_get_by_id_self);
         vPC[5] = slot.cachedOffset();
@@ -1235 +1233 @@
     }
 
-    if (slotBase == structureID->prototype()) {
-        ASSERT(!JSImmediate::isImmediate(slotBase));
+    if (slot.slotBase() == structureID->prototype()) {
+        ASSERT(slot.slotBase()->isObject());
+
+        JSObject* slotBaseObject = static_cast<JSObject*>(slot.slotBase());
+
+        // Heavy access to a prototype is a good indication that it's not being
+        // used as a dictionary.
+        if (slotBaseObject->structureID()->isDictionary()) {
+            RefPtr<StructureID> transition = StructureID::fromDictionaryTransition(slotBaseObject->structureID());
+            slotBaseObject->setStructureID(transition.release());
+            static_cast<JSObject*>(baseValue)->structureID()->setCachedPrototypeChain(0);
+        }
 
         vPC[0] = getOpcode(op_get_by_id_proto);
-        vPC[5] = static_cast<JSCell*>(slotBase)->structureID();
+        vPC[5] = slotBaseObject->structureID();
         vPC[6] = slot.cachedOffset();
 
@@ -1247 +1255 @@
 
     size_t count = 0;
-    while (baseCell != slotBase) {
-        baseCell = static_cast<JSCell*>(baseCell->structureID()->prototype());
-        // If we didn't find slotBase in baseCell's prototype chain, then baseCell
+    JSObject* o = static_cast<JSObject*>(baseValue);
+    while (slot.slotBase() != o) {
+        JSValue* v = o->structureID()->prototype();
+
+        // If we didn't find slotBase in baseValue's prototype chain, then baseValue
         // must be a proxy for another object.
-        if (baseCell->isNull()) {
+        if (v->isNull()) {
             vPC[0] = getOpcode(op_get_by_id_generic);
             return;
         }
+
+        o = static_cast<JSObject*>(v);
+
+        // Heavy access to a prototype is a good indication that it's not being
+        // used as a dictionary.
+        if (o->structureID()->isDictionary()) {
+            RefPtr<StructureID> transition = StructureID::fromDictionaryTransition(o->structureID());
+            o->setStructureID(transition.release());
+            static_cast<JSObject*>(baseValue)->structureID()->setCachedPrototypeChain(0);
+        }
+
         ++count;
     }