From e16775163aed5e7e6a68d63074eb6c8f1db0e7fd Mon Sep 17 00:00:00 2001 From: Michael Starzinger Date: Thu, 22 Aug 2019 12:54:51 +0200 Subject: [PATCH] [wasm] Preserve identity on {WasmJSFunction} re-export. This preserves the object identity of a {WebAssembly.Function} instance that is being re-exported by a module. Such functions are considered to have an internal [[FunctionAddress]] slot and hence require their object identity to be preserved (similar to {WasmExportedFunction} already). R=jkummerow@chromium.org TEST=mjsunit/wasm/type-reflection BUG=v8:7742 Change-Id: I88ba75fcd91ce04440008467f3b218a1ac3047db Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1763545 Reviewed-by: Jakob Kummerow Commit-Queue: Michael Starzinger Cr-Commit-Position: refs/heads/master@{#63346} --- src/diagnostics/objects-printer.cc | 11 ++++--- src/runtime/runtime-wasm.cc | 4 +-- src/wasm/module-instantiate.cc | 42 ++++++++++++++------------ src/wasm/wasm-interpreter.cc | 4 +-- src/wasm/wasm-objects-inl.h | 10 +++++-- src/wasm/wasm-objects.cc | 45 +++++++++++++++------------- src/wasm/wasm-objects.h | 32 ++++++++++++++------ test/cctest/wasm/wasm-run-utils.cc | 2 +- test/mjsunit/wasm/type-reflection.js | 3 +- 9 files changed, 92 insertions(+), 61 deletions(-) diff --git a/src/diagnostics/objects-printer.cc b/src/diagnostics/objects-printer.cc index fc85a0b185..1d295a2090 100644 --- a/src/diagnostics/objects-printer.cc +++ b/src/diagnostics/objects-printer.cc @@ -1465,13 +1465,16 @@ void JSFunction::JSFunctionPrint(std::ostream& os) { // NOLINT } if (WasmExportedFunction::IsWasmExportedFunction(*this)) { WasmExportedFunction function = WasmExportedFunction::cast(*this); - os << "\n - WASM instance " - << reinterpret_cast(function.instance().ptr()); - os << "\n - WASM function index " << function.function_index(); + os << "\n - WASM instance: " << Brief(function.instance()); + os << "\n - WASM function index: " << function.function_index(); + } + if (WasmJSFunction::IsWasmJSFunction(*this)) { + WasmJSFunction function = WasmJSFunction::cast(*this); + os << "\n - WASM wrapper around: " << Brief(function.GetCallable()); } shared().PrintSourceCode(os); JSObjectPrintBody(os, *this); - os << "\n - feedback vector: "; + os << " - feedback vector: "; if (!shared().HasFeedbackMetadata()) { os << "feedback metadata is not available in SFI\n"; } else if (has_feedback_vector()) { diff --git a/src/runtime/runtime-wasm.cc b/src/runtime/runtime-wasm.cc index 7ef3daff26..57e59c07be 100644 --- a/src/runtime/runtime-wasm.cc +++ b/src/runtime/runtime-wasm.cc @@ -430,8 +430,8 @@ RUNTIME_FUNCTION(Runtime_WasmRefFunc) { isolate->set_context(instance->native_context()); CONVERT_UINT32_ARG_CHECKED(function_index, 0); - Handle function = - WasmInstanceObject::GetOrCreateWasmExportedFunction(isolate, instance, + Handle function = + WasmInstanceObject::GetOrCreateWasmExternalFunction(isolate, instance, function_index); return *function; diff --git a/src/wasm/module-instantiate.cc b/src/wasm/module-instantiate.cc index 8f1fc6b10c..976c3cde00 100644 --- a/src/wasm/module-instantiate.cc +++ b/src/wasm/module-instantiate.cc @@ -835,6 +835,14 @@ bool InstanceBuilder::ProcessImportedFunction( module_name, import_name); return false; } + // Store any {WasmExternalFunction} callable in the instance before the call + // is resolved to preserve its identity. This handles exported functions as + // well as functions constructed via other means (e.g. WebAssembly.Function). + if (WasmExternalFunction::IsWasmExternalFunction(*value)) { + WasmInstanceObject::SetWasmExternalFunction( + isolate_, instance, func_index, + Handle::cast(value)); + } auto js_receiver = Handle::cast(value); FunctionSig* expected_sig = module_->functions[func_index].sig; auto resolved = @@ -855,10 +863,6 @@ bool InstanceBuilder::ProcessImportedFunction( Address imported_target = imported_function->GetWasmCallTarget(); ImportedFunctionEntry entry(instance, func_index); entry.SetWasmToWasm(*imported_instance, imported_target); - // Also store the {WasmExportedFunction} in the instance to preserve its - // identity. - WasmInstanceObject::SetWasmExportedFunction( - isolate_, instance, func_index, imported_function); break; } case compiler::WasmImportCallKind::kWasmToCapi: { @@ -1373,7 +1377,7 @@ void InstanceBuilder::InitGlobals(Handle instance) { break; case WasmInitExpr::kRefFuncConst: { DCHECK(enabled_.anyref); - auto function = WasmInstanceObject::GetOrCreateWasmExportedFunction( + auto function = WasmInstanceObject::GetOrCreateWasmExternalFunction( isolate_, instance, global.init.val.function_index); tagged_globals_->set(global.offset, *function); break; @@ -1450,10 +1454,10 @@ void InstanceBuilder::ProcessExports(Handle instance) { const WasmImport& import = module_->import_table[index]; if (import.kind == kExternalFunction) { Handle value = sanitized_imports_[index].value; - if (WasmExportedFunction::IsWasmExportedFunction(*value)) { - WasmInstanceObject::SetWasmExportedFunction( + if (WasmExternalFunction::IsWasmExternalFunction(*value)) { + WasmInstanceObject::SetWasmExternalFunction( isolate_, instance, import.index, - Handle::cast(value)); + Handle::cast(value)); } } } @@ -1498,10 +1502,10 @@ void InstanceBuilder::ProcessExports(Handle instance) { case kExternalFunction: { // Wrap and export the code as a JSFunction. // TODO(wasm): reduce duplication with LoadElemSegment() further below - MaybeHandle wasm_exported_function = - WasmInstanceObject::GetOrCreateWasmExportedFunction( + Handle wasm_external_function = + WasmInstanceObject::GetOrCreateWasmExternalFunction( isolate_, instance, exp.index); - desc.set_value(wasm_exported_function.ToHandleChecked()); + desc.set_value(wasm_external_function); if (is_asm_js && String::Equals(isolate_, name, @@ -1661,27 +1665,27 @@ bool LoadElemSegmentImpl(Isolate* isolate, Handle instance, .Set(sig_id, instance, func_index); } - // For AnyRef tables, we have to generate the WasmExportedFunction eagerly. + // For AnyRef tables, we have to generate the WasmExternalFunction eagerly. // Later we cannot know if an entry is a placeholder or not. if (table_object->type() == kWasmAnyRef) { - Handle wasm_exported_function = - WasmInstanceObject::GetOrCreateWasmExportedFunction(isolate, instance, + Handle wasm_external_function = + WasmInstanceObject::GetOrCreateWasmExternalFunction(isolate, instance, func_index); WasmTableObject::Set(isolate, table_object, entry_index, - wasm_exported_function); + wasm_external_function); } else { // Update the table object's other dispatch tables. - MaybeHandle wasm_exported_function = - WasmInstanceObject::GetWasmExportedFunction(isolate, instance, + MaybeHandle wasm_external_function = + WasmInstanceObject::GetWasmExternalFunction(isolate, instance, func_index); - if (wasm_exported_function.is_null()) { + if (wasm_external_function.is_null()) { // No JSFunction entry yet exists for this function. Create a {Tuple2} // holding the information to lazily allocate one. WasmTableObject::SetFunctionTablePlaceholder( isolate, table_object, entry_index, instance, func_index); } else { table_object->entries().set(entry_index, - *wasm_exported_function.ToHandleChecked()); + *wasm_external_function.ToHandleChecked()); } // UpdateDispatchTables() updates all other dispatch tables, since // we have not yet added the dispatch table we are currently building. diff --git a/src/wasm/wasm-interpreter.cc b/src/wasm/wasm-interpreter.cc index c1ba4c5ba0..3dd4751645 100644 --- a/src/wasm/wasm-interpreter.cc +++ b/src/wasm/wasm-interpreter.cc @@ -3046,8 +3046,8 @@ class ThreadImpl { code->at(pc)); HandleScope handle_scope(isolate_); // Avoid leaking handles. - Handle function = - WasmInstanceObject::GetOrCreateWasmExportedFunction( + Handle function = + WasmInstanceObject::GetOrCreateWasmExternalFunction( isolate_, instance_object_, imm.index); Push(WasmValue(function)); len = 1 + imm.length; diff --git a/src/wasm/wasm-objects-inl.h b/src/wasm/wasm-objects-inl.h index a5cc79bd4d..ab8aad4490 100644 --- a/src/wasm/wasm-objects-inl.h +++ b/src/wasm/wasm-objects-inl.h @@ -261,8 +261,8 @@ OPTIONAL_ACCESSORS(WasmInstanceObject, managed_native_allocations, Foreign, kManagedNativeAllocationsOffset) OPTIONAL_ACCESSORS(WasmInstanceObject, exceptions_table, FixedArray, kExceptionsTableOffset) -OPTIONAL_ACCESSORS(WasmInstanceObject, wasm_exported_functions, FixedArray, - kWasmExportedFunctionsOffset) +OPTIONAL_ACCESSORS(WasmInstanceObject, wasm_external_functions, FixedArray, + kWasmExternalFunctionsOffset) void WasmInstanceObject::clear_padding() { if (FIELD_SIZE(kOptionalPaddingOffset) != 0) { @@ -363,6 +363,12 @@ ACCESSORS(WasmCapiFunctionData, wrapper_code, Code, kWrapperCodeOffset) ACCESSORS(WasmCapiFunctionData, serialized_signature, PodArray, kSerializedSignatureOffset) +// WasmExternalFunction +WasmExternalFunction::WasmExternalFunction(Address ptr) : JSFunction(ptr) { + SLOW_DCHECK(IsWasmExternalFunction(*this)); +} +CAST_ACCESSOR(WasmExternalFunction) + // WasmIndirectFunctionTable OBJECT_CONSTRUCTORS_IMPL(WasmIndirectFunctionTable, Struct) CAST_ACCESSOR(WasmIndirectFunctionTable) diff --git a/src/wasm/wasm-objects.cc b/src/wasm/wasm-objects.cc index 151ad5a89e..a8b3e51d91 100644 --- a/src/wasm/wasm-objects.cc +++ b/src/wasm/wasm-objects.cc @@ -963,7 +963,7 @@ Handle WasmTableObject::Get(Isolate* isolate, // Check if we already compiled a wrapper for the function but did not store // it in the table slot yet. - entry = WasmInstanceObject::GetOrCreateWasmExportedFunction(isolate, instance, + entry = WasmInstanceObject::GetOrCreateWasmExternalFunction(isolate, instance, function_index); entries->set(entry_index, *entry); return entry; @@ -1858,27 +1858,27 @@ bool WasmInstanceObject::InitTableEntries(Isolate* isolate, dst, src, count); } -MaybeHandle WasmInstanceObject::GetWasmExportedFunction( +MaybeHandle WasmInstanceObject::GetWasmExternalFunction( Isolate* isolate, Handle instance, int index) { - MaybeHandle result; - if (instance->has_wasm_exported_functions()) { - Object val = instance->wasm_exported_functions().get(index); + MaybeHandle result; + if (instance->has_wasm_external_functions()) { + Object val = instance->wasm_external_functions().get(index); if (!val.IsUndefined(isolate)) { - result = Handle(WasmExportedFunction::cast(val), + result = Handle(WasmExternalFunction::cast(val), isolate); } } return result; } -Handle -WasmInstanceObject::GetOrCreateWasmExportedFunction( +Handle +WasmInstanceObject::GetOrCreateWasmExternalFunction( Isolate* isolate, Handle instance, int function_index) { - MaybeHandle maybe_result = - WasmInstanceObject::GetWasmExportedFunction(isolate, instance, + MaybeHandle maybe_result = + WasmInstanceObject::GetWasmExternalFunction(isolate, instance, function_index); - Handle result; + Handle result; if (maybe_result.ToHandle(&result)) { return result; } @@ -1903,27 +1903,27 @@ WasmInstanceObject::GetOrCreateWasmExportedFunction( isolate, function.sig, function.imported); module_object->export_wrappers().set(wrapper_index, *wrapper); } - result = WasmExportedFunction::New( + result = Handle::cast(WasmExportedFunction::New( isolate, instance, function_index, - static_cast(function.sig->parameter_count()), wrapper); + static_cast(function.sig->parameter_count()), wrapper)); - WasmInstanceObject::SetWasmExportedFunction(isolate, instance, function_index, + WasmInstanceObject::SetWasmExternalFunction(isolate, instance, function_index, result); return result; } -void WasmInstanceObject::SetWasmExportedFunction( +void WasmInstanceObject::SetWasmExternalFunction( Isolate* isolate, Handle instance, int index, - Handle val) { + Handle val) { Handle functions; - if (!instance->has_wasm_exported_functions()) { - // lazily-allocate the wasm exported functions. + if (!instance->has_wasm_external_functions()) { + // Lazily allocate the wasm external functions array. functions = isolate->factory()->NewFixedArray( static_cast(instance->module()->functions.size())); - instance->set_wasm_exported_functions(*functions); + instance->set_wasm_external_functions(*functions); } else { functions = - Handle(instance->wasm_exported_functions(), isolate); + Handle(instance->wasm_external_functions(), isolate); } functions->set(index, *val); } @@ -2343,6 +2343,11 @@ PodArray WasmCapiFunction::GetSerializedSignature() const { return shared().wasm_capi_function_data().serialized_signature(); } +bool WasmExternalFunction::IsWasmExternalFunction(Object object) { + return WasmExportedFunction::IsWasmExportedFunction(object) || + WasmJSFunction::IsWasmJSFunction(object); +} + Handle WasmExceptionTag::New(Isolate* isolate, int index) { Handle result = Handle::cast(isolate->factory()->NewStruct( diff --git a/src/wasm/wasm-objects.h b/src/wasm/wasm-objects.h index da65a62392..1e40e855d6 100644 --- a/src/wasm/wasm-objects.h +++ b/src/wasm/wasm-objects.h @@ -41,6 +41,7 @@ class WasmCapiFunction; class WasmDebugInfo; class WasmExceptionTag; class WasmExportedFunction; +class WasmExternalFunction; class WasmInstanceObject; class WasmJSFunction; class WasmModuleObject; @@ -440,7 +441,7 @@ class WasmInstanceObject : public JSObject { DECL_OPTIONAL_ACCESSORS(indirect_function_table_refs, FixedArray) DECL_OPTIONAL_ACCESSORS(managed_native_allocations, Foreign) DECL_OPTIONAL_ACCESSORS(exceptions_table, FixedArray) - DECL_OPTIONAL_ACCESSORS(wasm_exported_functions, FixedArray) + DECL_OPTIONAL_ACCESSORS(wasm_external_functions, FixedArray) DECL_PRIMITIVE_ACCESSORS(memory_start, byte*) DECL_PRIMITIVE_ACCESSORS(memory_size, size_t) DECL_PRIMITIVE_ACCESSORS(memory_mask, size_t) @@ -499,7 +500,7 @@ class WasmInstanceObject : public JSObject { V(kIndirectFunctionTablesOffset, kTaggedSize) \ V(kManagedNativeAllocationsOffset, kTaggedSize) \ V(kExceptionsTableOffset, kTaggedSize) \ - V(kWasmExportedFunctionsOffset, kTaggedSize) \ + V(kWasmExternalFunctionsOffset, kTaggedSize) \ V(kRealStackLimitAddressOffset, kSystemPointerSize) \ V(kDataSegmentStartsOffset, kSystemPointerSize) \ V(kDataSegmentSizesOffset, kSystemPointerSize) \ @@ -538,7 +539,7 @@ class WasmInstanceObject : public JSObject { kIndirectFunctionTablesOffset, kManagedNativeAllocationsOffset, kExceptionsTableOffset, - kWasmExportedFunctionsOffset}; + kWasmExternalFunctionsOffset}; V8_EXPORT_PRIVATE const wasm::WasmModule* module(); @@ -581,22 +582,22 @@ class WasmInstanceObject : public JSObject { // Iterates all fields in the object except the untagged fields. class BodyDescriptor; - static MaybeHandle GetWasmExportedFunction( + static MaybeHandle GetWasmExternalFunction( Isolate* isolate, Handle instance, int index); - // Acquires the {WasmExportedFunction} for a given {function_index} from the + // Acquires the {WasmExternalFunction} for a given {function_index} from the // cache of the given {instance}, or creates a new {WasmExportedFunction} if // it does not exist yet. The new {WasmExportedFunction} is added to the // cache of the {instance} immediately. - V8_EXPORT_PRIVATE static Handle - GetOrCreateWasmExportedFunction(Isolate* isolate, + V8_EXPORT_PRIVATE static Handle + GetOrCreateWasmExternalFunction(Isolate* isolate, Handle instance, int function_index); - static void SetWasmExportedFunction(Isolate* isolate, + static void SetWasmExternalFunction(Isolate* isolate, Handle instance, int index, - Handle val); + Handle val); // Imports a constructed {WasmJSFunction} into the indirect function table of // this instance. Note that this might trigger wrapper compilation, since a @@ -719,6 +720,19 @@ class WasmCapiFunction : public JSFunction { OBJECT_CONSTRUCTORS(WasmCapiFunction, JSFunction); }; +// Any external function that can be imported/exported in modules. This abstract +// class just dispatches to the following concrete classes: +// - {WasmExportedFunction}: A proper Wasm function exported from a module. +// - {WasmJSFunction}: A function constructed via WebAssembly.Function in JS. +// // TODO(wasm): Potentially {WasmCapiFunction} will be added here as well. +class WasmExternalFunction : public JSFunction { + public: + static bool IsWasmExternalFunction(Object object); + + DECL_CAST(WasmExternalFunction) + OBJECT_CONSTRUCTORS(WasmExternalFunction, JSFunction); +}; + class WasmIndirectFunctionTable : public Struct { public: DECL_PRIMITIVE_ACCESSORS(size, uint32_t) diff --git a/test/cctest/wasm/wasm-run-utils.cc b/test/cctest/wasm/wasm-run-utils.cc index da2c59e0ac..528d71f53c 100644 --- a/test/cctest/wasm/wasm-run-utils.cc +++ b/test/cctest/wasm/wasm-run-utils.cc @@ -159,7 +159,7 @@ void TestingModuleBuilder::FreezeSignatureMapAndInitializeWrapperCache() { Handle TestingModuleBuilder::WrapCode(uint32_t index) { FreezeSignatureMapAndInitializeWrapperCache(); SetExecutable(); - return WasmInstanceObject::GetOrCreateWasmExportedFunction( + return WasmInstanceObject::GetOrCreateWasmExternalFunction( isolate_, instance_object(), index); } diff --git a/test/mjsunit/wasm/type-reflection.js b/test/mjsunit/wasm/type-reflection.js index a51f114b25..a9a0b87143 100644 --- a/test/mjsunit/wasm/type-reflection.js +++ b/test/mjsunit/wasm/type-reflection.js @@ -613,6 +613,5 @@ load('test/mjsunit/wasm/wasm-module-builder.js'); builder.addExport("fun2", fun_index); let instance = builder.instantiate({ m: { fun: fun }}); assertSame(instance.exports.fun1, instance.exports.fun2); - // TODO(7742): Fix after https://github.com/WebAssembly/js-types/issues/11. - assertNotSame(fun, instance.exports.fun1); + assertSame(fun, instance.exports.fun1); })();