[wasm] Always enable guard regions on 64-bit platforms

This change makes full 8 GiB guard regions always enabled on 64-bit
platforms.

Additionally, since all Wasm memory allocation paths have some form of
guard regions, this removes and simplifies most of the logic around
whether to enable guard regions.

This is a reland of https://crrev.com/c/985142.

Bug: v8:7619
Change-Id: I8bf1f86d6f89fd0bb2144431c7628f15a6b00ba0
Reviewed-on: https://chromium-review.googlesource.com/996466
Reviewed-by: Brad Nelson <bradnelson@chromium.org>
Commit-Queue: Eric Holk <eholk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52412}
This commit is contained in:
Eric Holk 2018-04-05 10:09:46 -07:00 committed by Commit Bot
parent 8cb01d3d90
commit ad221d144a
7 changed files with 44 additions and 75 deletions

View File

@ -1651,9 +1651,19 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {
WasmContext* wasm_context = instance->wasm_context()->get();
uint32_t globals_size = module_->globals_size;
if (globals_size > 0) {
constexpr bool enable_guard_regions = false;
if (!NewArrayBuffer(isolate_, globals_size, enable_guard_regions)
.ToHandle(&globals_)) {
void* backing_store =
isolate_->array_buffer_allocator()->Allocate(globals_size);
if (backing_store == nullptr) {
thrower_->RangeError("Out of memory: wasm globals");
return {};
}
globals_ =
isolate_->factory()->NewJSArrayBuffer(SharedFlag::kNotShared, TENURED);
constexpr bool is_external = false;
constexpr bool is_wasm_memory = false;
JSArrayBuffer::Setup(globals_, isolate_, is_external, backing_store,
globals_size, SharedFlag::kNotShared, is_wasm_memory);
if (globals_.is_null()) {
thrower_->RangeError("Out of memory: wasm globals");
return {};
}
@ -2348,14 +2358,12 @@ Handle<JSArrayBuffer> InstanceBuilder::AllocateMemory(uint32_t num_pages) {
thrower_->RangeError("Out of memory: wasm memory too large");
return Handle<JSArrayBuffer>::null();
}
const bool enable_guard_regions = use_trap_handler();
const bool is_shared_memory =
module_->has_shared_memory && i::FLAG_experimental_wasm_threads;
i::SharedFlag shared_flag =
is_shared_memory ? i::SharedFlag::kShared : i::SharedFlag::kNotShared;
Handle<JSArrayBuffer> mem_buffer;
if (!NewArrayBuffer(isolate_, num_pages * kWasmPageSize, enable_guard_regions,
shared_flag)
if (!NewArrayBuffer(isolate_, num_pages * kWasmPageSize, shared_flag)
.ToHandle(&mem_buffer)) {
thrower_->RangeError("Out of memory: wasm memory");
}

View File

@ -652,15 +652,12 @@ void WebAssemblyMemory(const v8::FunctionCallbackInfo<v8::Value>& args) {
}
}
size_t size = static_cast<size_t>(i::wasm::kWasmPageSize) *
static_cast<size_t>(initial);
const bool enable_guard_regions =
internal::trap_handler::IsTrapHandlerEnabled();
i::SharedFlag shared_flag =
is_shared_memory ? i::SharedFlag::kShared : i::SharedFlag::kNotShared;
i::Handle<i::JSArrayBuffer> buffer;
if (!i::wasm::NewArrayBuffer(i_isolate, size, enable_guard_regions,
shared_flag)
size_t size = static_cast<size_t>(i::wasm::kWasmPageSize) *
static_cast<size_t>(initial);
if (!i::wasm::NewArrayBuffer(i_isolate, size, shared_flag)
.ToHandle(&buffer)) {
thrower.RangeError("could not allocate memory");
return;

View File

@ -14,20 +14,18 @@ namespace wasm {
namespace {
void* TryAllocateBackingStore(WasmMemoryTracker* memory_tracker, Heap* heap,
size_t size, bool require_guard_regions,
void** allocation_base,
size_t size, void** allocation_base,
size_t* allocation_length) {
#if V8_TARGET_ARCH_32_BIT
DCHECK(!require_guard_regions);
#endif
// We always allocate the largest possible offset into the heap, so the
// addressable memory after the guard page can be made inaccessible.
// We always allocate the largest possible offset into the heap, so the
// addressable memory after the guard page can be made inaccessible.
#if V8_TARGET_ARCH_64_BIT
*allocation_length = RoundUp(kWasmMaxHeapOffset, CommitPageSize());
#else
*allocation_length =
require_guard_regions
? RoundUp(kWasmMaxHeapOffset, CommitPageSize())
: RoundUp(
base::bits::RoundUpToPowerOfTwo32(static_cast<uint32_t>(size)),
kWasmPageSize);
RoundUp(base::bits::RoundUpToPowerOfTwo32(static_cast<uint32_t>(size)),
kWasmPageSize);
#endif
DCHECK_GE(*allocation_length, size);
DCHECK_GE(*allocation_length, kWasmPageSize);
@ -181,12 +179,11 @@ void* WasmMemoryTracker::GetEmptyBackingStore(void** allocation_base,
Heap* heap) {
if (empty_backing_store_.allocation_base == nullptr) {
constexpr size_t buffer_length = 0;
const bool require_guard_regions = trap_handler::IsTrapHandlerEnabled();
void* local_allocation_base;
size_t local_allocation_length;
void* buffer_start = TryAllocateBackingStore(
this, heap, buffer_length, require_guard_regions,
&local_allocation_base, &local_allocation_length);
void* buffer_start = TryAllocateBackingStore(this, heap, buffer_length,
&local_allocation_base,
&local_allocation_length);
empty_backing_store_ =
AllocationData(local_allocation_base, local_allocation_length,
@ -232,7 +229,6 @@ Handle<JSArrayBuffer> SetupArrayBuffer(Isolate* isolate, void* backing_store,
}
MaybeHandle<JSArrayBuffer> NewArrayBuffer(Isolate* isolate, size_t size,
bool require_guard_regions,
SharedFlag shared) {
// Check against kMaxInt, since the byte length is stored as int in the
// JSArrayBuffer. Note that wasm_max_mem_pages can be raised from the command
@ -253,10 +249,10 @@ MaybeHandle<JSArrayBuffer> NewArrayBuffer(Isolate* isolate, size_t size,
? memory_tracker->GetEmptyBackingStore(
&allocation_base, &allocation_length, isolate->heap())
: TryAllocateBackingStore(memory_tracker, isolate->heap(), size,
require_guard_regions, &allocation_base,
&allocation_length);
if (size > 0 && memory == nullptr) return {};
&allocation_base, &allocation_length);
if (memory == nullptr) {
return {};
}
#if DEBUG
// Double check the API allocator actually zero-initialized the memory.

View File

@ -114,8 +114,7 @@ class WasmMemoryTracker {
};
MaybeHandle<JSArrayBuffer> NewArrayBuffer(
Isolate*, size_t size, bool require_guard_regions,
SharedFlag shared = SharedFlag::kNotShared);
Isolate*, size_t size, SharedFlag shared = SharedFlag::kNotShared);
Handle<JSArrayBuffer> SetupArrayBuffer(
Isolate*, void* backing_store, size_t size, bool is_external,

View File

@ -345,8 +345,7 @@ namespace {
MaybeHandle<JSArrayBuffer> GrowMemoryBuffer(Isolate* isolate,
Handle<JSArrayBuffer> old_buffer,
uint32_t pages,
uint32_t maximum_pages,
bool use_trap_handler) {
uint32_t maximum_pages) {
if (!old_buffer->is_growable()) return {};
Address old_mem_start = nullptr;
uint32_t old_size = 0;
@ -397,8 +396,7 @@ MaybeHandle<JSArrayBuffer> GrowMemoryBuffer(Isolate* isolate,
// We couldn't reuse the old backing store, so create a new one and copy the
// old contents in.
Handle<JSArrayBuffer> new_buffer;
if (!wasm::NewArrayBuffer(isolate, new_size, use_trap_handler)
.ToHandle(&new_buffer)) {
if (!wasm::NewArrayBuffer(isolate, new_size).ToHandle(&new_buffer)) {
return {};
}
if (old_size == 0) return new_buffer;
@ -502,10 +500,7 @@ int32_t WasmMemoryObject::Grow(Isolate* isolate,
maximum_pages = Min(FLAG_wasm_max_mem_pages,
static_cast<uint32_t>(memory_object->maximum_pages()));
}
// TODO(kschimpf): We need to fix this by adding a field to WasmMemoryObject
// that defines the style of memory being used.
if (!GrowMemoryBuffer(isolate, old_buffer, pages, maximum_pages,
trap_handler::IsTrapHandlerEnabled())
if (!GrowMemoryBuffer(isolate, old_buffer, pages, maximum_pages)
.ToHandle(&new_buffer)) {
return -1;
}

View File

@ -1079,15 +1079,8 @@ TEST(Run_WasmModule_Buffer_Externalized_GrowMemMemSize) {
{
Isolate* isolate = CcTest::InitIsolateOnce();
HandleScope scope(isolate);
#if V8_TARGET_ARCH_64_BIT
constexpr bool require_guard_regions = true;
#else
constexpr bool require_guard_regions = false;
#endif
Handle<JSArrayBuffer> buffer;
CHECK(
wasm::NewArrayBuffer(isolate, 16 * kWasmPageSize, require_guard_regions)
.ToHandle(&buffer));
CHECK(wasm::NewArrayBuffer(isolate, 16 * kWasmPageSize).ToHandle(&buffer));
Handle<WasmMemoryObject> mem_obj =
WasmMemoryObject::New(isolate, buffer, 100);
auto const contents = v8::Utils::ToLocal(buffer)->Externalize();
@ -1108,15 +1101,8 @@ TEST(Run_WasmModule_Buffer_Externalized_Detach) {
// https://bugs.chromium.org/p/chromium/issues/detail?id=731046
Isolate* isolate = CcTest::InitIsolateOnce();
HandleScope scope(isolate);
#if V8_TARGET_ARCH_64_BIT
constexpr bool require_guard_regions = true;
#else
constexpr bool require_guard_regions = false;
#endif
Handle<JSArrayBuffer> buffer;
CHECK(
wasm::NewArrayBuffer(isolate, 16 * kWasmPageSize, require_guard_regions)
.ToHandle(&buffer));
CHECK(wasm::NewArrayBuffer(isolate, 16 * kWasmPageSize).ToHandle(&buffer));
auto const contents = v8::Utils::ToLocal(buffer)->Externalize();
wasm::DetachMemoryBuffer(isolate, buffer, true);
constexpr bool is_wasm_memory = true;
@ -1132,14 +1118,8 @@ TEST(Run_WasmModule_Buffer_Externalized_Regression_UseAfterFree) {
// Regresion test for https://crbug.com/813876
Isolate* isolate = CcTest::InitIsolateOnce();
HandleScope scope(isolate);
#if V8_TARGET_ARCH_64_BIT
const bool require_guard_regions = trap_handler::IsTrapHandlerEnabled();
#else
constexpr bool require_guard_regions = false;
#endif
Handle<JSArrayBuffer> buffer;
CHECK(wasm::NewArrayBuffer(isolate, 16 * kWasmPageSize, require_guard_regions)
.ToHandle(&buffer));
CHECK(wasm::NewArrayBuffer(isolate, 16 * kWasmPageSize).ToHandle(&buffer));
Handle<WasmMemoryObject> mem = WasmMemoryObject::New(isolate, buffer, 128);
auto contents = v8::Utils::ToLocal(buffer)->Externalize();
WasmMemoryObject::Grow(isolate, mem, 0);
@ -1161,9 +1141,7 @@ TEST(Run_WasmModule_Reclaim_Memory) {
Handle<JSArrayBuffer> buffer;
for (int i = 0; i < 256; ++i) {
HandleScope scope(isolate);
constexpr bool require_guard_regions = true;
CHECK(NewArrayBuffer(isolate, kWasmPageSize, require_guard_regions,
SharedFlag::kNotShared)
CHECK(NewArrayBuffer(isolate, kWasmPageSize, SharedFlag::kNotShared)
.ToHandle(&buffer));
}
}

View File

@ -40,13 +40,9 @@ byte* TestingModuleBuilder::AddMemory(uint32_t size) {
CHECK_EQ(0, mem_size_);
DCHECK(!instance_object_->has_memory_object());
test_module_.has_memory = true;
const bool enable_guard_regions =
trap_handler::IsTrapHandlerEnabled() && test_module_.is_wasm();
uint32_t alloc_size =
enable_guard_regions ? RoundUp(size, CommitPageSize()) : size;
uint32_t alloc_size = RoundUp(size, kWasmPageSize);
Handle<JSArrayBuffer> new_buffer;
CHECK(wasm::NewArrayBuffer(isolate_, alloc_size, enable_guard_regions)
.ToHandle(&new_buffer));
CHECK(wasm::NewArrayBuffer(isolate_, alloc_size).ToHandle(&new_buffer));
CHECK(!new_buffer.is_null());
mem_start_ = reinterpret_cast<byte*>(new_buffer->backing_store());
mem_size_ = size;