diff --git a/Makefile b/Makefile index 8f9f26f5e..f88fb1e3d 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ NODE_OPTS = -TEST_TIMEOUT = 6000 +TEST_TIMEOUT = 10000 LERNA = ./node_modules/.bin/lerna FLOWTYPED = ./node_modules/.bin/flow-typed diff --git a/packages/repl/src/index.js b/packages/repl/src/index.js index 0db5bbe0d..319986f5c 100644 --- a/packages/repl/src/index.js +++ b/packages/repl/src/index.js @@ -12,7 +12,7 @@ const { } = require("webassemblyjs/lib/interpreter/kernel/memory"); const { decode } = require("@webassemblyjs/wasm-parser"); const t = require("@webassemblyjs/ast"); -const typeCheck = require("@webassemblyjs/validation").stack; +const { getValidationErrors } = require("@webassemblyjs/validation"); const denormalizeTypeReferences = require("@webassemblyjs/ast/lib/transform/denormalize-type-references") .transform; @@ -308,7 +308,7 @@ export function createRepl({ isVerbose, onAssert, onLog, onOk }) { if (enableTypeChecking === true) { denormalizeTypeReferences(moduleNode); - const typeErrors = typeCheck(t.program([moduleNode])); + const typeErrors = getValidationErrors(t.program([moduleNode])); if (typeErrors.length > 0) { const containsImmutableGlobalViolation = typeErrors.some(x => diff --git a/packages/validation/src/imports.js b/packages/validation/src/imports.js new file mode 100644 index 000000000..f92e0762c --- /dev/null +++ b/packages/validation/src/imports.js @@ -0,0 +1,26 @@ +// @flow + +import { traverse } from "@webassemblyjs/ast"; + +/** + * Determine if a sequence of instructions form a constant expression + * + * See https://webassembly.github.io/spec/core/multipage/valid/instructions.html#valid-constant + */ +export default function( + ast: Program /*, moduleContext: Object */ +): Array { + const errors = []; + + traverse(ast, { + ModuleImport({ node }) { + const { mutability } = node.descr; + + if (mutability === "var") { + errors.push("mutable globals cannot be imported"); + } + } + }); + + return errors; +} diff --git a/packages/validation/src/index.js b/packages/validation/src/index.js index 5796b4b8c..b1d011126 100644 --- a/packages/validation/src/index.js +++ b/packages/validation/src/index.js @@ -1,13 +1,13 @@ // @flow import importOrderValidate from "./import-order"; +import isConst from "./is-const"; import typeChecker from "./type-checker"; +import imports from "./imports"; +import { moduleContextFromModuleAST } from "@webassemblyjs/helper-module-context"; export default function validateAST(ast: Program) { - const errors = []; - - errors.push(...importOrderValidate(ast)); - errors.push(...typeChecker(ast)); + const errors = getValidationErrors(ast); if (errors.length !== 0) { const errorMessage = "Validation errors:\n" + errors.join("\n"); @@ -16,7 +16,35 @@ export default function validateAST(ast: Program) { } } -export { isConst } from "./is-const"; +export function getValidationErrors(ast: Program): Array { + const errors = []; + + let modules = []; + + // $FlowIgnore + if (ast.type === "Module") { + modules = [ast]; + } + + // $FlowIgnore + if (ast.type === "Program") { + modules = ast.body.filter(({ type }) => type === "Module"); + } + + modules.forEach(m => { + const moduleContext = moduleContextFromModuleAST(m); + + // $FlowIgnore + errors.push(...imports(ast, moduleContext)); + errors.push(...isConst(ast, moduleContext)); + errors.push(...importOrderValidate(ast)); + errors.push(...typeChecker(ast, moduleContext)); + }); + + return errors; +} + export { getType, typeEq } from "./type-inference"; +export { isConst }; export const stack = typeChecker; diff --git a/packages/validation/src/is-const.js b/packages/validation/src/is-const.js index f9227fdca..811fb8675 100644 --- a/packages/validation/src/is-const.js +++ b/packages/validation/src/is-const.js @@ -1,30 +1,24 @@ // @flow +import { traverse } from "@webassemblyjs/ast"; + /** * Determine if a sequence of instructions form a constant expression * * See https://webassembly.github.io/spec/core/multipage/valid/instructions.html#valid-constant - * - * TODO(sven): get_global x should check the mutability of x, but we don't have - * access to the program at this point. */ -export function isConst(instrs: Array): boolean { - if (instrs.length === 0) { - return false; - } - - return instrs.reduce((acc, instr) => { - // Bailout - if (acc === false) { - return acc; - } - +export default function isConst( + ast: Program, + moduleContext: Object +): Array { + function isConstInstruction(instr): boolean { if (instr.id === "const") { return true; } if (instr.id === "get_global") { - return true; + const index = instr.args[0].value; + return !moduleContext.isMutableGlobal(index); } // FIXME(sven): this shoudln't be needed, we need to inject our end @@ -34,5 +28,23 @@ export function isConst(instrs: Array): boolean { } return false; - }, true); + } + + const errors = []; + + traverse(ast, { + Global(path) { + const isValid = path.node.init.reduce( + (acc, instr) => acc && isConstInstruction(instr), + true + ); + if (!isValid) { + errors.push( + "constant expression required: initializer expression cannot reference mutable global" + ); + } + } + }); + + return errors; } diff --git a/packages/validation/test/fixtures/global-initializer/module.wast b/packages/validation/test/fixtures/global-initializer/module.wast new file mode 100644 index 000000000..7574ddbd9 --- /dev/null +++ b/packages/validation/test/fixtures/global-initializer/module.wast @@ -0,0 +1,7 @@ +(module + (global (mut i32) (i32.const 1)) + (global i32 (get_global 0)) + + (global i32 (i32.const 0)) + (global i32 (get_global 1)) +) diff --git a/packages/validation/test/fixtures/global-initializer/output.txt b/packages/validation/test/fixtures/global-initializer/output.txt new file mode 100644 index 000000000..5d9660c10 --- /dev/null +++ b/packages/validation/test/fixtures/global-initializer/output.txt @@ -0,0 +1 @@ +constant expression required: initializer expression cannot reference mutable global diff --git a/packages/validation/test/index.js b/packages/validation/test/index.js index 79dfa6719..8d0cbc079 100644 --- a/packages/validation/test/index.js +++ b/packages/validation/test/index.js @@ -22,7 +22,7 @@ describe("validation", () => { describe("wast", () => { const pre = f => { - const errors = validations.stack(parse(f)); + const errors = validations.getValidationErrors(parse(f)); return errorsToString(errors); }; @@ -35,7 +35,7 @@ describe("validation", () => { const module = wabt.parseWat(suite, f); const { buffer } = module.toBinary({ write_debug_names: false }); - const errors = validations.stack(decode(buffer)); + const errors = validations.getValidationErrors(decode(buffer)); return errorsToString(errors); }; diff --git a/packages/validation/test/is-const.js b/packages/validation/test/is-const.js deleted file mode 100644 index ba541e0cf..000000000 --- a/packages/validation/test/is-const.js +++ /dev/null @@ -1,42 +0,0 @@ -// @flow - -const t = require("@webassemblyjs/ast"); -const { assert } = require("chai"); - -const { isConst } = require("../lib/is-const"); - -function i32ConstOf(v) { - return t.objectInstruction("const", "i32", [t.numberLiteralFromRaw(v)]); -} - -describe("validation", () => { - describe("is const", () => { - it("should return true for a const expression", () => { - const exprs = [t.objectInstruction("const", "i32")]; - - assert.isTrue(isConst(exprs)); - }); - - it("should return false if not constant", () => { - const exprs = [t.objectInstruction("neg", "i32", [i32ConstOf(1)])]; - - assert.isFalse(isConst(exprs)); - }); - - it("should return false if empty", () => { - assert.isFalse(isConst([])); - }); - - it("should return false with multiple non const expressions", () => { - const exprs = [i32ConstOf(1), t.instruction("nop")]; - - assert.isFalse(isConst(exprs)); - }); - - it("should return true with multiple const expressions", () => { - const exprs = [i32ConstOf(1), i32ConstOf(2)]; - - assert.isTrue(isConst(exprs)); - }); - }); -}); diff --git a/packages/webassemblyjs/src/interpreter/runtime/values/global.js b/packages/webassemblyjs/src/interpreter/runtime/values/global.js index 304b99a5b..2af3ad91d 100644 --- a/packages/webassemblyjs/src/interpreter/runtime/values/global.js +++ b/packages/webassemblyjs/src/interpreter/runtime/values/global.js @@ -1,6 +1,6 @@ // @flow -import { isConst, getType, typeEq } from "@webassemblyjs/validation"; +import { getType, typeEq } from "@webassemblyjs/validation"; const { evaluate } = require("../../partial-evaluation"); const { CompileError } = require("../../../errors"); @@ -12,10 +12,6 @@ export function createInstance( let value; const { valtype, mutability } = node.globalType; - if (node.init.length > 0 && isConst(node.init) === false) { - throw new CompileError("constant expression required"); - } - // None or multiple constant expressions in the initializer seems not possible // TODO(sven): find a specification reference for that // FIXME(sven): +1 because of the implicit end, change the order of validations diff --git a/packages/webassemblyjs/src/interpreter/runtime/values/module.js b/packages/webassemblyjs/src/interpreter/runtime/values/module.js index e13961238..a4a205e2c 100644 --- a/packages/webassemblyjs/src/interpreter/runtime/values/module.js +++ b/packages/webassemblyjs/src/interpreter/runtime/values/module.js @@ -59,11 +59,6 @@ function instantiateImports( } function handleGlobalImport(node: ModuleImport, descr: GlobalType) { - // Validation: The mutability of globaltype must be const. - if (descr.mutability === "var") { - throw new CompileError("Mutable globals cannot be imported"); - } - const element = getExternalElementOrThrow(node.module, node.name); const externglobalinstance = externvalue.createGlobalInstance(