Merge pull request #392 from xtuc/refactor-validation

Refactor validation

Author: xtuc

Makefile

@@ -1,6 +1,6 @@
 NODE_OPTS =
 
-TEST_TIMEOUT = 6000
+TEST_TIMEOUT = 10000
 
 LERNA = ./node_modules/.bin/lerna
 FLOWTYPED = ./node_modules/.bin/flow-typed

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 =>

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<string> {
+  const errors = [];
+
+  traverse(ast, {
+    ModuleImport({ node }) {
+      const { mutability } = node.descr;
+
+      if (mutability === "var") {
+        errors.push("mutable globals cannot be imported");
+      }
+    }
+  });
+
+  return errors;
+}

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<string> {
+  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;

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<Instruction>): 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<string> {
+  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<Instruction>): 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;
 }

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))
+)

packages/validation/test/fixtures/global-initializer/output.txt

@@ -0,0 +1 @@
+constant expression required: initializer expression cannot reference mutable global

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);
     };

packages/validation/test/is-const.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