From db5ebb2d8a4d16f6daeee322ba72d45e8b41d6ef Mon Sep 17 00:00:00 2001 From: Jakob Kummerow Date: Wed, 28 Aug 2019 14:03:58 +0200 Subject: [PATCH] [wasm-c-api][ptr-compr] Fix call target caching The previous pseudo-smi storage scheme for caching call target addresses in a struct without requiring a custom visitor only works on uncompressed 64-bit platforms. This patch fixes other platforms (natural or compressed 32-bit) by boxing the address in a Foreign. Change-Id: I3c182c1d9ccae4858cac2757fc3daa40d1520998 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1771780 Commit-Queue: Jakob Kummerow Reviewed-by: Michael Starzinger Cr-Commit-Position: refs/heads/master@{#63422} --- src/builtins/base.tq | 2 +- src/wasm/c-api.cc | 34 +++++++++++++++++++++++++++------- src/wasm/wasm-objects-inl.h | 2 +- src/wasm/wasm-objects.h | 2 +- 4 files changed, 30 insertions(+), 10 deletions(-) diff --git a/src/builtins/base.tq b/src/builtins/base.tq index 323e718dea..df660253cc 100644 --- a/src/builtins/base.tq +++ b/src/builtins/base.tq @@ -986,7 +986,7 @@ extern class WasmExportedFunctionData extends Struct { // The remaining fields are for fast calling from C++. The contract is // that they are lazily populated, and either all will be present or none. c_wrapper_code: Object; - wasm_call_target: Smi; // Pseudo-smi: one-bit shift on all platforms. + wasm_call_target: Smi | Foreign; packed_args_size: Smi; } diff --git a/src/wasm/c-api.cc b/src/wasm/c-api.cc index 50002e838f..2789f22ad3 100644 --- a/src/wasm/c-api.cc +++ b/src/wasm/c-api.cc @@ -1354,6 +1354,26 @@ i::Handle WasmRefToV8(i::Isolate* isolate, const Ref* ref) { return impl(ref)->v8_object(); } +i::Handle CallTargetForCaching(i::Isolate* isolate, + i::Address real_call_target) { + if (i::kTaggedSize == i::kInt32Size) { + return isolate->factory()->NewForeign(real_call_target); + } else { + // 64-bit uncompressed platform. + return i::handle(i::Smi((real_call_target << i::kSmiTagSize) | i::kSmiTag), + isolate); + } +} + +i::Address CallTargetFromCache(i::Object cached_call_target) { + if (i::kTaggedSize == i::kInt32Size) { + return i::Foreign::cast(cached_call_target).foreign_address(); + } else { + // 64-bit uncompressed platform. + return cached_call_target.ptr() >> i::kSmiTagSize; + } +} + void PrepareFunctionData(i::Isolate* isolate, i::Handle function_data, i::wasm::FunctionSig* sig) { @@ -1366,12 +1386,12 @@ void PrepareFunctionData(i::Isolate* isolate, // Compute packed args size. function_data->set_packed_args_size( i::wasm::CWasmArgumentsPacker::TotalSize(sig)); - // Get call target (function table offset). This is an Address, we store - // it as a pseudo-Smi by shifting it by one bit, so the GC leaves it alone. - i::Address call_target = - function_data->instance().GetCallTarget(function_data->function_index()); - i::Smi smi_target((call_target << i::kSmiTagSize) | i::kSmiTag); - function_data->set_wasm_call_target(smi_target); + // Get call target (function table offset), and wrap it as a cacheable object + // (pseudo-Smi or Foreign, depending on platform). + i::Handle call_target = CallTargetForCaching( + isolate, + function_data->instance().GetCallTarget(function_data->function_index())); + function_data->set_wasm_call_target(*call_target); } void PushArgs(i::wasm::FunctionSig* sig, const Val args[], @@ -1495,7 +1515,7 @@ auto Func::call(const Val args[], Val results[]) const -> own { i::Handle wrapper_code = i::Handle( i::Code::cast(function_data->c_wrapper_code()), isolate); i::Address call_target = - function_data->wasm_call_target().ptr() >> i::kSmiTagSize; + CallTargetFromCache(function_data->wasm_call_target()); i::wasm::CWasmArgumentsPacker packer(function_data->packed_args_size()); PushArgs(sig, args, &packer, store); diff --git a/src/wasm/wasm-objects-inl.h b/src/wasm/wasm-objects-inl.h index 43e4c6cbd2..cc2de230a9 100644 --- a/src/wasm/wasm-objects-inl.h +++ b/src/wasm/wasm-objects-inl.h @@ -324,7 +324,7 @@ SMI_ACCESSORS(WasmExportedFunctionData, jump_table_offset, kJumpTableOffsetOffset) SMI_ACCESSORS(WasmExportedFunctionData, function_index, kFunctionIndexOffset) ACCESSORS(WasmExportedFunctionData, c_wrapper_code, Object, kCWrapperCodeOffset) -ACCESSORS(WasmExportedFunctionData, wasm_call_target, Smi, +ACCESSORS(WasmExportedFunctionData, wasm_call_target, Object, kWasmCallTargetOffset) SMI_ACCESSORS(WasmExportedFunctionData, packed_args_size, kPackedArgsSizeOffset) diff --git a/src/wasm/wasm-objects.h b/src/wasm/wasm-objects.h index 066cef988f..43f3da8c88 100644 --- a/src/wasm/wasm-objects.h +++ b/src/wasm/wasm-objects.h @@ -792,7 +792,7 @@ class WasmExportedFunctionData : public Struct { DECL_INT_ACCESSORS(jump_table_offset) DECL_INT_ACCESSORS(function_index) DECL_ACCESSORS(c_wrapper_code, Object) - DECL_ACCESSORS(wasm_call_target, Smi) + DECL_ACCESSORS(wasm_call_target, Object) DECL_INT_ACCESSORS(packed_args_size) DECL_CAST(WasmExportedFunctionData)