Use precise index masking for FTL GetByArgumentByVal
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 24 Jan 2018 00:40:12 +0000 (00:40 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 24 Jan 2018 00:40:12 +0000 (00:40 +0000)
https://bugs.webkit.org/show_bug.cgi?id=182006

Reviewed by Keith Miller.

This protects speculative out-of-bounds on arguments[index].

Making this work right involved fixing a possible overflow situation with
numberOfArgumentsToSkip.

* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::findArgumentPositionForLocal):
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::dump):
* dfg/DFGNode.h:
(JSC::DFG::Node::hasNumberOfArgumentsToSkip):
(JSC::DFG::Node::numberOfArgumentsToSkip):
* dfg/DFGStackLayoutPhase.cpp:
(JSC::DFG::StackLayoutPhase::run):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileGetMyArgumentByVal):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Source/JavaScriptCore/dfg/DFGGraph.cpp
Source/JavaScriptCore/dfg/DFGNode.h
Source/JavaScriptCore/dfg/DFGStackLayoutPhase.cpp
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

index 7746bd2..9941458 100644 (file)
@@ -1,3 +1,27 @@
+2018-01-23  Filip Pizlo  <fpizlo@apple.com>
+
+        Use precise index masking for FTL GetByArgumentByVal
+        https://bugs.webkit.org/show_bug.cgi?id=182006
+
+        Reviewed by Keith Miller.
+        
+        This protects speculative out-of-bounds on arguments[index].
+        
+        Making this work right involved fixing a possible overflow situation with
+        numberOfArgumentsToSkip.
+
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::findArgumentPositionForLocal):
+        * dfg/DFGGraph.cpp:
+        (JSC::DFG::Graph::dump):
+        * dfg/DFGNode.h:
+        (JSC::DFG::Node::hasNumberOfArgumentsToSkip):
+        (JSC::DFG::Node::numberOfArgumentsToSkip):
+        * dfg/DFGStackLayoutPhase.cpp:
+        (JSC::DFG::StackLayoutPhase::run):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileGetMyArgumentByVal):
+
 2018-01-23  David Kilzer  <ddkilzer@apple.com>
 
         Follow-up for: oss-fuzz jsc build is broken: StringImpl.h:27:10: fatal error: 'unicode/ustring.h' file not found
index 86c0a07..31f8982 100644 (file)
@@ -502,8 +502,6 @@ private:
                 break;
             if (operand.offset() < static_cast<int>(inlineCallFrame->stackOffset + CallFrame::headerSizeInRegisters))
                 continue;
-            if (operand.offset() == inlineCallFrame->stackOffset + CallFrame::thisArgumentOffset())
-                continue;
             if (operand.offset() >= static_cast<int>(inlineCallFrame->stackOffset + CallFrame::thisArgumentOffset() + inlineCallFrame->argumentsWithFixup.size()))
                 continue;
             int argument = VirtualRegister(operand.offset() - inlineCallFrame->stackOffset).toArgument();
index 67abe09..2bf238b 100644 (file)
@@ -217,6 +217,8 @@ void Graph::dump(PrintStream& out, const char* prefix, Node* node, DumpContext*
         out.print(comma, NodeFlagsDump(node->flags()));
     if (node->prediction())
         out.print(comma, SpeculationDump(node->prediction()));
+    if (node->hasNumberOfArgumentsToSkip())
+        out.print(comma, "numberOfArgumentsToSkip = ", node->numberOfArgumentsToSkip());
     if (node->hasArrayMode())
         out.print(comma, node->arrayMode());
     if (node->hasArithUnaryType())
index 953ad96..75c4647 100644 (file)
@@ -2640,10 +2640,15 @@ public:
     {
         m_misc.epoch = epoch.toUnsigned();
     }
+    
+    bool hasNumberOfArgumentsToSkip()
+    {
+        return op() == CreateRest || op() == PhantomCreateRest || op() == GetRestLength || op() == GetMyArgumentByVal || op() == GetMyArgumentByValOutOfBounds;
+    }
 
     unsigned numberOfArgumentsToSkip()
     {
-        ASSERT(op() == CreateRest || op() == PhantomCreateRest || op() == GetRestLength || op() == GetMyArgumentByVal || op() == GetMyArgumentByValOutOfBounds);
+        ASSERT(hasNumberOfArgumentsToSkip());
         return m_opInfo.as<unsigned>();
     }
 
index 8742c0e..1aeec68 100644 (file)
@@ -112,7 +112,7 @@ public:
                     CallFrameSlot::argumentCount + inlineCallFrame->stackOffset).toLocal());
             }
             
-            for (unsigned argument = inlineCallFrame->argumentsWithFixup.size(); argument-- > 1;) {
+            for (unsigned argument = inlineCallFrame->argumentsWithFixup.size(); argument--;) {
                 usedLocals.set(VirtualRegister(
                     virtualRegisterForArgument(argument).offset() +
                     inlineCallFrame->stackOffset).toLocal());
@@ -178,7 +178,7 @@ public:
                     allocation, VirtualRegister(inlineCallFrame->stackOffset + CallFrameSlot::argumentCount));
             }
             
-            for (unsigned argument = inlineCallFrame->argumentsWithFixup.size(); argument-- > 1;) {
+            for (unsigned argument = inlineCallFrame->argumentsWithFixup.size(); argument--;) {
                 ArgumentPosition& position = m_graph.m_argumentPositions[
                     data.argumentPositionStart + argument];
                 VariableAccessData* variable = position.someVariable();
index 1da9a8c..d3242de 100644 (file)
@@ -3975,17 +3975,20 @@ private:
         InlineCallFrame* inlineCallFrame = m_node->child1()->origin.semantic.inlineCallFrame;
         
         LValue index = lowInt32(m_node->child2());
-        if (m_node->numberOfArgumentsToSkip())
-            index = m_out.add(index, m_out.constInt32(m_node->numberOfArgumentsToSkip()));
         
-        LValue limit;
+        LValue originalLimit;
         if (inlineCallFrame && !inlineCallFrame->isVarargs())
-            limit = m_out.constInt32(inlineCallFrame->argumentCountIncludingThis - 1);
+            originalLimit = m_out.constInt32(inlineCallFrame->argumentCountIncludingThis);
         else {
             VirtualRegister argumentCountRegister = AssemblyHelpers::argumentCount(inlineCallFrame);
-            limit = m_out.sub(m_out.load32(payloadFor(argumentCountRegister)), m_out.int32One);
+            originalLimit = m_out.load32(payloadFor(argumentCountRegister));
         }
         
+        LValue limit = m_out.sub(originalLimit, m_out.int32One);
+        
+        if (m_node->numberOfArgumentsToSkip())
+            limit = m_out.sub(limit, m_out.constInt32(m_node->numberOfArgumentsToSkip()));
+        
         LValue isOutOfBounds = m_out.aboveOrEqual(index, limit);
         LBasicBlock continuation = nullptr;
         LBasicBlock lastNext = nullptr;
@@ -4001,17 +4004,30 @@ private:
         } else
             speculate(OutOfBounds, noValue(), 0, isOutOfBounds);
         
+        if (m_node->numberOfArgumentsToSkip())
+            index = m_out.add(index, m_out.constInt32(m_node->numberOfArgumentsToSkip()));
+        index = m_out.add(index, m_out.int32One);
+        
+        index = m_out.zeroExt(index, Int64);
+        
+        index = m_out.bitAnd(
+            index,
+            m_out.aShr(
+                m_out.sub(
+                    index,
+                    m_out.opaque(m_out.zeroExt(originalLimit, Int64))),
+                m_out.constInt32(63)));
+        
         TypedPointer base;
         if (inlineCallFrame) {
             if (inlineCallFrame->argumentCountIncludingThis > 1)
-                base = addressFor(inlineCallFrame->argumentsWithFixup[1].virtualRegister());
+                base = addressFor(inlineCallFrame->argumentsWithFixup[0].virtualRegister());
         } else
-            base = addressFor(virtualRegisterForArgument(1));
+            base = addressFor(virtualRegisterForArgument(0));
         
         LValue result;
         if (base) {
-            LValue pointer = m_out.baseIndex(
-                base.value(), m_out.zeroExt(index, pointerType()), ScaleEight);
+            LValue pointer = m_out.baseIndex(base.value(), index, ScaleEight);
             result = m_out.load64(TypedPointer(m_heaps.variables.atAnyIndex(), pointer));
         } else
             result = m_out.constInt64(JSValue::encode(jsUndefined()));