[wasm] Remove --wasm-guard-pages flag

This flag was originally added as a staging mechanism to let us land and test
guard regions without the full trap handler feature landing. Additionally, we
thought we might enable guard regions without trap handlers on some systems.

Trap handlers are now supported, and there's not a real compelling reason for
why we need guard regions without trap handlers. Keeping the separate flag leads
to confusion, since some code treats guard regions and trap handlers the same,
while other code treats them as independent.

Removing this flag and its associated special cases makes everything more
uniform and predictable.

R=gdeepti@chromium.org

Change-Id: Icebab91d1f1e0c55e7a35c75b880085d37fa14ae
Reviewed-on: https://chromium-review.googlesource.com/706570Reviewed-by: Deepti Gandluri <gdeepti@chromium.org>
Reviewed-by: Mircea Trofin <mtrofin@chromium.org>
Commit-Queue: Eric Holk <eholk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48411}

src/compiler/wasm-compiler.cc

@@ -3482,8 +3482,7 @@ Node* WasmGraphBuilder::LoadMem(wasm::ValueType type, MachineType memtype,
 
   if (memtype.representation() == MachineRepresentation::kWord8 ||
       jsgraph()->machine()->UnalignedLoadSupported(memtype.representation())) {
-    if (FLAG_wasm_trap_handler && V8_TRAP_HANDLER_SUPPORTED) {
-      DCHECK(FLAG_wasm_guard_pages);
+    if (trap_handler::UseTrapHandler()) {
       load = graph()->NewNode(jsgraph()->machine()->ProtectedLoad(memtype),
                               MemBuffer(offset), index, *effect_, *control_);
       SetSourcePosition(load, position);

src/flag-definitions.h

@@ -540,10 +540,6 @@ DEFINE_BOOL(wasm_no_stack_checks, false,
 DEFINE_BOOL(wasm_trap_handler, false,
             "use signal handlers to catch out of bounds memory access in wasm"
             " (experimental, currently Linux x86_64 only)")
-DEFINE_BOOL(wasm_guard_pages, false,
-            "add guard pages to the end of WebWassembly memory"
-            " (experimental, no effect on 32-bit)")
-DEFINE_IMPLICATION(wasm_trap_handler, wasm_guard_pages)
 DEFINE_BOOL(wasm_code_fuzzer_gen_test, false,
             "Generate a test case when running the wasm-code fuzzer")
 DEFINE_BOOL(print_wasm_code, false, "Print WebAssembly code")

src/wasm/module-compiler.cc

@@ -1800,7 +1800,7 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {
     memory->set_is_neuterable(false);
     memory->set_is_wasm_buffer(true);
 
-    DCHECK_IMPLIES(EnableGuardRegions(),
+    DCHECK_IMPLIES(trap_handler::UseTrapHandler(),
                    module_->is_asm_js() || memory->has_guard_region());
   } else if (initial_pages > 0) {
     memory_ = AllocateMemory(initial_pages);
@@ -2480,7 +2480,7 @@ 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 = EnableGuardRegions();
+  const bool enable_guard_regions = trap_handler::UseTrapHandler();
   Handle<JSArrayBuffer> mem_buffer = NewArrayBuffer(
       isolate_, num_pages * WasmModule::kPageSize, enable_guard_regions);
 

src/wasm/wasm-js.cc

@@ -578,7 +578,7 @@ void WebAssemblyMemory(const v8::FunctionCallbackInfo<v8::Value>& args) {
   size_t size = static_cast<size_t>(i::wasm::WasmModule::kPageSize) *
                 static_cast<size_t>(initial);
   i::Handle<i::JSArrayBuffer> buffer = i::wasm::NewArrayBuffer(
-      i_isolate, size, i::FLAG_wasm_guard_pages,
+      i_isolate, size, internal::trap_handler::UseTrapHandler(),
       is_shared_memory ? i::SharedFlag::kShared : i::SharedFlag::kNotShared);
   if (buffer.is_null()) {
     thrower.RangeError("could not allocate memory");

src/wasm/wasm-memory.cc

@@ -18,7 +18,7 @@ void* TryAllocateBackingStore(Isolate* isolate, size_t size,
   // systems. It may be safer to fail instead, given that other code might do
   // things that would be unsafe if they expected guard pages where there
   // weren't any.
-  if (enable_guard_regions && kGuardRegionsSupported) {
+  if (enable_guard_regions) {
     // TODO(eholk): On Windows we want to make sure we don't commit the guard
     // pages yet.
 
@@ -84,8 +84,6 @@ Handle<JSArrayBuffer> NewArrayBuffer(Isolate* isolate, size_t size,
     return Handle<JSArrayBuffer>::null();
   }
 
-  enable_guard_regions = enable_guard_regions && kGuardRegionsSupported;
-
   void* allocation_base = nullptr;  // Set by TryAllocateBackingStore
   size_t allocation_length = 0;     // Set by TryAllocateBackingStore
   // Do not reserve memory till non zero memory is encountered.

src/wasm/wasm-memory.h

@@ -13,17 +13,6 @@ namespace v8 {
 namespace internal {
 namespace wasm {
 
-#if V8_TARGET_ARCH_64_BIT
-const bool kGuardRegionsSupported = true;
-#else
-const bool kGuardRegionsSupported = false;
-#endif
-
-inline bool EnableGuardRegions() {
-  return FLAG_wasm_guard_pages && kGuardRegionsSupported &&
-         !FLAG_experimental_wasm_threads;
-}
-
 Handle<JSArrayBuffer> NewArrayBuffer(
     Isolate*, size_t size, bool enable_guard_regions,
     SharedFlag shared = SharedFlag::kNotShared);

src/wasm/wasm-objects.cc

@@ -329,7 +329,7 @@ Handle<JSArrayBuffer> GrowMemoryBuffer(Isolate* isolate,
     return Handle<JSArrayBuffer>::null();
   }
   const bool enable_guard_regions = old_buffer.is_null()
-                                        ? wasm::EnableGuardRegions()
+                                        ? trap_handler::UseTrapHandler()
                                         : old_buffer->has_guard_region();
   size_t new_size =
       static_cast<size_t>(old_pages + pages) * WasmModule::kPageSize;
@@ -386,7 +386,7 @@ Handle<WasmMemoryObject> WasmMemoryObject::New(Isolate* isolate,
       isolate->factory()->NewJSObject(memory_ctor, TENURED));
   auto wasm_context = Managed<WasmContext>::Allocate(isolate);
   if (buffer.is_null()) {
-    const bool enable_guard_regions = wasm::EnableGuardRegions();
+    const bool enable_guard_regions = trap_handler::UseTrapHandler();
     buffer = wasm::SetupArrayBuffer(isolate, nullptr, 0, nullptr, 0, false,
                                     enable_guard_regions);
     wasm_context->get()->mem_size = 0;

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

@@ -40,7 +40,8 @@ byte* TestingModuleBuilder::AddMemory(uint32_t size) {
   DCHECK(!instance_object_->has_memory_buffer());
   DCHECK(!instance_object_->has_memory_object());
   test_module_.has_memory = true;
-  bool enable_guard_regions = EnableGuardRegions() && test_module_.is_wasm();
+  const bool enable_guard_regions =
+      trap_handler::UseTrapHandler() && test_module_.is_wasm();
   uint32_t alloc_size =
       enable_guard_regions ? RoundUp(size, base::OS::CommitPageSize()) : size;
   Handle<JSArrayBuffer> new_buffer =

tools/testrunner/local/variants.py

@@ -12,7 +12,7 @@ ALL_VARIANT_FLAGS = {
   # https://chromium-review.googlesource.com/c/452620/ for more discussion.
   "nooptimization": [["--noopt"]],
   "stress_asm_wasm": [["--validate-asm", "--stress-validate-asm", "--suppress-asm-messages"]],
-  "wasm_traps": [["--wasm_guard_pages", "--wasm_trap_handler", "--invoke-weak-callbacks"]],
+  "wasm_traps": [["--wasm_trap_handler", "--invoke-weak-callbacks"]],
 }
 
 # FAST_VARIANTS implies no --always-opt.
@@ -25,7 +25,7 @@ FAST_VARIANT_FLAGS = {
   # https://chromium-review.googlesource.com/c/452620/ for more discussion.
   "nooptimization": [["--noopt"]],
   "stress_asm_wasm": [["--validate-asm", "--stress-validate-asm", "--suppress-asm-messages"]],
-  "wasm_traps": [["--wasm_guard_pages", "--wasm_trap_handler", "--invoke-weak-callbacks"]],
+  "wasm_traps": [["--wasm_trap_handler", "--invoke-weak-callbacks"]],
 }
 
 ALL_VARIANTS = set(["default", "stress", "stress_incremental_marking",