From 4089299494353c5f81af53e3f3b33d3857ccc49a Mon Sep 17 00:00:00 2001 From: Deepti Gandluri Date: Tue, 29 Jan 2019 11:06:38 -0800 Subject: [PATCH] Add a contents based constructor to the SharedArrayBuffer API The motivation of this change was originally to preserve is_growable flag over PostMessage in d8. Adding a more general constructor that uses SharedArrayBuffer::Contents. Change-Id: Ib8f6c36d659e91f6cfb6487f56de34fa7e8841a9 Bug: v8:8564 Reviewed-on: https://chromium-review.googlesource.com/c/1383093 Commit-Queue: Deepti Gandluri Reviewed-by: Ben Smith Reviewed-by: Adam Klein Cr-Commit-Position: refs/heads/master@{#59184} --- include/v8.h | 15 +++++++++-- src/api.cc | 54 +++++++++++++++++++++++++++------------- src/d8.cc | 5 ++-- src/wasm/wasm-objects.cc | 4 ++- 4 files changed, 55 insertions(+), 23 deletions(-) diff --git a/include/v8.h b/include/v8.h index 07ca773812..fb3469a994 100644 --- a/include/v8.h +++ b/include/v8.h @@ -5177,7 +5177,8 @@ class V8_EXPORT SharedArrayBuffer : public Object { allocation_length_(0), allocation_mode_(Allocator::AllocationMode::kNormal), deleter_(nullptr), - deleter_data_(nullptr) {} + deleter_data_(nullptr), + is_growable_(false) {} void* AllocationBase() const { return allocation_base_; } size_t AllocationLength() const { return allocation_length_; } @@ -5189,12 +5190,13 @@ class V8_EXPORT SharedArrayBuffer : public Object { size_t ByteLength() const { return byte_length_; } DeleterCallback Deleter() const { return deleter_; } void* DeleterData() const { return deleter_data_; } + bool IsGrowable() const { return is_growable_; } private: Contents(void* data, size_t byte_length, void* allocation_base, size_t allocation_length, Allocator::AllocationMode allocation_mode, DeleterCallback deleter, - void* deleter_data); + void* deleter_data, bool is_growable); void* data_; size_t byte_length_; @@ -5203,6 +5205,7 @@ class V8_EXPORT SharedArrayBuffer : public Object { Allocator::AllocationMode allocation_mode_; DeleterCallback deleter_; void* deleter_data_; + bool is_growable_; friend class SharedArrayBuffer; }; @@ -5230,6 +5233,14 @@ class V8_EXPORT SharedArrayBuffer : public Object { Isolate* isolate, void* data, size_t byte_length, ArrayBufferCreationMode mode = ArrayBufferCreationMode::kExternalized); + /** + * Create a new SharedArrayBuffer over an existing memory block. Propagate + * flags to indicate whether the underlying buffer can be grown. + */ + static Local New( + Isolate* isolate, const SharedArrayBuffer::Contents&, + ArrayBufferCreationMode mode = ArrayBufferCreationMode::kExternalized); + /** * Returns true if SharedArrayBuffer is externalized, that is, does not * own its memory block. diff --git a/src/api.cc b/src/api.cc index 78817d6712..13b9a92543 100644 --- a/src/api.cc +++ b/src/api.cc @@ -7718,6 +7718,27 @@ Local DataView::New(Local shared_array_buffer, return Utils::ToLocal(obj); } +namespace { +i::Handle SetupSharedArrayBuffer( + Isolate* isolate, void* data, size_t byte_length, + ArrayBufferCreationMode mode) { + CHECK(i::FLAG_harmony_sharedarraybuffer); + // Embedders must guarantee that the external backing store is valid. + CHECK(byte_length == 0 || data != nullptr); + i::Isolate* i_isolate = reinterpret_cast(isolate); + LOG_API(i_isolate, SharedArrayBuffer, New); + ENTER_V8_NO_SCRIPT_NO_EXCEPTION(i_isolate); + i::Handle obj = + i_isolate->factory()->NewJSArrayBuffer(i::SharedFlag::kShared); + bool is_wasm_memory = + i_isolate->wasm_engine()->memory_tracker()->IsWasmMemory(data); + i::JSArrayBuffer::Setup(obj, i_isolate, + mode == ArrayBufferCreationMode::kExternalized, data, + byte_length, i::SharedFlag::kShared, is_wasm_memory); + return obj; +} + +} // namespace bool v8::SharedArrayBuffer::IsExternal() const { return Utils::OpenHandle(this)->is_external(); @@ -7740,14 +7761,15 @@ v8::SharedArrayBuffer::Contents v8::SharedArrayBuffer::Externalize() { v8::SharedArrayBuffer::Contents::Contents( void* data, size_t byte_length, void* allocation_base, size_t allocation_length, Allocator::AllocationMode allocation_mode, - DeleterCallback deleter, void* deleter_data) + DeleterCallback deleter, void* deleter_data, bool is_growable) : data_(data), byte_length_(byte_length), allocation_base_(allocation_base), allocation_length_(allocation_length), allocation_mode_(allocation_mode), deleter_(deleter), - deleter_data_(deleter_data) { + deleter_data_(deleter_data), + is_growable_(is_growable) { DCHECK_LE(allocation_base_, data_); DCHECK_LE(byte_length_, allocation_length_); } @@ -7765,7 +7787,8 @@ v8::SharedArrayBuffer::Contents v8::SharedArrayBuffer::GetContents() { : reinterpret_cast(ArrayBufferDeleter), self->is_wasm_memory() ? static_cast(self->GetIsolate()->wasm_engine()) - : static_cast(self->GetIsolate()->array_buffer_allocator())); + : static_cast(self->GetIsolate()->array_buffer_allocator()), + self->is_growable()); return contents; } @@ -7795,22 +7818,19 @@ Local v8::SharedArrayBuffer::New(Isolate* isolate, Local v8::SharedArrayBuffer::New( Isolate* isolate, void* data, size_t byte_length, ArrayBufferCreationMode mode) { - CHECK(i::FLAG_harmony_sharedarraybuffer); - // Embedders must guarantee that the external backing store is valid. - CHECK(byte_length == 0 || data != nullptr); - i::Isolate* i_isolate = reinterpret_cast(isolate); - LOG_API(i_isolate, SharedArrayBuffer, New); - ENTER_V8_NO_SCRIPT_NO_EXCEPTION(i_isolate); - i::Handle obj = - i_isolate->factory()->NewJSArrayBuffer(i::SharedFlag::kShared); - bool is_wasm_memory = - i_isolate->wasm_engine()->memory_tracker()->IsWasmMemory(data); - i::JSArrayBuffer::Setup(obj, i_isolate, - mode == ArrayBufferCreationMode::kExternalized, data, - byte_length, i::SharedFlag::kShared, is_wasm_memory); - return Utils::ToLocalShared(obj); + i::Handle buffer = + SetupSharedArrayBuffer(isolate, data, byte_length, mode); + return Utils::ToLocalShared(buffer); } +Local v8::SharedArrayBuffer::New( + Isolate* isolate, const SharedArrayBuffer::Contents& contents, + ArrayBufferCreationMode mode) { + i::Handle buffer = SetupSharedArrayBuffer( + isolate, contents.Data(), contents.ByteLength(), mode); + buffer->set_is_growable(contents.IsGrowable()); + return Utils::ToLocalShared(buffer); +} Local v8::Symbol::New(Isolate* isolate, Local name) { i::Isolate* i_isolate = reinterpret_cast(isolate); diff --git a/src/d8.cc b/src/d8.cc index 62ac87fed8..9897cc8b57 100644 --- a/src/d8.cc +++ b/src/d8.cc @@ -3244,10 +3244,9 @@ class Deserializer : public ValueDeserializer::Delegate { Isolate* isolate, uint32_t clone_id) override { DCHECK_NOT_NULL(data_); if (clone_id < data_->shared_array_buffer_contents().size()) { - SharedArrayBuffer::Contents contents = + const SharedArrayBuffer::Contents contents = data_->shared_array_buffer_contents().at(clone_id); - return SharedArrayBuffer::New(isolate_, contents.Data(), - contents.ByteLength()); + return SharedArrayBuffer::New(isolate_, contents); } return MaybeLocal(); } diff --git a/src/wasm/wasm-objects.cc b/src/wasm/wasm-objects.cc index b752b79656..cee536141a 100644 --- a/src/wasm/wasm-objects.cc +++ b/src/wasm/wasm-objects.cc @@ -1055,7 +1055,9 @@ int32_t WasmMemoryObject::Grow(Isolate* isolate, uint32_t pages) { TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.wasm"), "GrowMemory"); Handle old_buffer(memory_object->array_buffer(), isolate); - if (!old_buffer->is_growable()) return -1; + // TODO(gdeepti): Remove check for is_shared when Growing Shared memory + // is supported. + if (!old_buffer->is_growable() || old_buffer->is_shared()) return -1; // Checks for maximum memory size, compute new size. uint32_t maximum_pages = wasm::max_mem_pages();