[wasm] GrowMemory should use array_buffer_allocator instead of realloc.

src/runtime/runtime-wasm.cc

 // Copyright 2016 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 "src/runtime/runtime-utils.h"
 
 #include "src/arguments.h"
 #include "src/assembler.h"
 #include "src/compiler/wasm-compiler.h"
 #include "src/conversions.h"
@@ skipping 68 matching lines @@
     // 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 + delta_pages * wasm::WasmModule::kPageSize;
     if (new_size >
         wasm::WasmModule::kMaxMemPages * wasm::WasmModule::kPageSize) {
       return *isolate->factory()->NewNumberFromInt(-1);
     }
-    new_mem_start = static_cast<Address>(realloc(old_mem_start, new_size));
+    new_mem_start =
+        static_cast<Address>(isolate->array_buffer_allocator()->Allocate(
+            static_cast<uint32_t>(new_size)));
     if (new_mem_start == NULL) {
       return *isolate->factory()->NewNumberFromInt(-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
+    // Copy contents of the old buffer to the new buffer before detaching old
+    // buffer
+    memcpy(new_mem_start, old_mem_start, old_size);
     old_buffer->set_is_external(true);
     isolate->heap()->UnregisterArrayBuffer(*old_buffer);

[gdeepti, 2016/09/08 06:04:47]

Is this the right way to deal with the old buffer? My understanding is
unregistering the old_buffer will get the garbage collector to free the memory. 

[Michael Lippautz, 2016/09/08 09:43:31]

UnregisterArrayBuffer will make the GC stop tracking the backing store of this
JSArrayBuffer. So it's not that the buffer is freed but rather ownership is
transferred from the GC to the embedder.

The prime example is graphics code that works with JSArrayBuffer until a certain
point where they want to send it to the GPU without copy, so they unregister the
buffer, effectively taking over ownership (from the GC).

For wasm you have 2 choices:
- Don't externalize and UnregisterArrayBuffer (get rid of line 104 and 105). The
GC will collect the backing store for you in this case because you didn't
specify it as external on line 109.
- Add a array_buffer_allocator()->Free() after UnregisterArrayBuffer because
wasm is now the owner of the backing store.

[ahaas, 2016/09/08 12:23:33]

Hi Deepti, I talked with Ben about it, and we think that it would be best to go
for option 1, "don't externalize and UnregisterArrayBuffer".

[gdeepti, 2016/09/08 22:44:50]

Thanks for explaining that, I've removed the code that externalizes and uses
UnregisterArrayBuffer. 

-    // Zero initializing uninitialized memory from realloc
-    memset(new_mem_start + old_size, 0, new_size - old_size);
   }
 
   Handle<JSArrayBuffer> buffer = isolate->factory()->NewJSArrayBuffer();
   JSArrayBuffer::Setup(buffer, isolate, false, new_mem_start, new_size);
   buffer->set_is_neuterable(false);
 
   // Set new buffer to be wasm memory
   module_object->SetInternalField(kWasmMemArrayBuffer, *buffer);
-
   CHECK(wasm::UpdateWasmModuleMemory(module_object, old_mem_start,
                                      new_mem_start, old_size, new_size));
 
   return *isolate->factory()->NewNumberFromInt(old_size /
                                                wasm::WasmModule::kPageSize);
 }
 
 RUNTIME_FUNCTION(Runtime_WasmThrowTypeError) {
   HandleScope scope(isolate);
   DCHECK_EQ(0, args.length());
   THROW_NEW_ERROR_RETURN_FAILURE(
       isolate, NewTypeError(MessageTemplate::kWasmTrapTypeError));
 }
 }  // namespace internal
 }  // namespace v8