Changeset 252160 in webkit for trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

Timestamp:

Nov 6, 2019, 4:29:19 PM (6 years ago)

Author:

[email protected]

Message:

JSGlobalObject::fireWatchpointAndMakeAllArrayStructuresSlowPut() should fire its watchpoint as the last step.
​https://bugs.webkit.org/show_bug.cgi?id=203867
<rdar://problem/56813514>

Reviewed by Saam Barati.

JSTests:

• stress/racy-slow-put-cloned-arguments-when-having-a-bad-time.js: Added.

Source/JavaScriptCore:

JSGlobalObject::fireWatchpointAndMakeAllArrayStructuresSlowPut() should make all
the array structures SlowPut before firing the watchpoint. Otherwise, the
concurrent JIT may think it's grabbing the slow put version of the structure, but
is actually grabbing the non-SlowPut version because it happened to beat the
mutator in a race to read the structure before the mutator makes it SlowPut.

Also removed some assertions in DFGSpeculativeJIT.cpp that are vulnerable to races
between when the mutator makes all array structures SlowPut and when it fires the
HavingABadTime watchpoint. The FTL equivalent did not have these assertions.

• dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compileCreateRest):
(JSC::DFG::SpeculativeJIT::compileNewArray):
(JSC::DFG::SpeculativeJIT::compileNewArrayWithSpread):

• runtime/JSGlobalObject.cpp:

(JSC::JSGlobalObject::fireWatchpointAndMakeAllArrayStructuresSlowPut):

File:

• trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp (modified) (3 diffs)

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

@@ -7816 +7816 @@
         // arguments to have arrayLength exceed MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH.
         bool shouldAllowForArrayStorageStructureForLargeArrays = false;
-        ASSERT(m_jit.graph().globalObjectFor(node->origin.semantic)->restParameterStructure()->indexingMode() == ArrayWithContiguous || m_jit.graph().globalObjectFor(node->origin.semantic)->isHavingABadTime());
         compileAllocateNewArrayWithSize(m_jit.graph().globalObjectFor(node->origin.semantic), arrayResultGPR, arrayLengthGPR, ArrayWithContiguous, shouldAllowForArrayStorageStructureForLargeArrays);
 
@@ -7980 +7979 @@
     RegisteredStructure structure = m_jit.graph().registerStructure(globalObject->arrayStructureForIndexingTypeDuringAllocation(node->indexingType()));
     if (!globalObject->isHavingABadTime() && !hasAnyArrayStorage(node->indexingType())) {
-        DFG_ASSERT(m_jit.graph(), node, structure->indexingType() == node->indexingType(), structure->indexingType(), node->indexingType());
         ASSERT(
             hasUndecided(structure->indexingType())
@@ -8174 +8172 @@
             // non-ArrayStorage shaped array.
             bool shouldAllowForArrayStorageStructureForLargeArrays = false;
-            ASSERT(m_jit.graph().globalObjectFor(node->origin.semantic)->restParameterStructure()->indexingType() == ArrayWithContiguous || m_jit.graph().globalObjectFor(node->origin.semantic)->isHavingABadTime());
             compileAllocateNewArrayWithSize(m_jit.graph().globalObjectFor(node->origin.semantic), resultGPR, lengthGPR, ArrayWithContiguous, shouldAllowForArrayStorageStructureForLargeArrays);
         }