JSImmutableButterfly can't be allocated from a subspace with HeapCell::Kind::Auxiliary
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 27 Jun 2018 01:08:25 +0000 (01:08 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 27 Jun 2018 01:08:25 +0000 (01:08 +0000)
https://bugs.webkit.org/show_bug.cgi?id=186878
<rdar://problem/40568659>

Reviewed by Filip Pizlo.

Source/JavaScriptCore:

This patch fixes a bug in our JSImmutableButterfly implementation uncovered by
our stress GC bots. Before this patch, JSImmutableButterfly was allocated
with HeapCell::Kind::Auxiliary. This is wrong. Things that are JSCells can't
be allocated from HeapCell::Kind::Auxiliary. This patch adds a new HeapCell::Kind
called JSCellWithInteriorPointers. It behaves like JSCell in all ways, except
conservative scan knows to treat it like a butterfly in when we we may be
pointing into the middle of it.

The way we were crashing on the stress GC bots is that our conservative marking
won't do cell visiting for things that are Auxiliary. This meant that if the
stack were the only thing pointing to a JSImmutableButterfly when a GC took place,
that JSImmutableButterfly would not be visited. This is now fixed.

* bytecompiler/NodesCodegen.cpp:
(JSC::ArrayNode::emitBytecode):
* debugger/Debugger.cpp:
* heap/ConservativeRoots.cpp:
(JSC::ConservativeRoots::genericAddPointer):
* heap/Heap.cpp:
(JSC::GatherHeapSnapshotData::operator() const):
(JSC::RemoveDeadHeapSnapshotNodes::operator() const):
(JSC::Heap::globalObjectCount):
(JSC::Heap::objectTypeCounts):
(JSC::Heap::deleteAllCodeBlocks):
* heap/HeapCell.cpp:
(WTF::printInternal):
* heap/HeapCell.h:
(JSC::isJSCellKind):
(JSC::hasInteriorPointers):
* heap/HeapUtil.h:
(JSC::HeapUtil::findGCObjectPointersForMarking):
(JSC::HeapUtil::isPointerGCObjectJSCell):
* heap/MarkedBlock.cpp:
(JSC::MarkedBlock::Handle::didAddToDirectory):
* heap/SlotVisitor.cpp:
(JSC::SlotVisitor::appendJSCellOrAuxiliary):
* runtime/JSGlobalObject.cpp:
* runtime/JSImmutableButterfly.h:
(JSC::JSImmutableButterfly::subspaceFor):
* runtime/VM.cpp:
(JSC::VM::VM):
* runtime/VM.h:
* tools/CellProfile.h:
(JSC::CellProfile::CellProfile):
(JSC::CellProfile::isJSCell const):
* tools/HeapVerifier.cpp:
(JSC::HeapVerifier::validateCell):

LayoutTests:

Make these test not susceptible to conservative scan leaks by ensuring at least
one object gets collected when we allocate many of them. Before, these were just
testing that a fixed number of objects were collected.

* editing/selection/navigation-clears-editor-state-expected.txt:
* editing/selection/navigation-clears-editor-state.html:
* fast/dom/reference-cycle-leaks.html:
* fast/misc/resources/test-observegc.js:
* fast/misc/test-observegc-expected.txt:
* platform/mac-wk2/plugins/refcount-leaks-expected.txt:
* plugins/refcount-leaks-expected.txt:
* plugins/refcount-leaks.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@233236 268f45cc-cd09-0410-ab3c-d52691b4dbfc

25 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/selection/navigation-clears-editor-state-expected.txt
LayoutTests/editing/selection/navigation-clears-editor-state.html
LayoutTests/fast/dom/reference-cycle-leaks.html
LayoutTests/fast/misc/resources/test-observegc.js
LayoutTests/fast/misc/test-observegc-expected.txt
LayoutTests/platform/mac-wk2/plugins/refcount-leaks-expected.txt
LayoutTests/plugins/refcount-leaks-expected.txt
LayoutTests/plugins/refcount-leaks.html
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
Source/JavaScriptCore/debugger/Debugger.cpp
Source/JavaScriptCore/heap/ConservativeRoots.cpp
Source/JavaScriptCore/heap/Heap.cpp
Source/JavaScriptCore/heap/HeapCell.cpp
Source/JavaScriptCore/heap/HeapCell.h
Source/JavaScriptCore/heap/HeapUtil.h
Source/JavaScriptCore/heap/MarkedBlock.cpp
Source/JavaScriptCore/heap/SlotVisitor.cpp
Source/JavaScriptCore/runtime/JSGlobalObject.cpp
Source/JavaScriptCore/runtime/JSImmutableButterfly.h
Source/JavaScriptCore/runtime/VM.cpp
Source/JavaScriptCore/runtime/VM.h
Source/JavaScriptCore/tools/CellProfile.h
Source/JavaScriptCore/tools/HeapVerifier.cpp

index c5c9b9f..0866a39 100644 (file)
@@ -1,3 +1,24 @@
+2018-06-26  Saam Barati  <sbarati@apple.com>
+
+        JSImmutableButterfly can't be allocated from a subspace with HeapCell::Kind::Auxiliary
+        https://bugs.webkit.org/show_bug.cgi?id=186878
+        <rdar://problem/40568659>
+
+        Reviewed by Filip Pizlo.
+
+        Make these test not susceptible to conservative scan leaks by ensuring at least
+        one object gets collected when we allocate many of them. Before, these were just
+        testing that a fixed number of objects were collected.
+
+        * editing/selection/navigation-clears-editor-state-expected.txt:
+        * editing/selection/navigation-clears-editor-state.html:
+        * fast/dom/reference-cycle-leaks.html:
+        * fast/misc/resources/test-observegc.js:
+        * fast/misc/test-observegc-expected.txt:
+        * platform/mac-wk2/plugins/refcount-leaks-expected.txt:
+        * plugins/refcount-leaks-expected.txt:
+        * plugins/refcount-leaks.html:
+
 2018-06-26  Truitt Savell  <tsavell@apple.com>
 
         [iOS] Rebaseline two webanimations tests after r233164
index b40cca5..369e27c 100644 (file)
@@ -3,10 +3,7 @@ This tests navigating away from a document after setting a selection deletes the
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-iframe = appendIframe()
-PASS internals.numberOfLiveDocuments() is initialDocumentCount + 1
-iframe.src = "resources/select-iframe-focusin-document-crash-frame.html"
-PASS internals.numberOfLiveDocuments() is initialDocumentCount + 1
+PASS freed more than 60 documents
 PASS successfullyParsed is true
 
 TEST COMPLETE
index a0ee4fb..d3e0952 100644 (file)
@@ -35,21 +35,36 @@ async function runTest()
 {
     initialDocumentCount = internals.numberOfLiveDocuments();
 
-    evalAndLog('iframe = appendIframe()');
+    let frames = [];
+    const count = 200;
+    for (let i = 0; i < count; ++i)
+        frames.push(appendIframe());
 
     await wait(0); // Make sure the transient document created by inserting an iframe is removed.
     GCController.collect();
 
-    shouldBe('internals.numberOfLiveDocuments()', 'initialDocumentCount + 1');
-    setEditorStates(iframe);
+    for (let frame of frames)
+        setEditorStates(frame);
 
     await wait(0); // Wait for UI update timer to fire.
 
-    evalAndLog('iframe.src = "resources/select-iframe-focusin-document-crash-frame.html"');
-    iframe.onload = () => {
-        GCController.collect();
-        shouldBe('internals.numberOfLiveDocuments()', 'initialDocumentCount + 1');
-        finishJSTest();                
+    let resolved = 0;
+    for (let frame of frames) {
+        frame.src = "resources/select-iframe-focusin-document-crash-frame.html";
+        frame.onload = () => {
+            ++resolved;
+            if (resolved !== count)
+                return;
+            let before = internals.numberOfLiveDocuments();
+            GCController.collect();
+            let after = internals.numberOfLiveDocuments();
+            let delta = before - after;
+            if (delta > 0.3 * count)
+                debug("PASS freed more than 60 documents");
+            else
+                debug("FAIL freed fewer than 60 documents");
+            finishJSTest();                
+        };
     }
 }
 
@@ -57,7 +72,7 @@ if (!window.GCController || !window.internals) {
     debug('This test requires GCController and internals');
 } else {
     if (window.testRunner)
-        setTimeout(() => testRunner.notifyDone(), 3000);
+        setTimeout(() => testRunner.notifyDone(), 5000);
     // Clear out any lingering documents from the previous tests.
     GCController.collect();
     GCController.collect();
index 0953d57..e69b2a3 100644 (file)
@@ -7,7 +7,7 @@ description('Tests for leaks caused by reference cycles that pass through the DO
 function checkForNodeLeaks(testFunction, underlyingClass)
 {
     // Bump this number as high as we need to, to get reproducible results.
-    const repetitions = 20;
+    const repetitions = 40;
 
     gc();
     const beforeCount = internals.numberOfLiveNodes();
index 65e68f5..dc27d7b 100644 (file)
@@ -1,9 +1,19 @@
 description("Ensures that window.internals.observegc works as expected");
 
-var testObject = { testProperty : "testValue" };
+var observers = [];
+for (let i = 0; i < 1000; ++i) {
+    let testObject = { testProperty : "testValue" };
+    let observer = internals.observeGC(testObject);
+    observers.push(observer);
+    testObject = null;
+}
 
-var observer = internals.observeGC(testObject);
-testObject = null;
 gc();
 
-shouldBe('observer.wasCollected', 'true');
+var anyCollected = false;
+for (let observer of observers) {
+    if (observer.wasCollected)
+        anyCollected = true;
+}
+
+shouldBe('anyCollected', 'true');
index c9edc87..808d13c 100644 (file)
@@ -3,7 +3,7 @@ Ensures that window.internals.observegc works as expected
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-PASS observer.wasCollected is true
+PASS anyCollected is true
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 8c6dc35..6f752dc 100644 (file)
@@ -1,12 +1,8 @@
+PASS countAfterCreate === countOrig + 50 is true
+FAIL countAfterGC < countAfterCreate should be true. Was false.
+PASS refAfterGet === refOrig + 50 is true
+FAIL refAfterGetGC < refAfterGet should be true. Was false.
+PASS refAfterPass === refBeforePass + 50 is true
+FAIL refAfterPassAndGC < refAfterPass should be true. Was false.
 Test that we can get an NPObject returned through a method on an NPAPI Object.
-Prints "SUCCESS" on success, "FAILURE" on failure.  
 
---- num test objects:
-countAfterCreate == countOrig + 3? PASS
-countOrig == countAfterGC? FAIL
-
---- refcount on plug.testObject:
-originally: 2
-after GC: 5
-after passing: 8
-FAILURE
index d51a342..9e35f69 100644 (file)
@@ -1,12 +1,8 @@
+PASS countAfterCreate === countOrig + 50 is true
+PASS countAfterGC < countAfterCreate is true
+PASS refAfterGet === refOrig + 50 is true
+PASS refAfterGetGC < refAfterGet is true
+PASS refAfterPass === refBeforePass + 50 is true
+PASS refAfterPassAndGC < refAfterPass is true
 Test that we can get an NPObject returned through a method on an NPAPI Object.
-Prints "SUCCESS" on success, "FAILURE" on failure.  
 
---- num test objects:
-countAfterCreate == countOrig + 3? PASS
-countOrig == countAfterGC? PASS
-
---- refcount on plug.testObject:
-originally: 2
-after GC: 2
-after passing: 2
-SUCCESS
index 69f6ee0..0eb4d79 100644 (file)
@@ -1,83 +1,74 @@
+<head>
+<script src="../resources/js-test-pre.js"></script>
+</head>
+
 <script>
-  function noop(x) {
-  }
+function noop(x) {
+}
 
-  function doGC() {
-    if (window.GCController) {
-      // GC twice to make sure everything is cleaned up.
-      for (var i = 0; i < 2; i++) {
-        window.GCController.collect();
-      }
+function doGC() {
+    // GC twice to make sure everything is cleaned up.
+    for (var i = 0; i < 2; i++) {
+        gc();
     }
-  }
-
-  function runtest() {
+}
+
+var countOrig;
+var countAfterCreate;
+var countAfterGC;
+var testObj;
+var refOrig;
+var refAfterGet;
+var refAfterGetGC;
+var refBeforePass;
+var refAfterPass;
+var refAfterPassAndGC;
+
+function runtest() {
     if (window.testRunner)
-      testRunner.dumpAsText();
-
-
-    var output = document.getElementById("output");
-    output.innerHTML = "";
+        testRunner.dumpAsText();
 
     // Test that objects are deleted after their JS references are released.
-    var countOrig = plug.testObjectCount;
-    o1 = plug.testCreateTestObject();
-    o2 = plug.testCreateTestObject();
-    o3 = plug.testCreateTestObject();
-    var countAfterCreate = plug.testObjectCount;
-    o1 = o2 = o3 = null;
+    countOrig = plug.testObjectCount;
+    let x;
+    for (let i = 0; i < 50; ++i)
+        x = plug.testCreateTestObject();
+    countAfterCreate = plug.testObjectCount;
+    x = null;
     doGC();
-    var countAfterGC = plug.testObjectCount;
-
-    output.innerHTML += "--- num test objects:<br>";
-    output.innerHTML += "countAfterCreate == countOrig + 3? "
-        + ((countAfterCreate == countOrig + 3) ? "PASS" : "FAIL")
-        + "<br>";
-    output.innerHTML += "countOrig == countAfterGC? "
-        + ((countOrig == countAfterGC) ? "PASS" : "FAIL")
-        + "<br>";
-    output.innerHTML += "<br>";
+    countAfterGC = plug.testObjectCount;
+    shouldBe('countAfterCreate === countOrig + 50', 'true');
+    shouldBe('countAfterGC < countAfterCreate', 'true');
 
     // Test that the object refcount returns to normal after JS references
     // are released.
-    var testObj = plug.testObject;
-    var refOrig = testObj.refCount;
-    var o1 = plug.testObject;
-    var o2 = plug.testObject;
-    var o3 = plug.testObject;
-    var refAfterGet = testObj.refCount;
-    o1 = o2 = o3 = null;
+    testObj = plug.testObject;
+    refOrig = testObj.refCount;
+    for (let i = 0; i < 50; ++i)
+        plug.testObject;
+    refAfterGet = testObj.refCount;
     doGC();
-    var refAfterGetGC = testObj.refCount;
+    refAfterGetGC = testObj.refCount;
+    shouldBe('refAfterGet === refOrig + 50', 'true');
+    shouldBe('refAfterGetGC < refAfterGet', 'true');
 
     // Test that calling NPN_Invoke with our object as a parameter returns
     // our refcount to normal (may require a GC).
-    plug.testPassTestObject("noop", testObj);
-    plug.testPassTestObject("noop", testObj);
-    plug.testPassTestObject("noop", testObj);
+    refBeforePass = testObj.refCount;
+    for (let i = 0; i < 50; ++i)
+        plug.testPassTestObject("noop", testObj);
+    refAfterPass = testObj.refCount;
     doGC();
-    var refAfterPass = testObj.refCount;
-
-    output.innerHTML += "--- refcount on plug.testObject:<br>";
-    output.innerHTML += "originally: " + refOrig + "<br>";
-    output.innerHTML += "after GC: " + refAfterGetGC + "<br>";
-    output.innerHTML += "after passing: " + refAfterPass + "<br>";
-
-    var success = (countAfterGC == countOrig) && (refAfterPass == refOrig);
-    output.innerHTML += (success ? "SUCCESS" : "FAILURE");
-  }
+    refAfterPassAndGC = testObj.refCount;
+    shouldBe('refAfterPass === refBeforePass + 50', 'true');
+    shouldBe('refAfterPassAndGC < refAfterPass', 'true');
+}
 </script>
 
 <body onload="runtest()">
 
-Test that we can get an NPObject returned through a method on
-an NPAPI Object.<P>
-
-Prints "SUCCESS" on success, "FAILURE" on failure.
-
-<embed name="plug" type="application/x-webkit-test-netscape">
-
-<div id=output>FAILURE</div>
+    Test that we can get an NPObject returned through a method on
+    an NPAPI Object.<P>
 
+    <embed name="plug" type="application/x-webkit-test-netscape">
 </body>
-
index 591bc5c..9e997cf 100644 (file)
@@ -1,3 +1,59 @@
+2018-06-26  Saam Barati  <sbarati@apple.com>
+
+        JSImmutableButterfly can't be allocated from a subspace with HeapCell::Kind::Auxiliary
+        https://bugs.webkit.org/show_bug.cgi?id=186878
+        <rdar://problem/40568659>
+
+        Reviewed by Filip Pizlo.
+
+        This patch fixes a bug in our JSImmutableButterfly implementation uncovered by
+        our stress GC bots. Before this patch, JSImmutableButterfly was allocated
+        with HeapCell::Kind::Auxiliary. This is wrong. Things that are JSCells can't
+        be allocated from HeapCell::Kind::Auxiliary. This patch adds a new HeapCell::Kind
+        called JSCellWithInteriorPointers. It behaves like JSCell in all ways, except
+        conservative scan knows to treat it like a butterfly in when we we may be
+        pointing into the middle of it.
+        
+        The way we were crashing on the stress GC bots is that our conservative marking
+        won't do cell visiting for things that are Auxiliary. This meant that if the
+        stack were the only thing pointing to a JSImmutableButterfly when a GC took place,
+        that JSImmutableButterfly would not be visited. This is now fixed.
+
+        * bytecompiler/NodesCodegen.cpp:
+        (JSC::ArrayNode::emitBytecode):
+        * debugger/Debugger.cpp:
+        * heap/ConservativeRoots.cpp:
+        (JSC::ConservativeRoots::genericAddPointer):
+        * heap/Heap.cpp:
+        (JSC::GatherHeapSnapshotData::operator() const):
+        (JSC::RemoveDeadHeapSnapshotNodes::operator() const):
+        (JSC::Heap::globalObjectCount):
+        (JSC::Heap::objectTypeCounts):
+        (JSC::Heap::deleteAllCodeBlocks):
+        * heap/HeapCell.cpp:
+        (WTF::printInternal):
+        * heap/HeapCell.h:
+        (JSC::isJSCellKind):
+        (JSC::hasInteriorPointers):
+        * heap/HeapUtil.h:
+        (JSC::HeapUtil::findGCObjectPointersForMarking):
+        (JSC::HeapUtil::isPointerGCObjectJSCell):
+        * heap/MarkedBlock.cpp:
+        (JSC::MarkedBlock::Handle::didAddToDirectory):
+        * heap/SlotVisitor.cpp:
+        (JSC::SlotVisitor::appendJSCellOrAuxiliary):
+        * runtime/JSGlobalObject.cpp:
+        * runtime/JSImmutableButterfly.h:
+        (JSC::JSImmutableButterfly::subspaceFor):
+        * runtime/VM.cpp:
+        (JSC::VM::VM):
+        * runtime/VM.h:
+        * tools/CellProfile.h:
+        (JSC::CellProfile::CellProfile):
+        (JSC::CellProfile::isJSCell const):
+        * tools/HeapVerifier.cpp:
+        (JSC::HeapVerifier::validateCell):
+
 2018-06-26  Mark Lam  <mark.lam@apple.com>
 
         Skip some unnecessary work in Interpreter::getStackTrace().
index 0c909d0..850b3d2 100644 (file)
@@ -406,6 +406,7 @@ RegisterID* ArrayNode::emitBytecode(BytecodeGenerator& generator, RegisterID* ds
     auto newArray = [&] (RegisterID* dst, ElementNode* elements, unsigned length, bool hadVariableExpression) {
         if (length && !hadVariableExpression) {
             recommendedIndexingType |= CopyOnWrite;
+            ASSERT(generator.vm()->heap.isDeferred()); // We run bytecode generator under a DeferGC. If we stopped doing that, we'd need to put a DeferGC here as we filled in these slots.
             auto* array = JSImmutableButterfly::create(*generator.vm(), recommendedIndexingType, length);
             unsigned index = 0;
             for (ElementNode* element = elements; index < length; element = element->next()) {
index b6570bf..cc9102f 100644 (file)
@@ -51,7 +51,7 @@ struct GatherSourceProviders : public MarkedBlock::VoidFunctor {
 
     IterationStatus operator()(HeapCell* heapCell, HeapCell::Kind kind) const
     {
-        if (kind != HeapCell::JSCell)
+        if (!isJSCellKind(kind))
             return IterationStatus::Continue;
         
         JSCell* cell = static_cast<JSCell*>(heapCell);
index 30a26dd..a420fe8 100644 (file)
@@ -73,7 +73,7 @@ inline void ConservativeRoots::genericAddPointer(void* p, HeapVersion markingVer
     HeapUtil::findGCObjectPointersForMarking(
         m_heap, markingVersion, newlyAllocatedVersion, filter, p,
         [&] (void* p, HeapCell::Kind cellKind) {
-            if (cellKind == HeapCell::JSCell)
+            if (isJSCellKind(cellKind))
                 markHook.markKnownJSCell(static_cast<JSCell*>(p));
             
             if (m_size == m_capacity)
index 57fbed3..b6bc8d9 100644 (file)
@@ -720,7 +720,7 @@ struct GatherHeapSnapshotData : MarkedBlock::CountFunctor {
 
     IterationStatus operator()(HeapCell* heapCell, HeapCell::Kind kind) const
     {
-        if (kind == HeapCell::JSCell) {
+        if (isJSCellKind(kind)) {
             JSCell* cell = static_cast<JSCell*>(heapCell);
             cell->methodTable()->heapSnapshot(cell, m_builder);
         }
@@ -747,7 +747,7 @@ struct RemoveDeadHeapSnapshotNodes : MarkedBlock::CountFunctor {
 
     IterationStatus operator()(HeapCell* cell, HeapCell::Kind kind) const
     {
-        if (kind == HeapCell::JSCell)
+        if (isJSCellKind(kind))
             m_snapshot.sweepCell(static_cast<JSCell*>(cell));
         return IterationStatus::Continue;
     }
@@ -836,7 +836,7 @@ size_t Heap::globalObjectCount()
     m_objectSpace.forEachLiveCell(
         iterationScope,
         [&] (HeapCell* heapCell, HeapCell::Kind kind) -> IterationStatus {
-            if (kind != HeapCell::JSCell)
+            if (!isJSCellKind(kind))
                 return IterationStatus::Continue;
             JSCell* cell = static_cast<JSCell*>(heapCell);
             if (cell->isObject() && asObject(cell)->isGlobalObject())
@@ -873,7 +873,7 @@ std::unique_ptr<TypeCountSet> Heap::objectTypeCounts()
     m_objectSpace.forEachLiveCell(
         iterationScope,
         [&] (HeapCell* cell, HeapCell::Kind kind) -> IterationStatus {
-            if (kind == HeapCell::JSCell)
+            if (isJSCellKind(kind))
                 recordType(*vm(), *result, static_cast<JSCell*>(cell));
             return IterationStatus::Continue;
         });
@@ -915,7 +915,7 @@ void Heap::deleteAllCodeBlocks(DeleteAllCodeEffort effort)
         // it uses a callee check, but then it will call into dead code.
         HeapIterationScope heapIterationScope(*this);
         vm.webAssemblyCodeBlockSpace.forEachLiveCell([&] (HeapCell* cell, HeapCell::Kind kind) {
-            ASSERT_UNUSED(kind, kind == HeapCell::Kind::JSCell);
+            ASSERT_UNUSED(kind, kind == HeapCell::JSCell);
             JSWebAssemblyCodeBlock* codeBlock = static_cast<JSWebAssemblyCodeBlock*>(cell);
             codeBlock->clearJSCallICs(vm);
         });
index cd792c1..f4f31fe 100644 (file)
@@ -60,6 +60,9 @@ void printInternal(PrintStream& out, HeapCell::Kind kind)
     case HeapCell::JSCell:
         out.print("JSCell");
         return;
+    case HeapCell::JSCellWithInteriorPointers:
+        out.print("JSCellWithInteriorPointers");
+        return;
     case HeapCell::Auxiliary:
         out.print("Auxiliary");
         return;
index 68f7b17..eb59e8a 100644 (file)
@@ -43,6 +43,7 @@ class HeapCell {
 public:
     enum Kind : int8_t {
         JSCell,
+        JSCellWithInteriorPointers,
         Auxiliary
     };
     
@@ -86,6 +87,16 @@ public:
 #endif
 };
 
+inline bool isJSCellKind(HeapCell::Kind kind)
+{
+    return kind == HeapCell::JSCell || kind == HeapCell::JSCellWithInteriorPointers;
+}
+
+inline bool hasInteriorPointers(HeapCell::Kind kind)
+{
+    return kind == HeapCell::Auxiliary || kind == HeapCell::JSCellWithInteriorPointers;
+}
+
 } // namespace JSC
 
 namespace WTF {
index 01cfdbc..9ef03ea 100644 (file)
@@ -88,7 +88,7 @@ public:
             MarkedBlock* previousCandidate = MarkedBlock::blockFor(previousPointer);
             if (!filter.ruleOut(bitwise_cast<Bits>(previousCandidate))
                 && set.contains(previousCandidate)
-                && previousCandidate->handle().cellKind() == HeapCell::Auxiliary) {
+                && hasInteriorPointers(previousCandidate->handle().cellKind())) {
                 previousPointer = static_cast<char*>(previousCandidate->handle().cellAlign(previousPointer));
                 if (previousCandidate->handle().isLiveCell(markingVersion, newlyAllocatedVersion, isMarking, previousPointer))
                     func(previousPointer, previousCandidate->handle().cellKind());
@@ -110,12 +110,11 @@ public:
                 func(pointer, cellKind);
         };
     
-        if (candidate->handle().cellKind() == HeapCell::JSCell) {
-            if (!MarkedBlock::isAtomAligned(pointer))
+        if (isJSCellKind(cellKind)) {
+            if (MarkedBlock::isAtomAligned(pointer))
+                tryPointer(pointer);
+            if (!hasInteriorPointers(cellKind))
                 return;
-        
-            tryPointer(pointer);
-            return;
         }
     
         // A butterfly could point into the middle of an object.
@@ -144,14 +143,14 @@ public:
                 if (result) {
                     if (result > largeAllocations.begin()
                         && result[-1]->cell() == pointer
-                        && result[-1]->attributes().cellKind == HeapCell::JSCell)
+                        && isJSCellKind(result[-1]->attributes().cellKind))
                         return true;
                     if (result[0]->cell() == pointer
-                        && result[0]->attributes().cellKind == HeapCell::JSCell)
+                        && isJSCellKind(result[0]->attributes().cellKind))
                         return true;
                     if (result + 1 < largeAllocations.end()
                         && result[1]->cell() == pointer
-                        && result[1]->attributes().cellKind == HeapCell::JSCell)
+                        && isJSCellKind(result[1]->attributes().cellKind))
                         return true;
                 }
             }
index c425344..b1a10a1 100644 (file)
@@ -351,7 +351,7 @@ void MarkedBlock::Handle::didAddToDirectory(BlockDirectory* directory, size_t in
     
     m_attributes = directory->attributes();
 
-    if (m_attributes.cellKind != HeapCell::JSCell)
+    if (!isJSCellKind(m_attributes.cellKind))
         RELEASE_ASSERT(m_attributes.destruction == DoesNotNeedDestruction);
     
     double markCountBias = -(Options::minMarkedBlockUtilization() * cellsPerBlock());
index b89686e..e737f39 100644 (file)
@@ -196,14 +196,15 @@ void SlotVisitor::appendJSCellOrAuxiliary(HeapCell* heapCell)
     
     // In debug mode, we validate before marking since this makes it clearer what the problem
     // was. It's also slower, so we don't do it normally.
-    if (!ASSERT_DISABLED && heapCell->cellKind() == HeapCell::JSCell)
+    if (!ASSERT_DISABLED && isJSCellKind(heapCell->cellKind()))
         validateCell(static_cast<JSCell*>(heapCell));
     
     if (Heap::testAndSetMarked(m_markingVersion, heapCell))
         return;
     
     switch (heapCell->cellKind()) {
-    case HeapCell::JSCell: {
+    case HeapCell::JSCell:
+    case HeapCell::JSCellWithInteriorPointers: {
         // We have ample budget to perform validation here.
     
         JSCell* jsCell = static_cast<JSCell*>(heapCell);
index 5e4ed5c..6a9b78d 100644 (file)
@@ -1239,7 +1239,7 @@ inline void ObjectsWithBrokenIndexingFinder::visit(JSCell* cell)
 
 IterationStatus ObjectsWithBrokenIndexingFinder::operator()(HeapCell* cell, HeapCell::Kind kind) const
 {
-    if (kind == HeapCell::JSCell) {
+    if (isJSCellKind(kind)) {
         // FIXME: This const_cast exists because this isn't a C++ lambda.
         // https://bugs.webkit.org/show_bug.cgi?id=159644
         const_cast<ObjectsWithBrokenIndexingFinder*>(this)->visit(static_cast<JSCell*>(cell));
index 3ace0ad..4b6ac7b 100644 (file)
@@ -89,7 +89,7 @@ public:
     static CompleteSubspace* subspaceFor(VM& vm)
     {
         // We allocate out of the JSValue gigacage as other code expects all butterflies to live there.
-        return &vm.jsValueGigacageAuxiliarySpace;
+        return &vm.immutableButterflyJSValueGigacageAuxiliarySpace;
     }
 
     // Only call this if you just allocated this butterfly.
index 98f9a44..24b9f60 100644 (file)
@@ -254,6 +254,7 @@ VM::VM(VMType vmType, HeapType heapType)
     , jsValueGigacageAllocator(std::make_unique<GigacageAlignedMemoryAllocator>(Gigacage::JSValue))
     , auxiliaryHeapCellType(std::make_unique<HeapCellType>(CellAttributes(DoesNotNeedDestruction, HeapCell::Auxiliary)))
     , cellJSValueOOBHeapCellType(std::make_unique<HeapCellType>(CellAttributes(DoesNotNeedDestruction, HeapCell::JSCell)))
+    , immutableButterflyHeapCellType(std::make_unique<HeapCellType>(CellAttributes(DoesNotNeedDestruction, HeapCell::JSCellWithInteriorPointers)))
     , cellDangerousBitsHeapCellType(std::make_unique<HeapCellType>(CellAttributes(DoesNotNeedDestruction, HeapCell::JSCell)))
     , destructibleCellHeapCellType(std::make_unique<HeapCellType>(CellAttributes(NeedsDestruction, HeapCell::JSCell)))
     , stringHeapCellType(std::make_unique<JSStringHeapCellType>())
@@ -264,6 +265,7 @@ VM::VM(VMType vmType, HeapType heapType)
 #endif
     , primitiveGigacageAuxiliarySpace("Primitive Gigacage Auxiliary", heap, auxiliaryHeapCellType.get(), primitiveGigacageAllocator.get())
     , jsValueGigacageAuxiliarySpace("JSValue Gigacage Auxiliary", heap, auxiliaryHeapCellType.get(), jsValueGigacageAllocator.get())
+    , immutableButterflyJSValueGigacageAuxiliarySpace("ImmutableButterfly Gigacage JSCellWithInteriorPointers", heap, immutableButterflyHeapCellType.get(), jsValueGigacageAllocator.get())
     , cellJSValueOOBSpace("JSCell JSValueOOB", heap, cellJSValueOOBHeapCellType.get(), fastMallocAllocator.get())
     , cellDangerousBitsSpace("JSCell DangerousBits", heap, cellDangerousBitsHeapCellType.get(), fastMallocAllocator.get())
     , jsValueGigacageCellSpace("JSValue Gigacage JSCell", heap, cellJSValueOOBHeapCellType.get(), jsValueGigacageAllocator.get())
index cb0d353..eb1ae65 100644 (file)
@@ -306,6 +306,7 @@ public:
 
     std::unique_ptr<HeapCellType> auxiliaryHeapCellType;
     std::unique_ptr<HeapCellType> cellJSValueOOBHeapCellType;
+    std::unique_ptr<HeapCellType> immutableButterflyHeapCellType;
     std::unique_ptr<HeapCellType> cellDangerousBitsHeapCellType;
     std::unique_ptr<HeapCellType> destructibleCellHeapCellType;
     std::unique_ptr<JSStringHeapCellType> stringHeapCellType;
@@ -317,6 +318,7 @@ public:
     
     CompleteSubspace primitiveGigacageAuxiliarySpace; // Typed arrays, strings, bitvectors, etc go here.
     CompleteSubspace jsValueGigacageAuxiliarySpace; // Butterflies, arrays of JSValues, etc go here.
+    CompleteSubspace immutableButterflyJSValueGigacageAuxiliarySpace; // JSImmutableButterfly goes here.
 
     // We make cross-cutting assumptions about typed arrays being in the primitive Gigacage and butterflies
     // being in the JSValue gigacage. For some types, it's super obvious where they should go, and so we
index f546b60..d0fc0aa 100644 (file)
@@ -45,7 +45,7 @@ struct CellProfile {
         , m_liveness(liveness)
         , m_timestamp(MonotonicTime::now())
     {
-        if (m_kind == HeapCell::JSCell && m_liveness != Dead)
+        if (isJSCellKind(m_kind) && m_liveness != Dead)
             m_className = jsCell()->structure()->classInfo()->className;
     }
 
@@ -65,7 +65,7 @@ struct CellProfile {
         return static_cast<JSCell*>(m_cell);
     }
 
-    bool isJSCell() const { return m_kind == HeapCell::JSCell; }
+    bool isJSCell() const { return isJSCellKind(m_kind); }
     
     HeapCell::Kind kind() const { return m_kind; }
 
index dff1707..2990ff7 100644 (file)
@@ -188,7 +188,7 @@ bool HeapVerifier::validateCell(HeapCell* cell, VM* expectedVM)
         return false;
     }
 
-    if (cell->cellKind() != HeapCell::JSCell)
+    if (!isJSCellKind(cell->cellKind()))
         return true; // Nothing more to validate.
 
     JSCell* jsCell = static_cast<JSCell*>(cell);