[wasm-gc] Support Table<struct|array index>

Bug: v8:7748
Change-Id: I4057a9288fe3d2dc0df308ce51be92e417572bd1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3865483Reviewed-by: Manos Koukoutos <manoskouk@chromium.org>
Commit-Queue: Matthias Liedtke <mliedtke@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82871}

src/wasm/canonical-types.h

@@ -21,7 +21,7 @@ namespace wasm {
 // types.
 // A recursive group is a subsequence of types explicitly marked in the type
 // section of a wasm module. Identical recursive groups have to be canonicalized
-// to a single canonical group and are are considered identical. Respective
+// to a single canonical group and are considered identical. Respective
 // types in two identical groups are considered identical for all purposes.
 // Two groups are considered identical if they have the same shape, and all
 // type indices referenced in the same position in both groups reference:

src/wasm/module-decoder-impl.h

@@ -811,7 +811,7 @@ class ModuleDecoderTemplate : public Decoder {
           table->imported = true;
           const byte* type_position = pc();
           ValueType type = consume_value_type();
-          if (!WasmTable::IsValidTableType(type, module_.get())) {
+          if (!type.is_object_reference()) {
             errorf(type_position, "Invalid table type %s", type.name().c_str());
             break;
           }
@@ -920,10 +920,8 @@ class ModuleDecoderTemplate : public Decoder {
       }
 
       ValueType table_type = consume_value_type();
-      if (!WasmTable::IsValidTableType(table_type, module_.get())) {
-        error(type_position,
-              "Currently, only externref and function references are allowed "
-              "as table types");
+      if (!table_type.is_object_reference()) {
+        error(type_position, "Only reference types can be used as table types");
         continue;
       }
       if (!has_initializer && !table_type.is_defaultable()) {

src/wasm/wasm-js.cc

@@ -1223,7 +1223,7 @@ void WebAssemblyTable(const v8::FunctionCallbackInfo<v8::Value>& args) {
 
   if (initial > 0 && args.Length() >= 2 && !args[1]->IsUndefined()) {
     i::Handle<i::Object> element = Utils::OpenHandle(*args[1]);
-    if (!i::WasmTableObject::IsValidElement(i_isolate, table_obj, element)) {
+    if (!i::WasmTableObject::IsValidJSElement(i_isolate, table_obj, element)) {
       thrower.TypeError(
           "Argument 2 must be undefined, null, or a value of type compatible "
           "with the type of the new table.");
@@ -2235,7 +2235,8 @@ void WebAssemblyTableGrow(const v8::FunctionCallbackInfo<v8::Value>& args) {
 
   if (args.Length() >= 2 && !args[1]->IsUndefined()) {
     init_value = Utils::OpenHandle(*args[1]);
-    if (!i::WasmTableObject::IsValidElement(i_isolate, receiver, init_value)) {
+    if (!i::WasmTableObject::IsValidJSElement(i_isolate, receiver,
+                                              init_value)) {
       thrower.TypeError("Argument 1 must be a valid type for the table");
       return;
     }
@@ -2320,7 +2321,7 @@ void WebAssemblyTableSet(const v8::FunctionCallbackInfo<v8::Value>& args) {
           ? Utils::OpenHandle(*args[1])
           : DefaultReferenceValue(i_isolate, table_object->type());
 
-  if (!i::WasmTableObject::IsValidElement(i_isolate, table_object, element)) {
+  if (!i::WasmTableObject::IsValidJSElement(i_isolate, table_object, element)) {
     thrower.TypeError("Argument 1 is invalid for table of type %s",
                       table_object->type().name().c_str());
     return;

src/wasm/wasm-module.h

@@ -610,23 +610,6 @@ struct V8_EXPORT_PRIVATE WasmModule {
 struct WasmTable {
   MOVE_ONLY_WITH_DEFAULT_CONSTRUCTORS(WasmTable);
 
-  // 'module' can be nullptr
-  // TODO(9495): Update this function as more table types are supported, or
-  // remove it completely when all reference types are allowed.
-  static bool IsValidTableType(ValueType type, const WasmModule* module) {
-    if (!type.is_object_reference()) return false;
-    HeapType heap_type = type.heap_type();
-    return heap_type == HeapType::kFunc || heap_type == HeapType::kExtern ||
-           heap_type == HeapType::kAny || heap_type == HeapType::kData ||
-           heap_type == HeapType::kArray || heap_type == HeapType::kEq ||
-           heap_type == HeapType::kI31 || heap_type == HeapType::kString ||
-           heap_type == HeapType::kStringViewWtf8 ||
-           heap_type == HeapType::kStringViewWtf16 ||
-           heap_type == HeapType::kStringViewIter ||
-           (module != nullptr && heap_type.is_index() &&
-            module->has_signature(heap_type.ref_index()));
-  }
-
   ValueType type = kWasmVoid;     // table type.
   uint32_t initial_size = 0;      // initial table size.
   uint32_t maximum_size = 0;      // maximum table size.

src/wasm/wasm-objects.cc

@@ -149,12 +149,7 @@ Handle<WasmTableObject> WasmTableObject::New(
     Isolate* isolate, Handle<WasmInstanceObject> instance, wasm::ValueType type,
     uint32_t initial, bool has_maximum, uint32_t maximum,
     Handle<FixedArray>* entries, Handle<Object> initial_value) {
-  // TODO(7748): Make this work with other types when spec clears up.
-  {
-    const WasmModule* module =
-        instance.is_null() ? nullptr : instance->module();
-    CHECK(wasm::WasmTable::IsValidTableType(type, module));
-  }
+  CHECK(type.is_object_reference());
 
   Handle<FixedArray> backing_store = isolate->factory()->NewFixedArray(initial);
   for (int i = 0; i < static_cast<int>(initial); ++i) {
@@ -292,12 +287,17 @@ int WasmTableObject::Grow(Isolate* isolate, Handle<WasmTableObject> table,
         UNREACHABLE();
       default:
         DCHECK(!table->instance().IsUndefined());
-        // TODO(7748): Relax this once we have struct/array/i31ref tables.
-        DCHECK(WasmInstanceObject::cast(table->instance())
-                   .module()
-                   ->has_signature(table->type().ref_index()));
-        init_value = i::WasmInternalFunction::FromExternal(init_value, isolate)
-                         .ToHandleChecked();
+        const bool kIsFunc = WasmInstanceObject::cast(table->instance())
+                                 .module()
+                                 ->has_signature(table->type().ref_index());
+        if (kIsFunc) {
+          init_value =
+              i::WasmInternalFunction::FromExternal(init_value, isolate)
+                  .ToHandleChecked();
+        } else if (!i::FLAG_wasm_gc_js_interop &&
+                   entry_repr == ValueRepr::kJS) {
+          i::wasm::TryUnpackObjectWrapper(isolate, init_value);
+        }
     }
   }
 
@@ -313,18 +313,18 @@ bool WasmTableObject::IsInBounds(Isolate* isolate,
   return entry_index < static_cast<uint32_t>(table->current_length());
 }
 
-bool WasmTableObject::IsValidElement(Isolate* isolate,
-                                     Handle<WasmTableObject> table,
-                                     Handle<Object> entry) {
+bool WasmTableObject::IsValidJSElement(Isolate* isolate,
+                                       Handle<WasmTableObject> table,
+                                       Handle<Object> entry) {
+  // Any `entry` has to be in its JS representation.
+  DCHECK(!entry->IsWasmInternalFunction());
+  DCHECK_IMPLIES(!v8_flags.wasm_gc_js_interop,
+                 !entry->IsWasmArray() && !entry->IsWasmStruct());
   const char* error_message;
   const WasmModule* module =
       !table->instance().IsUndefined()
           ? WasmInstanceObject::cast(table->instance()).module()
           : nullptr;
-  if (entry->IsWasmInternalFunction()) {
-    entry =
-        handle(Handle<WasmInternalFunction>::cast(entry)->external(), isolate);
-  }
   return wasm::TypecheckJSObject(isolate, module, entry, table->type(),
                                  &error_message);
 }
@@ -369,7 +369,8 @@ void WasmTableObject::Set(Isolate* isolate, Handle<WasmTableObject> table,
                           ValueRepr entry_repr) {
   // Callers need to perform bounds checks, type check, and error handling.
   DCHECK(IsInBounds(isolate, table, index));
-  DCHECK(IsValidElement(isolate, table, entry));
+  DCHECK_IMPLIES(entry_repr == WasmTableObject::kJS,
+                 IsValidJSElement(isolate, table, entry));
 
   Handle<FixedArray> entries(table->entries(), isolate);
   // The FixedArray is addressed with int's.
@@ -401,12 +402,18 @@ void WasmTableObject::Set(Isolate* isolate, Handle<WasmTableObject> table,
       UNREACHABLE();
     default:
       DCHECK(!table->instance().IsUndefined());
-      // TODO(7748): Relax this once we have struct/array/i31ref tables.
-      DCHECK(WasmInstanceObject::cast(table->instance())
-                 .module()
-                 ->has_signature(table->type().ref_index()));
-      SetFunctionTableEntry(isolate, table, entries, entry_index, entry,
-                            entry_repr);
+      if (WasmInstanceObject::cast(table->instance())
+              .module()
+              ->has_signature(table->type().ref_index())) {
+        SetFunctionTableEntry(isolate, table, entries, entry_index, entry,
+                              entry_repr);
+        return;
+      }
+      // Indexed struct and array types.
+      if (!i::FLAG_wasm_gc_js_interop && entry_repr == ValueRepr::kJS) {
+        i::wasm::TryUnpackObjectWrapper(isolate, entry);
+      }
+      entries->set(entry_index, *entry);
       return;
   }
 }
@@ -465,10 +472,23 @@ Handle<Object> WasmTableObject::Get(Isolate* isolate,
       UNREACHABLE();
     default:
       DCHECK(!table->instance().IsUndefined());
-      // TODO(7748): Relax this once we have struct/array/i31ref tables.
-      DCHECK(WasmInstanceObject::cast(table->instance())
-                 .module()
-                 ->has_signature(table->type().ref_index()));
+      const WasmModule* module =
+          WasmInstanceObject::cast(table->instance()).module();
+      if (module->has_array(table->type().ref_index()) ||
+          module->has_struct(table->type().ref_index())) {
+        if (as_repr == ValueRepr::kJS && !FLAG_wasm_gc_js_interop &&
+            !entry->IsNull()) {
+          // Transform wasm object into JS-compliant representation.
+          Handle<JSObject> wrapper =
+              isolate->factory()->NewJSObject(isolate->object_function());
+          JSObject::AddProperty(
+              isolate, wrapper,
+              isolate->factory()->wasm_wrapped_object_symbol(), entry, NONE);
+          return wrapper;
+        }
+        return entry;
+      }
+      DCHECK(module->has_signature(table->type().ref_index()));
       if (entry->IsWasmInternalFunction()) {
         return as_repr == ValueRepr::kJS
                    ? handle(
@@ -2370,11 +2390,11 @@ bool TypecheckJSObject(Isolate* isolate, const WasmModule* module,
         case HeapType::kI31: {
           // TODO(7748): Change this when we have a decision on the JS API for
           // structs/arrays.
+          // TODO(7748): Reiterate isSmi() check for i31refs once spec work is
+          // done: Probably all JS number objects shall be allowed if
+          // representable as a 31 bit SMI.
           if (!v8_flags.wasm_gc_js_interop) {
-            // The value can be a struct / array as this function is also used
-            // for checking objects not coming from JS (like data segments).
-            if (!value->IsSmi() && !value->IsWasmStruct() &&
-                !value->IsWasmArray() && !value->IsString() &&
+            if (!value->IsSmi() && !value->IsString() &&
                 !TryUnpackObjectWrapper(isolate, value)) {
               *error_message =
                   "eqref/dataref/i31ref object must be null (if nullable) or "
@@ -2472,13 +2492,35 @@ bool TypecheckJSObject(Isolate* isolate, const WasmModule* module,
                 "function object";
 
             return false;
+          } else {
+            // A struct or array type with index is expected.
+            DCHECK(module->has_struct(expected.ref_index()) ||
+                   module->has_array(expected.ref_index()));
+            if (value->IsNull()) {
+              if (expected.is_non_nullable()) {
+                *error_message =
+                    "invalid null value for non-nullable element type";
+                return false;
+              }
+              return true;
+            }
+            if (v8_flags.wasm_gc_js_interop
+                    ? !value->IsWasmStruct() && !value->IsWasmArray()
+                    : !TryUnpackObjectWrapper(isolate, value)) {
+              *error_message = "object incompatible with wasm type";
+              return false;
+            }
+            auto wasm_obj = Handle<WasmObject>::cast(value);
+            WasmTypeInfo type_info = wasm_obj->map().wasm_type_info();
+            uint32_t actual_idx = type_info.type_index();
+            const WasmModule* actual_module = type_info.instance().module();
+            if (!IsHeapSubtypeOf(HeapType(actual_idx), expected.heap_type(),
+                                 actual_module, module)) {
+              *error_message = "object is not a subtype of element type";
+              return false;
+            }
+            return true;
           }
-          // TODO(7748): Implement when the JS API for structs/arrays is decided
-          // on.
-          *error_message =
-              "passing struct/array-typed objects between Webassembly and "
-              "Javascript is not supported yet.";
-          return false;
       }
     }
     case kRtt:

src/wasm/wasm-objects.h

@@ -196,8 +196,8 @@ class WasmTableObject
   static bool IsInBounds(Isolate* isolate, Handle<WasmTableObject> table,
                          uint32_t entry_index);
 
-  static bool IsValidElement(Isolate* isolate, Handle<WasmTableObject> table,
-                             Handle<Object> entry);
+  static bool IsValidJSElement(Isolate* isolate, Handle<WasmTableObject> table,
+                               Handle<Object> entry);
 
   V8_EXPORT_PRIVATE static void Set(Isolate* isolate,
                                     Handle<WasmTableObject> table,

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

@@ -112,7 +112,7 @@ class TestModuleBuilder {
 
   byte AddTable(ValueType type, uint32_t initial_size, bool has_maximum_size,
                 uint32_t maximum_size) {
-    CHECK(WasmTable::IsValidTableType(type, &mod));
+    CHECK(type.is_object_reference());
     mod.tables.emplace_back();
     WasmTable& table = mod.tables.back();
     table.type = type;

test/unittests/wasm/module-decoder-unittest.cc

@@ -2084,7 +2084,7 @@ TEST_F(WasmModuleVerifyTest, IllegalTableTypes) {
 
   using Vec = std::vector<byte>;
 
-  static Vec table_types[] = {{kRefNullCode, 0}, {kRefNullCode, 1}};
+  static Vec table_types[] = {{kI32Code}, {kF64Code}};
 
   for (Vec type : table_types) {
     Vec data = {
@@ -2100,10 +2100,7 @@ TEST_F(WasmModuleVerifyTest, IllegalTableTypes) {
     data.insert(data.end(), {byte{0}, byte{10}});
 
     auto result = DecodeModule(data.data(), data.data() + data.size());
-
-    EXPECT_NOT_OK(result,
-                  "Currently, only externref and function references are "
-                  "allowed as table types");
+    EXPECT_NOT_OK(result, "Only reference types can be used as table types");
   }
 }