Title: [294619] trunk
Revision
294619
Author
ysuz...@apple.com
Date
2022-05-22 03:00:56 -0700 (Sun, 22 May 2022)

Log Message

Clear StructureCache if it has Structure with relevant JSGlobalObjects
https://bugs.webkit.org/show_bug.cgi?id=240768
rdar://93232129

Reviewed by Saam Barati.

We need to clear Structures in StructureCache when having-a-bad-time: it is possible that Structure could have this have-a-bad-time
relevant JSGlobalObjects in its prototype chain. We are clearing it for InternalFunction's allocation cache. We should do the
same thing for JSGlobalObject's StructureCache.

This patch adds new watchpoint, structureCacheClearedWatchpoint. And use it in DFG. This watchpoint fires when the cache is cleared,
and it can happen even though JSGlobalObject is not getting have-a-bad-time.

* JSTests/stress/global-object-have-a-bad-time-dependency.js: Added.
(shouldBe):
(cons):
* Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* Source/_javascript_Core/dfg/DFGConstantFoldingPhase.cpp:
(JSC::DFG::ConstantFoldingPhase::foldConstants):
* Source/_javascript_Core/runtime/JSGlobalObject.cpp:
(JSC::JSGlobalObject::JSGlobalObject):
(JSC::JSGlobalObject::fireWatchpointAndMakeAllArrayStructuresSlowPut):
(JSC::JSGlobalObject::clearStructureCache):
* Source/_javascript_Core/runtime/JSGlobalObject.h:
(JSC::JSGlobalObject::structureCacheClearedWatchpoint):
(JSC::JSGlobalObject::isStructureCacheCleared const):
* Source/_javascript_Core/runtime/StructureCache.h:
(JSC::StructureCache::forEach):
* Source/_javascript_Core/runtime/WeakGCMap.h:

Canonical link: https://commits.webkit.org/250845@main

Modified Paths

Added Paths

Diff

Added: trunk/JSTests/stress/global-object-have-a-bad-time-dependency.js (0 => 294619)


--- trunk/JSTests/stress/global-object-have-a-bad-time-dependency.js	                        (rev 0)
+++ trunk/JSTests/stress/global-object-have-a-bad-time-dependency.js	2022-05-22 10:00:56 UTC (rev 294619)
@@ -0,0 +1,30 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+const alien_global_object = createGlobalObject();
+
+const a = {};
+const b = alien_global_object.Object();
+
+a.__proto__ = b;
+
+function cons() {
+
+}
+
+cons.prototype = a;
+
+// Cache
+Reflect.construct(Array, [1.1, 2.2, 3.3], cons);
+
+// Clear rareData to avoid the check in ObjectsWithBrokenIndexingFinder<mode>::visit(JSObject* object).
+cons.prototype = null;
+cons.prototype = a;
+
+// Have a bad time.
+b.__proto__ = new Proxy({}, {});
+
+// This will create a double array having a Proxy object in its prototype chain.
+shouldBe(!!describe(Reflect.construct(Array, [1.1, 2.2, 3.3], cons)).match(/ArrayWithSlowPutArrayStorage/), true);

Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (294618 => 294619)


--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h	2022-05-22 02:34:32 UTC (rev 294618)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h	2022-05-22 10:00:56 UTC (rev 294619)
@@ -3134,14 +3134,8 @@
                 structure = globalObject->nullPrototypeObjectStructure();
             else if (base.isObject()) {
                 // Having a bad time clears the structureCache, and so it should invalidate this structure.
-                bool isHavingABadTime = globalObject->isHavingABadTime();
-                // Normally, we would always install a watchpoint. In this case, however, if we haveABadTime, we
-                // still want to optimize. There is no watchpoint for that case though, so we need to make sure this load
-                // does not get hoisted above the check.
-                WTF::loadLoadFence();
-                if (!isHavingABadTime)
-                    m_graph.watchpoints().addLazily(globalObject->havingABadTimeWatchpoint());
-                structure = globalObject->structureCache().emptyObjectStructureConcurrently(base.getObject(), JSFinalObject::defaultInlineCapacity);
+                if (m_graph.isWatchingStructureCacheClearedWatchpoint(globalObject))
+                    structure = globalObject->structureCache().emptyObjectStructureConcurrently(base.getObject(), JSFinalObject::defaultInlineCapacity);
             }
 
             if (structure) {

Modified: trunk/Source/_javascript_Core/dfg/DFGConstantFoldingPhase.cpp (294618 => 294619)


--- trunk/Source/_javascript_Core/dfg/DFGConstantFoldingPhase.cpp	2022-05-22 02:34:32 UTC (rev 294618)
+++ trunk/Source/_javascript_Core/dfg/DFGConstantFoldingPhase.cpp	2022-05-22 10:00:56 UTC (rev 294619)
@@ -838,14 +838,8 @@
                         structure = globalObject->nullPrototypeObjectStructure();
                     else if (base.isObject()) {
                         // Having a bad time clears the structureCache, and so it should invalidate this structure.
-                        bool isHavingABadTime = globalObject->isHavingABadTime();
-                        // Normally, we would always install a watchpoint. In this case, however, if we haveABadTime, we
-                        // still want to optimize. There is no watchpoint for that case though, so we need to make sure this load
-                        // does not get hoisted above the check.
-                        WTF::loadLoadFence();
-                        if (!isHavingABadTime)
-                            m_graph.watchpoints().addLazily(globalObject->havingABadTimeWatchpoint());
-                        structure = globalObject->structureCache().emptyObjectStructureConcurrently(base.getObject(), JSFinalObject::defaultInlineCapacity);
+                        if (m_graph.isWatchingStructureCacheClearedWatchpoint(globalObject))
+                            structure = globalObject->structureCache().emptyObjectStructureConcurrently(base.getObject(), JSFinalObject::defaultInlineCapacity);
                     }
                     
                     if (structure) {

Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.h (294618 => 294619)


--- trunk/Source/_javascript_Core/dfg/DFGGraph.h	2022-05-22 02:34:32 UTC (rev 294618)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.h	2022-05-22 10:00:56 UTC (rev 294619)
@@ -831,6 +831,15 @@
         return isWatchingGlobalObjectWatchpoint(globalObject, set);
     }
 
+    bool isWatchingStructureCacheClearedWatchpoint(JSGlobalObject* globalObject)
+    {
+        if (m_plan.isUnlinked())
+            return false;
+
+        InlineWatchpointSet& set = globalObject->structureCacheClearedWatchpoint();
+        return isWatchingGlobalObjectWatchpoint(globalObject, set);
+    }
+
     Profiler::Compilation* compilation() { return m_plan.compilation(); }
 
     DesiredIdentifiers& identifiers() { return m_plan.identifiers(); }

Modified: trunk/Source/_javascript_Core/runtime/JSGlobalObject.cpp (294618 => 294619)


--- trunk/Source/_javascript_Core/runtime/JSGlobalObject.cpp	2022-05-22 02:34:32 UTC (rev 294618)
+++ trunk/Source/_javascript_Core/runtime/JSGlobalObject.cpp	2022-05-22 10:00:56 UTC (rev 294619)
@@ -647,6 +647,7 @@
     , m_setAddWatchpointSet(IsWatched)
     , m_arrayJoinWatchpointSet(IsWatched)
     , m_numberToStringWatchpointSet(IsWatched)
+    , m_structureCacheClearedWatchpoint(IsWatched)
     , m_runtimeFlags()
     , m_stackTraceLimit(Options::defaultErrorStackTraceLimit())
     , m_customGetterFunctionSet(vm)
@@ -1961,27 +1962,60 @@
         RELEASE_ASSERT_NOT_REACHED();
     };
 
-    if (JSFunction* function = jsDynamicCast<JSFunction*>(object)) {
+    auto checkStructureHasRelevantGlobalObject = [&](Structure* structure) -> bool {
+        if (hasBrokenIndexing(structure->indexingType())) {
+            bool isRelevantGlobalObject =
+                (mode == BadTimeFinderMode::SingleGlobal
+                    ? m_globalObject == structure->globalObject()
+                    : m_globalObjects->contains(structure->globalObject()))
+                || (structure->hasMonoProto() && !structure->storedPrototype().isNull() && isInAffectedGlobalObject(asObject(structure->storedPrototype())));
+            return isRelevantGlobalObject;
+        }
+        return false;
+    };
+
+    if (object->inherits<JSFunction>()) {
+        JSFunction* function = jsCast<JSFunction*>(object);
         if (FunctionRareData* rareData = function->rareData()) {
             // We only use this to cache JSFinalObjects. They do not start off with a broken indexing type.
             ASSERT(!(rareData->objectAllocationStructure() && hasBrokenIndexing(rareData->objectAllocationStructure()->indexingType())));
 
             if (Structure* structure = rareData->internalFunctionAllocationStructure()) {
-                if (hasBrokenIndexing(structure->indexingType())) {
-                    bool isRelevantGlobalObject =
-                        (mode == BadTimeFinderMode::SingleGlobal
-                            ? m_globalObject == structure->globalObject()
-                            : m_globalObjects->contains(structure->globalObject()))
-                        || (structure->hasMonoProto() && !structure->storedPrototype().isNull() && isInAffectedGlobalObject(asObject(structure->storedPrototype())));
-                    if (mode == BadTimeFinderMode::SingleGlobal && m_needsMultiGlobalsScan)
-                        return IterationStatus::Done; // Bailing early and let the MultipleGlobals path handle everything.
-                    if (isRelevantGlobalObject)
-                        rareData->clearInternalFunctionAllocationProfile("have a bad time breaking internal function allocation");
-                }
+                bool isRelevantGlobalObject = checkStructureHasRelevantGlobalObject(structure);
+                if (mode == BadTimeFinderMode::SingleGlobal && m_needsMultiGlobalsScan)
+                    return IterationStatus::Done; // Bailing early and let the MultipleGlobals path handle everything.
+                if (isRelevantGlobalObject)
+                    rareData->clearInternalFunctionAllocationProfile("have a bad time breaking internal function allocation");
             }
         }
     }
 
+    if (object->inherits<JSGlobalObject>()) {
+        JSGlobalObject* globalObject = jsCast<JSGlobalObject*>(object);
+        // If this globalObject is already having a bad time, then structures in its StructureCache
+        // does not affect on this new JSGlobalObject's haveABadTime since they are already slow mode.
+        if (!globalObject->isHavingABadTime()) {
+            VM& vm = globalObject->vm();
+            ASSERT(vm.heap.isDeferred());
+            bool willClear = false;
+            globalObject->structureCache().forEach([&](Structure* structure) {
+                bool isRelevantGlobalObject = checkStructureHasRelevantGlobalObject(structure);
+                if (mode == BadTimeFinderMode::SingleGlobal && m_needsMultiGlobalsScan)
+                    return IterationStatus::Done;
+                if (isRelevantGlobalObject)
+                    willClear = true;
+                return IterationStatus::Continue;
+            });
+            if (mode == BadTimeFinderMode::SingleGlobal && m_needsMultiGlobalsScan)
+                return IterationStatus::Done; // Bailing early and let the MultipleGlobals path handle everything.
+
+            // StructureCache contains Structures which is no longer valid after relevant JSGlobalObject's haveABadTime.
+            // We do not make such a JSGlobalObject status haveABadTime since still its own objects are intact.
+            if (willClear)
+                globalObject->clearStructureCache(vm);
+        }
+    }
+
     // Run this filter first, since it's cheap, and ought to filter out a lot of objects.
     if (!hasBrokenIndexing(object))
         return IterationStatus::Continue;
@@ -2022,7 +2056,7 @@
     // W_SC, R_BT, R_SC, W_BT: ^ Same
     // W_SC, R_BT, W_BT, R_SC: ^ Same
     // W_SC, W_BT, R_BT, R_SC: No watchpoint is installed, but we could not see old structures from the cache.
-    m_structureCache.clear(); // We may be caching array structures in here.
+    clearStructureCache(vm);
 
     // Make sure that all JSArray allocations that load the appropriate structure from
     // this object now load a structure that uses SlowPut.
@@ -2051,6 +2085,12 @@
     ASSERT(isHavingABadTime()); // The watchpoint is what tells us that we're having a bad time.
 };
 
+void JSGlobalObject::clearStructureCache(VM& vm)
+{
+    m_structureCache.clear(); // We may be caching array structures in here.
+    m_structureCacheClearedWatchpoint.fireAll(vm, "Clearing StructureCache");
+}
+
 void JSGlobalObject::haveABadTime(VM& vm)
 {
     ASSERT(&vm == &this->vm());

Modified: trunk/Source/_javascript_Core/runtime/JSGlobalObject.h (294618 => 294619)


--- trunk/Source/_javascript_Core/runtime/JSGlobalObject.h	2022-05-22 02:34:32 UTC (rev 294618)
+++ trunk/Source/_javascript_Core/runtime/JSGlobalObject.h	2022-05-22 10:00:56 UTC (rev 294619)
@@ -528,6 +528,7 @@
     InlineWatchpointSet m_arraySpeciesWatchpointSet { ClearWatchpoint };
     InlineWatchpointSet m_arrayJoinWatchpointSet;
     InlineWatchpointSet m_numberToStringWatchpointSet;
+    InlineWatchpointSet m_structureCacheClearedWatchpoint;
     InlineWatchpointSet m_arrayBufferSpeciesWatchpointSet { ClearWatchpoint };
     InlineWatchpointSet m_sharedArrayBufferSpeciesWatchpointSet { ClearWatchpoint };
     std::unique_ptr<ObjectPropertyChangeAdaptiveWatchpoint<InlineWatchpointSet>> m_arrayConstructorSpeciesWatchpoint;
@@ -563,6 +564,7 @@
         RELEASE_ASSERT(Options::useJIT());
         return m_numberToStringWatchpointSet;
     }
+    InlineWatchpointSet& structureCacheClearedWatchpoint() { return m_structureCacheClearedWatchpoint; }
     InlineWatchpointSet& arrayBufferSpeciesWatchpointSet(ArrayBufferSharingMode sharingMode)
     {
         switch (sharingMode) {
@@ -1072,6 +1074,7 @@
     }
         
     void haveABadTime(VM&);
+    void clearStructureCache(VM&);
         
     static bool objectPrototypeIsSaneConcurrently(Structure* objectPrototypeStructure);
     bool arrayPrototypeChainIsSaneConcurrently(Structure* arrayPrototypeStructure, Structure* objectPrototypeStructure);

Modified: trunk/Source/_javascript_Core/runtime/StructureCache.h (294618 => 294619)


--- trunk/Source/_javascript_Core/runtime/StructureCache.h	2022-05-22 02:34:32 UTC (rev 294618)
+++ trunk/Source/_javascript_Core/runtime/StructureCache.h	2022-05-22 10:00:56 UTC (rev 294619)
@@ -54,6 +54,13 @@
     JS_EXPORT_PRIVATE Structure* emptyStructureForPrototypeFromBaseStructure(JSGlobalObject*, JSObject*, Structure*);
     JS_EXPORT_PRIVATE Structure* emptyObjectStructureConcurrently(JSObject* prototype, unsigned inlineCapacity);
 
+    template<typename Func>
+    void forEach(Func func)
+    {
+        Locker locker { m_lock };
+        m_structures.forEach(func);
+    }
+
 private:
     Structure* createEmptyStructure(JSGlobalObject*, JSObject* prototype, const TypeInfo&, const ClassInfo*, IndexingType, unsigned inlineCapacity, bool makePolyProtoStructure, FunctionExecutable*);
 

Modified: trunk/Source/_javascript_Core/runtime/WeakGCMap.h (294618 => 294619)


--- trunk/Source/_javascript_Core/runtime/WeakGCMap.h	2022-05-22 02:34:32 UTC (rev 294618)
+++ trunk/Source/_javascript_Core/runtime/WeakGCMap.h	2022-05-22 10:00:56 UTC (rev 294619)
@@ -103,6 +103,9 @@
 
     void pruneStaleEntries() final;
 
+    template<typename Func>
+    void forEach(Func);
+
 private:
     HashMapType m_map;
     VM& m_vm;

Modified: trunk/Source/_javascript_Core/runtime/WeakGCMapInlines.h (294618 => 294619)


--- trunk/Source/_javascript_Core/runtime/WeakGCMapInlines.h	2022-05-22 02:34:32 UTC (rev 294618)
+++ trunk/Source/_javascript_Core/runtime/WeakGCMapInlines.h	2022-05-22 10:00:56 UTC (rev 294619)
@@ -28,6 +28,7 @@
 #include "HeapInlines.h"
 #include "WeakGCMap.h"
 #include "WeakInlines.h"
+#include <wtf/IterationStatus.h>
 
 namespace JSC {
 
@@ -74,4 +75,17 @@
     });
 }
 
+template<typename KeyArg, typename ValueArg, typename HashArg, typename KeyTraitsArg>
+template<typename Func>
+inline void WeakGCMap<KeyArg, ValueArg, HashArg, KeyTraitsArg>::forEach(Func func)
+{
+    ASSERT(m_vm.heap.isDeferred());
+    for (auto& entry : m_map) {
+        if (entry.value) {
+            if (func(entry.value.get()) == IterationStatus::Done)
+                return;
+        }
+    }
+}
+
 } // namespace JSC
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to