[wasm] Support reftypes tables in WasmModuleBuilder

WasmModuleBuilder is a class that is used to build Wasm modules in the
asm.js parser, in the fuzzer, as well as some tests. When it comes to
Wasm tables, WasmModuleBuilder currently supports only basic tables
(before the reftypes proposal) using an ad-hoc indirect-function index
vector.
This CL adds proper support for element sections and tables that use
them in the full potential of the reftypes extension. The new
functionality will only be used in the fuzzer and potentially some tests
in the future. Along this, we drop some functionality from
WasmModuleBuilder that was only used in tests and is redundant with the
new architecture.
Additionally, we remove tables other than externref and funcref from the
fuzzer (which were not supported properly or used anyway). We will
reintroduce them at a later time.

Bug: v8:11954
Change-Id: I0a4f6e7b63b6e3d9f7da03b5202fbf14d8678332
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3122162
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#76597}

src/asmjs/asm-parser.cc

@@ -698,7 +698,8 @@ void AsmJsParser::ValidateFunctionTable() {
         FAIL("Function table definition doesn't match use");
       }
       module_builder_->SetIndirectFunction(
-          static_cast<uint32_t>(table_info->index + count), info->index);
+          0, static_cast<uint32_t>(table_info->index + count), info->index,
+          WasmModuleBuilder::WasmElemSegment::kRelativeToDeclaredFunctions);
     }
     ++count;
     if (Check(',')) {
@@ -2134,7 +2135,10 @@ AsmType* AsmJsParser::ValidateCall() {
     EXPECT_TOKENn(']');
     VarInfo* function_info = GetVarInfo(function_name);
     if (function_info->kind == VarKind::kUnused) {
-      uint32_t index = module_builder_->AllocateIndirectFunctions(mask + 1);
+      if (module_builder_->NumTables() == 0) {
+        module_builder_->AddTable(kWasmFuncRef, 0);
+      }
+      uint32_t index = module_builder_->IncreaseTableMinSize(0, mask + 1);
       if (index == std::numeric_limits<uint32_t>::max()) {
         FAILn("Exceeded maximum function table size");
       }

src/wasm/wasm-module-builder.cc

@@ -264,7 +264,7 @@ WasmModuleBuilder::WasmModuleBuilder(Zone* zone)
       functions_(zone),
       tables_(zone),
       data_segments_(zone),
-      indirect_functions_(zone),
+      element_segments_(zone),
       globals_(zone),
       exceptions_(zone),
       signature_map_(zone),
@@ -323,75 +323,52 @@ uint32_t WasmModuleBuilder::AddArrayType(ArrayType* type) {
 const uint32_t WasmModuleBuilder::kNullIndex =
     std::numeric_limits<uint32_t>::max();
 
-// TODO(9495): Add support for typed function tables and more init. expressions.
-uint32_t WasmModuleBuilder::AllocateIndirectFunctions(uint32_t count) {
-  DCHECK(allocating_indirect_functions_allowed_);
-  uint32_t index = static_cast<uint32_t>(indirect_functions_.size());
-  DCHECK_GE(FLAG_wasm_max_table_size, index);
-  if (count > FLAG_wasm_max_table_size - index) {
+uint32_t WasmModuleBuilder::IncreaseTableMinSize(uint32_t table_index,
+                                                 uint32_t count) {
+  DCHECK_LT(table_index, tables_.size());
+  uint32_t old_min_size = tables_[table_index].min_size;
+  if (count > FLAG_wasm_max_table_size - old_min_size) {
     return std::numeric_limits<uint32_t>::max();
   }
-  uint32_t new_size = static_cast<uint32_t>(indirect_functions_.size()) + count;
-  DCHECK(max_table_size_ == 0 || new_size <= max_table_size_);
-  indirect_functions_.resize(new_size, kNullIndex);
-  uint32_t max = max_table_size_ > 0 ? max_table_size_ : new_size;
-  if (tables_.empty()) {
-    // This cannot use {AddTable} because that would flip the
-    // {allocating_indirect_functions_allowed_} flag.
-    tables_.push_back({kWasmFuncRef, new_size, max, true, {}});
-  } else {
-    // There can only be the indirect function table so far, otherwise the
-    // {allocating_indirect_functions_allowed_} flag would have been false.
-    DCHECK_EQ(1u, tables_.size());
-    DCHECK_EQ(kWasmFuncRef, tables_[0].type);
-    DCHECK(tables_[0].has_maximum);
-    tables_[0].min_size = new_size;
-    tables_[0].max_size = max;
-  }
-  return index;
-}
-
-void WasmModuleBuilder::SetIndirectFunction(uint32_t indirect,
-                                            uint32_t direct) {
-  indirect_functions_[indirect] = direct;
-}
-
-void WasmModuleBuilder::SetMaxTableSize(uint32_t max) {
-  DCHECK_GE(FLAG_wasm_max_table_size, max);
-  DCHECK_GE(max, indirect_functions_.size());
-  max_table_size_ = max;
-  DCHECK(allocating_indirect_functions_allowed_);
-  if (!tables_.empty()) {
-    tables_[0].max_size = max;
-  }
+  tables_[table_index].min_size = old_min_size + count;
+  tables_[table_index].max_size =
+      std::max(old_min_size + count, tables_[table_index].max_size);
+  return old_min_size;
 }
 
 uint32_t WasmModuleBuilder::AddTable(ValueType type, uint32_t min_size) {
-#if DEBUG
-  allocating_indirect_functions_allowed_ = false;
-#endif
   tables_.push_back({type, min_size, 0, false, {}});
   return static_cast<uint32_t>(tables_.size() - 1);
 }
 
 uint32_t WasmModuleBuilder::AddTable(ValueType type, uint32_t min_size,
                                      uint32_t max_size) {
-#if DEBUG
-  allocating_indirect_functions_allowed_ = false;
-#endif
   tables_.push_back({type, min_size, max_size, true, {}});
   return static_cast<uint32_t>(tables_.size() - 1);
 }
 
 uint32_t WasmModuleBuilder::AddTable(ValueType type, uint32_t min_size,
                                      uint32_t max_size, WasmInitExpr init) {
-#if DEBUG
-  allocating_indirect_functions_allowed_ = false;
-#endif
   tables_.push_back({type, min_size, max_size, true, std::move(init)});
   return static_cast<uint32_t>(tables_.size() - 1);
 }
 
+void WasmModuleBuilder::AddElementSegment(WasmElemSegment segment) {
+  element_segments_.push_back(std::move(segment));
+}
+
+void WasmModuleBuilder::SetIndirectFunction(
+    uint32_t table_index, uint32_t index_in_table,
+    uint32_t direct_function_index,
+    WasmElemSegment::FunctionIndexingMode indexing_mode) {
+  WasmElemSegment segment(zone_, kWasmFuncRef, table_index,
+                          WasmInitExpr(static_cast<int>(index_in_table)));
+  segment.indexing_mode = indexing_mode;
+  segment.entries.emplace_back(WasmElemSegment::Entry::kRefFuncEntry,
+                               direct_function_index);
+  AddElementSegment(std::move(segment));
+}
+
 uint32_t WasmModuleBuilder::AddImport(base::Vector<const char> name,
                                       FunctionSig* sig,
                                       base::Vector<const char> module) {
@@ -750,31 +727,67 @@ void WasmModuleBuilder::WriteTo(ZoneBuffer* buffer) const {
     FixupSection(buffer, start);
   }
 
-  // == emit function table elements ===========================================
-  if (indirect_functions_.size() > 0) {
+  // == emit element segments ==================================================
+  if (element_segments_.size() > 0) {
     size_t start = EmitSection(kElementSectionCode, buffer);
-    buffer->write_u8(1);              // count of entries
-    buffer->write_u8(0);              // table index
-    uint32_t first_element = 0;
-    while (first_element < indirect_functions_.size() &&
-           indirect_functions_[first_element] == kNullIndex) {
-      first_element++;
-    }
-    uint32_t last_element =
-        static_cast<uint32_t>(indirect_functions_.size() - 1);
-    while (last_element >= first_element &&
-           indirect_functions_[last_element] == kNullIndex) {
-      last_element--;
-    }
-    buffer->write_u8(kExprI32Const);  // offset
-    buffer->write_u32v(first_element);
-    buffer->write_u8(kExprEnd);
-    uint32_t element_count = last_element - first_element + 1;
-    buffer->write_size(element_count);
-    for (uint32_t i = first_element; i <= last_element; i++) {
-      buffer->write_size(indirect_functions_[i] + function_imports_.size());
+    buffer->write_size(element_segments_.size());
+    for (const WasmElemSegment& segment : element_segments_) {
+      bool is_active = segment.status == WasmElemSegment::kStatusActive;
+      // If this segment is expressible in the backwards-compatible syntax
+      // (before reftypes proposal), we should emit it in that syntax.
+      // This is the case if the segment is active and all entries are function
+      // references. Note that this is currently the only path that allows
+      // kRelativeToImports function indexing mode.
+      // TODO(manoskouk): Remove this logic once reftypes has shipped.
+      bool backwards_compatible =
+          is_active && segment.table_index == 0 &&
+          std::all_of(
+              segment.entries.begin(), segment.entries.end(), [](auto& entry) {
+                return entry.kind ==
+                       WasmModuleBuilder::WasmElemSegment::Entry::kRefFuncEntry;
+              });
+      if (backwards_compatible) {
+        buffer->write_u8(0);
+        WriteInitializerExpression(buffer, segment.offset, segment.type);
+        buffer->write_size(segment.entries.size());
+        for (const WasmElemSegment::Entry entry : segment.entries) {
+          buffer->write_u32v(
+              segment.indexing_mode == WasmElemSegment::kRelativeToImports
+                  ? entry.index
+                  : entry.index +
+                        static_cast<uint32_t>(function_imports_.size()));
+        }
+      } else {
+        DCHECK_EQ(segment.indexing_mode, WasmElemSegment::kRelativeToImports);
+        // If we pick the general syntax, we always explicitly emit the table
+        // index and the type, and use the expressions-as-elements syntax. I.e.
+        // the initial byte is one of 0x05, 0x06, and 0x07.
+        uint8_t kind_mask =
+            segment.status == WasmElemSegment::kStatusActive
+                ? 0b10
+                : segment.status == WasmElemSegment::kStatusDeclarative ? 0b11
+                                                                        : 0b01;
+        uint8_t expressions_as_elements_mask = 0b100;
+        buffer->write_u8(kind_mask | expressions_as_elements_mask);
+        if (is_active) {
+          buffer->write_u32v(segment.table_index);
+          WriteInitializerExpression(buffer, segment.offset, segment.type);
+        }
+        WriteValueType(buffer, segment.type);
+        buffer->write_size(segment.entries.size());
+        for (const WasmElemSegment::Entry entry : segment.entries) {
+          uint8_t opcode =
+              entry.kind == WasmElemSegment::Entry::kGlobalGetEntry
+                  ? kExprGlobalGet
+                  : entry.kind == WasmElemSegment::Entry::kRefFuncEntry
+                        ? kExprRefFunc
+                        : kExprRefNull;
+          buffer->write_u8(opcode);
+          buffer->write_u32v(entry.index);
+          buffer->write_u8(kExprEnd);
+        }
+      }
     }
-
     FixupSection(buffer, start);
   }
 

src/wasm/wasm-module-builder.h

@@ -246,6 +246,68 @@ class V8_EXPORT_PRIVATE WasmModuleBuilder : public ZoneObject {
   WasmModuleBuilder(const WasmModuleBuilder&) = delete;
   WasmModuleBuilder& operator=(const WasmModuleBuilder&) = delete;
 
+  // Static representation of wasm element segment (table initializer). This is
+  // different than the version in wasm-module.h.
+  class WasmElemSegment {
+   public:
+    // asm.js gives function indices starting with the first non-imported
+    // function.
+    enum FunctionIndexingMode {
+      kRelativeToImports,
+      kRelativeToDeclaredFunctions
+    };
+    enum Status {
+      kStatusActive,      // copied automatically during instantiation.
+      kStatusPassive,     // copied explicitly after instantiation.
+      kStatusDeclarative  // purely declarative and never copied.
+    };
+    struct Entry {
+      enum Kind { kGlobalGetEntry, kRefFuncEntry, kRefNullEntry } kind;
+      uint32_t index;
+      Entry(Kind kind, uint32_t index) : kind(kind), index(index) {}
+      Entry() : kind(kRefNullEntry), index(0) {}
+    };
+
+    // Construct an active segment.
+    WasmElemSegment(Zone* zone, ValueType type, uint32_t table_index,
+                    WasmInitExpr offset)
+        : type(type),
+          table_index(table_index),
+          offset(std::move(offset)),
+          entries(zone),
+          status(kStatusActive) {
+      DCHECK(IsValidOffsetKind(offset.kind()));
+    }
+
+    // Construct a passive or declarative segment, which has no table
+    // index or offset.
+    WasmElemSegment(Zone* zone, ValueType type, bool declarative)
+        : type(type),
+          table_index(0),
+          entries(zone),
+          status(declarative ? kStatusDeclarative : kStatusPassive) {
+      DCHECK(IsValidOffsetKind(offset.kind()));
+    }
+
+    MOVE_ONLY_NO_DEFAULT_CONSTRUCTOR(WasmElemSegment);
+
+    ValueType type;
+    uint32_t table_index;
+    WasmInitExpr offset;
+    FunctionIndexingMode indexing_mode = kRelativeToImports;
+    ZoneVector<Entry> entries;
+    Status status;
+
+   private:
+    // This ensures no {WasmInitExpr} with subexpressions is used, which would
+    // cause a memory leak because those are stored in an std::vector. Such
+    // offset would also be mistyped.
+    bool IsValidOffsetKind(WasmInitExpr::Operator kind) {
+      return kind == WasmInitExpr::kI32Const ||
+             kind == WasmInitExpr::kGlobalGet;
+    }
+  };
+
   // Building methods.
   uint32_t AddImport(base::Vector<const char> name, FunctionSig* sig,
                      base::Vector<const char> module = {});
@@ -256,16 +318,23 @@ class V8_EXPORT_PRIVATE WasmModuleBuilder : public ZoneObject {
                            bool mutability,
                            base::Vector<const char> module = {});
   void AddDataSegment(const byte* data, uint32_t size, uint32_t dest);
+  // Add an element segment to this {WasmModuleBuilder}. {segment}'s enties
+  // have to be initialized.
+  void AddElementSegment(WasmElemSegment segment);
+  // Helper method to create an active segment with one function. Assumes that
+  // table segment at {table_index} is typed as funcref.
+  void SetIndirectFunction(uint32_t table_index, uint32_t index_in_table,
+                           uint32_t direct_function_index,
+                           WasmElemSegment::FunctionIndexingMode indexing_mode);
+  // Increase the starting size of the table at {table_index} by {count}. Also
+  // increases the maximum table size if needed. Returns the former starting
+  // size, or the maximum uint32_t value if the maximum table size has been
+  // exceeded.
+  uint32_t IncreaseTableMinSize(uint32_t table_index, uint32_t count);
   uint32_t AddSignature(FunctionSig* sig);
   uint32_t AddException(FunctionSig* type);
   uint32_t AddStructType(StructType* type);
   uint32_t AddArrayType(ArrayType* type);
-  // In the current implementation, it's supported to have uninitialized slots
-  // at the beginning and/or end of the indirect function table, as long as
-  // the filled slots form a contiguous block in the middle.
-  uint32_t AllocateIndirectFunctions(uint32_t count);
-  void SetIndirectFunction(uint32_t indirect, uint32_t direct);
-  void SetMaxTableSize(uint32_t max);
   uint32_t AddTable(ValueType type, uint32_t min_size);
   uint32_t AddTable(ValueType type, uint32_t min_size, uint32_t max_size);
   uint32_t AddTable(ValueType type, uint32_t min_size, uint32_t max_size,
@@ -315,6 +384,8 @@ class V8_EXPORT_PRIVATE WasmModuleBuilder : public ZoneObject {
 
   int NumTypes() { return static_cast<int>(types_.size()); }
 
+  int NumTables() { return static_cast<int>(tables_.size()); }
+
   int NumFunctions() { return static_cast<int>(functions_.size()); }
 
   FunctionSig* GetExceptionType(int index) {
@@ -389,12 +460,11 @@ class V8_EXPORT_PRIVATE WasmModuleBuilder : public ZoneObject {
   ZoneVector<WasmFunctionBuilder*> functions_;
   ZoneVector<WasmTable> tables_;
   ZoneVector<WasmDataSegment> data_segments_;
-  ZoneVector<uint32_t> indirect_functions_;
+  ZoneVector<WasmElemSegment> element_segments_;
   ZoneVector<WasmGlobal> globals_;
   ZoneVector<int> exceptions_;
   ZoneUnorderedMap<FunctionSig, uint32_t> signature_map_;
   int start_function_index_;
-  uint32_t max_table_size_ = 0;
   uint32_t min_memory_size_;
   uint32_t max_memory_size_;
   bool has_max_memory_size_;
@@ -402,8 +472,6 @@ class V8_EXPORT_PRIVATE WasmModuleBuilder : public ZoneObject {
 #if DEBUG
   // Once AddExportedImport is called, no more imports can be added.
   bool adding_imports_allowed_ = true;
-  // Indirect functions must be allocated before adding extra tables.
-  bool allocating_indirect_functions_allowed_ = true;
 #endif
 };
 

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

@@ -303,10 +303,15 @@ TEST(WrapperReplacement_IndirectExport) {
     uint32_t function_index = f->func_index();
 
     // Export a table of indirect functions.
-    uint32_t table_index = builder->AllocateIndirectFunctions(2);
+    const uint32_t table_size = 2;
+    const uint32_t table_index =
+        builder->AddTable(kWasmFuncRef, table_size, table_size);
     builder->AddExport(base::CStrVector("exported_table"), kExternalTable, 0);
+
     // Point from the exported table to the Wasm function.
-    builder->SetIndirectFunction(0, function_index);
+    builder->SetIndirectFunction(
+        table_index, 0, function_index,
+        WasmModuleBuilder::WasmElemSegment::kRelativeToImports);
 
     // Compile the module.
     Handle<WasmInstanceObject> instance =

test/fuzzer/wasm-compile.cc

@@ -1960,27 +1960,40 @@ class WasmCompileFuzzer : public WasmExecutionFuzzer {
       if (i == 0) builder.AddExport(base::CStrVector("main"), f);
     }
 
-    builder.AllocateIndirectFunctions(num_functions);
-    for (int i = 0; i < num_functions; ++i) {
-      builder.SetIndirectFunction(i, i);
-    }
-
-    int num_tables = range.get<uint8_t>() % (kMaxTables + 1);
+    // Always generate at least one table for call_indirect.
+    int num_tables = range.get<uint8_t>() % kMaxTables + 1;
 
     for (int i = 0; i < num_tables; i++) {
-      uint32_t min_size = range.get<uint8_t>() % kMaxTableSize;
+      // Table 0 has to reference all functions in the program. This is so that
+      // all functions count as declared so they can be referenced with
+      // ref.func.
+      // TODO(11954): Consider removing this restriction.
+      uint32_t min_size =
+          i == 0 ? num_functions : range.get<uint8_t>() % kMaxTableSize;
       uint32_t max_size =
           range.get<uint8_t>() % (kMaxTableSize - min_size) + min_size;
-      int which_ref = range.get<uint8_t>() % 3;
-      ValueType type =
-          which_ref == 0
-              ? kWasmExternRef
-              : which_ref == 1
-                    ? kWasmFuncRef
-                    : ValueType::Ref(function_signatures[range.get<uint8_t>() %
-                                                         num_functions],
-                                     kNullable);
-      builder.AddTable(type, min_size, max_size);
+      // Table 0 is always funcref.
+      // TODO(11954): Remove this requirement this once we support call_indirect
+      // with other table indices.
+      // TODO(11954): Support typed function tables.
+      bool use_funcref = i == 0 || range.get<bool>();
+      ValueType type = use_funcref ? kWasmFuncRef : kWasmExternRef;
+      uint32_t table_index = builder.AddTable(type, min_size, max_size);
+      if (type == kWasmFuncRef) {
+        // For function tables, initialize them with functions from the program.
+        // Currently, the fuzzer assumes that every function table contains the
+        // functions in the program in the order they are defined.
+        // TODO(11954): Consider generalizing this.
+        WasmModuleBuilder::WasmElemSegment segment(
+            zone, kWasmFuncRef, table_index, WasmInitExpr(0));
+        for (int entry_index = 0; entry_index < static_cast<int>(min_size);
+             entry_index++) {
+          segment.entries.emplace_back(
+              WasmModuleBuilder::WasmElemSegment::Entry::kRefFuncEntry,
+              entry_index % num_functions);
+        }
+        builder.AddElementSegment(std::move(segment));
+      }
     }
     builder.SetMaxMemorySize(32);
     // We enable shared memory to be able to test atomics.

test/wasm-api-tests/reflect.cc

@@ -43,7 +43,7 @@ TEST_F(WasmCapiTest, Reflect) {
   builder()->AddExportedGlobal(kWasmF64, false, WasmInitExpr(0.0),
                                base::CStrVector(kGlobalName));
 
-  builder()->AllocateIndirectFunctions(12);
+  builder()->AddTable(kWasmFuncRef, 12, 12);
   builder()->AddExport(base::CStrVector(kTableName), kExternalTable, 0);
 
   builder()->SetMinMemorySize(1);

test/wasm-api-tests/table.cc

@@ -37,9 +37,8 @@ void ExpectResult(int expected, const Func* func, int arg1, int arg2) {
 }  // namespace
 
 TEST_F(WasmCapiTest, Table) {
-  builder()->AllocateIndirectFunctions(2);
-  builder()->SetMaxTableSize(10);
-  builder()->AddExport(base::CStrVector("table"), kExternalTable, 0);
+  const uint32_t table_index = builder()->AddTable(kWasmFuncRef, 2, 10);
+  builder()->AddExport(base::CStrVector("table"), kExternalTable, table_index);
   const uint32_t sig_i_i_index = builder()->AddSignature(wasm_i_i_sig());
   ValueType reps[] = {kWasmI32, kWasmI32, kWasmI32};
   FunctionSig call_sig(1, 2, reps);
@@ -54,7 +53,9 @@ TEST_F(WasmCapiTest, Table) {
   AddExportedFunction(base::CStrVector("g"), g_code, sizeof(g_code),
                       wasm_i_i_sig());
   // Set table[1] to {f}, which has function index 1.
-  builder()->SetIndirectFunction(1, 1);
+  builder()->SetIndirectFunction(
+      table_index, 1, 1,
+      WasmModuleBuilder::WasmElemSegment::kRelativeToImports);
 
   Instantiate(nullptr);