WebAssembly: implement the elements section
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Dec 2016 01:31:43 +0000 (01:31 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Dec 2016 01:31:43 +0000 (01:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=165715

Reviewed by Keith Miller.

JSTests:

* wasm/Builder.js:
(export.default.Builder.prototype._registerSectionBuilders.switch.case.string_appeared_here.this.section):
(export.default.Builder.prototype._registerSectionBuilders.switch):
* wasm/Builder_WebAssemblyBinary.js:
(const.emitters.Element):
* wasm/function-tests/basic-element.js: Added.
* wasm/js-api/element.js: Added.
(assertBadBinary):
(assertBadBinary.badInstantiation):

Source/JavaScriptCore:

This is a straight forward implementation of the Element
section in the Wasm spec:
https://github.com/WebAssembly/design/blob/master/BinaryEncoding.md#element-section

There are a few ambiguities I encountered when implementing this, so I've
filed bugs against the Wasm design repo, and corresponding bugzilla bugs
for us to address after they've been discussed by the various Wasm folks:
- https://bugs.webkit.org/show_bug.cgi?id=165827
- https://bugs.webkit.org/show_bug.cgi?id=165826
- https://bugs.webkit.org/show_bug.cgi?id=165825

* wasm/WasmFormat.h:
* wasm/WasmModuleParser.cpp:
(JSC::Wasm::ModuleParser::parseElement):
(JSC::Wasm::ModuleParser::parseInitExpr):
(JSC::Wasm::ModuleParser::parseData):
* wasm/WasmModuleParser.h:
* wasm/js/WebAssemblyModuleRecord.cpp:
(JSC::WebAssemblyModuleRecord::evaluate):

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

JSTests/ChangeLog
JSTests/wasm/Builder.js
JSTests/wasm/Builder_WebAssemblyBinary.js
JSTests/wasm/function-tests/basic-element.js [new file with mode: 0644]
JSTests/wasm/js-api/element.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/wasm/WasmFormat.h
Source/JavaScriptCore/wasm/WasmModuleParser.cpp
Source/JavaScriptCore/wasm/WasmModuleParser.h
Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp

index 68e8f70..7947658 100644 (file)
@@ -1,5 +1,22 @@
 2016-12-13  Saam Barati  <sbarati@apple.com>
 
+        WebAssembly: implement the elements section
+        https://bugs.webkit.org/show_bug.cgi?id=165715
+
+        Reviewed by Keith Miller.
+
+        * wasm/Builder.js:
+        (export.default.Builder.prototype._registerSectionBuilders.switch.case.string_appeared_here.this.section):
+        (export.default.Builder.prototype._registerSectionBuilders.switch):
+        * wasm/Builder_WebAssemblyBinary.js:
+        (const.emitters.Element):
+        * wasm/function-tests/basic-element.js: Added.
+        * wasm/js-api/element.js: Added.
+        (assertBadBinary):
+        (assertBadBinary.badInstantiation):
+
+2016-12-13  Saam Barati  <sbarati@apple.com>
+
         WebAssembly: implement the table section and table import
         https://bugs.webkit.org/show_bug.cgi?id=165716
 
index ad7a53b..322da03 100644 (file)
@@ -419,39 +419,39 @@ export default class Builder {
             case "Function":
                 this[section] = function() {
                     const s = this._addSection(section);
-                    const exportBuilder = {
+                    const functionBuilder = {
                         End: () => this
                         // FIXME: add ability to add this with whatever.
                     };
-                    return exportBuilder;
+                    return functionBuilder;
                 };
                 break;
 
             case "Table":
                 this[section] = function() {
                     const s = this._addSection(section);
-                    const exportBuilder = {
+                    const tableBuilder = {
                         End: () => this,
                         Table: ({initial, maximum, element}) => {
                             s.data.push({tableDescription: {initial, maximum, element}});
-                            return exportBuilder;
+                            return tableBuilder;
                         }
                     };
-                    return exportBuilder;
+                    return tableBuilder;
                 };
                 break;
 
             case "Memory":
                 this[section] = function() {
                     const s = this._addSection(section);
-                    const exportBuilder = {
+                    const memoryBuilder = {
                         End: () => this,
                         InitialMaxPages: (initial, max) => {
                             s.data.push({ initial, max });
-                            return exportBuilder;
+                            return memoryBuilder;
                         }
                     };
-                    return exportBuilder;
+                    return memoryBuilder;
                 };
                 break;
 
@@ -488,8 +488,18 @@ export default class Builder {
                 break;
 
             case "Element":
-                // FIXME implement element https://bugs.webkit.org/show_bug.cgi?id=161709
-                this[section] = () => { throw new Error(`Unimplemented: section type "${section}"`); };
+                this[section] = function() {
+                    const s = this._addSection(section);
+                    const elementBuilder = {
+                        End: () => this,
+                        Element: ({tableIndex = 0, offset, functionIndices}) => {
+                            s.data.push({tableIndex, offset, functionIndices});
+                            return elementBuilder;
+                        }
+                    };
+
+                    return elementBuilder;
+                };
                 break;
 
             case "Code":
index cebcb4a..349ca52 100644 (file)
@@ -135,7 +135,23 @@ const emitters = {
     Start: (section, bin) => {
         put(bin, "varuint32", section.data[0]);
     },
-    Element: (section, bin) => { throw new Error(`Not yet implemented`); },
+    Element: (section, bin) => {
+        const data = section.data;
+        put(bin, "varuint32", data.length);
+        for (const {tableIndex, offset, functionIndices} of data) {
+            put(bin, "varuint32", tableIndex);
+
+            // FIXME allow complex init_expr here. https://bugs.webkit.org/show_bug.cgi?id=165700
+            // For now we only handle i32.const as offset.
+            put(bin, "uint8", WASM.description.opcode["i32.const"].value);
+            put(bin, WASM.description.opcode["i32.const"].immediate[0].type, offset);
+            put(bin, "uint8", WASM.description.opcode["end"].value);
+
+            put(bin, "varuint32", functionIndices.length);
+            for (const functionIndex of functionIndices)
+                put(bin, "varuint32", functionIndex);
+        }
+    },
 
     Code: (section, bin) => {
         put(bin, "varuint32", section.data.length);
diff --git a/JSTests/wasm/function-tests/basic-element.js b/JSTests/wasm/function-tests/basic-element.js
new file mode 100644 (file)
index 0000000..114f3c8
--- /dev/null
@@ -0,0 +1,34 @@
+import Builder from '../Builder.js';
+import * as assert from '../assert.js';
+
+
+const tableDescription = {initial: 1, element: "anyfunc"};
+const builder = new Builder()
+    .Type().End()
+    .Import()
+        .Table("imp", "table", tableDescription)
+    .End()
+    .Function().End()
+    .Element()
+        .Element({tableIndex: 0, offset: 0, functionIndices: [0]})
+    .End()
+    .Code()
+        .Function("foo", {params: ["i32"], ret: "i32"})
+            .GetLocal(0)
+            .I32Const(42)
+            .I32Add()
+            .Return()
+        .End()
+    .End();
+
+const bin = builder.WebAssembly().get();
+const module = new WebAssembly.Module(bin);
+const table = new WebAssembly.Table(tableDescription);
+new WebAssembly.Instance(module, {imp: {table}});
+const foo = table.get(0);
+const objs = [];
+for (let i = 0; i < 10000; i++) {
+    objs.push(new String("foo"));
+    if (foo(20) !== 20 + 42)
+        throw new Error("bad!!!");
+}
diff --git a/JSTests/wasm/js-api/element.js b/JSTests/wasm/js-api/element.js
new file mode 100644 (file)
index 0000000..4c81c3e
--- /dev/null
@@ -0,0 +1,161 @@
+import Builder from '../Builder.js';
+import * as assert from '../assert.js';
+
+function assertBadBinary(builder, str) {
+    const bin = builder.WebAssembly().get();
+    let threw = false;
+    try {
+        new WebAssembly.Module(bin);
+    } catch(e) {
+        threw = true;
+        assert.truthy(e.toString().indexOf(str) !== -1);
+        assert.truthy(e instanceof WebAssembly.CompileError);
+    }
+    assert.truthy(threw);
+}
+
+const badElementSectionString = "couldn't parse section Element";
+
+{
+    // Bad element section b/c no Table section/import.
+    const builder = new Builder()
+        .Type().End()
+        .Function().End()
+        .Element()
+            .Element({tableIndex: 0, offset: 0, functionIndices: [0]})
+        .End()
+        .Code()
+            .Function("foo", {params: ["i32"], ret: "i32"})
+                .GetLocal(0)
+                .I32Const(42)
+                .I32Add()
+                .Return()
+            .End()
+        .End();
+
+    assertBadBinary(builder, badElementSectionString);
+}
+
+{
+    // Bad table index.
+    const builder = new Builder()
+        .Type().End()
+        .Function().End()
+        .Table()
+            .Table({element: "anyfunc", initial: 20})
+        .End()
+        .Element()
+            .Element({tableIndex: 1, offset: 0, functionIndices: [0]})
+        .End()
+        .Code()
+            .Function("foo", {params: ["i32"], ret: "i32"})
+                .GetLocal(0)
+                .I32Const(42)
+                .I32Add()
+                .Return()
+            .End()
+        .End();
+
+    assertBadBinary(builder, badElementSectionString);
+}
+
+{
+    // Overflow table maximum size.
+    const builder = new Builder()
+        .Type().End()
+        .Function().End()
+        .Table()
+            .Table({element: "anyfunc", initial: 20, maximum: 20})
+        .End()
+        .Element()
+            .Element({offset: 19, functionIndices: [0, 1]})
+        .End()
+        .Code()
+            .Function("foo", {params: ["i32"], ret: "i32"})
+                .GetLocal(0)
+                .I32Const(42)
+                .I32Add()
+                .Return()
+            .End()
+        .End();
+
+    assertBadBinary(builder, badElementSectionString);
+}
+
+{
+    // Overflow table maximum size.
+    const builder = new Builder()
+        .Type().End()
+        .Function().End()
+        .Table()
+            .Table({element: "anyfunc", initial: 20, maximum: 20})
+        .End()
+        .Element()
+            .Element({offset: 20, functionIndices: [0]})
+        .End()
+        .Code()
+            .Function("foo", {params: ["i32"], ret: "i32"})
+                .GetLocal(0)
+                .I32Const(42)
+                .I32Add()
+                .Return()
+            .End()
+        .End();
+
+    assertBadBinary(builder, badElementSectionString);
+}
+
+{
+    // Overflow function index space.
+    const builder = new Builder()
+        .Type().End()
+        .Function().End()
+        .Table()
+            .Table({element: "anyfunc", initial: 20, maximum: 20})
+        .End()
+        .Element()
+            .Element({offset: 0, functionIndices: [0, 0, 1]})
+        .End()
+        .Code()
+            .Function("foo", {params: ["i32"], ret: "i32"})
+                .GetLocal(0)
+                .I32Const(42)
+                .I32Add()
+                .Return()
+            .End()
+        .End();
+
+    assertBadBinary(builder, badElementSectionString);
+}
+
+{
+    function badInstantiation(actualTable, errorType, msg) {
+        // Overflow function index space.
+        const builder = new Builder()
+            .Type().End()
+            .Import()
+                .Table("imp", "table", {element: "anyfunc", initial: 19}) // unspecified maximum.
+            .End()
+            .Function().End()
+            .Element()
+                .Element({offset: 19, functionIndices: [0, 0, 0, 0, 0]})
+            .End()
+            .Code()
+                .Function("foo", {params: ["i32"], ret: "i32"})
+                    .GetLocal(0)
+                    .I32Const(42)
+                    .I32Add()
+                    .Return()
+                .End()
+            .End();
+
+        const bin = builder.WebAssembly().get();
+        const module = new WebAssembly.Module(bin);
+        assert.throws(() => new WebAssembly.Instance(module, {imp: {table: actualTable}}), errorType, msg);
+    }
+
+    for (let i = 19; i < 19 + 5; i++) {
+        const table = new WebAssembly.Table({element: "anyfunc", initial: i});
+        badInstantiation(table, RangeError, "Element is trying to set an out of bounds table index");
+    }
+}
index 1f676ba..950414a 100644 (file)
@@ -1,3 +1,30 @@
+2016-12-13  Saam Barati  <sbarati@apple.com>
+
+        WebAssembly: implement the elements section
+        https://bugs.webkit.org/show_bug.cgi?id=165715
+
+        Reviewed by Keith Miller.
+
+        This is a straight forward implementation of the Element
+        section in the Wasm spec:
+        https://github.com/WebAssembly/design/blob/master/BinaryEncoding.md#element-section
+        
+        There are a few ambiguities I encountered when implementing this, so I've
+        filed bugs against the Wasm design repo, and corresponding bugzilla bugs
+        for us to address after they've been discussed by the various Wasm folks:
+        - https://bugs.webkit.org/show_bug.cgi?id=165827
+        - https://bugs.webkit.org/show_bug.cgi?id=165826
+        - https://bugs.webkit.org/show_bug.cgi?id=165825
+
+        * wasm/WasmFormat.h:
+        * wasm/WasmModuleParser.cpp:
+        (JSC::Wasm::ModuleParser::parseElement):
+        (JSC::Wasm::ModuleParser::parseInitExpr):
+        (JSC::Wasm::ModuleParser::parseData):
+        * wasm/WasmModuleParser.h:
+        * wasm/js/WebAssemblyModuleRecord.cpp:
+        (JSC::WebAssemblyModuleRecord::evaluate):
+
 2016-12-13  Chris Dumez  <cdumez@apple.com>
 
         Unreviewed, rolling out r209544.
index 8589585..3910608 100644 (file)
@@ -147,6 +147,11 @@ struct Segment {
     }
 };
 
+struct Element {
+    uint32_t offset;
+    Vector<uint32_t> functionIndices;
+};
+
 class TableInformation {
 public:
     TableInformation()
@@ -185,6 +190,7 @@ struct ModuleInformation {
     Vector<Export> exports;
     std::optional<uint32_t> startFunctionIndexSpace;
     Vector<Segment::Ptr> data;
+    Vector<Element> elements;
     TableInformation tableInformation;
 
     ~ModuleInformation();
index f92679c..212a9a3 100644 (file)
@@ -492,8 +492,65 @@ bool ModuleParser::parseStart()
 
 bool ModuleParser::parseElement()
 {
-    // FIXME https://bugs.webkit.org/show_bug.cgi?id=161709
-    RELEASE_ASSERT_NOT_REACHED();
+    if (!m_hasTable)
+        return false;
+
+    uint32_t elementCount;
+    if (!parseVarUInt32(elementCount))
+        return false;
+    if (!m_module->elements.tryReserveCapacity(elementCount))
+        return false;
+
+    for (unsigned i = 0; i < elementCount; ++i) {
+        uint32_t tableIndex;
+        if (!parseVarUInt32(tableIndex))
+            return false;
+        // We only support one table for now.
+        if (tableIndex != 0)
+            return false;
+
+        uint32_t offset;
+        if (!parseInitExpr(offset))
+            return false;
+
+        uint32_t indexCount;
+        if (!parseVarUInt32(indexCount))
+            return false;
+
+        ASSERT(!!m_module->tableInformation);
+        if (std::optional<uint32_t> maximum = m_module->tableInformation.maximum()) {
+            // FIXME: should indexCount being zero be a validation error?
+            // https://bugs.webkit.org/show_bug.cgi?id=165826
+            if (indexCount) {
+                // FIXME: right now, provably out of bounds writes are validation errors.
+                // Should they be though?
+                // https://bugs.webkit.org/show_bug.cgi?id=165827
+                uint64_t lastWrittenIndex = static_cast<uint64_t>(indexCount) + static_cast<uint64_t>(offset) - 1;
+                if (lastWrittenIndex >= static_cast<uint64_t>(*maximum))
+                    return false;
+            }
+        }
+
+        Element element;
+        if (!element.functionIndices.tryReserveCapacity(indexCount))
+            return false;
+
+        element.offset = offset;
+
+        for (unsigned i = 0; i < indexCount; ++i) {
+            uint32_t functionIndex;
+            if (!parseVarUInt32(functionIndex))
+                return false;
+
+            if (functionIndex >= m_functionIndexSpace.size())
+                return false;
+
+            element.functionIndices.uncheckedAppend(functionIndex);
+        }
+
+        m_module->elements.uncheckedAppend(WTFMove(element));
+    }
+
     return true;
 }
 
@@ -520,6 +577,22 @@ bool ModuleParser::parseCode()
     return true;
 }
 
+bool ModuleParser::parseInitExpr(uint32_t& value)
+{
+    // FIXME allow complex init_expr here. https://bugs.webkit.org/show_bug.cgi?id=165700
+    // For now we only handle i32.const as offset.
+
+    uint8_t opcode;
+    uint8_t endOpcode;
+    if (!parseUInt8(opcode)
+        || opcode != Wasm::I32Const
+        || !parseVarUInt32(value)
+        || !parseUInt8(endOpcode)
+        || endOpcode != Wasm::End)
+        return false;
+    return true;
+}
+
 bool ModuleParser::parseData()
 {
     uint32_t segmentCount;
@@ -534,21 +607,13 @@ bool ModuleParser::parseData()
         if (verbose)
             dataLogLn("  segment #", segmentNumber);
         uint32_t index;
-        uint8_t opcode;
         uint32_t offset;
-        uint8_t endOpcode;
         uint32_t dataByteLength;
         if (!parseVarUInt32(index)
             || index)
             return false;
 
-        // FIXME allow complex init_expr here. https://bugs.webkit.org/show_bug.cgi?id=165700
-        // For now we only handle i32.const as offset.
-        if (!parseUInt8(opcode)
-            || opcode != Wasm::I32Const
-            || !parseVarUInt32(offset)
-            || !parseUInt8(endOpcode)
-            || endOpcode != Wasm::End)
+        if (!parseInitExpr(offset))
             return false;
         if (verbose)
             dataLogLn("    offset: ", offset);
index 4d1726e..3d8a861 100644 (file)
@@ -81,6 +81,7 @@ private:
     bool WARN_UNUSED_RETURN parseMemoryHelper(bool isImport);
     bool WARN_UNUSED_RETURN parseTableHelper(bool isImport);
     bool WARN_UNUSED_RETURN parseResizableLimits(uint32_t& initial, std::optional<uint32_t>& maximum);
+    bool WARN_UNUSED_RETURN parseInitExpr(uint32_t&);
 
     VM* m_vm;
     std::unique_ptr<ModuleInformation> m_module;
index 8c8e3ad..7ee44d9 100644 (file)
@@ -209,6 +209,51 @@ JSValue WebAssemblyModuleRecord::evaluate(ExecState* state)
         }
     }
 
+    {
+        JSWebAssemblyModule* module = m_instance->module();
+        const Wasm::ModuleInformation& moduleInformation = module->moduleInformation();
+        JSWebAssemblyTable* table = m_instance->table();
+        for (const Wasm::Element& element : moduleInformation.elements) {
+            // It should be a validation error to have any elements without a table.
+            // Also, it could be that a table wasn't imported, or that the table
+            // imported wasn't compatible. However, those should error out before
+            // getting here.
+            ASSERT(!!table);
+            if (!element.functionIndices.size())
+                continue;
+
+            uint32_t tableIndex = element.offset;
+            uint64_t lastWrittenIndex = static_cast<uint64_t>(tableIndex) + static_cast<uint64_t>(element.functionIndices.size()) - 1;
+            if (lastWrittenIndex >= table->size())
+                return JSValue::decode(throwVMRangeError(state, scope, ASCIILiteral("Element is trying to set an out of bounds table index")));
+
+            for (uint32_t i = 0; i < element.functionIndices.size(); ++i) {
+                // FIXME: This essentially means we're exporting an import.
+                // We need a story here. We need to create a WebAssemblyFunction
+                // for the import.
+                // https://bugs.webkit.org/show_bug.cgi?id=165510
+                uint32_t functionIndex = element.functionIndices[i];
+                if (functionIndex < module->importCount()) {
+                    return JSValue::decode(
+                        throwVMRangeError(state, scope, ASCIILiteral("Element is setting the table value with an import. This is not yet implemented. FIXME.")));
+                }
+
+                JSWebAssemblyCallee* jsEntrypointCallee = module->jsEntrypointCalleeFromFunctionIndexSpace(functionIndex);
+                JSWebAssemblyCallee* wasmEntrypointCallee = module->wasmEntrypointCalleeFromFunctionIndexSpace(functionIndex);
+                Wasm::Signature* signature = module->signatureForFunctionIndexSpace(functionIndex);
+                // FIXME: Say we export local function "foo" at funciton index 0.
+                // What if we also set it to the table an Element w/ index 0.
+                // Does (new Instance(...)).exports.foo === table.get(0)?
+                // https://bugs.webkit.org/show_bug.cgi?id=165825
+                WebAssemblyFunction* function = WebAssemblyFunction::create(
+                    vm, m_instance->globalObject(), signature->arguments.size(), String(), m_instance.get(), jsEntrypointCallee, wasmEntrypointCallee, signature);
+
+                table->setFunction(vm, tableIndex, function);
+                ++tableIndex;
+            }
+        }
+    }
+
     if (WebAssemblyFunction* startFunction = m_startFunction.get()) {
         ProtoCallFrame protoCallFrame;
         protoCallFrame.init(nullptr, startFunction, JSValue(), 1, nullptr);