Add mode to wasm validator to check for web-environment constraints (#584)

In the web embedding, modules are not allowed to import or export
functions which have i64 params or return values. Add a mode to the
validator to check for this, and add flags to s2wasm and wasm-as to
enable or disable this check. Also add tests.

8 files changed, 80 insertions, 23 deletions

diff --git a/check.py b/check.py
index 7e8586150..2da0d30e6 100755
--- a/check.py
+++ b/check.py
@@ -132,11 +132,11 @@ except:
 
 # utilities
 
-def run_command(cmd, stderr=None):
+def run_command(cmd, expected_status=0, stderr=None):
   print 'executing: ', ' '.join(cmd)
   proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=stderr)
   out, err = proc.communicate()
-  if proc.returncode != 0: raise Exception(('run_command failed', err))
+  if proc.returncode != expected_status: raise Exception(('run_command failed', err))
   return out
 
 def fail(actual, expected):
@@ -544,6 +544,19 @@ output = run_command(cmd)
 # bar should be linked from the archive
 fail_if_not_contained(output, '(func $bar')
 
+print '\n[ running validation tests... ]\n'
+wasm_as = os.path.join('bin', 'wasm-as')
+# Ensure the tests validate by default
+cmd = [wasm_as, os.path.join('test', 'validator', 'invalid_export.wast')]
+run_command(cmd)
+cmd = [wasm_as, os.path.join('test', 'validator', 'invalid_import.wast')]
+run_command(cmd)
+cmd = [wasm_as, '--validate=web', os.path.join('test', 'validator', 'invalid_export.wast')]
+run_command(cmd, expected_status=1)
+cmd = [wasm_as, '--validate=web', os.path.join('test', 'validator', 'invalid_import.wast')]
+run_command(cmd, expected_status=1)
+cmd = [wasm_as, '--validate=none', os.path.join('test', 'validator', 'invalid_return.wast')]
+run_command(cmd)
 
 if torture:
 
diff --git a/src/tools/s2wasm.cpp b/src/tools/s2wasm.cpp
index 54404c478..2ee094af4 100644
--- a/src/tools/s2wasm.cpp
+++ b/src/tools/s2wasm.cpp
@@ -32,10 +32,10 @@ using namespace wasm;
 int main(int argc, const char *argv[]) {
   bool ignoreUnknownSymbols = false;
   bool generateEmscriptenGlue = false;
-  bool validateOutput = true;
   std::string startFunction;
   std::vector<std::string> archiveLibraries;
   Options options("s2wasm", "Link .s file into .wast");
+  options.extra["validate"] = "wasm";
   options
       .add("--output", "-o", "Output file (stdout if not specified)",
            Options::Arguments::One,
@@ -83,10 +83,14 @@ int main(int argc, const char *argv[]) {
            [&archiveLibraries](Options *o, const std::string &argument) {
              archiveLibraries.push_back(argument);
            })
-      .add("--no-validate", "", "Disable validation of the output module",
-           Options::Arguments::Zero,
-           [&validateOutput](Options *, const std::string &) {
-            validateOutput = false;
+      .add("--validate", "-v", "Control validation of the output module",
+           Options::Arguments::One,
+           [](Options *o, const std::string &argument) {
+             if (argument != "web" && argument != "none" && argument != "wasm") {
+               std::cerr << "Valid arguments for --validate flag are 'wasm', 'web' and 'none'.\n";
+               exit(1);
+             }
+             o->extra["validate"] = argument;
            })
       .add_positional("INFILE", Options::Arguments::One,
                       [](Options *o, const std::string &argument) {
@@ -138,9 +142,12 @@ int main(int argc, const char *argv[]) {
     linker.emscriptenGlue(meta);
   }
 
-  if (options.debug) std::cerr << "Validating..." << std::endl;
-  if (validateOutput && !wasm::WasmValidator().validate(linker.getOutput().wasm)) {
-    Fatal() << "Error: linked module is not valid.\n";
+  if (options.extra["validate"] != "none") {
+    if (options.debug) std::cerr << "Validating..." << std::endl;
+    if (!wasm::WasmValidator().validate(linker.getOutput().wasm,
+        options.extra["validate"] == "web")) {
+      Fatal() << "Error: linked module is not valid.\n";
+    }
   }
 
   if (options.debug) std::cerr << "Printing..." << std::endl;
diff --git a/src/tools/wasm-as.cpp b/src/tools/wasm-as.cpp
index e5f5af085..e2425b24a 100644
--- a/src/tools/wasm-as.cpp
+++ b/src/tools/wasm-as.cpp
@@ -29,12 +29,23 @@ using namespace wasm;
 
 int main(int argc, const char *argv[]) {
   Options options("wasm-as", "Assemble a .wast (WebAssembly text format) into a .wasm (WebAssembly binary format)");
-  options.add("--output", "-o", "Output file (stdout if not specified)",
-              Options::Arguments::One,
-              [](Options *o, const std::string &argument) {
-                o->extra["output"] = argument;
-                Colors::disable();
-              })
+  options.extra["validate"] = "wasm";
+  options
+      .add("--output", "-o", "Output file (stdout if not specified)",
+           Options::Arguments::One,
+           [](Options *o, const std::string &argument) {
+             o->extra["output"] = argument;
+             Colors::disable();
+           })
+      .add("--validate", "-v", "Control validation of the output module",
+           Options::Arguments::One,
+           [](Options *o, const std::string &argument) {
+             if (argument != "web" && argument != "none" && argument != "wasm") {
+               std::cerr << "Valid arguments for --validate flag are 'wasm', 'web', and 'none'.\n";
+               exit(1);
+             }
+             o->extra["validate"] = argument;
+           })
       .add_positional("INFILE", Options::Arguments::One,
                       [](Options *o, const std::string &argument) {
                         o->extra["infile"] = argument;
@@ -56,6 +67,14 @@ int main(int argc, const char *argv[]) {
     Fatal() << "error in parsing input";
   }
 
+  if (options.extra["validate"] != "none") {
+    if (options.debug) std::cerr << "Validating..." << std::endl;
+    if (!wasm::WasmValidator().validate(wasm,
+        options.extra["validate"] == "web")) {
+      Fatal() << "Error: input module is not valid.\n";
+    }
+  }
+
   if (options.debug) std::cerr << "binarification..." << std::endl;
   BufferWithRandomAccess buffer(options.debug);
   WasmBinaryWriter writer(&wasm, buffer, options.debug);
diff --git a/src/wasm-binary.h b/src/wasm-binary.h
index aa3295636..5cb56f770 100644
--- a/src/wasm-binary.h
+++ b/src/wasm-binary.h
@@ -1243,10 +1243,6 @@ public:
     }
 
     processFunctions();
-
-    if (!WasmValidator().validate(wasm)) {
-      abort();
-    }
   }
 
   bool more() {
diff --git a/src/wasm-validator.h b/src/wasm-validator.h
index 1504b3a71..76e142725 100644
--- a/src/wasm-validator.h
+++ b/src/wasm-validator.h
@@ -27,14 +27,15 @@
 namespace wasm {
 
 struct WasmValidator : public PostWalker<WasmValidator, Visitor<WasmValidator>> {
-  bool valid;
+  bool valid = true;
+  bool validateWebConstraints = false;
 
   std::map<Name, WasmType> breakTypes; // breaks to a label must all have the same type, and the right type
   WasmType returnType = unreachable; // type used in returns
 
 public:
-  bool validate(Module& module) {
-    valid = true;
+  bool validate(Module& module, bool validateWeb=false) {
+    validateWebConstraints = validateWeb;
     walkModule(&module);
     return valid;
   }
@@ -234,6 +235,23 @@ public:
     }
   }
 
+  void visitImport(Import* curr) {
+    if (!validateWebConstraints) return;
+    shouldBeUnequal(curr->type->result, i64, curr->name, "Imported function must not have i64 return type");
+    for (WasmType param : curr->type->params) {
+      shouldBeUnequal(param, i64, curr->name, "Imported function must not have i64 parameters");
+    }
+  }
+
+  void visitExport(Export* curr) {
+    if (!validateWebConstraints) return;
+    Function* f = getModule()->getFunction(curr->value);
+    shouldBeUnequal(f->result, i64, f->name, "Exported function must not have i64 return type");
+    for (auto param : f->params) {
+      shouldBeUnequal(param, i64, f->name, "Exported function must not have i64 parameters");
+    }
+  }
+
   void visitFunction(Function *curr) {
     // if function has no result, it is ignored
     // if body is unreachable, it might be e.g. a return
diff --git a/test/validator/invalid_export.wast b/test/validator/invalid_export.wast
new file mode 100644
index 000000000..e10e22a39
--- /dev/null
+++ b/test/validator/invalid_export.wast
@@ -0,0 +1 @@
+(module (func $export64 (result i64) (i64.const 1)) (export "a" $export64))
\ No newline at end of file
diff --git a/test/validator/invalid_import.wast b/test/validator/invalid_import.wast
new file mode 100644
index 000000000..e1b05b617
--- /dev/null
+++ b/test/validator/invalid_import.wast
@@ -0,0 +1 @@
+(module (import $bad "test" "bad" (param i64)))
\ No newline at end of file
diff --git a/test/validator/invalid_return.wast b/test/validator/invalid_return.wast
new file mode 100644
index 000000000..02ce049d6
--- /dev/null
+++ b/test/validator/invalid_return.wast
@@ -0,0 +1,2 @@
+(module
+ (func $foo (result i32) (i64.const 1)))
\ No newline at end of file