[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 68 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::kMaxMemPages * 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, kWasmMaxHeapOffset);
+
+  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* AllocateBackingStore(Isolate* isolate, size_t size, bool guard) {
+  if (guard) {
+#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 make inaccessible.

[Mircea Trofin, 2016/10/28 22:16:29]

can be made

[Eric Holk, 2016/10/29 00:04:30]

Done.

+    const size_t alloc_size = kWasmMaxHeapOffset;
+    DCHECK(size % base::OS::CommitPageSize() == 0);

[Mircea Trofin, 2016/10/28 22:16:29]

DCHECK_EQ(0, size % etc)

[Eric Holk, 2016/10/29 00:04:30]

Done.

+
+    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;

[Mircea Trofin, 2016/10/28 22:16:29]

because it may fail, perhaps name the API TryAllocateBackingStore

[Eric Holk, 2016/10/29 00:04:30]

Done.

+    }
+
+    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");
+#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 guard) {

[Mircea Trofin, 2016/10/28 22:16:29]

guard->enable_guard_regions

also, if this flag is set, is it guaranteed you get guard regions?

[Eric Holk, 2016/10/29 00:04:30]

Done.

Yes (at least on 64-bit platforms)

+  if (size > (WasmModule::kMaxMemPages * WasmModule::kPageSize)) {
+    // TODO(titzer): lift restriction on maximum memory allocated here.
+    return Handle<JSArrayBuffer>::null();
+  }
+
+  void* memory = AllocateBackingStore(isolate, size, guard);
+  bool is_external = guard;
+
+  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(guard);
+
+#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 331 matching lines @@
     instance->SetInternalField(kWasmMemObject,
                                isolate_->heap()->undefined_value());
 
     //--------------------------------------------------------------------------
     // Set up the globals for the new instance.
     //--------------------------------------------------------------------------
     MaybeHandle<JSArrayBuffer> old_globals;
     MaybeHandle<JSArrayBuffer> globals;
     uint32_t globals_size = module_->globals_size;
     if (globals_size > 0) {
+      const bool guard = false;
       Handle<JSArrayBuffer> global_buffer =
-          NewArrayBuffer(isolate_, globals_size);
+          NewArrayBuffer(isolate_, globals_size, guard);
       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 18 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 ||

[Mircea Trofin, 2016/10/28 22:16:29]

asmjs, not wasm?

[Eric Holk, 2016/10/29 00:04:30]

Yes. For AsmJs origin we still do the bounds checking and therefore don't need
guard pages. Some of our tests have buffers that aren't page size multiples, so
we can't add a guard region to those.

This check is making sure that if we accept an externally-allocated memory then
it was allocated with guard pages. The way this happens now is that the
allocator for WebAssembly.Memory sets up the guard pages, and using some other
object would be a TypeError.

+                                                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 494 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::kMaxMemPages) {
       thrower_->RangeError("Out of memory: wasm memory too large");
       return Handle<JSArrayBuffer>::null();
     }
+    const bool guard = FLAG_wasm_guard_pages;
     Handle<JSArrayBuffer> mem_buffer =
-        NewArrayBuffer(isolate_, min_mem_pages * WasmModule::kPageSize);
+        NewArrayBuffer(isolate_, min_mem_pages * WasmModule::kPageSize, guard);
 
     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(MaybeHandle<JSArrayBuffer> globals,
@@ skipping 470 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()) {
+    base::OS::Unprotect(old_buffer->backing_store(), new_size);
+    reinterpret_cast<v8::Isolate*>(isolate)
+        ->AdjustAmountOfExternalAllocatedMemory(pages * WasmModule::kPageSize);

[Mircea Trofin, 2016/10/28 22:16:29]

would it make sense to add this API to the internal isolate?

[Eric Holk, 2016/10/29 00:04:30]

Perhaps, but I'd rather leave it how it is.
AdjustAmountOfExternalAllocatedMemory accesses fields in the internal
embedder_data_ field, and there doesn't seem to be a lot of precedent for
accessing this through the internal API.

[Michael Lippautz, 2016/10/29 07:39:19]

+1 for leaving now. This API is used by several consumers (internal and
external) and we have soft and hard limits in place that tell us when to do
which GC. So, unless you want to hunt down memory issues later on, just leave it
as is.

+    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 guard = FLAG_wasm_guard_pages;
+    buffer = NewArrayBuffer(isolate, new_size, guard);
+    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));
 }