[wasm] Refactor GrowMemory runtime call.

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 1738 matching lines @@
 
 void SetInstanceMemory(Handle<JSObject> instance, JSArrayBuffer* buffer) {
   DisallowHeapAllocation no_gc;
   DCHECK(IsWasmObject(*instance));
   instance->SetInternalField(kWasmMemArrayBuffer, buffer);
   WasmCompiledModule* module =
       WasmCompiledModule::cast(instance->GetInternalField(kWasmCompiledModule));
   module->set_ptr_to_heap(buffer);
 }
 
+int32_t GrowInstanceMemory(Isolate* isolate, Handle<JSObject> instance,
+                           uint32_t pages) {
+  Address old_mem_start, new_mem_start;

[Mircea Trofin, 2016/10/04 22:01:53]

could you please initialize all these and the uints below?

[gdeepti, 2016/10/05 02:59:57]

Done.

+  uint32_t old_size, new_size;
+
+  MaybeHandle<JSArrayBuffer> maybe_mem_buffer =
+      GetInstanceMemory(isolate, instance);
+  Handle<JSArrayBuffer> old_buffer;
+  if (!maybe_mem_buffer.ToHandle(&old_buffer)) {
+    // If module object does not have linear memory associated with it,
+    // Allocate new array buffer of given size.
+    old_mem_start = nullptr;
+    old_size = 0;
+    // TODO(gdeepti): Fix bounds check to take into account size of memtype.
+    new_size = pages * wasm::WasmModule::kPageSize;
+    // The code generated in the wasm compiler guarantees this precondition.
+    DCHECK(pages <= wasm::WasmModule::kMaxMemPages);
+    new_mem_start =
+        static_cast<Address>(isolate->array_buffer_allocator()->Allocate(
+            static_cast<uint32_t>(new_size)));
+    if (new_mem_start == NULL) {

[Mircea Trofin, 2016/10/04 22:01:53]

nullptr?

[gdeepti, 2016/10/05 02:59:56]

Used NewArrayBuffer method instead of allocating, that takes care of this check.

+      return -1;
+    }
+#if DEBUG
+    // Double check the API allocator actually zero-initialized the memory.

[Mircea Trofin, 2016/10/04 22:01:53]

could you encapsulate this and then reuse it below, too? e.g.

void ValidateAllocatorInitializedMemory(blah blah) {
#if DEBUG
  ...
#endif
}

[gdeepti, 2016/10/05 02:59:56]

Used NewArrayBuffer method instead which sets up a new array buffer and
validates that memory is initialized. 

+    for (size_t i = old_size; i < new_size; i++) {
+      DCHECK_EQ(0, new_mem_start[i]);
+    }
+#endif
+  } else {
+    old_mem_start = static_cast<Address>(old_buffer->backing_store());
+    old_size = old_buffer->byte_length()->Number();
+    // If the old memory was zero-sized, we should have been in the
+    // "undefined" case above.
+    DCHECK_NOT_NULL(old_mem_start);
+    DCHECK_NE(0, old_size);
+
+    new_size = old_size + pages * wasm::WasmModule::kPageSize;

[Mircea Trofin, 2016/10/04 22:01:53]

In addition to the check below, we should check that
std::numeric_limits<uint32_t> - old_size >= pages*wasm::WasmModule::kPageSize.

[gdeepti, 2016/10/05 02:59:57]

Done.

+    if (new_size >
+        wasm::WasmModule::kMaxMemPages * wasm::WasmModule::kPageSize) {
+      return -1;
+    }
+    new_mem_start =
+        static_cast<Address>(isolate->array_buffer_allocator()->Allocate(
+            static_cast<uint32_t>(new_size)));

[Mircea Trofin, 2016/10/04 22:01:53]

this static_cast is unnecessary, new_size is uint32_t already

[gdeepti, 2016/10/05 02:59:57]

Done.

+    if (new_mem_start == NULL) {

[Mircea Trofin, 2016/10/04 22:01:53]

nullptr

[gdeepti, 2016/10/05 02:59:57]

Used NewArrayBuffer method instead of allocating, that takes care of this check.

+      return -1;
+    }
+#if DEBUG
+    // Double check the API allocator actually zero-initialized the memory.
+    for (size_t i = old_size; i < new_size; i++) {
+      DCHECK_EQ(0, new_mem_start[i]);
+    }
+#endif
+    memcpy(new_mem_start, old_mem_start, old_size);

[Mircea Trofin, 2016/10/04 22:01:53]

we should check earlier that old_size <= new_size, otherwise this memcpy will
try copying more than it should.

[gdeepti, 2016/10/05 02:59:57]

Done.

+  }
+
+  Handle<JSArrayBuffer> buffer = isolate->factory()->NewJSArrayBuffer();
+  JSArrayBuffer::Setup(buffer, isolate, false, new_mem_start, new_size);
+  wasm::SetInstanceMemory(instance, *buffer);
+  if (!UpdateWasmModuleMemory(instance, old_mem_start, new_mem_start, old_size,
+                              new_size)) {
+    return -1;
+  }
+  return (old_size / WasmModule::kPageSize);

[Mircea Trofin, 2016/10/04 22:01:53]

A DCHECK here that old_size % WasmModule::kPageSize == 0?

[gdeepti, 2016/10/05 02:59:57]

Done.

+}
+
 namespace testing {
 
 void ValidateInstancesChain(Isolate* isolate, Handle<JSObject> module_obj,
                             int instance_count) {
   CHECK_GE(instance_count, 0);
   DisallowHeapAllocation no_gc;
   WasmCompiledModule* compiled_module =
       WasmCompiledModule::cast(module_obj->GetInternalField(0));
   CHECK_EQ(
       JSObject::cast(compiled_module->ptr_to_weak_module_object()->value()),
@@ skipping 34 matching lines @@
   WasmCompiledModule* compiled_module =
       WasmCompiledModule::cast(instance->GetInternalField(kWasmCompiledModule));
   CHECK(compiled_module->has_weak_module_object());
   CHECK(compiled_module->ptr_to_weak_module_object()->cleared());
 }
 
 }  // namespace testing
 }  // namespace wasm
 }  // namespace internal
 }  // namespace v8