[wasm][bug] Compare signatures correctly in ResolveWasmImportCall

Changes:
- Implement WasmExportedFunction::MatchesSignature.
- Use it over comparison with == in ResolveWasmImportCall.
- Add a test which exposes the existing bug.
- Add a few reminder TODOs.

Bug: v8:9495
Change-Id: Ibbe31dbf550be212dbf2170ab8cdab9b4b6de734
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2438060
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70215}

src/compiler/wasm-compiler.cc

@@ -7116,17 +7116,17 @@ std::pair<WasmImportCallKind, Handle<JSReceiver>> ResolveWasmImportCall(
     const wasm::WasmFeatures& enabled_features) {
   if (WasmExportedFunction::IsWasmExportedFunction(*callable)) {
     auto imported_function = Handle<WasmExportedFunction>::cast(callable);
-    auto func_index = imported_function->function_index();
-    auto module = imported_function->instance().module();
-    const wasm::FunctionSig* imported_sig = module->functions[func_index].sig;
-    if (*imported_sig != *expected_sig) {
+    if (!imported_function->MatchesSignature(module, expected_sig)) {
       return std::make_pair(WasmImportCallKind::kLinkError, callable);
     }
-    if (static_cast<uint32_t>(func_index) >= module->num_imported_functions) {
+    uint32_t func_index =
+        static_cast<uint32_t>(imported_function->function_index());
+    if (func_index >=
+        imported_function->instance().module()->num_imported_functions) {
       return std::make_pair(WasmImportCallKind::kWasmToWasm, callable);
     }
     Isolate* isolate = callable->GetIsolate();
-    // Resolve the short-cut to the underlying callable and continue.
+    // Resolve the shortcut to the underlying callable and continue.
     Handle<WasmInstanceObject> instance(imported_function->instance(), isolate);
     ImportedFunctionEntry entry(instance, func_index);
     callable = handle(entry.callable(), isolate);

src/wasm/wasm-objects.cc

@@ -1674,6 +1674,7 @@ Handle<WasmExceptionObject> WasmExceptionObject::New(
   return exception;
 }
 
+// TODO(9495): Update this if function type variance is introduced.
 bool WasmExceptionObject::MatchesSignature(const wasm::FunctionSig* sig) {
   DCHECK_EQ(0, sig->return_count());
   DCHECK_LE(sig->parameter_count(), std::numeric_limits<int>::max());
@@ -1687,6 +1688,7 @@ bool WasmExceptionObject::MatchesSignature(const wasm::FunctionSig* sig) {
   return true;
 }
 
+// TODO(9495): Update this if function type variance is introduced.
 bool WasmCapiFunction::MatchesSignature(const wasm::FunctionSig* sig) const {
   // TODO(jkummerow): Unify with "SignatureHelper" in c-api.cc.
   int param_count = static_cast<int>(sig->parameter_count());
@@ -1948,6 +1950,23 @@ const wasm::FunctionSig* WasmExportedFunction::sig() {
   return instance().module()->functions[function_index()].sig;
 }
 
+bool WasmExportedFunction::MatchesSignature(
+    const WasmModule* other_module, const wasm::FunctionSig* other_sig) {
+  const wasm::FunctionSig* sig = this->sig();
+  if (sig->parameter_count() != other_sig->parameter_count() ||
+      sig->return_count() != other_sig->return_count()) {
+    return false;
+  }
+
+  for (int i = 0; i < sig->all().size(); i++) {
+    if (!wasm::EquivalentTypes(sig->all()[i], other_sig->all()[i],
+                               this->instance().module(), other_module)) {
+      return false;
+    }
+  }
+  return true;
+}
+
 // static
 bool WasmJSFunction::IsWasmJSFunction(Object object) {
   if (!object.IsJSFunction()) return false;
@@ -2012,6 +2031,7 @@ const wasm::FunctionSig* WasmJSFunction::GetSignature(Zone* zone) {
   return zone->New<wasm::FunctionSig>(return_count, parameter_count, types);
 }
 
+// TODO(9495): Update this if function type variance is introduced.
 bool WasmJSFunction::MatchesSignature(const wasm::FunctionSig* sig) {
   DCHECK_LE(sig->all().size(), kMaxInt);
   int sig_size = static_cast<int>(sig->all().size());

src/wasm/wasm-objects.h

@@ -666,6 +666,9 @@ class WasmExportedFunction : public JSFunction {
 
   V8_EXPORT_PRIVATE const wasm::FunctionSig* sig();
 
+  bool MatchesSignature(const wasm::WasmModule* other_module,
+                        const wasm::FunctionSig* other_sig);
+
   DECL_CAST(WasmExportedFunction)
   OBJECT_CONSTRUCTORS(WasmExportedFunction, JSFunction);
 };

test/mjsunit/wasm/imported-function-types.js

@@ -0,0 +1,44 @@
+// Copyright 2020 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.
+
+// Flags: --experimental-wasm-typed-funcref
+
+load("test/mjsunit/wasm/wasm-module-builder.js");
+
+var exporting_module = (function() {
+  var builder = new WasmModuleBuilder();
+
+  var binaryType = builder.addType(kSig_i_ii);
+  var unaryType = builder.addType(kSig_i_i);
+
+  builder.addFunction("func1", makeSig([wasmRefType(binaryType)], [kWasmI32])).
+    addBody([kExprI32Const, 42, kExprI32Const, 12, kExprLocalGet, 0,
+             kExprCallRef]).
+    exportFunc();
+
+  builder.addFunction("func2", makeSig([wasmRefType(unaryType)], [kWasmI32])).
+    addBody([kExprI32Const, 42, kExprLocalGet, 0, kExprCallRef]).
+    exportFunc();
+
+  return builder.instantiate({});
+})();
+
+var importing_module = function(imported_function) {
+  var builder = new WasmModuleBuilder();
+
+  var unaryType = builder.addType(kSig_i_i);
+
+  builder.addImport("other", "func",
+    makeSig([wasmRefType(unaryType)], [kWasmI32]));
+
+  return builder.instantiate({other: {func: imported_function}});
+};
+
+// Same form/different index should be fine.
+importing_module(exporting_module.exports.func2);
+// Same index/different form should throw.
+assertThrows(
+    () => importing_module(exporting_module.exports.func1),
+    WebAssembly.LinkError,
+    /imported function does not match the expected type/);