Attachment 341855 Details for Bug 186223 – Patch

[patch] Patch

bug-186223-20180603083956.patch (text/plain), 13.75 KB, created by Yusuke Suzuki on 2018-06-02 16:39:57 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2018-06-02 16:39:57 PDT

Size: 13.75 KB

patch

obsolete

>Subversion Revision: 232444
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 9fb98b5e0344af42517574926e147b21df9912f9..6ab9dd0c0baaee0e55c6f8d529ada5dc09adfe91 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,50 @@
>+2018-06-02  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 NOBODY (OOPS!).
>+
>+        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-01  Yusuke Suzuki  <utatane.tea@gmail.com>
> 
>         Baseline op_jtrue emits an insane amount of code
>diff --git a/Source/JavaScriptCore/dfg/DFGAbstractHeap.h b/Source/JavaScriptCore/dfg/DFGAbstractHeap.h
>index 4a4ebb1341fb4facd316838cbf8f68edb66145e6..76b38b1dd81b2c9fb614c87637023e4b43989d64 100644
>--- a/Source/JavaScriptCore/dfg/DFGAbstractHeap.h
>+++ b/Source/JavaScriptCore/dfg/DFGAbstractHeap.h
>@@ -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) \
>diff --git a/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h b/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
>index dae05fdc70d0c682458248c29f6d9fc1e85651c0..3ccfaf6e48335392402a3170c6e748c4e0e78fed 100644
>--- a/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
>+++ b/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
>@@ -3529,6 +3529,7 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
>     case LoopHint:
>     case ZombieHint:
>     case ExitOK:
>+    case ClearCatchLocals:
>         break;
> 
>     case CheckTypeInfoFlags: {
>diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>index caf9f05363064d3e27d60b3b3d7d7f7471624108..e98dfe58995e684f5192517b48f2d1f5c5d73b1c 100644
>--- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>@@ -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;
>diff --git a/Source/JavaScriptCore/dfg/DFGClobberize.h b/Source/JavaScriptCore/dfg/DFGClobberize.h
>index e19c749912e3ca5a71e19a37d55e7d8e96423621..6c58663463e2198f31a3233ba814c9cc4763d23b 100644
>--- a/Source/JavaScriptCore/dfg/DFGClobberize.h
>+++ b/Source/JavaScriptCore/dfg/DFGClobberize.h
>@@ -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
>diff --git a/Source/JavaScriptCore/dfg/DFGDoesGC.cpp b/Source/JavaScriptCore/dfg/DFGDoesGC.cpp
>index 20bffecc9bc239ae686af197daf7eb27016b5c84..d2333daae685caaa8604916495b456f72ed6eeec 100644
>--- a/Source/JavaScriptCore/dfg/DFGDoesGC.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGDoesGC.cpp
>@@ -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:
>diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
>index 2a7acc49560130e566cb3e98475cf41efc587474..9d6ef7a18833a8e31f5fd69e86f2e9fbe4b98663 100644
>--- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
>@@ -2167,6 +2167,7 @@ class FixupPhase : public Phase {
>         case Unreachable:
>         case ExtractOSREntryLocal:
>         case ExtractCatchLocal:
>+        case ClearCatchLocals:
>         case LoopHint:
>         case MovHint:
>         case InitializeEntrypointArguments:
>diff --git a/Source/JavaScriptCore/dfg/DFGMayExit.cpp b/Source/JavaScriptCore/dfg/DFGMayExit.cpp
>index d41d1a6db8cfa5b27f81ce02d523a99a004afa85..3c91e47ea16a27f15560ec7deed29ae98737ce2f 100644
>--- a/Source/JavaScriptCore/dfg/DFGMayExit.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGMayExit.cpp
>@@ -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:
>diff --git a/Source/JavaScriptCore/dfg/DFGNodeType.h b/Source/JavaScriptCore/dfg/DFGNodeType.h
>index 6164c7fa16098bbcfa76b1405a531685a55a0c44..a50e5208c9621e157905283cb2556d173124719d 100644
>--- a/Source/JavaScriptCore/dfg/DFGNodeType.h
>+++ b/Source/JavaScriptCore/dfg/DFGNodeType.h
>@@ -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) \
>diff --git a/Source/JavaScriptCore/dfg/DFGOSREntry.cpp b/Source/JavaScriptCore/dfg/DFGOSREntry.cpp
>index 13e15777f883d89f12f986e8d98e66dd359ee610..438f8f21dfcfb9af9ddd6bbd4ca8fb459dad2a5f 100644
>--- a/Source/JavaScriptCore/dfg/DFGOSREntry.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGOSREntry.cpp
>@@ -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;
> }
>diff --git a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
>index 0119a25c4257d1addef405cf2898277d2780cb33..e0dd115b7e7301de4ed537c95fc7fdfae7ab662d 100644
>--- a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
>@@ -1214,6 +1214,7 @@ class PredictionPropagationPhase : public Phase {
>         case InitializeEntrypointArguments:
>         case WeakSetAdd:
>         case WeakMapSet:
>+        case ClearCatchLocals:
>             break;
>             
>         // This gets ignored because it only pretends to produce a value.
>diff --git a/Source/JavaScriptCore/dfg/DFGSafeToExecute.h b/Source/JavaScriptCore/dfg/DFGSafeToExecute.h
>index 93916d8fe21713f6e823532ddaed2e81b08a141c..267c8a1a5134ab8aae8315f165fe5f614820c359 100644
>--- a/Source/JavaScriptCore/dfg/DFGSafeToExecute.h
>+++ b/Source/JavaScriptCore/dfg/DFGSafeToExecute.h
>@@ -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:
>diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>index 6996d42ec53e7f92dd5de1686bbbefd270a10b7a..b04bde2f0712b5badf15208e4278e74ad30c9600 100644
>--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>@@ -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());
>diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
>index 848fd3bc144487fe8f33d9f5c70f0c9ccf387248..ae7fe8b92981d49e42aec5b2eb12f3bb172c117f 100644
>--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
>+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
>@@ -1480,6 +1480,7 @@ class SpeculativeJIT {
>     void compileLogShadowChickenTail(Node*);
>     void compileHasIndexedProperty(Node*);
>     void compileExtractCatchLocal(Node*);
>+    void compileClearCatchLocals(Node*);
>     void compileProfileType(Node*);
> 
>     void moveTrueTo(GPRReg);
>diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
>index 93d7710861948cf562c5c16d0d6a526df50b1099..2dc247ea01026492e3772b81a11d1b8092b6452b 100644
>--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
>@@ -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;
>diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
>index 8c21cd3bb10c7cfd256361bb3c50ac4007688710..b82370f76f019c7bb273dd1bdebef49856362b5c 100644
>--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
>@@ -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(
>diff --git a/Source/JavaScriptCore/ftl/FTLCapabilities.cpp b/Source/JavaScriptCore/ftl/FTLCapabilities.cpp
>index dc6b68628db06b698337cc676bb7fdd32dcd3fe0..fcef95389685851d8853a1fc1bdab14e9588e141 100644
>--- a/Source/JavaScriptCore/ftl/FTLCapabilities.cpp
>+++ b/Source/JavaScriptCore/ftl/FTLCapabilities.cpp
>@@ -115,6 +115,7 @@ inline CapabilityLevel canCompile(Node* node)
>     case Upsilon:
>     case ExtractOSREntryLocal:
>     case ExtractCatchLocal:
>+    case ClearCatchLocals:
>     case LoopHint:
>     case SkipScope:
>     case GetGlobalObject:
>diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>index 56bfad5cf9f123446935d977ea96ce5cf9ec59db..de31efb5fc1067a7a327d816dd358c31add0fc81 100644
>--- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>+++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>@@ -567,6 +567,9 @@ class LowerDFGToB3 {
>         case ExtractCatchLocal:
>             compileExtractCatchLocal();
>             break;
>+        case ClearCatchLocals:
>+            compileClearCatchLocals();
>+            break;
>         case GetStack:
>             compileGetStack();
>             break;
>@@ -1694,6 +1697,13 @@ class LowerDFGToB3 {
>         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()
>     {

Actions: View | Formatted Diff | Diff

Attachments on bug 186223: 341855