Title: [283556] trunk
Revision
283556
Author
[email protected]
Date
2021-10-05 10:49:50 -0700 (Tue, 05 Oct 2021)

Log Message

[JSC] JSPropertyNameEnumerator should not have cached prototype chain since empty JSPropertyNameEnumerator is shared
https://bugs.webkit.org/show_bug.cgi?id=231202
JSTests:

Reviewed by Keith Miller.

* stress/for-in-validation-poly-proto.js: Added.
(shouldBe):
(test):
(factory.Test):
(factory):
* stress/for-in-validation-watchpoint.js: Added.
(shouldBe):
(test):
(Test):
(factory):

Source/_javascript_Core:

rdar://83815122

Reviewed by Keith Miller.

r282014 assumed an invariant that JSPropertyNameEnumerator's StructureChain is immutable.
This invariant is also used in validation of JSPropertyNameEnumerator. However, this
invariant was broken since we now have shared empty sentinel JSPropertyNameEnumerator, which can
be used for different structures having different prototype chain.

Since now we have shared JSPropertyNameEnumerator, JSPropertyNameEnumerator should not have
StructureChain in its member. When invalidating StructureChain in Structure, we also clear
cached JSPropertyNameEnumerator so that we do not get a stale JSPropertyNameEnumerator from
Structure even though watchpoint-based validation is not used.

This patch also removes ValidatedViaWatchpoint flag in JSPropertyNameEnumerator due to the same
reason. We should not modify JSPropertyNameEnumerator once it is instantiated. Instead, we encode
this flag as a lowest bit of m_cachedPropertyNameEnumerator. If it is validated via traversing (not watchpoints),
then this bit is set. So when loading that pointer from StructureRareData, we can quickly detect
it without even accessing to the enumerator. This fixes the issue, and it is even cleaner.
We rename m_cachedPropertyNameEnumerator to m_cachedPropertyNameEnumeratorAndFlag since it now
includes this flag.

While reviewing the code, we also found that watchpoint-based validation didn't care about PolyProto.
We should disable watchpoint-based validation if PolyProto is used.

* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileGetPropertyEnumerator):
* ftl/FTLAbstractHeapRepository.h:
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileCompareStrictEq):
* jit/JITPropertyAccess.cpp:
(JSC::JIT::emit_op_get_property_enumerator):
* llint/LLIntSlowPaths.cpp:
(JSC::LLInt::LLINT_SLOW_PATH_DECL):
* llint/LowLevelInterpreter64.asm:
* runtime/JSPropertyNameEnumerator.cpp:
(JSC::JSPropertyNameEnumerator::visitChildrenImpl):
* runtime/JSPropertyNameEnumerator.h:
(JSC::propertyNameEnumerator):
* runtime/Structure.cpp:
(JSC::Structure::visitChildrenImpl):
(JSC::Structure::setCachedPropertyNameEnumerator):
(JSC::Structure::cachedPropertyNameEnumeratorAndFlag const):
* runtime/Structure.h:
(JSC::Structure::propertyNameEnumeratorShouldWatch const):
* runtime/StructureInlines.h:
(JSC::Structure::prototypeChain const):
(JSC::Structure::clearCachedPrototypeChain):
* runtime/StructureRareData.cpp:
(JSC::StructureRareData::visitChildrenImpl):
* runtime/StructureRareData.h:
* runtime/StructureRareDataInlines.h:
(JSC::StructureRareData::cachedPropertyNameEnumerator const):
(JSC::StructureRareData::cachedPropertyNameEnumeratorAndFlag const):
(JSC::StructureRareData::setCachedPropertyNameEnumerator):
(JSC::StructureChainInvalidationWatchpoint::fireInternal):
(JSC::StructureRareData::tryCachePropertyNameEnumeratorViaWatchpoint):
(JSC::StructureRareData::clearCachedPropertyNameEnumerator):
(JSC::StructureRareData::invalidateWatchpointBasedValidation): Deleted.

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (283555 => 283556)


--- trunk/JSTests/ChangeLog	2021-10-05 16:57:27 UTC (rev 283555)
+++ trunk/JSTests/ChangeLog	2021-10-05 17:49:50 UTC (rev 283556)
@@ -1,3 +1,21 @@
+2021-10-05  Yusuke Suzuki  <[email protected]>
+
+        [JSC] JSPropertyNameEnumerator should not have cached prototype chain since empty JSPropertyNameEnumerator is shared
+        https://bugs.webkit.org/show_bug.cgi?id=231202
+
+        Reviewed by Keith Miller.
+
+        * stress/for-in-validation-poly-proto.js: Added.
+        (shouldBe):
+        (test):
+        (factory.Test):
+        (factory):
+        * stress/for-in-validation-watchpoint.js: Added.
+        (shouldBe):
+        (test):
+        (Test):
+        (factory):
+
 2021-10-04  Saam Barati  <[email protected]>
 
         IntrinsicGetterAccessCase implementation of __proto__ needs to handle get_by_id_with_this

Added: trunk/JSTests/stress/for-in-validation-poly-proto.js (0 => 283556)


--- trunk/JSTests/stress/for-in-validation-poly-proto.js	                        (rev 0)
+++ trunk/JSTests/stress/for-in-validation-poly-proto.js	2021-10-05 17:49:50 UTC (rev 283556)
@@ -0,0 +1,34 @@
+//@ requireOptions("--forcePolyProto=1")
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function test(object) {
+    var result = []
+    for (var k in object)
+        result.push(k);
+    return JSON.stringify(result);
+}
+noInline(test);
+
+var constructors = []
+Object.prototype.hey = 32;
+function factory() {
+    function Test() { }
+    constructors.push(Test);
+    return new Test;
+}
+
+var object = factory();
+shouldBe(test(object), `["hey"]`);
+shouldBe(test(object), `["hey"]`);
+shouldBe(test(object), `["hey"]`);
+var object2 = factory();
+shouldBe(test(object2), `["hey"]`);
+shouldBe(test(object2), `["hey"]`);
+shouldBe(test(object2), `["hey"]`);
+object2.__proto__ = { ok: 33 };
+shouldBe(test(object2), `["ok","hey"]`);
+shouldBe(test(object2), `["ok","hey"]`);
+shouldBe(test(object2), `["ok","hey"]`);

Added: trunk/JSTests/stress/for-in-validation-watchpoint.js (0 => 283556)


--- trunk/JSTests/stress/for-in-validation-watchpoint.js	                        (rev 0)
+++ trunk/JSTests/stress/for-in-validation-watchpoint.js	2021-10-05 17:49:50 UTC (rev 283556)
@@ -0,0 +1,34 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function test(object) {
+    var result = []
+    for (var k in object)
+        result.push(k);
+    return JSON.stringify(result);
+}
+noInline(test);
+
+function Test() { }
+function factory() { return new Test; }
+
+var object = factory();
+shouldBe(test(object), `[]`);
+shouldBe(test(object), `[]`);
+shouldBe(test(object), `[]`);
+object.ok = 42;
+delete object.ok;
+shouldBe(test(object), `[]`);
+shouldBe(test(object), `[]`);
+shouldBe(test(object), `[]`);
+Test.prototype.ok = 42;
+delete Test.prototype.ok;
+shouldBe(test(object), `[]`);
+shouldBe(test(object), `[]`);
+shouldBe(test(object), `[]`);
+Test.prototype.ok = 42;
+test({});
+test({});
+shouldBe(test(object), `["ok"]`);

Modified: trunk/Source/_javascript_Core/ChangeLog (283555 => 283556)


--- trunk/Source/_javascript_Core/ChangeLog	2021-10-05 16:57:27 UTC (rev 283555)
+++ trunk/Source/_javascript_Core/ChangeLog	2021-10-05 17:49:50 UTC (rev 283556)
@@ -1,3 +1,67 @@
+2021-10-05  Yusuke Suzuki  <[email protected]>
+
+        [JSC] JSPropertyNameEnumerator should not have cached prototype chain since empty JSPropertyNameEnumerator is shared
+        https://bugs.webkit.org/show_bug.cgi?id=231202
+        rdar://83815122
+
+        Reviewed by Keith Miller.
+
+        r282014 assumed an invariant that JSPropertyNameEnumerator's StructureChain is immutable.
+        This invariant is also used in validation of JSPropertyNameEnumerator. However, this
+        invariant was broken since we now have shared empty sentinel JSPropertyNameEnumerator, which can
+        be used for different structures having different prototype chain.
+
+        Since now we have shared JSPropertyNameEnumerator, JSPropertyNameEnumerator should not have
+        StructureChain in its member. When invalidating StructureChain in Structure, we also clear
+        cached JSPropertyNameEnumerator so that we do not get a stale JSPropertyNameEnumerator from
+        Structure even though watchpoint-based validation is not used.
+
+        This patch also removes ValidatedViaWatchpoint flag in JSPropertyNameEnumerator due to the same
+        reason. We should not modify JSPropertyNameEnumerator once it is instantiated. Instead, we encode
+        this flag as a lowest bit of m_cachedPropertyNameEnumerator. If it is validated via traversing (not watchpoints),
+        then this bit is set. So when loading that pointer from StructureRareData, we can quickly detect
+        it without even accessing to the enumerator. This fixes the issue, and it is even cleaner.
+        We rename m_cachedPropertyNameEnumerator to m_cachedPropertyNameEnumeratorAndFlag since it now
+        includes this flag.
+
+        While reviewing the code, we also found that watchpoint-based validation didn't care about PolyProto.
+        We should disable watchpoint-based validation if PolyProto is used.
+
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileGetPropertyEnumerator):
+        * ftl/FTLAbstractHeapRepository.h:
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileCompareStrictEq):
+        * jit/JITPropertyAccess.cpp:
+        (JSC::JIT::emit_op_get_property_enumerator):
+        * llint/LLIntSlowPaths.cpp:
+        (JSC::LLInt::LLINT_SLOW_PATH_DECL):
+        * llint/LowLevelInterpreter64.asm:
+        * runtime/JSPropertyNameEnumerator.cpp:
+        (JSC::JSPropertyNameEnumerator::visitChildrenImpl):
+        * runtime/JSPropertyNameEnumerator.h:
+        (JSC::propertyNameEnumerator):
+        * runtime/Structure.cpp:
+        (JSC::Structure::visitChildrenImpl):
+        (JSC::Structure::setCachedPropertyNameEnumerator):
+        (JSC::Structure::cachedPropertyNameEnumeratorAndFlag const):
+        * runtime/Structure.h:
+        (JSC::Structure::propertyNameEnumeratorShouldWatch const):
+        * runtime/StructureInlines.h:
+        (JSC::Structure::prototypeChain const):
+        (JSC::Structure::clearCachedPrototypeChain):
+        * runtime/StructureRareData.cpp:
+        (JSC::StructureRareData::visitChildrenImpl):
+        * runtime/StructureRareData.h:
+        * runtime/StructureRareDataInlines.h:
+        (JSC::StructureRareData::cachedPropertyNameEnumerator const):
+        (JSC::StructureRareData::cachedPropertyNameEnumeratorAndFlag const):
+        (JSC::StructureRareData::setCachedPropertyNameEnumerator):
+        (JSC::StructureChainInvalidationWatchpoint::fireInternal):
+        (JSC::StructureRareData::tryCachePropertyNameEnumeratorViaWatchpoint):
+        (JSC::StructureRareData::clearCachedPropertyNameEnumerator):
+        (JSC::StructureRareData::invalidateWatchpointBasedValidation): Deleted.
+
 2021-10-04  Saam Barati  <[email protected]>
 
         Display return values in nicer way in the jsc REPL and add a prettyPrint function

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (283555 => 283556)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2021-10-05 16:57:27 UTC (rev 283555)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2021-10-05 17:49:50 UTC (rev 283556)
@@ -13954,7 +13954,7 @@
         if (rareData) {
             FrozenValue* frozenRareData = m_graph.freeze(rareData);
             m_jit.move(TrustedImmPtr(frozenRareData), scratch1GPR);
-            m_jit.loadPtr(CCallHelpers::Address(scratch1GPR, StructureRareData::offsetOfCachedPropertyNameEnumerator()), scratch1GPR);
+            m_jit.loadPtr(CCallHelpers::Address(scratch1GPR, StructureRareData::offsetOfCachedPropertyNameEnumeratorAndFlag()), scratch1GPR);
         } else {
             if (onlyStructure)
                 m_jit.move(TrustedImmPtr(onlyStructure), scratch1GPR);
@@ -13963,11 +13963,11 @@
             m_jit.loadPtr(CCallHelpers::Address(scratch1GPR, Structure::previousOrRareDataOffset()), scratch1GPR);
             slowCases.append(m_jit.branchTestPtr(CCallHelpers::Zero, scratch1GPR));
             slowCases.append(m_jit.branchIfStructure(scratch1GPR));
-            m_jit.loadPtr(CCallHelpers::Address(scratch1GPR, StructureRareData::offsetOfCachedPropertyNameEnumerator()), scratch1GPR);
+            m_jit.loadPtr(CCallHelpers::Address(scratch1GPR, StructureRareData::offsetOfCachedPropertyNameEnumeratorAndFlag()), scratch1GPR);
         }
 
         slowCases.append(m_jit.branchTestPtr(CCallHelpers::Zero, scratch1GPR));
-        slowCases.append(m_jit.branchTest32(CCallHelpers::Zero, CCallHelpers::Address(scratch1GPR, JSPropertyNameEnumerator::flagsOffset()), CCallHelpers::TrustedImm32(JSPropertyNameEnumerator::ValidatedViaWatchpoint)));
+        slowCases.append(m_jit.branchTestPtr(CCallHelpers::NonZero, scratch1GPR, CCallHelpers::TrustedImm32(StructureRareData::cachedPropertyNameEnumeratorIsValidatedViaTraversingFlag)));
         doneCases.append(m_jit.jump());
 
         slowCases.link(&m_jit);

Modified: trunk/Source/_javascript_Core/ftl/FTLAbstractHeapRepository.h (283555 => 283556)


--- trunk/Source/_javascript_Core/ftl/FTLAbstractHeapRepository.h	2021-10-05 16:57:27 UTC (rev 283555)
+++ trunk/Source/_javascript_Core/ftl/FTLAbstractHeapRepository.h	2021-10-05 17:49:50 UTC (rev 283556)
@@ -110,7 +110,6 @@
     macro(JSObject_butterfly, JSObject::butterflyOffset()) \
     macro(JSPropertyNameEnumerator_cachedInlineCapacity, JSPropertyNameEnumerator::cachedInlineCapacityOffset()) \
     macro(JSPropertyNameEnumerator_cachedPropertyNamesVector, JSPropertyNameEnumerator::cachedPropertyNamesVectorOffset()) \
-    macro(JSPropertyNameEnumerator_cachedPrototypeChain, JSPropertyNameEnumerator::offsetOfCachedPrototypeChain()) \
     macro(JSPropertyNameEnumerator_cachedStructureID, JSPropertyNameEnumerator::cachedStructureIDOffset()) \
     macro(JSPropertyNameEnumerator_endGenericPropertyIndex, JSPropertyNameEnumerator::endGenericPropertyIndexOffset()) \
     macro(JSPropertyNameEnumerator_endStructurePropertyIndex, JSPropertyNameEnumerator::endStructurePropertyIndexOffset()) \
@@ -153,7 +152,7 @@
     macro(Structure_structureID, Structure::structureIDOffset()) \
     macro(StructureRareData_cachedKeys, StructureRareData::offsetOfCachedPropertyNames(CachedPropertyNamesKind::Keys)) \
     macro(StructureRareData_cachedGetOwnPropertyNames, StructureRareData::offsetOfCachedPropertyNames(CachedPropertyNamesKind::GetOwnPropertyNames)) \
-    macro(StructureRareData_cachedPropertyNameEnumerator, StructureRareData::offsetOfCachedPropertyNameEnumerator()) \
+    macro(StructureRareData_cachedPropertyNameEnumeratorAndFlag, StructureRareData::offsetOfCachedPropertyNameEnumeratorAndFlag()) \
     macro(HashMapImpl_capacity, HashMapImpl<HashMapBucket<HashMapBucketDataKey>>::offsetOfCapacity()) \
     macro(HashMapImpl_buffer,  HashMapImpl<HashMapBucket<HashMapBucketDataKey>>::offsetOfBuffer()) \
     macro(HashMapImpl_head,  HashMapImpl<HashMapBucket<HashMapBucketDataKey>>::offsetOfHead()) \

Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (283555 => 283556)


--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2021-10-05 16:57:27 UTC (rev 283555)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2021-10-05 17:49:50 UTC (rev 283556)
@@ -13413,12 +13413,12 @@
             m_out.branch(isRareData, unsure(rareDataCase), unsure(genericCase));
 
             m_out.appendTo(rareDataCase, validationCase);
-            LValue cached = m_out.loadPtr(previousOrRareData, m_heaps.StructureRareData_cachedPropertyNameEnumerator);
-            m_out.branch(m_out.notNull(cached), unsure(validationCase), unsure(genericCase));
+            LValue cachedAndFlag = m_out.loadPtr(previousOrRareData, m_heaps.StructureRareData_cachedPropertyNameEnumeratorAndFlag);
+            m_out.branch(m_out.notNull(cachedAndFlag), unsure(validationCase), unsure(genericCase));
 
             m_out.appendTo(validationCase, genericCase);
-            results.append(m_out.anchor(cached));
-            m_out.branch(m_out.testNonZero32(m_out.load32(cached, m_heaps.JSPropertyNameEnumerator_flags), m_out.constInt32(JSPropertyNameEnumerator::ValidatedViaWatchpoint)), unsure(continuation), unsure(genericCase));
+            results.append(m_out.anchor(cachedAndFlag));
+            m_out.branch(m_out.testIsZeroPtr(cachedAndFlag, m_out.constIntPtr(StructureRareData::cachedPropertyNameEnumeratorIsValidatedViaTraversingFlag)), unsure(continuation), unsure(genericCase));
 
             m_out.appendTo(genericCase, continuation);
             results.append(m_out.anchor(vmCall(pointerType(), operationGetPropertyEnumeratorCell, weakPointer(globalObject), base)));

Modified: trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp (283555 => 283556)


--- trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp	2021-10-05 16:57:27 UTC (rev 283555)
+++ trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp	2021-10-05 17:49:50 UTC (rev 283556)
@@ -2902,10 +2902,10 @@
     loadPtr(Address(regT1, Structure::previousOrRareDataOffset()), regT1);
     genericCases.append(branchTestPtr(Zero, regT1));
     genericCases.append(branchIfStructure(regT1));
-    loadPtr(Address(regT1, StructureRareData::offsetOfCachedPropertyNameEnumerator()), regT1);
+    loadPtr(Address(regT1, StructureRareData::offsetOfCachedPropertyNameEnumeratorAndFlag()), regT1);
 
     genericCases.append(branchTestPtr(Zero, regT1));
-    genericCases.append(branchTest32(Zero, Address(regT1, JSPropertyNameEnumerator::flagsOffset()), TrustedImm32(JSPropertyNameEnumerator::ValidatedViaWatchpoint)));
+    genericCases.append(branchTestPtr(NonZero, regT1, TrustedImm32(StructureRareData::cachedPropertyNameEnumeratorIsValidatedViaTraversingFlag)));
     emitPutVirtualRegister(dst, regT1);
     doneCases.append(jump());
 

Modified: trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp (283555 => 283556)


--- trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp	2021-10-05 16:57:27 UTC (rev 283555)
+++ trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp	2021-10-05 17:49:50 UTC (rev 283556)
@@ -974,7 +974,7 @@
                             metadata.m_offset = slot.cachedOffset();
                             metadata.m_newStructureID = newStructure->id();
                             if (!(bytecode.m_flags.isDirect())) {
-                                StructureChain* chain = newStructure->prototypeChain(globalObject, asObject(baseCell));
+                                StructureChain* chain = newStructure->prototypeChain(vm, globalObject, asObject(baseCell));
                                 ASSERT(chain);
                                 metadata.m_structureChain.set(vm, codeBlock, chain);
                             }

Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm (283555 => 283556)


--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm	2021-10-05 16:57:27 UTC (rev 283555)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm	2021-10-05 17:49:50 UTC (rev 283556)
@@ -3119,9 +3119,9 @@
     btpz t1, .slowPath
     bbeq JSCell::m_type[t1], StructureType, .slowPath
 
-    loadp StructureRareData::m_cachedPropertyNameEnumerator[t1], t1
+    loadp StructureRareData::m_cachedPropertyNameEnumeratorAndFlag[t1], t1
     btpz t1, .slowPath
-    btiz JSPropertyNameEnumerator::m_flags[t1], (constexpr JSPropertyNameEnumerator::ValidatedViaWatchpoint), .slowPath
+    btpnz t1, (constexpr StructureRareData::cachedPropertyNameEnumeratorIsValidatedViaTraversingFlag), .slowPath
 
     return(t1)
 

Modified: trunk/Source/_javascript_Core/runtime/JSPropertyNameEnumerator.cpp (283555 => 283556)


--- trunk/Source/_javascript_Core/runtime/JSPropertyNameEnumerator.cpp	2021-10-05 16:57:27 UTC (rev 283555)
+++ trunk/Source/_javascript_Core/runtime/JSPropertyNameEnumerator.cpp	2021-10-05 17:49:50 UTC (rev 283556)
@@ -86,7 +86,6 @@
         visitor.markAuxiliary(propertyNames);
         visitor.append(propertyNames, propertyNames + thisObject->sizeOfPropertyNames());
     }
-    visitor.append(thisObject->m_prototypeChain);
 
     if (thisObject->cachedStructureID()) {
         VM& vm = visitor.vm();

Modified: trunk/Source/_javascript_Core/runtime/JSPropertyNameEnumerator.h (283555 => 283556)


--- trunk/Source/_javascript_Core/runtime/JSPropertyNameEnumerator.h	2021-10-05 16:57:27 UTC (rev 283555)
+++ trunk/Source/_javascript_Core/runtime/JSPropertyNameEnumerator.h	2021-10-05 17:49:50 UTC (rev 283556)
@@ -44,8 +44,6 @@
         GenericMode = 1 << 2,
         // Profiling Only
         HasSeenOwnStructureModeStructureMismatch = 1 << 3,
-        // JSPropertyNameEnumerator Only
-        ValidatedViaWatchpoint = 1 << 4,
     };
 
     static constexpr uint8_t enumerationModeMask = (GenericMode << 1) - 1;
@@ -72,10 +70,6 @@
         return m_propertyNames.get()[index].get();
     }
 
-    StructureChain* cachedPrototypeChain() const { return m_prototypeChain.get(); }
-    void setCachedPrototypeChain(VM& vm, StructureChain* prototypeChain) { return m_prototypeChain.set(vm, this, prototypeChain); }
-    static ptrdiff_t offsetOfCachedPrototypeChain() { return OBJECT_OFFSETOF(JSPropertyNameEnumerator, m_prototypeChain); }
-
     Structure* cachedStructure(VM& vm) const
     {
         if (!m_cachedStructureID)
@@ -100,15 +94,6 @@
         return OBJECT_OFFSETOF(JSPropertyNameEnumerator, m_propertyNames);
     }
 
-    bool validatedViaWatchpoint() const { return m_flags & JSPropertyNameEnumerator::ValidatedViaWatchpoint; }
-    void setValidatedViaWatchpoint(bool validatedViaWatchpoint)
-    {
-        if (validatedViaWatchpoint)
-            m_flags |= JSPropertyNameEnumerator::ValidatedViaWatchpoint;
-        else
-            m_flags &= ~JSPropertyNameEnumerator::ValidatedViaWatchpoint;
-    }
-
     JSString* computeNext(JSGlobalObject*, JSObject* base, uint32_t& currentIndex, Flag&, bool shouldAllocateIndexedNameString = true);
 
     DECLARE_VISIT_CHILDREN;
@@ -119,8 +104,9 @@
     JSPropertyNameEnumerator(VM&, Structure*, uint32_t, uint32_t, WriteBarrier<JSString>*, unsigned);
     void finishCreation(VM&, RefPtr<PropertyNameArrayData>&&);
 
+    // JSPropertyNameEnumerator is immutable data structure, which allows VM to cache the empty one.
+    // After instantiating JSPropertyNameEnumerator, we must not change any fields.
     AuxiliaryBarrier<WriteBarrier<JSString>*> m_propertyNames;
-    WriteBarrier<StructureChain> m_prototypeChain;
     StructureID m_cachedStructureID;
     uint32_t m_indexedLength;
     uint32_t m_endStructurePropertyIndex;
@@ -138,13 +124,16 @@
 
     uint32_t indexedLength = base->getEnumerableLength(globalObject);
 
-    JSPropertyNameEnumerator* enumerator = nullptr;
-
     Structure* structure = base->structure(vm);
-    if (!indexedLength
-        && (enumerator = structure->cachedPropertyNameEnumerator())) {
-        if (enumerator->validatedViaWatchpoint() || enumerator->cachedPrototypeChain() == structure->prototypeChain(globalObject, base))
-            return enumerator;
+    if (!indexedLength) {
+        uintptr_t enumeratorAndFlag = structure->cachedPropertyNameEnumeratorAndFlag();
+        if (enumeratorAndFlag) {
+            if (!(enumeratorAndFlag & StructureRareData::cachedPropertyNameEnumeratorIsValidatedViaTraversingFlag))
+                return bitwise_cast<JSPropertyNameEnumerator*>(enumeratorAndFlag);
+            structure->prototypeChain(vm, globalObject, base); // Refresh cached structure chain.
+            if (auto* enumerator = structure->cachedPropertyNameEnumerator())
+                return enumerator;
+        }
     }
 
     uint32_t numberStructureProperties = 0;
@@ -163,14 +152,15 @@
         numberStructureProperties = 0;
     }
 
+    JSPropertyNameEnumerator* enumerator = nullptr;
     if (!indexedLength && !propertyNames.size())
         enumerator = vm.emptyPropertyNameEnumerator();
     else
         enumerator = JSPropertyNameEnumerator::create(vm, structureAfterGettingPropertyNames, indexedLength, numberStructureProperties, WTFMove(propertyNames));
     if (!indexedLength && successfullyNormalizedChain && structureAfterGettingPropertyNames == structure) {
-        enumerator->setCachedPrototypeChain(vm, structure->prototypeChain(globalObject, base));
+        StructureChain* chain = structure->prototypeChain(vm, globalObject, base);
         if (structure->canCachePropertyNameEnumerator(vm))
-            structure->setCachedPropertyNameEnumerator(vm, enumerator);
+            structure->setCachedPropertyNameEnumerator(vm, enumerator, chain);
     }
     return enumerator;
 }

Modified: trunk/Source/_javascript_Core/runtime/Structure.cpp (283555 => 283556)


--- trunk/Source/_javascript_Core/runtime/Structure.cpp	2021-10-05 16:57:27 UTC (rev 283555)
+++ trunk/Source/_javascript_Core/runtime/Structure.cpp	2021-10-05 17:49:50 UTC (rev 283556)
@@ -1229,9 +1229,15 @@
     ConcurrentJSLocker locker(thisObject->m_lock);
     
     visitor.append(thisObject->m_globalObject);
-    if (!thisObject->isObject())
+    if (!thisObject->isObject()) {
+        // We do not need to clear JSPropertyNameEnumerator since it is never cached for non-object Structure.
+        // We do not have code clearing JSPropertyNameEnumerator since this function can be called concurrently.
         thisObject->m_cachedPrototypeChain.clear();
-    else {
+#if ASSERT_ENABLED
+        if (auto* rareData = thisObject->tryRareData())
+            ASSERT(!rareData->cachedPropertyNameEnumerator());
+#endif
+    } else {
         visitor.append(thisObject->m_prototype);
         visitor.append(thisObject->m_cachedPrototypeChain);
     }
@@ -1398,12 +1404,14 @@
     return false;
 }
 
-void Structure::setCachedPropertyNameEnumerator(VM& vm, JSPropertyNameEnumerator* enumerator)
+void Structure::setCachedPropertyNameEnumerator(VM& vm, JSPropertyNameEnumerator* enumerator, StructureChain* chain)
 {
+    ASSERT(typeInfo().isObject());
     ASSERT(!isDictionary());
     if (!hasRareData())
         allocateRareData(vm);
-    rareData()->setCachedPropertyNameEnumerator(vm, enumerator);
+    ASSERT(chain == m_cachedPrototypeChain.get());
+    rareData()->setCachedPropertyNameEnumerator(vm, this, enumerator, chain);
 }
 
 JSPropertyNameEnumerator* Structure::cachedPropertyNameEnumerator() const
@@ -1413,6 +1421,13 @@
     return rareData()->cachedPropertyNameEnumerator();
 }
 
+uintptr_t Structure::cachedPropertyNameEnumeratorAndFlag() const
+{
+    if (!hasRareData())
+        return 0;
+    return rareData()->cachedPropertyNameEnumeratorAndFlag();
+}
+
 bool Structure::canCachePropertyNameEnumerator(VM& vm) const
 {
     if (!this->canCacheOwnPropertyNames())

Modified: trunk/Source/_javascript_Core/runtime/Structure.h (283555 => 283556)


--- trunk/Source/_javascript_Core/runtime/Structure.h	2021-10-05 16:57:27 UTC (rev 283555)
+++ trunk/Source/_javascript_Core/runtime/Structure.h	2021-10-05 17:49:50 UTC (rev 283556)
@@ -307,7 +307,6 @@
     JSValue prototypeForLookup(JSGlobalObject*) const;
     JSValue prototypeForLookup(JSGlobalObject*, JSCell* base) const;
     StructureChain* prototypeChain(VM&, JSGlobalObject*, JSObject* base) const;
-    StructureChain* prototypeChain(JSGlobalObject*, JSObject* base) const;
     DECLARE_VISIT_CHILDREN;
     
     // A Structure is cheap to mark during GC if doing so would only add a small and bounded amount
@@ -530,8 +529,9 @@
             setHasReadOnlyOrGetterSetterPropertiesExcludingProto(true);
     }
 
-    void setCachedPropertyNameEnumerator(VM&, JSPropertyNameEnumerator*);
+    void setCachedPropertyNameEnumerator(VM&, JSPropertyNameEnumerator*, StructureChain*);
     JSPropertyNameEnumerator* cachedPropertyNameEnumerator() const;
+    uintptr_t cachedPropertyNameEnumeratorAndFlag() const;
     bool canCachePropertyNameEnumerator(VM&) const;
     bool canAccessPropertiesQuicklyForEnumeration() const;
 
@@ -637,7 +637,7 @@
 
     bool propertyNameEnumeratorShouldWatch() const
     {
-        return dfgShouldWatch();
+        return dfgShouldWatch() && !hasPolyProto();
     }
         
     void addTransitionWatchpoint(Watchpoint* watchpoint) const
@@ -849,6 +849,8 @@
     
     void startWatchingInternalProperties(VM&);
 
+    void clearCachedPrototypeChain();
+
     static constexpr int s_maxTransitionLength = 64;
     static constexpr int s_maxTransitionLengthForNonEvalPutById = 512;
 

Modified: trunk/Source/_javascript_Core/runtime/StructureInlines.h (283555 => 283556)


--- trunk/Source/_javascript_Core/runtime/StructureInlines.h	2021-10-05 16:57:27 UTC (rev 283555)
+++ trunk/Source/_javascript_Core/runtime/StructureInlines.h	2021-10-05 17:49:50 UTC (rev 283556)
@@ -342,16 +342,12 @@
     // We cache our prototype chain so our clients can share it.
     if (!isValid(globalObject, m_cachedPrototypeChain.get(), base)) {
         JSValue prototype = prototypeForLookup(globalObject, base);
+        const_cast<Structure*>(this)->clearCachedPrototypeChain();
         m_cachedPrototypeChain.set(vm, this, StructureChain::create(vm, prototype.isNull() ? nullptr : asObject(prototype)));
     }
     return m_cachedPrototypeChain.get();
 }
 
-inline StructureChain* Structure::prototypeChain(JSGlobalObject* globalObject, JSObject* base) const
-{
-    return prototypeChain(globalObject->vm(), globalObject, base);
-}
-
 inline bool Structure::isValid(JSGlobalObject* globalObject, StructureChain* cachedPrototypeChain, JSObject* base) const
 {
     if (!cachedPrototypeChain)
@@ -759,4 +755,12 @@
     return map()->get(StructureTransitionTable::Hash::Key(rep, attributes, transitionKind));
 }
 
+inline void Structure::clearCachedPrototypeChain()
+{
+    m_cachedPrototypeChain.clear();
+    if (!hasRareData())
+        return;
+    rareData()->clearCachedPropertyNameEnumerator();
+}
+
 } // namespace JSC

Modified: trunk/Source/_javascript_Core/runtime/StructureRareData.cpp (283555 => 283556)


--- trunk/Source/_javascript_Core/runtime/StructureRareData.cpp	2021-10-05 16:57:27 UTC (rev 283555)
+++ trunk/Source/_javascript_Core/runtime/StructureRareData.cpp	2021-10-05 17:49:50 UTC (rev 283556)
@@ -79,7 +79,7 @@
         for (unsigned index = 0; index < numberOfCachedSpecialPropertyKeys; ++index)
             visitor.appendUnbarriered(thisObject->cachedSpecialProperty(static_cast<CachedSpecialPropertyKey>(index)));
     }
-    visitor.append(thisObject->m_cachedPropertyNameEnumerator);
+    visitor.appendUnbarriered(thisObject->cachedPropertyNameEnumerator());
     for (unsigned index = 0; index < numberOfCachedPropertyNames; ++index) {
         auto* cached = thisObject->m_cachedPropertyNames[index].unvalidatedGet();
         if (cached != cachedPropertyNamesSentinel())

Modified: trunk/Source/_javascript_Core/runtime/StructureRareData.h (283555 => 283556)


--- trunk/Source/_javascript_Core/runtime/StructureRareData.h	2021-10-05 16:57:27 UTC (rev 283555)
+++ trunk/Source/_javascript_Core/runtime/StructureRareData.h	2021-10-05 17:49:50 UTC (rev 283556)
@@ -89,7 +89,9 @@
     void cacheSpecialProperty(JSGlobalObject*, VM&, Structure* baseStructure, JSValue, CachedSpecialPropertyKey, const PropertySlot&);
 
     JSPropertyNameEnumerator* cachedPropertyNameEnumerator() const;
-    void setCachedPropertyNameEnumerator(VM&, JSPropertyNameEnumerator*);
+    uintptr_t cachedPropertyNameEnumeratorAndFlag() const;
+    void setCachedPropertyNameEnumerator(VM&, Structure*, JSPropertyNameEnumerator*, StructureChain*);
+    void clearCachedPropertyNameEnumerator();
 
     JSImmutableButterfly* cachedPropertyNames(CachedPropertyNamesKind) const;
     JSImmutableButterfly* cachedPropertyNamesIgnoringSentinel(CachedPropertyNamesKind) const;
@@ -108,9 +110,9 @@
         return OBJECT_OFFSETOF(StructureRareData, m_cachedPropertyNames) + sizeof(WriteBarrier<JSImmutableButterfly>) * static_cast<unsigned>(kind);
     }
 
-    static ptrdiff_t offsetOfCachedPropertyNameEnumerator()
+    static ptrdiff_t offsetOfCachedPropertyNameEnumeratorAndFlag()
     {
-        return OBJECT_OFFSETOF(StructureRareData, m_cachedPropertyNameEnumerator);
+        return OBJECT_OFFSETOF(StructureRareData, m_cachedPropertyNameEnumeratorAndFlag);
     }
 
     DECLARE_EXPORT_INFO;
@@ -117,7 +119,8 @@
 
     void finalizeUnconditionally(VM&);
 
-    void invalidateWatchpointBasedValidation();
+    static constexpr uintptr_t cachedPropertyNameEnumeratorIsValidatedViaTraversingFlag = 1;
+    static constexpr uintptr_t cachedPropertyNameEnumeratorMask = ~static_cast<uintptr_t>(1);
 
 private:
     friend class LLIntOffsetsExtractor;
@@ -135,12 +138,12 @@
     bool canCacheSpecialProperty(CachedSpecialPropertyKey);
     void giveUpOnSpecialPropertyCache(CachedSpecialPropertyKey);
 
-    bool tryCachePropertyNameEnumeratorViaWatchpoint(VM&, StructureChain*);
+    bool tryCachePropertyNameEnumeratorViaWatchpoint(VM&, Structure*, StructureChain*);
 
     WriteBarrier<Structure> m_previous;
     // FIXME: We should have some story for clearing these property names caches in GC.
     // https://bugs.webkit.org/show_bug.cgi?id=192659
-    WriteBarrier<JSPropertyNameEnumerator> m_cachedPropertyNameEnumerator;
+    uintptr_t m_cachedPropertyNameEnumeratorAndFlag { 0 };
     FixedVector<StructureChainInvalidationWatchpoint> m_cachedPropertyNameEnumeratorWatchpoints;
     WriteBarrier<JSImmutableButterfly> m_cachedPropertyNames[numberOfCachedPropertyNames] { };
 

Modified: trunk/Source/_javascript_Core/runtime/StructureRareDataInlines.h (283555 => 283556)


--- trunk/Source/_javascript_Core/runtime/StructureRareDataInlines.h	2021-10-05 16:57:27 UTC (rev 283555)
+++ trunk/Source/_javascript_Core/runtime/StructureRareDataInlines.h	2021-10-05 17:49:50 UTC (rev 283556)
@@ -90,15 +90,20 @@
 
 inline JSPropertyNameEnumerator* StructureRareData::cachedPropertyNameEnumerator() const
 {
-    return m_cachedPropertyNameEnumerator.get();
+    return bitwise_cast<JSPropertyNameEnumerator*>(m_cachedPropertyNameEnumeratorAndFlag & cachedPropertyNameEnumeratorMask);
 }
 
-inline void StructureRareData::setCachedPropertyNameEnumerator(VM& vm, JSPropertyNameEnumerator* enumerator)
+inline uintptr_t StructureRareData::cachedPropertyNameEnumeratorAndFlag() const
 {
-    m_cachedPropertyNameEnumerator.set(vm, this, enumerator);
+    return m_cachedPropertyNameEnumeratorAndFlag;
+}
+
+inline void StructureRareData::setCachedPropertyNameEnumerator(VM& vm, Structure* baseStructure, JSPropertyNameEnumerator* enumerator, StructureChain* chain)
+{
     m_cachedPropertyNameEnumeratorWatchpoints = FixedVector<StructureChainInvalidationWatchpoint>();
-    bool validatedViaWatchpoint = tryCachePropertyNameEnumeratorViaWatchpoint(vm, enumerator->cachedPrototypeChain());
-    enumerator->setValidatedViaWatchpoint(validatedViaWatchpoint);
+    bool validatedViaWatchpoint = tryCachePropertyNameEnumeratorViaWatchpoint(vm, baseStructure, chain);
+    m_cachedPropertyNameEnumeratorAndFlag = ((validatedViaWatchpoint ? 0 : cachedPropertyNameEnumeratorIsValidatedViaTraversingFlag) | bitwise_cast<uintptr_t>(enumerator));
+    vm.heap.writeBarrier(this, enumerator);
 }
 
 inline JSImmutableButterfly* StructureRareData::cachedPropertyNames(CachedPropertyNamesKind kind) const
@@ -169,11 +174,14 @@
 {
     if (!m_structureRareData->isLive())
         return;
-    m_structureRareData->invalidateWatchpointBasedValidation();
+    m_structureRareData->clearCachedPropertyNameEnumerator();
 }
 
-inline bool StructureRareData::tryCachePropertyNameEnumeratorViaWatchpoint(VM& vm, StructureChain* chain)
+inline bool StructureRareData::tryCachePropertyNameEnumeratorViaWatchpoint(VM& vm, Structure* baseStructure, StructureChain* chain)
 {
+    if (baseStructure->hasPolyProto())
+        return false;
+
     unsigned size = 0;
     for (auto* current = chain->head(); *current; ++current) {
         ++size;
@@ -193,9 +201,9 @@
     return true;
 }
 
-inline void StructureRareData::invalidateWatchpointBasedValidation()
+inline void StructureRareData::clearCachedPropertyNameEnumerator()
 {
-    m_cachedPropertyNameEnumerator.clear();
+    m_cachedPropertyNameEnumeratorAndFlag = 0;
     m_cachedPropertyNameEnumeratorWatchpoints = FixedVector<StructureChainInvalidationWatchpoint>();
 }
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to