Merge pull request #1033 from WebAssembly/fuzz2

More misc fuzz fixes

9 files changed, 167 insertions, 102 deletions

diff --git a/src/parsing.h b/src/parsing.h
index be3c112f3..15f22040d 100644
--- a/src/parsing.h
+++ b/src/parsing.h
@@ -32,6 +32,50 @@
 
 namespace wasm {
 
+struct ParseException {
+  std::string text;
+  size_t line, col;
+
+  ParseException() : text("unknown parse error"), line(-1), col(-1) {}
+  ParseException(std::string text) : text(text), line(-1), col(-1) {}
+  ParseException(std::string text, size_t line, size_t col) : text(text), line(line), col(col) {}
+
+  void dump(std::ostream& o) {
+    Colors::magenta(o);
+    o << "[";
+    Colors::red(o);
+    o << "parse exception: ";
+    Colors::green(o);
+    o << text;
+    if (line != size_t(-1)) {
+      Colors::normal(o);
+      o << " (at " << line << ":" << col << ")";
+    }
+    Colors::magenta(o);
+    o << "]";
+    Colors::normal(o);
+  }
+};
+
+struct MapParseException {
+  std::string text;
+
+  MapParseException() : text("unknown parse error") {}
+  MapParseException(std::string text) : text(text) {}
+
+  void dump(std::ostream& o) {
+    Colors::magenta(o);
+    o << "[";
+    Colors::red(o);
+    o << "map parse exception: ";
+    Colors::green(o);
+    o << text;
+    Colors::magenta(o);
+    o << "]";
+    Colors::normal(o);
+  }
+};
+
 inline Expression* parseConst(cashew::IString s, WasmType type, MixedArena& allocator) {
   const char *str = s.str;
   auto ret = allocator.alloc<Const>();
@@ -73,7 +117,9 @@ inline Expression* parseConst(cashew::IString s, WasmType type, MixedArena& allo
     }
     if (positive[0] == 'n' && positive[1] == 'a' && positive[2] == 'n') {
       const char * modifier = positive[3] == ':' ? positive + 4 : nullptr;
-      assert(modifier ? positive[4] == '0' && positive[5] == 'x' : 1);
+      if (!(modifier ? positive[4] == '0' && positive[5] == 'x' : 1)) {
+        throw ParseException("bad nan input");
+      }
       switch (type) {
         case f32: {
           uint32_t pattern;
@@ -163,55 +209,13 @@ inline Expression* parseConst(cashew::IString s, WasmType type, MixedArena& allo
     }
     default: return nullptr;
   }
-  assert(ret->value.type == type);
+  if (ret->value.type != type) {
+    throw ParseException("parsed type does not match expected type");
+  }
   //std::cerr << "make constant " << str << " ==> " << ret->value << '\n';
   return ret;
 }
 
-struct ParseException {
-  std::string text;
-  size_t line, col;
-
-  ParseException() : text("unknown parse error"), line(-1), col(-1) {}
-  ParseException(std::string text) : text(text), line(-1), col(-1) {}
-  ParseException(std::string text, size_t line, size_t col) : text(text), line(line), col(col) {}
-
-  void dump(std::ostream& o) {
-    Colors::magenta(o);
-    o << "[";
-    Colors::red(o);
-    o << "parse exception: ";
-    Colors::green(o);
-    o << text;
-    if (line != size_t(-1)) {
-      Colors::normal(o);
-      o << " (at " << line << ":" << col << ")";
-    }
-    Colors::magenta(o);
-    o << "]";
-    Colors::normal(o);
-  }
-};
-
-struct MapParseException {
-  std::string text;
-
-  MapParseException() : text("unknown parse error") {}
-  MapParseException(std::string text) : text(text) {}
-
-  void dump(std::ostream& o) {
-    Colors::magenta(o);
-    o << "[";
-    Colors::red(o);
-    o << "map parse exception: ";
-    Colors::green(o);
-    o << text;
-    Colors::magenta(o);
-    o << "]";
-    Colors::normal(o);
-  }
-};
-
 // Helper for parsers that may not have unique label names. This transforms
 // the names into unique ones, as required by Binaryen IR.
 struct UniqueNameMapper {
@@ -246,11 +250,20 @@ struct UniqueNameMapper {
   }
 
   Name sourceToUnique(Name sName) {
-    return labelMappings.at(sName).back();
+    if (labelMappings.find(sName) == labelMappings.end()) {
+      throw ParseException("bad label in sourceToUnique");
+    }
+    if (labelMappings[sName].empty()) {
+      throw ParseException("use of popped label in sourceToUnique");
+    }
+    return labelMappings[sName].back();
   }
 
   Name uniqueToSource(Name name) {
-    return reverseLabelMapping.at(name);
+    if (reverseLabelMapping.find(name) == reverseLabelMapping.end()) {
+      throw ParseException("label mismatch in uniqueToSource");
+    }
+    return reverseLabelMapping[name];
   }
 
   void clear() {
diff --git a/src/shell-interface.h b/src/shell-interface.h
index 076787c9b..ef92f40cd 100644
--- a/src/shell-interface.h
+++ b/src/shell-interface.h
@@ -156,6 +156,9 @@ struct ShellExternalInterface : ModuleInstance::ExternalInterface {
         trap("callIndirect: bad argument type");
       }
     }
+    if (func->result != result) {
+      trap("callIndirect: bad result type");
+    }
     return instance.callFunctionInternal(func->name, arguments);
   }
 
diff --git a/src/tools/wasm-opt.cpp b/src/tools/wasm-opt.cpp
index 0603ae93c..184dbae1b 100644
--- a/src/tools/wasm-opt.cpp
+++ b/src/tools/wasm-opt.cpp
@@ -73,15 +73,16 @@ struct ExecutionResults {
 
   Literal run(Function* func, Module& wasm) {
     ShellExternalInterface interface;
-    ModuleInstance instance(wasm, &interface);
-    LiteralList arguments;
-    for (WasmType param : func->params) {
-      // zeros in arguments TODO: more?
-      arguments.push_back(Literal(param));
-    }
     try {
-      return instance.callFunctionInternal(func->name, arguments);
+      ModuleInstance instance(wasm, &interface);
+      LiteralList arguments;
+      for (WasmType param : func->params) {
+        // zeros in arguments TODO: more?
+        arguments.push_back(Literal(param));
+      }
+      return instance.callFunction(func->name, arguments);
     } catch (const TrapException&) {
+      // may throw in instance creation (init of offsets) or call itself
       return Literal();
     }
   }
diff --git a/src/wasm-interpreter.h b/src/wasm-interpreter.h
index b06afb1a1..0b01fb1d3 100644
--- a/src/wasm-interpreter.h
+++ b/src/wasm-interpreter.h
@@ -692,6 +692,7 @@ private:
   // stack traces.
   std::vector<Name> functionStack;
 
+public:
   // Call a function, starting an invocation.
   Literal callFunction(Name name, LiteralList& arguments) {
     // if the last call ended in a jump up the stack, it might have left stuff for us to clean up here
@@ -700,7 +701,6 @@ private:
     return callFunctionInternal(name, arguments);
   }
 
-public:
   // Internal function call. Must be public so that callTable implementations can use it (refactor?)
   Literal callFunctionInternal(Name name, LiteralList& arguments) {
 
@@ -903,7 +903,6 @@ public:
     Flow flow = RuntimeExpressionRunner(*this, scope).visit(function->body);
     assert(!flow.breaking() || flow.breakTo == RETURN_FLOW); // cannot still be breaking, it means we missed our stop
     Literal ret = flow.value;
-    if (function->result == none) ret = Literal();
     if (function->result != ret.type) {
       std::cerr << "calling " << function->name << " resulted in " << ret << " but the function type is " << function->result << '\n';
       WASM_UNREACHABLE();
diff --git a/src/wasm-s-parser.h b/src/wasm-s-parser.h
index b02b6523b..53594e7aa 100644
--- a/src/wasm-s-parser.h
+++ b/src/wasm-s-parser.h
@@ -53,10 +53,10 @@ class Element {
 public:
   Element(MixedArena& allocator) : isList_(true), list_(allocator), line(-1), col(-1), loc(nullptr) {}
 
-  bool isList() { return isList_; }
-  bool isStr() { return !isList_; }
-  bool dollared() { return isStr() && dollared_; }
-  bool quoted() { return isStr() && quoted_; }
+  bool isList() const { return isList_; }
+  bool isStr() const { return !isList_; }
+  bool dollared() const { return isStr() && dollared_; }
+  bool quoted() const { return isStr() && quoted_; }
 
   size_t line, col;
   SourceLocation* loc;
@@ -69,8 +69,8 @@ public:
   }
 
   // string methods
-  cashew::IString str();
-  const char* c_str();
+  cashew::IString str() const;
+  const char* c_str() const;
   Element* setString(cashew::IString str__, bool dollared__, bool quoted__);
   Element* setMetadata(size_t line_, size_t col_, SourceLocation* loc_);
 
diff --git a/src/wasm-validator.h b/src/wasm-validator.h
index 1b704101d..d14a56a10 100644
--- a/src/wasm-validator.h
+++ b/src/wasm-validator.h
@@ -478,6 +478,9 @@ public:
     shouldBeUnequal(curr->ifTrue->type, none, curr, "select left must be valid");
     shouldBeUnequal(curr->ifFalse->type, none, curr, "select right must be valid");
     shouldBeTrue(curr->condition->type == unreachable || curr->condition->type == i32, curr, "select condition must be valid");
+    if (curr->ifTrue->type != unreachable && curr->ifFalse->type != unreachable) {
+      shouldBeEqual(curr->ifTrue->type, curr->ifFalse->type, curr, "select sides must be equal");
+    }
   }
 
   void visitDrop(Drop* curr) {
@@ -772,7 +775,7 @@ public:
           // The block has an added type, not derived from the ast itself, so it is
           // ok for it to be either i32 or unreachable.
           if (!(isConcreteWasmType(oldType) && newType == unreachable)) {
-            parent.fail() << "stale type found in " << getFunction()->name << " on " << curr << "\n(marked as " << printWasmType(oldType) << ", should be " << printWasmType(newType) << ")\n";
+            parent.fail() << "stale type found in " << (getFunction() ? getFunction()->name : Name("(global scope)")) << " on " << curr << "\n(marked as " << printWasmType(oldType) << ", should be " << printWasmType(newType) << ")\n";
             parent.valid = false;
           }
           curr->type = oldType;
diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
index c4bc66f76..54b9a5bda 100644
--- a/src/wasm/wasm-binary.cpp
+++ b/src/wasm/wasm-binary.cpp
@@ -1684,6 +1684,14 @@ void WasmBinaryBuilder::processFunctions() {
   for (auto& func : functions) {
     wasm.addFunction(func);
   }
+
+  // we should have seen all the functions
+  // we assume this later down in fact, when we read wasm.functions[index],
+  // as index was validated vs functionTypes.size()
+  if (wasm.functions.size() != functionTypes.size()) {
+    throw ParseException("did not see the right number of functions");
+  }
+
   // now that we have names for each function, apply things
 
   if (startIndex != static_cast<Index>(-1)) {
diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp
index 1842237a5..6536d78fb 100644
--- a/src/wasm/wasm-s-parser.cpp
+++ b/src/wasm/wasm-s-parser.cpp
@@ -37,11 +37,20 @@ int unhex(char c) {
   if (c >= '0' && c <= '9') return c - '0';
   if (c >= 'a' && c <= 'f') return c - 'a' + 10;
   if (c >= 'A' && c <= 'F') return c - 'A' + 10;
-  abort();
+  throw wasm::ParseException("invalid hexadecimal");
 }
 }
 
 namespace wasm {
+
+static Address getCheckedAddress(const Element* s, const char* errorText) {
+  uint64_t num = atoll(s->c_str());
+  if (num > std::numeric_limits<Address::address_t>::max()) {
+    throw ParseException(errorText, s->line, s->col);
+  }
+  return num;
+}
+
 Element::List& Element::list() {
   if (!isList()) throw ParseException("expected list", line, col);
   return list_;
@@ -53,12 +62,12 @@ Element* Element::operator[](unsigned i) {
   return list()[i];
 }
 
-IString Element::str() {
+IString Element::str() const {
   if (!isStr()) throw ParseException("expected string", line, col);
   return str_;
 }
 
-const char* Element::c_str() {
+const char* Element::c_str() const {
   if (!isStr()) throw ParseException("expected string", line, col);
   return str_.str;
 }
@@ -409,6 +418,7 @@ void SExpressionWasmBuilder::preParseFunctionType(Element& s) {
     if (need) {
       functionType->name = Name::fromInt(wasm.functionTypes.size());
       functionTypeNames.push_back(functionType->name);
+      if (wasm.getFunctionTypeOrNull(functionType->name)) throw ParseException("duplicate function type", s.line, s.col);
       wasm.addFunctionType(functionType.release());
     }
   }
@@ -569,20 +579,21 @@ void SExpressionWasmBuilder::parseFunction(Element& s, bool preParseImport) {
   }
   if (importModule.is()) {
     // this is an import, actually
-    assert(preParseImport);
+    if (!preParseImport) throw ParseException("!preParseImport in func");
     std::unique_ptr<Import> im = make_unique<Import>();
     im->name = name;
     im->module = importModule;
     im->base = importBase;
     im->kind = ExternalKind::Function;
     im->functionType = wasm.getFunctionType(type)->name;
+    if (wasm.getImportOrNull(im->name)) throw ParseException("duplicate import", s.line, s.col);
     wasm.addImport(im.release());
-    assert(!currFunction);
+    if (currFunction) throw ParseException("import module inside function dec");
     currLocalTypes.clear();
     nameMapper.clear();
     return;
   }
-  assert(!preParseImport);
+  if (preParseImport) throw ParseException("preParseImport in func");
   if (brokeToAutoBlock) {
     ensureAutoBlock();
     autoBlock->name = FAKE_RETURN;
@@ -597,6 +608,7 @@ void SExpressionWasmBuilder::parseFunction(Element& s, bool preParseImport) {
   if (currFunction->result != result) throw ParseException("bad func declaration", s.line, s.col);
   currFunction->body = body;
   currFunction->type = type;
+  if (wasm.getFunctionOrNull(currFunction->name)) throw ParseException("duplicate function", s.line, s.col);
   wasm.addFunction(currFunction.release());
   currLocalTypes.clear();
   nameMapper.clear();
@@ -612,7 +624,7 @@ WasmType SExpressionWasmBuilder::stringToWasmType(const char* str, bool allowErr
     if (str[1] == '6' && str[2] == '4' && (prefix || str[3] == 0)) return f64;
   }
   if (allowError) return none;
-  abort();
+  throw ParseException("invalid wasm type");
 }
 
 Expression* SExpressionWasmBuilder::parseExpression(Element& s) {
@@ -852,7 +864,7 @@ Expression* SExpressionWasmBuilder::makeExpression(Element& s) {
       default: abort_on(str);
     }
   }
-  abort();
+  abort_on("unrecognized input string for parsing");
 }
 
 Expression* SExpressionWasmBuilder::makeBinary(Element& s, BinaryOp op, WasmType type) {
@@ -951,6 +963,15 @@ Expression* SExpressionWasmBuilder::makeHost(Element& s, HostOp op) {
   } else {
     parseCallOperands(s, 1, s.size(), ret);
   }
+  if (ret->op == HostOp::GrowMemory) {
+    if (ret->operands.size() != 1) {
+      throw ParseException("grow_memory needs one operand");
+    }
+  } else {
+    if (ret->operands.size() != 0) {
+      throw ParseException("host needs zero operands");
+    }
+  }
   ret->finalize();
   return ret;
 }
@@ -1112,11 +1133,11 @@ Expression* SExpressionWasmBuilder::makeLoad(Element& s, WasmType type) {
     ret->bytes = 1;
     extra++;
   } else if (extra[0] == '1') {
-    assert(extra[1] == '6');
+    if (extra[1] != '6') throw ParseException("expected load16");
     ret->bytes = 2;
     extra += 2;
   } else if (extra[0] == '3') {
-    assert(extra[1] == '2');
+    if (extra[1] != '2') throw ParseException("expected load32");
     ret->bytes = 4;
     extra += 2;
   }
@@ -1127,10 +1148,12 @@ Expression* SExpressionWasmBuilder::makeLoad(Element& s, WasmType type) {
   while (!s[i]->isList()) {
     const char *str = s[i]->c_str();
     const char *eq = strchr(str, '=');
-    assert(eq);
+    if (!eq) throw ParseException("no = in load attribute");
     eq++;
     if (str[0] == 'a') {
-      ret->align = atoi(eq);
+      uint64_t align = atoll(eq);
+      if (align > std::numeric_limits<uint32_t>::max()) throw ParseException("bad align");
+      ret->align = align;
     } else if (str[0] == 'o') {
       uint64_t offset = atoll(eq);
       if (offset > std::numeric_limits<uint32_t>::max()) throw ParseException("bad offset");
@@ -1152,11 +1175,11 @@ Expression* SExpressionWasmBuilder::makeStore(Element& s, WasmType type) {
     ret->bytes = 1;
     extra++;
   } else if (extra[0] == '1') {
-    assert(extra[1] == '6');
+    if (extra[1] != '6') throw ParseException("expected store16");
     ret->bytes = 2;
     extra += 2;
   } else if (extra[0] == '3') {
-    assert(extra[1] == '2');
+    if (extra[1] != '2') throw ParseException("expected store32");;
     ret->bytes = 4;
     extra += 2;
   }
@@ -1166,10 +1189,12 @@ Expression* SExpressionWasmBuilder::makeStore(Element& s, WasmType type) {
   while (!s[i]->isList()) {
     const char *str = s[i]->c_str();
     const char *eq = strchr(str, '=');
-    assert(eq);
+    if (!eq) throw ParseException("missing = in store attribute");;
     eq++;
     if (str[0] == 'a') {
-      ret->align = atoi(eq);
+      uint64_t align = atoll(eq);
+      if (align > std::numeric_limits<uint32_t>::max()) throw ParseException("bad align");
+      ret->align = align;
     } else if (str[0] == 'o') {
       ret->offset = atoi(eq);
     } else throw ParseException("bad store attribute");
@@ -1299,7 +1324,14 @@ Name SExpressionWasmBuilder::getLabel(Element& s) {
     return nameMapper.sourceToUnique(s.str());
   } else {
     // offset, break to nth outside label
-    uint64_t offset = std::stoll(s.c_str(), nullptr, 0);
+    uint64_t offset;
+    try {
+      offset = std::stoll(s.c_str(), nullptr, 0);
+    } catch (std::invalid_argument) {
+      throw ParseException("invalid break offset");
+    } catch (std::out_of_range) {
+      throw ParseException("out of range break offset");
+    }
     if (offset > nameMapper.labelStack.size()) throw ParseException("invalid label", s.line, s.col);
     if (offset == nameMapper.labelStack.size()) {
       // a break to the function's scope. this means we need an automatic block, with a name
@@ -1335,6 +1367,7 @@ Expression* SExpressionWasmBuilder::makeBreakTable(Element& s) {
   while (!s[i]->isList()) {
     ret->targets.push_back(getLabel(*s[i++]));
   }
+  if (ret->targets.size() == 0) throw ParseException("switch with no targets");
   ret->default_ = ret->targets.back();
   ret->targets.pop_back();
   ret->condition = parseExpression(s[i++]);
@@ -1424,17 +1457,18 @@ void SExpressionWasmBuilder::parseMemory(Element& s, bool preParseImport) {
       im->module = importModule;
       im->base = importBase;
       im->name = importModule;
+      if (wasm.getImportOrNull(im->name)) throw ParseException("duplicate import", s.line, s.col);
       wasm.addImport(im.release());
       i++;
     } else {
-      assert(inner.size() > 0 ? inner[0]->str() != IMPORT : true);
+      if (!(inner.size() > 0 ? inner[0]->str() != IMPORT : true)) throw ParseException("bad import ending");
       // (memory (data ..)) format
       parseInnerData(*s[i]);
       wasm.memory.initial = wasm.memory.segments[0].data.size();
       return;
     }
   }
-  wasm.memory.initial = atoi(s[i++]->c_str());
+  wasm.memory.initial = getCheckedAddress(s[i++], "excessive memory init");
   if (i == s.size()) return;
   if (s[i]->isStr()) {
     uint64_t max = atoll(s[i]->c_str());
@@ -1449,7 +1483,7 @@ void SExpressionWasmBuilder::parseMemory(Element& s, bool preParseImport) {
     if (curr[0]->str() == DATA) {
       offsetValue = 0;
     } else {
-      offsetValue = atoi(curr[j++]->c_str());
+      offsetValue = getCheckedAddress(curr[j++], "excessive memory offset");
     }
     const char *input = curr[j]->c_str();
     auto* offset = allocator.alloc<Const>();
@@ -1507,7 +1541,7 @@ void SExpressionWasmBuilder::parseExport(Element& s) {
       ex->kind = ExternalKind::Global;
       if (wasm.getGlobalOrNull(ex->value) && wasm.getGlobal(ex->value)->mutable_) throw ParseException("cannot export a mutable global", s.line, s.col);
     } else {
-      WASM_UNREACHABLE();
+      throw ParseException("invalid export");
     }
   } else if (!s[2]->dollared() && !std::isdigit(s[2]->str()[0])) {
     ex->value = s[3]->str();
@@ -1519,7 +1553,7 @@ void SExpressionWasmBuilder::parseExport(Element& s) {
     } else if (s[2]->str() == GLOBAL) {
       ex->kind = ExternalKind::Global;
     } else {
-      WASM_UNREACHABLE();
+      throw ParseException("invalid ext export");
     }
   } else {
     // function
@@ -1571,7 +1605,7 @@ void SExpressionWasmBuilder::parseImport(Element& s) {
     } else if (im->kind == ExternalKind::Table) {
       im->name = Name("import$table$" + std::to_string(0));
     } else {
-      WASM_UNREACHABLE();
+      throw ParseException("invalid import");
     }
   }
   if (!s[i]->quoted()) {
@@ -1582,7 +1616,7 @@ void SExpressionWasmBuilder::parseImport(Element& s) {
     } else if (s[i]->str() == GLOBAL) {
       im->kind = ExternalKind::Global;
     } else {
-      WASM_UNREACHABLE();
+      throw ParseException("invalid ext import");
     }
     i++;
   } else if (!newStyle) {
@@ -1614,7 +1648,7 @@ void SExpressionWasmBuilder::parseImport(Element& s) {
       }
       if (inner.size() > j+1) {
         Element& result = *inner[j+1];
-        assert(result[0]->str() == RESULT);
+        if (result[0]->str() != RESULT) throw ParseException("expected result");
         type->result = stringToWasmType(result[1]->str());
       }
     }
@@ -1624,28 +1658,29 @@ void SExpressionWasmBuilder::parseImport(Element& s) {
       im->globalType = stringToWasmType(inner[j]->str());
     } else {
       auto& inner2 = *inner[j];
-      assert(inner2[0]->str() == MUT);
+      if (inner2[0]->str() != MUT) throw ParseException("expected mut");
       im->globalType = stringToWasmType(inner2[1]->str());
       throw ParseException("cannot import a mutable global", s.line, s.col);
     }
   } else if (im->kind == ExternalKind::Table) {
     if (j < inner.size() - 1) {
-      wasm.table.initial = atoi(inner[j++]->c_str());
+      wasm.table.initial = getCheckedAddress(inner[j++], "excessive table init size");
     }
     if (j < inner.size() - 1) {
-      wasm.table.max = atoi(inner[j++]->c_str());
+      wasm.table.max = getCheckedAddress(inner[j++], "excessive table max size");
     } else {
       wasm.table.max = Table::kMaxSize;
     }
     // ends with the table element type
   } else if (im->kind == ExternalKind::Memory) {
     if (j < inner.size()) {
-      wasm.memory.initial = atoi(inner[j++]->c_str());
+      wasm.memory.initial = getCheckedAddress(inner[j++], "excessive memory init size");
     }
     if (j < inner.size()) {
-      wasm.memory.max = atoi(inner[j++]->c_str());
+      wasm.memory.max = getCheckedAddress(inner[j++], "excessive memory max size");
     }
   }
+  if (wasm.getImportOrNull(im->name)) throw ParseException("duplicate import", s.line, s.col);
   wasm.addImport(im.release());
 }
 
@@ -1692,7 +1727,7 @@ void SExpressionWasmBuilder::parseGlobal(Element& s, bool preParseImport) {
   }
   if (importModule.is()) {
     // this is an import, actually
-    assert(preParseImport);
+    if (!preParseImport) throw ParseException("!preParseImport in global");
     if (mutable_) throw ParseException("cannot import a mutable global", s.line, s.col);
     std::unique_ptr<Import> im = make_unique<Import>();
     im->name = global->name;
@@ -1700,10 +1735,11 @@ void SExpressionWasmBuilder::parseGlobal(Element& s, bool preParseImport) {
     im->base = importBase;
     im->kind = ExternalKind::Global;
     im->globalType = type;
+    if (wasm.getImportOrNull(im->name)) throw ParseException("duplicate import", s.line, s.col);
     wasm.addImport(im.release());
     return;
   }
-  assert(!preParseImport);
+  if (preParseImport) throw ParseException("preParseImport in global");
   global->type = type;
   if (i < s.size()) {
     global->init = parseExpression(s[i++]);
@@ -1711,7 +1747,8 @@ void SExpressionWasmBuilder::parseGlobal(Element& s, bool preParseImport) {
     throw ParseException("global without init", s.line, s.col);
   }
   global->mutable_ = mutable_;
-  assert(i == s.size());
+  if (i != s.size()) throw ParseException("extra import elements");
+  if (wasm.getGlobalOrNull(global->name)) throw ParseException("duplicate import", s.line, s.col);
   wasm.addGlobal(global.release());
 }
 
@@ -1740,16 +1777,17 @@ void SExpressionWasmBuilder::parseTable(Element& s, bool preParseImport) {
     } else if (inner[0]->str() == IMPORT) {
       importModule = inner[1]->str();
       importBase = inner[2]->str();
-      assert(preParseImport);
+      if (!preParseImport) throw ParseException("!preParseImport in table");
       auto im = make_unique<Import>();
       im->kind = ExternalKind::Table;
       im->module = importModule;
       im->base = importBase;
       im->name = importModule;
+      if (wasm.getImportOrNull(im->name)) throw ParseException("duplicate import", s.line, s.col);
       wasm.addImport(im.release());
       i++;
     } else {
-      WASM_UNREACHABLE();
+      throw ParseException("invalid table");
     }
   }
   if (i == s.size()) return;
@@ -1815,7 +1853,6 @@ void SExpressionWasmBuilder::parseType(Element& s) {
     i++;
   }
   Element& func = *s[i];
-  assert(func.isList());
   for (size_t k = 1; k < func.size(); k++) {
     Element& curr = *func[k];
     if (curr[0]->str() == PARAM) {
@@ -1831,6 +1868,7 @@ void SExpressionWasmBuilder::parseType(Element& s) {
     type->name = Name::fromInt(wasm.functionTypes.size());
   }
   functionTypeNames.push_back(type->name);
+  if (wasm.getFunctionTypeOrNull(type->name)) throw ParseException("duplicate function type", s.line, s.col);
   wasm.addFunctionType(type.release());
 }
 
diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp
index d2bf4e75c..a879ebd05 100644
--- a/src/wasm/wasm.cpp
+++ b/src/wasm/wasm.cpp
@@ -74,7 +74,7 @@ Name GROW_WASM_MEMORY("__growWasmMemory"),
 
 const char* getExpressionName(Expression* curr) {
   switch (curr->_id) {
-    case Expression::Id::InvalidId: abort();
+    case Expression::Id::InvalidId: WASM_UNREACHABLE();
     case Expression::Id::BlockId: return "block";
     case Expression::Id::IfId: return "if";
     case Expression::Id::LoopId: return "loop";
@@ -500,7 +500,7 @@ void Host::finalize() {
       }
       break;
     }
-    default: abort();
+    default: WASM_UNREACHABLE();
   }
 }