Unreachable typing fixes (#1004)

* fix type of drop, set_local, set_global, load, etc: when operand is unreachable, so is the node itself

* support binary tests properly in test/passes

* fix unreachable typing of blocks with no name and an unreachable child

* fix continue emitting in asm2wasm

* properly handle emitting of unreachable load

6 files changed, 95 insertions, 5 deletions

diff --git a/src/asm2wasm.h b/src/asm2wasm.h
index 00481c9bd..2fbe4d0ae 100644
--- a/src/asm2wasm.h
+++ b/src/asm2wasm.h
@@ -2296,6 +2296,7 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
       Break *continuer = allocator.alloc<Break>();
       continuer->name = in;
       continuer->condition = process(ast[1]);
+      continuer->finalize();
       Block *block = builder.blockifyWithName(loop->body, out, continuer);
       loop->body = block;
       loop->finalize();
diff --git a/src/wasm-builder.h b/src/wasm-builder.h
index 6b3f1a7d7..61db3c9e8 100644
--- a/src/wasm-builder.h
+++ b/src/wasm-builder.h
@@ -288,7 +288,7 @@ public:
     if (!block) block = makeBlock(any);
     if (append) {
       block->list.push_back(append);
-      block->finalize(); // TODO: move out of if
+      block->finalize();
     }
     return block;
   }
@@ -302,7 +302,7 @@ public:
     block->name = name;
     if (append) {
       block->list.push_back(append);
-      block->finalize(); // TODO: move out of if
+      block->finalize();
     }
     return block;
   }
@@ -325,7 +325,7 @@ public:
         block->list.push_back(item);
       }
     }
-    block->finalize(); // TODO: move out of if
+    block->finalize();
     return block;
   }
 
diff --git a/src/wasm.h b/src/wasm.h
index b23dab918..3caab9dbc 100644
--- a/src/wasm.h
+++ b/src/wasm.h
@@ -309,6 +309,8 @@ public:
 
   ExpressionList operands;
   Name target;
+
+  void finalize();
 };
 
 class CallImport : public SpecificExpression<Expression::CallImportId> {
@@ -317,6 +319,8 @@ public:
 
   ExpressionList operands;
   Name target;
+
+  void finalize();
 };
 
 class FunctionType {
@@ -340,6 +344,8 @@ public:
   ExpressionList operands;
   Name fullType;
   Expression* target;
+
+  void finalize();
 };
 
 class GetLocal : public SpecificExpression<Expression::GetLocalId> {
@@ -355,6 +361,8 @@ public:
   SetLocal() {}
   SetLocal(MixedArena& allocator) {}
 
+  void finalize();
+
   Index index;
   Expression* value;
 
@@ -377,6 +385,8 @@ public:
 
   Name name;
   Expression* value;
+
+  void finalize();
 };
 
 class Load : public SpecificExpression<Expression::LoadId> {
@@ -391,6 +401,8 @@ public:
   Expression* ptr;
 
   // type must be set during creation, cannot be inferred
+
+  void finalize();
 };
 
 class Store : public SpecificExpression<Expression::StoreId> {
@@ -466,6 +478,8 @@ public:
   Drop(MixedArena& allocator) {}
 
   Expression* value;
+
+  void finalize();
 };
 
 class Return : public SpecificExpression<Expression::ReturnId> {
diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
index 159099a16..6448615a5 100644
--- a/src/wasm/wasm-binary.cpp
+++ b/src/wasm/wasm-binary.cpp
@@ -676,7 +676,8 @@ void WasmBinaryWriter::visitLoad(Load *curr) {
     }
     case f32: o << int8_t(BinaryConsts::F32LoadMem); break;
     case f64: o << int8_t(BinaryConsts::F64LoadMem); break;
-    default: abort();
+    case unreachable: return; // the pointer is unreachable, so we are never reached; just don't emit a load
+    default: WASM_UNREACHABLE();
   }
   emitMemoryAccess(curr->align, curr->bytes, curr->offset);
 }
@@ -1732,7 +1733,6 @@ Expression* WasmBinaryBuilder::getBlock(WasmType type) {
   Name label = getNextLabel();
   breakStack.push_back({label, type != none && type != unreachable});
   auto* block = Builder(wasm).blockify(getMaybeBlock(type));
-  block->finalize();
   breakStack.pop_back();
   block->cast<Block>()->name = label;
   return block;
@@ -1814,6 +1814,7 @@ Expression* WasmBinaryBuilder::visitCall() {
     call->target = import->name;
     type = wasm.getFunctionType(import->functionType);
     fillCall(call, type);
+    call->finalize();
     ret = call;
   } else {
     // this is a call of a defined function
@@ -1825,6 +1826,7 @@ Expression* WasmBinaryBuilder::visitCall() {
     type = functionTypes[adjustedIndex];
     fillCall(call, type);
     functionCalls[adjustedIndex].push_back(call); // we don't know function names yet
+    call->finalize();
     ret = call;
   }
   return ret;
@@ -1847,6 +1849,7 @@ void WasmBinaryBuilder::visitCallIndirect(CallIndirect *curr) {
     curr->operands[num - i - 1] = popNonVoidExpression();
   }
   curr->type = fullType->result;
+  curr->finalize();
 }
 
 void WasmBinaryBuilder::visitGetLocal(GetLocal *curr) {
@@ -1873,6 +1876,7 @@ void WasmBinaryBuilder::visitSetLocal(SetLocal *curr, uint8_t code) {
   curr->value = popNonVoidExpression();
   curr->type = curr->value->type;
   curr->setTee(code == BinaryConsts::TeeLocal);
+  curr->finalize();
 }
 
 void WasmBinaryBuilder::visitGetGlobal(GetGlobal *curr) {
@@ -1897,6 +1901,7 @@ void WasmBinaryBuilder::visitSetGlobal(SetGlobal *curr) {
   auto index = getU32LEB();
   curr->name = getGlobalName(index);
   curr->value = popNonVoidExpression();
+  curr->finalize();
 }
 
 void WasmBinaryBuilder::readMemoryAccess(Address& alignment, size_t bytes, Address& offset) {
@@ -1926,6 +1931,7 @@ bool WasmBinaryBuilder::maybeVisitLoad(Expression*& out, uint8_t code) {
   if (debug) std::cerr << "zz node: Load" << std::endl;
   readMemoryAccess(curr->align, curr->bytes, curr->offset);
   curr->ptr = popNonVoidExpression();
+  curr->finalize();
   out = curr;
   return true;
 }
@@ -2149,6 +2155,7 @@ void WasmBinaryBuilder::visitUnreachable(Unreachable *curr) {
 void WasmBinaryBuilder::visitDrop(Drop *curr) {
   if (debug) std::cerr << "zz node: Drop" << std::endl;
   curr->value = popNonVoidExpression();
+  curr->finalize();
 }
 
 } // namespace wasm
diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp
index 69509afec..f9e3912f7 100644
--- a/src/wasm/wasm-s-parser.cpp
+++ b/src/wasm/wasm-s-parser.cpp
@@ -928,6 +928,7 @@ Expression* SExpressionWasmBuilder::makeTeeLocal(Element& s) {
   ret->index = getLocalIndex(*s[1]);
   ret->value = parseExpression(s[2]);
   ret->setTee(true);
+  ret->finalize();
   return ret;
 }
 
@@ -936,6 +937,7 @@ Expression* SExpressionWasmBuilder::makeSetLocal(Element& s) {
   ret->index = getLocalIndex(*s[1]);
   ret->value = parseExpression(s[2]);
   ret->setTee(false);
+  ret->finalize();
   return ret;
 }
 
@@ -960,6 +962,7 @@ Expression* SExpressionWasmBuilder::makeSetGlobal(Element& s) {
   ret->name = getGlobalName(*s[1]);
   if (wasm.getGlobalOrNull(ret->name) && !wasm.getGlobalOrNull(ret->name)->mutable_) throw ParseException("set_global of immutable", s.line, s.col);
   ret->value = parseExpression(s[2]);
+  ret->finalize();
   return ret;
 }
 
@@ -1084,6 +1087,7 @@ Expression* SExpressionWasmBuilder::makeLoad(Element& s, WasmType type) {
     i++;
   }
   ret->ptr = parseExpression(s[i]);
+  ret->finalize();
   return ret;
 }
 
@@ -1210,6 +1214,7 @@ Expression* SExpressionWasmBuilder::makeCall(Element& s) {
   ret->target = target;
   ret->type = functionTypes[ret->target];
   parseCallOperands(s, 2, s.size(), ret);
+  ret->finalize();
   return ret;
 }
 
@@ -1219,6 +1224,7 @@ Expression* SExpressionWasmBuilder::makeCallImport(Element& s) {
   Import* import = wasm.getImport(ret->target);
   ret->type = wasm.getFunctionType(import->functionType)->result;
   parseCallOperands(s, 2, s.size(), ret);
+  ret->finalize();
   return ret;
 }
 
@@ -1232,6 +1238,7 @@ Expression* SExpressionWasmBuilder::makeCallIndirect(Element& s) {
   ret->type = fullType->result;
   parseCallOperands(s, 2, s.size() - 1, ret);
   ret->target = parseExpression(s[s.size() - 1]);
+  ret->finalize();
   return ret;
 }
 
diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp
index 96ca8fbc8..834d9e28f 100644
--- a/src/wasm/wasm.cpp
+++ b/src/wasm/wasm.cpp
@@ -198,6 +198,16 @@ void Block::finalize() {
   if (!name.is()) {
     // nothing branches here, so this is easy
     if (list.size() > 0) {
+      // if we have an unreachable child, we are unreachable
+      // (we don't need to recurse into children, they can't
+      // break to us)
+      for (auto* child : list) {
+        if (child->type == unreachable) {
+          type = unreachable;
+          return;
+        }
+      }
+      // children are reachable, so last element determines type
       type = list.back()->type;
     } else {
       type = none;
@@ -264,6 +274,31 @@ void Switch::finalize() {
   type = unreachable;
 }
 
+template<typename T>
+void handleUnreachableOperands(T* curr) {
+  for (auto* child : curr->operands) {
+    if (child->type == unreachable) {
+      curr->type = unreachable;
+      break;
+    }
+  }
+}
+
+void Call::finalize() {
+  handleUnreachableOperands(this);
+}
+
+void CallImport::finalize() {
+  handleUnreachableOperands(this);
+}
+
+void CallIndirect::finalize() {
+  handleUnreachableOperands(this);
+  if (target->type == unreachable) {
+    type = unreachable;
+  }
+}
+
 bool FunctionType::structuralComparison(FunctionType& b) {
   if (result != b.result) return false;
   if (params.size() != b.params.size()) return false;
@@ -290,6 +325,24 @@ void SetLocal::setTee(bool is) {
   else type = none;
 }
 
+void SetLocal::finalize() {
+  if (value->type == unreachable) {
+    type = unreachable;
+  }
+}
+
+void SetGlobal::finalize() {
+  if (value->type == unreachable) {
+    type = unreachable;
+  }
+}
+
+void Load::finalize() {
+  if (ptr->type == unreachable) {
+    type = unreachable;
+  }
+}
+
 void Store::finalize() {
   assert(valueType != none); // must be set
   if (ptr->type == unreachable || value->type == unreachable) {
@@ -423,6 +476,14 @@ void Select::finalize() {
   }
 }
 
+void Drop::finalize() {
+  if (value->type == unreachable) {
+    type = unreachable;
+  } else {
+    type = none;
+  }
+}
+
 void Host::finalize() {
   switch (op) {
     case PageSize: case CurrentMemory: case HasFeature: {