spec test updates, and many validation fixes

7 files changed, 205 insertions, 100 deletions

diff --git a/src/s2wasm.h b/src/s2wasm.h
index 54d36011f..bef31486a 100644
--- a/src/s2wasm.h
+++ b/src/s2wasm.h
@@ -635,7 +635,7 @@ class S2WasmBuilder {
       auto curr = allocator->alloc<Unary>();
       curr->op = op;
       curr->value = getInput();
-      curr->type = type;
+      curr->finalize();
       setOutput(curr, assign);
     };
     auto makeHost = [&](HostOp op) {
diff --git a/src/wasm-binary.h b/src/wasm-binary.h
index 795e89d98..9674d12c1 100644
--- a/src/wasm-binary.h
+++ b/src/wasm-binary.h
@@ -1015,7 +1015,7 @@ public:
       case Ctz:              o << int8_t(curr->type == i32 ? BinaryConsts::I32Ctz        : BinaryConsts::I64Ctz); break;
       case Popcnt:           o << int8_t(curr->type == i32 ? BinaryConsts::I32Popcnt     : BinaryConsts::I64Popcnt); break;
       case EqZ:              o << int8_t(curr->type == i32 ? BinaryConsts::I32EqZ        : BinaryConsts::I64EqZ); break;
-      case Neg:              o << int8_t(curr->type == f32 ? BinaryConsts::F32Neg        : BinaryConsts::F64Neg); break;
+      case Neg:              o << int8_t(curr->type == f64 ? BinaryConsts::F64Neg        : BinaryConsts::F32Neg); break; // TODO: wasm.h needs separate opcodes for all these
       case Abs:              o << int8_t(curr->type == f32 ? BinaryConsts::F32Abs        : BinaryConsts::F64Abs); break;
       case Ceil:             o << int8_t(curr->type == f32 ? BinaryConsts::F32Ceil       : BinaryConsts::F64Ceil); break;
       case Floor:            o << int8_t(curr->type == f32 ? BinaryConsts::F32Floor      : BinaryConsts::F64Floor); break;
diff --git a/src/wasm-builder.h b/src/wasm-builder.h
index c20c25f06..4dc195d5c 100644
--- a/src/wasm-builder.h
+++ b/src/wasm-builder.h
@@ -136,44 +136,7 @@ public:
   Unary* makeUnary(UnaryOp op, Expression *value) {
     auto* ret = allocator.alloc<Unary>();
     ret->op = op; ret->value = value;
-    switch (op) {
-      case Clz:
-      case Ctz:
-      case Popcnt:
-      case Neg:
-      case Abs:
-      case Ceil:
-      case Floor:
-      case Trunc:
-      case Nearest:
-      case Sqrt: ret->type = value->type; break;
-      case EqZ: ret->type = i32; break;
-      case ExtendSInt32: case ExtendUInt32: ret->type = i64; break;
-      case WrapInt64: ret->type = i32; break;
-      case PromoteFloat32: ret->type = f64; break;
-      case DemoteFloat64: ret->type = f32; break;
-      case TruncSFloat32ToInt32:
-      case TruncUFloat32ToInt32:
-      case TruncSFloat64ToInt32:
-      case TruncUFloat64ToInt32:
-      case ReinterpretFloat32: ret->type = i32; break;
-      case TruncSFloat32ToInt64:
-      case TruncUFloat32ToInt64:
-      case TruncSFloat64ToInt64:
-      case TruncUFloat64ToInt64:
-      case ReinterpretFloat64: ret->type = i64; break;
-      case ReinterpretInt32:
-      case ConvertSInt32ToFloat32:
-      case ConvertUInt32ToFloat32:
-      case ConvertSInt64ToFloat32:
-      case ConvertUInt64ToFloat32: ret->type = f32; break;
-      case ReinterpretInt64:
-      case ConvertSInt32ToFloat64:
-      case ConvertUInt32ToFloat64:
-      case ConvertSInt64ToFloat64:
-      case ConvertUInt64ToFloat64: ret->type = f64; break;
-      default: abort();
-    }
+    ret->finalize();
     return ret;
   }
   Binary* makeBinary(BinaryOp op, Expression *left, Expression *right) {
diff --git a/src/wasm-s-parser.h b/src/wasm-s-parser.h
index 4dfba25f8..74c2200a9 100644
--- a/src/wasm-s-parser.h
+++ b/src/wasm-s-parser.h
@@ -60,8 +60,6 @@ class Element {
   IString str_;
   bool dollared_;
 
-  size_t line, col;
-
 public:
   Element(MixedArena& allocator) : isList_(true), list_(allocator), line(-1), col(-1) {}
 
@@ -69,6 +67,8 @@ public:
   bool isStr() { return !isList_; }
   bool dollared() { return dollared_; }
 
+  size_t line, col;
+
   // list methods
 
   List& list() {
@@ -300,11 +300,9 @@ private:
     if (id == TYPE) return parseType(s);
     if (id != FUNC) return;
     size_t i = 1;
-    Name name;
-    if (s[i]->isStr()) {
-      name = s[i]->str();
-      i++;
-    } else {
+    Name name, exportName;
+    i = parseFunctionNames(s, name, exportName);
+    if (!name.is()) {
       // unnamed, use an index
       name = Name::fromInt(functionCounter);
     }
@@ -371,21 +369,26 @@ private:
     wasm.addStart(getFunctionName(*s[1]));
   }
 
-  void parseFunction(Element& s) {
+  // returns the next index in s
+  size_t parseFunctionNames(Element& s, Name& name, Name& exportName) {
     size_t i = 1;
-    Name name, exportName;
-    if (s[i]->isStr()) {
+    while (i < s.size() && s[i]->isStr()) {
       if (!s[i]->dollared()) {
         // an export name
         exportName = s[i]->str();
         i++;
-      }
-      if (s[i]->isStr()) {
-        assert(s[i]->dollared());
+      } else {
         name = s[i]->str();
         i++;
       }
     }
+    return i;
+  }
+
+  void parseFunction(Element& s) {
+    size_t i = 1;
+    Name name, exportName;
+    i = parseFunctionNames(s, name, exportName);
     if (!name.is()) {
       // unnamed, use an index
       name = Name::fromInt(functionCounter);
@@ -558,7 +561,7 @@ public:
         case 'e': {
           if (op[1] == 'q') {
             if (op[2] == 0) return makeBinary(s, BinaryOp::Eq, type);
-            if (op[2] == 'z') return makeUnary(s, UnaryOp::EqZ, i32);
+            if (op[2] == 'z') return makeUnary(s, UnaryOp::EqZ, type);
           }
           if (op[1] == 'x') return makeUnary(s, op[7] == 'u' ? UnaryOp::ExtendUInt32 : UnaryOp::ExtendSInt32, type);
           abort_on(op);
@@ -736,7 +739,50 @@ private:
     auto ret = allocator.alloc<Unary>();
     ret->op = op;
     ret->value = parseExpression(s[1]);
-    ret->type = type;
+    ret->finalize();
+    // type is the reported type, e.g. i64.ctz reports i64 (but has a return type of i32, in this case)
+    // verify the reported type is correct
+    switch (op) {
+      case EqZ:
+      case Neg:
+      case Abs:
+      case Ceil:
+      case Floor:
+      case Trunc:
+      case Nearest:
+      case Sqrt:
+      case Clz:
+      case Ctz:
+      case Popcnt: {
+        if (ret->value->type != unreachable && type != ret->value->type) throw ParseException(std::string("bad type for ") + getExpressionName(ret) + ": " + printWasmType(type) + " vs value type " + printWasmType(ret->value->type), s.line, s.col);
+        break;
+      }
+      case ExtendSInt32: case ExtendUInt32:
+      case WrapInt64:
+      case PromoteFloat32:
+      case DemoteFloat64:
+      case TruncSFloat32ToInt32:
+      case TruncUFloat32ToInt32:
+      case TruncSFloat64ToInt32:
+      case TruncUFloat64ToInt32:
+      case ReinterpretFloat32:
+      case TruncSFloat32ToInt64:
+      case TruncUFloat32ToInt64:
+      case TruncSFloat64ToInt64:
+      case TruncUFloat64ToInt64:
+      case ReinterpretFloat64:
+      case ReinterpretInt32:
+      case ConvertSInt32ToFloat32:
+      case ConvertUInt32ToFloat32:
+      case ConvertSInt64ToFloat32:
+      case ConvertUInt64ToFloat32:
+      case ReinterpretInt64:
+      case ConvertSInt32ToFloat64:
+      case ConvertUInt32ToFloat64:
+      case ConvertSInt64ToFloat64:
+      case ConvertUInt64ToFloat64: break;
+      default: WASM_UNREACHABLE();
+    }
     return ret;
   }
 
@@ -1150,6 +1196,7 @@ private:
     size_t i = 2;
     if (s[i]->isStr()) {
       wasm.memory.max = atoi(s[i]->c_str());
+      if (wasm.memory.max > Memory::kMaxSize) throw ParseException("total memory must be <= 4GB");
       i++;
     }
     while (i < s.size()) {
diff --git a/src/wasm-validator.h b/src/wasm-validator.h
index f93e06524..3d8596ad2 100644
--- a/src/wasm-validator.h
+++ b/src/wasm-validator.h
@@ -30,6 +30,7 @@ struct WasmValidator : public PostWalker<WasmValidator, Visitor<WasmValidator>>
   bool valid;
 
   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) {
@@ -51,7 +52,7 @@ public:
     }
   }
   void visitIf(If *curr) {
-    shouldBeTrue(curr->condition->type == unreachable || curr->condition->type == i32, curr, "if condition must be i32");
+    shouldBeTrue(curr->condition->type == unreachable || curr->condition->type == i32 || curr->condition->type == i64, curr, "if condition must be valid");
   }
   void visitLoop(Loop *curr) {
     if (curr->in.is()) {
@@ -96,29 +97,39 @@ public:
   }
   void visitCall(Call *curr) {
     auto* target = getModule()->getFunction(curr->target);
+    shouldBeTrue(curr->operands.size() == target->params.size(), curr, "call param number must match");
     for (size_t i = 0; i < curr->operands.size(); i++) {
-      shouldBeTrue(curr->operands[i]->type == target->params[i], curr, "call param types must match");
+      shouldBeEqualOrFirstIsUnreachable(curr->operands[i]->type, target->params[i], curr, "call param types must match");
     }
   }
   void visitCallImport(CallImport *curr) {
     auto* target = getModule()->getImport(curr->target)->type;
+    shouldBeTrue(curr->operands.size() == target->params.size(), curr, "call param number must match");
     for (size_t i = 0; i < curr->operands.size(); i++) {
-      shouldBeTrue(curr->operands[i]->type == target->params[i], curr, "call param types must match");
+      shouldBeEqualOrFirstIsUnreachable(curr->operands[i]->type, target->params[i], curr, "call param types must match");
+    }
+  }
+  void visitCallIndirect(CallIndirect *curr) {
+    auto* type = curr->fullType;
+    shouldBeEqualOrFirstIsUnreachable(curr->target->type, i32, curr, "indirect call target must be an i32");
+    shouldBeTrue(curr->operands.size() == type->params.size(), curr, "call param number must match");
+    for (size_t i = 0; i < curr->operands.size(); i++) {
+      shouldBeEqualOrFirstIsUnreachable(curr->operands[i]->type, type->params[i], curr, "call param types must match");
     }
   }
   void visitSetLocal(SetLocal *curr) {
     if (curr->value->type != unreachable) {
-      shouldBeEqual(curr->type, curr->value->type, curr, "set_local type must be correct");
+      shouldBeEqualOrFirstIsUnreachable(curr->value->type, curr->type, curr, "set_local type must be correct");
     }
   }
   void visitLoad(Load *curr) {
     validateAlignment(curr->align);
-    shouldBeEqual(curr->ptr->type, i32, curr, "load pointer type must be i32");
+    shouldBeEqualOrFirstIsUnreachable(curr->ptr->type, i32, curr, "load pointer type must be i32");
   }
   void visitStore(Store *curr) {
     validateAlignment(curr->align);
-    shouldBeEqual(curr->ptr->type, i32, curr, "store pointer type must be i32");
-    shouldBeEqual(curr->value->type, curr->type, curr, "store value type must match");
+    shouldBeEqualOrFirstIsUnreachable(curr->ptr->type, i32, curr, "store pointer type must be i32");
+    shouldBeEqualOrFirstIsUnreachable(curr->value->type, curr->type, curr, "store value type must match");
   }
   void visitBinary(Binary *curr) {
     if (curr->left->type != unreachable && curr->right->type != unreachable) {
@@ -126,6 +137,15 @@ public:
     }
   }
   void visitUnary(Unary *curr) {
+    shouldBeUnequal(curr->value->type, none, curr, "unaries must not receive a none as their input");
+    switch (curr->op) {
+      case EqZ: {
+        shouldBeEqual(curr->type, i32, curr, "eqz must return i32");
+        break;
+      }
+      default: {}
+    }
+    if (curr->value->type == unreachable) return;
     switch (curr->op) {
       case Clz:
       case Ctz:
@@ -143,7 +163,7 @@ public:
         break;
       }
       case EqZ: {
-        shouldBeEqual(curr->type, i32, curr, "relational unaries must return i32");
+        shouldBeTrue(curr->value->type == i32 || curr->value->type == i64, curr, "eqz input must be i32 or i64");
         break;
       }
       case ExtendSInt32:           shouldBeEqual(curr->value->type, i32, curr, "extend type must be correct"); break;
@@ -175,15 +195,42 @@ public:
     }
   }
 
+  void visitReturn(Return* curr) {
+    if (curr->value) {
+      returnType = curr->value->type;
+    }
+  }
+
+  void visitHost(Host* curr) {
+    switch (curr->op) {
+      case GrowMemory: {
+        shouldBeEqual(curr->operands.size(), size_t(1), curr, "grow_memory must have 1 operand");
+        shouldBeEqualOrFirstIsUnreachable(curr->operands[0]->type, i32, curr, "grow_memory must have i32 operand");
+        break;
+      }
+      case PageSize:
+      case CurrentMemory:
+      case HasFeature: break;
+      default: WASM_UNREACHABLE();
+    }
+  }
+
   void visitFunction(Function *curr) {
     // if function has no result, it is ignored
     // if body is unreachable, it might be e.g. a return
-    if (curr->result != none && curr->body->type != unreachable) {
-      shouldBeEqual(curr->result, curr->body->type, curr->body, "function result must match, if function returns");
+    if (curr->result != none) {
+      if (curr->body->type != unreachable) {
+        shouldBeEqual(curr->result, curr->body->type, curr->body, "function body type must match, if function returns");
+      }
+      if (returnType != unreachable) {
+        shouldBeEqual(curr->result, returnType, curr->body, "function result must match, if function returns");
+      }
     }
+    returnType = unreachable;
   }
   void visitMemory(Memory *curr) {
     shouldBeFalse(curr->initial > curr->max, "memory", "memory max >= initial");
+    shouldBeTrue(curr->max <= Memory::kMaxSize, "memory", "total memory must be <= 4GB");
     size_t top = 0;
     for (auto& segment : curr->segments) {
       shouldBeFalse(segment.offset < top, "memory", "segment offset is small enough");
@@ -296,6 +343,16 @@ private:
   }
 
   template<typename T, typename S>
+  bool shouldBeEqualOrFirstIsUnreachable(S left, S right, T curr, const char* text) {
+    if (left != unreachable && left != right) {
+      fail() << "" << left << " != " << right << ": " << text << ", on \n" << curr << std::endl;
+      valid = false;
+      return false;
+    }
+    return true;
+  }
+
+  template<typename T, typename S>
   bool shouldBeUnequal(S left, S right, T curr, const char* text) {
     if (left == right) {
       fail() << "" << left << " == " << right << ": " << text << ", on \n" << curr << std::endl;
diff --git a/src/wasm.cpp b/src/wasm.cpp
index 98268310a..09d9c3c16 100644
--- a/src/wasm.cpp
+++ b/src/wasm.cpp
@@ -22,66 +22,65 @@ namespace wasm {
 
 struct BlockTypeSeeker : public PostWalker<BlockTypeSeeker, Visitor<BlockTypeSeeker>> {
   Block* target; // look for this one
-  WasmType type = unreachable;
+  std::vector<WasmType> types;
 
   BlockTypeSeeker(Block* target) : target(target) {}
 
-  void noteType(WasmType other) {
-    // once none, stop. it then indicates a poison value, that must not be consumed
-    // and ignore unreachable
-    if (type != none) {
-      if (other == none) {
-        type = none;
-      } else if (other != unreachable) {
-        if (type == unreachable) {
-          type = other;
-        } else if (type != other) {
-          type = none; // poison value, we saw multiple types; this should not be consumed
-        }
-      }
-    }
-  }
-
   void visitBreak(Break *curr) {
     if (curr->name == target->name) {
-      noteType(curr->value ? curr->value->type : none);
+      types.push_back(curr->value ? curr->value->type : none);
     }
   }
 
   void visitSwitch(Switch *curr) {
     for (auto name : curr->targets) {
-      if (name == target->name) noteType(curr->value ? curr->value->type : none);
+      if (name == target->name) types.push_back(curr->value ? curr->value->type : none);
     }
   }
 
   void visitBlock(Block *curr) {
     if (curr == target) {
-      if (curr->list.size() > 0) noteType(curr->list.back()->type);
-    } else {
-      type = unreachable; // ignore all breaks til now, they were captured by someone with the same name
+      if (curr->list.size() > 0) {
+        types.push_back(curr->list.back()->type);
+      } else {
+        types.push_back(none);
+      }
+    } else if (curr->name == target->name) {
+      types.clear(); // ignore all breaks til now, they were captured by someone with the same name
     }
   }
 };
 
 void Block::finalize() {
-  if (list.size() > 0) {
-    auto last = list.back()->type;
-    if (last != unreachable) {
-      // well that was easy
-      type = last;
-      return;
-    }
-  }
   if (!name.is()) {
-    // that was rather silly
-    type = unreachable;
+    // nothing branches here, so this is easy
+    if (list.size() > 0) {
+      type = list.back()->type;
+    } else {
+      type = unreachable;
+    }
     return;
   }
-  // oh no this is hard
+
   BlockTypeSeeker seeker(this);
   Expression* temp = this;
   seeker.walk(temp);
-  type = seeker.type;
+  type = unreachable;
+  for (auto other : seeker.types) {
+    // once none, stop. it then indicates a poison value, that must not be consumed
+    // and ignore unreachable
+    if (type != none) {
+      if (other == none) {
+        type = none;
+      } else if (other != unreachable) {
+        if (type == unreachable) {
+          type = other;
+        } else if (type != other) {
+          type = none; // poison value, we saw multiple types; this should not be consumed
+        }
+      }
+    }
+  }
 }
 
 } // namespace wasm
diff --git a/src/wasm.h b/src/wasm.h
index f68459401..68c388df8 100644
--- a/src/wasm.h
+++ b/src/wasm.h
@@ -1118,7 +1118,46 @@ public:
 
   bool isRelational() { return op == EqZ; }
 
-  // no finalize since some opcodes have more than one type, so user must set it anyhow
+  void finalize() {
+    switch (op) {
+      case Clz:
+      case Ctz:
+      case Popcnt:
+      case Neg:
+      case Abs:
+      case Ceil:
+      case Floor:
+      case Trunc:
+      case Nearest:
+      case Sqrt: type = value->type; break;
+      case EqZ: type = i32; break;
+      case ExtendSInt32: case ExtendUInt32: type = i64; break;
+      case WrapInt64: type = i32; break;
+      case PromoteFloat32: type = f64; break;
+      case DemoteFloat64: type = f32; break;
+      case TruncSFloat32ToInt32:
+      case TruncUFloat32ToInt32:
+      case TruncSFloat64ToInt32:
+      case TruncUFloat64ToInt32:
+      case ReinterpretFloat32: type = i32; break;
+      case TruncSFloat32ToInt64:
+      case TruncUFloat32ToInt64:
+      case TruncSFloat64ToInt64:
+      case TruncUFloat64ToInt64:
+      case ReinterpretFloat64: type = i64; break;
+      case ReinterpretInt32:
+      case ConvertSInt32ToFloat32:
+      case ConvertUInt32ToFloat32:
+      case ConvertSInt64ToFloat32:
+      case ConvertUInt64ToFloat32: type = f32; break;
+      case ReinterpretInt64:
+      case ConvertSInt32ToFloat64:
+      case ConvertUInt32ToFloat64:
+      case ConvertSInt64ToFloat64:
+      case ConvertUInt64ToFloat64: type = f64; break;
+      default: WASM_UNREACHABLE();
+    }
+  }
 };
 
 class Binary : public SpecificExpression<Expression::BinaryId> {