More binary updates for 0xc (#733)

Refine tables to explicitly exist or not. Previously they were printed
or encoded if it had any segments, or an initial or max size. However
tables can be defined but empty, so we had a special hack that defined
an empty segment when we really just wanted an empty table. Now, just
make the existence explicit.

Update Function table encoding for 0xc (Table and Element sections)

Add end opcodes after function bodies (these are consumed by
getMaybeBlock with the same behavior that it had before when it reached
the function end, so no explicit decode)

Update call_indirect encoding for 0xc (no arity, call target is last)

8 files changed, 70 insertions, 40 deletions

diff --git a/src/asm2wasm.h b/src/asm2wasm.h
index afd85feb6..f00004b1f 100644
--- a/src/asm2wasm.h
+++ b/src/asm2wasm.h
@@ -777,6 +777,7 @@ void Asm2WasmBuilder::processAsm(Ref ast) {
           // TODO: when not using aliasing function pointers, we could merge them by noticing that
           //       index 0 in each table is the null func, and each other index should only have one
           //       non-null func. However, that breaks down when function pointer casts are emulated.
+          wasm.table.exists = true;
           if (wasm.table.segments.size() == 0) {
             wasm.table.segments.emplace_back(wasm.allocator.alloc<Const>()->set(Literal(uint32_t(0))));
           }
diff --git a/src/binaryen-c.cpp b/src/binaryen-c.cpp
index 310757cad..b8933147f 100644
--- a/src/binaryen-c.cpp
+++ b/src/binaryen-c.cpp
@@ -756,6 +756,7 @@ void BinaryenSetFunctionTable(BinaryenModuleRef module, BinaryenFunctionRef* fun
   }
 
   auto* wasm = (Module*)module;
+  wasm->table.exists = true;
   Table::Segment segment(wasm->allocator.alloc<Const>()->set(Literal(int32_t(0))));
   for (BinaryenIndex i = 0; i < numFuncs; i++) {
     segment.data.push_back(((Function*)funcs[i])->name);
diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp
index 4f2c17d40..e5487d58e 100644
--- a/src/passes/Print.cpp
+++ b/src/passes/Print.cpp
@@ -726,7 +726,7 @@ struct PrintSExpression : public Visitor<PrintSExpression> {
       visitGlobal(child.get());
       o << maybeNewLine;
     }
-    if (curr->table.segments.size() > 0 || curr->table.initial > 0 || curr->table.max != Table::kMaxSize) {
+    if (curr->table.exists) {
       visitTable(&curr->table);
       o << maybeNewLine;
     }
diff --git a/src/wasm-binary.h b/src/wasm-binary.h
index 485ad98c7..be09f640a 100644
--- a/src/wasm-binary.h
+++ b/src/wasm-binary.h
@@ -532,10 +532,11 @@ public:
     writeTypes();
     writeImports();
     writeFunctionSignatures();
-    writeFunctionTable();
+    writeFunctionTableDeclaration();
     writeMemory();
     writeGlobals();
     writeExports();
+    writeTableElements();
     writeStart();
     writeFunctions();
     writeDataSegments();
@@ -572,7 +573,7 @@ public:
   }
 
   void finishSection(int32_t start) {
-    int32_t size = o.size() - start - 6; // section size does not include the 6 bytes of the code and size field
+    int32_t size = o.size() - start - 5; // section size does not include the 5 bytes of the size field itself
     o.writeAt(start, U32LEB(size));
   }
 
@@ -740,6 +741,7 @@ public:
       if (numLocalsByType[f64]) o << U32LEB(numLocalsByType[f64]) << binaryWasmType(f64);
 
       writeExpression(function->body);
+      o << int8_t(BinaryConsts::End);
       size_t size = o.size() - start;
       assert(size <= std::numeric_limits<uint32_t>::max());
       if (debug) std::cerr << "body size: " << size << ", writing at " << sizePos << ", next starts at " << o.size() << std::endl;
@@ -841,14 +843,25 @@ public:
     return mappedGlobals[name];
   }
 
-  void writeFunctionTable() {
-    if (wasm->table.segments.size() == 0) return;
-    if (debug) std::cerr << "== writeFunctionTable" << std::endl;
+  void writeFunctionTableDeclaration() {
+    if (!wasm->table.exists) return; // or is imported!!
+    if (debug) std::cerr << "== writeFunctionTableDeclaration" << std::endl;
     auto start = startSection(BinaryConsts::Section::Table);
-    o << U32LEB(wasm->table.initial);
-    o << U32LEB(wasm->table.max);
+    o << U32LEB(1); // Declare 1 table.
+    o << U32LEB(BinaryConsts::ElementType::AnyFunc);
+    Address max = wasm->table.max == Table::kMaxSize ? Address(0) : wasm->table.max;
+    writeResizableLimits(wasm->table.initial, max);
+    finishSection(start);
+  }
+
+  void writeTableElements() {
+    if (!wasm->table.exists) return;
+    if (debug) std::cerr << "== writeTableElements" << std::endl;
+    auto start = startSection(BinaryConsts::Section::Element);
+
     o << U32LEB(wasm->table.segments.size());
     for (auto& segment : wasm->table.segments) {
+      o << U32LEB(0); // Table index; 0 in the MVP (and binaryen IR only has 1 table)
       writeExpression(segment.offset);
       o << int8_t(BinaryConsts::End);
       o << U32LEB(segment.data.size());
@@ -1032,11 +1045,12 @@ public:
   }
   void visitCallIndirect(CallIndirect *curr) {
     if (debug) std::cerr << "zz node: CallIndirect" << std::endl;
-    recurse(curr->target);
+
     for (auto* operand : curr->operands) {
       recurse(operand);
     }
-    o << int8_t(BinaryConsts::CallIndirect) << U32LEB(curr->operands.size()) << U32LEB(getFunctionTypeIndex(curr->fullType));
+    recurse(curr->target);
+    o << int8_t(BinaryConsts::CallIndirect) << U32LEB(getFunctionTypeIndex(curr->fullType));
   }
   void visitGetLocal(GetLocal *curr) {
     if (debug) std::cerr << "zz node: GetLocal " << (o.size() + 1) << std::endl;
@@ -1360,6 +1374,7 @@ public:
         case BinaryConsts::Section::Function: readFunctionSignatures(); break;
         case BinaryConsts::Section::Code: readFunctions(); break;
         case BinaryConsts::Section::Export: readExports(); break;
+        case BinaryConsts::Section::Element: readTableElements(); break;
         case BinaryConsts::Section::Global: {
           readGlobals();
           // imports can read global imports, so we run getGlobalName and create the mapping
@@ -1369,7 +1384,7 @@ public:
           break;
         }
         case BinaryConsts::Section::Data: readDataSegments(); break;
-        case BinaryConsts::Section::Table: readFunctionTable(); break;
+        case BinaryConsts::Section::Table: readFunctionTableDeclaration(); break;
 
         default:
           if (!readUserSection()) abort();
@@ -1621,7 +1636,8 @@ public:
         case ExternalKind::Table: {
           auto elementType = getU32LEB();
           WASM_UNUSED(elementType);
-          assert(elementType == BinaryConsts::ElementType::AnyFunc);
+          if (elementType != BinaryConsts::ElementType::AnyFunc) throw ParseException("Imported table type is not AnyFunc");
+          wasm.table.exists = true;
           getResizableLimits(wasm.table.initial, &wasm.table.max);
           break;
         }
@@ -1675,7 +1691,7 @@ public:
       assert(size > 0);
       endOfFunction = pos + size;
       auto type = functionTypes[i];
-      if (debug) std::cerr << "reading" << i << std::endl;
+      if (debug) std::cerr << "reading " << i << std::endl;
       size_t nextVar = 0;
       auto addVar = [&]() {
         Name name = cashew::IString(("var$" + std::to_string(nextVar++)).c_str(), false);
@@ -1719,6 +1735,7 @@ public:
       currFunction = nullptr;
       functions.push_back(func);
     }
+    if (debug) std::cerr << " end function bodies" << std::endl;
   }
 
   std::map<Export*, Index> exportIndexes;
@@ -1868,17 +1885,29 @@ public:
 
   std::map<Index, std::vector<Index>> functionTable;
 
-  void readFunctionTable() {
-    if (debug) std::cerr << "== readFunctionTable" << std::endl;
-    wasm.table.initial = getU32LEB();
-    wasm.table.max = getU32LEB();
-    auto num = getU32LEB();
-    for (size_t i = 0; i < num; i++) {
+  void readFunctionTableDeclaration() {
+    if (debug) std::cerr << "== readFunctionTableDeclaration" << std::endl;
+    auto numTables = getU32LEB();
+    if (numTables != 1) throw ParseException("Only 1 table definition allowed in MVP");
+    wasm.table.exists = true;
+    auto elemType = getU32LEB();
+    if (elemType != BinaryConsts::ElementType::AnyFunc) throw ParseException("ElementType must be AnyFunc in MVP");
+    getResizableLimits(wasm.table.initial, &wasm.table.max);
+  }
+
+  void readTableElements() {
+    if (debug) std::cerr << "== readTableElements" << std::endl;
+    auto numSegments = getU32LEB();
+    if (numSegments >= Table::kMaxSize) throw ParseException("Too many segments");
+    for (size_t i = 0; i < numSegments; i++) {
+      auto tableIndex = getU32LEB();
+      if (tableIndex != 0) throw ParseException("Table elements must refer to table 0 in MVP");
       wasm.table.segments.emplace_back(readExpression());
-      auto& temporary = functionTable[i];
+
+      auto& indexSegment = functionTable[i];
       auto size = getU32LEB();
       for (Index j = 0; j < size; j++) {
-        temporary.push_back(getU32LEB());
+        indexSegment.push_back(getU32LEB());
       }
     }
   }
@@ -1901,8 +1930,7 @@ public:
 
   BinaryConsts::ASTNodes readExpression(Expression*& curr) {
     if (pos == endOfFunction) {
-      curr = nullptr;
-      return BinaryConsts::End;
+      throw ParseException("Reached function end without seeing End opcode");
     }
     if (debug) std::cerr << "zz recurse into " << ++depth << " at " << pos << std::endl;
     uint8_t code = getInt8();
@@ -2102,17 +2130,14 @@ public:
   }
   void visitCallIndirect(CallIndirect *curr) {
     if (debug) std::cerr << "zz node: CallIndirect" << std::endl;
-    auto arity = getU32LEB();
-    WASM_UNUSED(arity);
     auto* fullType = wasm.functionTypes.at(getU32LEB()).get();
     curr->fullType = fullType->name;
     auto num = fullType->params.size();
-    assert(num == arity);
     curr->operands.resize(num);
+    curr->target = popExpression();
     for (size_t i = 0; i < num; i++) {
       curr->operands[num - i - 1] = popExpression();
     }
-    curr->target = popExpression();
     curr->type = fullType->result;
   }
   void visitGetLocal(GetLocal *curr) {
diff --git a/src/wasm-linker.cpp b/src/wasm-linker.cpp
index 587d3c9d8..014dfe6a9 100644
--- a/src/wasm-linker.cpp
+++ b/src/wasm-linker.cpp
@@ -122,7 +122,8 @@ void Linker::layout() {
 
   // Pad the indirect function table with a dummy function
   makeDummyFunction();
-  ensureTableIsPopulated();
+  // Always create a table, even if it's empty.
+  out.wasm.table.exists = true;
 
   // Pre-assign the function indexes
   for (auto& pair : out.indirectIndexes) {
@@ -141,7 +142,7 @@ void Linker::layout() {
 
   // Emit the pre-assigned function names in sorted order
   for (const auto& P : functionNames) {
-    ensureTableIsPopulated();
+    ensureTableSegment();
     getTableDataRef().push_back(P.second);
   }
 
@@ -319,7 +320,7 @@ void Linker::emscriptenGlue(std::ostream& o) {
   emscripten::generateEmscriptenMetadata(o, out.wasm, segmentsByAddress, staticBump, out.initializerFunctions);
 }
 
-void Linker::ensureTableIsPopulated() {
+void Linker::ensureTableSegment() {
   if (out.wasm.table.segments.size() == 0) {
     auto emptySegment = out.wasm.allocator.alloc<Const>()->set(Literal(uint32_t(0)));
     out.wasm.table.segments.emplace_back(emptySegment);
@@ -340,7 +341,7 @@ std::vector<Name> Linker::getTableData() {
 
 Index Linker::getFunctionIndex(Name name) {
   if (!functionIndexes.count(name)) {
-    ensureTableIsPopulated();
+    ensureTableSegment();
     functionIndexes[name] = getTableData().size();
     getTableDataRef().push_back(name);
     if (debug) {
diff --git a/src/wasm-linker.h b/src/wasm-linker.h
index 2a6e3c01a..4a0e1e0b9 100644
--- a/src/wasm-linker.h
+++ b/src/wasm-linker.h
@@ -269,7 +269,7 @@ class Linker {
 
   // Makes sure the table has a single segment, with offset 0,
   // to which we can add content.
-  void ensureTableIsPopulated();
+  void ensureTableSegment();
 
   std::vector<Name>& getTableDataRef();
   std::vector<Name> getTableData();
diff --git a/src/wasm-s-parser.h b/src/wasm-s-parser.h
index 90f5a2ea3..72949eb2b 100644
--- a/src/wasm-s-parser.h
+++ b/src/wasm-s-parser.h
@@ -1346,7 +1346,7 @@ private:
   }
 
   Expression* makeCallIndirect(Element& s) {
-    if (!seenTable) throw ParseException("no table");
+    if (!wasm.table.exists) throw ParseException("no table");
     auto ret = allocator.alloc<CallIndirect>();
     IString type = s[1]->str();
     auto* fullType = wasm.checkFunctionType(type);
@@ -1609,8 +1609,8 @@ private:
         hasMemory = true;
       } else if ((*s[3])[0]->str() == TABLE) {
         im->kind = ExternalKind::Table;
-        if (seenTable) throw ParseException("more than one table");
-        seenTable = true;
+        if (wasm.table.exists) throw ParseException("more than one table");
+        wasm.table.exists = true;
       } else if ((*s[3])[0]->str() == GLOBAL) {
         im->kind = ExternalKind::Global;
       } else {
@@ -1781,11 +1781,10 @@ private:
     wasm.addGlobal(global.release());
   }
 
-  bool seenTable = false;
 
   void parseTable(Element& s, bool preParseImport = false) {
-    if (seenTable) throw ParseException("more than one table");
-    seenTable = true;
+    if (wasm.table.exists) throw ParseException("more than one table");
+    wasm.table.exists = true;
     Index i = 1;
     if (i == s.size()) return; // empty table in old notation
     if (s[i]->dollared()) {
@@ -1855,7 +1854,7 @@ private:
   }
 
   void parseInnerElem(Element& s, Index i = 1, Expression* offset = nullptr) {
-    if (!seenTable) throw ParseException("elem without table", s.line, s.col);
+    if (!wasm.table.exists) throw ParseException("elem without table", s.line, s.col);
     if (!offset) {
       offset = allocator.alloc<Const>()->set(Literal(int32_t(0)));
     }
diff --git a/src/wasm.h b/src/wasm.h
index 54461d2e7..ca9392d98 100644
--- a/src/wasm.h
+++ b/src/wasm.h
@@ -1466,11 +1466,14 @@ public:
     }
   };
 
+  // Currently the wasm object always 'has' one Table. It 'exists' if it has been defined or imported.
+  // The table can exist but be empty and have no defined initial or max size.
+  bool exists;
   Name name;
   Address initial, max;
   std::vector<Segment> segments;
 
-  Table() : initial(0), max(kMaxSize) {
+  Table() : exists(false), initial(0), max(kMaxSize) {
     name = Name::fromInt(0);
   }
 };