Skip to content

Commit b0077b3

Browse files
dtigCommit Bot
dtig
authored and
Commit Bot
committed
[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 <[email protected]> Reviewed-by: Ben Titzer <[email protected]> Reviewed-by: Adam Klein <[email protected]> Cr-Commit-Position: refs/heads/master@{#60641}
1 parent 4a68b29 commit b0077b3

11 files changed

Lines changed: 42 additions & 36 deletions

File tree

‎include/v8.h‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5164,8 +5164,7 @@ class V8_EXPORT SharedArrayBuffer : public Object {
51645164
allocation_length_(0),
51655165
allocation_mode_(Allocator::AllocationMode::kNormal),
51665166
deleter_(nullptr),
5167-
deleter_data_(nullptr),
5168-
is_growable_(false) {}
5167+
deleter_data_(nullptr) {}
51695168

51705169
void* AllocationBase() const { return allocation_base_; }
51715170
size_t AllocationLength() const { return allocation_length_; }
@@ -5177,13 +5176,12 @@ class V8_EXPORT SharedArrayBuffer : public Object {
51775176
size_t ByteLength() const { return byte_length_; }
51785177
DeleterCallback Deleter() const { return deleter_; }
51795178
void* DeleterData() const { return deleter_data_; }
5180-
bool IsGrowable() const { return is_growable_; }
51815179

51825180
private:
51835181
Contents(void* data, size_t byte_length, void* allocation_base,
51845182
size_t allocation_length,
51855183
Allocator::AllocationMode allocation_mode, DeleterCallback deleter,
5186-
void* deleter_data, bool is_growable);
5184+
void* deleter_data);
51875185

51885186
void* data_;
51895187
size_t byte_length_;
@@ -5192,7 +5190,6 @@ class V8_EXPORT SharedArrayBuffer : public Object {
51925190
Allocator::AllocationMode allocation_mode_;
51935191
DeleterCallback deleter_;
51945192
void* deleter_data_;
5195-
bool is_growable_;
51965193

51975194
friend class SharedArrayBuffer;
51985195
};
@@ -5224,9 +5221,11 @@ class V8_EXPORT SharedArrayBuffer : public Object {
52245221
* Create a new SharedArrayBuffer over an existing memory block. Propagate
52255222
* flags to indicate whether the underlying buffer can be grown.
52265223
*/
5227-
static Local<SharedArrayBuffer> New(
5228-
Isolate* isolate, const SharedArrayBuffer::Contents&,
5229-
ArrayBufferCreationMode mode = ArrayBufferCreationMode::kExternalized);
5224+
V8_DEPRECATED("Use New method with data, and byte_length instead.",
5225+
static Local<SharedArrayBuffer> New(
5226+
Isolate* isolate, const SharedArrayBuffer::Contents&,
5227+
ArrayBufferCreationMode mode =
5228+
ArrayBufferCreationMode::kExternalized));
52305229

52315230
/**
52325231
* Returns true if SharedArrayBuffer is externalized, that is, does not

‎src/api.cc‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7737,15 +7737,14 @@ v8::SharedArrayBuffer::Contents v8::SharedArrayBuffer::Externalize() {
77377737
v8::SharedArrayBuffer::Contents::Contents(
77387738
void* data, size_t byte_length, void* allocation_base,
77397739
size_t allocation_length, Allocator::AllocationMode allocation_mode,
7740-
DeleterCallback deleter, void* deleter_data, bool is_growable)
7740+
DeleterCallback deleter, void* deleter_data)
77417741
: data_(data),
77427742
byte_length_(byte_length),
77437743
allocation_base_(allocation_base),
77447744
allocation_length_(allocation_length),
77457745
allocation_mode_(allocation_mode),
77467746
deleter_(deleter),
7747-
deleter_data_(deleter_data),
7748-
is_growable_(is_growable) {
7747+
deleter_data_(deleter_data) {
77497748
DCHECK_LE(allocation_base_, data_);
77507749
DCHECK_LE(byte_length_, allocation_length_);
77517750
}
@@ -7763,8 +7762,7 @@ v8::SharedArrayBuffer::Contents v8::SharedArrayBuffer::GetContents() {
77637762
: reinterpret_cast<Contents::DeleterCallback>(ArrayBufferDeleter),
77647763
self->is_wasm_memory()
77657764
? static_cast<void*>(self->GetIsolate()->wasm_engine())
7766-
: static_cast<void*>(self->GetIsolate()->array_buffer_allocator()),
7767-
self->is_growable());
7765+
: static_cast<void*>(self->GetIsolate()->array_buffer_allocator()));
77687766
return contents;
77697767
}
77707768

@@ -7804,7 +7802,6 @@ Local<SharedArrayBuffer> v8::SharedArrayBuffer::New(
78047802
ArrayBufferCreationMode mode) {
78057803
i::Handle<i::JSArrayBuffer> buffer = SetupSharedArrayBuffer(
78067804
isolate, contents.Data(), contents.ByteLength(), mode);
7807-
buffer->set_is_growable(contents.IsGrowable());
78087805
return Utils::ToLocalShared(buffer);
78097806
}
78107807

‎src/asmjs/asm-js.cc‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -355,11 +355,11 @@ MaybeHandle<Object> AsmJs::InstantiateAsmWasm(Isolate* isolate,
355355
instantiate_timer.Start();
356356
Handle<HeapNumber> uses_bitset(wasm_data->uses_bitset(), isolate);
357357
Handle<Script> script(Script::cast(shared->script()), isolate);
358+
const auto& wasm_engine = isolate->wasm_engine();
358359

359360
// Allocate the WasmModuleObject.
360361
Handle<WasmModuleObject> module =
361-
isolate->wasm_engine()->FinalizeTranslatedAsmJs(isolate, wasm_data,
362-
script);
362+
wasm_engine->FinalizeTranslatedAsmJs(isolate, wasm_data, script);
363363

364364
// TODO(mstarzinger): The position currently points to the module definition
365365
// but should instead point to the instantiation site (more intuitive).
@@ -387,7 +387,7 @@ MaybeHandle<Object> AsmJs::InstantiateAsmWasm(Isolate* isolate,
387387
ReportInstantiationFailure(script, position, "Requires heap buffer");
388388
return MaybeHandle<Object>();
389389
}
390-
memory->set_is_growable(false);
390+
wasm_engine->memory_tracker()->MarkWasmMemoryNotGrowable(memory);
391391
size_t size = memory->byte_length();
392392
// Check the asm.js heap size against the valid limits.
393393
if (!IsValidAsmjsMemorySize(size)) {
@@ -400,8 +400,7 @@ MaybeHandle<Object> AsmJs::InstantiateAsmWasm(Isolate* isolate,
400400

401401
wasm::ErrorThrower thrower(isolate, "AsmJs::Instantiate");
402402
MaybeHandle<Object> maybe_module_object =
403-
isolate->wasm_engine()->SyncInstantiate(isolate, &thrower, module,
404-
foreign, memory);
403+
wasm_engine->SyncInstantiate(isolate, &thrower, module, foreign, memory);
405404
if (maybe_module_object.is_null()) {
406405
// An exception caused by the module start function will be set as pending
407406
// and bypass the {ErrorThrower}, this happens in case of a stack overflow.

‎src/d8.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3272,7 +3272,8 @@ class Deserializer : public ValueDeserializer::Delegate {
32723272
if (clone_id < data_->shared_array_buffer_contents().size()) {
32733273
const SharedArrayBuffer::Contents contents =
32743274
data_->shared_array_buffer_contents().at(clone_id);
3275-
return SharedArrayBuffer::New(isolate_, contents);
3275+
return SharedArrayBuffer::New(isolate_, contents.Data(),
3276+
contents.ByteLength());
32763277
}
32773278
return MaybeLocal<SharedArrayBuffer>();
32783279
}

‎src/objects-printer.cc‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1364,7 +1364,6 @@ void JSArrayBuffer::JSArrayBufferPrint(std::ostream& os) { // NOLINT
13641364
if (was_detached()) os << "\n - detached";
13651365
if (is_shared()) os << "\n - shared";
13661366
if (is_wasm_memory()) os << "\n - is_wasm_memory";
1367-
if (is_growable()) os << "\n - growable";
13681367
JSObjectPrintBody(os, *this, !was_detached());
13691368
}
13701369

‎src/objects/js-array-buffer-inl.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,6 @@ BIT_FIELD_ACCESSORS(JSArrayBuffer, bit_field, was_detached,
109109
JSArrayBuffer::WasDetachedBit)
110110
BIT_FIELD_ACCESSORS(JSArrayBuffer, bit_field, is_shared,
111111
JSArrayBuffer::IsSharedBit)
112-
BIT_FIELD_ACCESSORS(JSArrayBuffer, bit_field, is_growable,
113-
JSArrayBuffer::IsGrowableBit)
114112

115113
size_t JSArrayBufferView::byte_offset() const {
116114
return READ_UINTPTR_FIELD(*this, kByteOffsetOffset);

‎src/objects/js-array-buffer.h‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,6 @@ class JSArrayBuffer : public JSObject {
7171
// [is_shared]: tells whether this is an ArrayBuffer or a SharedArrayBuffer.
7272
DECL_BOOLEAN_ACCESSORS(is_shared)
7373

74-
// [is_growable]: indicates whether it's possible to grow this buffer.
75-
DECL_BOOLEAN_ACCESSORS(is_growable)
76-
7774
// [is_wasm_memory]: whether the buffer is tracked by the WasmMemoryTracker.
7875
DECL_BOOLEAN_ACCESSORS(is_wasm_memory)
7976

‎src/wasm/wasm-js.cc‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1576,10 +1576,6 @@ void WebAssemblyMemoryGrow(const v8::FunctionCallbackInfo<v8::Value>& args) {
15761576
max_size64 = i::wasm::max_mem_pages();
15771577
}
15781578
i::Handle<i::JSArrayBuffer> old_buffer(receiver->array_buffer(), i_isolate);
1579-
if (!old_buffer->is_growable()) {
1580-
thrower.RangeError("This memory cannot be grown");
1581-
return;
1582-
}
15831579

15841580
DCHECK_LE(max_size64, std::numeric_limits<uint32_t>::max());
15851581

‎src/wasm/wasm-memory.cc‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,22 @@ bool WasmMemoryTracker::HasFullGuardRegions(const void* buffer_start) {
284284
return start + kWasmMaxHeapOffset < limit;
285285
}
286286

287+
void WasmMemoryTracker::MarkWasmMemoryNotGrowable(
288+
Handle<JSArrayBuffer> buffer) {
289+
base::MutexGuard scope_lock(&mutex_);
290+
const auto& allocation = allocations_.find(buffer->backing_store());
291+
if (allocation == allocations_.end()) return;
292+
allocation->second.is_growable = false;
293+
}
294+
295+
bool WasmMemoryTracker::IsWasmMemoryGrowable(Handle<JSArrayBuffer> buffer) {
296+
base::MutexGuard scope_lock(&mutex_);
297+
if (buffer->backing_store() == nullptr) return true;
298+
const auto& allocation = allocations_.find(buffer->backing_store());
299+
if (allocation == allocations_.end()) return false;
300+
return allocation->second.is_growable;
301+
}
302+
287303
bool WasmMemoryTracker::FreeMemoryIfIsWasmMemory(Isolate* isolate,
288304
const void* buffer_start) {
289305
base::MutexGuard scope_lock(&mutex_);
@@ -580,7 +596,6 @@ Handle<JSArrayBuffer> SetupArrayBuffer(Isolate* isolate, void* backing_store,
580596
JSArrayBuffer::Setup(buffer, isolate, is_external, backing_store, size,
581597
shared, is_wasm_memory);
582598
buffer->set_is_detachable(false);
583-
buffer->set_is_growable(true);
584599
return buffer;
585600
}
586601

‎src/wasm/wasm-memory.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ class WasmMemoryTracker {
5656
void* buffer_start = nullptr;
5757
size_t buffer_length = 0;
5858
bool is_shared = false;
59+
// Wasm memories are growable by default, this will be false only when
60+
// shared with an asmjs module.
61+
bool is_growable = true;
5962

6063
// Track Wasm Memory instances across isolates, this is populated on
6164
// PostMessage using persistent handles for memory objects.
@@ -116,6 +119,10 @@ class WasmMemoryTracker {
116119
// free the buffer manually.
117120
bool FreeMemoryIfIsWasmMemory(Isolate* isolate, const void* buffer_start);
118121

122+
void MarkWasmMemoryNotGrowable(Handle<JSArrayBuffer> buffer);
123+
124+
bool IsWasmMemoryGrowable(Handle<JSArrayBuffer> buffer);
125+
119126
// When WebAssembly.Memory is transferred over PostMessage, register the
120127
// allocation as shared and track the memory objects that will need
121128
// updating if memory is resized.

0 commit comments

Comments
 (0)