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

This is a reland of ad221d14

Original change's description:
> [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}

Bug: v8:7619
Change-Id: I0f311305472ca2305ad2fa9163560ff54c1422c2
Reviewed-on: https://chromium-review.googlesource.com/999872
Commit-Queue: Eric Holk <eholk@chromium.org>
Reviewed-by: Brad Nelson <bradnelson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52921}

src/wasm/module-compiler.cc

@@ -1680,9 +1680,19 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {
   MaybeHandle<JSArrayBuffer> old_globals;
   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 {};
     }
@@ -2463,14 +2473,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");
   }

src/wasm/wasm-js.cc

@@ -651,15 +651,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;

src/wasm/wasm-memory.cc

@@ -15,18 +15,17 @@ namespace wasm {
 namespace {
 
 void* TryAllocateBackingStore(WasmMemoryTracker* memory_tracker, Heap* heap,
-                              size_t size, bool require_guard_regions,
+                              size_t size, bool require_full_guard_regions,
                               void** allocation_base,
                               size_t* allocation_length) {
   using AllocationStatus = WasmMemoryTracker::AllocationStatus;
-
 #if V8_TARGET_ARCH_32_BIT
-  DCHECK(!require_guard_regions);
+  DCHECK(!require_full_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.
   *allocation_length =
-      require_guard_regions
+      require_full_guard_regions
           ? RoundUp(kWasmMaxHeapOffset, CommitPageSize())
           : RoundUp(
                 base::bits::RoundUpToPowerOfTwo32(static_cast<uint32_t>(size)),
@@ -188,11 +187,12 @@ 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();
+    const bool require_full_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,
+        this, heap, buffer_length, require_full_guard_regions,
         &local_allocation_base, &local_allocation_length);
 
     empty_backing_store_ =
@@ -254,7 +254,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
@@ -270,15 +269,27 @@ MaybeHandle<JSArrayBuffer> NewArrayBuffer(Isolate* isolate, size_t size,
   void* allocation_base = nullptr;
   size_t allocation_length = 0;
 
-  void* memory =
-      (size == 0)
-          ? 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 {};
+  void* memory;
+  if (size == 0) {
+    memory = memory_tracker->GetEmptyBackingStore(
+        &allocation_base, &allocation_length, isolate->heap());
+  } else {
+    bool require_full_guard_regions = trap_handler::IsTrapHandlerEnabled();
+    memory = TryAllocateBackingStore(memory_tracker, isolate->heap(), size,
+                                     require_full_guard_regions,
+                                     &allocation_base, &allocation_length);
+    if (memory == nullptr && require_full_guard_regions) {
+      // If we failed to allocate with full guard regions, fall back on
+      // mini-guards.
+      require_full_guard_regions = false;
+      memory = TryAllocateBackingStore(memory_tracker, isolate->heap(), size,
+                                       require_full_guard_regions,
+                                       &allocation_base, &allocation_length);
+    }
+  }
+  if (memory == nullptr) {
+    return {};
+  }
 
 #if DEBUG
   // Double check the API allocator actually zero-initialized the memory.

src/wasm/wasm-memory.h

@@ -146,8 +146,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,

src/wasm/wasm-objects.cc

@@ -422,8 +422,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 {};
   void* old_mem_start = old_buffer->backing_store();
   uint32_t old_size = 0;
@@ -471,8 +470,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;
@@ -576,10 +574,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;
   }

test/cctest/wasm/test-run-wasm-module.cc

@@ -1082,15 +1082,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();
@@ -1111,15 +1104,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;
@@ -1135,14 +1121,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);
@@ -1164,9 +1144,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));
   }
 }

test/cctest/wasm/wasm-run-utils.cc

@@ -69,13 +69,9 @@ byte* TestingModuleBuilder::AddMemory(uint32_t size) {
   DCHECK_IMPLIES(test_module_->origin() == kWasmOrigin,
                  size % kWasmPageSize == 0);
   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;
@@ -88,6 +84,10 @@ byte* TestingModuleBuilder::AddMemory(uint32_t size) {
       (test_module_->maximum_pages != 0) ? test_module_->maximum_pages : -1);
   instance_object_->set_memory_object(*memory_object);
   WasmMemoryObject::AddInstance(isolate_, memory_object, instance_object_);
+  // TODO(wasm): Delete the following two lines when test-run-wasm will use a
+  // multiple of kPageSize as memory size. At the moment, the effect of these
+  // two lines is used to shrink the memory for testing purposes.
+  instance_object_->SetRawMemory(mem_start_, mem_size_);
   return mem_start_;
 }