We should not pop from an empty stack in the Wasm function parser.
authorkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 2 Nov 2016 17:48:23 +0000 (17:48 +0000)
committerkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 2 Nov 2016 17:48:23 +0000 (17:48 +0000)
https://bugs.webkit.org/show_bug.cgi?id=164275

Reviewed by Filip Pizlo.

This patch prevents an issue in the wasm parser where we might
accidentially pop from the expression stack without if there
are no entries. It also fixes a similar issue with else
blocks where a user might try to put an else on the top level
of a function.

* wasm/WasmB3IRGenerator.cpp:
* wasm/WasmFunctionParser.h:
(JSC::Wasm::FunctionParser<Context>::parseExpression):
(JSC::Wasm::FunctionParser<Context>::popExpressionStack):
* wasm/WasmValidate.cpp:
(JSC::Wasm::Validate::setErrorMessage):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp
Source/JavaScriptCore/wasm/WasmFunctionParser.h
Source/JavaScriptCore/wasm/WasmValidate.cpp

index 68a1689..4633e7a 100644 (file)
@@ -1,3 +1,23 @@
+2016-11-02  Keith Miller  <keith_miller@apple.com>
+
+        We should not pop from an empty stack in the Wasm function parser.
+        https://bugs.webkit.org/show_bug.cgi?id=164275
+
+        Reviewed by Filip Pizlo.
+
+        This patch prevents an issue in the wasm parser where we might
+        accidentially pop from the expression stack without if there
+        are no entries. It also fixes a similar issue with else
+        blocks where a user might try to put an else on the top level
+        of a function.
+
+        * wasm/WasmB3IRGenerator.cpp:
+        * wasm/WasmFunctionParser.h:
+        (JSC::Wasm::FunctionParser<Context>::parseExpression):
+        (JSC::Wasm::FunctionParser<Context>::popExpressionStack):
+        * wasm/WasmValidate.cpp:
+        (JSC::Wasm::Validate::setErrorMessage):
+
 2016-11-02  Romain Bellessort  <romain.bellessort@crf.canon.fr>
 
         [Readable Streams API] Enable creation of ReadableByteStreamController
index 07b329e..1de6e27 100644 (file)
@@ -212,6 +212,8 @@ public:
 
     void dump(const Vector<ControlType>& controlStack, const ExpressionList& expressionStack);
 
+    void setErrorMessage(String&&) { UNREACHABLE_FOR_PLATFORM(); }
+
 private:
     ExpressionType emitCheckAndPreparePointer(ExpressionType pointer, uint32_t offset, uint32_t sizeOfOp);
     ExpressionType emitLoadOp(LoadOpType, Origin, ExpressionType pointer, uint32_t offset);
index 6e0f2b1..4238696 100644 (file)
@@ -56,6 +56,8 @@ private:
     bool WARN_UNUSED_RETURN parseUnreachableExpression(OpType);
     bool WARN_UNUSED_RETURN unifyControl(Vector<ExpressionType>&, unsigned level);
 
+    bool WARN_UNUSED_RETURN popExpressionStack(ExpressionType& result);
+
     Context& m_context;
     Vector<ExpressionType, 1> m_expressionStack;
     Vector<ControlType> m_controlStack;
@@ -144,8 +146,14 @@ bool FunctionParser<Context>::parseExpression(OpType op)
 {
     switch (op) {
     FOR_EACH_WASM_BINARY_OP(CREATE_CASE) {
-        ExpressionType right = m_expressionStack.takeLast();
-        ExpressionType left = m_expressionStack.takeLast();
+        ExpressionType right;
+        if (!popExpressionStack(right))
+            return false;
+
+        ExpressionType left;
+        if (!popExpressionStack(left))
+            return false;
+
         ExpressionType result;
         if (!m_context.binaryOp(static_cast<BinaryOpType>(op), left, right, result))
             return false;
@@ -154,9 +162,12 @@ bool FunctionParser<Context>::parseExpression(OpType op)
     }
 
     FOR_EACH_WASM_UNARY_OP(CREATE_CASE) {
-        ExpressionType arg = m_expressionStack.takeLast();
+        ExpressionType value;
+        if (!popExpressionStack(value))
+            return false;
+
         ExpressionType result;
-        if (!m_context.unaryOp(static_cast<UnaryOpType>(op), arg, result))
+        if (!m_context.unaryOp(static_cast<UnaryOpType>(op), value, result))
             return false;
         m_expressionStack.append(result);
         return true;
@@ -171,7 +182,10 @@ bool FunctionParser<Context>::parseExpression(OpType op)
         if (!parseVarUInt32(offset))
             return false;
 
-        ExpressionType pointer = m_expressionStack.takeLast();
+        ExpressionType pointer;
+        if (!popExpressionStack(pointer))
+            return false;
+
         ExpressionType result;
         if (!m_context.load(static_cast<LoadOpType>(op), pointer, result, offset))
             return false;
@@ -188,8 +202,14 @@ bool FunctionParser<Context>::parseExpression(OpType op)
         if (!parseVarUInt32(offset))
             return false;
 
-        ExpressionType value = m_expressionStack.takeLast();
-        ExpressionType pointer = m_expressionStack.takeLast();
+        ExpressionType value;
+        if (!popExpressionStack(value))
+            return false;
+
+        ExpressionType pointer;
+        if (!popExpressionStack(pointer))
+            return false;
+
         return m_context.store(static_cast<StoreOpType>(op), pointer, value, offset);
     }
 
@@ -227,7 +247,9 @@ bool FunctionParser<Context>::parseExpression(OpType op)
         uint32_t index;
         if (!parseVarUInt32(index))
             return false;
-        ExpressionType value = m_expressionStack.takeLast();
+        ExpressionType value;
+        if (!popExpressionStack(value))
+            return false;
         return m_context.setLocal(index, value);
     }
 
@@ -284,7 +306,10 @@ bool FunctionParser<Context>::parseExpression(OpType op)
         if (!parseValueType(inlineSignature))
             return false;
 
-        ExpressionType condition = m_expressionStack.takeLast();
+        ExpressionType condition;
+        if (!popExpressionStack(condition))
+            return false;
+
         ControlType control;
         if (!m_context.addIf(condition, inlineSignature, control))
             return false;
@@ -294,6 +319,10 @@ bool FunctionParser<Context>::parseExpression(OpType op)
     }
 
     case OpType::Else: {
+        if (!m_controlStack.size()) {
+            m_context.setErrorMessage("Attempted to use else block at the top-level of a function");
+            return false;
+        }
         return m_context.addElse(m_controlStack.last(), m_expressionStack);
     }
 
@@ -304,9 +333,10 @@ bool FunctionParser<Context>::parseExpression(OpType op)
             return false;
 
         ExpressionType condition = Context::emptyExpression;
-        if (op == OpType::BrIf)
-            condition = m_expressionStack.takeLast();
-        else
+        if (op == OpType::BrIf) {
+            if (!popExpressionStack(condition))
+                return false;
+        } else
             m_unreachableBlocks = 1;
 
         ControlType& data = m_controlStack[m_controlStack.size() - 1 - target];
@@ -316,8 +346,11 @@ bool FunctionParser<Context>::parseExpression(OpType op)
 
     case OpType::Return: {
         Vector<ExpressionType, 1> returnValues;
-        if (m_signature->returnType != Void)
-            returnValues.append(m_expressionStack.takeLast());
+        if (m_signature->returnType != Void) {
+            ExpressionType returnValue;
+            if (!popExpressionStack(returnValue))
+            returnValues.append(returnValue);
+        }
 
         m_unreachableBlocks = 1;
         return m_context.addReturn(returnValues);
@@ -403,6 +436,18 @@ bool FunctionParser<Context>::parseUnreachableExpression(OpType op)
     return true;
 }
 
+template<typename Context>
+bool FunctionParser<Context>::popExpressionStack(ExpressionType& result)
+{
+    if (!m_expressionStack.size()) {
+        result = m_expressionStack.takeLast();
+        return true;
+    }
+
+    m_context.setErrorMessage("Attempted to use an stack value when none existed");
+    return false;
+}
+
 #undef CREATE_CASE
 
 } } // namespace JSC::Wasm
index b697e44..b809cdd 100644 (file)
@@ -104,6 +104,7 @@ public:
 
     void dump(const Vector<ControlType>& controlStack, const ExpressionList& expressionStack);
 
+    void setErrorMessage(String&& message) { ASSERT(m_errorMessage.isNull()); m_errorMessage = WTFMove(message); }
     String errorMessage() const { return m_errorMessage; }
     Validate(ExpressionType returnType)
         : m_returnType(returnType)