[wasm] Enable cctests when out of bounds trap handler is enabled

Change-Id: I47f0d5578a7c26aa7a30c97175eefc1a9c935d77
Reviewed-on: https://chromium-review.googlesource.com/455318
Commit-Queue: Eric Holk <eholk@chromium.org>
Commit-Queue: Brad Nelson <bradnelson@chromium.org>
Reviewed-by: Brad Nelson <bradnelson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43808}

src/wasm/wasm-interpreter.cc

@@ -637,9 +637,15 @@ static inline int32_t ExecuteGrowMemory(uint32_t delta_pages,
             instance->module->max_mem_pages) {
       return -1;
     }
-    new_mem_start = static_cast<byte*>(realloc(instance->mem_start, new_size));
-    if (!new_mem_start) {
-      return -1;
+    if (EnableGuardRegions()) {
+      v8::base::OS::Unprotect(instance->mem_start, new_size);
+      new_mem_start = instance->mem_start;
+    } else {
+      new_mem_start =
+          static_cast<byte*>(realloc(instance->mem_start, new_size));
+      if (!new_mem_start) {
+        return -1;
+      }
     }
     // Zero initializing uninitialized memory from realloc
     memset(new_mem_start + old_size, 0, new_size - old_size);

src/wasm/wasm-module.cc

@@ -67,16 +67,6 @@ static void MemoryFinalizer(const v8::WeakCallbackInfo<void>& data) {
   GlobalHandles::Destroy(reinterpret_cast<Object**>(p));
 }
 
-#if V8_TARGET_ARCH_64_BIT
-const bool kGuardRegionsSupported = true;
-#else
-const bool kGuardRegionsSupported = false;
-#endif
-
-bool EnableGuardRegions() {
-  return FLAG_wasm_guard_pages && kGuardRegionsSupported;
-}
-
 static void RecordStats(Isolate* isolate, Code* code) {
   isolate->counters()->wasm_generated_code_size()->Increment(code->body_size());
   isolate->counters()->wasm_reloc_size()->Increment(
@@ -806,6 +796,42 @@ Handle<JSArrayBuffer> wasm::NewArrayBuffer(Isolate* isolate, size_t size,
                           enable_guard_regions);
 }
 
+void wasm::UnpackAndRegisterProtectedInstructions(
+    Isolate* isolate, Handle<FixedArray> code_table) {
+  for (int i = 0; i < code_table->length(); ++i) {
+    Handle<Code> code;
+    // This is sometimes undefined when we're called from cctests.
+    if (!code_table->GetValue<Code>(isolate, i).ToHandle(&code)) {
+      continue;
+    }
+
+    if (code->kind() != Code::WASM_FUNCTION) {
+      continue;
+    }
+
+    const intptr_t base = reinterpret_cast<intptr_t>(code->entry());
+
+    Zone zone(isolate->allocator(), "Wasm Module");
+    ZoneVector<trap_handler::ProtectedInstructionData> unpacked(&zone);
+    const int mode_mask =
+        RelocInfo::ModeMask(RelocInfo::WASM_PROTECTED_INSTRUCTION_LANDING);
+    for (RelocIterator it(*code, mode_mask); !it.done(); it.next()) {
+      trap_handler::ProtectedInstructionData data;
+      data.instr_offset = it.rinfo()->data();
+      data.landing_offset = reinterpret_cast<intptr_t>(it.rinfo()->pc()) - base;
+      unpacked.emplace_back(data);
+    }
+    if (unpacked.size() > 0) {
+      int size = code->CodeSize();
+      const int index = RegisterHandlerData(reinterpret_cast<void*>(base), size,
+                                            unpacked.size(), &unpacked[0]);
+      // TODO(eholk): if index is negative, fail.
+      DCHECK(index >= 0);
+      code->set_trap_handler_index(Smi::FromInt(index));
+    }
+  }
+}
+
 std::ostream& wasm::operator<<(std::ostream& os, const WasmModule& module) {
   os << "WASM module with ";
   os << (module.min_mem_pages * module.kPageSize) << " min mem";
@@ -1244,36 +1270,7 @@ class InstantiationHelper {
     // Unpack and notify signal handler of protected instructions.
     //--------------------------------------------------------------------------
     if (trap_handler::UseTrapHandler()) {
-      for (int i = 0; i < code_table->length(); ++i) {
-        Handle<Code> code = code_table->GetValueChecked<Code>(isolate_, i);
-
-        if (code->kind() != Code::WASM_FUNCTION) {
-          continue;
-        }
-
-        const intptr_t base = reinterpret_cast<intptr_t>(code->entry());
-
-        Zone zone(isolate_->allocator(), "Wasm Module");
-        ZoneVector<trap_handler::ProtectedInstructionData> unpacked(&zone);
-        const int mode_mask =
-            RelocInfo::ModeMask(RelocInfo::WASM_PROTECTED_INSTRUCTION_LANDING);
-        for (RelocIterator it(*code, mode_mask); !it.done(); it.next()) {
-          trap_handler::ProtectedInstructionData data;
-          data.instr_offset = it.rinfo()->data();
-          data.landing_offset =
-              reinterpret_cast<intptr_t>(it.rinfo()->pc()) - base;
-          unpacked.emplace_back(data);
-        }
-        if (unpacked.size() > 0) {
-          int size = code->CodeSize();
-          const int index =
-              RegisterHandlerData(reinterpret_cast<void*>(base), size,
-                                  unpacked.size(), &unpacked[0]);
-          // TODO(eholk): if index is negative, fail.
-          DCHECK(index >= 0);
-          code->set_trap_handler_index(Smi::FromInt(index));
-        }
-      }
+      UnpackAndRegisterProtectedInstructions(isolate_, code_table);
     }
 
     //--------------------------------------------------------------------------

src/wasm/wasm-module.h

@@ -443,6 +443,19 @@ V8_EXPORT_PRIVATE void AsyncCompileAndInstantiate(
     Isolate* isolate, Handle<JSPromise> promise, const ModuleWireBytes& bytes,
     MaybeHandle<JSReceiver> imports);
 
+#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;
+}
+
+void UnpackAndRegisterProtectedInstructions(Isolate* isolate,
+                                            Handle<FixedArray> code_table);
+
 namespace testing {
 void ValidateInstancesChain(Isolate* isolate,
                             Handle<WasmModuleObject> module_obj,

test/cctest/wasm/test-wasm-trap-position.cc

@@ -99,14 +99,6 @@ TEST(Unreachable) {
 
 // Trigger a trap for loading from out-of-bounds.
 TEST(IllegalLoad) {
-  if (trap_handler::UseTrapHandler()) {
-    // r.module().AddMemory() does not allocate guard pages, so we skip this
-    // test for now when using trap handlers. The simple out of bounds access
-    // case is covered by mjsunit tests, so we are still getting test coverage.
-    //
-    // TODO(eholk): make this test work with trap handlers.
-    return;
-  }
   WasmRunner<void> r(kExecuteCompiled);
   TestSignatures sigs;
   // Set the execution context, such that a runtime error can be thrown.

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

@@ -98,7 +98,16 @@ class TestingModule : public ModuleEnv {
 
   ~TestingModule() {
     if (instance->mem_start) {
-      free(instance->mem_start);
+      if (EnableGuardRegions() && module_.origin == kWasmOrigin) {
+        // See the corresponding code in AddMemory. We use a different
+        // allocation path when guard regions are enabled, which means we have
+        // to free it differently too.
+        const size_t alloc_size =
+            RoundUp(kWasmMaxHeapOffset, v8::base::OS::CommitPageSize());
+        v8::base::OS::Free(instance->mem_start, alloc_size);
+      } else {
+        free(instance->mem_start);
+      }
     }
     if (interpreter_) delete interpreter_;
   }
@@ -110,7 +119,17 @@ class TestingModule : public ModuleEnv {
     CHECK_NULL(instance->mem_start);
     CHECK_EQ(0, instance->mem_size);
     module_.has_memory = true;
-    instance->mem_start = reinterpret_cast<byte*>(malloc(size));
+    if (EnableGuardRegions() && module_.origin == kWasmOrigin) {
+      const size_t alloc_size =
+          RoundUp(kWasmMaxHeapOffset, v8::base::OS::CommitPageSize());
+      instance->mem_start = reinterpret_cast<byte*>(
+          v8::base::OS::AllocateGuarded(alloc_size * 2));
+      instance->mem_start += alloc_size;
+      const size_t guard_size = RoundUp(size, v8::base::OS::CommitPageSize());
+      v8::base::OS::Unprotect(instance->mem_start, guard_size);
+    } else {
+      instance->mem_start = reinterpret_cast<byte*>(malloc(size));
+    }
     CHECK(size == 0 || instance->mem_start);
     memset(instance->mem_start, 0, size);
     instance->mem_size = size;
@@ -573,6 +592,9 @@ class WasmFunctionCompiler : private GraphAndBuilders {
     DCHECK(code_table->get(static_cast<int>(function_index()))
                ->IsUndefined(isolate()));
     code_table->set(static_cast<int>(function_index()), *code);
+    if (trap_handler::UseTrapHandler()) {
+      UnpackAndRegisterProtectedInstructions(isolate(), code_table);
+    }
   }
 
   byte AllocateLocal(ValueType type) {