[cleanup] Unify exports handling in WasmModuleBuilder

Change-Id: Id474294a808f5c77321cd12ff5333eb6000b04fa
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1692933
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62655}

src/asmjs/asm-parser.cc

@@ -253,7 +253,7 @@ void AsmJsParser::DeclareGlobal(VarInfo* info, bool mutable_variable,
                                 const WasmInitExpr& init) {
   info->kind = VarKind::kGlobal;
   info->type = type;
-  info->index = module_builder_->AddGlobal(vtype, false, true, init);
+  info->index = module_builder_->AddGlobal(vtype, true, init);
   info->mutable_variable = mutable_variable;
 }
 

src/wasm/wasm-module-builder.cc

@@ -230,11 +230,8 @@ WasmModuleBuilder::WasmModuleBuilder(Zone* zone)
     : zone_(zone),
       signatures_(zone),
       function_imports_(zone),
-      function_exports_(zone),
       global_imports_(zone),
-      global_exports_(zone),
-      memory_exports_(zone),
-      table_exports_(zone),
+      exports_(zone),
       functions_(zone),
       data_segments_(zone),
       indirect_functions_(zone),
@@ -314,33 +311,33 @@ void WasmModuleBuilder::MarkStartFunction(WasmFunctionBuilder* function) {
 }
 
 void WasmModuleBuilder::AddExport(Vector<const char> name,
-                                  WasmFunctionBuilder* function) {
-  DCHECK_LE(function->func_index(), std::numeric_limits<int>::max());
-  function_exports_.push_back({name, static_cast<int>(function->func_index())});
+                                  ImportExportKindCode kind, uint32_t index) {
+  DCHECK_LE(index, std::numeric_limits<int>::max());
+  exports_.push_back({name, kind, static_cast<int>(index)});
 }
 
 uint32_t WasmModuleBuilder::AddExportedGlobal(ValueType type, bool mutability,
                                               const WasmInitExpr& init,
                                               Vector<const char> name) {
-  uint32_t index = AddGlobal(type, true, mutability, init);
-  global_exports_.push_back({name, index});
+  uint32_t index = AddGlobal(type, mutability, init);
+  AddExport(name, kExternalGlobal, index);
   return index;
 }
 
-void WasmModuleBuilder::AddExportedImport(Vector<const char> name,
-                                          int import_index) {
+void WasmModuleBuilder::ExportImportedFunction(Vector<const char> name,
+                                               int import_index) {
 #if DEBUG
   // The size of function_imports_ must not change any more.
   adding_imports_allowed_ = false;
 #endif
-  function_exports_.push_back(
-      {name, import_index - static_cast<int>(function_imports_.size())});
+  exports_.push_back(
+      {name, kExternalFunction,
+       import_index - static_cast<int>(function_imports_.size())});
 }
 
-uint32_t WasmModuleBuilder::AddGlobal(ValueType type, bool exported,
-                                      bool mutability,
+uint32_t WasmModuleBuilder::AddGlobal(ValueType type, bool mutability,
                                       const WasmInitExpr& init) {
-  globals_.push_back({type, exported, mutability, init});
+  globals_.push_back({type, mutability, init});
   return static_cast<uint32_t>(globals_.size() - 1);
 }
 
@@ -353,16 +350,6 @@ void WasmModuleBuilder::SetMaxMemorySize(uint32_t value) {
   max_memory_size_ = value;
 }
 
-void WasmModuleBuilder::AddExportedMemory(Vector<const char> name,
-                                          uint32_t index) {
-  memory_exports_.push_back({name, index});
-}
-
-void WasmModuleBuilder::AddExportedTable(Vector<const char> name,
-                                         uint32_t index) {
-  table_exports_.push_back({name, index});
-}
-
 void WasmModuleBuilder::SetHasSharedMemory() { has_shared_memory_ = true; }
 
 void WasmModuleBuilder::WriteTo(ZoneBuffer* buffer) const {
@@ -518,31 +505,28 @@ void WasmModuleBuilder::WriteTo(ZoneBuffer* buffer) const {
   }
 
   // == emit exports ===========================================================
-  size_t num_exports = global_exports_.size() + function_exports_.size() +
-                       memory_exports_.size() + table_exports_.size();
-  if (num_exports > 0) {
+  if (exports_.size() > 0) {
     size_t start = EmitSection(kExportSectionCode, buffer);
-    buffer->write_size(num_exports);
-    for (auto global_export : global_exports_) {
-      buffer->write_string(global_export.name);
-      buffer->write_u8(kExternalGlobal);
-      buffer->write_size(global_export.global_index + global_imports_.size());
-    }
-    for (auto function_export : function_exports_) {
-      buffer->write_string(function_export.name);
-      buffer->write_u8(kExternalFunction);
-      buffer->write_size(function_export.function_index +
-                         function_imports_.size());
-    }
-    for (auto memory_export : memory_exports_) {
-      buffer->write_string(memory_export.name);
-      buffer->write_u8(kExternalMemory);
-      buffer->write_size(memory_export.memory_index);
-    }
-    for (auto table_export : table_exports_) {
-      buffer->write_string(table_export.name);
-      buffer->write_u8(kExternalTable);
-      buffer->write_size(table_export.table_index);
+    buffer->write_size(exports_.size());
+    for (auto ex : exports_) {
+      buffer->write_string(ex.name);
+      buffer->write_u8(ex.kind);
+      switch (ex.kind) {
+        case kExternalFunction:
+          buffer->write_size(ex.index + function_imports_.size());
+          break;
+        case kExternalGlobal:
+          buffer->write_size(ex.index + global_imports_.size());
+          break;
+        case kExternalMemory:
+        case kExternalTable:
+          // The WasmModuleBuilder doesn't support importing tables or memories
+          // yet, so there is no index offset to add.
+          buffer->write_size(ex.index);
+          break;
+        case kExternalException:
+          UNREACHABLE();
+      }
     }
     FixupSection(buffer, start);
   }

src/wasm/wasm-module-builder.h

@@ -231,7 +231,7 @@ class V8_EXPORT_PRIVATE WasmModuleBuilder : public ZoneObject {
   // Building methods.
   uint32_t AddImport(Vector<const char> name, FunctionSig* sig);
   WasmFunctionBuilder* AddFunction(FunctionSig* sig = nullptr);
-  uint32_t AddGlobal(ValueType type, bool exported, bool mutability = true,
+  uint32_t AddGlobal(ValueType type, bool mutability = true,
                      const WasmInitExpr& init = WasmInitExpr());
   uint32_t AddGlobalImport(Vector<const char> name, ValueType type,
                            bool mutability);
@@ -244,12 +244,14 @@ class V8_EXPORT_PRIVATE WasmModuleBuilder : public ZoneObject {
   void SetIndirectFunction(uint32_t indirect, uint32_t direct);
   void SetMaxTableSize(uint32_t max);
   void MarkStartFunction(WasmFunctionBuilder* builder);
-  void AddExport(Vector<const char> name, WasmFunctionBuilder* builder);
+  void AddExport(Vector<const char> name, ImportExportKindCode kind,
+                 uint32_t index);
+  void AddExport(Vector<const char> name, WasmFunctionBuilder* builder) {
+    AddExport(name, kExternalFunction, builder->func_index());
+  }
   uint32_t AddExportedGlobal(ValueType type, bool mutability,
                              const WasmInitExpr& init, Vector<const char> name);
-  void AddExportedImport(Vector<const char> name, int import_index);
-  void AddExportedMemory(Vector<const char> name, uint32_t memory_index);
-  void AddExportedTable(Vector<const char> name, uint32_t table_index);
+  void ExportImportedFunction(Vector<const char> name, int import_index);
   void SetMinMemorySize(uint32_t value);
   void SetMaxMemorySize(uint32_t value);
   void SetHasSharedMemory();
@@ -268,40 +270,24 @@ class V8_EXPORT_PRIVATE WasmModuleBuilder : public ZoneObject {
     uint32_t sig_index;
   };
 
-  struct WasmFunctionExport {
-    Vector<const char> name;
-    // Can be negative for re-exported imported functions.
-    int function_index;
-  };
-
   struct WasmGlobalImport {
     Vector<const char> name;
     ValueTypeCode type_code;
     bool mutability;
   };
 
-  struct WasmGlobalExport {
+  struct WasmExport {
     Vector<const char> name;
-    uint32_t global_index;
+    ImportExportKindCode kind;
+    int index;  // Can be negative for re-exported imports.
   };
 
   struct WasmGlobal {
     ValueType type;
-    bool exported;
     bool mutability;
     WasmInitExpr init;
   };
 
-  struct WasmMemoryExport {
-    Vector<const char> name;
-    uint32_t memory_index;
-  };
-
-  struct WasmTableExport {
-    Vector<const char> name;
-    uint32_t table_index;
-  };
-
   struct WasmDataSegment {
     ZoneVector<byte> data;
     uint32_t dest;
@@ -311,11 +297,8 @@ class V8_EXPORT_PRIVATE WasmModuleBuilder : public ZoneObject {
   Zone* zone_;
   ZoneVector<FunctionSig*> signatures_;
   ZoneVector<WasmFunctionImport> function_imports_;
-  ZoneVector<WasmFunctionExport> function_exports_;
   ZoneVector<WasmGlobalImport> global_imports_;
-  ZoneVector<WasmGlobalExport> global_exports_;
-  ZoneVector<WasmMemoryExport> memory_exports_;
-  ZoneVector<WasmTableExport> table_exports_;
+  ZoneVector<WasmExport> exports_;
   ZoneVector<WasmFunctionBuilder*> functions_;
   ZoneVector<WasmDataSegment> data_segments_;
   ZoneVector<uint32_t> indirect_functions_;

test/cctest/wasm/test-run-wasm-module.cc

@@ -429,8 +429,8 @@ TEST(Run_WasmModule_Global) {
     TestSignatures sigs;
 
     WasmModuleBuilder* builder = new (&zone) WasmModuleBuilder(&zone);
-    uint32_t global1 = builder->AddGlobal(kWasmI32, false);
-    uint32_t global2 = builder->AddGlobal(kWasmI32, false);
+    uint32_t global1 = builder->AddGlobal(kWasmI32);
+    uint32_t global2 = builder->AddGlobal(kWasmI32);
     WasmFunctionBuilder* f1 = builder->AddFunction(sigs.i_v());
     byte code1[] = {
         WASM_I32_ADD(WASM_GET_GLOBAL(global1), WASM_GET_GLOBAL(global2))};
@@ -745,9 +745,9 @@ TEST(Run_WasmModule_Global_init) {
 
     WasmModuleBuilder* builder = new (&zone) WasmModuleBuilder(&zone);
     uint32_t global1 =
-        builder->AddGlobal(kWasmI32, false, false, WasmInitExpr(777777));
+        builder->AddGlobal(kWasmI32, false, WasmInitExpr(777777));
     uint32_t global2 =
-        builder->AddGlobal(kWasmI32, false, false, WasmInitExpr(222222));
+        builder->AddGlobal(kWasmI32, false, WasmInitExpr(222222));
     WasmFunctionBuilder* f1 = builder->AddFunction(sigs.i_v());
     byte code[] = {
         WASM_I32_ADD(WASM_GET_GLOBAL(global1), WASM_GET_GLOBAL(global2))};
@@ -773,12 +773,11 @@ static void RunWasmModuleGlobalInitTest(ValueType type, CType expected) {
       WasmModuleBuilder* builder = new (&zone) WasmModuleBuilder(&zone);
 
       for (int i = 0; i < padding; i++) {  // pad global before
-        builder->AddGlobal(kWasmI32, false, false, WasmInitExpr(i + 20000));
+        builder->AddGlobal(kWasmI32, false, WasmInitExpr(i + 20000));
       }
-      uint32_t global =
-          builder->AddGlobal(type, false, false, WasmInitExpr(expected));
+      uint32_t global = builder->AddGlobal(type, false, WasmInitExpr(expected));
       for (int i = 0; i < padding; i++) {  // pad global after
-        builder->AddGlobal(kWasmI32, false, false, WasmInitExpr(i + 30000));
+        builder->AddGlobal(kWasmI32, false, WasmInitExpr(i + 30000));
       }
 
       WasmFunctionBuilder* f1 = builder->AddFunction(&sig);

test/cctest/wasm/test-streaming-compilation.cc

@@ -313,7 +313,7 @@ ZoneBuffer GetModuleWithInvalidSection(Zone* zone) {
   TestSignatures sigs;
   WasmModuleBuilder builder(zone);
   // Add an invalid global to the module. The decoder will fail there.
-  builder.AddGlobal(kWasmStmt, false, true,
+  builder.AddGlobal(kWasmStmt, true,
                     WasmInitExpr(WasmInitExpr::kGlobalIndex, 12));
   {
     WasmFunctionBuilder* f = builder.AddFunction(sigs.i_iii());

test/fuzzer/wasm-compile.cc

@@ -825,10 +825,9 @@ class WasmCompileFuzzer : public WasmExecutionFuzzer {
 
     for (int i = 0; i < num_globals; ++i) {
       ValueType type = GetValueType(&range);
-      const bool exported = range.get<bool>();
       // 1/8 of globals are immutable.
       const bool mutability = (range.get<uint8_t>() % 8) != 0;
-      builder.AddGlobal(type, exported, mutability, WasmInitExpr());
+      builder.AddGlobal(type, mutability, WasmInitExpr());
       globals.push_back(type);
       if (mutability) mutable_globals.push_back(static_cast<uint8_t>(i));
     }

test/wasm-api-tests/callbacks.cc

@@ -193,7 +193,7 @@ TEST_F(WasmCapiTest, DirectCallCapiFunction) {
                             kWasmF64,    kWasmAnyRef};
   FunctionSig wasm_sig(5, 5, wasm_types);
   int func_index = builder()->AddImport(CStrVector("func"), &wasm_sig);
-  builder()->AddExportedImport(CStrVector("func"), func_index);
+  builder()->ExportImportedFunction(CStrVector("func"), func_index);
   Instantiate(imports);
   int32_t a0 = 42;
   int64_t a1 = 0x1234c0ffee;

test/wasm-api-tests/memory.cc

@@ -14,7 +14,7 @@ using ::wasm::MemoryType;
 TEST_F(WasmCapiTest, Memory) {
   builder()->SetMinMemorySize(2);
   builder()->SetMaxMemorySize(3);
-  builder()->AddExportedMemory(CStrVector("memory"), 0);
+  builder()->AddExport(CStrVector("memory"), kExternalMemory, 0);
 
   ValueType i32_type[] = {kWasmI32, kWasmI32};
   FunctionSig return_i32(1, 0, i32_type);
@@ -36,12 +36,10 @@ TEST_F(WasmCapiTest, Memory) {
 
   Instantiate(nullptr);
 
-  // TODO(jkummerow): Getting exports by index leaks implementation details
-  // of the module builder. It would be nicer to get exports by name instead.
-  Func* size_func = GetExportedFunction(0);
-  Func* load_func = GetExportedFunction(1);
-  Func* store_func = GetExportedFunction(2);
-  Memory* memory = GetExportedMemory(3);
+  Memory* memory = GetExportedMemory(0);
+  Func* size_func = GetExportedFunction(1);
+  Func* load_func = GetExportedFunction(2);
+  Func* store_func = GetExportedFunction(3);
 
   // Check initial state.
   EXPECT_EQ(2u, memory->size());

test/wasm-api-tests/reflect.cc

@@ -44,10 +44,10 @@ TEST_F(WasmCapiTest, Reflect) {
                                CStrVector(kGlobalName));
 
   builder()->AllocateIndirectFunctions(12);
-  builder()->AddExportedTable(CStrVector(kTableName), 0);
+  builder()->AddExport(CStrVector(kTableName), kExternalTable, 0);
 
   builder()->SetMinMemorySize(1);
-  builder()->AddExportedMemory(CStrVector(kMemoryName), 0);
+  builder()->AddExport(CStrVector(kMemoryName), kExternalMemory, 0);
 
   Instantiate(nullptr);
 

test/wasm-api-tests/table.cc

@@ -39,7 +39,7 @@ void ExpectResult(int expected, const Func* func, int arg1, int arg2) {
 TEST_F(WasmCapiTest, Table) {
   builder()->AllocateIndirectFunctions(2);
   builder()->SetMaxTableSize(10);
-  builder()->AddExportedTable(CStrVector("table"), 0);
+  builder()->AddExport(CStrVector("table"), kExternalTable, 0);
   const uint32_t sig_i_i_index = builder()->AddSignature(wasm_i_i_sig());
   ValueType reps[] = {kWasmI32, kWasmI32, kWasmI32};
   FunctionSig call_sig(1, 2, reps);
@@ -56,10 +56,10 @@ TEST_F(WasmCapiTest, Table) {
 
   Instantiate(nullptr);
 
-  Func* call_indirect = GetExportedFunction(0);
-  Func* f = GetExportedFunction(1);
-  Func* g = GetExportedFunction(2);
-  Table* table = GetExportedTable(3);
+  Table* table = GetExportedTable(0);
+  Func* call_indirect = GetExportedFunction(1);
+  Func* f = GetExportedFunction(2);
+  Func* g = GetExportedFunction(3);
   own<Func*> h = Func::make(store(), cpp_i_i_sig(), Negate);
 
   // Check initial table state.