[wasm] Add guard regions to end of WebAssembly.Memory buffers

src/wasm/wasm-module.cc

 // Copyright 2015 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <memory>
 
 #include "src/base/atomic-utils.h"
 #include "src/code-stubs.h"
 
 #include "src/macro-assembler.h"
@@ skipping 96 matching lines @@
 void ReplaceReferenceInCode(Handle<Code> code, Handle<Object> old_ref,
                             Handle<Object> new_ref) {
   for (RelocIterator it(*code, 1 << RelocInfo::EMBEDDED_OBJECT); !it.done();
        it.next()) {
     if (it.rinfo()->target_object() == *old_ref) {
       it.rinfo()->set_target_object(*new_ref);
     }
   }
 }
 
-Handle<JSArrayBuffer> NewArrayBuffer(Isolate* isolate, size_t size) {
-  if (size > (WasmModule::kV8MaxPages * WasmModule::kPageSize)) {
-    // TODO(titzer): lift restriction on maximum memory allocated here.
-    return Handle<JSArrayBuffer>::null();
-  }
-  void* memory = isolate->array_buffer_allocator()->Allocate(size);
-  if (memory == nullptr) {
-    return Handle<JSArrayBuffer>::null();
-  }
+#if V8_HOST_ARCH_64_BIT
+static void MemoryFinalizer(const v8::WeakCallbackInfo<void>& data) {
+  JSArrayBuffer** p = reinterpret_cast<JSArrayBuffer**>(data.GetParameter());
+  JSArrayBuffer* buffer = *p;
 
-#if DEBUG
-  // Double check the API allocator actually zero-initialized the memory.
-  const byte* bytes = reinterpret_cast<const byte*>(memory);
-  for (size_t i = 0; i < size; ++i) {
-    DCHECK_EQ(0, bytes[i]);
-  }
+  void* memory = buffer->backing_store();
+  base::OS::Free(memory,
+                 RoundUp(kWasmMaxHeapOffset, base::OS::CommitPageSize()));
+
+  data.GetIsolate()->AdjustAmountOfExternalAllocatedMemory(
+      -buffer->byte_length()->Number());
+
+  GlobalHandles::Destroy(reinterpret_cast<Object**>(p));
+}
 #endif
 
-  Handle<JSArrayBuffer> buffer = isolate->factory()->NewJSArrayBuffer();
-  JSArrayBuffer::Setup(buffer, isolate, false, memory, static_cast<int>(size));
-  buffer->set_is_neuterable(false);
-  return buffer;
+void* TryAllocateBackingStore(Isolate* isolate, size_t size, bool guard) {
+  if (guard) {

[titzer, 2016/11/07 19:54:53]

I think this needs to be TARGET_ARCH.

How about guard && V8_TARGET_ARCH_64_BIT?
That way, the flags FLAG_wasm_trap_handler and FLAG_wasm_guard_pages have no
effect on 32-bit systems instead of just crashing.

[Eric Holk, 2016/11/08 23:58:15]

Done. I also updated the help text for --wasm_guard_pages to indicate that the
flag has no effect on 32-bit systems.

Making the flag have no effect on 32-bit makes me a little nervous. We might
have other code expect guard regions and do things that would be unsafe without
them. Having no effect is fine for now, but I think before we have this on by
default it would be good to revisit. I added a TODO.

+#if V8_HOST_ARCH_64_BIT
+    void* memory;
+    // TODO(eholk): On Windows we want to make sure we don't commit the guard
+    // pages yet.
+
+    // We always allocate the largest possible offset into the heap, so the
+    // addressable memory after the guard page can be made inaccessible.
+    const size_t alloc_size =
+        RoundUp(kWasmMaxHeapOffset, base::OS::CommitPageSize());
+    DCHECK_EQ(0, size % base::OS::CommitPageSize());
+
+    size_t allocated_size = 0;
+    const bool is_executable = false;
+    memory = base::OS::Allocate(alloc_size, &allocated_size, is_executable);
+    if (allocated_size < alloc_size) {
+      base::OS::Free(memory, allocated_size);
+      return nullptr;
+    }
+
+    if (memory == nullptr) {
+      return nullptr;
+    }
+
+    byte* bytes = reinterpret_cast<byte*>(memory);
+    base::OS::Guard(bytes + size, alloc_size - size);
+
+    reinterpret_cast<v8::Isolate*>(isolate)
+        ->AdjustAmountOfExternalAllocatedMemory(size);
+
+    return memory;
+#else
+    DCHECK(false && "Guard pages are not supported on 32-bit systems");
+    return nullptr;
+#endif
+  } else {
+    void* memory = isolate->array_buffer_allocator()->Allocate(size);
+    return memory;
+  }
 }
 
 void RelocateMemoryReferencesInCode(Handle<FixedArray> code_table,
                                     Address old_start, Address start,
                                     uint32_t prev_size, uint32_t new_size) {
   for (int i = 0; i < code_table->length(); ++i) {
     DCHECK(code_table->get(i)->IsCode());
     Handle<Code> code = Handle<Code>(Code::cast(code_table->get(i)));
     AllowDeferredHandleDereference embedding_raw_address;
     int mask = (1 << RelocInfo::WASM_MEMORY_REFERENCE) |
@@ skipping 475 matching lines @@
     TRACE_CHAIN(WasmCompiledModule::cast(wasm_module->GetInternalField(0)));
     TRACE("}\n");
   }
   compiled_module->reset_weak_owning_instance();
   GlobalHandles::Destroy(reinterpret_cast<Object**>(p));
   TRACE("}\n");
 }
 
 }  // namespace
 
+Handle<JSArrayBuffer> wasm::NewArrayBuffer(Isolate* isolate, size_t size,
+                                           bool enable_guard_regions) {

[titzer, 2016/11/07 19:54:52]

I like this name better. Can we make the header file match it?

[Eric Holk, 2016/11/08 23:58:15]

Done.

+  if (size > (WasmModule::kV8MaxPages * WasmModule::kPageSize)) {
+    // TODO(titzer): lift restriction on maximum memory allocated here.
+    return Handle<JSArrayBuffer>::null();
+  }
+
+  void* memory = TryAllocateBackingStore(isolate, size, enable_guard_regions);
+  bool is_external = enable_guard_regions;

[titzer, 2016/11/07 19:54:52]

What about making is_external a return value from TryAllocateBackingStore? Then
we can use it down below without an #ifdef.

[Eric Holk, 2016/11/08 23:58:15]

Done, although it seems like V8_TARGET_ARCH_64_BIT is only available from the
preprocessor for some reason, so we still need an #ifdef.

+
+  if (memory == nullptr) {
+    return Handle<JSArrayBuffer>::null();
+  }
+
+#if DEBUG
+  // Double check the API allocator actually zero-initialized the memory.
+  const byte* bytes = reinterpret_cast<const byte*>(memory);
+  for (size_t i = 0; i < size; ++i) {
+    DCHECK_EQ(0, bytes[i]);
+  }
+#endif
+
+  Handle<JSArrayBuffer> buffer = isolate->factory()->NewJSArrayBuffer();
+  JSArrayBuffer::Setup(buffer, isolate, is_external, memory,
+                       static_cast<int>(size));
+  buffer->set_is_neuterable(false);
+  buffer->set_has_guard_region(enable_guard_regions);
+
+#if V8_HOST_ARCH_64_BIT
+  if (is_external) {
+    // We mark the buffer as external if we allocated it here with guard
+    // pages. That means we need to arrange for it to be freed.
+
+    // TODO(eholk): Finalizers may not run when the main thread is shutting
+    // down, which means we may leak memory here.
+    Handle<Object> global_handle = isolate->global_handles()->Create(*buffer);
+    GlobalHandles::MakeWeak(global_handle.location(), global_handle.location(),
+                            &MemoryFinalizer, v8::WeakCallbackType::kFinalizer);
+  }
+#endif
+
+  return buffer;
+}
+
 const char* wasm::SectionName(WasmSectionCode code) {
   switch (code) {
     case kUnknownSectionCode:
       return "Unknown";
     case kTypeSectionCode:
       return "Type";
     case kImportSectionCode:
       return "Import";
     case kFunctionSectionCode:
       return "Function";
@@ skipping 425 matching lines @@
     Handle<JSObject> instance = factory->NewJSObjectFromMap(map, TENURED);
     instance->SetInternalField(kWasmMemObject,
                                isolate_->heap()->undefined_value());
 
     //--------------------------------------------------------------------------
     // Set up the globals for the new instance.
     //--------------------------------------------------------------------------
     MaybeHandle<JSArrayBuffer> old_globals;
     uint32_t globals_size = module_->globals_size;
     if (globals_size > 0) {
+      const bool enable_guard_regions = false;
       Handle<JSArrayBuffer> global_buffer =
-          NewArrayBuffer(isolate_, globals_size);
+          NewArrayBuffer(isolate_, globals_size, enable_guard_regions);
       globals_ = global_buffer;
       if (globals_.is_null()) {
         thrower_->RangeError("Out of memory: wasm globals");
         return nothing;
       }
       Address old_address = owner.is_null()
                                 ? nullptr
                                 : GetGlobalStartAddressFromCodeTemplate(
                                       isolate_->heap()->undefined_value(),
                                       JSObject::cast(*owner.ToHandleChecked()));
@@ skipping 30 matching lines @@
     //--------------------------------------------------------------------------
     MaybeHandle<JSArrayBuffer> old_memory;
 
     uint32_t min_mem_pages = module_->min_mem_pages;
     isolate_->counters()->wasm_min_mem_pages_count()->AddSample(min_mem_pages);
     // TODO(wasm): re-enable counter for max_mem_pages when we use that field.
 
     if (!memory_.is_null()) {
       // Set externally passed ArrayBuffer non neuterable.
       memory_->set_is_neuterable(false);
+
+      DCHECK_IMPLIES(FLAG_wasm_guard_pages, module_->origin == kAsmJsOrigin ||
+                                                memory_->has_guard_region());
     } else if (min_mem_pages > 0) {
       memory_ = AllocateMemory(min_mem_pages);
       if (memory_.is_null()) return nothing;  // failed to allocate memory
     }
 
     if (!memory_.is_null()) {
       instance->SetInternalField(kWasmMemArrayBuffer, *memory_);
       Address mem_start = static_cast<Address>(memory_->backing_store());
       uint32_t mem_size =
           static_cast<uint32_t>(memory_->byte_length()->Number());
@@ skipping 431 matching lines @@
       }
     }
   }
 
   // Allocate memory for a module instance as a new JSArrayBuffer.
   Handle<JSArrayBuffer> AllocateMemory(uint32_t min_mem_pages) {
     if (min_mem_pages > WasmModule::kV8MaxPages) {
       thrower_->RangeError("Out of memory: wasm memory too large");
       return Handle<JSArrayBuffer>::null();
     }
-    Handle<JSArrayBuffer> mem_buffer =
-        NewArrayBuffer(isolate_, min_mem_pages * WasmModule::kPageSize);
+    const bool enable_guard_regions = FLAG_wasm_guard_pages;
+    Handle<JSArrayBuffer> mem_buffer = NewArrayBuffer(
+        isolate_, min_mem_pages * WasmModule::kPageSize, enable_guard_regions);
 
     if (mem_buffer.is_null()) {
       thrower_->RangeError("Out of memory: wasm memory");
     }
     return mem_buffer;
   }
 
   // Process the exports, creating wrappers for functions, tables, memories,
   // and globals.
   void ProcessExports(Handle<FixedArray> code_table,
@@ skipping 540 matching lines @@
     // "undefined" case above.
     DCHECK_NOT_NULL(old_mem_start);
     DCHECK(old_size + pages * WasmModule::kPageSize <=
            std::numeric_limits<uint32_t>::max());
     new_size = old_size + pages * WasmModule::kPageSize;
   }
 
   if (new_size <= old_size || max_pages * WasmModule::kPageSize < new_size) {
     return -1;
   }
-  Handle<JSArrayBuffer> buffer = NewArrayBuffer(isolate, new_size);
-  if (buffer.is_null()) return -1;
-  Address new_mem_start = static_cast<Address>(buffer->backing_store());
-  if (old_size != 0) {
-    memcpy(new_mem_start, old_mem_start, old_size);
+
+  Handle<JSArrayBuffer> buffer;
+
+  if (FLAG_wasm_guard_pages && !old_buffer.is_null()) {

[titzer, 2016/11/07 19:54:53]

How about !old_buffer.is_null() and old_buffer.has_guard_region()?

[Eric Holk, 2016/11/08 23:58:15]

Done.

+    base::OS::Unprotect(old_buffer->backing_store(), new_size);

[titzer, 2016/11/07 19:54:53]

Add a comment that we aren't moving the memory here, just adjusting the size.

[Eric Holk, 2016/11/08 23:58:15]

Done.

+    reinterpret_cast<v8::Isolate*>(isolate)
+        ->AdjustAmountOfExternalAllocatedMemory(pages * WasmModule::kPageSize);
+    Handle<Object> new_size_object =
+        isolate->factory()->NewNumberFromSize(new_size);
+    old_buffer->set_byte_length(*new_size_object);
+
+    SetInstanceMemory(instance, *old_buffer);
+    Handle<FixedArray> code_table = GetCompiledModule(*instance)->code_table();
+    RelocateMemoryReferencesInCode(code_table, old_mem_start, old_mem_start,
+                                   old_size, new_size);
+    buffer = old_buffer;
+  } else {
+    const bool enable_guard_regions = FLAG_wasm_guard_pages;
+    buffer = NewArrayBuffer(isolate, new_size, enable_guard_regions);
+    if (buffer.is_null()) return -1;
+    Address new_mem_start = static_cast<Address>(buffer->backing_store());
+    if (old_size != 0) {
+      memcpy(new_mem_start, old_mem_start, old_size);
+    }
+    SetInstanceMemory(instance, *buffer);
+    Handle<FixedArray> code_table = GetCompiledModule(*instance)->code_table();
+    RelocateMemoryReferencesInCode(code_table, old_mem_start, new_mem_start,
+                                   old_size, new_size);
   }
-  SetInstanceMemory(instance, *buffer);
-  Handle<FixedArray> code_table = GetCompiledModule(*instance)->code_table();
-  RelocateMemoryReferencesInCode(code_table, old_mem_start, new_mem_start,
-                                 old_size, new_size);
+
   Handle<Object> memory_object(instance->GetInternalField(kWasmMemObject),
                                isolate);
   if (!memory_object->IsUndefined(isolate)) {
     WasmJs::SetWasmMemoryArrayBuffer(isolate, memory_object, buffer);
   }
 
   DCHECK(old_size % WasmModule::kPageSize == 0);
   return (old_size / WasmModule::kPageSize);
 }
 
@@ skipping 68 matching lines @@
     CHECK_NOT_NULL(result.val);
     module = const_cast<WasmModule*>(result.val);
   }
 
   Handle<WasmModuleWrapper> module_wrapper =
       WasmModuleWrapper::New(isolate, module);
 
   compiled_module->set_module_wrapper(module_wrapper);
   DCHECK(WasmCompiledModule::IsWasmCompiledModule(*compiled_module));
 }