Add signature checking when directly import a foreign function

Review-Url: https://codereview.chromium.org/2204703002
Cr-Commit-Position: refs/heads/master@{#38349}

src/runtime/runtime-test.cc

@@ -282,6 +282,70 @@ RUNTIME_FUNCTION(Runtime_ClearFunctionTypeFeedback) {
   return isolate->heap()->undefined_value();
 }
 
+RUNTIME_FUNCTION(Runtime_CheckWasmWrapperElision) {
+  // This only supports the case where the function being exported
+  // calls an intermediate function, and the intermediate function
+  // calls exactly one imported function
+  HandleScope scope(isolate);
+  CHECK(args.length() == 2);
+  // It takes two parameters, the first one is the JSFunction,
+  // The second one is the type
+  CONVERT_ARG_HANDLE_CHECKED(JSFunction, function, 0);
+  // If type is 0, it means that it is supposed to be a direct call into a WASM
+  // function
+  // If type is 1, it means that it is supposed to have wrappers
+  CONVERT_ARG_HANDLE_CHECKED(Smi, type, 1);
+  Handle<Code> export_code = handle(function->code());
+  CHECK(export_code->kind() == Code::JS_TO_WASM_FUNCTION);
+  int const mask = RelocInfo::ModeMask(RelocInfo::CODE_TARGET);
+  // check the type of the $export_fct
+  Handle<Code> export_fct;
+  int count = 0;
+  for (RelocIterator it(*export_code, mask); !it.done(); it.next()) {
+    RelocInfo* rinfo = it.rinfo();
+    Address target_address = rinfo->target_address();
+    Code* target = Code::GetCodeFromTargetAddress(target_address);
+    if (target->kind() == Code::WASM_FUNCTION) {
+      ++count;
+      export_fct = handle(target);
+    }
+  }
+  CHECK(count == 1);
+  // check the type of the intermediate_fct
+  Handle<Code> intermediate_fct;
+  count = 0;
+  for (RelocIterator it(*export_fct, mask); !it.done(); it.next()) {
+    RelocInfo* rinfo = it.rinfo();
+    Address target_address = rinfo->target_address();
+    Code* target = Code::GetCodeFromTargetAddress(target_address);
+    if (target->kind() == Code::WASM_FUNCTION) {
+      ++count;
+      intermediate_fct = handle(target);
+    }
+  }
+  CHECK(count == 1);
+  // check the type of the imported exported function, it should be also a WASM
+  // function in our case
+  Handle<Code> imported_fct;
+  Code::Kind target_kind;
+  if (type->value() == 0) {
+    target_kind = Code::WASM_FUNCTION;
+  } else if (type->value() == 1) {
+    target_kind = Code::WASM_TO_JS_FUNCTION;
+  }
+  count = 0;
+  for (RelocIterator it(*intermediate_fct, mask); !it.done(); it.next()) {
+    RelocInfo* rinfo = it.rinfo();
+    Address target_address = rinfo->target_address();
+    Code* target = Code::GetCodeFromTargetAddress(target_address);
+    if (target->kind() == target_kind) {
+      ++count;
+      imported_fct = handle(target);
+    }
+  }
+  CHECK_LE(count, 1);
+  return isolate->heap()->ToBoolean(count == 1);
+}
 
 RUNTIME_FUNCTION(Runtime_NotifyContextDisposed) {
   HandleScope scope(isolate);

src/runtime/runtime.h

@@ -845,6 +845,7 @@ namespace internal {
   F(GetOptimizationCount, 1, 1)               \
   F(GetUndetectable, 0, 1)                    \
   F(ClearFunctionTypeFeedback, 1, 1)          \
+  F(CheckWasmWrapperElision, 2, 1)            \
   F(NotifyContextDisposed, 0, 1)              \
   F(SetAllocationTimeout, -1 /* 2 || 3 */, 1) \
   F(DebugPrint, 1, 1)                         \

src/wasm/wasm-js.cc

@@ -546,9 +546,12 @@ void WasmJs::InstallWasmFunctionMap(Isolate* isolate, Handle<Context> context) {
     CHECK_EQ(0, internal_fields);
     int pre_allocated =
         prev_map->GetInObjectProperties() - prev_map->unused_property_fields();
-    int instance_size;
-    int in_object_properties;
-    JSFunction::CalculateInstanceSizeHelper(instance_type, internal_fields + 1,
+    int instance_size = 0;
+    int in_object_properties = 0;
+    int wasm_internal_fields = internal_fields + 1  // module instance object
+                               + 1                  // function arity
+                               + 1;                 // function signature
+    JSFunction::CalculateInstanceSizeHelper(instance_type, wasm_internal_fields,
                                             0, &instance_size,
                                             &in_object_properties);
 

src/wasm/wasm-module.cc

@@ -25,6 +25,12 @@ namespace v8 {
 namespace internal {
 namespace wasm {
 
+enum JSFunctionExportInternalField {
+  kInternalModuleInstance,
+  kInternalArity,
+  kInternalSignature
+};
+
 static const int kPlaceholderMarker = 1000000000;
 
 static const char* wasmSections[] = {
@@ -116,7 +122,7 @@ std::ostream& operator<<(std::ostream& os, const WasmFunctionName& pair) {
 
 Handle<JSFunction> WrapExportCodeAsJSFunction(
     Isolate* isolate, Handle<Code> export_code, Handle<String> name, int arity,
-    Handle<JSObject> module_instance) {
+    MaybeHandle<ByteArray> maybe_signature, Handle<JSObject> module_instance) {
   Handle<SharedFunctionInfo> shared =
       isolate->factory()->NewSharedFunctionInfo(name, export_code, false);
   shared->set_length(arity);
@@ -125,7 +131,14 @@ Handle<JSFunction> WrapExportCodeAsJSFunction(
       isolate->wasm_function_map(), name, export_code);
   function->set_shared(*shared);
 
-  function->SetInternalField(0, *module_instance);
+  function->SetInternalField(kInternalModuleInstance, *module_instance);
+  // add another Internal Field as the function arity
+  function->SetInternalField(kInternalArity, Smi::FromInt(arity));
+  // add another Internal Field as the signature of the foreign function
+  Handle<ByteArray> signature;
+  if (maybe_signature.ToHandle(&signature)) {
+    function->SetInternalField(kInternalSignature, *signature);
+  }
   return function;
 }
 
@@ -183,6 +196,7 @@ enum WasmExportMetadata {
   kExportName,                  // String
   kExportArity,                 // Smi, an int
   kExportedFunctionIndex,       // Smi, an uint32_t
+  kExportedSignature,           // ByteArray. A copy of the data in FunctionSig
   kWasmExportMetadataTableSize  // Sentinel value.
 };
 
@@ -648,8 +662,37 @@ bool CompileWrappersToImportedFunctions(Isolate* isolate,
       MaybeHandle<JSFunction> function = LookupFunction(
           *thrower, isolate->factory(), ffi, index, module_name, function_name);
       if (function.is_null()) return false;
-
-      {
+      Handle<Code> code;
+      Handle<JSFunction> func = function.ToHandleChecked();
+      Handle<Code> export_wrapper_code = handle(func->code());
+      bool isMatch = false;
+      if (export_wrapper_code->kind() == Code::JS_TO_WASM_FUNCTION) {
+        int exported_param_count =
+            Smi::cast(func->GetInternalField(kInternalArity))->value();
+        Handle<ByteArray> exportedSig = Handle<ByteArray>(
+            ByteArray::cast(func->GetInternalField(kInternalSignature)));
+        if (exported_param_count == param_count &&
+            exportedSig->length() == sig_data->length() &&
+            memcmp(exportedSig->data(), sig_data->data(),
+                   exportedSig->length()) == 0) {
+          isMatch = true;
+        }
+      }
+      if (isMatch) {
+        int wasm_count = 0;
+        int const mask = RelocInfo::ModeMask(RelocInfo::CODE_TARGET);
+        for (RelocIterator it(*export_wrapper_code, mask); !it.done();
+             it.next()) {
+          RelocInfo* rinfo = it.rinfo();
+          Address target_address = rinfo->target_address();
+          Code* target = Code::GetCodeFromTargetAddress(target_address);
+          if (target->kind() == Code::WASM_FUNCTION) {
+            ++wasm_count;
+            code = handle(target);
+          }
+        }
+        DCHECK(wasm_count == 1);
+      } else {
         // Copy the signature to avoid a raw pointer into a heap object when
         // GC can happen.
         Zone zone(isolate->allocator());
@@ -659,12 +702,10 @@ bool CompileWrappersToImportedFunctions(Isolate* isolate,
                sizeof(MachineRepresentation) * sig_data_size);
         FunctionSig sig(ret_count, param_count, reps);
 
-        Handle<Code> code = compiler::CompileWasmToJSWrapper(
-            isolate, function.ToHandleChecked(), &sig, index, module_name,
-            function_name);
-
-        imports.push_back(code);
+        code = compiler::CompileWasmToJSWrapper(isolate, func, &sig, index,
+                                                module_name, function_name);
       }
+      imports.push_back(code);
     }
   }
   return true;
@@ -969,8 +1010,10 @@ bool SetupExportsObject(Handle<FixedArray> compiled_module, Isolate* isolate,
         Handle<String> name =
             export_metadata->GetValueChecked<String>(isolate, kExportName);
         int arity = Smi::cast(export_metadata->get(kExportArity))->value();
+        MaybeHandle<ByteArray> signature =
+            export_metadata->GetValue<ByteArray>(isolate, kExportedSignature);
         Handle<JSFunction> function = WrapExportCodeAsJSFunction(
-            isolate, export_code, name, arity, instance);
+            isolate, export_code, name, arity, signature, instance);
         desc.set_value(function);
         Maybe<bool> status = JSReceiver::DefineOwnProperty(
             isolate, exports_object, name, &desc, Object::THROW_ON_ERROR);
@@ -1081,6 +1124,15 @@ MaybeHandle<FixedArray> WasmModule::CompileFunctions(
       Handle<FixedArray> export_metadata =
           factory->NewFixedArray(kWasmExportMetadataTableSize, TENURED);
       const WasmExport& exp = export_table[i];
+      FunctionSig* funcSig = functions[exp.func_index].sig;
+      Handle<ByteArray> exportedSig =
+          factory->NewByteArray(static_cast<int>(funcSig->parameter_count() +
+                                                 funcSig->return_count()),
+                                TENURED);
+      exportedSig->copy_in(0,
+                           reinterpret_cast<const byte*>(funcSig->raw_data()),
+                           exportedSig->length());
+      export_metadata->set(kExportedSignature, *exportedSig);
       WasmName str = GetName(exp.name_offset, exp.name_length);
       Handle<String> name = factory->InternalizeUtf8String(str);
       Handle<Code> code =
@@ -1333,9 +1385,11 @@ MaybeHandle<JSObject> WasmModule::Instantiate(
     Handle<Code> startup_code =
         metadata->GetValueChecked<Code>(isolate, kExportCode);
     int arity = Smi::cast(metadata->get(kExportArity))->value();
+    MaybeHandle<ByteArray> startup_signature =
+        metadata->GetValue<ByteArray>(isolate, kExportedSignature);
     Handle<JSFunction> startup_fct = WrapExportCodeAsJSFunction(
         isolate, startup_code, factory->InternalizeUtf8String("start"), arity,
-        js_object);
+        startup_signature, js_object);
     RecordStats(isolate, *startup_code);
     // Call the JS function.
     Handle<Object> undefined = isolate->factory()->undefined_value();

src/wasm/wasm-module.h

@@ -371,6 +371,7 @@ int GetNumberOfFunctions(JSObject* wasm);
 Handle<JSFunction> WrapExportCodeAsJSFunction(Isolate* isolate,
                                               Handle<Code> export_code,
                                               Handle<String> name, int arity,
+                                              MaybeHandle<ByteArray> signature,
                                               Handle<JSObject> module_instance);
 
 // Check whether the given object is a wasm object.

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

@@ -211,10 +211,16 @@ class TestingModule : public ModuleEnv {
     WasmJs::InstallWasmFunctionMap(isolate_, isolate_->native_context());
     Handle<Code> ret_code =
         compiler::CompileJSToWasmWrapper(isolate_, this, code, index);
+    FunctionSig* funcSig = this->module->functions[index].sig;
+    Handle<ByteArray> exportedSig = isolate_->factory()->NewByteArray(
+        static_cast<int>(funcSig->parameter_count() + funcSig->return_count()),
+        TENURED);
+    exportedSig->copy_in(0, reinterpret_cast<const byte*>(funcSig->raw_data()),
+                         exportedSig->length());
     Handle<JSFunction> ret = WrapExportCodeAsJSFunction(
         isolate_, ret_code, name,
         static_cast<int>(this->module->functions[index].sig->parameter_count()),
-        module_object);
+        exportedSig, module_object);
     return ret;
   }
 

test/mjsunit/wasm/test-import-export-wrapper.js

+// Copyright 2016 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: --expose-wasm --allow-natives-syntax
+
+load("test/mjsunit/wasm/wasm-constants.js");
+load("test/mjsunit/wasm/wasm-module-builder.js");
+
+var expect_elison = 0;
+var expect_no_elison = 1;
+// function calls stack: first_export -> first_func -> first_import ->
+// second_export -> second_import
+// In this case, first_import and second_export have same signature,
+// So that wrappers will be removed
+(function TestWasmWrapperElision() {
+    var imported = function (a) {
+        return a;
+    };
+
+    var second_module = new WasmModuleBuilder();
+    var sig_index = second_module.addType(kSig_i_i);
+    second_module
+        .addImportWithModule("import_module_2", "import_name_2", sig_index);
+    second_module
+        .addFunction("second_export", sig_index)
+        .addBody([
+            kExprGetLocal, 0,
+            kExprCallImport, kArity1, 0,
+            kExprReturn, kArity1
+        ])
+        .exportFunc();
+
+    var first_module = new WasmModuleBuilder();
+    var sig_index = first_module.addType(kSig_i_i);
+    first_module
+        .addImportWithModule("import_module_1", "import_name_1", sig_index);
+    first_module
+        .addFunction("first_export", sig_index)
+        .addBody([
+            kExprGetLocal, 0,
+            kExprCallFunction, kArity1, 1,
+            kExprReturn, kArity1
+        ])
+        .exportFunc();
+    first_module
+        .addFunction("first_func", sig_index)
+        .addBody([
+            kExprI32Const, 1,
+            kExprGetLocal, 0,
+            kExprI32Add,
+            kExprCallImport, kArity1, 0,
+            kExprReturn, kArity1
+        ]);
+
+    var f = second_module
+        .instantiate({import_module_2: {import_name_2: imported}})
+        .exports.second_export;
+    var the_export = first_module
+        .instantiate({import_module_1: {import_name_1: f}})
+        .exports.first_export;
+    assertEquals(the_export(2), 3);
+    assertEquals(the_export(-1), 0);
+    assertEquals(the_export(0), 1);
+    assertEquals(the_export(5.5), 6);
+    assertEquals(%CheckWasmWrapperElision(the_export, expect_elison), true);
+})();
+
+// function calls stack: first_export -> first_func -> first_import ->
+// second_export -> second_import
+// In this case, second_export has less params than first_import,
+// So that wrappers will not be removed
+(function TestWasmWrapperNoElisionLessParams() {
+    var imported = function (a) {
+        return a;
+    };
+
+    var second_module = new WasmModuleBuilder();
+    var sig_index_1 = second_module.addType(kSig_i_i);
+    second_module
+        .addImportWithModule("import_module_2", "import_name_2", sig_index_1);
+    second_module
+        .addFunction("second_export", sig_index_1)
+        .addBody([
+            kExprGetLocal, 0,
+            kExprCallImport, kArity1, 0,
+            kExprReturn, kArity1
+        ])
+        .exportFunc();
+
+    var first_module = new WasmModuleBuilder();
+    var sig_index_2 = first_module.addType(kSig_i_ii);
+    first_module
+        .addImportWithModule("import_module_1", "import_name_1", sig_index_2);
+    first_module
+        .addFunction("first_export", sig_index_2)
+        .addBody([
+            kExprGetLocal, 0,
+            kExprGetLocal, 1,
+            kExprCallFunction, kArity2, 1,
+            kExprReturn, kArity1
+        ])
+        .exportFunc();
+    first_module
+        .addFunction("first_func", sig_index_2)
+        .addBody([
+            kExprGetLocal, 0,
+            kExprGetLocal, 1,
+            kExprCallImport, kArity2, 0,
+            kExprReturn, kArity1
+        ]);
+
+    var f = second_module
+        .instantiate({import_module_2: {import_name_2: imported}})
+        .exports.second_export;
+    var the_export = first_module
+        .instantiate({import_module_1: {import_name_1: f}})
+        .exports.first_export;
+    assertEquals(the_export(4, 5), 4);
+    assertEquals(the_export(-1, 4), -1);
+    assertEquals(the_export(0, 2), 0);
+    assertEquals(the_export(9.9, 4.3), 9);
+    assertEquals(%CheckWasmWrapperElision(the_export, expect_no_elison), true);
+})();
+
+// function calls stack: first_export -> first_func -> first_import ->
+// second_export -> second_import
+// In this case, second_export has more params than first_import,
+// So that wrappers will not be removed
+(function TestWasmWrapperNoElisionMoreParams() {
+    var imported = function (a, b, c) {
+        return a+b+c;
+    };
+
+    var second_module = new WasmModuleBuilder();
+    var sig_index_3 = second_module.addType(kSig_i_iii);
+    second_module
+        .addImportWithModule("import_module_2", "import_name_2", sig_index_3);
+    second_module
+        .addFunction("second_export", sig_index_3)
+        .addBody([
+            kExprGetLocal, 0,
+            kExprGetLocal, 1,
+            kExprGetLocal, 2,
+            kExprCallImport, kArity3, 0,
+            kExprReturn, kArity1
+        ])
+        .exportFunc();
+
+    var first_module = new WasmModuleBuilder();
+    var sig_index_2 = first_module.addType(kSig_i_ii);
+    first_module
+        .addImportWithModule("import_module_1", "import_name_1", sig_index_2);
+    first_module
+        .addFunction("first_export", sig_index_2)
+        .addBody([
+            kExprGetLocal, 0,
+            kExprGetLocal, 1,
+            kExprCallFunction, kArity2, 1,
+            kExprReturn, kArity1
+        ])
+        .exportFunc();
+    first_module
+        .addFunction("first_func", sig_index_2)
+        .addBody([
+            kExprGetLocal, 0,
+            kExprGetLocal, 1,
+            kExprCallImport, kArity2, 0,
+            kExprReturn, kArity1
+        ]);
+
+    var f = second_module
+        .instantiate({import_module_2: {import_name_2: imported}})
+        .exports.second_export;
+    var the_export = first_module
+        .instantiate({import_module_1: {import_name_1: f}})
+        .exports.first_export;
+    assertEquals(the_export(5, 6), 11);
+    assertEquals(the_export(-1, -4), -5);
+    assertEquals(the_export(0, 0), 0);
+    assertEquals(the_export(1.1, 2.7), 3);
+    assertEquals(%CheckWasmWrapperElision(the_export, expect_no_elison), true);
+})();
+
+// function calls stack: first_export -> first_func -> first_import ->
+// second_export -> second_import
+// In this case, second_export has different params type with first_import,
+// So that wrappers will not be removed
+(function TestWasmWrapperNoElisionTypeMismatch() {
+    var imported = function (a, b) {
+        return a+b;
+    };
+
+    var second_module = new WasmModuleBuilder();
+    var sig_index_2 = second_module.addType(kSig_d_dd);
+    second_module
+        .addImportWithModule("import_module_2", "import_name_2", sig_index_2);
+    second_module
+        .addFunction("second_export", sig_index_2)
+        .addBody([
+            kExprGetLocal, 0,
+            kExprGetLocal, 1,
+            kExprCallImport, kArity2, 0,
+            kExprReturn, kArity1
+        ])
+        .exportFunc();
+
+    var first_module = new WasmModuleBuilder();
+    var sig_index_2 = first_module.addType(kSig_i_ii);
+    first_module
+        .addImportWithModule("import_module_1", "import_name_1", sig_index_2);
+    first_module
+        .addFunction("first_export", sig_index_2)
+        .addBody([
+            kExprGetLocal, 0,
+            kExprGetLocal, 1,
+            kExprCallFunction, kArity2, 1,
+            kExprReturn, kArity1
+        ])
+        .exportFunc();
+    first_module
+        .addFunction("first_func", sig_index_2)
+        .addBody([
+            kExprGetLocal, 0,
+            kExprGetLocal, 1,
+            kExprCallImport, kArity2, 0,
+            kExprReturn, kArity1
+        ]);
+
+    var f = second_module
+        .instantiate({import_module_2: {import_name_2: imported}})
+        .exports.second_export;
+    var the_export = first_module
+        .instantiate({import_module_1: {import_name_1: f}})
+        .exports.first_export;
+    assertEquals(the_export(2.8, 9.1), 11);
+    assertEquals(the_export(-1.7, -2.5), -3);
+    assertEquals(the_export(0.0, 0.0), 0);
+    assertEquals(the_export(2, -2), 0);
+    assertEquals(%CheckWasmWrapperElision(the_export, expect_no_elison), true);
+})();

test/mjsunit/wasm/wasm-constants.js

@@ -100,6 +100,11 @@ var kSig_l_ll = makeSig([kAstI64, kAstI64], [kAstI64]);
 var kSig_i_dd = makeSig([kAstF64, kAstF64], [kAstI32]);
 var kSig_v_v = makeSig([], []);
 var kSig_i_v = makeSig([], [kAstI32]);
+var kSig_v_i = makeSig([kAstI32], []);
+var kSig_v_ii = makeSig([kAstI32, kAstI32], []);
+var kSig_v_iii = makeSig([kAstI32, kAstI32, kAstI32], []);
+var kSig_v_d = makeSig([kAstF64], []);
+var kSig_v_dd = makeSig([kAstF64, kAstF64], []);
 
 function makeSig(params, results) {
   return {params: params, results: results};