Commit b0077b3b authored by Deepti Gandluri's avatar Deepti Gandluri Committed by Commit Bot

[wasm] Move is_growable from JSArrayBuffer object to AllocationData

Some state related to WasmMemories is cached on the JSArrayBuffer
object (is_growable, is_wasm_memory). The problem with this is in
some PostMessage flows, this information can get lost depending on
how JSArrayBuffers are deserialized. In this particular case when
the WasmMemory is postMessaged, it goes through the Blink
DedicatedWorkerMessagingProxy::PostMessageToWorkerGlobalScope flow,
which reconstructs the ArrayBuffer from the backing store, and size,
and loses the is_growable flag, leading to a failure to grow memory.

Moving the is_growable flag so that AllocationData can be the source
of truth for all wasm memory state, and is consistently preserved
across PostMessage.

Change-Id: I775f66ddeff68b8cafc18b75ca5460dfb0343c8b
Bug: v8:9065
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1549789
Commit-Queue: Deepti Gandluri <gdeepti@chromium.org>
Reviewed-by: 's avatarBen Titzer <titzer@chromium.org>
Reviewed-by: 's avatarAdam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60641}
parent 4a68b29c
......@@ -5164,8 +5164,7 @@ class V8_EXPORT SharedArrayBuffer : public Object {
allocation_length_(0),
allocation_mode_(Allocator::AllocationMode::kNormal),
deleter_(nullptr),
deleter_data_(nullptr),
is_growable_(false) {}
deleter_data_(nullptr) {}
void* AllocationBase() const { return allocation_base_; }
size_t AllocationLength() const { return allocation_length_; }
......@@ -5177,13 +5176,12 @@ 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, bool is_growable);
void* deleter_data);
void* data_;
size_t byte_length_;
......@@ -5192,7 +5190,6 @@ class V8_EXPORT SharedArrayBuffer : public Object {
Allocator::AllocationMode allocation_mode_;
DeleterCallback deleter_;
void* deleter_data_;
bool is_growable_;
friend class SharedArrayBuffer;
};
......@@ -5224,9 +5221,11 @@ class V8_EXPORT SharedArrayBuffer : public Object {
* Create a new SharedArrayBuffer over an existing memory block. Propagate
* flags to indicate whether the underlying buffer can be grown.
*/
static Local<SharedArrayBuffer> New(
Isolate* isolate, const SharedArrayBuffer::Contents&,
ArrayBufferCreationMode mode = ArrayBufferCreationMode::kExternalized);
V8_DEPRECATED("Use New method with data, and byte_length instead.",
static Local<SharedArrayBuffer> New(
Isolate* isolate, const SharedArrayBuffer::Contents&,
ArrayBufferCreationMode mode =
ArrayBufferCreationMode::kExternalized));
/**
* Returns true if SharedArrayBuffer is externalized, that is, does not
......
......@@ -7737,15 +7737,14 @@ 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, bool is_growable)
DeleterCallback deleter, void* deleter_data)
: data_(data),
byte_length_(byte_length),
allocation_base_(allocation_base),
allocation_length_(allocation_length),
allocation_mode_(allocation_mode),
deleter_(deleter),
deleter_data_(deleter_data),
is_growable_(is_growable) {
deleter_data_(deleter_data) {
DCHECK_LE(allocation_base_, data_);
DCHECK_LE(byte_length_, allocation_length_);
}
......@@ -7763,8 +7762,7 @@ v8::SharedArrayBuffer::Contents v8::SharedArrayBuffer::GetContents() {
: reinterpret_cast<Contents::DeleterCallback>(ArrayBufferDeleter),
self->is_wasm_memory()
? static_cast<void*>(self->GetIsolate()->wasm_engine())
: static_cast<void*>(self->GetIsolate()->array_buffer_allocator()),
self->is_growable());
: static_cast<void*>(self->GetIsolate()->array_buffer_allocator()));
return contents;
}
......@@ -7804,7 +7802,6 @@ Local<SharedArrayBuffer> v8::SharedArrayBuffer::New(
ArrayBufferCreationMode mode) {
i::Handle<i::JSArrayBuffer> buffer = SetupSharedArrayBuffer(
isolate, contents.Data(), contents.ByteLength(), mode);
buffer->set_is_growable(contents.IsGrowable());
return Utils::ToLocalShared(buffer);
}
......
......@@ -355,11 +355,11 @@ MaybeHandle<Object> AsmJs::InstantiateAsmWasm(Isolate* isolate,
instantiate_timer.Start();
Handle<HeapNumber> uses_bitset(wasm_data->uses_bitset(), isolate);
Handle<Script> script(Script::cast(shared->script()), isolate);
const auto& wasm_engine = isolate->wasm_engine();
// Allocate the WasmModuleObject.
Handle<WasmModuleObject> module =
isolate->wasm_engine()->FinalizeTranslatedAsmJs(isolate, wasm_data,
script);
wasm_engine->FinalizeTranslatedAsmJs(isolate, wasm_data, script);
// TODO(mstarzinger): The position currently points to the module definition
// but should instead point to the instantiation site (more intuitive).
......@@ -387,7 +387,7 @@ MaybeHandle<Object> AsmJs::InstantiateAsmWasm(Isolate* isolate,
ReportInstantiationFailure(script, position, "Requires heap buffer");
return MaybeHandle<Object>();
}
memory->set_is_growable(false);
wasm_engine->memory_tracker()->MarkWasmMemoryNotGrowable(memory);
size_t size = memory->byte_length();
// Check the asm.js heap size against the valid limits.
if (!IsValidAsmjsMemorySize(size)) {
......@@ -400,8 +400,7 @@ MaybeHandle<Object> AsmJs::InstantiateAsmWasm(Isolate* isolate,
wasm::ErrorThrower thrower(isolate, "AsmJs::Instantiate");
MaybeHandle<Object> maybe_module_object =
isolate->wasm_engine()->SyncInstantiate(isolate, &thrower, module,
foreign, memory);
wasm_engine->SyncInstantiate(isolate, &thrower, module, foreign, memory);
if (maybe_module_object.is_null()) {
// An exception caused by the module start function will be set as pending
// and bypass the {ErrorThrower}, this happens in case of a stack overflow.
......
......@@ -3272,7 +3272,8 @@ class Deserializer : public ValueDeserializer::Delegate {
if (clone_id < data_->shared_array_buffer_contents().size()) {
const SharedArrayBuffer::Contents contents =
data_->shared_array_buffer_contents().at(clone_id);
return SharedArrayBuffer::New(isolate_, contents);
return SharedArrayBuffer::New(isolate_, contents.Data(),
contents.ByteLength());
}
return MaybeLocal<SharedArrayBuffer>();
}
......
......@@ -1364,7 +1364,6 @@ void JSArrayBuffer::JSArrayBufferPrint(std::ostream& os) { // NOLINT
if (was_detached()) os << "\n - detached";
if (is_shared()) os << "\n - shared";
if (is_wasm_memory()) os << "\n - is_wasm_memory";
if (is_growable()) os << "\n - growable";
JSObjectPrintBody(os, *this, !was_detached());
}
......
......@@ -109,8 +109,6 @@ BIT_FIELD_ACCESSORS(JSArrayBuffer, bit_field, was_detached,
JSArrayBuffer::WasDetachedBit)
BIT_FIELD_ACCESSORS(JSArrayBuffer, bit_field, is_shared,
JSArrayBuffer::IsSharedBit)
BIT_FIELD_ACCESSORS(JSArrayBuffer, bit_field, is_growable,
JSArrayBuffer::IsGrowableBit)
size_t JSArrayBufferView::byte_offset() const {
return READ_UINTPTR_FIELD(*this, kByteOffsetOffset);
......
......@@ -71,9 +71,6 @@ class JSArrayBuffer : public JSObject {
// [is_shared]: tells whether this is an ArrayBuffer or a SharedArrayBuffer.
DECL_BOOLEAN_ACCESSORS(is_shared)
// [is_growable]: indicates whether it's possible to grow this buffer.
DECL_BOOLEAN_ACCESSORS(is_growable)
// [is_wasm_memory]: whether the buffer is tracked by the WasmMemoryTracker.
DECL_BOOLEAN_ACCESSORS(is_wasm_memory)
......
......@@ -1576,10 +1576,6 @@ void WebAssemblyMemoryGrow(const v8::FunctionCallbackInfo<v8::Value>& args) {
max_size64 = i::wasm::max_mem_pages();
}
i::Handle<i::JSArrayBuffer> old_buffer(receiver->array_buffer(), i_isolate);
if (!old_buffer->is_growable()) {
thrower.RangeError("This memory cannot be grown");
return;
}
DCHECK_LE(max_size64, std::numeric_limits<uint32_t>::max());
......
......@@ -284,6 +284,22 @@ bool WasmMemoryTracker::HasFullGuardRegions(const void* buffer_start) {
return start + kWasmMaxHeapOffset < limit;
}
void WasmMemoryTracker::MarkWasmMemoryNotGrowable(
Handle<JSArrayBuffer> buffer) {
base::MutexGuard scope_lock(&mutex_);
const auto& allocation = allocations_.find(buffer->backing_store());
if (allocation == allocations_.end()) return;
allocation->second.is_growable = false;
}
bool WasmMemoryTracker::IsWasmMemoryGrowable(Handle<JSArrayBuffer> buffer) {
base::MutexGuard scope_lock(&mutex_);
if (buffer->backing_store() == nullptr) return true;
const auto& allocation = allocations_.find(buffer->backing_store());
if (allocation == allocations_.end()) return false;
return allocation->second.is_growable;
}
bool WasmMemoryTracker::FreeMemoryIfIsWasmMemory(Isolate* isolate,
const void* buffer_start) {
base::MutexGuard scope_lock(&mutex_);
......@@ -580,7 +596,6 @@ Handle<JSArrayBuffer> SetupArrayBuffer(Isolate* isolate, void* backing_store,
JSArrayBuffer::Setup(buffer, isolate, is_external, backing_store, size,
shared, is_wasm_memory);
buffer->set_is_detachable(false);
buffer->set_is_growable(true);
return buffer;
}
......
......@@ -56,6 +56,9 @@ class WasmMemoryTracker {
void* buffer_start = nullptr;
size_t buffer_length = 0;
bool is_shared = false;
// Wasm memories are growable by default, this will be false only when
// shared with an asmjs module.
bool is_growable = true;
// Track Wasm Memory instances across isolates, this is populated on
// PostMessage using persistent handles for memory objects.
......@@ -116,6 +119,10 @@ class WasmMemoryTracker {
// free the buffer manually.
bool FreeMemoryIfIsWasmMemory(Isolate* isolate, const void* buffer_start);
void MarkWasmMemoryNotGrowable(Handle<JSArrayBuffer> buffer);
bool IsWasmMemoryGrowable(Handle<JSArrayBuffer> buffer);
// When WebAssembly.Memory is transferred over PostMessage, register the
// allocation as shared and track the memory objects that will need
// updating if memory is resized.
......
......@@ -1255,7 +1255,8 @@ int32_t WasmMemoryObject::Grow(Isolate* isolate,
uint32_t pages) {
TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.wasm"), "GrowMemory");
Handle<JSArrayBuffer> old_buffer(memory_object->array_buffer(), isolate);
if (!old_buffer->is_growable()) return -1;
auto memory_tracker = isolate->wasm_engine()->memory_tracker();
if (!memory_tracker->IsWasmMemoryGrowable(old_buffer)) return -1;
// Checks for maximum memory size, compute new size.
uint32_t maximum_pages = wasm::max_mem_pages();
......@@ -1284,16 +1285,13 @@ int32_t WasmMemoryObject::Grow(Isolate* isolate,
if (!AdjustBufferPermissions(isolate, old_buffer, new_size)) {
return -1;
}
wasm::WasmMemoryTracker* const memory_tracker =
isolate->wasm_engine()->memory_tracker();
void* backing_store = old_buffer->backing_store();
if (memory_tracker->IsWasmSharedMemory(backing_store)) {
// This memory is shared between different isolates.
DCHECK(old_buffer->is_shared());
// Update pending grow state, and trigger a grow interrupt on all the
// isolates that share this buffer.
isolate->wasm_engine()->memory_tracker()->SetPendingUpdateOnGrow(
old_buffer, new_size);
memory_tracker->SetPendingUpdateOnGrow(old_buffer, new_size);
// Handle interrupts for this isolate so that the instances with this
// isolate are updated.
isolate->stack_guard()->HandleInterrupts();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment