s2wasm: Validate the result module (#574)

Add an s2wasm option `--no-validate` to disable validation for debugging purposes.
Also fix several validation errors by adding calls to `finalize()` after creating expressions, and ensuring that an import is created earlier in `Linker::getImportThunk`.

4 files changed, 38 insertions, 18 deletions

diff --git a/src/s2wasm.h b/src/s2wasm.h
index afebd4d9c..bf66f4407 100644
--- a/src/s2wasm.h
+++ b/src/s2wasm.h
@@ -732,6 +732,7 @@ class S2WasmBuilder {
       auto curr = allocator->alloc<Host>();
       curr->op = op;
       curr->operands.push_back(getInput());
+      curr->finalize();
       setOutput(curr, assign);
     };
     auto makeLoad = [&](WasmType type) {
@@ -1028,8 +1029,12 @@ class S2WasmBuilder {
         bstack.push_back(block);
         bstack.push_back(curr);
       } else if (match("end_loop")) {
+        auto* loop = bstack.back()->cast<Loop>();
         bstack.pop_back();
+        auto* implicitBlock = bstack.back()->cast<Block>();
         bstack.pop_back();
+        implicitBlock->finalize();
+        loop->finalize();
       } else if (match("br_table")) {
         auto curr = allocator->alloc<Switch>();
         curr->condition = getInput();
@@ -1052,6 +1057,7 @@ class S2WasmBuilder {
           skipComma();
           curr->condition = getInput();
         }
+        curr->finalize();
         addToBlock(curr);
       } else if (match("call")) {
         makeCall(none);
diff --git a/src/tools/s2wasm.cpp b/src/tools/s2wasm.cpp
index a6bbdea18..54404c478 100644
--- a/src/tools/s2wasm.cpp
+++ b/src/tools/s2wasm.cpp
@@ -24,6 +24,7 @@
 #include "s2wasm.h"
 #include "wasm-linker.h"
 #include "wasm-printing.h"
+#include "wasm-validator.h"
 
 using namespace cashew;
 using namespace wasm;
@@ -31,6 +32,7 @@ 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");
@@ -81,6 +83,11 @@ 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_positional("INFILE", Options::Arguments::One,
                       [](Options *o, const std::string &argument) {
                         o->extra["infile"] = argument;
@@ -131,10 +138,16 @@ 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.debug) std::cerr << "Printing..." << std::endl;
   Output output(options.extra["output"], Flags::Text, options.debug ? Flags::Debug : Flags::Release);
   WasmPrinter::printModule(&linker.getOutput().wasm, output.getStream());
   output << meta.str();
 
   if (options.debug) std::cerr << "Done." << std::endl;
+  return 0;
 }
diff --git a/src/wasm-builder.h b/src/wasm-builder.h
index 038fdbed5..ae8d5fa4c 100644
--- a/src/wasm-builder.h
+++ b/src/wasm-builder.h
@@ -33,10 +33,10 @@ struct NameType {
 // General AST node builder
 
 class Builder {
-  MixedArena &allocator;
+  Module& wasm;
 
 public:
-  Builder(Module& wasm) : allocator(wasm.allocator) {}
+  Builder(Module& wasm) : wasm(wasm) {}
 
   // make* functions, create nodes
 
@@ -65,10 +65,10 @@ public:
   }
 
   Nop* makeNop() {
-    return allocator.alloc<Nop>();
+    return wasm.allocator.alloc<Nop>();
   }
   Block* makeBlock(Expression* first = nullptr) {
-    auto* ret = allocator.alloc<Block>();
+    auto* ret = wasm.allocator.alloc<Block>();
     if (first) {
       ret->list.push_back(first);
       ret->finalize();
@@ -76,19 +76,19 @@ public:
     return ret;
   }
   If* makeIf(Expression* condition, Expression* ifTrue, Expression* ifFalse=nullptr) {
-    auto* ret = allocator.alloc<If>();
+    auto* ret = wasm.allocator.alloc<If>();
     ret->condition = condition; ret->ifTrue = ifTrue; ret->ifFalse = ifFalse;
     ret->finalize();
     return ret;
   }
   Loop* makeLoop(Name out, Name in, Expression* body) {
-    auto* ret = allocator.alloc<Loop>();
+    auto* ret = wasm.allocator.alloc<Loop>();
     ret->out = out; ret->in = in; ret->body = body;
     ret->finalize();
     return ret;
   }
   Break* makeBreak(Name name, Expression* value=nullptr, Expression* condition=nullptr) {
-    auto* ret = allocator.alloc<Break>();
+    auto* ret = wasm.allocator.alloc<Break>();
     ret->name = name; ret->value = value; ret->condition = condition;
     ret->finalize();
     return ret;
@@ -98,13 +98,14 @@ public:
   // Call
   // Also do a version which takes a sig?
   CallImport* makeCallImport(Name target, const std::vector<Expression*>& args) {
-    auto* call = allocator.alloc<CallImport>();
+    auto* call = wasm.allocator.alloc<CallImport>();
+    call->type = wasm.getImport(target)->type->result;
     call->target = target;
     call->operands.set(args);
     return call;
   }
   CallIndirect* makeCallIndirect(FunctionType* type, Expression* target, const std::vector<Expression*>& args) {
-    auto* call = allocator.alloc<CallIndirect>();
+    auto* call = wasm.allocator.alloc<CallIndirect>();
     call->fullType = type->name;
     call->type = type->result;
     call->target = target;
@@ -113,13 +114,13 @@ public:
   }
   // FunctionType
   GetLocal* makeGetLocal(Index index, WasmType type) {
-    auto* ret = allocator.alloc<GetLocal>();
+    auto* ret = wasm.allocator.alloc<GetLocal>();
     ret->index = index;
     ret->type = type;
     return ret;
   }
   SetLocal* makeSetLocal(Index index, Expression* value) {
-    auto* ret = allocator.alloc<SetLocal>();
+    auto* ret = wasm.allocator.alloc<SetLocal>();
     ret->index = index;
     ret->value = value;
     ret->type = value->type;
@@ -127,37 +128,37 @@ public:
   }
   // Load
   Store* makeStore(unsigned bytes, uint32_t offset, unsigned align, Expression *ptr, Expression *value) {
-    auto* ret = allocator.alloc<Store>();
+    auto* ret = wasm.allocator.alloc<Store>();
     ret->bytes = bytes; ret->offset = offset; ret->align = align; ret->ptr = ptr; ret->value = value;
     ret->type = value->type;
     return ret;
   }
   Const* makeConst(Literal value) {
-    auto* ret = allocator.alloc<Const>();
+    auto* ret = wasm.allocator.alloc<Const>();
     ret->value = value;
     ret->type = value.type;
     return ret;
   }
   Unary* makeUnary(UnaryOp op, Expression *value) {
-    auto* ret = allocator.alloc<Unary>();
+    auto* ret = wasm.allocator.alloc<Unary>();
     ret->op = op; ret->value = value;
     ret->finalize();
     return ret;
   }
   Binary* makeBinary(BinaryOp op, Expression *left, Expression *right) {
-    auto* ret = allocator.alloc<Binary>();
+    auto* ret = wasm.allocator.alloc<Binary>();
     ret->op = op; ret->left = left; ret->right = right;
     ret->finalize();
     return ret;
   }
   // Select
   Return* makeReturn(Expression *value) {
-    auto* ret = allocator.alloc<Return>();
+    auto* ret = wasm.allocator.alloc<Return>();
     ret->value = value;
     return ret;
   }
   Host* makeHost(HostOp op, Name nameOperand, std::vector<Expression*>&& operands) {
-    auto* ret = allocator.alloc<Host>();
+    auto* ret = wasm.allocator.alloc<Host>();
     ret->op = op;
     ret->nameOperand = nameOperand;
     ret->operands.set(operands);
diff --git a/src/wasm-linker.cpp b/src/wasm-linker.cpp
index d6c0edb4a..1ec2defc9 100644
--- a/src/wasm-linker.cpp
+++ b/src/wasm-linker.cpp
@@ -384,6 +384,7 @@ void Linker::makeDynCallThunks() {
 Function* Linker::getImportThunk(Name name, const FunctionType* funcType) {
   Name thunkName = std::string("__importThunk_") + name.c_str();
   if (Function* thunk = out.wasm.checkFunction(thunkName)) return thunk;
+  ensureImport(name, getSig(funcType));
   wasm::Builder wasmBuilder(out.wasm);
   std::vector<NameType> params;
   Index p = 0;
@@ -396,6 +397,5 @@ Function* Linker::getImportThunk(Name name, const FunctionType* funcType) {
   Expression* call = wasmBuilder.makeCallImport(name, args);
   f->body = call;
   out.wasm.addFunction(f);
-  ensureImport(name, getSig(funcType));
   return f;
 }