[wasm] Fix importing of re-exported JavaScript callable.

This fixes a corner-case where a {WasmExportedFunction} that represents
a re-export of a JavaScript callable from another module was identified
correctly, but not all corner-cases were correctly covered. Concretely
we failed to check for function signatures incompatible with JavaScript.

R=ahaas@chromium.org
TEST=mjsunit/regress/wasm/regress-9447
BUG=v8:9447

Change-Id: Ia6c73c82f4c1b9c357c08cde039be6af100727d6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1690941
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62632}
This commit is contained in:
Michael Starzinger 2019-07-09 14:36:15 +02:00 committed by Commit Bot
parent fa2bed3f74
commit f71ccd7a2b
7 changed files with 100 additions and 42 deletions

View File

@ -5990,47 +5990,49 @@ std::unique_ptr<OptimizedCompilationJob> NewJSToWasmCompilationJob(
std::move(debug_name), WasmAssemblerOptions());
}
WasmImportCallKind GetWasmImportCallKind(Handle<JSReceiver> target,
wasm::FunctionSig* expected_sig,
bool has_bigint_feature) {
if (WasmExportedFunction::IsWasmExportedFunction(*target)) {
auto imported_function = Handle<WasmExportedFunction>::cast(target);
std::pair<WasmImportCallKind, Handle<JSReceiver>> ResolveWasmImportCall(
Handle<JSReceiver> callable, wasm::FunctionSig* expected_sig,
bool has_bigint_feature) {
if (WasmExportedFunction::IsWasmExportedFunction(*callable)) {
auto imported_function = Handle<WasmExportedFunction>::cast(callable);
auto func_index = imported_function->function_index();
auto module = imported_function->instance().module();
wasm::FunctionSig* imported_sig = module->functions[func_index].sig;
if (*imported_sig != *expected_sig) {
return WasmImportCallKind::kLinkError;
return std::make_pair(WasmImportCallKind::kLinkError, callable);
}
if (static_cast<uint32_t>(func_index) < module->num_imported_functions) {
// TODO(wasm): this redirects all imported-reexported functions
// through the call builtin. Fall through to JS function cases below?
return WasmImportCallKind::kUseCallBuiltin;
if (static_cast<uint32_t>(func_index) >= module->num_imported_functions) {
return std::make_pair(WasmImportCallKind::kWasmToWasm, callable);
}
return WasmImportCallKind::kWasmToWasm;
Isolate* isolate = callable->GetIsolate();
// Resolve the short-cut to the underlying callable and continue.
Handle<WasmInstanceObject> instance(imported_function->instance(), isolate);
ImportedFunctionEntry entry(instance, func_index);
callable = handle(entry.callable(), isolate);
}
if (WasmJSFunction::IsWasmJSFunction(*target)) {
auto js_function = Handle<WasmJSFunction>::cast(target);
if (WasmJSFunction::IsWasmJSFunction(*callable)) {
auto js_function = Handle<WasmJSFunction>::cast(callable);
if (!js_function->MatchesSignature(expected_sig)) {
return WasmImportCallKind::kLinkError;
return std::make_pair(WasmImportCallKind::kLinkError, callable);
}
// TODO(7742): Implement proper handling of this case.
UNIMPLEMENTED();
}
if (WasmCapiFunction::IsWasmCapiFunction(*target)) {
auto capi_function = Handle<WasmCapiFunction>::cast(target);
if (WasmCapiFunction::IsWasmCapiFunction(*callable)) {
auto capi_function = Handle<WasmCapiFunction>::cast(callable);
if (!capi_function->IsSignatureEqual(expected_sig)) {
return WasmImportCallKind::kLinkError;
return std::make_pair(WasmImportCallKind::kLinkError, callable);
}
return WasmImportCallKind::kWasmToCapi;
return std::make_pair(WasmImportCallKind::kWasmToCapi, callable);
}
// Assuming we are calling to JS, check whether this would be a runtime error.
if (!wasm::IsJSCompatibleSignature(expected_sig, has_bigint_feature)) {
return WasmImportCallKind::kRuntimeTypeError;
return std::make_pair(WasmImportCallKind::kRuntimeTypeError, callable);
}
// For JavaScript calls, determine whether the target has an arity match
// and whether it has a sloppy receiver.
if (target->IsJSFunction()) {
Handle<JSFunction> function = Handle<JSFunction>::cast(target);
if (callable->IsJSFunction()) {
Handle<JSFunction> function = Handle<JSFunction>::cast(callable);
SharedFunctionInfo shared = function->shared();
// Check for math intrinsics.
@ -6039,7 +6041,9 @@ WasmImportCallKind GetWasmImportCallKind(Handle<JSReceiver> target,
wasm::FunctionSig* sig = wasm::WasmOpcodes::Signature(wasm::kExpr##name); \
if (!sig) sig = wasm::WasmOpcodes::AsmjsSignature(wasm::kExpr##name); \
DCHECK_NOT_NULL(sig); \
if (*expected_sig == *sig) return WasmImportCallKind::k##name; \
if (*expected_sig == *sig) { \
return std::make_pair(WasmImportCallKind::k##name, callable); \
} \
}
#define COMPARE_SIG_FOR_BUILTIN_F64(name) \
case Builtins::kMath##name: \
@ -6086,19 +6090,23 @@ WasmImportCallKind GetWasmImportCallKind(Handle<JSReceiver> target,
if (IsClassConstructor(shared.kind())) {
// Class constructor will throw anyway.
return WasmImportCallKind::kUseCallBuiltin;
return std::make_pair(WasmImportCallKind::kUseCallBuiltin, callable);
}
bool sloppy = is_sloppy(shared.language_mode()) && !shared.native();
if (shared.internal_formal_parameter_count() ==
expected_sig->parameter_count()) {
return sloppy ? WasmImportCallKind::kJSFunctionArityMatchSloppy
: WasmImportCallKind::kJSFunctionArityMatch;
return std::make_pair(
sloppy ? WasmImportCallKind::kJSFunctionArityMatchSloppy
: WasmImportCallKind::kJSFunctionArityMatch,
callable);
}
return sloppy ? WasmImportCallKind::kJSFunctionArityMismatchSloppy
: WasmImportCallKind::kJSFunctionArityMismatch;
return std::make_pair(
sloppy ? WasmImportCallKind::kJSFunctionArityMismatchSloppy
: WasmImportCallKind::kJSFunctionArityMismatch,
callable);
}
// Unknown case. Use the call builtin.
return WasmImportCallKind::kUseCallBuiltin;
return std::make_pair(WasmImportCallKind::kUseCallBuiltin, callable);
}
wasm::WasmOpcode GetMathIntrinsicOpcode(WasmImportCallKind kind,

View File

@ -6,6 +6,7 @@
#define V8_COMPILER_WASM_COMPILER_H_
#include <memory>
#include <utility>
// Clients of this interface shouldn't depend on lots of compiler internals.
// Do not include anything from src/compiler here!
@ -110,8 +111,12 @@ enum class WasmImportCallKind : uint8_t {
constexpr WasmImportCallKind kDefaultImportCallKind =
WasmImportCallKind::kJSFunctionArityMatchSloppy;
V8_EXPORT_PRIVATE WasmImportCallKind
GetWasmImportCallKind(Handle<JSReceiver> callable, wasm::FunctionSig* sig,
// Resolves which import call wrapper is required for the given JS callable.
// Returns the kind of wrapper need and the ultimate target callable. Note that
// some callables (e.g. a {WasmExportedFunction} or {WasmJSFunction}) just wrap
// another target, which is why the ultimate target is returned as well.
V8_EXPORT_PRIVATE std::pair<WasmImportCallKind, Handle<JSReceiver>>
ResolveWasmImportCall(Handle<JSReceiver> callable, wasm::FunctionSig* sig,
bool has_bigint_feature);
// Compiles an import call wrapper, which allows WASM to call imports.

View File

@ -837,8 +837,10 @@ bool InstanceBuilder::ProcessImportedFunction(
}
auto js_receiver = Handle<JSReceiver>::cast(value);
FunctionSig* expected_sig = module_->functions[func_index].sig;
auto kind = compiler::GetWasmImportCallKind(js_receiver, expected_sig,
enabled_.bigint);
auto resolved = compiler::ResolveWasmImportCall(js_receiver, expected_sig,
enabled_.bigint);
compiler::WasmImportCallKind kind = resolved.first;
js_receiver = resolved.second;
switch (kind) {
case compiler::WasmImportCallKind::kLinkError:
ReportLinkError("imported function does not match the expected type",
@ -846,7 +848,7 @@ bool InstanceBuilder::ProcessImportedFunction(
return false;
case compiler::WasmImportCallKind::kWasmToWasm: {
// The imported function is a WASM function from another instance.
auto imported_function = Handle<WasmExportedFunction>::cast(value);
auto imported_function = Handle<WasmExportedFunction>::cast(js_receiver);
Handle<WasmInstanceObject> imported_instance(
imported_function->instance(), isolate_);
// The import reference is the instance object itself.
@ -861,7 +863,8 @@ bool InstanceBuilder::ProcessImportedFunction(
}
case compiler::WasmImportCallKind::kWasmToCapi: {
NativeModule* native_module = instance->module_object().native_module();
Address host_address = WasmCapiFunction::cast(*value).GetHostCallTarget();
Address host_address =
WasmCapiFunction::cast(*js_receiver).GetHostCallTarget();
WasmCodeRefScope code_ref_scope;
WasmCode* wasm_code = compiler::CompileWasmCapiCallWrapper(
isolate_->wasm_engine(), native_module, expected_sig, host_address);
@ -1216,8 +1219,9 @@ void InstanceBuilder::CompileImportWrappers(
auto js_receiver = Handle<JSReceiver>::cast(value);
uint32_t func_index = module_->import_table[index].index;
FunctionSig* sig = module_->functions[func_index].sig;
auto kind =
compiler::GetWasmImportCallKind(js_receiver, sig, enabled_.bigint);
auto resolved =
compiler::ResolveWasmImportCall(js_receiver, sig, enabled_.bigint);
compiler::WasmImportCallKind kind = resolved.first;
if (kind == compiler::WasmImportCallKind::kWasmToWasm ||
kind == compiler::WasmImportCallKind::kLinkError ||
kind == compiler::WasmImportCallKind::kWasmToCapi) {

View File

@ -2017,8 +2017,10 @@ void WasmInstanceObject::ImportWasmJSFunctionIntoTable(
instance->module_object().native_module();
// TODO(mstarzinger): Cache and reuse wrapper code.
const wasm::WasmFeatures enabled = native_module->enabled_features();
compiler::WasmImportCallKind kind =
compiler::GetWasmImportCallKind(callable, sig, enabled.bigint);
auto resolved =
compiler::ResolveWasmImportCall(callable, sig, enabled.bigint);
compiler::WasmImportCallKind kind = resolved.first;
callable = resolved.second; // Update to ultimate target.
DCHECK_NE(compiler::WasmImportCallKind::kLinkError, kind);
wasm::CompilationEnv env = native_module->CreateCompilationEnv();
wasm::WasmCompilationResult result = compiler::CompileWasmImportCallWrapper(

View File

@ -47,8 +47,10 @@ TestingModuleBuilder::TestingModuleBuilder(
if (maybe_import) {
// Manually compile an import wrapper and insert it into the instance.
CodeSpaceMemoryModificationScope modification_scope(isolate_->heap());
auto kind = compiler::GetWasmImportCallKind(maybe_import->js_function,
maybe_import->sig, false);
auto resolved = compiler::ResolveWasmImportCall(maybe_import->js_function,
maybe_import->sig, false);
compiler::WasmImportCallKind kind = resolved.first;
Handle<JSReceiver> callable = resolved.second;
WasmImportWrapperCache::ModificationScope cache_scope(
native_module_->import_wrapper_cache());
WasmImportWrapperCache::CacheKey key(kind, maybe_import->sig);
@ -60,7 +62,7 @@ TestingModuleBuilder::TestingModuleBuilder(
}
ImportedFunctionEntry(instance_object_, maybe_import_index)
.SetWasmToJs(isolate_, maybe_import->js_function, import_wrapper);
.SetWasmToJs(isolate_, callable, import_wrapper);
}
if (tier == ExecutionTier::kInterpreter) {

View File

@ -0,0 +1,37 @@
// Copyright 2019 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: --no-experimental-wasm-bigint
load('test/mjsunit/wasm/wasm-module-builder.js');
// Generate a re-exported function that wraps a JavaScript callable, but with a
// function signature that is incompatible (i.e. i64 return type) with JS.
var fun1 = (function GenerateFun1() {
let builder = new WasmModuleBuilder();
function fun() { return 0 }
let fun_index = builder.addImport("m", "fun", kSig_l_v)
builder.addExport("fun", fun_index);
let instance = builder.instantiate({ m: { fun: fun }});
return instance.exports.fun;
})();
// Generate an exported function that calls the above re-export from another
// module, still with a function signature that is incompatible with JS.
var fun2 = (function GenerateFun2() {
let builder = new WasmModuleBuilder();
let fun_index = builder.addImport("m", "fun", kSig_l_v)
builder.addFunction('main', kSig_v_v)
.addBody([
kExprCallFunction, fun_index,
kExprDrop
])
.exportFunc();
let instance = builder.instantiate({ m: { fun: fun1 }});
return instance.exports.main;
})();
// Both exported functions should throw, no matter how often they get wrapped.
assertThrows(fun1, TypeError, /wasm function signature contains illegal type/);
assertThrows(fun2, TypeError, /wasm function signature contains illegal type/);

View File

@ -333,7 +333,7 @@ load('test/mjsunit/wasm/wasm-module-builder.js');
assertEquals(7, instance.exports.main());
})();*/
(function TestFunctionModuleImportMatchingSig() {
(function TestFunctionModuleImportMismatchingSig() {
let builder = new WasmModuleBuilder();
let fun1 = new WebAssembly.Function({parameters:[], results:[]}, _ => 7);
let fun2 = new WebAssembly.Function({parameters:["i32"], results:[]}, _ => 8);