From e90af2c7026aa28dbfe80924e3517316dd1c3f46 Mon Sep 17 00:00:00 2001 From: Clemens Hammacher Date: Thu, 21 Jun 2018 12:27:53 +0200 Subject: [PATCH] [api] Properly define BufferReference instead of CallerOwnedBuffer The name {CallerOwnedBuffer} does not make sense in all situations, especially if such an object is returned instead of being passed as argument. I am working on moving the wasm wire bytes off the JS heap, and hence will return unowned references via the API. To prepare this change, I deprecate the existing {CallerOwnedBuffer} and introduce a new {BufferReference} struct with proper field names. R=titzer@chromium.org, adamk@chromium.org Bug: v8:7868 Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng Change-Id: Ic8953951447038a831b15a336a52a199bfbeafd5 Reviewed-on: https://chromium-review.googlesource.com/1108207 Reviewed-by: Yang Guo Reviewed-by: Ben Titzer Commit-Queue: Clemens Hammacher Cr-Commit-Position: refs/heads/master@{#53929} --- include/v8.h | 44 +++++++++++++++++---- src/api.cc | 20 +++++----- test/cctest/wasm/test-wasm-codegen.cc | 8 ++-- test/cctest/wasm/test-wasm-serialization.cc | 27 ++++--------- 4 files changed, 57 insertions(+), 42 deletions(-) diff --git a/include/v8.h b/include/v8.h index b063a23303..9c82ef2816 100644 --- a/include/v8.h +++ b/include/v8.h @@ -4308,10 +4308,29 @@ class V8_EXPORT Proxy : public Object { class V8_EXPORT WasmCompiledModule : public Object { public: typedef std::pair, size_t> SerializedModule; + +// The COMMA macro allows us to use ',' inside of the V8_DEPRECATE_SOON macro. +#define COMMA , + V8_DEPRECATE_SOON( + "Use BufferReference.", + typedef std::pair CallerOwnedBuffer); +#undef COMMA + /** - * A buffer that is owned by the caller. + * A unowned reference to a byte buffer. */ - typedef std::pair CallerOwnedBuffer; + struct BufferReference { + const uint8_t* start; + size_t size; + BufferReference(const uint8_t* start, size_t size) + : start(start), size(size) {} + // Temporarily allow conversion to and from CallerOwnedBuffer. + V8_DEPRECATE_SOON( + "Use BufferReference directly.", + inline BufferReference(CallerOwnedBuffer)); // NOLINT(runtime/explicit) + V8_DEPRECATE_SOON("Use BufferReference directly.", + inline operator CallerOwnedBuffer()); + }; /** * An opaque, native heap object for transferring wasm modules. It @@ -4328,7 +4347,7 @@ class V8_EXPORT WasmCompiledModule : public Object { private: typedef std::pair, size_t> OwnedBuffer; friend class WasmCompiledModule; - TransferrableModule(OwnedBuffer&& code, OwnedBuffer&& bytes) + TransferrableModule(OwnedBuffer code, OwnedBuffer bytes) : compiled_code(std::move(code)), wire_bytes(std::move(bytes)) {} OwnedBuffer compiled_code = {nullptr, 0}; @@ -4365,18 +4384,18 @@ class V8_EXPORT WasmCompiledModule : public Object { * uncompiled bytes. */ static MaybeLocal DeserializeOrCompile( - Isolate* isolate, const CallerOwnedBuffer& serialized_module, - const CallerOwnedBuffer& wire_bytes); + Isolate* isolate, BufferReference serialized_module, + BufferReference wire_bytes); V8_INLINE static WasmCompiledModule* Cast(Value* obj); private: static MaybeLocal Deserialize( - Isolate* isolate, const CallerOwnedBuffer& serialized_module, - const CallerOwnedBuffer& wire_bytes); + Isolate* isolate, BufferReference serialized_module, + BufferReference wire_bytes); static MaybeLocal Compile(Isolate* isolate, const uint8_t* start, size_t length); - static CallerOwnedBuffer AsCallerOwned( + static BufferReference AsReference( const TransferrableModule::OwnedBuffer& buff) { return {buff.first.get(), buff.second}; } @@ -4385,6 +4404,15 @@ class V8_EXPORT WasmCompiledModule : public Object { static void CheckCast(Value* obj); }; +// TODO(clemensh): Remove after M69 branch. +WasmCompiledModule::BufferReference::BufferReference( + WasmCompiledModule::CallerOwnedBuffer buf) + : BufferReference(buf.first, buf.second) {} +WasmCompiledModule::BufferReference:: +operator WasmCompiledModule::CallerOwnedBuffer() { + return {start, size}; +} + // TODO(mtrofin): when streaming compilation is done, we can rename this // to simply WasmModuleObjectBuilder class V8_EXPORT WasmModuleObjectBuilderStreaming final { diff --git a/src/api.cc b/src/api.cc index 30f978ecae..ea7c126aa5 100644 --- a/src/api.cc +++ b/src/api.cc @@ -7481,8 +7481,8 @@ MaybeLocal WasmCompiledModule::FromTransferrableModule( Isolate* isolate, const WasmCompiledModule::TransferrableModule& transferrable_module) { MaybeLocal ret = - Deserialize(isolate, AsCallerOwned(transferrable_module.compiled_code), - AsCallerOwned(transferrable_module.wire_bytes)); + Deserialize(isolate, AsReference(transferrable_module.compiled_code), + AsReference(transferrable_module.wire_bytes)); return ret; } @@ -7500,14 +7500,13 @@ WasmCompiledModule::SerializedModule WasmCompiledModule::Serialize() { } MaybeLocal WasmCompiledModule::Deserialize( - Isolate* isolate, - const WasmCompiledModule::CallerOwnedBuffer& serialized_module, - const WasmCompiledModule::CallerOwnedBuffer& wire_bytes) { + Isolate* isolate, WasmCompiledModule::BufferReference serialized_module, + WasmCompiledModule::BufferReference wire_bytes) { i::Isolate* i_isolate = reinterpret_cast(isolate); i::MaybeHandle maybe_module_object = i::wasm::DeserializeNativeModule( - i_isolate, {serialized_module.first, serialized_module.second}, - {wire_bytes.first, wire_bytes.second}); + i_isolate, {serialized_module.start, serialized_module.size}, + {wire_bytes.start, wire_bytes.size}); i::Handle module_object; if (!maybe_module_object.ToHandle(&module_object)) { return MaybeLocal(); @@ -7517,15 +7516,14 @@ MaybeLocal WasmCompiledModule::Deserialize( } MaybeLocal WasmCompiledModule::DeserializeOrCompile( - Isolate* isolate, - const WasmCompiledModule::CallerOwnedBuffer& serialized_module, - const WasmCompiledModule::CallerOwnedBuffer& wire_bytes) { + Isolate* isolate, WasmCompiledModule::BufferReference serialized_module, + WasmCompiledModule::BufferReference wire_bytes) { MaybeLocal ret = Deserialize(isolate, serialized_module, wire_bytes); if (!ret.IsEmpty()) { return ret; } - return Compile(isolate, wire_bytes.first, wire_bytes.second); + return Compile(isolate, wire_bytes.start, wire_bytes.size); } MaybeLocal WasmCompiledModule::Compile(Isolate* isolate, diff --git a/test/cctest/wasm/test-wasm-codegen.cc b/test/cctest/wasm/test-wasm-codegen.cc index 189ef46878..cbd943ca09 100644 --- a/test/cctest/wasm/test-wasm-codegen.cc +++ b/test/cctest/wasm/test-wasm-codegen.cc @@ -58,10 +58,10 @@ void BuildTrivialModule(Zone* zone, ZoneBuffer* buffer) { } bool TestModule(Isolate* isolate, - v8::WasmCompiledModule::CallerOwnedBuffer wire_bytes) { + v8::WasmCompiledModule::BufferReference wire_bytes) { HandleScope scope(isolate); - v8::WasmCompiledModule::CallerOwnedBuffer serialized_module(nullptr, 0); + v8::WasmCompiledModule::BufferReference serialized_module(nullptr, 0); MaybeLocal module = v8::WasmCompiledModule::DeserializeOrCompile( reinterpret_cast(isolate), serialized_module, @@ -76,8 +76,8 @@ TEST(PropertiesOfCodegenCallbacks) { Zone zone(&allocator, ZONE_NAME); ZoneBuffer buffer(&zone); BuildTrivialModule(&zone, &buffer); - v8::WasmCompiledModule::CallerOwnedBuffer wire_bytes = {buffer.begin(), - buffer.size()}; + v8::WasmCompiledModule::BufferReference wire_bytes = {buffer.begin(), + buffer.size()}; Isolate* isolate = CcTest::InitIsolateOnce(); HandleScope scope(isolate); testing::SetupIsolateForWasmModule(isolate); diff --git a/test/cctest/wasm/test-wasm-serialization.cc b/test/cctest/wasm/test-wasm-serialization.cc index 2324c95d10..680f74908b 100644 --- a/test/cctest/wasm/test-wasm-serialization.cc +++ b/test/cctest/wasm/test-wasm-serialization.cc @@ -64,26 +64,22 @@ class WasmSerializationTest { builder->WriteTo(*buffer); } - void ClearSerializedData() { - serialized_bytes_.first = nullptr; - serialized_bytes_.second = 0; - } + void ClearSerializedData() { serialized_bytes_ = {nullptr, 0}; } void InvalidateVersion() { uint32_t* slot = reinterpret_cast( - const_cast(serialized_bytes_.first) + + const_cast(serialized_bytes_.start) + SerializedCodeData::kVersionHashOffset); *slot = Version::Hash() + 1; } void InvalidateWireBytes() { - memset(const_cast(wire_bytes_.first), '\0', - wire_bytes_.second / 2); + memset(const_cast(wire_bytes_.start), 0, wire_bytes_.size / 2); } void InvalidateLength() { uint32_t* slot = reinterpret_cast( - const_cast(serialized_bytes_.first) + + const_cast(serialized_bytes_.start) + SerializedCodeData::kPayloadLengthOffset); *slot = 0u; } @@ -92,7 +88,7 @@ class WasmSerializationTest { ErrorThrower thrower(current_isolate(), ""); v8::MaybeLocal deserialized = v8::WasmCompiledModule::DeserializeOrCompile( - current_isolate_v8(), serialized_bytes(), wire_bytes()); + current_isolate_v8(), serialized_bytes_, wire_bytes_); return deserialized; } @@ -106,7 +102,7 @@ class WasmSerializationTest { DisallowHeapAllocation assume_no_gc; CHECK_EQ(memcmp(reinterpret_cast( module_object->module_bytes()->GetCharsAddress()), - wire_bytes().first, wire_bytes().second), + wire_bytes_.start, wire_bytes_.size), 0); } Handle instance = @@ -138,13 +134,6 @@ class WasmSerializationTest { static const char* kFunctionName; Zone* zone() { return &zone_; } - const v8::WasmCompiledModule::CallerOwnedBuffer& wire_bytes() const { - return wire_bytes_; - } - - const v8::WasmCompiledModule::CallerOwnedBuffer& serialized_bytes() const { - return serialized_bytes_; - } void SetUp() { ZoneBuffer buffer(&zone_); @@ -208,8 +197,8 @@ class WasmSerializationTest { v8::internal::AccountingAllocator allocator_; Zone zone_; v8::WasmCompiledModule::SerializedModule data_; - v8::WasmCompiledModule::CallerOwnedBuffer wire_bytes_; - v8::WasmCompiledModule::CallerOwnedBuffer serialized_bytes_; + v8::WasmCompiledModule::BufferReference wire_bytes_ = {nullptr, 0}; + v8::WasmCompiledModule::BufferReference serialized_bytes_ = {nullptr, 0}; v8::Isolate* current_isolate_v8_; };