Named lookups on HTML documents produce inconsistent results in JavaScriptCore bindings

https://bugs.webkit.org/show_bug.cgi?id=104623

Reviewed by Geoffrey Garen.

Source/JavaScriptCore:

Add the notion of objects that HasImpureGetOwnPropertySlot, and use that to inhibit prototype chain caching
in some cases. This appears to be perf-neutral on benchmarks that we track.

* dfg/DFGRepatch.cpp:
(JSC::DFG::tryCacheGetByID):
(JSC::DFG::tryBuildGetByIDProtoList):
* jit/JITStubs.cpp:
(JSC::JITThunks::tryCacheGetByID):
(JSC::DEFINE_STUB_FUNCTION):
* runtime/JSTypeInfo.h:
(JSC):
(JSC::TypeInfo::hasImpureGetOwnPropertySlot):
* runtime/Operations.h:
(JSC::normalizePrototypeChainForChainAccess):

Source/WebCore:

All DOM objects that have named getters or directly override getOwnPropertySlot are now marked as
HasImpureGetOwnPropertySlot.

Tests: fast/js/prototype-chain-caching-with-impure-get-own-property-slot-traps
        fast/js/dfg-prototype-chain-caching-with-impure-get-own-property-slot-traps

* bindings/scripts/CodeGeneratorJS.pm:
(GenerateHeader):

LayoutTests:

* fast/js/dfg-prototype-chain-caching-with-impure-get-own-property-slot-traps-expected.txt: Added.
* fast/js/dfg-prototype-chain-caching-with-impure-get-own-property-slot-traps.html: Added.
* fast/js/prototype-chain-caching-with-impure-get-own-property-slot-traps-expected.txt: Added.
* fast/js/prototype-chain-caching-with-impure-get-own-property-slot-traps.html: Added.
* fast/js/script-tests/dfg-prototype-chain-caching-with-impure-get-own-property-slot-traps.js: Added.
(f):
* fast/js/script-tests/prototype-chain-caching-with-impure-get-own-property-slot-traps.js: Added.
(f):

Change-Id: Ie17e39f2b8139778455e28aca9428698f4dd362f
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@137700 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Reviewed-by: Jocelyn Turcotte <jocelyn.turcotte@digia.com>

4 files changed, 28 insertions, 7 deletions

diff --git a/Source/JavaScriptCore/dfg/DFGRepatch.cpp b/Source/JavaScriptCore/dfg/DFGRepatch.cpp
index 07a509061..9a7e7df70 100644
--- a/Source/JavaScriptCore/dfg/DFGRepatch.cpp
+++ b/Source/JavaScriptCore/dfg/DFGRepatch.cpp
@@ -341,7 +341,7 @@ static bool tryCacheGetByID(ExecState* exec, JSValue baseValue, const Identifier
         return false;
     
     PropertyOffset offset = slot.cachedOffset();
-    size_t count = normalizePrototypeChain(exec, baseValue, slot.slotBase(), propertyName, offset);
+    size_t count = normalizePrototypeChainForChainAccess(exec, baseValue, slot.slotBase(), propertyName, offset);
     if (count == InvalidPrototypeChain)
         return false;
 
@@ -569,7 +569,7 @@ static bool tryBuildGetByIDProtoList(ExecState* exec, JSValue baseValue, const I
     ASSERT(slot.slotBase().isObject());
     
     PropertyOffset offset = slot.cachedOffset();
-    size_t count = normalizePrototypeChain(exec, baseValue, slot.slotBase(), propertyName, offset);
+    size_t count = normalizePrototypeChainForChainAccess(exec, baseValue, slot.slotBase(), propertyName, offset);
     if (count == InvalidPrototypeChain)
         return false;
 
diff --git a/Source/JavaScriptCore/jit/JITStubs.cpp b/Source/JavaScriptCore/jit/JITStubs.cpp
index f47ac08ef..fbef1fcb9 100644
--- a/Source/JavaScriptCore/jit/JITStubs.cpp
+++ b/Source/JavaScriptCore/jit/JITStubs.cpp
@@ -908,6 +908,7 @@ NEVER_INLINE void JITThunks::tryCacheGetByID(CallFrame* callFrame, CodeBlock* co
 
     // Uncacheable: give up.
     if (!slot.isCacheable()) {
+        stubInfo->accessType = access_get_by_id_generic;
         ctiPatchCallByReturnAddress(codeBlock, returnAddress, FunctionPtr(cti_op_get_by_id_generic));
         return;
     }
@@ -916,6 +917,7 @@ NEVER_INLINE void JITThunks::tryCacheGetByID(CallFrame* callFrame, CodeBlock* co
     Structure* structure = baseCell->structure();
 
     if (structure->isUncacheableDictionary() || structure->typeInfo().prohibitsPropertyCaching()) {
+        stubInfo->accessType = access_get_by_id_generic;
         ctiPatchCallByReturnAddress(codeBlock, returnAddress, FunctionPtr(cti_op_get_by_id_generic));
         return;
     }
@@ -934,6 +936,7 @@ NEVER_INLINE void JITThunks::tryCacheGetByID(CallFrame* callFrame, CodeBlock* co
     }
 
     if (structure->isDictionary()) {
+        stubInfo->accessType = access_get_by_id_generic;
         ctiPatchCallByReturnAddress(codeBlock, returnAddress, FunctionPtr(cti_op_get_by_id_generic));
         return;
     }
@@ -943,6 +946,12 @@ NEVER_INLINE void JITThunks::tryCacheGetByID(CallFrame* callFrame, CodeBlock* co
         
         JSObject* slotBaseObject = asObject(slot.slotBase());
         size_t offset = slot.cachedOffset();
+
+        if (structure->typeInfo().hasImpureGetOwnPropertySlot()) {
+            stubInfo->accessType = access_get_by_id_generic;
+            ctiPatchCallByReturnAddress(codeBlock, returnAddress, FunctionPtr(cti_op_get_by_id_generic));
+            return;
+        }
         
         // Since we're accessing a prototype in a loop, it's a good bet that it
         // should not be treated as a dictionary.
@@ -960,9 +969,10 @@ NEVER_INLINE void JITThunks::tryCacheGetByID(CallFrame* callFrame, CodeBlock* co
     }
 
     PropertyOffset offset = slot.cachedOffset();
-    size_t count = normalizePrototypeChain(callFrame, baseValue, slot.slotBase(), propertyName, offset);
+    size_t count = normalizePrototypeChainForChainAccess(callFrame, baseValue, slot.slotBase(), propertyName, offset);
     if (count == InvalidPrototypeChain) {
         stubInfo->accessType = access_get_by_id_generic;
+        ctiPatchCallByReturnAddress(codeBlock, returnAddress, FunctionPtr(cti_op_get_by_id_generic));
         return;
     }
 
@@ -1689,6 +1699,12 @@ DEFINE_STUB_FUNCTION(EncodedJSValue, op_get_by_id_proto_list)
         ctiPatchCallByReturnAddress(codeBlock, STUB_RETURN_ADDRESS, FunctionPtr(cti_op_get_by_id_proto_fail));
     else if (slot.slotBase() == baseValue.asCell()->structure()->prototypeForLookup(callFrame)) {
         ASSERT(!baseValue.asCell()->structure()->isDictionary());
+        
+        if (baseValue.asCell()->structure()->typeInfo().hasImpureGetOwnPropertySlot()) {
+            ctiPatchCallByReturnAddress(codeBlock, STUB_RETURN_ADDRESS, FunctionPtr(cti_op_get_by_id_proto_fail));
+            return JSValue::encode(result);
+        }
+        
         // Since we're accessing a prototype in a loop, it's a good bet that it
         // should not be treated as a dictionary.
         if (slotBaseObject->structure()->isDictionary()) {
@@ -1705,7 +1721,7 @@ DEFINE_STUB_FUNCTION(EncodedJSValue, op_get_by_id_proto_list)
                 ctiPatchCallByReturnAddress(codeBlock, STUB_RETURN_ADDRESS, FunctionPtr(cti_op_get_by_id_proto_list_full));
         }
     } else {
-        size_t count = normalizePrototypeChain(callFrame, baseValue, slot.slotBase(), propertyName, offset);
+        size_t count = normalizePrototypeChainForChainAccess(callFrame, baseValue, slot.slotBase(), propertyName, offset);
         if (count == InvalidPrototypeChain) {
             ctiPatchCallByReturnAddress(codeBlock, STUB_RETURN_ADDRESS, FunctionPtr(cti_op_get_by_id_proto_fail));
             return JSValue::encode(result);
diff --git a/Source/JavaScriptCore/runtime/JSTypeInfo.h b/Source/JavaScriptCore/runtime/JSTypeInfo.h
index 6f63260fe..07dd0c9d4 100644
--- a/Source/JavaScriptCore/runtime/JSTypeInfo.h
+++ b/Source/JavaScriptCore/runtime/JSTypeInfo.h
@@ -46,6 +46,7 @@ namespace JSC {
     static const unsigned OverridesVisitChildren = 1 << 7;
     static const unsigned OverridesGetPropertyNames = 1 << 8;
     static const unsigned ProhibitsPropertyCaching = 1 << 9;
+    static const unsigned HasImpureGetOwnPropertySlot = 1 << 10;
 
     class TypeInfo {
     public:
@@ -80,6 +81,7 @@ namespace JSC {
         bool overridesVisitChildren() const { return isSetOnFlags1(OverridesVisitChildren); }
         bool overridesGetPropertyNames() const { return isSetOnFlags2(OverridesGetPropertyNames); }
         bool prohibitsPropertyCaching() const { return isSetOnFlags2(ProhibitsPropertyCaching); }
+        bool hasImpureGetOwnPropertySlot() const { return isSetOnFlags2(HasImpureGetOwnPropertySlot); }
 
         static ptrdiff_t flagsOffset()
         {
diff --git a/Source/JavaScriptCore/runtime/Operations.h b/Source/JavaScriptCore/runtime/Operations.h
index 7301bf6ec..8e0a0a393 100644
--- a/Source/JavaScriptCore/runtime/Operations.h
+++ b/Source/JavaScriptCore/runtime/Operations.h
@@ -302,15 +302,18 @@ namespace JSC {
 
 #define InvalidPrototypeChain (std::numeric_limits<size_t>::max())
 
-    inline size_t normalizePrototypeChain(CallFrame* callFrame, JSValue base, JSValue slotBase, const Identifier& propertyName, PropertyOffset& slotOffset)
+    inline size_t normalizePrototypeChainForChainAccess(CallFrame* callFrame, JSValue base, JSValue slotBase, const Identifier& propertyName, PropertyOffset& slotOffset)
     {
         JSCell* cell = base.asCell();
         size_t count = 0;
-
+        
         while (slotBase != cell) {
             if (cell->isProxy())
                 return InvalidPrototypeChain;
             
+            if (cell->structure()->typeInfo().hasImpureGetOwnPropertySlot())
+                return InvalidPrototypeChain;
+            
             JSValue v = cell->structure()->prototypeForLookup(callFrame);
 
             // If we didn't find slotBase in base's prototype chain, then base
@@ -328,7 +331,7 @@ namespace JSC {
                 if (slotBase == cell)
                     slotOffset = cell->structure()->get(callFrame->globalData(), propertyName); 
             }
-
+            
             ++count;
         }