WebAssembly: handle a few corner cases

https://bugs.webkit.org/show_bug.cgi?id=162884 Reviewed by Keith Miller. JSTests: * stress/wasm/generate-wasmops-header.js: (const.opcodeIterator): max opcode value Source/JavaScriptCore: * wasm/JSWASMModule.cpp: missing include broke cmake build * wasm/WASMFunctionParser.h: (JSC::WASM::FunctionParser<Context>::parseBlock): check op is valid (JSC::WASM::FunctionParser<Context>::parseExpression): switch covers all cases * wasm/WASMOps.h: (JSC::WASM::isValidOpType): op is valid * wasm/WASMParser.h: (JSC::WASM::Parser::consumeString): avoid str[i] being one-past-the-end (JSC::WASM::Parser::parseUInt32): shift math around to avoid overflow git-svn-id: http://svn.webkit.org/repository/webkit/trunk@206794 268f45cc-cd09-0410-ab3c-d52691b4dbfc
paul99 · Oct 4, 2016 · ade1810 · ade1810
1 parent 852b413
commit ade1810
Show file tree

Hide file tree

Showing 7 changed files with 71 additions and 7 deletions.
diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
@@ -1,3 +1,13 @@
+2016-10-04  JF Bastien  <[email protected]>
+
+        WebAssembly: handle a few corner cases
+        https://bugs.webkit.org/show_bug.cgi?id=162884
+
+        Reviewed by Keith Miller.
+
+        * stress/wasm/generate-wasmops-header.js:
+        (const.opcodeIterator): max opcode value
+
 2016-10-03  JF Bastien  <[email protected]>
 
         Auto-generate WASMOps.h, share with testing JSON file

diff --git a/JSTests/stress/wasm/generate-wasmops-header.js b/JSTests/stress/wasm/generate-wasmops-header.js
@@ -4,12 +4,12 @@ const jsonFile = 'wasm.json';
 const wasm = JSON.parse(read(jsonFile));
 const opcodes = wasm.opcode;
 
-// Iterate through opcodes which match filter.
-const opcodeIterator = (filter) => {
+// Iterate through opcodes which match filter, and on each iteration yield ret.
+const opcodeIterator = (filter, ret = op => { return {name: op, opcode: opcodes[op]}; }) => {
     return function*() {
         for (let op in opcodes)
             if (filter(opcodes[op]))
-                yield {name: op, opcode: opcodes[op]};
+                yield ret(op);
     };
 };
 
@@ -38,6 +38,21 @@ const defines = [
     ...opcodeMacroizer(op => (op.category === "arithmetic" || op.category === "comparison") && op.parameter.length === 2),
     "\n\n"].join("");
 
+const opValueSet = new Set(opcodeIterator(op => true, op => opcodes[op].value)());
+const maxOpValue = Math.max(...opValueSet);
+const validOps = (() => {
+    // Create a bitset of valid ops.
+    let v = "";
+    for (let i = 0; i < maxOpValue / 8; ++i) {
+        let entry = 0;
+        for (let j = 0; j < 8; ++j)
+            if (opValueSet.has(i * 8 + j))
+                entry |= 1 << j;
+        v += (i ? ", " : "") + "0x" + entry.toString(16);
+    }
+    return v;
+})();
+
 const template = `/*
  * Copyright (C) 2016 Apple Inc. All rights reserved.
  *
@@ -69,6 +84,8 @@ const template = `/*
 
 #if ENABLE(WEBASSEMBLY)
 
+#include <cstdint>
+
 namespace JSC { namespace WASM {
 
 ${defines}
@@ -85,6 +102,14 @@ enum OpType : uint8_t {
     FOR_EACH_WASM_OP(CREATE_ENUM_VALUE)
 };
 
+template<typename Int>
+inline bool isValidOpType(Int i)
+{
+    // Bitset of valid ops.
+    static const uint8_t valid[] = { ${validOps} };
+    return 0 <= i && i <= ${maxOpValue} && (valid[i / 8] & (1 << (i % 8)));
+}
+
 enum class BinaryOpType : uint8_t {
     FOR_EACH_WASM_BINARY_OP(CREATE_ENUM_VALUE)
 };

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,20 @@
+2016-10-04  JF Bastien  <[email protected]>
+
+        WebAssembly: handle a few corner cases
+        https://bugs.webkit.org/show_bug.cgi?id=162884
+
+        Reviewed by Keith Miller.
+
+        * wasm/JSWASMModule.cpp: missing include broke cmake build
+        * wasm/WASMFunctionParser.h:
+        (JSC::WASM::FunctionParser<Context>::parseBlock): check op is valid
+        (JSC::WASM::FunctionParser<Context>::parseExpression): switch covers all cases
+        * wasm/WASMOps.h:
+        (JSC::WASM::isValidOpType): op is valid
+        * wasm/WASMParser.h:
+        (JSC::WASM::Parser::consumeString): avoid str[i] being one-past-the-end
+        (JSC::WASM::Parser::parseUInt32): shift math around to avoid overflow
+
 2016-10-04  Yusuke Suzuki  <[email protected]>
 
         REGRESSION (r206778): Release JSC test ChakraCore.yaml/ChakraCore/test/Error/validate_line_column.js.default failing

diff --git a/Source/JavaScriptCore/wasm/JSWASMModule.cpp b/Source/JavaScriptCore/wasm/JSWASMModule.cpp
@@ -35,6 +35,7 @@
 #include "JSCellInlines.h"
 #include "JSFunction.h"
 #include "SlotVisitorInlines.h"
+#include "StructureInlines.h"
 
 namespace JSC {
 

diff --git a/Source/JavaScriptCore/wasm/WASMFunctionParser.h b/Source/JavaScriptCore/wasm/WASMFunctionParser.h
@@ -101,7 +101,7 @@ bool FunctionParser<Context>::parseBlock()
 {
     while (true) {
         uint8_t op;
-        if (!parseUInt7(op))
+        if (!parseUInt7(op) || !isValidOpType(op))
             return false;
 
         if (verbose) {
@@ -260,8 +260,7 @@ bool FunctionParser<Context>::parseExpression(OpType op)
         return false;
     }
 
-    // Unknown opcode.
-    return false;
+    ASSERT_NOT_REACHED();
 }
 
 template<typename Context>

diff --git a/Source/JavaScriptCore/wasm/WASMOps.h b/Source/JavaScriptCore/wasm/WASMOps.h
@@ -29,6 +29,8 @@
 
 #if ENABLE(WEBASSEMBLY)
 
+#include <cstdint>
+
 namespace JSC { namespace WASM {
 
 #define FOR_EACH_WASM_SPECIAL_OP(macro) \
@@ -171,6 +173,14 @@ enum OpType : uint8_t {
     FOR_EACH_WASM_OP(CREATE_ENUM_VALUE)
 };
 
+template<typename Int>
+inline bool isValidOpType(Int i)
+{
+    // Bitset of valid ops.
+    static const uint8_t valid[] = { 0xff, 0x8f, 0xff, 0x2, 0xff, 0xff, 0x7f, 0xa, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x1f };
+    return 0 <= i && i <= 188 && (valid[i / 8] & (1 << (i % 8)));
+}
+
 enum class BinaryOpType : uint8_t {
     FOR_EACH_WASM_BINARY_OP(CREATE_ENUM_VALUE)
 };

diff --git a/Source/JavaScriptCore/wasm/WASMParser.h b/Source/JavaScriptCore/wasm/WASMParser.h
@@ -79,6 +79,8 @@ ALWAYS_INLINE bool Parser::consumeCharacter(char c)
 ALWAYS_INLINE bool Parser::consumeString(const char* str)
 {
     unsigned start = m_offset;
+    if (m_offset >= m_sourceLength)
+        return false;
     for (unsigned i = 0; str[i]; i++) {
         if (!consumeCharacter(str[i])) {
             m_offset = start;
@@ -90,7 +92,7 @@ ALWAYS_INLINE bool Parser::consumeString(const char* str)
 
 ALWAYS_INLINE bool Parser::parseUInt32(uint32_t& result)
 {
-    if (m_offset + 4 >= m_sourceLength)
+    if (m_sourceLength < 4 || m_offset >= m_sourceLength - 4)
         return false;
     result = *reinterpret_cast<const uint32_t*>(m_source.data() + m_offset);
     m_offset += 4;