[wasm] Fix DetachArrayBuffer for WebAssembly.Memory on grow

DetachArrayBuffer makes incorrect assumptions about the state of the ArrayBuffer. It assumes that that the ArrayBuffer is internal to wasm unless guard pages are enabled, this is not the case as the ArrayBuffer can be externalized outside of wasm, in this case through gin.

BUG=chromium:700384

Review-Url: https://codereview.chromium.org/2754153002
Cr-Commit-Position: refs/heads/master@{#43880}
This commit is contained in:
gdeepti 2017-03-16 18:12:07 -07:00 committed by Commit bot
parent 3e1e90dec2
commit 1fce7d604a
2 changed files with 70 additions and 4 deletions

View File

@ -2211,23 +2211,24 @@ void UncheckedUpdateInstanceMemory(Isolate* isolate,
void DetachArrayBuffer(Isolate* isolate, Handle<JSArrayBuffer> buffer) {
const bool has_guard_regions =
(!buffer.is_null() && buffer->has_guard_region());
const bool is_external = buffer->is_external();
void* backing_store = buffer->backing_store();
if (backing_store != nullptr) {
DCHECK(!buffer->is_neuterable());
int64_t byte_length = NumberToSize(buffer->byte_length());
buffer->set_is_neuterable(true);
if (!has_guard_regions) {
if (!has_guard_regions && !is_external) {
buffer->set_is_external(true);
isolate->heap()->UnregisterArrayBuffer(*buffer);
}
buffer->Neuter();
if (!has_guard_regions) {
isolate->array_buffer_allocator()->Free(backing_store, byte_length);
} else {
if (has_guard_regions) {
base::OS::Free(backing_store, RoundUp(i::wasm::kWasmMaxHeapOffset,
base::OS::CommitPageSize()));
reinterpret_cast<v8::Isolate*>(isolate)
->AdjustAmountOfExternalAllocatedMemory(-byte_length);
} else if (!has_guard_regions && !is_external) {
isolate->array_buffer_allocator()->Free(backing_store, byte_length);
}
}
}

View File

@ -974,3 +974,68 @@ TEST(MemoryWithOOBEmptyDataSegment) {
}
Cleanup();
}
TEST(Run_WasmModule_Buffer_Externalized_GrowMem) {
{
Isolate* isolate = CcTest::InitIsolateOnce();
HandleScope scope(isolate);
// Initial memory size = 16 + GrowWebAssemblyMemory(4) + GrowMemory(6)
static const int kExpectedValue = 26;
TestSignatures sigs;
v8::internal::AccountingAllocator allocator;
Zone zone(&allocator, ZONE_NAME);
WasmModuleBuilder* builder = new (&zone) WasmModuleBuilder(&zone);
WasmFunctionBuilder* f = builder->AddFunction(sigs.i_v());
ExportAsMain(f);
byte code[] = {WASM_GROW_MEMORY(WASM_I32V_1(6)), WASM_DROP,
WASM_MEMORY_SIZE};
EMIT_CODE_WITH_END(f, code);
ZoneBuffer buffer(&zone);
builder->WriteTo(buffer);
testing::SetupIsolateForWasmModule(isolate);
ErrorThrower thrower(isolate, "Test");
const Handle<WasmInstanceObject> instance =
testing::CompileInstantiateWasmModuleForTesting(
isolate, &thrower, buffer.begin(), buffer.end(),
ModuleOrigin::kWasmOrigin);
CHECK(!instance.is_null());
MaybeHandle<JSArrayBuffer> maybe_memory =
GetInstanceMemory(isolate, instance);
Handle<JSArrayBuffer> memory = maybe_memory.ToHandleChecked();
// Fake the Embedder flow by creating a memory object, externalize and grow.
Handle<WasmMemoryObject> mem_obj =
WasmMemoryObject::New(isolate, memory, 100);
// TODO(eholk): Skipping calls to externalize when guard pages are enabled
// for now. This will have to be dealt with when turning on guard pages as
// currently gin assumes that it can take ownership of the ArrayBuffer.
// Potential for crashes as this might lead to externalizing an already
// externalized buffer.
if (!memory->has_guard_region()) v8::Utils::ToLocal(memory)->Externalize();
void* backing_store = memory->backing_store();
uint64_t byte_length = NumberToSize(memory->byte_length());
uint32_t result = GrowWebAssemblyMemory(isolate, mem_obj, 4);
CHECK_EQ(16, result);
if (!memory->has_guard_region()) {
isolate->array_buffer_allocator()->Free(backing_store, byte_length);
}
memory = handle(mem_obj->buffer());
byte_length = NumberToSize(memory->byte_length());
instance->set_memory_buffer(*memory);
// Externalize should make no difference without the JS API as in this case
// the buffer is not detached.
if (!memory->has_guard_region()) v8::Utils::ToLocal(memory)->Externalize();
result = testing::RunWasmModuleForTesting(isolate, instance, 0, nullptr,
ModuleOrigin::kWasmOrigin);
CHECK_EQ(kExpectedValue, result);
// Free the buffer as the tracker does not know about it.
if (!memory->has_guard_region()) {
isolate->array_buffer_allocator()->Free(
memory->backing_store(), NumberToSize(memory->byte_length()));
}
}
Cleanup();
}