Title: [226875] branches/safari-605-branch

Diff

Modified: branches/safari-605-branch/JSTests/ChangeLog (226874 => 226875)


--- branches/safari-605-branch/JSTests/ChangeLog	2018-01-12 06:31:13 UTC (rev 226874)
+++ branches/safari-605-branch/JSTests/ChangeLog	2018-01-12 06:31:16 UTC (rev 226875)
@@ -1,5 +1,19 @@
 2018-01-11  Jason Marcell  <jmarc...@apple.com>
 
+        Cherry-pick r226811. rdar://problem/36458907
+
+    2018-01-11  Saam Barati  <sbar...@apple.com>
+
+            When inserting Unreachable in byte code parser we need to flush all the right things
+            https://bugs.webkit.org/show_bug.cgi?id=181509
+            <rdar://problem/36423110>
+
+            Reviewed by Mark Lam.
+
+            * stress/proper-flushing-when-we-insert-unreachable-after-force-exit-in-bytecode-parser.js: Added.
+
+2018-01-11  Jason Marcell  <jmarc...@apple.com>
+
         Cherry-pick r226767. rdar://problem/36450818
 
     2018-01-11  Saam Barati  <sbar...@apple.com>

Added: branches/safari-605-branch/JSTests/stress/proper-flushing-when-we-insert-unreachable-after-force-exit-in-bytecode-parser.js (0 => 226875)


--- branches/safari-605-branch/JSTests/stress/proper-flushing-when-we-insert-unreachable-after-force-exit-in-bytecode-parser.js	                        (rev 0)
+++ branches/safari-605-branch/JSTests/stress/proper-flushing-when-we-insert-unreachable-after-force-exit-in-bytecode-parser.js	2018-01-12 06:31:16 UTC (rev 226875)
@@ -0,0 +1,27 @@
+function test(b, f) {
+    if (b)
+        return f(b);
+}
+noInline(test);
+
+function throwError(b) {
+    if (b) {
+        try {
+            throw new Error;
+        } catch(e) { }
+    }
+    return 2;
+}
+noInline(throwError);
+
+function makeFoo() {
+    return function foo(b) {
+        throwError(b);
+        OSRExit();
+    }
+}
+
+let foos = [makeFoo(), makeFoo()];
+for (let i = 0; i < 10000; ++i) {
+    test(!!(i%2), foos[((Math.random() * 100) | 0) % foos.length]);
+}

Modified: branches/safari-605-branch/Source/_javascript_Core/ChangeLog (226874 => 226875)


--- branches/safari-605-branch/Source/_javascript_Core/ChangeLog	2018-01-12 06:31:13 UTC (rev 226874)
+++ branches/safari-605-branch/Source/_javascript_Core/ChangeLog	2018-01-12 06:31:16 UTC (rev 226875)
@@ -1,5 +1,35 @@
 2018-01-11  Jason Marcell  <jmarc...@apple.com>
 
+        Cherry-pick r226811. rdar://problem/36458907
+
+    2018-01-11  Saam Barati  <sbar...@apple.com>
+
+            When inserting Unreachable in byte code parser we need to flush all the right things
+            https://bugs.webkit.org/show_bug.cgi?id=181509
+            <rdar://problem/36423110>
+
+            Reviewed by Mark Lam.
+
+            I added code in r226655 that had its own mechanism for preserving liveness when
+            inserting Unreachable nodes after ForceOSRExit. There are two ways to preserve
+            liveness: PhantomLocal and Flush. Certain values *must* be flushed to the stack.
+            I got some of these values wrong, which was leading to a crash when recovering the
+            callee value from an inlined frame. Instead of making the same mistake and repeating
+            similar code again, this patch refactors this logic to be shared with the other
+            liveness preservation code in the DFG bytecode parser. This is what I should have
+            done in my initial patch.
+
+            * bytecode/InlineCallFrame.h:
+            (JSC::remapOperand):
+            * dfg/DFGByteCodeParser.cpp:
+            (JSC::DFG::flushImpl):
+            (JSC::DFG::flushForTerminalImpl):
+            (JSC::DFG::ByteCodeParser::flush):
+            (JSC::DFG::ByteCodeParser::flushForTerminal):
+            (JSC::DFG::ByteCodeParser::parse):
+
+2018-01-11  Jason Marcell  <jmarc...@apple.com>
+
         Cherry-pick r226788. rdar://problem/36450828
 
     2018-01-11  Michael Saboff  <msab...@apple.com>

Modified: branches/safari-605-branch/Source/_javascript_Core/bytecode/InlineCallFrame.h (226874 => 226875)


--- branches/safari-605-branch/Source/_javascript_Core/bytecode/InlineCallFrame.h	2018-01-12 06:31:13 UTC (rev 226874)
+++ branches/safari-605-branch/Source/_javascript_Core/bytecode/InlineCallFrame.h	2018-01-12 06:31:16 UTC (rev 226875)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2011-2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2011-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -257,6 +257,13 @@
     }
 }
 
+ALWAYS_INLINE VirtualRegister remapOperand(InlineCallFrame* inlineCallFrame, VirtualRegister reg)
+{
+    if (inlineCallFrame)
+        return VirtualRegister(reg.offset() + inlineCallFrame->stackOffset);
+    return reg;
+}
+
 } // namespace JSC
 
 namespace WTF {

Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (226874 => 226875)


--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2018-01-12 06:31:13 UTC (rev 226874)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2018-01-12 06:31:16 UTC (rev 226875)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2011-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2011-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -82,6 +82,51 @@
 dataLog(__VA_ARGS__); \
 } while (false)
 
+template <typename F1, typename F2>
+static ALWAYS_INLINE void flushImpl(Graph& graph, InlineCallFrame* inlineCallFrame, const F1& addFlushDirect, const F2& addPhantomLocalDirect)
+{
+    int numArguments;
+    if (inlineCallFrame) {
+        ASSERT(!graph.hasDebuggerEnabled());
+        numArguments = inlineCallFrame->argumentsWithFixup.size();
+        if (inlineCallFrame->isClosureCall)
+            addFlushDirect(remapOperand(inlineCallFrame, VirtualRegister(CallFrameSlot::callee)));
+        if (inlineCallFrame->isVarargs())
+            addFlushDirect(remapOperand(inlineCallFrame, VirtualRegister(CallFrameSlot::argumentCount)));
+    } else
+        numArguments = graph.baselineCodeBlockFor(inlineCallFrame)->numParameters();
+
+    for (unsigned argument = numArguments; argument-- > 1;)
+        addFlushDirect(remapOperand(inlineCallFrame, virtualRegisterForArgument(argument)));
+
+    if (!inlineCallFrame && graph.needsFlushedThis())
+        addFlushDirect(remapOperand(inlineCallFrame, virtualRegisterForArgument(0)));
+    else
+        addPhantomLocalDirect(remapOperand(inlineCallFrame, virtualRegisterForArgument(0)));
+
+    if (graph.needsScopeRegister())
+        addFlushDirect(graph.m_codeBlock->scopeRegister());
+}
+
+template <typename F1, typename F2>
+static ALWAYS_INLINE void flushForTerminalImpl(Graph& graph, CodeOrigin origin, const F1& addFlushDirect, const F2& addPhantomLocalDirect)
+{
+    origin.walkUpInlineStack([&] (CodeOrigin origin) {
+        unsigned bytecodeIndex = origin.bytecodeIndex;
+        InlineCallFrame* inlineCallFrame = origin.inlineCallFrame;
+        flushImpl(graph, inlineCallFrame, addFlushDirect, addPhantomLocalDirect);
+
+        CodeBlock* codeBlock = graph.baselineCodeBlockFor(inlineCallFrame);
+        FullBytecodeLiveness& fullLiveness = graph.livenessFor(codeBlock);
+        const FastBitVector& livenessAtBytecode = fullLiveness.getLiveness(bytecodeIndex);
+
+        for (unsigned local = codeBlock->m_numCalleeLocals; local--;) {
+            if (livenessAtBytecode[local])
+                addPhantomLocalDirect(remapOperand(inlineCallFrame, virtualRegisterForLocal(local)));
+        }
+    });
+}
+
 // === ByteCodeParser ===
 //
 // This class is used to compile the dataflow graph from a CodeBlock.
@@ -561,55 +606,16 @@
 
     void flush(InlineStackEntry* inlineStackEntry)
     {
-        int numArguments;
-        if (InlineCallFrame* inlineCallFrame = inlineStackEntry->m_inlineCallFrame) {
-            ASSERT(!m_hasDebuggerEnabled);
-            numArguments = inlineCallFrame->argumentsWithFixup.size();
-            if (inlineCallFrame->isClosureCall)
-                flushDirect(inlineStackEntry->remapOperand(VirtualRegister(CallFrameSlot::callee)));
-            if (inlineCallFrame->isVarargs())
-                flushDirect(inlineStackEntry->remapOperand(VirtualRegister(CallFrameSlot::argumentCount)));
-        } else
-            numArguments = inlineStackEntry->m_codeBlock->numParameters();
-        for (unsigned argument = numArguments; argument-- > 1;)
-            flushDirect(inlineStackEntry->remapOperand(virtualRegisterForArgument(argument)));
-        if (!inlineStackEntry->m_inlineCallFrame && m_graph.needsFlushedThis())
-            flushDirect(virtualRegisterForArgument(0));
-        else
-            phantomLocalDirect(virtualRegisterForArgument(0));
-
-        if (m_graph.needsScopeRegister())
-            flushDirect(m_codeBlock->scopeRegister());
+        auto addFlushDirect = [&] (VirtualRegister reg) { flushDirect(reg); };
+        auto addPhantomLocalDirect = [&] (VirtualRegister reg) { phantomLocalDirect(reg); };
+        flushImpl(m_graph, inlineStackEntry->m_inlineCallFrame, addFlushDirect, addPhantomLocalDirect);
     }
 
     void flushForTerminal()
     {
-        CodeOrigin origin = currentCodeOrigin();
-        unsigned bytecodeIndex = origin.bytecodeIndex;
-
-        for (InlineStackEntry* inlineStackEntry = m_inlineStackTop; inlineStackEntry; inlineStackEntry = inlineStackEntry->m_caller) {
-            flush(inlineStackEntry);
-
-            ASSERT(origin.inlineCallFrame == inlineStackEntry->m_inlineCallFrame);
-            InlineCallFrame* inlineCallFrame = inlineStackEntry->m_inlineCallFrame;
-            CodeBlock* codeBlock = m_graph.baselineCodeBlockFor(inlineCallFrame);
-            FullBytecodeLiveness& fullLiveness = m_graph.livenessFor(codeBlock);
-            const FastBitVector& livenessAtBytecode = fullLiveness.getLiveness(bytecodeIndex);
-
-            for (unsigned local = codeBlock->m_numCalleeLocals; local--;) {
-                if (livenessAtBytecode[local]) {
-                    VirtualRegister reg = virtualRegisterForLocal(local);
-                    if (inlineCallFrame)
-                        reg = inlineStackEntry->remapOperand(reg);
-                    phantomLocalDirect(reg);
-                }
-            }
-
-            if (inlineCallFrame) {
-                bytecodeIndex = inlineCallFrame->directCaller.bytecodeIndex;
-                origin = inlineCallFrame->directCaller;
-            }
-        }
+        auto addFlushDirect = [&] (VirtualRegister reg) { flushDirect(reg); };
+        auto addPhantomLocalDirect = [&] (VirtualRegister reg) { phantomLocalDirect(reg); };
+        flushForTerminalImpl(m_graph, currentCodeOrigin(), addFlushDirect, addPhantomLocalDirect);
     }
 
     void flushForReturn()
@@ -6597,19 +6603,20 @@
                     block->resize(nodeIndex + 1);
 
                     insertionSet.insertNode(block->size(), SpecNone, ExitOK, endOrigin);
-                    m_graph.forAllLiveInBytecode(endOrigin.semantic, [&] (VirtualRegister operand) {
+
+                    auto insertLivenessPreservingOp = [&] (NodeType op, VirtualRegister operand) {
                         VariableAccessData* variable = mapping.operand(operand);
-                        if (!variable)
+                        if (!variable) {
                             variable = newVariableAccessData(operand);
-
-                        auto op = PhantomLocal;
-                        if ((m_graph.needsScopeRegister() && operand == m_codeBlock->scopeRegister())
-                            || (operand.isArgument() && (operand != virtualRegisterForArgument(0) || m_graph.needsFlushedThis()))) {
-                            op = Flush;
+                            mapping.operand(operand) = variable;
                         }
                         insertionSet.insertNode(block->size(), SpecNone, op, endOrigin, OpInfo(variable));
-                    });
+                    };
+                    auto addFlushDirect = [&] (VirtualRegister operand) { insertLivenessPreservingOp(Flush, operand); };
+                    auto addPhantomLocalDirect = [&] (VirtualRegister operand) { insertLivenessPreservingOp(PhantomLocal, operand); };
 
+                    flushForTerminalImpl(m_graph, endOrigin.semantic, addFlushDirect, addPhantomLocalDirect);
+
                     insertionSet.insertNode(block->size(), SpecNone, Unreachable, endOrigin);
                     insertionSet.execute(block);
                     break;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to