[wasm] Support table.copy for multiple tables

Even though this is not spec'ed yet, it's good to have an implementation
so that we can use clusterfuzz on it.

I changed the parameter order (hopefully) everywhere to
(table_dst_index, table_src_index, ...). This corresponds to the
(dst, src, ...) parameter order for the entry indices.

R=binji@chromium.org

Bug: v8:7581 chromium:980475
Change-Id: I2fb36ffd4bb2f2be5b22c8366732295fa6759236
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1698386Reviewed-by: Ben Smith <binji@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62661}

src/compiler/wasm-compiler.cc

@@ -4837,13 +4837,13 @@ Node* WasmGraphBuilder::ElemDrop(uint32_t elem_segment_index,
                        mcgraph()->Int32Constant(1), Effect(), Control()));
 }
 
-Node* WasmGraphBuilder::TableCopy(uint32_t table_src_index,
-                                  uint32_t table_dst_index, Node* dst,
+Node* WasmGraphBuilder::TableCopy(uint32_t table_dst_index,
+                                  uint32_t table_src_index, Node* dst,
                                   Node* src, Node* size,
                                   wasm::WasmCodePosition position) {
   Node* args[] = {
-      graph()->NewNode(mcgraph()->common()->NumberConstant(table_src_index)),
       graph()->NewNode(mcgraph()->common()->NumberConstant(table_dst_index)),
+      graph()->NewNode(mcgraph()->common()->NumberConstant(table_src_index)),
       BuildConvertUint32ToSmiWithSaturation(dst, FLAG_wasm_max_table_size),
       BuildConvertUint32ToSmiWithSaturation(src, FLAG_wasm_max_table_size),
       BuildConvertUint32ToSmiWithSaturation(size, FLAG_wasm_max_table_size)};

src/compiler/wasm-compiler.h

@@ -405,7 +405,7 @@ class WasmGraphBuilder {
   Node* TableInit(uint32_t table_index, uint32_t elem_segment_index, Node* dst,
                   Node* src, Node* size, wasm::WasmCodePosition position);
   Node* ElemDrop(uint32_t elem_segment_index, wasm::WasmCodePosition position);
-  Node* TableCopy(uint32_t table_src_index, uint32_t table_dst_index, Node* dst,
+  Node* TableCopy(uint32_t table_dst_index, uint32_t table_src_index, Node* dst,
                   Node* src, Node* size, wasm::WasmCodePosition position);
   Node* TableGrow(uint32_t table_index, Node* value, Node* delta);
   Node* TableSize(uint32_t table_index);

src/runtime/runtime-wasm.cc

@@ -502,16 +502,18 @@ RUNTIME_FUNCTION(Runtime_WasmTableInit) {
 RUNTIME_FUNCTION(Runtime_WasmTableCopy) {
   HandleScope scope(isolate);
   DCHECK_EQ(5, args.length());
+  DCHECK(isolate->context().is_null());
+  isolate->set_context(GetNativeContextFromWasmInstanceOnStackTop(isolate));
   auto instance =
       Handle<WasmInstanceObject>(GetWasmInstanceOnStackTop(isolate), isolate);
-  CONVERT_UINT32_ARG_CHECKED(table_src_index, 0);
-  CONVERT_UINT32_ARG_CHECKED(table_dst_index, 1);
+  CONVERT_UINT32_ARG_CHECKED(table_dst_index, 0);
+  CONVERT_UINT32_ARG_CHECKED(table_src_index, 1);
   CONVERT_UINT32_ARG_CHECKED(dst, 2);
   CONVERT_UINT32_ARG_CHECKED(src, 3);
   CONVERT_UINT32_ARG_CHECKED(count, 4);
 
   bool oob = !WasmInstanceObject::CopyTableEntries(
-      isolate, instance, table_src_index, table_dst_index, dst, src, count);
+      isolate, instance, table_dst_index, table_src_index, dst, src, count);
   if (oob) return ThrowTableOutOfBounds(isolate, instance);
   return ReadOnlyRoots(isolate).undefined_value();
 }

src/wasm/function-body-decoder-impl.h

@@ -574,14 +574,14 @@ struct ElemDropImmediate {
 
 template <Decoder::ValidateFlag validate>
 struct TableCopyImmediate {
-  TableIndexImmediate<validate> table_src;
   TableIndexImmediate<validate> table_dst;
+  TableIndexImmediate<validate> table_src;
   unsigned length = 0;
 
   inline TableCopyImmediate(Decoder* decoder, const byte* pc) {
-    table_src = TableIndexImmediate<validate>(decoder, pc + 1);
-    table_dst =
-        TableIndexImmediate<validate>(decoder, pc + 1 + table_src.length);
+    table_dst = TableIndexImmediate<validate>(decoder, pc + 1);
+    table_src =
+        TableIndexImmediate<validate>(decoder, pc + 1 + table_dst.length);
     length = table_src.length + table_dst.length;
   }
 };

src/wasm/graph-builder-interface.cc

@@ -567,7 +567,7 @@ class WasmGraphBuildingInterface {
 
   void TableCopy(FullDecoder* decoder, const TableCopyImmediate<validate>& imm,
                  Vector<Value> args) {
-    BUILD(TableCopy, imm.table_src.index, imm.table_dst.index, args[0].node,
+    BUILD(TableCopy, imm.table_dst.index, imm.table_src.index, args[0].node,
           args[1].node, args[2].node, decoder->position());
   }
 

src/wasm/wasm-objects.cc

@@ -1589,11 +1589,6 @@ Address IndirectFunctionTableEntry::target() const {
              : table_->targets()[index_];
 }
 
-void IndirectFunctionTableEntry::CopyFrom(
-    const IndirectFunctionTableEntry& that) {
-  Set(that.sig_id(), that.target(), that.object_ref());
-}
-
 void ImportedFunctionEntry::SetWasmToJs(
     Isolate* isolate, Handle<JSReceiver> callable,
     const wasm::WasmCode* wasm_to_js_wrapper) {
@@ -1830,86 +1825,40 @@ int WasmInstanceObject::IndirectFunctionTableSize(
   return table->size();
 }
 
-namespace {
-void CopyTableEntriesImpl(Handle<WasmInstanceObject> instance, uint32_t dst,
-                          uint32_t src, uint32_t count, bool copy_backward) {
-  DCHECK(IsInBounds(dst, count, instance->indirect_function_table_size()));
-  if (copy_backward) {
-    for (uint32_t i = count; i > 0; i--) {
-      // TODO(ahaas): Use a table index here once table.copy supports multiple
-      // tables.
-      auto to_entry = IndirectFunctionTableEntry(instance, 0, dst + i - 1);
-      auto from_entry = IndirectFunctionTableEntry(instance, 0, src + i - 1);
-      to_entry.CopyFrom(from_entry);
-    }
-  } else {
-    for (uint32_t i = 0; i < count; i++) {
-      // TODO(ahaas): Use a table index here once table.copy supports multiple
-      // tables.
-      auto to_entry = IndirectFunctionTableEntry(instance, 0, dst + i);
-      auto from_entry = IndirectFunctionTableEntry(instance, 0, src + i);
-      to_entry.CopyFrom(from_entry);
-    }
-  }
-}
-}  // namespace
-
 // static
 bool WasmInstanceObject::CopyTableEntries(Isolate* isolate,
                                           Handle<WasmInstanceObject> instance,
-                                          uint32_t table_src_index,
                                           uint32_t table_dst_index,
+                                          uint32_t table_src_index,
                                           uint32_t dst, uint32_t src,
                                           uint32_t count) {
-  if (static_cast<int>(table_dst_index) >= instance->tables().length()) {
-    return false;
-  }
-  if (static_cast<int>(table_src_index) >= instance->tables().length()) {
-    return false;
-  }
-
-  // TODO(titzer): multiple tables in TableCopy
-  CHECK_EQ(0, table_src_index);
-  CHECK_EQ(0, table_dst_index);
-  auto table = handle(
+  CHECK_LT(table_dst_index, instance->tables().length());
+  CHECK_LT(table_src_index, instance->tables().length());
+  auto table_dst = handle(
+      WasmTableObject::cast(instance->tables().get(table_dst_index)), isolate);
+  auto table_src = handle(
       WasmTableObject::cast(instance->tables().get(table_src_index)), isolate);
-  uint32_t max = static_cast<uint32_t>(table->entries().length());
+  uint32_t max_dst = static_cast<uint32_t>(table_dst->entries().length());
+  uint32_t max_src = static_cast<uint32_t>(table_src->entries().length());
   bool copy_backward = src < dst && dst - src < count;
-  bool ok = ClampToBounds(dst, &count, max);
+  bool ok = ClampToBounds(dst, &count, max_dst);
   // Use & instead of && so the clamp is not short-circuited.
-  ok &= ClampToBounds(src, &count, max);
+  ok &= ClampToBounds(src, &count, max_src);
 
   // If performing a partial copy when copying backward, then the first access
   // will be out-of-bounds, so no entries should be copied.
   if (copy_backward && !ok) return ok;
 
-  if (dst == src || count == 0) return ok;  // no-op
-
-  // Broadcast table copy operation to all instances that import this table.
-  Handle<FixedArray> dispatch_tables(table->dispatch_tables(), isolate);
-  for (int i = 0; i < dispatch_tables->length();
-       i += kDispatchTableNumElements) {
-    Handle<WasmInstanceObject> target_instance(
-        WasmInstanceObject::cast(
-            dispatch_tables->get(i + kDispatchTableInstanceOffset)),
-        isolate);
-    CopyTableEntriesImpl(target_instance, dst, src, count, copy_backward);
+  // no-op
+  if ((dst == src && table_dst_index == table_src_index) || count == 0) {
+    return ok;
   }
 
-  // Copy the function entries.
-  auto dst_table = handle(
-      WasmTableObject::cast(instance->tables().get(table_dst_index)), isolate);
-  auto src_table = handle(
-      WasmTableObject::cast(instance->tables().get(table_src_index)), isolate);
-  if (copy_backward) {
-    for (uint32_t i = count; i > 0; i--) {
-      dst_table->entries().set(dst + i - 1,
-                               src_table->entries().get(src + i - 1));
-    }
-  } else {
-    for (uint32_t i = 0; i < count; i++) {
-      dst_table->entries().set(dst + i, src_table->entries().get(src + i));
-    }
+  for (uint32_t i = 0; i < count; ++i) {
+    uint32_t src_index = copy_backward ? (src + count - i - 1) : src + i;
+    uint32_t dst_index = copy_backward ? (dst + count - i - 1) : dst + i;
+    auto value = WasmTableObject::Get(isolate, table_src, src_index);
+    WasmTableObject::Set(isolate, table_dst, dst_index, value);
   }
   return ok;
 }

src/wasm/wasm-objects.h

@@ -74,8 +74,6 @@ class IndirectFunctionTableEntry {
                              int target_func_index);
   void Set(int sig_id, Address call_target, Object ref);
 
-  void CopyFrom(const IndirectFunctionTableEntry& that);
-
   Object object_ref() const;
   int sig_id() const;
   Address target() const;
@@ -580,8 +578,8 @@ class WasmInstanceObject : public JSObject {
   // Copies table entries. Returns {false} if the ranges are out-of-bounds.
   static bool CopyTableEntries(Isolate* isolate,
                                Handle<WasmInstanceObject> instance,
-                               uint32_t table_src_index,
-                               uint32_t table_dst_index, uint32_t dst,
+                               uint32_t table_dst_index,
+                               uint32_t table_src_index, uint32_t dst,
                                uint32_t src,
                                uint32_t count) V8_WARN_UNUSED_RESULT;
 

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

@@ -201,7 +201,7 @@ void TestingModuleBuilder::AddIndirectFunctionTable(
     for (uint32_t i = 0; i < table_size; ++i) {
       WasmFunction& function = test_module_->functions[function_indexes[i]];
       int sig_id = test_module_->signature_map.Find(*function.sig);
-      IndirectFunctionTableEntry(instance, 0, i)
+      IndirectFunctionTableEntry(instance, table_index, i)
           .Set(sig_id, instance, function.func_index);
       WasmTableObject::SetFunctionTablePlaceholder(
           isolate_, table_obj, i, instance_object_, function_indexes[i]);

test/common/wasm/wasm-macro-gen.h

@@ -632,8 +632,9 @@ inline WasmOpcode LoadStoreOpcodeOf(MachineType type, bool store) {
   dst, src, size, WASM_NUMERIC_OP(kExprTableInit), U32V_1(seg), \
       static_cast<byte>(table)
 #define WASM_ELEM_DROP(seg) WASM_NUMERIC_OP(kExprElemDrop), U32V_1(seg)
-#define WASM_TABLE_COPY(dst, src, size) \
-  dst, src, size, WASM_NUMERIC_OP(kExprTableCopy), TABLE_ZERO, TABLE_ZERO
+#define WASM_TABLE_COPY(table_dst, table_src, dst, src, size) \
+  dst, src, size, WASM_NUMERIC_OP(kExprTableCopy),            \
+      static_cast<byte>(table_dst), static_cast<byte>(table_src)
 #define WASM_TABLE_GROW(table, initial_value, delta)     \
   initial_value, delta, WASM_NUMERIC_OP(kExprTableGrow), \
       static_cast<byte>(table)

test/unittests/wasm/function-body-decoder-unittest.cc

@@ -3218,10 +3218,11 @@ TEST_F(FunctionBodyDecoderTest, TableCopy) {
   builder.InitializeTable();
   module = builder.module();
 
-  ExpectFailure(sigs.v_v(), {WASM_TABLE_COPY(WASM_ZERO, WASM_ZERO, WASM_ZERO)});
+  ExpectFailure(sigs.v_v(),
+                {WASM_TABLE_COPY(0, 0, WASM_ZERO, WASM_ZERO, WASM_ZERO)});
   WASM_FEATURE_SCOPE(bulk_memory);
   ExpectValidates(sigs.v_v(),
-                  {WASM_TABLE_COPY(WASM_ZERO, WASM_ZERO, WASM_ZERO)});
+                  {WASM_TABLE_COPY(0, 0, WASM_ZERO, WASM_ZERO, WASM_ZERO)});
 }
 
 TEST_F(FunctionBodyDecoderTest, TableGrow) {
@@ -3302,7 +3303,57 @@ TEST_F(FunctionBodyDecoderTest, TableOpsWithoutTable) {
     ExpectFailure(sigs.v_v(),
                   {WASM_TABLE_INIT(0, 0, WASM_ZERO, WASM_ZERO, WASM_ZERO)});
     ExpectFailure(sigs.v_v(),
-                  {WASM_TABLE_COPY(WASM_ZERO, WASM_ZERO, WASM_ZERO)});
+                  {WASM_TABLE_COPY(0, 0, WASM_ZERO, WASM_ZERO, WASM_ZERO)});
+  }
+}
+
+TEST_F(FunctionBodyDecoderTest, TableCopyMultiTable) {
+  WASM_FEATURE_SCOPE(bulk_memory);
+  WASM_FEATURE_SCOPE(anyref);
+  {
+    TestModuleBuilder builder;
+    builder.AddTable(kWasmAnyRef, 10, true, 20);
+    builder.AddPassiveElementSegment();
+    module = builder.module();
+    // We added one table, therefore table.copy on table 0 should work.
+    int table_src = 0;
+    int table_dst = 0;
+    ExpectValidates(sigs.v_v(),
+                    {WASM_TABLE_COPY(table_dst, table_src, WASM_ZERO, WASM_ZERO,
+                                     WASM_ZERO)});
+    // There is only one table, so table.copy on table 1 should fail.
+    table_src = 0;
+    table_dst = 1;
+    ExpectFailure(sigs.v_v(), {WASM_TABLE_COPY(table_dst, table_src, WASM_ZERO,
+                                               WASM_ZERO, WASM_ZERO)});
+    table_src = 1;
+    table_dst = 0;
+    ExpectFailure(sigs.v_v(), {WASM_TABLE_COPY(table_dst, table_src, WASM_ZERO,
+                                               WASM_ZERO, WASM_ZERO)});
+  }
+  {
+    TestModuleBuilder builder;
+    builder.AddTable(kWasmAnyRef, 10, true, 20);
+    builder.AddTable(kWasmAnyRef, 10, true, 20);
+    builder.AddPassiveElementSegment();
+    module = builder.module();
+    // We added two tables, therefore table.copy on table 0 should work.
+    int table_src = 0;
+    int table_dst = 0;
+    ExpectValidates(sigs.v_v(),
+                    {WASM_TABLE_COPY(table_dst, table_src, WASM_ZERO, WASM_ZERO,
+                                     WASM_ZERO)});
+    // Also table.copy on table 1 should work now.
+    table_src = 1;
+    table_dst = 0;
+    ExpectValidates(sigs.v_v(),
+                    {WASM_TABLE_COPY(table_dst, table_src, WASM_ZERO, WASM_ZERO,
+                                     WASM_ZERO)});
+    table_src = 0;
+    table_dst = 1;
+    ExpectValidates(sigs.v_v(),
+                    {WASM_TABLE_COPY(table_dst, table_src, WASM_ZERO, WASM_ZERO,
+                                     WASM_ZERO)});
   }
 }