Title: [228031] trunk
Revision
228031
Author
sbar...@apple.com
Date
2018-02-02 14:55:29 -0800 (Fri, 02 Feb 2018)

Log Message

When BytecodeParser inserts Unreachable after ForceOSRExit it needs to update ArgumentPositions for Flushes it inserts
https://bugs.webkit.org/show_bug.cgi?id=182368
<rdar://problem/36932466>

Reviewed by Mark Lam.

JSTests:

* stress/flush-after-force-exit-in-bytecodeparser-needs-to-update-argument-positions.js: Added.
(runNearStackLimit.t):
(runNearStackLimit):
(try.runNearStackLimit):
(catch):

Source/_javascript_Core:

When preserving liveness when inserting Unreachable nodes after ForceOSRExit,
we must add the VariableAccessData to the given argument position. Otherwise,
we may end up with a VariableAccessData that doesn't respect the shouldNeverUnbox bit.
If we end up with such a situation, it can lead to invalid IR after the
arguments elimination phase optimizes a GetByVal to a GetStack.

* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::flushImpl):
(JSC::DFG::ByteCodeParser::flushForTerminalImpl):
(JSC::DFG::ByteCodeParser::flush):
(JSC::DFG::ByteCodeParser::flushForTerminal):
(JSC::DFG::ByteCodeParser::InlineStackEntry::InlineStackEntry):
(JSC::DFG::ByteCodeParser::parse):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (228030 => 228031)


--- trunk/JSTests/ChangeLog	2018-02-02 22:52:56 UTC (rev 228030)
+++ trunk/JSTests/ChangeLog	2018-02-02 22:55:29 UTC (rev 228031)
@@ -1,3 +1,17 @@
+2018-02-02  Saam Barati  <sbar...@apple.com>
+
+        When BytecodeParser inserts Unreachable after ForceOSRExit it needs to update ArgumentPositions for Flushes it inserts
+        https://bugs.webkit.org/show_bug.cgi?id=182368
+        <rdar://problem/36932466>
+
+        Reviewed by Mark Lam.
+
+        * stress/flush-after-force-exit-in-bytecodeparser-needs-to-update-argument-positions.js: Added.
+        (runNearStackLimit.t):
+        (runNearStackLimit):
+        (try.runNearStackLimit):
+        (catch):
+
 2018-02-02  Yusuke Suzuki  <utatane....@gmail.com>
 
         Update test262 to Jan 30 version

Added: trunk/JSTests/stress/flush-after-force-exit-in-bytecodeparser-needs-to-update-argument-positions.js (0 => 228031)


--- trunk/JSTests/stress/flush-after-force-exit-in-bytecodeparser-needs-to-update-argument-positions.js	                        (rev 0)
+++ trunk/JSTests/stress/flush-after-force-exit-in-bytecodeparser-needs-to-update-argument-positions.js	2018-02-02 22:55:29 UTC (rev 228031)
@@ -0,0 +1,32 @@
+//@ runDefault("--useConcurrentGC=0", "--thresholdForJITAfterWarmUp=10", "--thresholdForJITSoon=10", "--thresholdForOptimizeAfterWarmUp=20", "--thresholdForOptimizeAfterLongWarmUp=20", "--thresholdForOptimizeSoon=20", "--thresholdForFTLOptimizeAfterWarmUp=20", "--thresholdForFTLOptimizeSoon=20", "--maximumEvalCacheableSourceLength=150000", "--maxPerThreadStackUsage=1048576")
+
+function runNearStackLimit(f) {
+    function t() {
+        try {
+            return t();
+        } catch (e) {
+            return f();
+        }
+    }
+    return t();
+};
+
+runNearStackLimit(() => { });
+runNearStackLimit(() => { });
+
+function f2(a, b) {
+    'use strict';
+    try {
+        a.push(arguments[0] + arguments[2] + a + undefinedVariable);
+    } catch (e) { }
+}
+
+try {
+    runNearStackLimit(() => {
+        return f2(1, 2, 3);
+    });
+} catch (e) {}
+
+try {
+    runNearStackLimit();
+} catch { }

Modified: trunk/Source/_javascript_Core/ChangeLog (228030 => 228031)


--- trunk/Source/_javascript_Core/ChangeLog	2018-02-02 22:52:56 UTC (rev 228030)
+++ trunk/Source/_javascript_Core/ChangeLog	2018-02-02 22:55:29 UTC (rev 228031)
@@ -1,3 +1,25 @@
+2018-02-02  Saam Barati  <sbar...@apple.com>
+
+        When BytecodeParser inserts Unreachable after ForceOSRExit it needs to update ArgumentPositions for Flushes it inserts
+        https://bugs.webkit.org/show_bug.cgi?id=182368
+        <rdar://problem/36932466>
+
+        Reviewed by Mark Lam.
+
+        When preserving liveness when inserting Unreachable nodes after ForceOSRExit,
+        we must add the VariableAccessData to the given argument position. Otherwise,
+        we may end up with a VariableAccessData that doesn't respect the shouldNeverUnbox bit.
+        If we end up with such a situation, it can lead to invalid IR after the
+        arguments elimination phase optimizes a GetByVal to a GetStack.
+
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::flushImpl):
+        (JSC::DFG::ByteCodeParser::flushForTerminalImpl):
+        (JSC::DFG::ByteCodeParser::flush):
+        (JSC::DFG::ByteCodeParser::flushForTerminal):
+        (JSC::DFG::ByteCodeParser::InlineStackEntry::InlineStackEntry):
+        (JSC::DFG::ByteCodeParser::parse):
+
 2018-02-02  Mark Lam  <mark....@apple.com>
 
         More ARM64_32 fixes.

Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (228030 => 228031)


--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2018-02-02 22:52:56 UTC (rev 228030)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2018-02-02 22:55:29 UTC (rev 228031)
@@ -525,17 +525,17 @@
             ASSERT(!m_graph.hasDebuggerEnabled());
             numArguments = inlineCallFrame->argumentsWithFixup.size();
             if (inlineCallFrame->isClosureCall)
-                addFlushDirect(remapOperand(inlineCallFrame, VirtualRegister(CallFrameSlot::callee)));
+                addFlushDirect(inlineCallFrame, remapOperand(inlineCallFrame, VirtualRegister(CallFrameSlot::callee)));
             if (inlineCallFrame->isVarargs())
-                addFlushDirect(remapOperand(inlineCallFrame, VirtualRegister(CallFrameSlot::argumentCount)));
+                addFlushDirect(inlineCallFrame, remapOperand(inlineCallFrame, VirtualRegister(CallFrameSlot::argumentCount)));
         } else
             numArguments = m_graph.baselineCodeBlockFor(inlineCallFrame)->numParameters();
 
         for (unsigned argument = numArguments; argument--;)
-            addFlushDirect(remapOperand(inlineCallFrame, virtualRegisterForArgument(argument)));
+            addFlushDirect(inlineCallFrame, remapOperand(inlineCallFrame, virtualRegisterForArgument(argument)));
 
         if (m_graph.needsScopeRegister())
-            addFlushDirect(m_graph.m_codeBlock->scopeRegister());
+            addFlushDirect(nullptr, m_graph.m_codeBlock->scopeRegister());
     }
 
     template<typename AddFlushDirectFunc, typename AddPhantomLocalDirectFunc>
@@ -553,7 +553,7 @@
 
                 for (unsigned local = codeBlock->m_numCalleeLocals; local--;) {
                     if (livenessAtBytecode[local])
-                        addPhantomLocalDirect(remapOperand(inlineCallFrame, virtualRegisterForLocal(local)));
+                        addPhantomLocalDirect(inlineCallFrame, remapOperand(inlineCallFrame, virtualRegisterForLocal(local)));
                 }
             });
     }
@@ -600,14 +600,14 @@
 
     void flush(InlineStackEntry* inlineStackEntry)
     {
-        auto addFlushDirect = [&] (VirtualRegister reg) { flushDirect(reg); };
+        auto addFlushDirect = [&] (InlineCallFrame*, VirtualRegister reg) { flushDirect(reg); };
         flushImpl(inlineStackEntry->m_inlineCallFrame, addFlushDirect);
     }
 
     void flushForTerminal()
     {
-        auto addFlushDirect = [&] (VirtualRegister reg) { flushDirect(reg); };
-        auto addPhantomLocalDirect = [&] (VirtualRegister reg) { phantomLocalDirect(reg); };
+        auto addFlushDirect = [&] (InlineCallFrame*, VirtualRegister reg) { flushDirect(reg); };
+        auto addPhantomLocalDirect = [&] (InlineCallFrame*, VirtualRegister reg) { phantomLocalDirect(reg); };
         flushForTerminalImpl(currentCodeOrigin(), addFlushDirect, addPhantomLocalDirect);
     }
 
@@ -1032,6 +1032,8 @@
     FrozenValue* m_constantOne;
     Vector<Node*, 16> m_constants;
 
+    HashMap<InlineCallFrame*, Vector<ArgumentPosition*>, WTF::DefaultHash<InlineCallFrame*>::Hash, WTF::NullableHashTraits<InlineCallFrame*>> m_inlineCallFrameToArgumentPositions;
+
     // The number of arguments passed to the function.
     unsigned m_numArguments;
     // The number of locals (vars + temporaries) used in the function.
@@ -6454,13 +6456,7 @@
     }
     
     int argumentCountIncludingThisWithFixup = std::max<int>(argumentCountIncludingThis, codeBlock->numParameters());
-    m_argumentPositions.resize(argumentCountIncludingThisWithFixup);
-    for (int i = 0; i < argumentCountIncludingThisWithFixup; ++i) {
-        byteCodeParser->m_graph.m_argumentPositions.append(ArgumentPosition());
-        ArgumentPosition* argumentPosition = &byteCodeParser->m_graph.m_argumentPositions.last();
-        m_argumentPositions[i] = argumentPosition;
-    }
-    
+
     if (m_caller) {
         // Inline case.
         ASSERT(codeBlock != byteCodeParser->m_codeBlock);
@@ -6511,6 +6507,14 @@
             m_switchRemap[i] = i;
     }
     
+    m_argumentPositions.resize(argumentCountIncludingThisWithFixup);
+    for (int i = 0; i < argumentCountIncludingThisWithFixup; ++i) {
+        byteCodeParser->m_graph.m_argumentPositions.append(ArgumentPosition());
+        ArgumentPosition* argumentPosition = &byteCodeParser->m_graph.m_argumentPositions.last();
+        m_argumentPositions[i] = argumentPosition;
+    }
+    byteCodeParser->m_inlineCallFrameToArgumentPositions.add(m_inlineCallFrame, m_argumentPositions);
+    
     byteCodeParser->m_inlineStackTop = this;
 }
 
@@ -6661,17 +6665,27 @@
 
                     insertionSet.insertNode(block->size(), SpecNone, ExitOK, endOrigin);
 
-                    auto insertLivenessPreservingOp = [&] (NodeType op, VirtualRegister operand) {
+                    auto insertLivenessPreservingOp = [&] (InlineCallFrame* inlineCallFrame, NodeType op, VirtualRegister operand) {
                         VariableAccessData* variable = mapping.operand(operand);
                         if (!variable) {
                             variable = newVariableAccessData(operand);
                             mapping.operand(operand) = variable;
                         }
+
+                        VirtualRegister argument = operand - (inlineCallFrame ? inlineCallFrame->stackOffset : 0);
+                        if (argument.isArgument() && !argument.isHeader()) {
+                            const Vector<ArgumentPosition*>& arguments = m_inlineCallFrameToArgumentPositions.get(inlineCallFrame);
+                            arguments[argument.toArgument()]->addVariable(variable);
+                        }
+
                         insertionSet.insertNode(block->size(), SpecNone, op, endOrigin, OpInfo(variable));
                     };
-                    auto addFlushDirect = [&] (VirtualRegister operand) { insertLivenessPreservingOp(Flush, operand); };
-                    auto addPhantomLocalDirect = [&] (VirtualRegister operand) { insertLivenessPreservingOp(PhantomLocal, operand); };
-
+                    auto addFlushDirect = [&] (InlineCallFrame* inlineCallFrame, VirtualRegister operand) {
+                        insertLivenessPreservingOp(inlineCallFrame, Flush, operand);
+                    };
+                    auto addPhantomLocalDirect = [&] (InlineCallFrame* inlineCallFrame, VirtualRegister operand) {
+                        insertLivenessPreservingOp(inlineCallFrame, PhantomLocal, operand);
+                    };
                     flushForTerminalImpl(endOrigin.semantic, addFlushDirect, addPhantomLocalDirect);
 
                     insertionSet.insertNode(block->size(), SpecNone, Unreachable, endOrigin);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to