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

Timestamp:

Dec 9, 2016, 5:22:15 PM (9 years ago)

Author:

[email protected]

Message:

GC might be forced to look at a nuked object due to ordering of AllocatePropertyStorage, MaterializeNewObject, and PutStructure
​https://bugs.webkit.org/show_bug.cgi?id=165672

Reviewed by Geoffrey Garen.

We need to make sure that the shady stuff in a property put happens after the
PutByOffset, since the PutByOffset is the place where we materialize. More generally, we
should strive to not have any fenceposts between Nodes where a GC would be illegal.

This gets us most of the way there by separating NukeStructureAndSetButterfly from
[Re]AllocatePropertyStorage. A transitioning put will now look something like:

GetButterfly
ReallocatePropertyStorage
PutByOffset
NukeStructureAndSetButterfly
PutStructure

Previously the structure would get nuked by ReallocatePropertyStorage, so if we placed
an object materialization just after it (before the PutByOffset) then any GC that
completed at that safepoint would encounter an unresolved visit race due to seeing a
nuked structure. We cannot have nuked structures at safepoints, and this change makes
sure that we don't - at least until someone tries to sink to the PutStructure. We will
eventually have to create a combined SetStructureAndButterfly node, but we don't need it
yet.

This also fixes a goof where the DFG's AllocatePropertyStorage was nulling the structure
instead of nuking it. This could easily have caused many crashes in GC.

• dfg/DFGAbstractInterpreterInlines.h:

(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):

• dfg/DFGByteCodeParser.cpp:

(JSC::DFG::ByteCodeParser::handlePutById):

• dfg/DFGClobberize.h:

(JSC::DFG::clobberize):

• dfg/DFGClobbersExitState.cpp:

(JSC::DFG::clobbersExitState):

• dfg/DFGConstantFoldingPhase.cpp:

(JSC::DFG::ConstantFoldingPhase::emitPutByOffset):

• dfg/DFGDoesGC.cpp:

(JSC::DFG::doesGC):

• dfg/DFGFixupPhase.cpp:

(JSC::DFG::FixupPhase::fixupNode):

• dfg/DFGMayExit.cpp:
• dfg/DFGNodeType.h:
• dfg/DFGOperations.cpp:
• dfg/DFGOperations.h:
• dfg/DFGPredictionPropagationPhase.cpp:
• dfg/DFGSafeToExecute.h:

(JSC::DFG::safeToExecute):

• dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compileAllocatePropertyStorage):
(JSC::DFG::SpeculativeJIT::compileReallocatePropertyStorage):
(JSC::DFG::SpeculativeJIT::compileNukeStructureAndSetButterfly):

• dfg/DFGSpeculativeJIT.h:
• dfg/DFGSpeculativeJIT32_64.cpp:

(JSC::DFG::SpeculativeJIT::compile):

• dfg/DFGSpeculativeJIT64.cpp:

(JSC::DFG::SpeculativeJIT::compile):

• dfg/DFGStoreBarrierInsertionPhase.cpp:
• dfg/DFGTypeCheckHoistingPhase.cpp:

(JSC::DFG::TypeCheckHoistingPhase::identifyRedundantStructureChecks):

• ftl/FTLCapabilities.cpp:

(JSC::FTL::canCompile):

• ftl/FTLLowerDFGToB3.cpp:

(JSC::FTL::DFG::LowerDFGToB3::compileNode):
(JSC::FTL::DFG::LowerDFGToB3::compileNukeStructureAndSetButterfly):
(JSC::FTL::DFG::LowerDFGToB3::storageForTransition):
(JSC::FTL::DFG::LowerDFGToB3::allocatePropertyStorage):
(JSC::FTL::DFG::LowerDFGToB3::reallocatePropertyStorage):
(JSC::FTL::DFG::LowerDFGToB3::allocatePropertyStorageWithSizeImpl):

• runtime/Options.cpp:

(JSC::recomputeDependentOptions):

• runtime/Options.h: Fix a bug - make it possible to turn on concurrent GC optionally again.

File:

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

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

@@ -7390 +7390 @@
 
         GPRFlushedCallResult result(this);
-        callOperation(operationReallocateButterflyToHavePropertyStorageWithInitialCapacity, result.gpr(), baseGPR);
+        callOperation(operationAllocateComplexPropertyStorageWithInitialCapacity, result.gpr(), baseGPR);
         m_jit.exceptionCheck();
         
@@ -7397 +7397 @@
     }
     
-    SpeculateCellOperand base(this, node->child1());
     GPRTemporary scratch1(this);
     GPRTemporary scratch2(this);
     GPRTemporary scratch3(this);
         
-    GPRReg baseGPR = base.gpr();
     GPRReg scratchGPR1 = scratch1.gpr();
     GPRReg scratchGPR2 = scratch2.gpr();
@@ -7416 +7414 @@
         
     addSlowPathGenerator(
-        slowPathCall(slowPath, this, operationAllocatePropertyStorageWithInitialCapacity, scratchGPR1));
-
-    m_jit.store32(TrustedImm32(0), JITCompiler::Address(baseGPR, JSCell::structureIDOffset()));
-    m_jit.storeButterfly(scratchGPR1, baseGPR);
+        slowPathCall(slowPath, this, operationAllocateSimplePropertyStorageWithInitialCapacity, scratchGPR1));
 
     storageResult(scratchGPR1, node);
@@ -7440 +7435 @@
 
         GPRFlushedCallResult result(this);
-        callOperation(operationReallocateButterflyToGrowPropertyStorage, result.gpr(), baseGPR, newSize / sizeof(JSValue));
+        callOperation(operationAllocateComplexPropertyStorage, result.gpr(), baseGPR, newSize / sizeof(JSValue));
         m_jit.exceptionCheck();
 
@@ -7447 +7442 @@
     }
     
-    SpeculateCellOperand base(this, node->child1());
     StorageOperand oldStorage(this, node->child2());
     GPRTemporary scratch1(this);
@@ -7453 +7447 @@
     GPRTemporary scratch3(this);
         
-    GPRReg baseGPR = base.gpr();
     GPRReg oldStorageGPR = oldStorage.gpr();
     GPRReg scratchGPR1 = scratch1.gpr();
@@ -7469 +7462 @@
 
     addSlowPathGenerator(
-        slowPathCall(slowPath, this, operationAllocatePropertyStorage, scratchGPR1, newSize / sizeof(JSValue)));
+        slowPathCall(slowPath, this, operationAllocateSimplePropertyStorage, scratchGPR1, newSize / sizeof(JSValue)));
 
     // We have scratchGPR1 = new storage, scratchGPR2 = scratch
@@ -7477 +7470 @@
     }
         
-    m_jit.nukeStructureAndStoreButterfly(scratchGPR1, baseGPR);
-
     storageResult(scratchGPR1, node);
+}
+
+void SpeculativeJIT::compileNukeStructureAndSetButterfly(Node* node)
+{
+    SpeculateCellOperand base(this, node->child1());
+    StorageOperand storage(this, node->child2());
+    
+    GPRReg baseGPR = base.gpr();
+    GPRReg storageGPR = storage.gpr();
+    
+    m_jit.nukeStructureAndStoreButterfly(storageGPR, baseGPR);
+    
+    noResult(node);
 }