[wasm] Keep external function reference for externref tables/globals

See crrev.com/c/3277878 for context.

We should only transform extenral to internal function references when
passing a function value to a function-typed global or table. For their
externref counterparts, we should preserve the reference unchanged.

Bug: v8:11510, chromium:1273705
Change-Id: Ic1719c4d31e175f3a37ced6e4e4dfcd61a19ae57
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3302790
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78108}

src/wasm/c-api.cc

@@ -2010,11 +2010,12 @@ auto Table::set(size_t index, const Ref* ref) -> bool {
   i::Isolate* isolate = table->GetIsolate();
   i::HandleScope handle_scope(isolate);
   i::Handle<i::Object> obj = WasmRefToV8(isolate, ref);
-  i::Handle<i::Object> entry;
-  if (!i::WasmInternalFunction::FromExternal(obj, isolate).ToHandle(&entry)) {
-    entry = obj;
+  // TODO(7748): Generalize the condition if other table types are allowed.
+  if ((table->type() == i::wasm::kWasmFuncRef || table->type().has_index()) &&
+      !obj->IsNull()) {
+    obj = i::WasmInternalFunction::FromExternal(obj, isolate).ToHandleChecked();
   }
-  i::WasmTableObject::Set(isolate, table, static_cast<uint32_t>(index), entry);
+  i::WasmTableObject::Set(isolate, table, static_cast<uint32_t>(index), obj);
   return true;
 }
 
@@ -2028,13 +2029,13 @@ auto Table::grow(size_t delta, const Ref* ref) -> bool {
   i::Isolate* isolate = table->GetIsolate();
   i::HandleScope scope(isolate);
   i::Handle<i::Object> obj = WasmRefToV8(isolate, ref);
-  i::Handle<i::Object> init_value;
-  if (!i::WasmInternalFunction::FromExternal(obj, isolate)
-           .ToHandle(&init_value)) {
-    init_value = obj;
+  // TODO(7748): Generalize the condition if other table types are allowed.
+  if ((table->type() == i::wasm::kWasmFuncRef || table->type().has_index()) &&
+      !obj->IsNull()) {
+    obj = i::WasmInternalFunction::FromExternal(obj, isolate).ToHandleChecked();
   }
-  int result = i::WasmTableObject::Grow(
-      isolate, table, static_cast<uint32_t>(delta), init_value);
+  int result = i::WasmTableObject::Grow(isolate, table,
+                                        static_cast<uint32_t>(delta), obj);
   return result >= 0;
 }
 

src/wasm/module-instantiate.cc

@@ -1474,12 +1474,11 @@ bool InstanceBuilder::ProcessImportedGlobal(Handle<WasmInstanceObject> instance,
       ReportLinkError(error_message, global_index, module_name, import_name);
       return false;
     }
-    auto stored_value =
-        WasmExternalFunction::IsWasmExternalFunction(*value)
-            ? WasmInternalFunction::FromExternal(value, isolate_)
-                  .ToHandleChecked()
-            : value;
-    WriteGlobalValue(global, WasmValue(stored_value, global.type));
+    if (IsSubtypeOf(global.type, kWasmFuncRef, module_) && !value->IsNull()) {
+      value =
+          WasmInternalFunction::FromExternal(value, isolate_).ToHandleChecked();
+    }
+    WriteGlobalValue(global, WasmValue(value, global.type));
     return true;
   }
 

src/wasm/wasm-js.cc

@@ -1206,7 +1206,8 @@ void WebAssemblyTable(const v8::FunctionCallbackInfo<v8::Value>& args) {
           "with the type of the new table.");
       return;
     }
-    if (element->IsJSFunction()) {
+    // TODO(7748): Generalize this if other table types are allowed.
+    if (type == i::wasm::kWasmFuncRef && !element->IsNull()) {
       element = i::WasmInternalFunction::FromExternal(element, i_isolate)
                     .ToHandleChecked();
     }
@@ -1980,14 +1981,16 @@ void WebAssemblyTableGrow(const v8::FunctionCallbackInfo<v8::Value>& args) {
     init_value = DefaultReferenceValue(i_isolate, receiver->type());
   }
 
-  i::Handle<i::Object> internal_init_value;
-  if (!i::WasmInternalFunction::FromExternal(init_value, i_isolate)
-           .ToHandle(&internal_init_value)) {
-    internal_init_value = init_value;
+  // TODO(7748): Generalize this if other table types are allowed.
+  bool has_function_type =
+      receiver->type() == i::wasm::kWasmFuncRef || receiver->type().has_index();
+  if (has_function_type && !init_value->IsNull()) {
+    init_value = i::WasmInternalFunction::FromExternal(init_value, i_isolate)
+                     .ToHandleChecked();
   }
 
-  int old_size = i::WasmTableObject::Grow(i_isolate, receiver, grow_by,
-                                          internal_init_value);
+  int old_size =
+      i::WasmTableObject::Grow(i_isolate, receiver, grow_by, init_value);
 
   if (old_size < 0) {
     thrower.RangeError("failed to grow table by %u", grow_by);
@@ -2059,13 +2062,15 @@ void WebAssemblyTableSet(const v8::FunctionCallbackInfo<v8::Value>& args) {
     return;
   }
 
-  i::Handle<i::Object> value;
-  if (!i::WasmInternalFunction::FromExternal(element, i_isolate)
-           .ToHandle(&value)) {
-    value = element;
+  // TODO(7748): Generalize this if other table types are allowed.
+  bool has_function_type = table_object->type() == i::wasm::kWasmFuncRef ||
+                           table_object->type().has_index();
+  if (has_function_type && !element->IsNull()) {
+    element = i::WasmInternalFunction::FromExternal(element, i_isolate)
+                  .ToHandleChecked();
   }
 
-  i::WasmTableObject::Set(i_isolate, table_object, index, value);
+  i::WasmTableObject::Set(i_isolate, table_object, index, element);
 }
 
 // WebAssembly.Table.type() -> TableType

test/mjsunit/regress/wasm/regress-1273705.js

+// Copyright 2021 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+const initial = function () {};
+const descriptor = {"element": "externref", "initial": 3};
+new WebAssembly.Table(descriptor, initial);

test/mjsunit/wasm/externref-table.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --experimental-wasm-reftypes
+// Flags: --experimental-wasm-typed-funcref --experimental-wasm-type-reflection
 
 d8.file.execute("test/mjsunit/wasm/wasm-module-builder.js");
 
@@ -136,3 +136,69 @@ function getDummy(val) {
   table.set(1);
   assertEquals(table.get(1), undefined);
 })();
+
+(function TestFunctionExternRefTableRoundtrip() {
+  // Test that
+  // - initialization, setting, and growing an externref table, and
+  // - (imported) externref globals
+  // preserve function references.
+  print(arguments.callee.name);
+
+  const js_function = function (i) { return i + 1; };
+  const wasm_js_function = new WebAssembly.Function(
+    {parameters:['i32', 'i32'], results: ['i32']},
+    function(a, b) { return a * b; })
+
+  let extern_type = wasmRefType(kWasmExternRef);
+
+  let builder = new WasmModuleBuilder();
+  let imported_global = builder.addImportedGlobal('m', 'n', extern_type, false);
+  let global = builder.addGlobal(kWasmExternRef, true).exportAs('global');
+  let table = builder.addTable(extern_type, 2, 10,
+                               WasmInitExpr.GlobalGet(imported_global))
+  builder.addFunction(
+      'setup', makeSig([extern_type, extern_type], []))
+    .addBody([
+      kExprLocalGet, 0, kExprGlobalSet, global.index,
+      kExprI32Const, 1, kExprLocalGet, 0, kExprTableSet, table.index,
+      kExprLocalGet, 1, kExprI32Const, 1, kNumericPrefix,
+      kExprTableGrow, table.index, kExprDrop])
+    .exportFunc();
+  builder.addFunction('get', makeSig([kWasmI32], [kWasmExternRef]))
+    .addBody([kExprLocalGet, 0, kExprTableGet, table.index])
+    .exportFunc();
+  let instance = builder.instantiate({m : {n : js_function}});
+
+  instance.exports.setup(wasm_js_function, instance.exports.setup);
+
+  assertEquals(instance.exports.global.value, wasm_js_function);
+  assertEquals(instance.exports.get(0), js_function);
+  assertEquals(instance.exports.get(1), wasm_js_function);
+  assertEquals(instance.exports.get(2), instance.exports.setup);
+})();
+
+(function TestFunctionExternRefTableRoundtrip2() {
+  // Test that initialization, setting, and growing an externref table in the JS
+  // API preserves function references.
+  print(arguments.callee.name);
+
+  let builder = new WasmModuleBuilder();
+  builder.addFunction('dummy', kSig_i_v)
+      .addBody([kExprI32Const, 0])
+      .exportAs('dummy');
+  let instance = builder.instantiate();
+  const js_function = function (i) { return i + 1; };
+  const wasm_js_function = new WebAssembly.Function(
+    {parameters:['i32', 'i32'], results: ['i32']},
+    function(a, b) { return a * b; })
+
+  const argument = { "element": "externref", "initial": 3 };
+  const table = new WebAssembly.Table(argument, js_function);
+  table.set(1, wasm_js_function);
+  table.set(2, instance.exports.dummy);
+  table.grow(1, wasm_js_function);
+  assertEquals(table.get(0), js_function);
+  assertEquals(table.get(1), wasm_js_function);
+  assertEquals(table.get(2), instance.exports.dummy);
+  assertEquals(table.get(3), wasm_js_function);
+})();