Changeset 194400 in webkit

Timestamp:

Dec 23, 2015, 3:45:17 PM (11 years ago)

Author:

keith_miller@apple.com

Message:

[JSC] Bugfix for intrinsic getters with dictionary structures.
​https://bugs.webkit.org/show_bug.cgi?id=152538

Reviewed by Mark Lam.

Intrinsic getters did not check if an object was a dictionary. This meant, if a property on
the prototype chain of a dictionary was an intrinsic getter we would IC it. Later, if a
property is added to the dictionary the IC would still return the result of the intrinsic.
The fix is to no longer IC intrinsic getters if the base object is a dictionary.

• jit/Repatch.cpp:

(JSC::tryCacheGetByID):

• tests/stress/typedarray-length-dictionary.js: Added.

(len):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• jit/Repatch.cpp (modified) (3 diffs)
• tests/stress/typedarray-length-dictionary.js (added)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2015-12-23  Keith Miller  <keith_miller@apple.com>
+
+        [JSC] Bugfix for intrinsic getters with dictionary structures.
+        https://bugs.webkit.org/show_bug.cgi?id=152538
+
+        Reviewed by Mark Lam.
+
+        Intrinsic getters did not check if an object was a dictionary. This meant, if a property on
+        the prototype chain of a dictionary was an intrinsic getter we would IC it. Later, if a
+        property is added to the dictionary the IC would still return the result of the intrinsic.
+        The fix is to no longer IC intrinsic getters if the base object is a dictionary.
+
+        * jit/Repatch.cpp:
+        (JSC::tryCacheGetByID):
+        * tests/stress/typedarray-length-dictionary.js: Added.
+        (len):
+
 2015-12-23  Andy VanWagoner  <andy@instructure.com>
 

trunk/Source/JavaScriptCore/jit/Repatch.cpp

@@ -225 +225 @@
 
     std::unique_ptr<AccessCase> newCase;
-    JSFunction* getter = nullptr;
-    ObjectPropertyConditionSet conditionSet;
-    JSCell* baseCell = baseValue.asCell();
-    Structure* structure = baseCell->structure(vm);
-
-    if (slot.isCacheableGetter())
-        getter = jsDynamicCast<JSFunction*>(slot.getterSetter()->getter());
 
     if (isJSArray(baseValue) && propertyName == exec->propertyNames().length)
@@ -237 +230 @@
     else if (isJSString(baseValue) && propertyName == exec->propertyNames().length)
         newCase = AccessCase::getLength(vm, codeBlock, AccessCase::StringLength);
-    else if (getter && AccessCase::canEmitIntrinsicGetter(getter, structure)) {
-        if (slot.slotBase() != baseValue) {
-            conditionSet = generateConditionsForPrototypePropertyHit(vm, codeBlock->ownerExecutable(), exec, structure, slot.slotBase(), propertyName.impl());
-            if (!conditionSet.isValid())
-                return GiveUpOnCache;
-        }
-
-        newCase = AccessCase::getIntrinsic(vm, codeBlock, getter, slot.cachedOffset(), structure, conditionSet);
-
-    } else {
+    else {
         if (!slot.isCacheable() && !slot.isUnset())
             return GiveUpOnCache;
+
+        ObjectPropertyConditionSet conditionSet;
+        JSCell* baseCell = baseValue.asCell();
+        Structure* structure = baseCell->structure(vm);
 
         bool loadTargetFromProxy = false;
@@ -298 +286 @@
             if (!conditionSet.isValid())
                 return GiveUpOnCache;
-            
+
             offset = slot.isUnset() ? invalidOffset : conditionSet.slotBaseCondition().offset();
         }
 
-        AccessCase::AccessType type;
-        if (slot.isCacheableValue())
-            type = AccessCase::Load;
-        else if (slot.isUnset())
-            type = AccessCase::Miss;
-        else if (slot.isCacheableGetter())
-            type = AccessCase::Getter;
-        else
-            type = AccessCase::CustomGetter;
-
-        newCase = AccessCase::get(
-            vm, codeBlock, type, offset, structure, conditionSet, loadTargetFromProxy,
-            slot.watchpointSet(), slot.isCacheableCustom() ? slot.customGetter() : nullptr,
-            slot.isCacheableCustom() ? slot.slotBase() : nullptr);
+        JSFunction* getter = nullptr;
+        if (slot.isCacheableGetter())
+            getter = jsDynamicCast<JSFunction*>(slot.getterSetter()->getter());
+
+        if (!loadTargetFromProxy && getter && AccessCase::canEmitIntrinsicGetter(getter, structure))
+            newCase = AccessCase::getIntrinsic(vm, codeBlock, getter, slot.cachedOffset(), structure, conditionSet);
+        else {
+            AccessCase::AccessType type;
+            if (slot.isCacheableValue())
+                type = AccessCase::Load;
+            else if (slot.isUnset())
+                type = AccessCase::Miss;
+            else if (slot.isCacheableGetter())
+                type = AccessCase::Getter;
+            else
+                type = AccessCase::CustomGetter;
+
+            newCase = AccessCase::get(
+                vm, codeBlock, type, offset, structure, conditionSet, loadTargetFromProxy,
+                slot.watchpointSet(), slot.isCacheableCustom() ? slot.customGetter() : nullptr,
+                slot.isCacheableCustom() ? slot.slotBase() : nullptr);
+        }
     }