diff --git a/packages/helper-module-context/src/index.js b/packages/helper-module-context/src/index.js index a742eda53..9e357f5a4 100644 --- a/packages/helper-module-context/src/index.js +++ b/packages/helper-module-context/src/index.js @@ -207,7 +207,7 @@ export class ModuleContext { defineGlobal(global /*: Global*/) { const type = global.globalType.valtype; - const mutability = global.mutability; + const mutability = global.globalType.mutability; this.globals.push({ type, mutability }); diff --git a/packages/validation/src/index.js b/packages/validation/src/index.js index 5796b4b8c..6ebcce526 100644 --- a/packages/validation/src/index.js +++ b/packages/validation/src/index.js @@ -1,13 +1,12 @@ // @flow import importOrderValidate from "./import-order"; +import isConst from "./is-const"; import typeChecker from "./type-checker"; +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 +15,19 @@ export default function validateAST(ast: Program) { } } -export { isConst } from "./is-const"; +export function getValidationErrors(ast: Program): Array { + const errors = []; + const moduleContext = moduleContextFromModuleAST(ast.body[0]); + + 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..573431d83 100644 --- a/packages/validation/src/is-const.js +++ b/packages/validation/src/is-const.js @@ -1,5 +1,7 @@ // @flow +import { traverse } from "@webassemblyjs/ast"; + /** * Determine if a sequence of instructions form a constant expression * @@ -8,31 +10,39 @@ * 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; - } +export default function isConst(ast: Program, moduleContext): Array { - return instrs.reduce((acc, instr) => { - // Bailout - if (acc === false) { - return acc; - } + function isConstInstruction(instr) { + if (instr.id === "const") { + return true; + } - if (instr.id === "const") { - return true; - } + if (instr.id === "get_global") { + const index = instr.args[0].value + return !moduleContext.isMutableGlobal(index); + } - if (instr.id === "get_global") { - return true; - } + // FIXME(sven): this shoudln't be needed, we need to inject our end + // instructions after the validations + if (instr.id === "end") { + return true; + } - // FIXME(sven): this shoudln't be needed, we need to inject our end - // instructions after the validations - if (instr.id === "end") { - return true; + return false; + } + + let index = 0; + const errors = [] + + traverse(ast, { + Global(path) { + const isValid = path.node.init.reduce((acc, instr) => acc && isConstInstruction(instr), true) + if(!isValid) { + errors.push('initializer expression cannot reference mutable global') + } } + }) - return false; - }, true); + return errors; } + diff --git a/packages/validation/src/type-checker.js b/packages/validation/src/type-checker.js index ba4fda666..ac648aabc 100644 --- a/packages/validation/src/type-checker.js +++ b/packages/validation/src/type-checker.js @@ -30,14 +30,11 @@ function checkTypes(a, b) { } } -export default function validate(ast) { +export default function validate(ast, moduleContext) { if (!ast.body || !ast.body[0] || !ast.body[0].fields) { return []; } - // Module context - const moduleContext = moduleContextFromModuleAST(ast.body[0]); - errors = []; // Simulate stack types throughout all function bodies diff --git a/packages/validation/test/fixtures/global-initilizer/module.wast b/packages/validation/test/fixtures/global-initializer/module.wast similarity index 55% rename from packages/validation/test/fixtures/global-initilizer/module.wast rename to packages/validation/test/fixtures/global-initializer/module.wast index c36cbbd94..7574ddbd9 100644 --- a/packages/validation/test/fixtures/global-initilizer/module.wast +++ b/packages/validation/test/fixtures/global-initializer/module.wast @@ -1,4 +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..2cd11005d --- /dev/null +++ b/packages/validation/test/fixtures/global-initializer/output.txt @@ -0,0 +1 @@ +initializer expression cannot reference mutable global diff --git a/packages/validation/test/fixtures/global-initilizer/output.txt b/packages/validation/test/fixtures/global-initilizer/output.txt deleted file mode 100644 index dfb1841f6..000000000 --- a/packages/validation/test/fixtures/global-initilizer/output.txt +++ /dev/null @@ -1 +0,0 @@ -initializer expression can only reference an imported global diff --git a/packages/validation/test/index.js b/packages/validation/test/index.js index cb0961e94..c38b4c4c2 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