LayoutTests/fast/css/parsing-css-matches-7.html always abandons its Document (disabli...
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 4 Jun 2018 04:13:42 +0000 (04:13 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 4 Jun 2018 04:13:42 +0000 (04:13 +0000)
https://bugs.webkit.org/show_bug.cgi?id=186223

Reviewed by Keith Miller.

After preparing catchOSREntryBuffer, we do not clear the active length of this scratch buffer.
It makes this buffer conservative GC root, and allows it to hold GC objects unnecessarily long.

This patch introduces DFG ClearCatchLocals node, which clears catchOSREntryBuffer's active length.
We model ExtractCatchLocal and ClearCatchLocals appropriately in DFG clobberize too to make
this ClearCatchLocals valid.

The existing tests for ExtractCatchLocal just pass.

* dfg/DFGAbstractHeap.h:
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::parseBlock):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGDoesGC.cpp:
(JSC::DFG::doesGC):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGMayExit.cpp:
* dfg/DFGNodeType.h:
* dfg/DFGOSREntry.cpp:
(JSC::DFG::prepareCatchOSREntry):
* dfg/DFGPredictionPropagationPhase.cpp:
* dfg/DFGSafeToExecute.h:
(JSC::DFG::safeToExecute):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileClearCatchLocals):
* dfg/DFGSpeculativeJIT.h:
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* ftl/FTLCapabilities.cpp:
(JSC::FTL::canCompile):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileNode):
(JSC::FTL::DFG::LowerDFGToB3::compileClearCatchLocals):

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

18 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGAbstractHeap.h
Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Source/JavaScriptCore/dfg/DFGClobberize.h
Source/JavaScriptCore/dfg/DFGDoesGC.cpp
Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
Source/JavaScriptCore/dfg/DFGMayExit.cpp
Source/JavaScriptCore/dfg/DFGNodeType.h
Source/JavaScriptCore/dfg/DFGOSREntry.cpp
Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
Source/JavaScriptCore/dfg/DFGSafeToExecute.h
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
Source/JavaScriptCore/ftl/FTLCapabilities.cpp
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

index fcc2f17..9db39aa 100644 (file)
@@ -1,3 +1,50 @@
+2018-06-03  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        LayoutTests/fast/css/parsing-css-matches-7.html always abandons its Document (disabling JIT fixes it)
+        https://bugs.webkit.org/show_bug.cgi?id=186223
+
+        Reviewed by Keith Miller.
+
+        After preparing catchOSREntryBuffer, we do not clear the active length of this scratch buffer.
+        It makes this buffer conservative GC root, and allows it to hold GC objects unnecessarily long.
+
+        This patch introduces DFG ClearCatchLocals node, which clears catchOSREntryBuffer's active length.
+        We model ExtractCatchLocal and ClearCatchLocals appropriately in DFG clobberize too to make
+        this ClearCatchLocals valid.
+
+        The existing tests for ExtractCatchLocal just pass.
+
+        * dfg/DFGAbstractHeap.h:
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::parseBlock):
+        * dfg/DFGClobberize.h:
+        (JSC::DFG::clobberize):
+        * dfg/DFGDoesGC.cpp:
+        (JSC::DFG::doesGC):
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+        * dfg/DFGMayExit.cpp:
+        * dfg/DFGNodeType.h:
+        * dfg/DFGOSREntry.cpp:
+        (JSC::DFG::prepareCatchOSREntry):
+        * dfg/DFGPredictionPropagationPhase.cpp:
+        * dfg/DFGSafeToExecute.h:
+        (JSC::DFG::safeToExecute):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileClearCatchLocals):
+        * dfg/DFGSpeculativeJIT.h:
+        * dfg/DFGSpeculativeJIT32_64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * ftl/FTLCapabilities.cpp:
+        (JSC::FTL::canCompile):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileNode):
+        (JSC::FTL::DFG::LowerDFGToB3::compileClearCatchLocals):
+
 2018-06-02  Darin Adler  <darin@apple.com>
 
         [Cocoa] Update some code to be more ARC-compatible to prepare for future ARC adoption
index 4a4ebb1..76b38b1 100644 (file)
@@ -77,6 +77,7 @@ namespace JSC { namespace DFG {
     macro(JSWeakMapFields) \
     macro(JSWeakSetFields) \
     macro(InternalState) \
+    macro(CatchLocals) \
     macro(Absolute) \
     /* DOMJIT tells the heap range with the pair of integers. */\
     macro(DOMState) \
index dae05fd..3ccfaf6 100644 (file)
@@ -3529,6 +3529,7 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
     case LoopHint:
     case ZombieHint:
     case ExitOK:
+    case ClearCatchLocals:
         break;
 
     case CheckTypeInfoFlags: {
index caf9f05..e98dfe5 100644 (file)
@@ -5680,6 +5680,8 @@ void ByteCodeParser::parseBlock(unsigned limit)
                 addToGraph(MovHint, OpInfo(profile.m_operand), value);
                 localsToSet.uncheckedAppend(std::make_pair(operand, value));
             });
+            if (numberOfLocals)
+                addToGraph(ClearCatchLocals);
 
             if (!m_graph.m_maxLocalsForCatchOSREntry)
                 m_graph.m_maxLocalsForCatchOSREntry = 0;
index e19c749..6c58663 100644 (file)
@@ -144,9 +144,16 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
     case Check:
     case CheckVarargs:
     case ExtractOSREntryLocal:
-    case ExtractCatchLocal:
     case CheckStructureImmediate:
         return;
+
+    case ExtractCatchLocal:
+        read(AbstractHeap(CatchLocals, node->catchOSREntryIndex()));
+        return;
+
+    case ClearCatchLocals:
+        write(CatchLocals);
+        return;
         
     case LazyJSConstant:
         // We should enable CSE of LazyJSConstant. It's a little annoying since LazyJSValue has
index 20bffec..d2333da 100644 (file)
@@ -224,8 +224,9 @@ bool doesGC(Graph& graph, Node* node)
     case WeakSetAdd:
     case WeakMapSet:
     case Unreachable:
-    case ExtractCatchLocal:
     case ExtractOSREntryLocal:
+    case ExtractCatchLocal:
+    case ClearCatchLocals:
     case CheckTierUpInLoop:
     case CheckTierUpAtReturn:
     case CheckTierUpAndOSREnter:
index 2a7acc4..9d6ef7a 100644 (file)
@@ -2167,6 +2167,7 @@ private:
         case Unreachable:
         case ExtractOSREntryLocal:
         case ExtractCatchLocal:
+        case ClearCatchLocals:
         case LoopHint:
         case MovHint:
         case InitializeEntrypointArguments:
index d41d1a6..3c91e47 100644 (file)
@@ -92,6 +92,7 @@ ExitMode mayExitImpl(Graph& graph, Node* node, StateType& state)
     case ValueRep:
     case ExtractOSREntryLocal:
     case ExtractCatchLocal:
+    case ClearCatchLocals:
     case LogicalNot:
     case NotifyWrite:
     case PutStructure:
index 6164c7f..a50e520 100644 (file)
@@ -96,6 +96,7 @@ namespace JSC { namespace DFG {
     /* variable from the scratch buffer. */\
     macro(ExtractOSREntryLocal, NodeResultJS) \
     macro(ExtractCatchLocal, NodeResultJS) \
+    macro(ClearCatchLocals, NodeMustGenerate) \
     \
     /* Tier-up checks from the DFG to the FTL. */\
     macro(CheckTierUpInLoop, NodeMustGenerate) \
index 13e1577..438f8f2 100644 (file)
@@ -400,6 +400,7 @@ MacroAssemblerCodePtr<ExceptionHandlerPtrTag> prepareCatchOSREntry(ExecState* ex
         ++index;
     });
 
+    // The active length of catchOSREntryBuffer will be zeroed by ClearCatchLocals node.
     dfgCommon->catchOSREntryBuffer->setActiveLength(sizeof(JSValue) * index);
     return catchEntrypoint->machineCode;
 }
index 0119a25..e0dd115 100644 (file)
@@ -1214,6 +1214,7 @@ private:
         case InitializeEntrypointArguments:
         case WeakSetAdd:
         case WeakMapSet:
+        case ClearCatchLocals:
             break;
             
         // This gets ignored because it only pretends to produce a value.
index 93916d8..267c8a1 100644 (file)
@@ -373,6 +373,7 @@ bool safeToExecute(AbstractStateType& state, Graph& graph, Node* node, bool igno
     case Unreachable:
     case ExtractOSREntryLocal:
     case ExtractCatchLocal:
+    case ClearCatchLocals:
     case CheckTierUpInLoop:
     case CheckTierUpAtReturn:
     case CheckTierUpAndOSREnter:
index 6996d42..b04bde2 100644 (file)
@@ -12939,6 +12939,17 @@ void SpeculativeJIT::compileExtractCatchLocal(Node* node)
     jsValueResult(resultRegs, node);
 }
 
+void SpeculativeJIT::compileClearCatchLocals(Node* node)
+{
+    ScratchBuffer* scratchBuffer = m_jit.jitCode()->common.catchOSREntryBuffer;
+    ASSERT(scratchBuffer);
+    GPRTemporary scratch(this);
+    GPRReg scratchGPR = scratch.gpr();
+    m_jit.move(TrustedImmPtr(scratchBuffer->addressOfActiveLength()), scratchGPR);
+    m_jit.storePtr(TrustedImmPtr(nullptr), scratchGPR);
+    noResult(node);
+}
+
 void SpeculativeJIT::compileProfileType(Node* node)
 {
     JSValueOperand value(this, node->child1());
index 848fd3b..ae7fe8b 100644 (file)
@@ -1480,6 +1480,7 @@ public:
     void compileLogShadowChickenTail(Node*);
     void compileHasIndexedProperty(Node*);
     void compileExtractCatchLocal(Node*);
+    void compileClearCatchLocals(Node*);
     void compileProfileType(Node*);
 
     void moveTrueTo(GPRReg);
index 93d7710..2dc247e 100644 (file)
@@ -4039,6 +4039,10 @@ void SpeculativeJIT::compile(Node* node)
         break;
     }
 
+    case ClearCatchLocals:
+        compileClearCatchLocals(node);
+        break;
+
     case CheckStructureOrEmpty:
         DFG_CRASH(m_jit.graph(), node, "CheckStructureOrEmpty only used in 64-bit DFG");
         break;
index 8c21cd3..b82370f 100644 (file)
@@ -4548,6 +4548,10 @@ void SpeculativeJIT::compile(Node* node)
         break;
     }
 
+    case ClearCatchLocals:
+        compileClearCatchLocals(node);
+        break;
+
 #if ENABLE(FTL_JIT)        
     case CheckTierUpInLoop: {
         MacroAssembler::Jump callTierUp = m_jit.branchAdd32(
index dc6b686..fcef953 100644 (file)
@@ -115,6 +115,7 @@ inline CapabilityLevel canCompile(Node* node)
     case Upsilon:
     case ExtractOSREntryLocal:
     case ExtractCatchLocal:
+    case ClearCatchLocals:
     case LoopHint:
     case SkipScope:
     case GetGlobalObject:
index 56bfad5..de31efb 100644 (file)
@@ -567,6 +567,9 @@ private:
         case ExtractCatchLocal:
             compileExtractCatchLocal();
             break;
+        case ClearCatchLocals:
+            compileClearCatchLocals();
+            break;
         case GetStack:
             compileGetStack();
             break;
@@ -1694,6 +1697,13 @@ private:
         EncodedJSValue* buffer = static_cast<EncodedJSValue*>(m_ftlState.jitCode->common.catchOSREntryBuffer->dataBuffer());
         setJSValue(m_out.load64(m_out.absolute(buffer + m_node->catchOSREntryIndex())));
     }
+
+    void compileClearCatchLocals()
+    {
+        ScratchBuffer* scratchBuffer = m_ftlState.jitCode->common.catchOSREntryBuffer;
+        ASSERT(scratchBuffer);
+        m_out.storePtr(m_out.constIntPtr(0), m_out.absolute(scratchBuffer->addressOfActiveLength()));
+    }
     
     void compileGetStack()
     {